RE: [PATCH V12 4/7] cpufreq: Add a notification message that the highest perf has changed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[AMD Official Use Only - General]

Hi Raphael:

> -----Original Message-----
> From: Rafael J. Wysocki <rafael@xxxxxxxxxx>
> Sent: Thursday, December 28, 2023 1:04 AM
> To: Meng, Li (Jassmine) <Li.Meng@xxxxxxx>
> Cc: Rafael J. Wysocki <rafael@xxxxxxxxxx>; Rafael J . Wysocki
> <rafael.j.wysocki@xxxxxxxxx>; Huang, Ray <Ray.Huang@xxxxxxx>; linux-
> pm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; x86@xxxxxxxxxx; linux-
> acpi@xxxxxxxxxxxxxxx; Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>; linux-
> kselftest@xxxxxxxxxxxxxxx; Fontenot, Nathan
> <Nathan.Fontenot@xxxxxxx>; Sharma, Deepak
> <Deepak.Sharma@xxxxxxx>; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; Limonciello, Mario
> <Mario.Limonciello@xxxxxxx>; Huang, Shimmer
> <Shimmer.Huang@xxxxxxx>; Yuan, Perry <Perry.Yuan@xxxxxxx>; Du,
> Xiaojian <Xiaojian.Du@xxxxxxx>; Viresh Kumar <viresh.kumar@xxxxxxxxxx>;
> Borislav Petkov <bp@xxxxxxxxx>; Oleksandr Natalenko
> <oleksandr@xxxxxxxxxxxxxx>
> 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@xxxxxxx>
> 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@xxxxxxxxxx>
> > > Cc: Rafael J . Wysocki <rafael.j.wysocki@xxxxxxxxx>; Huang, Ray
> > > <Ray.Huang@xxxxxxx>; linux-pm@xxxxxxxxxxxxxxx; linux-
> > > kernel@xxxxxxxxxxxxxxx; x86@xxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx;
> > > Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>;
> > > linux-kselftest@xxxxxxxxxxxxxxx; Fontenot, Nathan
> > > <Nathan.Fontenot@xxxxxxx>; Sharma, Deepak
> <Deepak.Sharma@xxxxxxx>;
> > > Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Limonciello, Mario
> > > <Mario.Limonciello@xxxxxxx>; Huang, Shimmer
> <Shimmer.Huang@xxxxxxx>;
> > > Yuan, Perry <Perry.Yuan@xxxxxxx>; Du, Xiaojian
> > > <Xiaojian.Du@xxxxxxx>; Viresh Kumar <viresh.kumar@xxxxxxxxxx>;
> > > Borislav Petkov <bp@xxxxxxxxx>; Oleksandr Natalenko
> > > <oleksandr@xxxxxxxxxxxxxx>
> > > 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@xxxxxxxxxx>
> > > > Sent: Tuesday, December 12, 2023 9:44 PM
> > > > To: Meng, Li (Jassmine) <Li.Meng@xxxxxxx>
> > > > Cc: Rafael J . Wysocki <rafael.j.wysocki@xxxxxxxxx>; Huang, Ray
> > > > <Ray.Huang@xxxxxxx>; linux-pm@xxxxxxxxxxxxxxx; linux-
> > > > kernel@xxxxxxxxxxxxxxx; x86@xxxxxxxxxx;
> > > > linux-acpi@xxxxxxxxxxxxxxx; Shuah Khan
> > > > <skhan@xxxxxxxxxxxxxxxxxxx>; linux-kselftest@xxxxxxxxxxxxxxx;
> > > > Fontenot, Nathan <Nathan.Fontenot@xxxxxxx>; Sharma, Deepak
> > > <Deepak.Sharma@xxxxxxx>;
> > > > Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Limonciello,
> Mario
> > > > <Mario.Limonciello@xxxxxxx>; Huang, Shimmer
> > > <Shimmer.Huang@xxxxxxx>;
> > > > Yuan, Perry <Perry.Yuan@xxxxxxx>; Du, Xiaojian
> > > <Xiaojian.Du@xxxxxxx>;
> > > > Viresh Kumar <viresh.kumar@xxxxxxxxxx>; Borislav Petkov
> > > > <bp@xxxxxxxxx>; Oleksandr Natalenko <oleksandr@xxxxxxxxxxxxxx>
> > > > 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@xxxxxxxxxx>
> > > > wrote:
> > > > >
> > > > > On Wed, Dec 6, 2023 at 9:58 PM Rafael J. Wysocki
> > > > > <rafael@xxxxxxxxxx>
> > > > wrote:
> > > > > >
> > > > > > On Tue, Dec 5, 2023 at 7:38 AM Meng Li <li.meng@xxxxxxx> 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@xxxxxxxxxxxxxx>
> > > > > > > Reviewed-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> > > > > > > Reviewed-by: Huang Rui <ray.huang@xxxxxxx>
> > > > > > > Reviewed-by: Perry Yuan <perry.yuan@xxxxxxx>
> > > > > > > Signed-off-by: Meng Li <li.meng@xxxxxxx>
> > > > > > > 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.




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux