From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Figa Subject: Re: [PATCH] drm/exynos: wait for the completion of pending page flip Date: Wed, 22 May 2013 10:23:24 +0200 Message-ID: <3860229.dLTq0xtmYi@flatron> References: <1369123698-19503-1-git-send-email-inki.dae@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ee0-f43.google.com (mail-ee0-f43.google.com [74.125.83.43]) by gabe.freedesktop.org (Postfix) with ESMTP id 66824E5D58 for ; Wed, 22 May 2013 01:23:28 -0700 (PDT) Received: by mail-ee0-f43.google.com with SMTP id d41so974804eek.2 for ; Wed, 22 May 2013 01:23:27 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: dri-devel@lists.freedesktop.org Cc: =?ISO-8859-1?Q?St=E9phane?= Marchesin , Kyungmin Park , Seung-Woo Kim List-Id: dri-devel@lists.freedesktop.org Hi Inki, On Wednesday 22 of May 2013 15:37:14 Inki Dae wrote: > 2013/5/22 St=E9phane Marchesin > = > > On Tue, May 21, 2013 at 9:22 PM, Inki Dae = wrote: > > > 2013/5/22 St=E9phane 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=3Dchromiumos/third_party/kernel-next.g > > it;a=3Dcommitdiff;h=3D2e77cd4e423967862ca01b1af82aa8b5b7429fc4;hp=3Daba= 002da > > 4c6e5efec4d43e1ce33930a79269349a > > = > > = > > http://git.chromium.org/gitweb/?p=3Dchromiumos/third_party/kernel-next.g > > it;a=3Dcommitdiff;h=3Db4ec8bfa750ef43a43c2da746c8afdbb51002882;hp=3D4f2= 8b9a7 > > 5c928f229443d7c6c3163159ceb6903a> = > > >> 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. > = at=B7tri=B7bu=B7tion = n. 1. The act of attributing, especially the act of establishing a particular = person as the creator of a work of art [1] So yes, mainline kernel has attribution. Actually posting something as = work of someone that is not the author of the posted work is considered = bad everywhere, isn't it? However looking at those two patches linked by St=E9phane, I'm not really = sure this patch is really just a squash of them. St=E9phane, are you 100% = sure that your claims are true? Best regards, Tomasz [1] http://www.thefreedictionary.com/attribution > > > 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) > > = > > =3D=3D > > = > > >> 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=E9phane > > = > > > Thanks, > > > Inki Dae > > > = > > >> St=E9phane > > >> = > > >> > + } > > >> > + > > >> > = > > >> > exynos_drm_fn_encoder(crtc, &mode, > > >> > = > > >> > exynos_drm_encoder_crtc_dpms); > > >> > = > > >> > exynos_crtc->dpms =3D 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 =3D fb; > > >> > = > > >> > @@ -344,6 +354,8 @@ int exynos_drm_crtc_create(struct drm_device > > >> > *dev, > > >> > unsigned int nr) > > >> > = > > >> > exynos_crtc->pipe =3D nr; > > >> > exynos_crtc->dpms =3D DRM_MODE_DPMS_OFF; > > >> > = > > >> > + init_waitqueue_head(&exynos_crtc->pending_flip_queue); > > >> > + atomic_set(&exynos_crtc->pending_flip, 0); > > >> > = > > >> > exynos_crtc->plane =3D 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 =3D dev->dev_private; > > >> > struct drm_pending_vblank_event *e, *t; > > >> > = > > >> > + struct drm_crtc *drm_crtc =3D dev_priv->crtc[crtc]; > > >> > + struct exynos_drm_crtc *exynos_crtc =3D > > = > > 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_w > > >> > ait); > > >> > 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