* [PATCH v2 0/3] gov_power_allocator: Allow binding before cooling devices
@ 2024-04-03 11:31 Nikita Travkin via B4 Relay
2024-04-03 11:31 ` [PATCH v2 1/3] thermal: gov_power_allocator: Allow binding without " Nikita Travkin via B4 Relay
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Nikita Travkin via B4 Relay @ 2024-04-03 11:31 UTC (permalink / raw)
To: Lukasz Luba, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui
Cc: Rafael J. Wysocki, linux-pm, linux-kernel, Nikita Travkin,
Nikita Travkin
Recent changes in IPA made it fail probing if the TZ has no cooling
devices attached on probe or no trip points defined.
This series restores prior behavior to:
- allow IPA to probe before cooling devices have attached;
- allow IPA to probe when the TZ has no passive/active trip points.
I've noticed that all thermal zones fail probing with -EINVAL on my
sc7180 based Acer Aspire 1 since 6.8. This series allows me to bring
them back.
Additionally there is a commit that supresses the "sustainable_power
will be estimated" warning on TZ that have no trip points (and thus IPA
will not be able to do anything for them anyway). This allowed me to
notice that some of the TZ with cooling_devices on my platform actually
lack the sustainable_power value.
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
Changes in v2:
- Split to two changes (Lukasz)
- Return 0 in allocate_actors_buffer() instead of suppressing -EINVAL
(Lukasz)
- Add a change to supress "sustainable_power will be estimated" warning
on "empty" TZ
- Link to v1: https://lore.kernel.org/r/20240321-gpa-no-cooling-devs-v1-1-5c9e0ef2062e@trvn.ru
---
Nikita Travkin (3):
thermal: gov_power_allocator: Allow binding without cooling devices
thermal: gov_power_allocator: Allow binding without trip points
thermal: gov_power_allocator: Suppress sustainable_power warning without trip_points
drivers/thermal/gov_power_allocator.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
---
base-commit: 727900b675b749c40ba1f6669c7ae5eb7eb8e837
change-id: 20240321-gpa-no-cooling-devs-c79ee3288325
Best regards,
--
Nikita Travkin <nikita@trvn.ru>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/3] thermal: gov_power_allocator: Allow binding without cooling devices
2024-04-03 11:31 [PATCH v2 0/3] gov_power_allocator: Allow binding before cooling devices Nikita Travkin via B4 Relay
@ 2024-04-03 11:31 ` Nikita Travkin via B4 Relay
2024-04-03 12:43 ` Lukasz Luba
2024-04-03 11:31 ` [PATCH v2 2/3] thermal: gov_power_allocator: Allow binding without trip points Nikita Travkin via B4 Relay
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Nikita Travkin via B4 Relay @ 2024-04-03 11:31 UTC (permalink / raw)
To: Lukasz Luba, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui
Cc: Rafael J. Wysocki, linux-pm, linux-kernel, Nikita Travkin,
Nikita Travkin
From: Nikita Travkin <nikita@trvn.ru>
IPA was recently refactored to split out memory allocation into a
separate funciton. That funciton was made to return -EINVAL if there is
zero power_actors and thus no memory to allocate. This causes IPA to
fail probing when the thermal zone has no attached cooling devices.
Since cooling devices can attach after the thermal zone is created and
the governer is attached to it, failing probe due to the lack of cooling
devices is incorrect.
Change the allocate_actors_buffer() to return success when there is no
cooling devices present.
Fixes: 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()")
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
drivers/thermal/gov_power_allocator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index 1b17dc4c219c..ec637071ef1f 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -606,7 +606,7 @@ static int allocate_actors_buffer(struct power_allocator_params *params,
/* There might be no cooling devices yet. */
if (!num_actors) {
- ret = -EINVAL;
+ ret = 0;
goto clean_state;
}
--
2.44.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/3] thermal: gov_power_allocator: Allow binding without trip points
2024-04-03 11:31 [PATCH v2 0/3] gov_power_allocator: Allow binding before cooling devices Nikita Travkin via B4 Relay
2024-04-03 11:31 ` [PATCH v2 1/3] thermal: gov_power_allocator: Allow binding without " Nikita Travkin via B4 Relay
@ 2024-04-03 11:31 ` Nikita Travkin via B4 Relay
2024-04-03 12:48 ` Lukasz Luba
2024-04-03 11:31 ` [PATCH v2 3/3] thermal: gov_power_allocator: Suppress sustainable_power warning without trip_points Nikita Travkin via B4 Relay
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Nikita Travkin via B4 Relay @ 2024-04-03 11:31 UTC (permalink / raw)
To: Lukasz Luba, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui
Cc: Rafael J. Wysocki, linux-pm, linux-kernel, Nikita Travkin,
Nikita Travkin
From: Nikita Travkin <nikita@trvn.ru>
IPA probe function was recently refactored to perform extra error checks
and make sure the thermal zone has trip points necessary for the IPA
operation. With this change, if a thermal zone is probed such that it
has no trip points that IPA can use, IPA will fail and the TZ won't be
created. This is the case if a platform defines a TZ without cooling
devices and only with "hot"/"critical" trip points, often found on some
Qualcomm devices [1].
Documentation across IPA code (notably get_governor_trips() kerneldoc)
suggests that IPA is supposed to handle such TZ even if it won't
actually do anything.
This commit partially reverts the previous change to allow IPA to bind
to such "empty" thermal zones.
[1] arch/arm64/boot/dts/qcom/sc7180.dtsi#n4776
Fixes: e83747c2f8e3 ("thermal: gov_power_allocator: Set up trip points earlier")
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
drivers/thermal/gov_power_allocator.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index ec637071ef1f..e25e48d76aa7 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -679,11 +679,6 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
return -ENOMEM;
get_governor_trips(tz, params);
- if (!params->trip_max) {
- dev_warn(&tz->device, "power_allocator: missing trip_max\n");
- kfree(params);
- return -EINVAL;
- }
ret = check_power_actors(tz, params);
if (ret < 0) {
@@ -714,9 +709,10 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
else
params->sustainable_power = tz->tzp->sustainable_power;
- estimate_pid_constants(tz, tz->tzp->sustainable_power,
- params->trip_switch_on,
- params->trip_max->temperature);
+ if (params->trip_max)
+ estimate_pid_constants(tz, tz->tzp->sustainable_power,
+ params->trip_switch_on,
+ params->trip_max->temperature);
reset_pid_controller(params);
--
2.44.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/3] thermal: gov_power_allocator: Suppress sustainable_power warning without trip_points
2024-04-03 11:31 [PATCH v2 0/3] gov_power_allocator: Allow binding before cooling devices Nikita Travkin via B4 Relay
2024-04-03 11:31 ` [PATCH v2 1/3] thermal: gov_power_allocator: Allow binding without " Nikita Travkin via B4 Relay
2024-04-03 11:31 ` [PATCH v2 2/3] thermal: gov_power_allocator: Allow binding without trip points Nikita Travkin via B4 Relay
@ 2024-04-03 11:31 ` Nikita Travkin via B4 Relay
2024-04-03 12:52 ` Lukasz Luba
2024-04-09 14:41 ` [PATCH v2 0/3] gov_power_allocator: Allow binding before cooling devices Leonard Lausen
2024-04-09 14:42 ` Leonard Lausen
4 siblings, 1 reply; 14+ messages in thread
From: Nikita Travkin via B4 Relay @ 2024-04-03 11:31 UTC (permalink / raw)
To: Lukasz Luba, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui
Cc: Rafael J. Wysocki, linux-pm, linux-kernel, Nikita Travkin,
Nikita Travkin
From: Nikita Travkin <nikita@trvn.ru>
IPA warns if the thermal zone it was attached to doesn't define
sustainable_power value. In some cases though IPA may be bound to an
"empty" TZ, in which case the lack of sustainable_power doesn't matter.
Suppress the warning in case when IPA is bound to an empty TZ to make it
easier to see the warnings that actually matter.
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
I've decided to add this along to supress those warnings for some TZ on
sc7180. Feel free to drop this patch if you think the warning should
always appear.
---
drivers/thermal/gov_power_allocator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index e25e48d76aa7..05a40f6b5928 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -704,7 +704,7 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
params->allocated_tzp = true;
}
- if (!tz->tzp->sustainable_power)
+ if (!tz->tzp->sustainable_power && params->trip_max)
dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n");
else
params->sustainable_power = tz->tzp->sustainable_power;
--
2.44.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] thermal: gov_power_allocator: Allow binding without cooling devices
2024-04-03 11:31 ` [PATCH v2 1/3] thermal: gov_power_allocator: Allow binding without " Nikita Travkin via B4 Relay
@ 2024-04-03 12:43 ` Lukasz Luba
2024-04-03 14:41 ` Rafael J. Wysocki
0 siblings, 1 reply; 14+ messages in thread
From: Lukasz Luba @ 2024-04-03 12:43 UTC (permalink / raw)
To: Nikita Travkin
Cc: Rafael J. Wysocki, Rafael J. Wysocki, linux-pm, Daniel Lezcano,
linux-kernel, Zhang Rui
On 4/3/24 12:31, Nikita Travkin via B4 Relay wrote:
> From: Nikita Travkin <nikita@trvn.ru>
>
> IPA was recently refactored to split out memory allocation into a
> separate funciton. That funciton was made to return -EINVAL if there is
> zero power_actors and thus no memory to allocate. This causes IPA to
> fail probing when the thermal zone has no attached cooling devices.
>
> Since cooling devices can attach after the thermal zone is created and
> the governer is attached to it, failing probe due to the lack of cooling
> devices is incorrect.
>
> Change the allocate_actors_buffer() to return success when there is no
> cooling devices present.
>
> Fixes: 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()")
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
> drivers/thermal/gov_power_allocator.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> index 1b17dc4c219c..ec637071ef1f 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -606,7 +606,7 @@ static int allocate_actors_buffer(struct power_allocator_params *params,
>
> /* There might be no cooling devices yet. */
> if (!num_actors) {
> - ret = -EINVAL;
> + ret = 0;
> goto clean_state;
> }
>
>
LGTM
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] thermal: gov_power_allocator: Allow binding without trip points
2024-04-03 11:31 ` [PATCH v2 2/3] thermal: gov_power_allocator: Allow binding without trip points Nikita Travkin via B4 Relay
@ 2024-04-03 12:48 ` Lukasz Luba
0 siblings, 0 replies; 14+ messages in thread
From: Lukasz Luba @ 2024-04-03 12:48 UTC (permalink / raw)
To: Nikita Travkin
Cc: Rafael J. Wysocki, Rafael J. Wysocki, Daniel Lezcano, linux-pm,
linux-kernel, Zhang Rui
On 4/3/24 12:31, Nikita Travkin via B4 Relay wrote:
> From: Nikita Travkin <nikita@trvn.ru>
>
> IPA probe function was recently refactored to perform extra error checks
> and make sure the thermal zone has trip points necessary for the IPA
> operation. With this change, if a thermal zone is probed such that it
> has no trip points that IPA can use, IPA will fail and the TZ won't be
> created. This is the case if a platform defines a TZ without cooling
> devices and only with "hot"/"critical" trip points, often found on some
> Qualcomm devices [1].
>
> Documentation across IPA code (notably get_governor_trips() kerneldoc)
> suggests that IPA is supposed to handle such TZ even if it won't
> actually do anything.
>
> This commit partially reverts the previous change to allow IPA to bind
> to such "empty" thermal zones.
>
> [1] arch/arm64/boot/dts/qcom/sc7180.dtsi#n4776
>
> Fixes: e83747c2f8e3 ("thermal: gov_power_allocator: Set up trip points earlier")
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
> drivers/thermal/gov_power_allocator.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> index ec637071ef1f..e25e48d76aa7 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -679,11 +679,6 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
> return -ENOMEM;
>
> get_governor_trips(tz, params);
> - if (!params->trip_max) {
> - dev_warn(&tz->device, "power_allocator: missing trip_max\n");
> - kfree(params);
> - return -EINVAL;
> - }
>
> ret = check_power_actors(tz, params);
> if (ret < 0) {
> @@ -714,9 +709,10 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
> else
> params->sustainable_power = tz->tzp->sustainable_power;
>
> - estimate_pid_constants(tz, tz->tzp->sustainable_power,
> - params->trip_switch_on,
> - params->trip_max->temperature);
> + if (params->trip_max)
> + estimate_pid_constants(tz, tz->tzp->sustainable_power,
> + params->trip_switch_on,
> + params->trip_max->temperature);
>
> reset_pid_controller(params);
>
>
LGTM
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] thermal: gov_power_allocator: Suppress sustainable_power warning without trip_points
2024-04-03 11:31 ` [PATCH v2 3/3] thermal: gov_power_allocator: Suppress sustainable_power warning without trip_points Nikita Travkin via B4 Relay
@ 2024-04-03 12:52 ` Lukasz Luba
2024-04-03 13:05 ` Nikita Travkin
0 siblings, 1 reply; 14+ messages in thread
From: Lukasz Luba @ 2024-04-03 12:52 UTC (permalink / raw)
To: Nikita Travkin
Cc: Rafael J. Wysocki, Rafael J. Wysocki, linux-pm, Zhang Rui,
Daniel Lezcano, linux-kernel
On 4/3/24 12:31, Nikita Travkin via B4 Relay wrote:
> From: Nikita Travkin <nikita@trvn.ru>
>
> IPA warns if the thermal zone it was attached to doesn't define
> sustainable_power value. In some cases though IPA may be bound to an
> "empty" TZ, in which case the lack of sustainable_power doesn't matter.
>
> Suppress the warning in case when IPA is bound to an empty TZ to make it
> easier to see the warnings that actually matter.
>
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
>
> I've decided to add this along to supress those warnings for some TZ on
> sc7180. Feel free to drop this patch if you think the warning should
> always appear.
That warning should stay, since in the development or integration phase
quite a lot of stuff is missing. This will warn that there is an issue.
The case with 'empty' TZ is an exception only to 'work' with IPA.
Thanks for the patches!
Regards,
Lukasz
> ---
> drivers/thermal/gov_power_allocator.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> index e25e48d76aa7..05a40f6b5928 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -704,7 +704,7 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
> params->allocated_tzp = true;
> }
>
> - if (!tz->tzp->sustainable_power)
> + if (!tz->tzp->sustainable_power && params->trip_max)
> dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n");
> else
> params->sustainable_power = tz->tzp->sustainable_power;
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] thermal: gov_power_allocator: Suppress sustainable_power warning without trip_points
2024-04-03 12:52 ` Lukasz Luba
@ 2024-04-03 13:05 ` Nikita Travkin
0 siblings, 0 replies; 14+ messages in thread
From: Nikita Travkin @ 2024-04-03 13:05 UTC (permalink / raw)
To: Lukasz Luba
Cc: Rafael J. Wysocki, Rafael J. Wysocki, linux-pm, Zhang Rui,
Daniel Lezcano, linux-kernel
ср, 3 апр. 2024 г. в 17:52, Lukasz Luba <lukasz.luba@arm.com>:
>
>
>
> On 4/3/24 12:31, Nikita Travkin via B4 Relay wrote:
> > From: Nikita Travkin <nikita@trvn.ru>
> >
> > IPA warns if the thermal zone it was attached to doesn't define
> > sustainable_power value. In some cases though IPA may be bound to an
> > "empty" TZ, in which case the lack of sustainable_power doesn't matter.
> >
> > Suppress the warning in case when IPA is bound to an empty TZ to make it
> > easier to see the warnings that actually matter.
> >
> > Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> > ---
> >
> > I've decided to add this along to supress those warnings for some TZ on
> > sc7180. Feel free to drop this patch if you think the warning should
> > always appear.
>
> That warning should stay, since in the development or integration phase
> quite a lot of stuff is missing. This will warn that there is an issue.
> The case with 'empty' TZ is an exception only to 'work' with IPA.
>
Yes, that's understandable, though by suppressing those I could
actually see the few actual warnings for TZ with cooling devices
and no value, which I couldn't see before because it looked like
"all of them" have the warning.
In any case, as I said, I'm fine with this not being applied :)
Thanks for your review!
Nikita
> Thanks for the patches!
>
> Regards,
> Lukasz
>
>
> > ---
> > drivers/thermal/gov_power_allocator.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> > index e25e48d76aa7..05a40f6b5928 100644
> > --- a/drivers/thermal/gov_power_allocator.c
> > +++ b/drivers/thermal/gov_power_allocator.c
> > @@ -704,7 +704,7 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
> > params->allocated_tzp = true;
> > }
> >
> > - if (!tz->tzp->sustainable_power)
> > + if (!tz->tzp->sustainable_power && params->trip_max)
> > dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n");
> > else
> > params->sustainable_power = tz->tzp->sustainable_power;
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] thermal: gov_power_allocator: Allow binding without cooling devices
2024-04-03 12:43 ` Lukasz Luba
@ 2024-04-03 14:41 ` Rafael J. Wysocki
2024-04-03 15:03 ` Lukasz Luba
0 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2024-04-03 14:41 UTC (permalink / raw)
To: Lukasz Luba, Nikita Travkin
Cc: Rafael J. Wysocki, Rafael J. Wysocki, linux-pm, Daniel Lezcano,
linux-kernel, Zhang Rui
On Wed, Apr 3, 2024 at 2:44 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
>
>
>
> On 4/3/24 12:31, Nikita Travkin via B4 Relay wrote:
> > From: Nikita Travkin <nikita@trvn.ru>
> >
> > IPA was recently refactored to split out memory allocation into a
> > separate funciton. That funciton was made to return -EINVAL if there is
> > zero power_actors and thus no memory to allocate. This causes IPA to
> > fail probing when the thermal zone has no attached cooling devices.
> >
> > Since cooling devices can attach after the thermal zone is created and
> > the governer is attached to it, failing probe due to the lack of cooling
> > devices is incorrect.
> >
> > Change the allocate_actors_buffer() to return success when there is no
> > cooling devices present.
> >
> > Fixes: 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()")
> > Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> > ---
> > drivers/thermal/gov_power_allocator.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> > index 1b17dc4c219c..ec637071ef1f 100644
> > --- a/drivers/thermal/gov_power_allocator.c
> > +++ b/drivers/thermal/gov_power_allocator.c
> > @@ -606,7 +606,7 @@ static int allocate_actors_buffer(struct power_allocator_params *params,
> >
> > /* There might be no cooling devices yet. */
> > if (!num_actors) {
> > - ret = -EINVAL;
> > + ret = 0;
> > goto clean_state;
> > }
> >
> >
>
> LGTM
>
> Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Applied as 6.9-rc material along with the [2/3], thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] thermal: gov_power_allocator: Allow binding without cooling devices
2024-04-03 14:41 ` Rafael J. Wysocki
@ 2024-04-03 15:03 ` Lukasz Luba
0 siblings, 0 replies; 14+ messages in thread
From: Lukasz Luba @ 2024-04-03 15:03 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael J. Wysocki, linux-pm, Nikita Travkin, Daniel Lezcano,
linux-kernel, Zhang Rui
On 4/3/24 15:41, Rafael J. Wysocki wrote:
> On Wed, Apr 3, 2024 at 2:44 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>
>>
>>
>> On 4/3/24 12:31, Nikita Travkin via B4 Relay wrote:
>>> From: Nikita Travkin <nikita@trvn.ru>
>>>
>>> IPA was recently refactored to split out memory allocation into a
>>> separate funciton. That funciton was made to return -EINVAL if there is
>>> zero power_actors and thus no memory to allocate. This causes IPA to
>>> fail probing when the thermal zone has no attached cooling devices.
>>>
>>> Since cooling devices can attach after the thermal zone is created and
>>> the governer is attached to it, failing probe due to the lack of cooling
>>> devices is incorrect.
>>>
>>> Change the allocate_actors_buffer() to return success when there is no
>>> cooling devices present.
>>>
>>> Fixes: 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()")
>>> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
>>> ---
>>> drivers/thermal/gov_power_allocator.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
>>> index 1b17dc4c219c..ec637071ef1f 100644
>>> --- a/drivers/thermal/gov_power_allocator.c
>>> +++ b/drivers/thermal/gov_power_allocator.c
>>> @@ -606,7 +606,7 @@ static int allocate_actors_buffer(struct power_allocator_params *params,
>>>
>>> /* There might be no cooling devices yet. */
>>> if (!num_actors) {
>>> - ret = -EINVAL;
>>> + ret = 0;
>>> goto clean_state;
>>> }
>>>
>>>
>>
>> LGTM
>>
>> Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
>
> Applied as 6.9-rc material along with the [2/3], thanks!
>
Thank you Rafael!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/3] gov_power_allocator: Allow binding before cooling devices
2024-04-03 11:31 [PATCH v2 0/3] gov_power_allocator: Allow binding before cooling devices Nikita Travkin via B4 Relay
` (2 preceding siblings ...)
2024-04-03 11:31 ` [PATCH v2 3/3] thermal: gov_power_allocator: Suppress sustainable_power warning without trip_points Nikita Travkin via B4 Relay
@ 2024-04-09 14:41 ` Leonard Lausen
2024-04-09 14:42 ` Leonard Lausen
4 siblings, 0 replies; 14+ messages in thread
From: Leonard Lausen @ 2024-04-09 14:41 UTC (permalink / raw)
To: nikita, Lukasz Luba, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui
Cc: Rafael J. Wysocki, linux-pm, linux-kernel, Nikita Travkin
Hi Nikita, Hi Łukasz,
thank you for fixing the e83747c2f8e3 ("thermal: gov_power_allocator: Set up trip points earlier") and 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()") regressions as part of v6.9-rc3. As the regression was introduced in v6.8, would it be possible to include the fix in a v6.8 patch release?
Thank you
Leonard
#regzbot introduced: 912e97c67cc3f333c4c5df8f51498c651792e658
#regzbot fixed-by: 1057c4c36ef8b236a2e28edef301da0801338c5f
#regzbot introduced: e83747c2f8e3cc5e284e37a8921099f1901d79d8
#regzbot fixed-by: da781936e7c301e6197eb6513775748e79fb2575
On 4/3/24 07:31, Nikita Travkin via B4 Relay wrote:
> Recent changes in IPA made it fail probing if the TZ has no cooling
> devices attached on probe or no trip points defined.
>
> This series restores prior behavior to:
>
> - allow IPA to probe before cooling devices have attached;
> - allow IPA to probe when the TZ has no passive/active trip points.
>
> I've noticed that all thermal zones fail probing with -EINVAL on my
> sc7180 based Acer Aspire 1 since 6.8. This series allows me to bring
> them back.
>
> Additionally there is a commit that supresses the "sustainable_power
> will be estimated" warning on TZ that have no trip points (and thus IPA
> will not be able to do anything for them anyway). This allowed me to
> notice that some of the TZ with cooling_devices on my platform actually
> lack the sustainable_power value.
>
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
> Changes in v2:
> - Split to two changes (Lukasz)
> - Return 0 in allocate_actors_buffer() instead of suppressing -EINVAL
> (Lukasz)
> - Add a change to supress "sustainable_power will be estimated" warning
> on "empty" TZ
> - Link to v1: https://lore.kernel.org/r/20240321-gpa-no-cooling-devs-v1-1-5c9e0ef2062e@trvn.ru
>
> ---
> Nikita Travkin (3):
> thermal: gov_power_allocator: Allow binding without cooling devices
> thermal: gov_power_allocator: Allow binding without trip points
> thermal: gov_power_allocator: Suppress sustainable_power warning without trip_points
>
> drivers/thermal/gov_power_allocator.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
> ---
> base-commit: 727900b675b749c40ba1f6669c7ae5eb7eb8e837
> change-id: 20240321-gpa-no-cooling-devs-c79ee3288325
>
> Best regards,
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/3] gov_power_allocator: Allow binding before cooling devices
2024-04-03 11:31 [PATCH v2 0/3] gov_power_allocator: Allow binding before cooling devices Nikita Travkin via B4 Relay
` (3 preceding siblings ...)
2024-04-09 14:41 ` [PATCH v2 0/3] gov_power_allocator: Allow binding before cooling devices Leonard Lausen
@ 2024-04-09 14:42 ` Leonard Lausen
2024-04-09 14:46 ` Nikita Travkin
4 siblings, 1 reply; 14+ messages in thread
From: Leonard Lausen @ 2024-04-09 14:42 UTC (permalink / raw)
To: nikita, Lukasz Luba, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui
Cc: Rafael J. Wysocki, linux-pm, linux-kernel, Nikita Travkin, regressions
Hi Nikita, Hi Łukasz,
thank you for fixing the e83747c2f8e3 ("thermal: gov_power_allocator: Set up trip points earlier") and 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()") regressions as part of v6.9-rc3. As the regression was introduced in v6.8, would it be possible to include the fix in a v6.8 patch release?
Thank you
Leonard
(Resending with regressions@lists.linux.dev in CC)
#regzbot introduced: 912e97c67cc3f333c4c5df8f51498c651792e658
#regzbot fixed-by: 1057c4c36ef8b236a2e28edef301da0801338c5f
#regzbot introduced: e83747c2f8e3cc5e284e37a8921099f1901d79d8
#regzbot fixed-by: da781936e7c301e6197eb6513775748e79fb2575
On 4/3/24 07:31, Nikita Travkin via B4 Relay wrote:
> Recent changes in IPA made it fail probing if the TZ has no cooling
> devices attached on probe or no trip points defined.
>
> This series restores prior behavior to:
>
> - allow IPA to probe before cooling devices have attached;
> - allow IPA to probe when the TZ has no passive/active trip points.
>
> I've noticed that all thermal zones fail probing with -EINVAL on my
> sc7180 based Acer Aspire 1 since 6.8. This series allows me to bring
> them back.
>
> Additionally there is a commit that supresses the "sustainable_power
> will be estimated" warning on TZ that have no trip points (and thus IPA
> will not be able to do anything for them anyway). This allowed me to
> notice that some of the TZ with cooling_devices on my platform actually
> lack the sustainable_power value.
>
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
> Changes in v2:
> - Split to two changes (Lukasz)
> - Return 0 in allocate_actors_buffer() instead of suppressing -EINVAL
> (Lukasz)
> - Add a change to supress "sustainable_power will be estimated" warning
> on "empty" TZ
> - Link to v1: https://lore.kernel.org/r/20240321-gpa-no-cooling-devs-v1-1-5c9e0ef2062e@trvn.ru
>
> ---
> Nikita Travkin (3):
> thermal: gov_power_allocator: Allow binding without cooling devices
> thermal: gov_power_allocator: Allow binding without trip points
> thermal: gov_power_allocator: Suppress sustainable_power warning without trip_points
>
> drivers/thermal/gov_power_allocator.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
> ---
> base-commit: 727900b675b749c40ba1f6669c7ae5eb7eb8e837
> change-id: 20240321-gpa-no-cooling-devs-c79ee3288325
>
> Best regards,
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/3] gov_power_allocator: Allow binding before cooling devices
2024-04-09 14:42 ` Leonard Lausen
@ 2024-04-09 14:46 ` Nikita Travkin
2024-04-11 6:38 ` Lukasz Luba
0 siblings, 1 reply; 14+ messages in thread
From: Nikita Travkin @ 2024-04-09 14:46 UTC (permalink / raw)
To: Leonard Lausen
Cc: nikita, Lukasz Luba, Rafael J. Wysocki, Daniel Lezcano,
Zhang Rui, Rafael J. Wysocki, linux-pm, linux-kernel,
regressions
вт, 9 апр. 2024 г. в 19:42, Leonard Lausen <leonard@lausen.nl>:
>
> Hi Nikita, Hi Łukasz,
>
> thank you for fixing the e83747c2f8e3 ("thermal: gov_power_allocator: Set up trip points earlier") and 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()") regressions as part of v6.9-rc3. As the regression was introduced in v6.8, would it be possible to include the fix in a v6.8 patch release?
>
Hi! I think these both have already been picked for stable:
https://lore.kernel.org/r/20240408125314.939341866@linuxfoundation.org
https://lore.kernel.org/r/20240408125314.969670696@linuxfoundation.org/
Nikita
> Thank you
> Leonard
>
> (Resending with regressions@lists.linux.dev in CC)
>
> #regzbot introduced: 912e97c67cc3f333c4c5df8f51498c651792e658
> #regzbot fixed-by: 1057c4c36ef8b236a2e28edef301da0801338c5f
>
>
> #regzbot introduced: e83747c2f8e3cc5e284e37a8921099f1901d79d8
> #regzbot fixed-by: da781936e7c301e6197eb6513775748e79fb2575
>
> On 4/3/24 07:31, Nikita Travkin via B4 Relay wrote:
> > Recent changes in IPA made it fail probing if the TZ has no cooling
> > devices attached on probe or no trip points defined.
> >
> > This series restores prior behavior to:
> >
> > - allow IPA to probe before cooling devices have attached;
> > - allow IPA to probe when the TZ has no passive/active trip points.
> >
> > I've noticed that all thermal zones fail probing with -EINVAL on my
> > sc7180 based Acer Aspire 1 since 6.8. This series allows me to bring
> > them back.
> >
> > Additionally there is a commit that supresses the "sustainable_power
> > will be estimated" warning on TZ that have no trip points (and thus IPA
> > will not be able to do anything for them anyway). This allowed me to
> > notice that some of the TZ with cooling_devices on my platform actually
> > lack the sustainable_power value.
> >
> > Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> > ---
> > Changes in v2:
> > - Split to two changes (Lukasz)
> > - Return 0 in allocate_actors_buffer() instead of suppressing -EINVAL
> > (Lukasz)
> > - Add a change to supress "sustainable_power will be estimated" warning
> > on "empty" TZ
> > - Link to v1: https://lore.kernel.org/r/20240321-gpa-no-cooling-devs-v1-1-5c9e0ef2062e@trvn.ru
> >
> > ---
> > Nikita Travkin (3):
> > thermal: gov_power_allocator: Allow binding without cooling devices
> > thermal: gov_power_allocator: Allow binding without trip points
> > thermal: gov_power_allocator: Suppress sustainable_power warning without trip_points
> >
> > drivers/thermal/gov_power_allocator.c | 16 ++++++----------
> > 1 file changed, 6 insertions(+), 10 deletions(-)
> > ---
> > base-commit: 727900b675b749c40ba1f6669c7ae5eb7eb8e837
> > change-id: 20240321-gpa-no-cooling-devs-c79ee3288325
> >
> > Best regards,
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/3] gov_power_allocator: Allow binding before cooling devices
2024-04-09 14:46 ` Nikita Travkin
@ 2024-04-11 6:38 ` Lukasz Luba
0 siblings, 0 replies; 14+ messages in thread
From: Lukasz Luba @ 2024-04-11 6:38 UTC (permalink / raw)
To: Nikita Travkin, Leonard Lausen
Cc: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Rafael J. Wysocki,
linux-pm, linux-kernel, regressions
On 4/9/24 15:46, Nikita Travkin wrote:
> вт, 9 апр. 2024 г. в 19:42, Leonard Lausen <leonard@lausen.nl>:
>>
>> Hi Nikita, Hi Łukasz,
>>
>> thank you for fixing the e83747c2f8e3 ("thermal: gov_power_allocator: Set up trip points earlier") and 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()") regressions as part of v6.9-rc3. As the regression was introduced in v6.8, would it be possible to include the fix in a v6.8 patch release?
>>
>
> Hi! I think these both have already been picked for stable:
>
> https://lore.kernel.org/r/20240408125314.939341866@linuxfoundation.org
> https://lore.kernel.org/r/20240408125314.969670696@linuxfoundation.org/
>
Correct
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-04-11 6:38 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-03 11:31 [PATCH v2 0/3] gov_power_allocator: Allow binding before cooling devices Nikita Travkin via B4 Relay
2024-04-03 11:31 ` [PATCH v2 1/3] thermal: gov_power_allocator: Allow binding without " Nikita Travkin via B4 Relay
2024-04-03 12:43 ` Lukasz Luba
2024-04-03 14:41 ` Rafael J. Wysocki
2024-04-03 15:03 ` Lukasz Luba
2024-04-03 11:31 ` [PATCH v2 2/3] thermal: gov_power_allocator: Allow binding without trip points Nikita Travkin via B4 Relay
2024-04-03 12:48 ` Lukasz Luba
2024-04-03 11:31 ` [PATCH v2 3/3] thermal: gov_power_allocator: Suppress sustainable_power warning without trip_points Nikita Travkin via B4 Relay
2024-04-03 12:52 ` Lukasz Luba
2024-04-03 13:05 ` Nikita Travkin
2024-04-09 14:41 ` [PATCH v2 0/3] gov_power_allocator: Allow binding before cooling devices Leonard Lausen
2024-04-09 14:42 ` Leonard Lausen
2024-04-09 14:46 ` Nikita Travkin
2024-04-11 6:38 ` Lukasz Luba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®