<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">2013/5/22 Inki Dae <span dir="ltr"><<a href="mailto:inki.dae@samsung.com" target="_blank">inki.dae@samsung.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div class="im">2013/5/22 Stéphane Marchesin <span dir="ltr"><<a href="mailto:stephane.marchesin@gmail.com" target="_blank">stephane.marchesin@gmail.com</a>></span><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>On Tue, May 21, 2013 at 9:22 PM, Inki Dae <<a href="mailto:inki.dae@samsung.com" target="_blank">inki.dae@samsung.com</a>> wrote:<br>
><br>
><br>
><br>
> 2013/5/22 Stéphane Marchesin <<a href="mailto:stephane.marchesin@gmail.com" target="_blank">stephane.marchesin@gmail.com</a>><br>
>><br>
>> On Tue, May 21, 2013 at 1:08 AM, Inki Dae <<a href="mailto:inki.dae@samsung.com" target="_blank">inki.dae@samsung.com</a>> wrote:<br>
>> > This patch fixes the issue that drm_vblank_get() is failed.<br>
>> ><br>
>> > The issus occurs when next page flip request is tried<br>
>> > if previous page flip event wasn't completed yet and then<br>
>> > dpms became off.<br>
>> ><br>
>> > So this patch make sure that page flip event is completed<br>
>> > before dpms goes to off.<br>
>><br>
>> Hi,<br>
>><br>
>> This patch is a squash of the two following patches from the Chrome OS<br>
>> tree with the KDS bits removed and the dpms off bit added:<br>
>><br>
>><br>
>> <a href="http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel-next.git;a=commitdiff;h=2e77cd4e423967862ca01b1af82aa8b5b7429fc4;hp=aba002da4c6e5efec4d43e1ce33930a79269349a" target="_blank">http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel-next.git;a=commitdiff;h=2e77cd4e423967862ca01b1af82aa8b5b7429fc4;hp=aba002da4c6e5efec4d43e1ce33930a79269349a</a><br>
>><br>
>> <a href="http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel-next.git;a=commitdiff;h=b4ec8bfa750ef43a43c2da746c8afdbb51002882;hp=4f28b9a75c928f229443d7c6c3163159ceb6903a" target="_blank">http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel-next.git;a=commitdiff;h=b4ec8bfa750ef43a43c2da746c8afdbb51002882;hp=4f28b9a75c928f229443d7c6c3163159ceb6903a</a><br>
>><br>
>> Please keep proper attribution.<br>
>><br>
><br>
> Those patches are just for Chrome OS. Please post them if you want for those<br>
> to be considered so that they can be reviewed.<br>
<br>
</div>We intend to, once they are rebased onto latest kernel. But what I'm<br>
pointing out is that you're removing proper attribution from Chrome OS<br>
patches, and this is the second time it has happened.<br></blockquote><div><br></div></div><div>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.<br>
</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
> That is why we attend open<br>
> source. One more comment, please do not abuse exynos_drm_crtc_page_flip()<br>
><br>
<br>
</div>What do you mean by abuse?<br>
<div><div><br>
>><br>
>> Signed-off-by: Inki Dae <<a href="mailto:inki.dae@samsung.com" target="_blank">inki.dae@samsung.com</a>><br>
>> Signed-off-by: Kyungmin Park <<a href="mailto:kyungmin.park@samsung.com" target="_blank">kyungmin.park@samsung.com</a>><br>
>> ---<br>
>> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 16 ++++++++++++++++<br>
>> 1 files changed, 16 insertions(+), 0 deletions(-)<br>
>><br>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c<br>
>> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c<br>
>> index e8894bc..69a77e9 100644<br>
>> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c<br>
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c<br>
>> @@ -48,6 +48,8 @@ struct exynos_drm_crtc {<br>
>> unsigned int pipe;<br>
>> unsigned int dpms;<br>
>> enum exynos_crtc_mode mode;<br>
>> + wait_queue_head_t pending_flip_queue;<br>
>> + atomic_t pending_flip;<br>
>> };<br>
>><br>
>> static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)<br>
>> @@ -61,6 +63,13 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc,<br>
>> int mode)<br>
>> return;<br>
>> }<br>
>><br>
>> + if (mode > DRM_MODE_DPMS_ON) {<br>
>> + /* wait for the completion of page flip. */<br>
>> + wait_event(exynos_crtc->pending_flip_queue,<br>
>> + atomic_read(&exynos_crtc->pending_flip) ==<br>
>> 0);<br>
>> + drm_vblank_off(crtc->dev, exynos_crtc->pipe);<br>
><br>
>> You should be using vblank_put/get.<br>
>><br>
><br>
> No, drm_vblank_put should be called by exynos_drm_crtc_finish_pageflip().<br>
> And know that this patch makes sure that pended page flip event is completed<br>
> before dpms goes to off.<br>
<br>
</div></div>You need to do vblank_put/get while you're waiting. Otherwise you<br>
don't know for sure that flips will happen. This is especially bad as<br>
you don't use a timeout.<br></blockquote><div><br></div></div></div><div>Understood. Right, drm_vblank_get call before wait_event and drm_vblank_put call after wait_event.<br><br></div></div></div></div></blockquote>
<div><br></div><div>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.<br></div><div><br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div>Thanks,<br>Inki Dae<br></div><div><div class="h5"><div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span><font color="#888888"><br>
Stéphane<br>
</font></span><div><div><br>
><br>
> Thanks,<br>
> Inki Dae<br>
><br>
>> Stéphane<br>
>><br>
>> > + }<br>
>> > +<br>
>> > exynos_drm_fn_encoder(crtc, &mode,<br>
>> > exynos_drm_encoder_crtc_dpms);<br>
>> > exynos_crtc->dpms = mode;<br>
>> > }<br>
>> > @@ -225,6 +234,7 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc<br>
>> > *crtc,<br>
>> > spin_lock_irq(&dev->event_lock);<br>
>> > list_add_tail(&event->base.link,<br>
>> > &dev_priv->pageflip_event_list);<br>
>> > + atomic_set(&exynos_crtc->pending_flip, 1);<br>
>> > spin_unlock_irq(&dev->event_lock);<br>
>> ><br>
>> > crtc->fb = fb;<br>
>> > @@ -344,6 +354,8 @@ int exynos_drm_crtc_create(struct drm_device *dev,<br>
>> > unsigned int nr)<br>
>> ><br>
>> > exynos_crtc->pipe = nr;<br>
>> > exynos_crtc->dpms = DRM_MODE_DPMS_OFF;<br>
>> > + init_waitqueue_head(&exynos_crtc->pending_flip_queue);<br>
>> > + atomic_set(&exynos_crtc->pending_flip, 0);<br>
>> > exynos_crtc->plane = exynos_plane_init(dev, 1 << nr, true);<br>
>> > if (!exynos_crtc->plane) {<br>
>> > kfree(exynos_crtc);<br>
>> > @@ -398,6 +410,8 @@ void exynos_drm_crtc_finish_pageflip(struct<br>
>> > drm_device *dev, int crtc)<br>
>> > {<br>
>> > struct exynos_drm_private *dev_priv = dev->dev_private;<br>
>> > struct drm_pending_vblank_event *e, *t;<br>
>> > + struct drm_crtc *drm_crtc = dev_priv->crtc[crtc];<br>
>> > + struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(drm_crtc);<br>
>> > struct timeval now;<br>
>> > unsigned long flags;<br>
>> ><br>
>> > @@ -419,6 +433,8 @@ void exynos_drm_crtc_finish_pageflip(struct<br>
>> > drm_device *dev, int crtc)<br>
>> > list_move_tail(&e->base.link,<br>
>> > &e->base.file_priv->event_list);<br>
>> > wake_up_interruptible(&e->base.file_priv->event_wait);<br>
>> > drm_vblank_put(dev, crtc);<br>
>> > + atomic_set(&exynos_crtc->pending_flip, 0);<br>
>> > + wake_up(&exynos_crtc->pending_flip_queue);<br>
>> > }<br>
>> ><br>
>> > spin_unlock_irqrestore(&dev->event_lock, flags);<br>
>> > --<br>
>> > 1.7.5.4<br>
>> ><br>
>> > _______________________________________________<br>
>> > dri-devel mailing list<br>
>> > <a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a><br>
>> > <a href="http://lists.freedesktop.org/mailman/listinfo/dri-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a><br>
>> _______________________________________________<br>
>> dri-devel mailing list<br>
>> <a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a><br>
>> <a href="http://lists.freedesktop.org/mailman/listinfo/dri-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a><br>
><br>
><br>
_______________________________________________<br>
dri-devel mailing list<br>
<a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/dri-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a><br>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>