Messages in this thread | ![/](/images/icornerl.gif) | | Date | Mon, 31 Aug 2020 16:56:38 +0530 | From | Viresh Kumar <> | Subject | Re: [RFC 0/3] cpufreq: cppc: Add support for frequency invariance |
| |
On 27-08-20, 12:27, Ionela Voinescu wrote: > I don't see it as anyone registering for freq invariance, rather the > freq invariance framework chooses its source of information (AMU, CPPC, > cpufreq).
Yeah, either way is fine for me.
> > i.e. if CPPC registers for it first then there is no need to check > > AMUs further (as CPPC will be using AMUs anyway), else we will > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > Not necessarily. Even if AMUs are present, they are only used for CPPC's > delivered and reference performance counters if the ACPI _CPC entry > specifies FFH as method: > > ResourceTemplate(){Register(FFixedHW, 0x40, 0, 1, 0x4)}, > ResourceTemplate(){Register(FFixedHW, 0x40, 0, 0, 0x4)},
Right.
> While I understand your point (accessing AMUs through CPPC's read > functions to implement invariance) I don't think it's worth tying the > two together. > > I see the two functionalities as independent: > - frequency invariance with whichever source of information is valid > (AMUs, cpufreq, etc) is separate from > - CPPC's delivered and reference performance counters, which currently > are used in cpufreq's .get() function. > > Therefore, taking each of the scenarios one by one: > - All CPUs support AMUs: the freq invariance initialisation code will > find AMUs valid and it will use them to set the scale factor; > completely independently, if the FFH method is specified for CPPC's > delivered and reference performance counters, it will also use > AMUs, even if, let's say, invariance is disabled. > > - None of the CPUs support AMUs, but the _CPC entry specifies some > platform specific counters for delivered and reference performance. > With the current mainline code neither cpufreq or counter based > invariance is supported, but the CPPC counters can be used in the > cppc_cpufreq driver for the .get() function. > > But with the above new functionality we can detect that AMUs are not > supported and expose the CPPC counters to replace them in > implementing invariance. > > - Mixed scenarios are also supported if we play our cards right and > implement the above per-cpu. > > > I'm thinking that having some well defined invariance sources might work > well: it will simplify the init function (go through all registered > sources and choose (per-cpu) the one that's valid) and allow for > otherwise generic invariance support. Something like: > > enum freq_inv_source {INV_CPUFREQ, INV_AMU_COUNTERS, INV_CPPC_COUNTERS}; > > struct freq_inv_source { > enum freq_inv_source source; > bool (*valid)(int cpu); > u64 (*read_corecnt)(int cpu); > u64 (*read_constcnt)(int cpu); > u64 (*max_rate)(int cpu); > u64 (*ref_rate)(int cpu); > } > > I am in the middle of unifying AMU counter and cpufreq invariance through > something like this, so if you like the idea and you don't think I'm > stepping too much on your toes with this, I can consider the usecase in > my (what should be) generic support. So in the end this might end up > being just a matter of adding a new invariance source (CPPC counters).
Okay, if you have already started working on that, no issues from my side. I can just get the relevant stuff from CPPC added once you provide that layer..
> My only worry is that while I know how a cpufreq source behaves and how > AMU counters behave, I'm not entirely sure what to expect from CPPC
Neither do I :)
> counters: if they are always appropriate for updates on the tick (not > blocking),
The update stuff may sleep here and so I had to do stuff in the irq-work handler in my patch.
> if they both stop during idle, if there is save/restore > functionality before/after idle, etc.
This I will check.
-- viresh
| ![\](/images/icornerr.gif) |