[PATCH 0/3] drm/exynos: add support for dynamic zpos in DECON and FIMD

[Andrzej Hajda]

Hi again,

It seems I have missed two questions:

On 11.12.2018 09:36, Andrzej Hajda wrote:
> On 11.12.2018 09:01, Inki Dae wrote:
>> 18. 12. 11. 오후 4:49에 Andrzej Hajda 이(가) 쓴 글:
>>> On 11.12.2018 00:45, Inki Dae wrote:
>>>> Hi Andrzej,
>>>>
>>>> 18. 12. 10. 오후 4:35에 Andrzej Hajda 이(가) 쓴 글:
>>>>> Hi Inki,
>>>>>
>>>>> On 10.12.2018 03:25, Inki Dae wrote:
>>>>>> Hi Andrzej,
>>>>>>
>>>>>> 18. 12. 6. 오후 6:38에 Andrzej Hajda 이(가) 쓴 글:
>>>>>>> Hi Inki,
>>>>>>>
>>>>>>> This small patchset adds dynamic zpos support for DECON and FIMD.
>>>>>> This patch will allow user space to change zpos. However, DECON and FIMD devices have fixed priority of HW overlays.
>>>>>> This would mean that zpos change by user space doesn't guarantee correct HW overlay priority.
>>>>>>
>>>>>> Why do you want to support mutable zpos?
>>>>> Practically you have patches which proves it works, you can see that
>>>>> changing zpos value results in adequate change in displayed image.
>>>>>
>>>>> Conceptually it is just a matter of disconnecting hardware windows
>>>>> present in DECON and FIMD from DRM planes which are software entities.
>>>>>
>>>>> You can reason about it this way:
>>>>>
>>>>> - drm plane is a framebuffer plus informations how it should be
>>>>> transformed/displayed on DECON/FIMD,
>>>>>
>>>>> - hw window in DECON/FIMD is just a channel through which plane is send
>>>>> to the display.
>>>>>
>>>>> DECON and FIMD has fixed hw windows order - windows with lower numbers
>>>>> are displayed below windows with higher number. To display planes in
>>>>> given z-order you just need to send them via windows with appropriate
>>>>> index - farthest plane should go always via win0, closer one via win1,
>>>>> ..., till the last plane.
>>>>>
>>>>> So for example if you have three planes and want to display them in
>>>>> following order (first one farthest, last one closest):
>>>>>
>>>>> plane2, plane1, plane3
>>>>>
>>>>> you should map them to planes as follow:
>>>>>
>>>>> plane2 -> win0, plane1 -> win1, plane3 -> win2
>>>>>
>>>>> then for example plane2 is disabled, we will have following mapping:
>>>>>
>>>>> plane1 -> win0, plane3 -> win1, win2 - disabled
>>>> Plane1 is displayed below Plane3.
>>> And this is what we wanted, the initial order was: plane2, plane1,
>>> plane3, first farthest (or lowest if you prefer this naming schema).
>>>
>>>
>>>>> then if you change zorder of planes to: plane3, plane1:
>>>>>
>>>>> plane3 -> win0, plane1 -> win1
>>>> Plane3 is displayed below Plane1 because DECON/FIMD HW aren't able to change HW overlay priroty.
>>> No, plane3 is displayed below plane1 because we sent it via win0, if we
>>> want inverse we can send plane3 via win1 and plane1 via win0.
>>>
>>>
>>>> However, user space wanted that Plane1 is displayed below Plane3. Like this, zpos change by user space doesn't guarantee correct HW overlay priority.
>>> As I said before in this example userspace wanted plane3 below plane1,
>>> and as I wrote in comment above any order of planes user want driver is
>>> able to realize with this patch.
>>>
>>>
>>>> And there is no any reason to change zpos in user space excepting Mixer device which supports HW overlay priority change.
>>> Lack of special registry for manipulating windows order does not mean it
>>> cannot support dynamic zpos.
>>>
>> Why changing zpos in user space is required?


I didn't say it is required, but usually it is better if the driver
supports more features than less.


Regards

Andrzej


>>
>>>> In fact, we supported mutable zpos before but changed it to immutable with same reason.
>>> It was just broken if I remember correctly.
>>>
>> No, I worked well and real user used it I remember.


I tried to find the code in git history, I have found some pre-atomic
patches for zpos. It is not clear how it worked and why mutable zpos has
been removed. Do you remember reasons?


Regards

Andrzej



>>
>>>> Lastly, your patch made real user broken.
>>> Who? How?
>> Window server of Tizen didn't work and you can test it with below image - I didn't check why it doesn't. Anyway, we don't have to break real user.
>> http://download.tizen.org/snapshots/tizen/unified/latest/images/standard/mobile-wayland-armv7l-tm2/
>
> Are you saying it works with latest mainline without my patches?
>
>
> Regards
>
> Andrzej
>
>
>> Thanks,
>> Inki Dae
>>
>>> Regards
>>>
>>> Andrzej
>>>
>>>
>>>> Thanks,
>>>> Inki Dae
>>>>
>>>>> As you see there is no relation between plane number and window number,
>>>>> but window number is equal to plane's position in zpos-ordered list of
>>>>> planes and this is exact value of normalized_zpos field in drm_plane_state.
>>>>>
>>>>>
>>>>> I hope this extended explanation will clarify things.
>>>>>
>>>>>
>>>>> Regards
>>>>>
>>>>> Andrzej
>>>>>
>>>>>
>>>>>
>>>>>> Thanks,
>>>>>> Inki Dae
>>>>>>
>>>>>>> It was tested on tm2 and trats2.
>>>>>>>
>>>>>>> Regards
>>>>>>> Andrzej
>>>>>>>
>>>>>>>
>>>>>>> Andrzej Hajda (3):
>>>>>>>   drm/exynos/decon5433: add dynamic zpos support
>>>>>>>   drm/exynos/fimd: create local helper for disabling hardware window
>>>>>>>   drm/exynos/fimd: add dynamic zpos support
>>>>>>>
>>>>>>>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 23 +++++-----
>>>>>>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 42 ++++++++-----------
>>>>>>>  2 files changed, 29 insertions(+), 36 deletions(-)
>>>>>>>
>>>
>
>