Re: [PATCH v10 00/17] drm/exynos: atomic modesetting support

[Inki Dae <inki.dae@samsung.com>]

Hi Gustavo,

On 2015년 06월 17일 05:35, Gustavo Padovan wrote:
> HI Inki,
> 
> 2015-06-15 Inki Dae <inki.dae@samsung.com>:
> 
>> Hi Gustavo,
>>
>> On 2015년 06월 02일 00:04, Gustavo Padovan wrote:
>>> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>>>
>>> Hi,
>>>
>>> Here goes the full support for atomic modesetting on exynos. I've
>>> split the patches in the various phases of atomic support.
>>>
>>> v2: fixes comments by Joonyoung
>>>         - remove unused var in patch 09
>>>         - use ->disable instead of outdated ->dpms in hdmi code
>>>         - remove WARN_ON from crtc enable/disable
>>>
>>> v3: fixes comment by Joonyoung
>>>         - move the removal of drm_helper_disable_unused_functions() to
>>>         separated patch
>>>
>>> v4: add patches that remove unnecessary calls to disable_plane()
>>>
>>> v5: fixes NULL CRTC crash on planes updates (reported by Inki and Tobias)
>>>
>>> v6: rebase on latest exynos_drm_next
>>>
>>> v7: fix comments by Joonyoung
>>>         - fix two checkpatch errors
>>>         - remove extra crtc->commit() call
>>>         - check for null fb on exynos_check_plane()
>>>
>>> v8: fix comments by Joonyoung
>>>         - fix merge error
>>>         - move drm_crtc_vblank_get to the commit that introduces atomic pageflip
>>>         - remove .prepare() in the apropiated patch
>>>         - add a new patch to move exynos_drm_crtc_disable()
>>>
>>> v9:  * fix comments by Joonyoung
>>>         - also remove encoder .prepare()
>>>         - do not shift exynos_update_plane() parameters
>>>         - remove unused .mode_set() and .mode_set_base()
>>>      * add specific exynos .atomic_commit()
>>>      * add split of exynos_crtc->ops->dpms() into enable() and disable()
>>>      * add other atomic clean ups
>>>
>>> v10: * fix comments by Joonyoung
>>> 	- add more comments on exynos_atomic_commit()
>>> 	- make exynos_crtc's .enable and .disable void
>>
>> I found out one issue that refresh rate gets low with display extension
>> mode test.
>>
>> I tested it with two crtc drivers - vidi and fimd on Trats2 board. Here
>> is how to test it,
>>
>> #echo 1 > /sys/devices/platform/exynos-drm-vidi/connection
>> # modetest -M exynos -v -s 31@29:720x1280 -s 23@21:640x480
>> setting mode 720x1280-60Hz@XR24 on connectors 31, crtc 29
>> setting mode 640x480-75Hz@XR24 on connectors 23, crtc 21
>> freq: 20.00Hz
>> freq: 20.00Hz
>>
>> As you can see, refresh rate is 20.
>>
>> Below is the result without atomic patch series,
>> # modetest -M exynos -v -s 31@29:720x1280 -s 23@21:640x480
>> setting mode 720x1280-60Hz@XR24 on connectors 31, crtc 29
>> setting mode 640x480-75Hz@XR24 on connectors 23, crtc 21
>> freq: 60.18Hz
>> freq: 49.09Hz
>>
>> I can see 60Hz for FIMD and 49Hz for vidi.
> 
> I gave this a try and figured out that this might be a vidi specific
> problem. If I try VIDI and FIMD I get the same results as you but with
> Mixer and FIMD(the setup I actually use to test) everything works fine.

Hm... Did Mixer and FIMD combination really work correctly with
extension mode?

> So somehow my patches caused a regression on vidi that I'm still
> ivestigating.

I think this issue is because page flip operations are performed in
atomic: if I see correctly, when page flip is requested by modetest, the
call flow is as follows,

drm_atomic_helper_page_flip
	drm_atomic_async_commit
		exynos_atomic_commit
			...
			drm_atomic_helper_wait_for_vblanks
			...

However, the function, drm_atomic_helper_wait_for_vblanks called by
exynos_atomic_commit, waits for the vblank completion of each crtc
driver . See wait_event_timeout function call in
drm_atomic_helper_wait_for_vblanks function.

This means that a page flip request of a crtc driver cannot be performed
until the vblank completion of another crtc driver. I think you should
have implemented exynos_atomic_commit function asynchronously, not
synchronously like you did. So it seems that this function should be
re-implemented.

If my analysis is correct and you cannot post the change set within this
week, I'm not sure whether the atomic patch series should go to
mainline. Anyway, I will decide it and have a pull-request at the end of
this week at least.

Thanks,
Inki Dae

> 
> 	Gustavo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel