Re: [PATCH] drm/exynos: fix vblank handling during dpms off
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(_crtc->pending_flip),
>> HZ/20))
>> atomic_set(_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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
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-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
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(_crtc->pending_flip),
> HZ/20))
> atomic_set(_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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
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-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
On Thu, Oct 9, 2014 at 7:08 PM, Thierry Reding wrote:
> On Thu, Oct 09, 2014 at 02:43:02PM +0900, Alexandre Courbot wrote:
>> On 10/02/2014 08:52 PM, Andrzej Hajda wrote:
>> >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.
>>
>> Indeed we (tegra) have just been hit by this. The problem seems to come from
>> the fact that we have been using drm_vblank_pre_modeset,
>> drm_vblank_post_modeset and drm_vblank_off conjointly. All these functions
>> depend on the value of vblank->inmodeset, and 7ffd7a68511 increases the
>> vblank reference counter only in drm_vblank_off, which can result in the
>> acquired reference never being released.
>>
>> The following seems to fix this for Tegra, by stopping using
>> drm_vblank_pre/post_modeset and relying on drm_vblank_off/on instead:
>>
>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>> index b08df07cad47..3955d81236d0 100644
>> --- a/drivers/gpu/drm/tegra/dc.c
>> +++ b/drivers/gpu/drm/tegra/dc.c
>> @@ -739,7 +739,6 @@ static const struct drm_crtc_funcs tegra_crtc_funcs = {
>>
>> static void tegra_crtc_disable(struct drm_crtc *crtc)
>> {
>> - struct tegra_dc *dc = to_tegra_dc(crtc);
>> struct drm_device *drm = crtc->dev;
>> struct drm_plane *plane;
>>
>> @@ -755,7 +754,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
>> }
>> }
>>
>> - drm_vblank_off(drm, dc->pipe);
>> + drm_crtc_vblank_off(crtc);
>> }
>>
>> static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
>> @@ -844,7 +843,7 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
>> u32 value;
>> int err;
>>
>> - drm_vblank_pre_modeset(crtc->dev, dc->pipe);
>> + drm_crtc_vblank_off(crtc);
>>
>> err = tegra_crtc_setup_clk(crtc, mode);
>> if (err) {
>> @@ -946,7 +945,7 @@ static void tegra_crtc_commit(struct drm_crtc *crtc)
>> value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
>> tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
>>
>> - drm_vblank_post_modeset(crtc->dev, dc->pipe);
>> + drm_crtc_vblank_on(crtc);
>> }
>>
>> static void tegra_crtc_load_lut(struct drm_crtc *crtc)
>>
>> Thierry, does this look ok to you?
>
> Yes, that looks like almost the same patch that I sent out yesterday.
> The difference is that I didn't replace the drm_vblank_pre_modeset()
> call with drm_vblank_off() like you did, but rather just dropped the
> former.
>
> I /think/ your version is more correct in that regard.
Feel free to take that extra line in your patch then. ;)
>
> Thierry
>
>> But there might be another issue, which is that calls to drm_vblank_get()
>> will return -EINVAL if invoked between drm_blank_off and drm_blank_on. Is
>> this really the desired behavior? Can it at least happen? If so, how are
>> drivers supposed to react to this situation?
>
> It shouldn't happen. If drm_vblank_off() and drm_vblank_on() are called
> around a modeset they should never conflict with drm_vblank_get(), at
> least on Tegra, because the modeset and page-flip IOCTLs will be
> serialized.
Ok, that's good. I was just wondering whether this case has been thought of.
Actually, and seeing how drm_vblank_pre/post_modeset() have become
useless (if not harmful), maybe it would be a good idea to come with a
fixup patch that gets rid of them altogether and make their callers
invoke drm_vblank_off/on() instead?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
On Thu, Oct 09, 2014 at 02:43:02PM +0900, Alexandre Courbot wrote:
> On 10/02/2014 08:52 PM, Andrzej Hajda wrote:
> >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.
>
> Indeed we (tegra) have just been hit by this. The problem seems to come from
> the fact that we have been using drm_vblank_pre_modeset,
> drm_vblank_post_modeset and drm_vblank_off conjointly. All these functions
> depend on the value of vblank->inmodeset, and 7ffd7a68511 increases the
> vblank reference counter only in drm_vblank_off, which can result in the
> acquired reference never being released.
>
> The following seems to fix this for Tegra, by stopping using
> drm_vblank_pre/post_modeset and relying on drm_vblank_off/on instead:
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index b08df07cad47..3955d81236d0 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -739,7 +739,6 @@ static const struct drm_crtc_funcs tegra_crtc_funcs = {
>
> static void tegra_crtc_disable(struct drm_crtc *crtc)
> {
> - struct tegra_dc *dc = to_tegra_dc(crtc);
> struct drm_device *drm = crtc->dev;
> struct drm_plane *plane;
>
> @@ -755,7 +754,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
> }
> }
>
> - drm_vblank_off(drm, dc->pipe);
> + drm_crtc_vblank_off(crtc);
> }
>
> static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
> @@ -844,7 +843,7 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
> u32 value;
> int err;
>
> - drm_vblank_pre_modeset(crtc->dev, dc->pipe);
> + drm_crtc_vblank_off(crtc);
>
> err = tegra_crtc_setup_clk(crtc, mode);
> if (err) {
> @@ -946,7 +945,7 @@ static void tegra_crtc_commit(struct drm_crtc *crtc)
> value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
> tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
>
> - drm_vblank_post_modeset(crtc->dev, dc->pipe);
> + drm_crtc_vblank_on(crtc);
> }
>
> static void tegra_crtc_load_lut(struct drm_crtc *crtc)
>
> Thierry, does this look ok to you?
Yes, that looks like almost the same patch that I sent out yesterday.
The difference is that I didn't replace the drm_vblank_pre_modeset()
call with drm_vblank_off() like you did, but rather just dropped the
former.
I /think/ your version is more correct in that regard.
Thierry
> But there might be another issue, which is that calls to drm_vblank_get()
> will return -EINVAL if invoked between drm_blank_off and drm_blank_on. Is
> this really the desired behavior? Can it at least happen? If so, how are
> drivers supposed to react to this situation?
It shouldn't happen. If drm_vblank_off() and drm_vblank_on() are called
around a modeset they should never conflict with drm_vblank_get(), at
least on Tegra, because the modeset and page-flip IOCTLs will be
serialized.
Thierry
pgpAl_lYksFXk.pgp
Description: PGP signature
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
On Thu, Oct 09, 2014 at 09:52:58AM +0100, Russell King - ARM Linux wrote: > On Thu, Oct 09, 2014 at 02:43:02PM +0900, Alexandre Courbot wrote: > > But there might be another issue, which is that calls to > > drm_vblank_get() will return -EINVAL if invoked between drm_blank_off > > and drm_blank_on. Is this really the desired behavior? Can it at least > > happen? If so, how are drivers supposed to react to this situation? > > I've not yet seen the commit which causes this problem, but I hope > that drm_wait_vblank() isn't affected by this. In current mainline, > drm_vblank_get() is used inside drm_wait_vblank(), which is called as > a result of userspace calling DRM_IOCTL_WAIT_VBLANK. > > So, what is the effect of this change on user applications making use > of the vblank wait ioctl - and is that change intended? There's no effect on user applications if the driver behaves properly. As far as I can tell, every driver that calls drm_vblank_off() but not drm_vblank_on() will break. You can easily test this by running libdrm modetest -s ... -v, which instead of toggling between the test pattern and an all-gray framebuffer will switch to the gray one once and then hang. I guess that was probably not intended, but according to the new rules all these drivers have now become buggy. So before merging this patch I think we need to fix existing drivers to avoid regressions. Thierry pgpy_4iWkMu4n.pgp Description: PGP signature
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
On Thu, Oct 09, 2014 at 02:43:02PM +0900, Alexandre Courbot wrote: > But there might be another issue, which is that calls to > drm_vblank_get() will return -EINVAL if invoked between drm_blank_off > and drm_blank_on. Is this really the desired behavior? Can it at least > happen? If so, how are drivers supposed to react to this situation? I've not yet seen the commit which causes this problem, but I hope that drm_wait_vblank() isn't affected by this. In current mainline, drm_vblank_get() is used inside drm_wait_vblank(), which is called as a result of userspace calling DRM_IOCTL_WAIT_VBLANK. So, what is the effect of this change on user applications making use of the vblank wait ioctl - and is that change intended? -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
On Thu, Oct 09, 2014 at 02:43:02PM +0900, Alexandre Courbot wrote: But there might be another issue, which is that calls to drm_vblank_get() will return -EINVAL if invoked between drm_blank_off and drm_blank_on. Is this really the desired behavior? Can it at least happen? If so, how are drivers supposed to react to this situation? I've not yet seen the commit which causes this problem, but I hope that drm_wait_vblank() isn't affected by this. In current mainline, drm_vblank_get() is used inside drm_wait_vblank(), which is called as a result of userspace calling DRM_IOCTL_WAIT_VBLANK. So, what is the effect of this change on user applications making use of the vblank wait ioctl - and is that change intended? -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
On Thu, Oct 09, 2014 at 09:52:58AM +0100, Russell King - ARM Linux wrote: On Thu, Oct 09, 2014 at 02:43:02PM +0900, Alexandre Courbot wrote: But there might be another issue, which is that calls to drm_vblank_get() will return -EINVAL if invoked between drm_blank_off and drm_blank_on. Is this really the desired behavior? Can it at least happen? If so, how are drivers supposed to react to this situation? I've not yet seen the commit which causes this problem, but I hope that drm_wait_vblank() isn't affected by this. In current mainline, drm_vblank_get() is used inside drm_wait_vblank(), which is called as a result of userspace calling DRM_IOCTL_WAIT_VBLANK. So, what is the effect of this change on user applications making use of the vblank wait ioctl - and is that change intended? There's no effect on user applications if the driver behaves properly. As far as I can tell, every driver that calls drm_vblank_off() but not drm_vblank_on() will break. You can easily test this by running libdrm modetest -s ... -v, which instead of toggling between the test pattern and an all-gray framebuffer will switch to the gray one once and then hang. I guess that was probably not intended, but according to the new rules all these drivers have now become buggy. So before merging this patch I think we need to fix existing drivers to avoid regressions. Thierry pgpy_4iWkMu4n.pgp Description: PGP signature
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
On Thu, Oct 09, 2014 at 02:43:02PM +0900, Alexandre Courbot wrote:
On 10/02/2014 08:52 PM, Andrzej Hajda wrote:
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.
Indeed we (tegra) have just been hit by this. The problem seems to come from
the fact that we have been using drm_vblank_pre_modeset,
drm_vblank_post_modeset and drm_vblank_off conjointly. All these functions
depend on the value of vblank-inmodeset, and 7ffd7a68511 increases the
vblank reference counter only in drm_vblank_off, which can result in the
acquired reference never being released.
The following seems to fix this for Tegra, by stopping using
drm_vblank_pre/post_modeset and relying on drm_vblank_off/on instead:
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index b08df07cad47..3955d81236d0 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -739,7 +739,6 @@ static const struct drm_crtc_funcs tegra_crtc_funcs = {
static void tegra_crtc_disable(struct drm_crtc *crtc)
{
- struct tegra_dc *dc = to_tegra_dc(crtc);
struct drm_device *drm = crtc-dev;
struct drm_plane *plane;
@@ -755,7 +754,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
}
}
- drm_vblank_off(drm, dc-pipe);
+ drm_crtc_vblank_off(crtc);
}
static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
@@ -844,7 +843,7 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
u32 value;
int err;
- drm_vblank_pre_modeset(crtc-dev, dc-pipe);
+ drm_crtc_vblank_off(crtc);
err = tegra_crtc_setup_clk(crtc, mode);
if (err) {
@@ -946,7 +945,7 @@ static void tegra_crtc_commit(struct drm_crtc *crtc)
value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- drm_vblank_post_modeset(crtc-dev, dc-pipe);
+ drm_crtc_vblank_on(crtc);
}
static void tegra_crtc_load_lut(struct drm_crtc *crtc)
Thierry, does this look ok to you?
Yes, that looks like almost the same patch that I sent out yesterday.
The difference is that I didn't replace the drm_vblank_pre_modeset()
call with drm_vblank_off() like you did, but rather just dropped the
former.
I /think/ your version is more correct in that regard.
Thierry
But there might be another issue, which is that calls to drm_vblank_get()
will return -EINVAL if invoked between drm_blank_off and drm_blank_on. Is
this really the desired behavior? Can it at least happen? If so, how are
drivers supposed to react to this situation?
It shouldn't happen. If drm_vblank_off() and drm_vblank_on() are called
around a modeset they should never conflict with drm_vblank_get(), at
least on Tegra, because the modeset and page-flip IOCTLs will be
serialized.
Thierry
pgpAl_lYksFXk.pgp
Description: PGP signature
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
On Thu, Oct 9, 2014 at 7:08 PM, Thierry Reding tred...@nvidia.com wrote:
On Thu, Oct 09, 2014 at 02:43:02PM +0900, Alexandre Courbot wrote:
On 10/02/2014 08:52 PM, Andrzej Hajda wrote:
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.
Indeed we (tegra) have just been hit by this. The problem seems to come from
the fact that we have been using drm_vblank_pre_modeset,
drm_vblank_post_modeset and drm_vblank_off conjointly. All these functions
depend on the value of vblank-inmodeset, and 7ffd7a68511 increases the
vblank reference counter only in drm_vblank_off, which can result in the
acquired reference never being released.
The following seems to fix this for Tegra, by stopping using
drm_vblank_pre/post_modeset and relying on drm_vblank_off/on instead:
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index b08df07cad47..3955d81236d0 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -739,7 +739,6 @@ static const struct drm_crtc_funcs tegra_crtc_funcs = {
static void tegra_crtc_disable(struct drm_crtc *crtc)
{
- struct tegra_dc *dc = to_tegra_dc(crtc);
struct drm_device *drm = crtc-dev;
struct drm_plane *plane;
@@ -755,7 +754,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
}
}
- drm_vblank_off(drm, dc-pipe);
+ drm_crtc_vblank_off(crtc);
}
static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
@@ -844,7 +843,7 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
u32 value;
int err;
- drm_vblank_pre_modeset(crtc-dev, dc-pipe);
+ drm_crtc_vblank_off(crtc);
err = tegra_crtc_setup_clk(crtc, mode);
if (err) {
@@ -946,7 +945,7 @@ static void tegra_crtc_commit(struct drm_crtc *crtc)
value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- drm_vblank_post_modeset(crtc-dev, dc-pipe);
+ drm_crtc_vblank_on(crtc);
}
static void tegra_crtc_load_lut(struct drm_crtc *crtc)
Thierry, does this look ok to you?
Yes, that looks like almost the same patch that I sent out yesterday.
The difference is that I didn't replace the drm_vblank_pre_modeset()
call with drm_vblank_off() like you did, but rather just dropped the
former.
I /think/ your version is more correct in that regard.
Feel free to take that extra line in your patch then. ;)
Thierry
But there might be another issue, which is that calls to drm_vblank_get()
will return -EINVAL if invoked between drm_blank_off and drm_blank_on. Is
this really the desired behavior? Can it at least happen? If so, how are
drivers supposed to react to this situation?
It shouldn't happen. If drm_vblank_off() and drm_vblank_on() are called
around a modeset they should never conflict with drm_vblank_get(), at
least on Tegra, because the modeset and page-flip IOCTLs will be
serialized.
Ok, that's good. I was just wondering whether this case has been thought of.
Actually, and seeing how drm_vblank_pre/post_modeset() have become
useless (if not harmful), maybe it would be a good idea to come with a
fixup patch that gets rid of them altogether and make their callers
invoke drm_vblank_off/on() instead?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
On 10/02/2014 08:52 PM, Andrzej Hajda wrote:
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.
Indeed we (tegra) have just been hit by this. The problem seems to come
from the fact that we have been using drm_vblank_pre_modeset,
drm_vblank_post_modeset and drm_vblank_off conjointly. All these
functions depend on the value of vblank->inmodeset, and 7ffd7a68511
increases the vblank reference counter only in drm_vblank_off, which can
result in the acquired reference never being released.
The following seems to fix this for Tegra, by stopping using
drm_vblank_pre/post_modeset and relying on drm_vblank_off/on instead:
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index b08df07cad47..3955d81236d0 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -739,7 +739,6 @@ static const struct drm_crtc_funcs tegra_crtc_funcs = {
static void tegra_crtc_disable(struct drm_crtc *crtc)
{
- struct tegra_dc *dc = to_tegra_dc(crtc);
struct drm_device *drm = crtc->dev;
struct drm_plane *plane;
@@ -755,7 +754,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
}
}
- drm_vblank_off(drm, dc->pipe);
+ drm_crtc_vblank_off(crtc);
}
static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
@@ -844,7 +843,7 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
u32 value;
int err;
- drm_vblank_pre_modeset(crtc->dev, dc->pipe);
+ drm_crtc_vblank_off(crtc);
err = tegra_crtc_setup_clk(crtc, mode);
if (err) {
@@ -946,7 +945,7 @@ static void tegra_crtc_commit(struct drm_crtc *crtc)
value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- drm_vblank_post_modeset(crtc->dev, dc->pipe);
+ drm_crtc_vblank_on(crtc);
}
static void tegra_crtc_load_lut(struct drm_crtc *crtc)
Thierry, does this look ok to you?
But there might be another issue, which is that calls to
drm_vblank_get() will return -EINVAL if invoked between drm_blank_off
and drm_blank_on. Is this really the desired behavior? Can it at least
happen? If so, how are drivers supposed to react to this situation?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
On 10/02/2014 08:52 PM, Andrzej Hajda wrote:
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.
Indeed we (tegra) have just been hit by this. The problem seems to come
from the fact that we have been using drm_vblank_pre_modeset,
drm_vblank_post_modeset and drm_vblank_off conjointly. All these
functions depend on the value of vblank-inmodeset, and 7ffd7a68511
increases the vblank reference counter only in drm_vblank_off, which can
result in the acquired reference never being released.
The following seems to fix this for Tegra, by stopping using
drm_vblank_pre/post_modeset and relying on drm_vblank_off/on instead:
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index b08df07cad47..3955d81236d0 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -739,7 +739,6 @@ static const struct drm_crtc_funcs tegra_crtc_funcs = {
static void tegra_crtc_disable(struct drm_crtc *crtc)
{
- struct tegra_dc *dc = to_tegra_dc(crtc);
struct drm_device *drm = crtc-dev;
struct drm_plane *plane;
@@ -755,7 +754,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
}
}
- drm_vblank_off(drm, dc-pipe);
+ drm_crtc_vblank_off(crtc);
}
static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
@@ -844,7 +843,7 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
u32 value;
int err;
- drm_vblank_pre_modeset(crtc-dev, dc-pipe);
+ drm_crtc_vblank_off(crtc);
err = tegra_crtc_setup_clk(crtc, mode);
if (err) {
@@ -946,7 +945,7 @@ static void tegra_crtc_commit(struct drm_crtc *crtc)
value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
- drm_vblank_post_modeset(crtc-dev, dc-pipe);
+ drm_crtc_vblank_on(crtc);
}
static void tegra_crtc_load_lut(struct drm_crtc *crtc)
Thierry, does this look ok to you?
But there might be another issue, which is that calls to
drm_vblank_get() will return -EINVAL if invoked between drm_blank_off
and drm_blank_on. Is this really the desired behavior? Can it at least
happen? If so, how are drivers supposed to react to this situation?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
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(_crtc->pending_flip),
HZ/20))
atomic_set(_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
> 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
> ---
> 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(_crtc->pending_flip),
> HZ/20))
> atomic_set(_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, >dev->mode_config.plane_list) {
> if (plane->crtc != crtc)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
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?
>From 6de01473746af225c688ee430123001d57d9af2a Mon Sep 17 00:00:00 2001
From: Joonyoung Shim
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
---
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(_crtc->pending_flip),
HZ/20))
atomic_set(_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, >dev->mode_config.plane_list) {
if (plane->crtc != crtc)
--
1.9.1
Thanks.
>
> Signed-off-by: Andrzej Hajda
> ---
> Hi Inki,
>
> This is fix (or just workaround) of the problem you have reported.
> Please carefully verify it, as I am not familiar with pageflip code.
>
> Regards
> Andrzej
> ---
> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> index 8e38e9f..57fa94d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> @@ -69,9 +69,10 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc,
> int mode)
> /* wait for the completion of page flip. */
> if (!wait_event_timeout(exynos_crtc->pending_flip_queue,
> !atomic_read(_crtc->pending_flip),
> - HZ/20))
> + HZ/20)) {
> atomic_set(_crtc->pending_flip, 0);
> - drm_vblank_off(crtc->dev, exynos_crtc->pipe);
> + drm_crtc_vblank_put(crtc);
> + }
> }
>
> if (manager->ops->dpms)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
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?
From 6de01473746af225c688ee430123001d57d9af2a Mon Sep 17 00:00:00 2001
From: Joonyoung Shim jy0922.s...@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.s...@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)
--
1.9.1
Thanks.
Signed-off-by: Andrzej Hajda a.ha...@samsung.com
---
Hi Inki,
This is fix (or just workaround) of the problem you have reported.
Please carefully verify it, as I am not familiar with pageflip code.
Regards
Andrzej
---
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 8e38e9f..57fa94d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -69,9 +69,10 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc,
int mode)
/* wait for the completion of page flip. */
if (!wait_event_timeout(exynos_crtc-pending_flip_queue,
!atomic_read(exynos_crtc-pending_flip),
- HZ/20))
+ HZ/20)) {
atomic_set(exynos_crtc-pending_flip, 0);
- drm_vblank_off(crtc-dev, exynos_crtc-pipe);
+ drm_crtc_vblank_put(crtc);
+ }
}
if (manager-ops-dpms)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
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.s...@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.s...@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-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drm/exynos: fix vblank handling during dpms off
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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] drm/exynos: fix vblank handling during dpms off
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.
Signed-off-by: Andrzej Hajda
---
Hi Inki,
This is fix (or just workaround) of the problem you have reported.
Please carefully verify it, as I am not familiar with pageflip code.
Regards
Andrzej
---
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 8e38e9f..57fa94d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -69,9 +69,10 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int
mode)
/* wait for the completion of page flip. */
if (!wait_event_timeout(exynos_crtc->pending_flip_queue,
!atomic_read(_crtc->pending_flip),
- HZ/20))
+ HZ/20)) {
atomic_set(_crtc->pending_flip, 0);
- drm_vblank_off(crtc->dev, exynos_crtc->pipe);
+ drm_crtc_vblank_put(crtc);
+ }
}
if (manager->ops->dpms)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] drm/exynos: fix vblank handling during dpms off
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.
Signed-off-by: Andrzej Hajda a.ha...@samsung.com
---
Hi Inki,
This is fix (or just workaround) of the problem you have reported.
Please carefully verify it, as I am not familiar with pageflip code.
Regards
Andrzej
---
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 8e38e9f..57fa94d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -69,9 +69,10 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int
mode)
/* wait for the completion of page flip. */
if (!wait_event_timeout(exynos_crtc-pending_flip_queue,
!atomic_read(exynos_crtc-pending_flip),
- HZ/20))
+ HZ/20)) {
atomic_set(exynos_crtc-pending_flip, 0);
- drm_vblank_off(crtc-dev, exynos_crtc-pipe);
+ drm_crtc_vblank_put(crtc);
+ }
}
if (manager-ops-dpms)
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
