[PATCH] drm/exynos: wait for the completion of pending page flip

Inki Dae inki.dae at samsung.com
Tue May 21 23:37:14 PDT 2013


2013/5/22 Stéphane Marchesin <stephane.marchesin at gmail.com>

> On Tue, May 21, 2013 at 9:22 PM, Inki Dae <inki.dae at samsung.com> wrote:
> >
> >
> >
> > 2013/5/22 Stéphane Marchesin <stephane.marchesin at gmail.com>
> >>
> >> On Tue, May 21, 2013 at 1:08 AM, Inki Dae <inki.dae at samsung.com> wrote:
> >> > This patch fixes the issue that drm_vblank_get() is failed.
> >> >
> >> > The issus occurs when next page flip request is tried
> >> > if previous page flip event wasn't completed yet and then
> >> > dpms became off.
> >> >
> >> > So this patch make sure that page flip event is completed
> >> > before dpms goes to off.
> >>
> >> Hi,
> >>
> >> This patch is a squash of the two following patches from the Chrome OS
> >> tree with the KDS bits removed and the dpms off bit added:
> >>
> >>
> >>
> http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel-next.git;a=commitdiff;h=2e77cd4e423967862ca01b1af82aa8b5b7429fc4;hp=aba002da4c6e5efec4d43e1ce33930a79269349a
> >>
> >>
> http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel-next.git;a=commitdiff;h=b4ec8bfa750ef43a43c2da746c8afdbb51002882;hp=4f28b9a75c928f229443d7c6c3163159ceb6903a
> >>
> >> Please keep proper attribution.
> >>
> >
> > Those patches are just for Chrome OS. Please post them if you want for
> those
> > to be considered so that they can be reviewed.
>
> We intend to, once they are rebased onto latest kernel. But what I'm
> pointing out is that you're removing proper attribution from Chrome OS
> patches, and this is the second time it has happened.
>

What is mean? does mainline kernel have the attribution? if not so, we
don't need to consider it. And please know that I can not be aware of you
have such patch set without any posting.


>
> > That is why we attend open
> > source. One more comment, please do not abuse exynos_drm_crtc_page_flip()
> >
>
> What do you mean by abuse?
>
> >>
> >> Signed-off-by: Inki Dae <inki.dae at samsung.com>
> >> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> >> ---
> >>  drivers/gpu/drm/exynos/exynos_drm_crtc.c |   16 ++++++++++++++++
> >>  1 files changed, 16 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> >> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> >> index e8894bc..69a77e9 100644
> >> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> >> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> >> @@ -48,6 +48,8 @@ struct exynos_drm_crtc {
> >>         unsigned int                    pipe;
> >>         unsigned int                    dpms;
> >>         enum exynos_crtc_mode           mode;
> >> +       wait_queue_head_t               pending_flip_queue;
> >> +       atomic_t                        pending_flip;
> >>  };
> >>
> >>  static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)
> >> @@ -61,6 +63,13 @@ static void exynos_drm_crtc_dpms(struct drm_crtc
> *crtc,
> >> int mode)
> >>                 return;
> >>         }
> >>
> >> +       if (mode > DRM_MODE_DPMS_ON) {
> >> +               /* wait for the completion of page flip. */
> >> +               wait_event(exynos_crtc->pending_flip_queue,
> >> +                               atomic_read(&exynos_crtc->pending_flip)
> ==
> >> 0);
> >> +               drm_vblank_off(crtc->dev, exynos_crtc->pipe);
> >
> >> You should be using vblank_put/get.
> >>
> >
> > No, drm_vblank_put should be called by exynos_drm_crtc_finish_pageflip().
> > And know that this patch makes sure that pended page flip event is
> completed
> > before dpms goes to off.
>
> You need to do vblank_put/get while you're waiting. Otherwise you
> don't know for sure that flips will happen. This is especially bad as
> you don't use a timeout.
>

Understood. Right, drm_vblank_get call before wait_event and drm_vblank_put
call after wait_event.

Thanks,
Inki Dae


>
> Stéphane
>
> >
> > Thanks,
> > Inki Dae
> >
> >> Stéphane
> >>
> >> > +       }
> >> > +
> >> >         exynos_drm_fn_encoder(crtc, &mode,
> >> > exynos_drm_encoder_crtc_dpms);
> >> >         exynos_crtc->dpms = mode;
> >> >  }
> >> > @@ -225,6 +234,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);
> >> >                 spin_unlock_irq(&dev->event_lock);
> >> >
> >> >                 crtc->fb = fb;
> >> > @@ -344,6 +354,8 @@ int exynos_drm_crtc_create(struct drm_device *dev,
> >> > unsigned int nr)
> >> >
> >> >         exynos_crtc->pipe = nr;
> >> >         exynos_crtc->dpms = DRM_MODE_DPMS_OFF;
> >> > +       init_waitqueue_head(&exynos_crtc->pending_flip_queue);
> >> > +       atomic_set(&exynos_crtc->pending_flip, 0);
> >> >         exynos_crtc->plane = exynos_plane_init(dev, 1 << nr, true);
> >> >         if (!exynos_crtc->plane) {
> >> >                 kfree(exynos_crtc);
> >> > @@ -398,6 +410,8 @@ void exynos_drm_crtc_finish_pageflip(struct
> >> > drm_device *dev, int crtc)
> >> >  {
> >> >         struct exynos_drm_private *dev_priv = dev->dev_private;
> >> >         struct drm_pending_vblank_event *e, *t;
> >> > +       struct drm_crtc *drm_crtc = dev_priv->crtc[crtc];
> >> > +       struct exynos_drm_crtc *exynos_crtc =
> to_exynos_crtc(drm_crtc);
> >> >         struct timeval now;
> >> >         unsigned long flags;
> >> >
> >> > @@ -419,6 +433,8 @@ void exynos_drm_crtc_finish_pageflip(struct
> >> > drm_device *dev, int crtc)
> >> >                 list_move_tail(&e->base.link,
> >> > &e->base.file_priv->event_list);
> >> >                 wake_up_interruptible(&e->base.file_priv->event_wait);
> >> >                 drm_vblank_put(dev, crtc);
> >> > +               atomic_set(&exynos_crtc->pending_flip, 0);
> >> > +               wake_up(&exynos_crtc->pending_flip_queue);
> >> >         }
> >> >
> >> >         spin_unlock_irqrestore(&dev->event_lock, flags);
> >> > --
> >> > 1.7.5.4
> >> >
> >> > _______________________________________________
> >> > dri-devel mailing list
> >> > dri-devel at lists.freedesktop.org
> >> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130522/d2369e31/attachment-0001.html>


More information about the dri-devel mailing list