Messages in this thread | | | Date | Thu, 28 Mar 2024 09:12:10 +0000 | Subject | Re: [PATCH] thermal: gov_power_allocator: Allow binding without cooling devices | From | Lukasz Luba <> |
| |
Hi Rafael,
On 3/27/24 15:25, Rafael J. Wysocki wrote: > On Thu, Mar 21, 2024 at 3:44 PM Nikita Travkin <nikita@trvn.ru> wrote: >> >> Commit e83747c2f8e3 ("thermal: gov_power_allocator: Set up trip points earlier") >> added a check that would fail binding the governer if there is no >> cooling devices bound to the thermal zone. Unfortunately this causes >> issues in cases when the TZ is bound to the governer before the cooling >> devices are attached to it. (I.e. when the tz is registered using >> thermal_zone_device_register_with_trips().) >> >> Additionally, the documentation across gov_power_allocator suggests it's >> intended to allow it to be bound to thermal zones without cooling >> devices (and thus without passive/active trip points), however the same >> change added a check for the trip point to be present, causing those TZ >> to fail probing. >> >> Those changes cause all thermal zones to fail on some devices (such as >> sc7180-acer-aspire1) and prevent the kernel from controlling the cpu/gpu >> frequency based on the temperature, as well as losing all the other >> "informational" thermal zones if power_allocator is set as default. >> >> This commit partially reverts the referenced one by dropping the trip >> point check and by allowing the TZ to probe even if no actor buffer was >> allocated to allow those TZ to probe again. >> >> Fixes: e83747c2f8e3 ("thermal: gov_power_allocator: Set up trip points earlier") >> Signed-off-by: Nikita Travkin <nikita@trvn.ru> >> --- >> I've noticed that all thermal zones fail probing with -EINVAL on my >> sc7180 based Acer Aspire 1 since 6.8. This commit allows me to bring >> them back. > > Łukasz, any comments?
Let me check this today.
> >> --- >> drivers/thermal/gov_power_allocator.c | 14 +++++--------- >> 1 file changed, 5 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c >> index 1b17dc4c219c..4f2d7f3b7508 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) { >> @@ -693,7 +688,7 @@ static int power_allocator_bind(struct thermal_zone_device *tz) >> } >> >> ret = allocate_actors_buffer(params, ret); >> - if (ret) { >> + if (ret && ret != -EINVAL) { >> dev_warn(&tz->device, "power_allocator: allocation failed\n"); >> kfree(params); >> return ret; >> @@ -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); >> >> >> --- >> base-commit: e7528c088874326d3060a46f572252be43755a86 >> change-id: 20240321-gpa-no-cooling-devs-c79ee3288325 >> >> Best regards, >> -- >> Nikita Travkin <nikita@trvn.ru> >>
| |