Messages in this thread Patch in this message | | | Date | Thu, 28 Mar 2024 09:50:14 +0000 | Subject | Re: [PATCH] thermal: gov_power_allocator: Allow binding without cooling devices | From | Lukasz Luba <> |
| |
On 3/28/24 09:12, Lukasz Luba wrote: > 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.
This patch description is mixing trips and cooling devices and refers to a commit which is only for guarding the trip points number to be not less than 2. In IPA we require 2 trip points.
>>> >>> 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")
Not that commit.
>>> 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");
This if() guards the binding of TZ with less than 2 trip points, not the cooling devices.
>>> - 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) {
This if() is from different commit.
>>> + if (ret && ret != -EINVAL) {
This is about 0 cooling devices in the thermal zone, but IPA won't work so why to even fake and forward binding?
Rafael should we support binding with 0 cooling devices?
>>> 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)
This is not supported, we need those 2 trip points.
>>> + 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> >>> >
| |