On 21/09/2020 14:20, Lukasz Luba wrote: > Devfreq cooling needs to now the correct status of the device in order > to operate. Do not rely on Devfreq last_status which might be a stale data > and get more up-to-date values of the load. > > Devfreq framework can change the device status in the background. To > mitigate this situation make a copy of the status structure and use it > for internal calculations. > > In addition this patch adds normalization function, which also makes sure > that whatever data comes from the device, it is in a sane range. > > Signed-off-by: Lukasz Luba <lukasz.luba@xxxxxxx> > --- > drivers/thermal/devfreq_cooling.c | 52 +++++++++++++++++++++++++------ > 1 file changed, 43 insertions(+), 9 deletions(-) > > diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c > index 7063ccb7b86d..cf045bd4d16b 100644 > --- a/drivers/thermal/devfreq_cooling.c > +++ b/drivers/thermal/devfreq_cooling.c > @@ -227,6 +227,24 @@ static inline unsigned long get_total_power(struct devfreq_cooling_device *dfc, > voltage); > } > > +static void _normalize_load(struct devfreq_dev_status *status) > +{ > + /* Make some space if needed */ > + if (status->busy_time > 0xffff) { > + status->busy_time >>= 10; > + status->total_time >>= 10; > + } > + > + if (status->busy_time > status->total_time) > + status->busy_time = status->total_time; > + > + status->busy_time *= 100; > + status->busy_time /= status->total_time ? : 1; > + > + /* Avoid division by 0 */ > + status->busy_time = status->busy_time ? : 1; > + status->total_time = 100; > +} Not sure that works if the devfreq governor is not on-demand. Is it possible to use the energy model directly in devfreq to compute the energy consumption given the state transitions since the last reading ? The power will be read directly from devfreq which will return: nrj + (current_power * (jiffies - last_update)) / (jiffies - last_update) The devfreq cooling device driver would result in a much simpler code, no? > static int devfreq_cooling_get_requested_power(struct thermal_cooling_device *cdev, > struct thermal_zone_device *tz, > @@ -234,14 +252,22 @@ static int devfreq_cooling_get_requested_power(struct thermal_cooling_device *cd > { > struct devfreq_cooling_device *dfc = cdev->devdata; > struct devfreq *df = dfc->devfreq; > - struct devfreq_dev_status *status = &df->last_status; > + struct devfreq_dev_status status; > unsigned long state; > - unsigned long freq = status->current_frequency; > + unsigned long freq; > unsigned long voltage; > u32 dyn_power = 0; > u32 static_power = 0; > int res; > > + mutex_lock(&df->lock); > + res = df->profile->get_dev_status(df->dev.parent, &status); > + mutex_unlock(&df->lock); > + if (res) > + return res; > + > + freq = status.current_frequency; > + > state = freq_get_state(dfc, freq); > if (state == THERMAL_CSTATE_INVALID) { > res = -EAGAIN; > @@ -269,16 +295,18 @@ static int devfreq_cooling_get_requested_power(struct thermal_cooling_device *cd > } else { > dyn_power = dfc->power_table[state]; > > + _normalize_load(&status); > + > /* Scale dynamic power for utilization */ > - dyn_power *= status->busy_time; > - dyn_power /= status->total_time; > + dyn_power *= status.busy_time; > + dyn_power /= status.total_time; > /* Get static power */ > static_power = get_static_power(dfc, freq); > > *power = dyn_power + static_power; > } > > - trace_thermal_power_devfreq_get_power(cdev, status, freq, *power); > + trace_thermal_power_devfreq_get_power(cdev, &status, freq, *power); > > return 0; > fail: > @@ -312,14 +340,20 @@ static int devfreq_cooling_power2state(struct thermal_cooling_device *cdev, > { > struct devfreq_cooling_device *dfc = cdev->devdata; > struct devfreq *df = dfc->devfreq; > - struct devfreq_dev_status *status = &df->last_status; > - unsigned long freq = status->current_frequency; > + struct devfreq_dev_status status; > unsigned long busy_time; > + unsigned long freq; > s32 dyn_power; > u32 static_power; > s32 est_power; > int i; > > + mutex_lock(&df->lock); > + status = df->last_status; > + mutex_unlock(&df->lock); > + > + freq = status.current_frequency; > + > if (dfc->power_ops->get_real_power) { > /* Scale for resource utilization */ > est_power = power * dfc->res_util; > @@ -331,8 +365,8 @@ static int devfreq_cooling_power2state(struct thermal_cooling_device *cdev, > dyn_power = dyn_power > 0 ? dyn_power : 0; > > /* Scale dynamic power for utilization */ > - busy_time = status->busy_time ?: 1; > - est_power = (dyn_power * status->total_time) / busy_time; > + busy_time = status.busy_time ?: 1; > + est_power = (dyn_power * status.total_time) / busy_time; > } > > /* > -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel