+Cc Inki,

Hi,

On 01/30/2015 02:10 AM, Gustavo Padovan wrote:
> From: Mandeep Singh Baines <m...@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 <m...@chromium.org>
> Signed-off-by: Gustavo Padovan <gustavo.pado...@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.
> 

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

Reply via email to