Re: [PATCH -v2] drm/exynos: track vblank events on a per crtc basis

2015-01-30 Joonyoung Shim jy0922.shim@samsung.com:

+Cc Inki,

Hi,

On 01/30/2015 02:10 AM, Gustavo Padovan wrote:

...

From: Mandeep Singh Baines msb@chromium.org

The goal of the change is to make sure we send the vblank event on the current vblank. My hope is to fix any races that might be causing flicker. After this change I only see a flicker in the transition plymouth and X11.

Simplified the code by tracking vblank events on a per-crtc basis. This allowed me to remove all error paths from the callback. It also allowed me to remove the vblank wait from the callback.

Signed-off-by: Mandeep Singh Baines msb@chromium.org Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk

---

drivers/gpu/drm/exynos/exynos_drm_crtc.c | 29 +++++++---------------------- drivers/gpu/drm/exynos/exynos_drm_drv.c | 19 ------------------- drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 ++---- 3 files changed, 9 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index a85c451..91c175b 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -34,9 +34,8 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode) if (mode > DRM_MODE_DPMS_ON) { /* wait for the completion of page flip. */ if (!wait_event_timeout(exynos_crtc->pending_flip_queue,

• 		!atomic_read(&exynos_crtc->pending_flip),
  

• 		HZ/20))
  

• 	atomic_set(&exynos_crtc->pending_flip, 0);
  

• 		(exynos_crtc->event == NULL), HZ/20))
  

• 	exynos_crtc->event = NULL;
  

  drm_crtc_vblank_off(crtc); }

@@ -166,7 +165,6 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc, uint32_t page_flip_flags) { struct drm_device *dev = crtc->dev;

• struct exynos_drm_private *dev_priv = dev->dev_private; struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc); struct drm_framebuffer *old_fb = crtc->primary->fb; unsigned int crtc_w, crtc_h;

@@ -195,9 +193,7 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc, }

spin_lock_irq(&dev->event_lock);

• list_add_tail(&event->base.link,
  

• 		&dev_priv->pageflip_event_list);
  

• atomic_set(&exynos_crtc->pending_flip, 1);
  

• exynos_crtc->event = event;
  

We will lose unfinished prior events by this change. That's why we use linked list.

I think you are right, but I was using exynos_crtc->event to do exactly the same as exynos_crtc->pending_flip. So we were losing a event in exynos_drm_crtc_dpms() before too. I change this patch to have a page_flip list on the crtc.

Gustavo