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

[Gustavo Padovan <gustavo@padovan.org>]

Hi Inki,

2015-06-17 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?

Yes.

collabora@singularity:~$ modetest -M exynos -s 31:1920x1080 -v -s
25:1366x768
setting mode 1920x1080-60Hz@XR24 on connectors 31, crtc 29
setting mode 1366x768-60Hz@XR24 on connectors 25, crtc 23
freq: 55.82Hz
freq: 55.38Hz
freq: 55.30Hz
freq: 55.38Hz
freq: 55.30Hz
freq: 55.38Hz
freq: 55.30Hz
freq: 55.38Hz
freq: 55.30Hz
freq: 55.38Hz

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

I have a patch for it already. I'll resend a v2 of my last series. 

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

We have about 3 months to fix this, until v4.2 is out, not one week. And
it is working for FIMD and Mixer.

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