[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0ir-V5ppRuw5q7PsZK4rsuvh7y4a3HNunJhovvurGh3hg@mail.gmail.com>
Date: Thu, 28 Mar 2024 12:26:17 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: srinivas pandruvada <srinivas.pandruvada@...ux.intel.com>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>, Linux PM <linux-pm@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 6/6] cpufreq: intel_pstate: Update the maximum CPU
frequency consistently
On Wed, Mar 27, 2024 at 7:08 PM srinivas pandruvada
<srinivas.pandruvada@...ux.intel.com> wrote:
>
> On Mon, 2024-03-25 at 18:06 +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >
> > There are 3 places at which the maximum CPU frequency may change,
> > store_no_turbo(), intel_pstate_update_limits() (when called by the
> > cpufreq core) and intel_pstate_notify_work() (when handling a HWP
> > change notification). Currently, cpuinfo.max_freq is only updated by
> > store_no_turbo(), although it principle it may be necessary to update
> > it at the other 2 places too.
>
> It also works for intel_pstate_notify_work() path as is without this
> change.
>
> To start with:
>
> $ sudo rdmsr 0x771
> 6080c14
> $ cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_*
> 2000000
> 800000
> 0
>
> Now trigger a max frequency change via SST. intel_pstate_notify_work()
> called because guaranteed also changed. We didn't subscribe for max
> change only (although we should).
>
> Max changed from 2GHz to 1.9 GHz.
>
> $ sudo rdmsr 0x771
> 6080e13
> [labuser@...-bkc ~]$ cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_*
> 1900000
> 800000
> 0
>
> Now trigger SST to change to max frequency to 2GHz.
>
> sudo rdmsr 0x771
> 6080c14
>
> cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_*
> 2000000
> 800000
> 0
>
> May be you mean something else.
No, I don't, and you are right.
When I was writing the changelog, I somehow forgot that
intel_pstate_notify_work() called __intel_pstate_update_max_freq(),
even though the code changes in the patch obviously take that into
account (I can't really explain what happened :-/).
I'll fix the changelog.
Cheers,
Rafael
Powered by blists - more mailing lists