Re: [PATCH v7 0/6] Exynos: Simple QoS for exynos-bus using interconnect

[Chanwoo Choi]

Hi Sylwester,

On 11/3/20 7:12 PM, Sylwester Nawrocki wrote:
> Hi Chanwoo, Georgi
> 
> On 03.11.2020 09:53, Chanwoo Choi wrote:
>> On 11/3/20 5:29 PM, Georgi Djakov wrote:
>>> On 11/3/20 09:54, Chanwoo Choi wrote:
> 
>>>> When I tested this patchset on Odroid-U3,
>>>> After setting 0 bps by interconnect[1][2],
>>>> the frequency of devfreq devs sustain the high frequency
>>>> according to the pm qos request.
>>>>
>>>> So, I try to find the cause of this situation.
>>>> In result, it seems that interconnect exynos driver
>>>> updates the pm qos request to devfreq device
>>>> during the kernel booting. Do you know why the exynos
>>>> interconnect driver request the pm qos during probe
>>>> without the mixer request?
>>>
>>> That's probably because of the sync_state support, that was introduced
>>> recently. The icc_sync_state callback needs to be added to the driver
>>> (i just left a comment on that patch), and then check again if it works.
>>>
>>> The idea of the sync_state is that there could be multiple users of a
>>> path and we must wait for all consumers to tell their bandwidth needs.
>>> Otherwise the first consumer may lower the bandwidth or disable a path
>>> needed for another consumer (driver), which has not probed yet. So we
>>> maintain a floor bandwidth until everyone has probed. By default the floor
>>> bandwidth is INT_MAX, but can be overridden by implementing the get_bw()
>>> callback.
> 
> Thanks for detailed explanation Georgi.
> 
>> Thanks for guide. I tested it with your comment of patch2.
>> It is well working without problem as I mentioned previously.
>>
>> I caught the reset operation of PM QoS requested from interconnect
>> on kernel log. In result, after completed the kernel booting,
>> there is no pm qos request if hdmi cable is not connected.
> 
> Thanks for the bug report Chanwoo, it's related to the sync_state
> feature as you guys already figured out.  I had to reorder some code 
> in the interconnect driver probe() to avoid some issues, 
> i.e. to register PM QoS request before icc_node_add() call but 
> I forgot to check initial state of the bus frequencies.
> 
> I thought the get_bw implementation might be needed but the default
> behaviour seems fine, the PM QoS derived bus frequencies will be 
> clamped in the devfreq to valid OPP values.
> 
> Chanwoo, in order to set the bandwidth to 0 we could also just blank 
> the display. Below are some of the commands I use for testing.
> 
> # blank display (disable the mixer entirely)
> echo 4 > /sys/devices/platform/exynos-drm/graphics/fb0/blank
> 
> # unblank display
> echo 0 > /sys/devices/platform/exynos-drm/graphics/fb0/blank
> 
> # modetest with 2 planes (higher bandwidth test)
> ./modetest -s 47:1920x1080 -P 45:1920x1080 -v
> 

Thanks for the test guide.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics