Messages in this thread | | | From | Xuewen Yan <> | Date | Mon, 13 Mar 2023 09:40:58 +0800 | Subject | Re: [PATCH] thermal/core/power_allocator: avoid cdev->state can not be reset |
| |
Hi Lukasz
On Fri, Mar 10, 2023 at 11:56 PM Lukasz Luba <lukasz.luba@arm.com> wrote: > > Hi Di, > > On 3/9/23 13:55, Di Shen wrote: > > Commit 0952177f2a1f (thermal/core/power_allocator: Update once cooling devices when temp is low) > > add a update flag to update cooling device only once when temp is low. > > But when the switch_on_temp is set to be a higher value, the cooling device state > > may not be reset to max, because the last_temp is smaller than the switch_on_temp. > > > > For example: > > First: > > swicth_on_temp=70 control_temp=85; > > > > Then userspace change the trip_temp: > > swicth_on_temp=45 control_temp=55 cur_temp=54 > > > > Then userspace reset the trip_temp: > > swicth_on_temp=70 control_temp=85 cur_temp=57 last_temp=54 > > > > At this time, the cooling device state should be reset to be max. > > However, because cur_temp(57) < switch_on_temp(70) > > last_temp(54) < swicth_on_temp(70) --> update = false > > When update is false, the cooling device state can not be reset. > > That's a tricky use case. How is that now possible,
We use the trip_temp in the Android System. Often, we set different control temperatures in different scenarios, and when we change the switch_on_temp from small to bigger, we find the power can not be reset to be max.
> > > > So delete the update condition, so that the cooling device state > > could be reset. > > IMO this is not the desired solution. Daniel reported the issue that > IPA triggers the event sent to user-space even when there is no need. > That's the motivation for the 0952177f2a1f change. > > To address your scenario properly, we need an interface which allows > to respond properly for such situation when someone from user-space > writes a new value to something fundamental as trip point. > > You also have a kernel config enabled: > CONFIG_THERMAL_WRITABLE_TRIPS > which IMO is only for debug kernels for system integrator (according > to the Kconfig description).
Yes, we use it to meet the temperature control needs of different scenarios. And now in android with google's GKI2.0, the config must be opened.
> > When you disable this config in your deploy/product kernel > than this issue would disappear. > > > > > Fixes: 0952177f2a1f (thermal/core/power_allocator: Update once cooling devices when temp is low) > > Signed-off-by: Di Shen <di.shen@unisoc.com> > > --- > > drivers/thermal/gov_power_allocator.c | 9 +++------ > > 1 file changed, 3 insertions(+), 6 deletions(-) > > > > That's why IMO this is not the solution.
Yes, but I think we should fix the bug, although the CONFIG_THERMAL_WRITABLE_TRIPS is just for debugging. How about record the last_trip_temp, and when the last_temp > last_trip_temp, set the update tobe true?
Thanks! BR, xuewen
> > Regards, > Lukasz
| |