2013/5/22 Kyungmin Park <kmp...@infradead.org> > > > On Wed, May 22, 2013 at 5:23 PM, Tomasz Figa <tomasz.f...@gmail.com>wrote: > >> Hi Inki, >> >> On Wednesday 22 of May 2013 15:37:14 Inki Dae wrote: >> > 2013/5/22 Stéphane Marchesin <stephane.marche...@gmail.com> >> > >> > > On Tue, May 21, 2013 at 9:22 PM, Inki Dae <inki....@samsung.com> >> wrote: >> > > > 2013/5/22 Stéphane Marchesin <stephane.marche...@gmail.com> >> > > > >> > > >> On Tue, May 21, 2013 at 1:08 AM, Inki Dae <inki....@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.g >> > > it;a=commitdiff;h=2e77cd4e423967862ca01b1af82aa8b5b7429fc4;hp=aba002da >> > > 4c6e5efec4d43e1ce33930a79269349a >> > > >> > > >> > > >> http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel-next.g >> > > it;a=commitdiff;h=b4ec8bfa750ef43a43c2da746c8afdbb51002882;hp=4f28b9a7 >> > > 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. >> > Hi Stephane, > >> > > >> > > 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. >> > First, we don't know what's going on Chrome OS. probably you think we > refer your codes. but we don't know chrome codes and doesn't refer it. it's > our finding from our use case. > Second, samsung has lots of division. some engineers are working with > Chrome OS. but ours is not their division. > now we're working on anoter platform and use exynos drm. > As you know "chinese wall". we're doing it properly. >
Actually, we referred to i915 codes, intel_display.c :) > > Thank you, > Kyungmin Park > >> > >> > 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·tri·bu·tion >> 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éphane, I'm not really >> sure this patch is really just a squash of them. Stéphane, 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 <inki....@samsung.com> >> > > >> Signed-off-by: Kyungmin Park <kyungmin.p...@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_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 >> _______________________________________________ >> 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
