Re: [PATCH v2 RESEND] thermal core: add option to run PM_POST_SUSPEND asynchronously

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 12/12/2023 00:25, Radu Solea wrote:
> On Wed, Dec 6, 2023 at 3:23 AM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>
>>
>> Hi Radu,
>>
>> On 06/12/2023 02:20, Radu Solea wrote:
>>> On Wed, Nov 29, 2023 at 4:20 AM Daniel Lezcano
>>> <daniel.lezcano@linaro.org> wrote:
>>>>
>>>> On 21/11/2023 00:40, Radu Solea wrote:
>>>>> Some thermal zones are bus connected and slow to resume, thus
>>>>> delaying actions which depend on completion of PM_POST_SUSPEND.
>>>>> Add optional execution path to resume thermal zones on the system
>>>>> unbounded workqueue.
>>>>>
>>>>> Signed-off-by: Radu Solea <radusolea@google.com>
>>>>> ---
>>>>
>>>> This async change may have a lot of hidden implications.
>>>>
>>>> Could you elaborate more the issue and how the async will fix the problem?
>>>>
>>>> If you have a platform being slow to resume, can you provide numbers
>>>> with and without this option?
>>>>
>>>> Thanks
>>>>      -- D.
>>>>
>>>> --
>>>> <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
>>>>
>>>
>>> In multicore systems PM_POST_SUSPEND is executed on a single core.
>>> Any work done in the notification chain delays all subsequent actions
>>> in the chain with respect to system time, including the completion of
>>> the write() to /sys/power/state.
>>> I didn't include numbers from my system since they are likely
>>> irrelevant for other systems out there. The particular number I'm
>>> chasing is ~50ms.
>>> This comes from having on-board peripherals as thermal zones, they
>>> execute async and significantly slower than the main core, add a/d
>>> conversions and bus delays to that and it's easy to see those numbers.
>>> Making the entire sequence synchronous to itself and async to
>>> PM_POST_SUSPEND isn't that much of a change, it allows the sequence to
>>> run on any core with spare cycles delayed with whatever the system
>>> unbounded queue load is at the time.
>>> (on my target system) I've seen consistent time gains (those same
>>> 50ms) to PM_POST_SUSPEND completion with this sequence actually
>>> completing before the chain finishes, this will vary from integration
>>> to integration.
>>
>> Sorry but I don't see how you can have a gain of 50ms by doing the
>> restore asynchronously.
>>
>> Can you give a more detailed description of the hardware? How many
>> thermal zones?
>>
>>
> I can't go into much detail about the hardware. But let's put it this
> way, if thermal_zone_device_update() takes 5 ms for each device (read
> temp, get trips, set trips, etc). Assume 5 onboard thermal zones, on a
> good day, ignoring system churn you'd get to around 25ms (already
> significant).
> Now on top of that add that these devices have multiple functions,
> like a PMIC for example. The resume sequence is the perfect time frame
> where you'd encounter more than one operation aimed at any one of
> these devices. Unless you have uncommonly smart drivers and devices,
> these will be queued.
> The driver in most cases will spin (hardly ideal, but realistic), even
> if they would yield the effect on the completion of the chain is at
> least the same or, likely, worse.
> 
> To the patch itself, I realized I've been somewhat hamfisted.
> thermal_zone_device_init() should not be deferred, and likely should
> execute for all zones before the in_suspend lock-out is released. I'll
> correct that once we've landed on something.
> 
> To my 50ms, it's almost the worst-case, but it happens way more often
> than would be comfortable.

If you call monitor_thermal_zone() instead of 
thermal_zone_device_update(), does it speed up the resume 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
>>

-- 
<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