Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230712084840.3594-1-di.shen@unisoc.com> <47ded4da-5c39-9132-7978-d0f8c8ee0318@arm.com>
In-Reply-To: <47ded4da-5c39-9132-7978-d0f8c8ee0318@arm.com>
From:   Di Shen <cindygm567@gmail.com>
Date:   Wed, 19 Jul 2023 18:09:12 +0800
Message-ID: <CAHYJL4o5yPEFMEHBuTCmC1d6qsw52LWr222_xibmGXcP3=052w@mail.gmail.com>
Subject: Re: [PATCH] Revert "thermal: power allocator: change the 'k_*' always
 in estimate_pid_constants()"
To:     Lukasz Luba <lukasz.luba@arm.com>
Cc:     Di Shen <di.shen@unisoc.com>, linux-pm@vger.kernel.org,
        rui.zhang@intel.com, rafael@kernel.org, daniel.lezcano@linaro.org,
        linux-kernel@vger.kernel.org, xuewen.yan@unisoc.com,
        jeson.gao@unisoc.com, orsonzhai@gmail.com, zhanglyra@gmail.com,
        amitk@kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Lukasz,
I'm happy to hear from you :)

On Wed, Jul 19, 2023 at 4:50=E2=80=AFPM Lukasz Luba <lukasz.luba@arm.com> w=
rote:
>
> Hi Di,
>
> On 7/12/23 09:48, Di Shen wrote:
> > This reverts commit 90a996544946d1d4834ec2ec8add586edd905779.
> >
> > The commit ensures that the pid constants are updated when
> > sustainable_power changes, but it makes it impossible for
> > the driver to set the pid constants when the sustainable_power
> > is not changed.
> >
> > When the driver tries to register a thermal zone device by
> > thermal_zone_device_register_with_trips(const char *type,
> >       struct thermal_trip *trips, int num_trips, int mask,
> >       void *devdata, struct thermal_zone_device_ops *ops,
> >       struct thermal_zone_params *tzp, int passive_delay,
> >       int polling_delay)
> > and passes the private thermal_zone_params structure data,
> >
> > thermal_zone_devcice_register_with_trips
> >       |
> > thermal_set_governor
> >       |
> > bind_to_tz
> >       |
> > power_allocator_bind
> >       |
> > estimate_pid_constants
> >
> > the tzp->k_* will not be the data that driver have given,
> > but the data estimated by sustainable_power.
> >
> > To make it possible for driver to add its own pid constants,
>
> That was dropped, the drivers shouldn't configure 'k_*' IPA
> parameters. There was also an ask to add those parameter
> values to the DT for setup - also not allowed.
>
> > the 'force' flag is needed to indicate whether the tzp->k_*
> > should be estimated by sustainable_power or not.
>
> We don't want to maintain many different ways of configurations,
> which can cause bugs in not tested corner cases.
>
Ok, I understand.

> Please use the user-space to change those 'k_*' parameters.
> There are this dedicated and safe sysfs interfaces for each
> thermal zone.
>
> The phones that I have on my desk do the update of 'k_*' parameters via
> sysfs. They do this in different scenarios. You can try to derive
> best 'k_*' values for your workload scenarios and than save
> them in the config file. You can update in runtime from user-space
> when you switch to your scenario (e.g. camera, game, video call).
>
Thank you for your kind suggestions, Lukasz. Now I totally understand.
Thank you.

> Regards,
> Lukasz

Best regards,
Di