[PATCH -v2] drm/exynos: track vblank events on a per crtc basis
Joonyoung Shim
jy0922.shim at samsung.com
Thu Jan 29 18:44:56 PST 2015
+Cc Inki,
Hi,
On 01/30/2015 02:10 AM, Gustavo Padovan wrote:
> From: Mandeep Singh Baines <msb at 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 at chromium.org>
> Signed-off-by: Gustavo Padovan <gustavo.padovan at 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.
Thanks.
> spin_unlock_irq(&dev->event_lock);
>
> crtc->primary->fb = fb;
> @@ -209,11 +205,7 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
> 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;
> }
> @@ -315,7 +307,6 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
> return ERR_PTR(-ENOMEM);
>
> init_waitqueue_head(&exynos_crtc->pending_flip_queue);
> - atomic_set(&exynos_crtc->pending_flip, 0);
>
> exynos_crtc->dpms = DRM_MODE_DPMS_OFF;
> exynos_crtc->pipe = pipe;
> @@ -382,26 +373,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)
> {
> 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 exynos_drm_crtc *exynos_crtc = to_exynos_crtc(drm_crtc);
> unsigned long flags;
>
> spin_lock_irqsave(&dev->event_lock, flags);
> + if (exynos_crtc->event) {
>
> - list_for_each_entry_safe(e, t, &dev_priv->pageflip_event_list,
> - 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_send_vblank_event(dev, -1, exynos_crtc->event);
> drm_vblank_put(dev, pipe);
> - atomic_set(&exynos_crtc->pending_flip, 0);
> wake_up(&exynos_crtc->pending_flip_queue);
> +
> }
>
> + exynos_crtc->event = NULL;
> spin_unlock_irqrestore(&dev->event_lock, flags);
> }
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index b60fd9b..731cdbc 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -61,7 +61,6 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
> if (!private)
> return -ENOMEM;
>
> - INIT_LIST_HEAD(&private->pageflip_event_list);
> dev_set_drvdata(dev->dev, dev);
> dev->dev_private = (void *)private;
>
> @@ -237,27 +236,9 @@ static void exynos_drm_preclose(struct drm_device *dev,
>
> 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;
> - unsigned long flags;
> -
> if (!file->driver_priv)
> return;
>
> - /* Release all events not unhandled by page flip handler. */
> - 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);
> - }
> - }
> -
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> -
> kfree(file->driver_priv);
> file->driver_priv = NULL;
> }
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index d490b49..7411af2 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -216,6 +216,7 @@ enum exynos_crtc_mode {
> * this pipe value.
> * @dpms: store the crtc dpms value
> * @mode: store the crtc mode value
> + * @event: vblank event that is currently queued for flip
> * @ops: pointer to callbacks for exynos drm specific functionality
> * @ctx: A pointer to the crtc's implementation specific context
> */
> @@ -226,7 +227,7 @@ struct exynos_drm_crtc {
> unsigned int dpms;
> enum exynos_crtc_mode mode;
> wait_queue_head_t pending_flip_queue;
> - atomic_t pending_flip;
> + struct drm_pending_vblank_event *event;
> struct exynos_drm_crtc_ops *ops;
> void *ctx;
> };
> @@ -256,9 +257,6 @@ struct drm_exynos_file_private {
> struct exynos_drm_private {
> 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
> * this array is used to be aware of which crtc did it request vblank.
>
More information about the dri-devel
mailing list