Commit e752747b authored by Mandeep Singh Baines's avatar Mandeep Singh Baines Committed by Inki Dae

drm/exynos: track vblank events on a per crtc basis

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: default avatarMandeep Singh Baines <msb@chromium.org>
Signed-off-by: default avatarGustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: default avatarInki Dae <inki.dae@samsung.com>
parent 5d09a67f
...@@ -34,9 +34,8 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode) ...@@ -34,9 +34,8 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)
if (mode > DRM_MODE_DPMS_ON) { if (mode > DRM_MODE_DPMS_ON) {
/* wait for the completion of page flip. */ /* wait for the completion of page flip. */
if (!wait_event_timeout(exynos_crtc->pending_flip_queue, if (!wait_event_timeout(exynos_crtc->pending_flip_queue,
!atomic_read(&exynos_crtc->pending_flip), (exynos_crtc->event == NULL), HZ/20))
HZ/20)) exynos_crtc->event = NULL;
atomic_set(&exynos_crtc->pending_flip, 0);
drm_crtc_vblank_off(crtc); drm_crtc_vblank_off(crtc);
} }
...@@ -164,11 +163,10 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc, ...@@ -164,11 +163,10 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
uint32_t page_flip_flags) uint32_t page_flip_flags)
{ {
struct drm_device *dev = crtc->dev; 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 exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
struct drm_framebuffer *old_fb = crtc->primary->fb; struct drm_framebuffer *old_fb = crtc->primary->fb;
unsigned int crtc_w, crtc_h; unsigned int crtc_w, crtc_h;
int ret = -EINVAL; int ret;
/* when the page flip is requested, crtc's dpms should be on */ /* when the page flip is requested, crtc's dpms should be on */
if (exynos_crtc->dpms > DRM_MODE_DPMS_ON) { if (exynos_crtc->dpms > DRM_MODE_DPMS_ON) {
...@@ -176,48 +174,49 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc, ...@@ -176,48 +174,49 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
return -EINVAL; return -EINVAL;
} }
mutex_lock(&dev->struct_mutex); if (!event)
return -EINVAL;
if (event) { spin_lock_irq(&dev->event_lock);
/* if (exynos_crtc->event) {
* the pipe from user always is 0 so we can set pipe number ret = -EBUSY;
* of current owner to event. goto out;
*/ }
event->pipe = exynos_crtc->pipe;
ret = drm_vblank_get(dev, exynos_crtc->pipe); ret = drm_vblank_get(dev, exynos_crtc->pipe);
if (ret) { if (ret) {
DRM_DEBUG("failed to acquire vblank counter\n"); DRM_DEBUG("failed to acquire vblank counter\n");
goto out;
}
goto out; exynos_crtc->event = event;
} spin_unlock_irq(&dev->event_lock);
/*
* the pipe from user always is 0 so we can set pipe number
* of current owner to event.
*/
event->pipe = exynos_crtc->pipe;
crtc->primary->fb = fb;
crtc_w = fb->width - crtc->x;
crtc_h = fb->height - crtc->y;
ret = exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
crtc_w, crtc_h, crtc->x, crtc->y,
crtc_w, crtc_h);
if (ret) {
crtc->primary->fb = old_fb;
spin_lock_irq(&dev->event_lock); spin_lock_irq(&dev->event_lock);
list_add_tail(&event->base.link, exynos_crtc->event = NULL;
&dev_priv->pageflip_event_list); drm_vblank_put(dev, exynos_crtc->pipe);
atomic_set(&exynos_crtc->pending_flip, 1);
spin_unlock_irq(&dev->event_lock); spin_unlock_irq(&dev->event_lock);
return ret;
crtc->primary->fb = fb;
crtc_w = fb->width - crtc->x;
crtc_h = fb->height - crtc->y;
ret = exynos_update_plane(crtc->primary, crtc, fb, 0, 0,
crtc_w, crtc_h, crtc->x, crtc->y,
crtc_w, crtc_h);
if (ret) {
crtc->primary->fb = old_fb;
spin_lock_irq(&dev->event_lock);
drm_vblank_put(dev, exynos_crtc->pipe);
list_del(&event->base.link);
atomic_set(&exynos_crtc->pending_flip, 0);
spin_unlock_irq(&dev->event_lock);
goto out;
}
} }
return 0;
out: out:
mutex_unlock(&dev->struct_mutex); spin_unlock_irq(&dev->event_lock);
return ret; return ret;
} }
...@@ -255,7 +254,6 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev, ...@@ -255,7 +254,6 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
return ERR_PTR(-ENOMEM); return ERR_PTR(-ENOMEM);
init_waitqueue_head(&exynos_crtc->pending_flip_queue); init_waitqueue_head(&exynos_crtc->pending_flip_queue);
atomic_set(&exynos_crtc->pending_flip, 0);
exynos_crtc->dpms = DRM_MODE_DPMS_OFF; exynos_crtc->dpms = DRM_MODE_DPMS_OFF;
exynos_crtc->pipe = pipe; exynos_crtc->pipe = pipe;
...@@ -313,26 +311,20 @@ void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int pipe) ...@@ -313,26 +311,20 @@ void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int pipe)
void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int pipe) void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int pipe)
{ {
struct exynos_drm_private *dev_priv = dev->dev_private; struct exynos_drm_private *dev_priv = dev->dev_private;
struct drm_pending_vblank_event *e, *t;
struct drm_crtc *drm_crtc = dev_priv->crtc[pipe]; struct drm_crtc *drm_crtc = dev_priv->crtc[pipe];
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(drm_crtc); struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(drm_crtc);
unsigned long flags; unsigned long flags;
spin_lock_irqsave(&dev->event_lock, flags); spin_lock_irqsave(&dev->event_lock, flags);
if (exynos_crtc->event) {
list_for_each_entry_safe(e, t, &dev_priv->pageflip_event_list, drm_send_vblank_event(dev, -1, exynos_crtc->event);
base.link) {
/* if event's pipe isn't same as crtc then ignore it. */
if (pipe != e->pipe)
continue;
list_del(&e->base.link);
drm_send_vblank_event(dev, -1, e);
drm_vblank_put(dev, pipe); drm_vblank_put(dev, pipe);
atomic_set(&exynos_crtc->pending_flip, 0);
wake_up(&exynos_crtc->pending_flip_queue); wake_up(&exynos_crtc->pending_flip_queue);
} }
exynos_crtc->event = NULL;
spin_unlock_irqrestore(&dev->event_lock, flags); spin_unlock_irqrestore(&dev->event_lock, flags);
} }
......
...@@ -60,7 +60,6 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags) ...@@ -60,7 +60,6 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
if (!private) if (!private)
return -ENOMEM; return -ENOMEM;
INIT_LIST_HEAD(&private->pageflip_event_list);
dev_set_drvdata(dev->dev, dev); dev_set_drvdata(dev->dev, dev);
dev->dev_private = (void *)private; dev->dev_private = (void *)private;
...@@ -223,25 +222,13 @@ static void exynos_drm_preclose(struct drm_device *dev, ...@@ -223,25 +222,13 @@ static void exynos_drm_preclose(struct drm_device *dev,
static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file) static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
{ {
struct exynos_drm_private *private = dev->dev_private;
struct drm_pending_vblank_event *v, *vt;
struct drm_pending_event *e, *et; struct drm_pending_event *e, *et;
unsigned long flags; unsigned long flags;
if (!file->driver_priv) if (!file->driver_priv)
return; return;
/* Release all events not unhandled by page flip handler. */
spin_lock_irqsave(&dev->event_lock, flags); spin_lock_irqsave(&dev->event_lock, flags);
list_for_each_entry_safe(v, vt, &private->pageflip_event_list,
base.link) {
if (v->base.file_priv == file) {
list_del(&v->base.link);
drm_vblank_put(dev, v->pipe);
v->base.destroy(&v->base);
}
}
/* Release all events handled by page flip handler but not freed. */ /* Release all events handled by page flip handler but not freed. */
list_for_each_entry_safe(e, et, &file->event_list, link) { list_for_each_entry_safe(e, et, &file->event_list, link) {
list_del(&e->link); list_del(&e->link);
......
...@@ -206,6 +206,7 @@ struct exynos_drm_crtc_ops { ...@@ -206,6 +206,7 @@ struct exynos_drm_crtc_ops {
* we can refer to the crtc to current hardware interrupt occurred through * we can refer to the crtc to current hardware interrupt occurred through
* this pipe value. * this pipe value.
* @dpms: store the crtc dpms value * @dpms: store the crtc dpms value
* @event: vblank event that is currently queued for flip
* @ops: pointer to callbacks for exynos drm specific functionality * @ops: pointer to callbacks for exynos drm specific functionality
* @ctx: A pointer to the crtc's implementation specific context * @ctx: A pointer to the crtc's implementation specific context
*/ */
...@@ -215,7 +216,7 @@ struct exynos_drm_crtc { ...@@ -215,7 +216,7 @@ struct exynos_drm_crtc {
unsigned int pipe; unsigned int pipe;
unsigned int dpms; unsigned int dpms;
wait_queue_head_t pending_flip_queue; wait_queue_head_t pending_flip_queue;
atomic_t pending_flip; struct drm_pending_vblank_event *event;
struct exynos_drm_crtc_ops *ops; struct exynos_drm_crtc_ops *ops;
void *ctx; void *ctx;
}; };
...@@ -245,9 +246,6 @@ struct drm_exynos_file_private { ...@@ -245,9 +246,6 @@ struct drm_exynos_file_private {
struct exynos_drm_private { struct exynos_drm_private {
struct drm_fb_helper *fb_helper; struct drm_fb_helper *fb_helper;
/* list head for new event to be added. */
struct list_head pageflip_event_list;
/* /*
* created crtc object would be contained at this array and * created crtc object would be contained at this array and
* this array is used to be aware of which crtc did it request vblank. * this array is used to be aware of which crtc did it request vblank.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment