[AMD Official Use Only - General]
Hi Raphael:
-----Original Message----- From: Rafael J. Wysocki rafael@kernel.org Sent: Thursday, December 28, 2023 1:04 AM To: Meng, Li (Jassmine) Li.Meng@amd.com Cc: Rafael J. Wysocki rafael@kernel.org; Rafael J . Wysocki rafael.j.wysocki@intel.com; Huang, Ray Ray.Huang@amd.com; linux- pm@vger.kernel.org; linux-kernel@vger.kernel.org; x86@kernel.org; linux- acpi@vger.kernel.org; Shuah Khan skhan@linuxfoundation.org; linux- kselftest@vger.kernel.org; Fontenot, Nathan Nathan.Fontenot@amd.com; Sharma, Deepak Deepak.Sharma@amd.com; Deucher, Alexander Alexander.Deucher@amd.com; Limonciello, Mario Mario.Limonciello@amd.com; Huang, Shimmer Shimmer.Huang@amd.com; Yuan, Perry Perry.Yuan@amd.com; Du, Xiaojian Xiaojian.Du@amd.com; Viresh Kumar viresh.kumar@linaro.org; Borislav Petkov bp@alien8.de; Oleksandr Natalenko oleksandr@natalenko.name Subject: Re: [PATCH V12 4/7] cpufreq: Add a notification message that the highest perf has changed
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
On Wed, Dec 27, 2023 at 2:40 AM Meng, Li (Jassmine) Li.Meng@amd.com wrote:
[AMD Official Use Only - General]
Hi Rafael:
-----Original Message----- From: Meng, Li (Jassmine) Sent: Tuesday, December 26, 2023 4:27 PM To: Rafael J. Wysocki rafael@kernel.org Cc: Rafael J . Wysocki rafael.j.wysocki@intel.com; Huang, Ray Ray.Huang@amd.com; linux-pm@vger.kernel.org; linux- kernel@vger.kernel.org; x86@kernel.org; linux-acpi@vger.kernel.org; Shuah Khan skhan@linuxfoundation.org; linux-kselftest@vger.kernel.org; Fontenot, Nathan Nathan.Fontenot@amd.com; Sharma, Deepak
Deucher, Alexander Alexander.Deucher@amd.com; Limonciello, Mario Mario.Limonciello@amd.com; Huang, Shimmer
Yuan, Perry Perry.Yuan@amd.com; Du, Xiaojian Xiaojian.Du@amd.com; Viresh Kumar viresh.kumar@linaro.org; Borislav Petkov bp@alien8.de; Oleksandr Natalenko oleksandr@natalenko.name Subject: RE: [PATCH V12 4/7] cpufreq: Add a notification message that the highest perf has changed
Hi Rafael:
-----Original Message----- From: Rafael J. Wysocki rafael@kernel.org Sent: Tuesday, December 12, 2023 9:44 PM To: Meng, Li (Jassmine) Li.Meng@amd.com Cc: Rafael J . Wysocki rafael.j.wysocki@intel.com; Huang, Ray Ray.Huang@amd.com; linux-pm@vger.kernel.org; linux- kernel@vger.kernel.org; x86@kernel.org; linux-acpi@vger.kernel.org; Shuah Khan skhan@linuxfoundation.org; linux-kselftest@vger.kernel.org; Fontenot, Nathan Nathan.Fontenot@amd.com; Sharma, Deepak
Deucher, Alexander Alexander.Deucher@amd.com; Limonciello,
Mario
Mario.Limonciello@amd.com; Huang, Shimmer
Yuan, Perry Perry.Yuan@amd.com; Du, Xiaojian
Viresh Kumar viresh.kumar@linaro.org; Borislav Petkov bp@alien8.de; Oleksandr Natalenko oleksandr@natalenko.name Subject: Re: [PATCH V12 4/7] cpufreq: Add a notification message that the highest perf has changed
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
On Wed, Dec 6, 2023 at 10:13 PM Rafael J. Wysocki rafael@kernel.org wrote:
On Wed, Dec 6, 2023 at 9:58 PM Rafael J. Wysocki rafael@kernel.org
wrote:
On Tue, Dec 5, 2023 at 7:38 AM Meng Li li.meng@amd.com wrote: > > ACPI 6.5 section 8.4.6.1.1.1 specifies that Notify event > 0x85 can be emmitted to cause the the OSPM to re-evaluate > the highest performance
Typos above. Given the number of iterations of this patch, this is kind of disappointing.
> register. Add support for this event.
Also it would be nice to describe how this is supposed to work at least roughly, so it is not necessary to reverse-engineer the patch to find out that.
> Tested-by: Oleksandr Natalenko oleksandr@natalenko.name > Reviewed-by: Mario Limonciello mario.limonciello@amd.com > Reviewed-by: Huang Rui ray.huang@amd.com > Reviewed-by: Perry Yuan perry.yuan@amd.com > Signed-off-by: Meng Li li.meng@amd.com > Link: >
https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model
> .html#processor-device-notification-values > --- > drivers/acpi/processor_driver.c | 6 ++++++ > drivers/cpufreq/cpufreq.c | 13 +++++++++++++ > include/linux/cpufreq.h | 5 +++++ > 3 files changed, 24 insertions(+) > > diff --git a/drivers/acpi/processor_driver.c > b/drivers/acpi/processor_driver.c index > 4bd16b3f0781..29b2fb68a35d > 100644 > --- a/drivers/acpi/processor_driver.c > +++ b/drivers/acpi/processor_driver.c > @@ -27,6 +27,7 @@ > #define ACPI_PROCESSOR_NOTIFY_PERFORMANCE 0x80 > #define ACPI_PROCESSOR_NOTIFY_POWER 0x81 > #define ACPI_PROCESSOR_NOTIFY_THROTTLING 0x82 > +#define ACPI_PROCESSOR_NOTIFY_HIGEST_PERF_CHANGED
0x85
> > MODULE_AUTHOR("Paul Diefenbaugh");
MODULE_DESCRIPTION("ACPI
> Processor Driver"); @@ -83,6 +84,11 @@ static void > acpi_processor_notify(acpi_handle handle, u32 event, void *data) > acpi_bus_generate_netlink_event(device-
pnp.device_class,
> dev_name(&device->dev), event, 0); > break; > + case ACPI_PROCESSOR_NOTIFY_HIGEST_PERF_CHANGED: > + cpufreq_update_highest_perf(pr->id);
And the design appears to be a bit ad-hoc here.
Because why does it have anything to do with cpufreq?
Well, clearly, cpufreq can be affected by this, but why would it be not affected the same way as in the
ACPI_PROCESSOR_NOTIFY_PERFORMANCE
case?
That is, why isn't cpufreq_update_limits() the right thing to do?
Seriously, I'm not going to apply this patch so long as my comments above are not addressed.
[Meng, Li (Jassmine)] Sorry for the delayed reply to the email. BIOS/AGESA is responsible to issue the Notify 0x85 to OS that the preferred core has changed. It will only affect the ranking of the preferred core, not the impact policy limits. AMD P-state driver will set the priority of the cores based on the preferred core ranking, and prioritize selecting higher priority core to run
the task.
[Meng, Li (Jassmine)] From ACPI v6.5, Table 5.197 Processor Device Notification Values: Hex value Description 0x80 Performance Present Capabilities Changed. Used to notify
OSPM that the number of supported processor performance states has changed. This notification causes OSPM to re-evaluate the _PPC object. See Section 8.4.5.3 for more information.
0x85 Highest Performance Changed. Used to notify OSPM that
the value of the CPPC Highest Performance Register has changed.
I think they are different notify events, so they need different functions to
handle these events.
But they effectively mean pretty much the same thing: the highest available performance state of the CPU has changed.
Why would the response need to be different?
[Meng, Li (Jassmine)] Thanks, I will modify this issue.