Received-SPF: pass (google.com: domain of linux-kernel+bounces-7104-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20231212134844.1213381-1-lukasz.luba@arm.com> <20231212134844.1213381-9-lukasz.luba@arm.com>
In-Reply-To: <20231212134844.1213381-9-lukasz.luba@arm.com>
From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: Wed, 20 Dec 2023 15:59:17 +0100
Message-ID: <CAJZ5v0gEFNhPYh8MdG6JPaXC0XggvyED+0QDuV+aLa5vASzhhA@mail.gmail.com>
Subject: Re: [PATCH v2 8/8] thermal: gov_power_allocator: Support new update
 callback of weights
To: Lukasz Luba <lukasz.luba@arm.com>
Cc: linux-kernel@vger.kernel.org, daniel.lezcano@linaro.org, rafael@kernel.org, 
	linux-pm@vger.kernel.org, rui.zhang@intel.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Dec 12, 2023 at 2:48=E2=80=AFPM Lukasz Luba <lukasz.luba@arm.com> w=
rote:
>
> When the thermal instance's weight is updated from the sysfs the governor
> update_tz() callback is triggered. Implement proper reaction to this
> event in the IPA, which would save CPU cycles spent in throttle().
> This will speed-up the main throttle() IPA function and clean it up
> a bit.
>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
>  drivers/thermal/gov_power_allocator.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_=
power_allocator.c
> index 574aa5822112..a9f1549e6355 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -61,6 +61,7 @@ static inline s64 div_frac(s64 x, s64 y)
>   *                     @trip_switch_on should be NULL.
>   * @trip_max:          last passive trip point of the thermal zone. The
>   *                     temperature we are controlling for.
> + * @total_weight:      Sum of all thermal instances weights
>   * @num_actors:                number of cooling devices supporting IPA =
callbacks
>   * @buffer_size:       IPA internal buffer size
>   * @req_power:         IPA buffer for requested power
> @@ -76,6 +77,7 @@ struct power_allocator_params {
>         u32 sustainable_power;
>         const struct thermal_trip *trip_switch_on;
>         const struct thermal_trip *trip_max;
> +       int total_weight;
>         int num_actors;
>         int buffer_size;
>         u32 *req_power;
> @@ -403,16 +405,11 @@ static int allocate_power(struct thermal_zone_devic=
e *tz, int control_temp)
>         u32 total_req_power =3D 0;
>         u32 *weighted_req_power;
>         u32 power_range, weight;
> -       int total_weight =3D 0;
>         int i =3D 0;
>
>         if (!params->num_actors)
>                 return -ENODEV;
>
> -       list_for_each_entry(instance, &tz->thermal_instances, tz_node)
> -               if (power_actor_is_valid(params, instance))
> -                       total_weight +=3D instance->weight;
> -
>         /* Clean all buffers for new power estimations */
>         memset(params->req_power, 0, params->buffer_size);
>
> @@ -430,7 +427,7 @@ static int allocate_power(struct thermal_zone_device =
*tz, int control_temp)
>                 if (cdev->ops->get_requested_power(cdev, &req_power[i]))
>                         continue;
>
> -               if (!total_weight)
> +               if (!params->total_weight)
>                         weight =3D 1 << FRAC_BITS;
>                 else
>                         weight =3D instance->weight;
> @@ -666,6 +663,12 @@ static void power_allocator_update_tz(struct thermal=
_zone_device *tz,
>
>                 allocate_actors_buffer(params, num_actors);
>                 break;
> +       case THERMAL_INSTANCE_WEIGHT_UPDATE:
> +               params->total_weight =3D 0;
> +               list_for_each_entry(instance, &tz->thermal_instances, tz_=
node)
> +                       if (power_actor_is_valid(params, instance))
> +                               params->total_weight +=3D instance->weigh=
t;
> +               break;
>         default:
>                 break;
>         }
> --

This one looks good to me, but if you decide to follow my advice from
the previous comments, it will need to be adjusted.