From: Rafael J. Wysocki <[email protected]>
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
and avoid updating the QoS request when the _PPC return value has not
changed.
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.
While at it, move the _PPC return value check against the state count
earlier to avoid setting performance_platform_limit to an invalid value.
Reported-by: Pratyush Yadav <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/processor_perflib.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 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,30 @@ static int acpi_processor_get_platform_l
}
}
+ index = ppc;
+
+ if (pr->performance_platform_limit == index ||
+ ppc >= pr->performance->state_count)
+ return 0;
+
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)))
+ if (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;
+ 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);
From: Rafael J. Wysocki <[email protected]>
After making acpi_processor_get_platform_limit() use the "no limit"
value for its frequency QoS request when _PPC returns 0, it is not
necessary to replace the frequency corresponding to the first _PSS
return package entry with the maximum turbo frequency of the given
CPU in intel_pstate_init_acpi_perf_limits() any more, so drop the
code doing that along with the comment explaining it.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/cpufreq/intel_pstate.c | 14 --------------
1 file changed, 14 deletions(-)
Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -452,20 +452,6 @@ static void intel_pstate_init_acpi_perf_
(u32) cpu->acpi_perf_data.states[i].control);
}
- /*
- * The _PSS table doesn't contain whole turbo frequency range.
- * This just contains +1 MHZ above the max non turbo frequency,
- * with control value corresponding to max turbo ratio. But
- * when cpufreq set policy is called, it will call with this
- * max frequency, which will cause a reduced performance as
- * this driver uses real max turbo frequency as the max
- * frequency. So correct this frequency in _PSS table to
- * correct max turbo frequency based on the turbo state.
- * Also need to convert to MHz as _PSS freq is in MHz.
- */
- if (!global.turbo_disabled)
- cpu->acpi_perf_data.states[0].core_frequency =
- policy->cpuinfo.max_freq / 1000;
cpu->valid_pss_table = true;
pr_debug("_PPC limits will be enforced\n");
On Tue, 2022-12-27 at 20:51 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> 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
> and avoid updating the QoS request when the _PPC return value has not
> changed.
>
> 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.
>
> While at it, move the _PPC return value check against the state count
> earlier to avoid setting performance_platform_limit to an invalid
> value.
>
> Reported-by: Pratyush Yadav <[email protected]>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/acpi/processor_perflib.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 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,30 @@ static int acpi_processor_get_platform_l
> }
> }
>
> + index = ppc;
> +
> + if (pr->performance_platform_limit == index ||
> + ppc >= pr->performance->state_count)
> + return 0;
Do we need to re initialize pr->performance_platform_limit to 0 in
acpi_processor_unregister_performance()?
If PPC was 1 before the offline and after online the above check will
cause it to return as the pr->performance_platform_limit is not
changed. Not sure if the PM QOS state is preserved after offline and
online. This is stored in a per CPU variable, not in dynamically
allocated memory which will be reallocated during online again.
Thanks,
Srinivas
> +
> 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)))
> + if (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;
> + 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);
>
>
>
On Tue, Dec 27 2022, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> 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
> and avoid updating the QoS request when the _PPC return value has not
> changed.
>
> 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.
>
> While at it, move the _PPC return value check against the state count
> earlier to avoid setting performance_platform_limit to an invalid value.
>
> Reported-by: Pratyush Yadav <[email protected]>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
Tested-by: Pratyush Yadav <[email protected]>
--
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
On Tue, Dec 27, 2022 at 9:55 PM srinivas pandruvada
<[email protected]> wrote:
>
> On Tue, 2022-12-27 at 20:51 +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > 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
> > and avoid updating the QoS request when the _PPC return value has not
> > changed.
> >
> > 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.
> >
> > While at it, move the _PPC return value check against the state count
> > earlier to avoid setting performance_platform_limit to an invalid
> > value.
> >
> > Reported-by: Pratyush Yadav <[email protected]>
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/acpi/processor_perflib.c | 27 +++++++++++++++++++++------
> > 1 file changed, 21 insertions(+), 6 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,30 @@ static int acpi_processor_get_platform_l
> > }
> > }
> >
> > + index = ppc;
> > +
> > + if (pr->performance_platform_limit == index ||
> > + ppc >= pr->performance->state_count)
> > + return 0;
>
> Do we need to re initialize pr->performance_platform_limit to 0 in
> acpi_processor_unregister_performance()?
>
> If PPC was 1 before the offline and after online the above check will
> cause it to return as the pr->performance_platform_limit is not
> changed. Not sure if the PM QOS state is preserved after offline and
> online. This is stored in a per CPU variable, not in dynamically
> allocated memory which will be reallocated during online again.
Good point in general, but the QoS request is tied to the cpufreq
policy, so it is not freed on offline.
However, if the policy goes away and is created again for the same CPU
(like when the intel_pstate mode is changed via its 'status' attribute
in sysfs), there may be a stale value in performance_platform_limit,
so it needs to be cleared in acpi_processor_ppc_init() when the QoS
request is first set to "no limit".
I'll update the patch accordingly. I think I'll also split it in two
to avoid making too many changes in one go.
From: Rafael J. Wysocki <[email protected]>
After making acpi_processor_get_platform_limit() use the "no limit"
value for its frequency QoS request when _PPC returns 0, it is not
necessary to replace the frequency corresponding to the first _PSS
return package entry with the maximum turbo frequency of the given
CPU in intel_pstate_init_acpi_perf_limits() any more, so drop the
code doing that along with the comment explaining it.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
v1 -> v2:
* Same as the previous [2/2], no changes.
---
drivers/cpufreq/intel_pstate.c | 14 --------------
1 file changed, 14 deletions(-)
Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -452,20 +452,6 @@ static void intel_pstate_init_acpi_perf_
(u32) cpu->acpi_perf_data.states[i].control);
}
- /*
- * The _PSS table doesn't contain whole turbo frequency range.
- * This just contains +1 MHZ above the max non turbo frequency,
- * with control value corresponding to max turbo ratio. But
- * when cpufreq set policy is called, it will call with this
- * max frequency, which will cause a reduced performance as
- * this driver uses real max turbo frequency as the max
- * frequency. So correct this frequency in _PSS table to
- * correct max turbo frequency based on the turbo state.
- * Also need to convert to MHz as _PSS freq is in MHz.
- */
- if (!global.turbo_disabled)
- cpu->acpi_perf_data.states[0].core_frequency =
- policy->cpuinfo.max_freq / 1000;
cpu->valid_pss_table = true;
pr_debug("_PPC limits will be enforced\n");
From: Rafael J. Wysocki <[email protected]>
Modify acpi_processor_get_platform_limit() to avoid updating its
frequency QoS request when the _PPC return value has not changed
by comparing that value to the previous _PPC return value stored in
the performance_platform_limit field of the struct acpi_processor
corresponding to the given CPU.
While at it, do the _PPC return value check against the state count
earlier to avoid setting performance_platform_limit to an invalid value.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
v1 -> v2:
* New patch.
* Contains some changes from the previous [1/2] and a change to reset
performance_platform_limit on QoS request creation.
---
drivers/acpi/processor_perflib.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
Index: linux-pm/drivers/acpi/processor_perflib.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_perflib.c
+++ linux-pm/drivers/acpi/processor_perflib.c
@@ -76,13 +76,16 @@ static int acpi_processor_get_platform_l
index = ppc;
+ if (pr->performance_platform_limit == index ||
+ ppc >= pr->performance->state_count)
+ return 0;
+
pr_debug("CPU %d: _PPC is %d - frequency %s limited\n", pr->id,
index, index ? "is" : "is not");
pr->performance_platform_limit = index;
- if (ppc >= pr->performance->state_count ||
- unlikely(!freq_qos_request_active(&pr->perflib_req)))
+ if (unlikely(!freq_qos_request_active(&pr->perflib_req)))
return 0;
/*
@@ -178,6 +181,13 @@ void acpi_processor_ppc_init(struct cpuf
if (!pr)
continue;
+ /*
+ * Reset performance_platform_limit in case there is a stale
+ * value in it, so as to make it match the "no limit" QoS value
+ * below.
+ */
+ pr->performance_platform_limit = 0;
+
ret = freq_qos_add_request(&policy->constraints,
&pr->perflib_req,
FREQ_QOS_MAX, INT_MAX);
From: Rafael J. Wysocki <[email protected]>
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 <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
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;
+ 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);
Hi Rafael,
On Wed, Dec 28 2022, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
>
> 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 <[email protected]>
Tested-by: Pratyush Yadav <[email protected]>
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
On Thu, Dec 29, 2022 at 1:58 PM Pratyush Yadav <[email protected]> wrote:
>
> Hi Rafael,
>
> On Wed, Dec 28 2022, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> >
> > 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?
Yes, I'll do that.
> Other than this,
>
> Reviewed-by: Pratyush Yadav <[email protected]>
> Tested-by: Pratyush Yadav <[email protected]>
Thanks!
> Thanks for working on this.
You're welcome.
Hi Rafael,
On Thu, Dec 29 2022, Rafael J. Wysocki wrote:
> On Thu, Dec 29, 2022 at 1:58 PM Pratyush Yadav <[email protected]> wrote:
>>
>> Hi Rafael,
>>
>> On Wed, Dec 28 2022, Rafael J. Wysocki wrote:
>> > From: Rafael J. Wysocki <[email protected]>
>> >
>> > 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 <[email protected]>
>> > Signed-off-by: Rafael J. Wysocki <[email protected]>
>> > ---
[...]
>>
>> 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?
>
> Yes, I'll do that.
Following up on this series. I do not see it queued anywhere in the
linux-pm [0] tree. I would like to have this in the v6.3 merge window if
possible.
[0] https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/
--
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
On Mon, Jan 30, 2023 at 3:18 PM Pratyush Yadav <[email protected]> wrote:
>
> Hi Rafael,
>
> On Thu, Dec 29 2022, Rafael J. Wysocki wrote:
>
> > On Thu, Dec 29, 2022 at 1:58 PM Pratyush Yadav <[email protected]> wrote:
> >>
> >> Hi Rafael,
> >>
> >> On Wed, Dec 28 2022, Rafael J. Wysocki wrote:
> >> > From: Rafael J. Wysocki <[email protected]>
> >> >
> >> > 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 <[email protected]>
> >> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> >> > ---
> [...]
> >>
> >> 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?
> >
> > Yes, I'll do that.
>
> Following up on this series. I do not see it queued anywhere in the
> linux-pm [0] tree. I would like to have this in the v6.3 merge window if
> possible.
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/
It's already in the mainline:
e8a0e30b742f cpufreq: intel_pstate: Drop ACPI _PSS states table patching
99387b016022 ACPI: processor: perflib: Avoid updating frequency QoS
unnecessarily
c02d5feb6e2f ACPI: processor: perflib: Use the "no limit" frequency QoS
Hi Rafael,
On Mon, Jan 30 2023, Rafael J. Wysocki wrote:
> On Mon, Jan 30, 2023 at 3:18 PM Pratyush Yadav <[email protected]> wrote:
>>
>> Hi Rafael,
>>
>> On Thu, Dec 29 2022, Rafael J. Wysocki wrote:
>>
>> > On Thu, Dec 29, 2022 at 1:58 PM Pratyush Yadav <[email protected]> wrote:
>> >>
>> >> Hi Rafael,
>> >>
>> >> On Wed, Dec 28 2022, Rafael J. Wysocki wrote:
>> >> > From: Rafael J. Wysocki <[email protected]>
>> >> >
>> >> > 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 <[email protected]>
>> >> > Signed-off-by: Rafael J. Wysocki <[email protected]>
>> >> > ---
>> [...]
>> >>
>> >> 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?
>> >
>> > Yes, I'll do that.
>>
>> Following up on this series. I do not see it queued anywhere in the
>> linux-pm [0] tree. I would like to have this in the v6.3 merge window if
>> possible.
>>
>> [0] https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/
>
> It's already in the mainline:
>
> e8a0e30b742f cpufreq: intel_pstate: Drop ACPI _PSS states table patching
> 99387b016022 ACPI: processor: perflib: Avoid updating frequency QoS
> unnecessarily
> c02d5feb6e2f ACPI: processor: perflib: Use the "no limit" frequency QoS
Hmm, I skimmed through the git log and did not spot any of these. Seems
like they were buried deeper in the logs due to merges. I can see them
now. My bad.
--
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
On Mon, 2023-01-30 at 15:58 +0100, Rafael J. Wysocki wrote:
> On Mon, Jan 30, 2023 at 3:18 PM Pratyush Yadav <[email protected]>
> wrote:
> >
> >
[...]
> > [0]
> > https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/
>
> It's already in the mainline:
>
> e8a0e30b742f cpufreq: intel_pstate: Drop ACPI _PSS states table
> patching
> 99387b016022 ACPI: processor: perflib: Avoid updating frequency QoS
> unnecessarily
> c02d5feb6e2f ACPI: processor: perflib: Use the "no limit" frequency
> QoS
I am checking 6.2-rc8.
I don't see these commits.
The last commit in mainline is
git log --oneline drivers/acpi/processor_perflib.c
f1a70bac90ca ACPI: processor: perflib: Adjust
acpi_processor_notify_smm() return value
Whereas linux-pm tree it is :
git log --oneline drivers/acpi/processor_perflib.c
99387b016022 ACPI: processor: perflib: Avoid updating frequency QoS
unnecessarily
c02d5feb6e2f ACPI: processor: perflib: Use the "no limit" frequency QoS
f1a70bac90ca ACPI: processor: perflib: Adjust
acpi_processor_notify_smm() return value
Thanks,
Srinivas
On Tue, Feb 14, 2023 at 2:40 PM srinivas pandruvada
<[email protected]> wrote:
>
> On Mon, 2023-01-30 at 15:58 +0100, Rafael J. Wysocki wrote:
> > On Mon, Jan 30, 2023 at 3:18 PM Pratyush Yadav <[email protected]>
> > wrote:
> > >
> > >
>
> [...]
>
> > > [0]
> > > https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/
> >
> > It's already in the mainline:
> >
> > e8a0e30b742f cpufreq: intel_pstate: Drop ACPI _PSS states table
> > patching
> > 99387b016022 ACPI: processor: perflib: Avoid updating frequency QoS
> > unnecessarily
> > c02d5feb6e2f ACPI: processor: perflib: Use the "no limit" frequency
> > QoS
>
> I am checking 6.2-rc8.
> I don't see these commits.
You are right, they are in linux-next only, sorry for the confusion.
I'm going to push them for 6.3-rc1 this week, though.
On Tue, 2023-02-14 at 14:57 +0100, Rafael J. Wysocki wrote:
> On Tue, Feb 14, 2023 at 2:40 PM srinivas pandruvada
> <[email protected]> wrote:
> >
> > On Mon, 2023-01-30 at 15:58 +0100, Rafael J. Wysocki wrote:
> > > On Mon, Jan 30, 2023 at 3:18 PM Pratyush Yadav
> > > <[email protected]>
> > > wrote:
> > > >
> > > >
> >
> > [...]
> >
> > > > [0]
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/
> > >
> > > It's already in the mainline:
> > >
> > > e8a0e30b742f cpufreq: intel_pstate: Drop ACPI _PSS states table
> > > patching
> > > 99387b016022 ACPI: processor: perflib: Avoid updating frequency
> > > QoS
> > > unnecessarily
> > > c02d5feb6e2f ACPI: processor: perflib: Use the "no limit"
> > > frequency
> > > QoS
> >
> > I am checking 6.2-rc8.
> > I don't see these commits.
>
> You are right, they are in linux-next only, sorry for the confusion.
>
> I'm going to push them for 6.3-rc1 this week, though.
I don't think they are marked for stable. Can we add that?
Thanks,
Srinivas
On Tue, Feb 14, 2023 at 3:25 PM srinivas pandruvada
<[email protected]> wrote:
>
> On Tue, 2023-02-14 at 14:57 +0100, Rafael J. Wysocki wrote:
> > On Tue, Feb 14, 2023 at 2:40 PM srinivas pandruvada
> > <[email protected]> wrote:
> > >
> > > On Mon, 2023-01-30 at 15:58 +0100, Rafael J. Wysocki wrote:
> > > > On Mon, Jan 30, 2023 at 3:18 PM Pratyush Yadav
> > > > <[email protected]>
> > > > wrote:
> > > > >
> > > > >
> > >
> > > [...]
> > >
> > > > > [0]
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/
> > > >
> > > > It's already in the mainline:
> > > >
> > > > e8a0e30b742f cpufreq: intel_pstate: Drop ACPI _PSS states table
> > > > patching
> > > > 99387b016022 ACPI: processor: perflib: Avoid updating frequency
> > > > QoS
> > > > unnecessarily
> > > > c02d5feb6e2f ACPI: processor: perflib: Use the "no limit"
> > > > frequency
> > > > QoS
> > >
> > > I am checking 6.2-rc8.
> > > I don't see these commits.
> >
> > You are right, they are in linux-next only, sorry for the confusion.
> >
> > I'm going to push them for 6.3-rc1 this week, though.
> I don't think they are marked for stable. Can we add that?
I'd rather not rebase them for that.
It is still possible to send an inclusion request to -stable when then
get into the mainline.