Re: [RFC PATCH 5/5] ACPI: processor: do update when maximum cooling state changed

From: Zhang Rui <rui.zhang@intel.com>
To: Daniel Lezcano <daniel.lezcano@linaro.org>, linux-pm@vger.kernel.org
Cc: tiwai@suse.de, viresh.kumar@linaro.org
Subject: Re: [RFC PATCH 5/5] ACPI: processor: do update when maximum cooling state changed
Date: Thu, 16 Apr 2020 12:46:41 +0800	[thread overview]
Message-ID: <eccb4e6a77ecd6ebaff9bd68b751ea20caa6a0a8.camel@intel.com> (raw)
In-Reply-To: <1e65a2c3-fdeb-4f48-f477-a7ec67cffd4e@linaro.org>

On Mon, 2020-04-13 at 20:06 +0200, Daniel Lezcano wrote:
> On 13/04/2020 04:01, Zhang Rui wrote:
> > On Sun, 2020-04-12 at 12:07 +0200, Daniel Lezcano wrote:
> 
> [ ... ]
> 
> > why we can not have a cdev->max_state field, and get it updated
> > right
> > after .get_max_state().
> > and .get_max_state()  is only invoked
> > a) during cooling device registration
> > b) when cooling device update its max_state via the new API.
> > 
> > > 
> > > In the function thermal_cooling_device_stats_update():
> > > 
> > >   Is it possible to just compare the 'new_state' parameter with
> > > stats->max_state and if it is greater increase the stats table
> > > and
> > > update max_state to the new_state ?
> > > 
> > 
> > the problem is that thermal_cooling_device_stats_update() is
> > invoked
> > only if thermal zone are updated or the cur_state sysfs attribute
> > is
> > changed.
> > There is no way for a cooling device driver to tell thermal
> > framework
> > that it has changed.
> > Say, for the problem on hand, the statistics table will not be
> > updated
> > in time when cpufreq driver probed.
> 
> Except I'm missing something, the statistics are only read from
> userspace via sysfs.

I agree.
> 
> userspace is not notified about a stat change. Is it really a problem
> the table is not updated right at the probe time ?

>  Does it really matter
> if the user sees the old table until an update happens on a new
> higher
> max state ?
> 
> The table is always consistent whenever the userspace reads the
> content.

> 
> A new entry will appear only if it is used, no?
> 
Hmm, IMO, stats table is not the biggest problem here.
The problem is that thermal framework is not aware of the max_state
change, and the thermal instances are never updated according to the
new max_state.
So, we should invoke .get_max_state() in thermal_zone_device_update()
and update the thermal instances accordingly.
And then, what we need to do is just to do stats update right after
.get_max_state() being invoked.

About how to update the stats table, I think adding new entries is not
enough, because the meaning of each cooling state may change when
max_state changes, thus I'd prefer a full reset/resizing of the table.

thanks,
rui
> 
> 
> 
> 
> 