| Message ID | 542D13CC.5000304@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 2014? 10? 02? 17:58, Joonyoung Shim wrote: > Hi Andrzej, > > On 10/01/2014 05:14 PM, Andrzej Hajda wrote: >> The patch disables vblanks during dpms off only if pagefilp has >> not been finished. It also replaces drm_vblank_off with drm_crtc_vblank_put. >> It fixes issue with page_flip ioctl not being able to acquire vblank counter. > > This problem isn't related with pageflip, it just causes from > 7ffd7a68511c710b84db3548a1997fd2625f580a commit (drm: Always reject > drm_vblank_get() after drm_vblank_off()). > > We need to use drm_vblank_on() as a counterpart to drm_vblank_off() > after the commit . > > How about below patch? Thanks you Joonyoung and Andrzej, drm_vblank_on/off() are legacy api so it would be better to use drm_vblank_crtc_on/off functions instead. And drm_vblank_crtc_off() makes sure that the latest vblank frame count is stored and restored by drm_vblank_crtc_on() again. So my opinion is, static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode) { [snip] if (mode > DRM_MODE_DPMS_ON) { /* wait for the completion of page flip. */ if (!wait_event_timeout(exynos_crtc->pending_flip_queue, !atomic_read(&exynos_crtc->pending_flip), HZ/20)) atomic_set(&exynos_crtc->pending_flip, 0); drm_crtc_vblank_off(crtc); //<- store the latest vblank frame count. } else { drm_crtc_vblank_on(crtc); //<- restore the vblank frame count. } [snip] } Tested and worked well with above patch. How about it? Thanks, Inki Dae > >>From 6de01473746af225c688ee430123001d57d9af2a Mon Sep 17 00:00:00 2001 > From: Joonyoung Shim <jy0922.shim@samsung.com> > Date: Thu, 2 Oct 2014 17:48:27 +0900 > Subject: [PATCH] drm/exynos: use drm_vblank_on() > > We need to use drm_vblank_on() as a counterpart to drm_vblank_off() > after the commit 7ffd7a68511c ("drm: Always reject drm_vblank_get() > after drm_vblank_off()"). If not, drm_vblank_get() will be failed > always after drm_vblank_off(). > > Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com> > --- > drivers/gpu/drm/exynos/exynos_drm_crtc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > index 8e38e9f..dfa209a 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > @@ -71,7 +71,6 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode) > !atomic_read(&exynos_crtc->pending_flip), > HZ/20)) > atomic_set(&exynos_crtc->pending_flip, 0); > - drm_vblank_off(crtc->dev, exynos_crtc->pipe); > } > > if (manager->ops->dpms) > @@ -90,6 +89,7 @@ static void exynos_drm_crtc_commit(struct drm_crtc *crtc) > struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc); > struct exynos_drm_manager *manager = exynos_crtc->manager; > > + drm_vblank_on(crtc->dev, exynos_crtc->pipe); > exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON); > > exynos_plane_commit(crtc->primary); > @@ -177,10 +177,12 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, > > static void exynos_drm_crtc_disable(struct drm_crtc *crtc) > { > + struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc); > struct drm_plane *plane; > int ret; > > exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); > + drm_vblank_off(crtc->dev, exynos_crtc->pipe); > > drm_for_each_legacy_plane(plane, &crtc->dev->mode_config.plane_list) { > if (plane->crtc != crtc) > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, +CC possible victims On 10/02/2014 12:52 PM, Inki Dae wrote: > On 2014? 10? 02? 17:58, Joonyoung Shim wrote: >> Hi Andrzej, >> >> On 10/01/2014 05:14 PM, Andrzej Hajda wrote: >>> The patch disables vblanks during dpms off only if pagefilp has >>> not been finished. It also replaces drm_vblank_off with drm_crtc_vblank_put. >>> It fixes issue with page_flip ioctl not being able to acquire vblank counter. >> This problem isn't related with pageflip, it just causes from >> 7ffd7a68511c710b84db3548a1997fd2625f580a commit (drm: Always reject >> drm_vblank_get() after drm_vblank_off()). >> >> We need to use drm_vblank_on() as a counterpart to drm_vblank_off() >> after the commit . This patch should break also other drivers, it seems at least following drms could be affected: armada, sti, tegra. I guess after disabling and re-enabling crtc vblanks should stop working, unless there are other bugs. Regards Andrzej -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/02/2014 12:52 PM, Inki Dae wrote: > On 2014? 10? 02? 17:58, Joonyoung Shim wrote: >> Hi Andrzej, >> >> On 10/01/2014 05:14 PM, Andrzej Hajda wrote: >>> The patch disables vblanks during dpms off only if pagefilp has >>> not been finished. It also replaces drm_vblank_off with drm_crtc_vblank_put. >>> It fixes issue with page_flip ioctl not being able to acquire vblank counter. >> This problem isn't related with pageflip, it just causes from >> 7ffd7a68511c710b84db3548a1997fd2625f580a commit (drm: Always reject >> drm_vblank_get() after drm_vblank_off()). >> >> We need to use drm_vblank_on() as a counterpart to drm_vblank_off() >> after the commit . >> >> How about below patch? > Thanks you Joonyoung and Andrzej, > > drm_vblank_on/off() are legacy api so it would be better to use > drm_vblank_crtc_on/off functions instead. > > And drm_vblank_crtc_off() makes sure that the latest vblank frame count > is stored and restored by drm_vblank_crtc_on() again. So my opinion is, > > static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode) > { > [snip] > > if (mode > DRM_MODE_DPMS_ON) { > /* wait for the completion of page flip. */ > if (!wait_event_timeout(exynos_crtc->pending_flip_queue, > !atomic_read(&exynos_crtc->pending_flip), > HZ/20)) > atomic_set(&exynos_crtc->pending_flip, 0); > drm_crtc_vblank_off(crtc); //<- store the latest vblank frame count. > } else { > drm_crtc_vblank_on(crtc); //<- restore the vblank frame count. > } > > [snip] > } > > > Tested and worked well with above patch. How about it? > > drm_crtc_vblank_on should be called after dpms on, otherwise it can fail enabling vblank. I have provided full explanation in my other email [1]. You can modify your patch or just use the one provided in [1]. [1]: http://permalink.gmane.org/gmane.comp.video.dri.devel/116152 Regards Andrzej -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014? 10? 10? 21:39, Andrzej Hajda wrote: > On 10/02/2014 12:52 PM, Inki Dae wrote: >> On 2014? 10? 02? 17:58, Joonyoung Shim wrote: >>> Hi Andrzej, >>> >>> On 10/01/2014 05:14 PM, Andrzej Hajda wrote: >>>> The patch disables vblanks during dpms off only if pagefilp has >>>> not been finished. It also replaces drm_vblank_off with drm_crtc_vblank_put. >>>> It fixes issue with page_flip ioctl not being able to acquire vblank counter. >>> This problem isn't related with pageflip, it just causes from >>> 7ffd7a68511c710b84db3548a1997fd2625f580a commit (drm: Always reject >>> drm_vblank_get() after drm_vblank_off()). >>> >>> We need to use drm_vblank_on() as a counterpart to drm_vblank_off() >>> after the commit . >>> >>> How about below patch? >> Thanks you Joonyoung and Andrzej, >> >> drm_vblank_on/off() are legacy api so it would be better to use >> drm_vblank_crtc_on/off functions instead. >> >> And drm_vblank_crtc_off() makes sure that the latest vblank frame count >> is stored and restored by drm_vblank_crtc_on() again. So my opinion is, >> >> static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode) >> { >> [snip] >> >> if (mode > DRM_MODE_DPMS_ON) { >> /* wait for the completion of page flip. */ >> if (!wait_event_timeout(exynos_crtc->pending_flip_queue, >> !atomic_read(&exynos_crtc->pending_flip), >> HZ/20)) >> atomic_set(&exynos_crtc->pending_flip, 0); >> drm_crtc_vblank_off(crtc); //<- store the latest vblank frame count. >> } else { >> drm_crtc_vblank_on(crtc); //<- restore the vblank frame count. >> } >> >> [snip] >> } >> >> >> Tested and worked well with above patch. How about it? >> >> > > drm_crtc_vblank_on should be called after dpms on, otherwise it can fail enabling vblank. I have provided > full explanation in my other email [1]. > You can modify your patch or just use the one provided in [1]. I will just merge your patch set after review and test. :) Thanks, Inki Dae > > [1]: http://permalink.gmane.org/gmane.comp.video.dri.devel/116152 > > Regards > Andrzej > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index 8e38e9f..dfa209a 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -71,7 +71,6 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode) !atomic_read(&exynos_crtc->pending_flip), HZ/20)) atomic_set(&exynos_crtc->pending_flip, 0); - drm_vblank_off(crtc->dev, exynos_crtc->pipe); } if (manager->ops->dpms) @@ -90,6 +89,7 @@ static void exynos_drm_crtc_commit(struct drm_crtc *crtc) struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc); struct exynos_drm_manager *manager = exynos_crtc->manager; + drm_vblank_on(crtc->dev, exynos_crtc->pipe); exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON); exynos_plane_commit(crtc->primary); @@ -177,10 +177,12 @@ static int exynos_drm_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, static void exynos_drm_crtc_disable(struct drm_crtc *crtc) { + struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc); struct drm_plane *plane; int ret; exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); + drm_vblank_off(crtc->dev, exynos_crtc->pipe); drm_for_each_legacy_plane(plane, &crtc->dev->mode_config.plane_list) { if (plane->crtc != crtc)