Re: [PATCH 1/3] cpufreq: Add a callback to update the min_freq_req from drivers

From: Rafael J. Wysocki
Date: Thu Oct 10 2024 - 13:57:41 EST


Hi,

On Tue, Oct 8, 2024 at 8:32 AM Dhananjay Ugwekar
<Dhananjay.Ugwekar@xxxxxxx> wrote:
>
> Hello Rafael,
>
> On 10/7/2024 9:18 PM, Rafael J. Wysocki wrote:
> > On Mon, Oct 7, 2024 at 5:46 PM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
> >>
> >> Hi,
> >>
> >> On Mon, Oct 7, 2024 at 6:40 AM Dhananjay Ugwekar
> >> <Dhananjay.Ugwekar@xxxxxxx> wrote:
> >>>
> >>> Hello Rafael,
> >>>
> >>> On 10/4/2024 11:47 PM, Rafael J. Wysocki wrote:
> >>>> On Thu, Oct 3, 2024 at 10:44 AM Dhananjay Ugwekar
> >>>> <Dhananjay.Ugwekar@xxxxxxx> wrote:
> >>>>>
> >>>>> Currently, there is no proper way to update the initial lower frequency
> >>>>> limit from cpufreq drivers.
> >>>>
> >>>> Why do you want to do it?
> >>>
> >>> We want to set the initial lower frequency limit at a more efficient level
> >>> (lowest_nonlinear_freq) than the lowest frequency, which helps save power in
> >>> some idle scenarios, and also improves benchmark results in some scenarios.
> >>> At the same time, we want to allow the user to set the lower limit back to
> >>> the inefficient lowest frequency.
> >>
> >> So you want the default value of scaling_min_freq to be greater than
> >> the total floor.
>
> Yes, we want to set the default min value to what we think is best for the platform.
>
> >>
> >> I have to say that I'm not particularly fond of this approach because
> >> it is adding a new meaning to scaling_min_freq: Setting it below the
> >> default would not cause the driver to use inefficient frequencies
> >
> > s/not/now/ (sorry)
>
> I believe we are not changing the meaning of the scaling_min_freq just setting it
> to the best value at boot and then allowing the user to have access to the entire
> frequency range for the platform. Also, we have cpuinfo_min_freq/max_freq to
> indicate to the user as to what the entire frequency range is for the platform
> (depending on boost enabled/disabled).

But the default you want to set is the lowest efficient frequency
which user space needs to be aware of. Otherwise it may make
suboptimal decisions.

> >
> > I should have double checked this before sending.
> >
> >> which user space may not be aware of.
>
> I guess, this part we can fix by documenting correctly ?
>
> >> Moreover, it would tell the
> >> driver how far it could go with that.
>
> Sorry, I didnt understand this part.

Sorry, never mind. I was just repeating myself.

> >>
> >> IMV it would be bettwr to have a separate interface for this kind of tuning.
>
> I feel like we can incorporate this change cleanly enough into scaling_min_freq,
> without adding a new interface which might further confuse the user. But please
> let me know your concerns and thoughts.

I'm not sure if you realize that the .show() operation for
scaling_min_freq prints policy->max and you can easily make your
driver's .verify() callback change it to whatever value is desired.
You may as well set it to the lowest efficient frequency if the one
passed to you is equal to FREQ_QOS_MIN_DEFAULT_VALUE.