2013/5/22 Inki Dae > > > > 2013/5/22 Stéphane Marchesin > >> On Tue, May 21, 2013 at 9:22 PM, Inki Dae wrote: >> > >> > >> > >> > 2013/5/22 Stéphane Marchesin >> >> >> >> On Tue, May 21, 2013 at 1:08 AM, Inki Dae >> 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 >> >> Signed-off-by: Kyungmin Park >> >> --- >> >> 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. > > drm_vblank_get/put are needed really? I think that exynos_crtc->pending_flip is 0 if vblank interrrupt was disabled so wait_event will be returned just. > 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@lists.freedesktop.org >> >> > http://lists.freedesktop.org/mailman/listinfo/dri-devel >> >> _______________________________________________ >> >> dri-devel mailing list >> >> dri-devel@lists.freedesktop.org >> >> http://lists.freedesktop.org/mailman/listinfo/dri-devel >> > >> > >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel >> > >