Re: [PATCH v2 2/2] cpufreq: CPPC: Dont read counters for idle CPUs

[Rafael J. Wysocki]

On Tue, Jul 15, 2025 at 8:28 AM Prashant Malani <pmalani@xxxxxxxxxx> wrote:
>
> +Sudeep.
>
> On Mon, 14 Jul 2025 at 02:31, Beata Michalska <beata.michalska@xxxxxxx> wrote:
> > So I believe this should be handled in CPUFreq core, if at all.
> > Would be good to get an input/opinion from the maintainers: Viresh and Rafael.
>
> Viresh, Rafael, Sudeep, could you kindly chime in? The unreliability
> of this frequency
> measurement method in CPPC is affecting the cached frequency saved by CPUFreq,
> which in turn affects future frequency set calls.

I gather that "the cached frequency saved by CPUFreq" means policy->cur.

> It would be great if we could solve this in CPUFreq core (maybe not
> rely on the cached frequency while setting the new one [3]?)

I see what you mean now.

Why don't you flag the driver as CPUFREQ_NEED_UPDATE_LIMITS?

That would kind of make sense given how the driver works overall, or
am I missing anything?

> >
> > In the meantime ....
> > It seems that the issue of getting counters on a CPU that is idle is not
> > in the counters themselves, but in the way how they are being read - at least
> > from what I can observe.
> > The first read experience longer delay between reading core and const counters,
> > and as const one is read as a second one, it misses some increments (within
> > calculated delta). So, what we could do within the driver is either:
> > - Add a way to request reading both counters in a single cpc_read (preferable
> >   I guess, though I would have to have a closer look at that)
>
> I already tried something like this; I used [1] which basically puts the
> 2 (constcnt, corecnt) register reads in a single CPC call;
> This did not help. The values are still highly variable. I never got
> merged FWIW.
>
> > - Add some logic that would make sure the reads are not far apart
>
> As Jie pointed out earlier, a lot of this has been discussed (see the references
> within the patch link [2]), so I'm not really sure what else can done
> to reduce this on
> Linux; there are two registers (SYS_AMEVCNTR0_CORE_EL0 &
> SYS_AMEVCNTR0_CORE_EL0), so there will always be some scheduler induced
> variability between the two reads.
>
> Best regards,
>
> [1] https://patchew.org/linux/20240229162520.970986-1-vanshikonda@xxxxxxxxxxxxxxxxxxxxxx/20240229162520.970986-4-vanshikonda@xxxxxxxxxxxxxxxxxxxxxx/
> [2] https://lore.kernel.org/linux-pm/20250131162439.3843071-1-beata.michalska@xxxxxxx/
> [3] https://elixir.bootlin.com/linux/v6.15.6/source/drivers/cpufreq/cpufreq.c#L2415
>
> --
> -Prashant
>