lkml.org 
[lkml]   [2022]   [Dec]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2 1/3] ACPI: processor: perflib: Use the "no limit" frequency QoS
Date
Hi Rafael,

On Wed, Dec 28 2022, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> When _PPC returns 0, it means that the CPU frequency is not limited by
> the platform firmware, so make acpi_processor_get_platform_limit()
> update the frequency QoS request used by it to "no limit" in that case.
>
> This addresses a problem with limiting CPU frequency artificially on
> some systems after CPU offline/online to the frequency that corresponds
> to the first entry in the _PSS return package.
>
> Reported-by: Pratyush Yadav <ptyadav@amazon.de>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> v1 -> v2:
> * Move some changes into a separate patch
> * Update the changelog accordingly
>
> ---
> drivers/acpi/processor_perflib.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> Index: linux-pm/drivers/acpi/processor_perflib.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/processor_perflib.c
> +++ linux-pm/drivers/acpi/processor_perflib.c
> @@ -53,6 +53,8 @@ static int acpi_processor_get_platform_l
> {
> acpi_status status = 0;
> unsigned long long ppc = 0;
> + s32 qos_value;
> + int index;
> int ret;
>
> if (!pr)
> @@ -72,17 +74,27 @@ static int acpi_processor_get_platform_l
> }
> }
>
> + index = ppc;
> +
> pr_debug("CPU %d: _PPC is %d - frequency %s limited\n", pr->id,
> - (int)ppc, ppc ? "" : "not");
> + index, index ? "is" : "is not");
>
> - pr->performance_platform_limit = (int)ppc;
> + pr->performance_platform_limit = index;
>
> if (ppc >= pr->performance->state_count ||
> unlikely(!freq_qos_request_active(&pr->perflib_req)))
> return 0;
>
> - ret = freq_qos_update_request(&pr->perflib_req,
> - pr->performance->states[ppc].core_frequency * 1000);
> + /*
> + * If _PPC returns 0, it means that all of the available states can be
> + * used ("no limit").
> + */
> + if (index == 0)
> + qos_value = FREQ_QOS_MAX_DEFAULT_VALUE;

One small thing I noticed: in acpi_processor_ppc_init() "no limit" value
is set to INT_MAX and here it is set to FREQ_QOS_MAX_DEFAULT_VALUE. Both
should evaluate to the same value but I think it would be nice if the
same thing is used in both places. Perhaps you can fix that up when
applying?

Other than this,

Reviewed-by: Pratyush Yadav <ptyadav@amazon.de>
Tested-by: Pratyush Yadav <ptyadav@amazon.de>

Thanks for working on this.

> + else
> + qos_value = pr->performance->states[index].core_frequency * 1000;
> +
> + ret = freq_qos_update_request(&pr->perflib_req, qos_value);
> if (ret < 0) {
> pr_warn("Failed to update perflib freq constraint: CPU%d (%d)\n",
> pr->id, ret);
>
>
>

--
Regards,
Pratyush Yadav



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879



\
 
 \ /
  Last update: 2023-03-26 23:22    [W:0.091 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site