brocken devfreq simple_ondemand for Odroid XU3/4?

[Lukasz Luba]

Hi Chanwoo,

On 6/29/20 2:43 AM, Chanwoo Choi wrote:
> Hi,
> 
> Sorry for late reply because of my perfornal issue. I count not check the email.

I hope you are good now.

> 
> On 6/26/20 8:22 PM, Bartlomiej Zolnierkiewicz wrote:
>>
>> On 6/25/20 2:12 PM, Kamil Konieczny wrote:
>>> On 25.06.2020 14:02, Lukasz Luba wrote:
>>>>
>>>>
>>>> On 6/25/20 12:30 PM, Kamil Konieczny wrote:
>>>>> Hi Lukasz,
>>>>>
>>>>> On 25.06.2020 12:02, Lukasz Luba wrote:
>>>>>> Hi Sylwester,
>>>>>>
>>>>>> On 6/24/20 4:11 PM, Sylwester Nawrocki wrote:
>>>>>>> Hi All,
>>>>>>>
>>>>>>> On 24.06.2020 12:32, Lukasz Luba wrote:
>>>>>>>> I had issues with devfreq governor which wasn't called by devfreq
>>>>>>>> workqueue. The old DELAYED vs DEFERRED work discussions and my patches
>>>>>>>> for it [1]. If the CPU which scheduled the next work went idle, the
>>>>>>>> devfreq workqueue will not be kicked and devfreq governor won't check
>>>>>>>> DMC status and will not decide to decrease the frequency based on low
>>>>>>>> busy_time.
>>>>>>>> The same applies for going up with the frequency. They both are
>>>>>>>> done by the governor but the workqueue must be scheduled periodically.
>>>>>>>
>>>>>>> As I have been working on resolving the video mixer IOMMU fault issue
>>>>>>> described here: https://patchwork.kernel.org/patch/10861757
>>>>>>> I did some investigation of the devfreq operation, mostly on Odroid U3.
>>>>>>>
>>>>>>> My conclusions are similar to what Lukasz says above. I would like to add
>>>>>>> that broken scheduling of the performance counters read and the devfreq
>>>>>>> updates seems to have one more serious implication. In each call, which
>>>>>>> normally should happen periodically with fixed interval we stop the counters,
>>>>>>> read counter values and start the counters again. But if period between
>>>>>>> calls becomes long enough to let any of the counters overflow, we will
>>>>>>> get wrong performance measurement results. My observations are that
>>>>>>> the workqueue job can be suspended for several seconds and conditions for
>>>>>>> the counter overflow occur sooner or later, depending among others
>>>>>>> on the CPUs load.
>>>>>>> Wrong bus load measurement can lead to setting too low interconnect bus
>>>>>>> clock frequency and then bad things happen in peripheral devices.
>>>>>>>
>>>>>>> I agree the workqueue issue needs to be fixed. I have some WIP code to use
>>>>>>> the performance counters overflow interrupts instead of SW polling and with
>>>>>>> that the interconnect bus clock control seems to work much better.
>>>>>>>
>>>>>>
>>>>>> Thank you for sharing your use case and investigation results. I think
>>>>>> we are reaching a decent number of developers to maybe address this
>>>>>> issue: 'workqueue issue needs to be fixed'.
>>>>>> I have been facing this devfreq workqueue issue ~5 times in different
>>>>>> platforms.
>>>>>>
>>>>>> Regarding the 'performance counters overflow interrupts' there is one
>>>>>> thing worth to keep in mind: variable utilization and frequency.
>>>>>> For example, in order to make a conclusion in algorithm deciding that
>>>>>> the device should increase or decrease the frequency, we fix the period
>>>>>> of observation, i.e. to 500ms. That can cause the long delay if the
>>>>>> utilization of the device suddenly drops. For example we set an
>>>>>> overflow threshold to value i.e. 1000 and we know that at 1000MHz
>>>>>> and full utilization (100%) the counter will reach that threshold
>>>>>> after 500ms (which we want, because we don't want too many interrupts
>>>>>> per sec). What if suddenly utilization drops to 2% (i.e. from 5GB/s
>>>>>> to 250MB/s (what if it drops to 25MB/s?!)), the counter will reach the
>>>>>> threshold after 50*500ms = 25s. It is impossible just for the counters
>>>>>> to predict next utilization and adjust the threshold. [...]
>>>>>
>>>>> irq triggers for underflow and overflow, so driver can adjust freq
>>>>>
>>>>
>>>> Probably possible on some platforms, depends on how many PMU registers
>>>> are available, what information can be can assign to them and type of
>>>> interrupt. A lot of hassle and still - platform and device specific.
>>>> Also, drivers should not adjust the freq, governors (different types
>>>> of them with different settings that they can handle) should do it.
>>>>
>>>> What the framework can do is to take this responsibility and provide
>>>> generic way to monitor the devices (or stop if they are suspended).
>>>> That should work nicely with the governors, which try to predict the
>>>> next best frequency. From my experience the more fluctuating intervals
>>>> the governors are called, the more odd decisions they make.
>>>> That's why I think having a predictable interval i.e. 100ms is something
>>>> desirable. Tuning the governors is easier in this case, statistics
>>>> are easier to trace and interpret, solution is not to platform specific,
>>>> etc.
>>>>
>>>> Kamil do you have plans to refresh and push your next version of the
>>>> workqueue solution?
>>>
>>> I do not, as Bartek takes over my work,
>>> +CC Bartek
>>
>> Hi Lukasz,
>>
>> As you remember in January Chanwoo has proposed another idea (to allow
>> selecting workqueue type by devfreq device driver):
>>
>> "I'm developing the RFC patch and then I'll send it as soon as possible."
>> (https://lore.kernel.org/linux-pm/6107fa2b-81ad-060d-89a2-d8941ac4d17e@samsung.com/)
>>
>> "After posting my suggestion, we can discuss it"
>> (https://lore.kernel.org/linux-pm/f5c5cd64-b72c-2802-f6ea-ab3d28483260@samsung.com/)
>>
>> so we have been waiting on the patch to be posted..
> 
> Sorry for this. I'll send it within few days.


Feel free to add me on CC, I can review&test the patches if you like.

Stay safe and healthy.

Regards,
Lukasz