Received-SPF: pass (google.com: domain of linux-kernel+bounces-121528-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240321-gpa-no-cooling-devs-v1-1-5c9e0ef2062e@trvn.ru>
In-Reply-To: <20240321-gpa-no-cooling-devs-v1-1-5c9e0ef2062e@trvn.ru>
From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: Wed, 27 Mar 2024 16:25:21 +0100
Message-ID: <CAJZ5v0jAq=iMKzYBz-Ni6Zqpbgxp3_0UYpPiAoSLcfGNJ8ruhQ@mail.gmail.com>
Subject: Re: [PATCH] thermal: gov_power_allocator: Allow binding without
 cooling devices
To: nikita@trvn.ru, Lukasz Luba <lukasz.luba@arm.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>, Zhang Rui <rui.zhang@intel.com>, 
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>, linux-pm@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Mar 21, 2024 at 3:44=E2=80=AFPM Nikita Travkin <nikita@trvn.ru> wro=
te:
>
> Commit e83747c2f8e3 ("thermal: gov_power_allocator: Set up trip points ea=
rlier")
> 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 ea=
rlier")
> 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.

=C5=81ukasz, any comments?

> ---
>  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 =3D check_power_actors(tz, params);
>         if (ret < 0) {
> @@ -693,7 +688,7 @@ static int power_allocator_bind(struct thermal_zone_d=
evice *tz)
>         }
>
>         ret =3D allocate_actors_buffer(params, ret);
> -       if (ret) {
> +       if (ret && ret !=3D -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 =3D 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>
>