[PATCH] drm/atomic-helper: check that drivers call drm_crtc_vblank_off

[PATCH] drm/atomic-helper: check that drivers call drm_crtc_vblank_off

[Daniel Vetter]

At least when they have vblank support they need to call this, or the
vblank core will happily call into their crtc->enable_vblank callback
even when the crtc is off. Which leads to a boom when the clocks are
off on most hardware (besides the inevitable confusion in the
book-keeping).

The consistency checks in drm_vblank.c will then make sure that
vblank_off/on calls are balanced, and if drivers forget to re-enable
it all the commits will stall, so I think we're covered.

It'd be nice to be able to place this check outside of commit helpers,
but tha's not really possible (due to nonblocking commits and all
that). Placing it into atomic helpers should at least cover most
drivers.

Also note that vblank support is still optional (for virtual drivers,
which tend to not have this), check for that.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index ae56d91433ff..cc9c0173e075 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -860,6 +860,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 
 	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
+		int ret;
 
 		/* Shut down everything that needs a full modeset. */
 		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
@@ -883,6 +884,14 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 			funcs->disable(crtc);
 		else
 			funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
+
+		if (!(dev->irq_enabled && dev->num_crtcs))
+			continue;
+
+		ret = drm_crtc_vblank_get(crtc);
+		WARN_ONCE(ret != -EINVAL, "driver forgot to call drm_crtc_vblank_off()\n");
+		if (ret)
+			drm_crtc_vblank_put(crtc);
 	}
 }
 
-- 
2.14.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/atomic-helper: check that drivers call drm_crtc_vblank_off

[Ville Syrjälä]

On Tue, Oct 17, 2017 at 04:59:39PM +0200, Daniel Vetter wrote:
> At least when they have vblank support they need to call this, or the
> vblank core will happily call into their crtc->enable_vblank callback
> even when the crtc is off. Which leads to a boom when the clocks are
> off on most hardware (besides the inevitable confusion in the
> book-keeping).
> 
> The consistency checks in drm_vblank.c will then make sure that
> vblank_off/on calls are balanced, and if drivers forget to re-enable
> it all the commits will stall, so I think we're covered.
> 
> It'd be nice to be able to place this check outside of commit helpers,
> but tha's not really possible (due to nonblocking commits and all
> that). Placing it into atomic helpers should at least cover most
> drivers.
> 
> Also note that vblank support is still optional (for virtual drivers,
> which tend to not have this), check for that.
> 
> Cc: Rob Clark <robdclark@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ae56d91433ff..cc9c0173e075 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -860,6 +860,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>  
>  	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
>  		const struct drm_crtc_helper_funcs *funcs;
> +		int ret;
>  
>  		/* Shut down everything that needs a full modeset. */
>  		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
> @@ -883,6 +884,14 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>  			funcs->disable(crtc);
>  		else
>  			funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
> +
> +		if (!(dev->irq_enabled && dev->num_crtcs))

num_crtcs confused me. Maybe we should rename it to num_vblank_crtcs or
something...

> +			continue;
> +
> +		ret = drm_crtc_vblank_get(crtc);
> +		WARN_ONCE(ret != -EINVAL, "driver forgot to call drm_crtc_vblank_off()\n");
> +		if (ret)

ret==0 presumably

> +			drm_crtc_vblank_put(crtc);
>  	}
>  }
>  
> -- 
> 2.14.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/atomic-helper: check that drivers call drm_crtc_vblank_off

[Daniel Vetter]

At least when they have vblank support they need to call this, or the
vblank core will happily call into their crtc->enable_vblank callback
even when the crtc is off. Which leads to a boom when the clocks are
off on most hardware (besides the inevitable confusion in the
book-keeping).

The consistency checks in drm_vblank.c will then make sure that
vblank_off/on calls are balanced, and if drivers forget to re-enable
it all the commits will stall, so I think we're covered.

It'd be nice to be able to place this check outside of commit helpers,
but tha's not really possible (due to nonblocking commits and all
that). Placing it into atomic helpers should at least cover most
drivers.

Also note that vblank support is still optional (for virtual drivers,
which tend to not have this), check for that.

v2: Fixup the handling for vblank_put (Rob).

Cc: Rob Clark <robdclark@gmail.com>
Tested-by: Rob Clark <robdclark@gmail.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index ae56d91433ff..029c8c1d97f6 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -860,6 +860,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 
 	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
+		int ret;
 
 		/* Shut down everything that needs a full modeset. */
 		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
@@ -883,6 +884,14 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 			funcs->disable(crtc);
 		else
 			funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
+
+		if (!(dev->irq_enabled && dev->num_crtcs))
+			continue;
+
+		ret = drm_crtc_vblank_get(crtc);
+		WARN_ONCE(ret != -EINVAL, "driver forgot to call drm_crtc_vblank_off()\n");
+		if (ret == 0)
+			drm_crtc_vblank_put(crtc);
 	}
 }
 
-- 
2.14.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/atomic-helper: check that drivers call drm_crtc_vblank_off

[Ville Syrjälä]

On Tue, Oct 17, 2017 at 05:27:14PM +0200, Daniel Vetter wrote:
> At least when they have vblank support they need to call this, or the
> vblank core will happily call into their crtc->enable_vblank callback
> even when the crtc is off. Which leads to a boom when the clocks are
> off on most hardware (besides the inevitable confusion in the
> book-keeping).
> 
> The consistency checks in drm_vblank.c will then make sure that
> vblank_off/on calls are balanced, and if drivers forget to re-enable
> it all the commits will stall, so I think we're covered.
> 
> It'd be nice to be able to place this check outside of commit helpers,
> but tha's not really possible (due to nonblocking commits and all
> that). Placing it into atomic helpers should at least cover most
> drivers.
> 
> Also note that vblank support is still optional (for virtual drivers,
> which tend to not have this), check for that.
> 
> v2: Fixup the handling for vblank_put (Rob).
> 
> Cc: Rob Clark <robdclark@gmail.com>
> Tested-by: Rob Clark <robdclark@gmail.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ae56d91433ff..029c8c1d97f6 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -860,6 +860,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>  
>  	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
>  		const struct drm_crtc_helper_funcs *funcs;
> +		int ret;
>  
>  		/* Shut down everything that needs a full modeset. */
>  		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
> @@ -883,6 +884,14 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>  			funcs->disable(crtc);
>  		else
>  			funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
> +
> +		if (!(dev->irq_enabled && dev->num_crtcs))
> +			continue;
> +
> +		ret = drm_crtc_vblank_get(crtc);
> +		WARN_ONCE(ret != -EINVAL, "driver forgot to call drm_crtc_vblank_off()\n");
> +		if (ret == 0)
> +			drm_crtc_vblank_put(crtc);
>  	}
>  }
>  
> -- 
> 2.14.1

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/atomic-helper: check that drivers call drm_crtc_vblank_off (rev2)

[Patchwork]

== Series Details ==

Series: drm/atomic-helper: check that drivers call drm_crtc_vblank_off (rev2)
URL   : https://patchwork.freedesktop.org/series/32141/
State : success

== Summary ==

Series 32141v2 drm/atomic-helper: check that drivers call drm_crtc_vblank_off
https://patchwork.freedesktop.org/api/1.0/series/32141/revisions/2/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                fail       -> PASS       (fi-gdg-551) fdo#102618
        Subgroup basic-flip-after-cursor-varying-size:
                skip       -> PASS       (fi-hsw-4770r)

fdo#102618 https://bugs.freedesktop.org/show_bug.cgi?id=102618

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:447s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:452s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:375s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:529s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:263s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:495s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:496s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:492s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:477s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:561s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:415s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:250s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:581s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:422s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:431s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:489s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:461s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:493s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:570s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:475s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:583s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:543s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:459s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:633s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:520s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:494s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:458s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:561s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:413s

920fa3252916e0ec9da1df709744c568f621a43d drm-tip: 2017y-10m-17d-09h-45m-06s UTC integration manifest
359dfde73041 drm/atomic-helper: check that drivers call drm_crtc_vblank_off

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6077/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/atomic-helper: check that drivers call drm_crtc_vblank_off

[Sean Paul]

On Tue, Oct 17, 2017 at 04:59:39PM +0200, Daniel Vetter wrote:
> At least when they have vblank support they need to call this, or the
> vblank core will happily call into their crtc->enable_vblank callback
> even when the crtc is off. Which leads to a boom when the clocks are
> off on most hardware (besides the inevitable confusion in the
> book-keeping).
> 
> The consistency checks in drm_vblank.c will then make sure that
> vblank_off/on calls are balanced, and if drivers forget to re-enable
> it all the commits will stall, so I think we're covered.
> 
> It'd be nice to be able to place this check outside of commit helpers,
> but tha's not really possible (due to nonblocking commits and all
> that). Placing it into atomic helpers should at least cover most
> drivers.
> 
> Also note that vblank support is still optional (for virtual drivers,
> which tend to not have this), check for that.
> 
> Cc: Rob Clark <robdclark@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ae56d91433ff..cc9c0173e075 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -860,6 +860,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>  
>  	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
>  		const struct drm_crtc_helper_funcs *funcs;
> +		int ret;
>  
>  		/* Shut down everything that needs a full modeset. */
>  		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
> @@ -883,6 +884,14 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>  			funcs->disable(crtc);
>  		else
>  			funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
> +
> +		if (!(dev->irq_enabled && dev->num_crtcs))
> +			continue;
> +
> +		ret = drm_crtc_vblank_get(crtc);
> +		WARN_ONCE(ret != -EINVAL, "driver forgot to call drm_crtc_vblank_off()\n");
> +		if (ret)

Shouldn't this be !ret

Sean

> +			drm_crtc_vblank_put(crtc);
>  	}
>  }
>  
> -- 
> 2.14.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/atomic-helper: check that drivers call drm_crtc_vblank_off

[Sean Paul]

On Tue, Oct 17, 2017 at 01:47:36PM -0400, Sean Paul wrote:
> On Tue, Oct 17, 2017 at 04:59:39PM +0200, Daniel Vetter wrote:
> > At least when they have vblank support they need to call this, or the
> > vblank core will happily call into their crtc->enable_vblank callback
> > even when the crtc is off. Which leads to a boom when the clocks are
> > off on most hardware (besides the inevitable confusion in the
> > book-keeping).
> > 
> > The consistency checks in drm_vblank.c will then make sure that
> > vblank_off/on calls are balanced, and if drivers forget to re-enable
> > it all the commits will stall, so I think we're covered.
> > 
> > It'd be nice to be able to place this check outside of commit helpers,
> > but tha's not really possible (due to nonblocking commits and all
> > that). Placing it into atomic helpers should at least cover most
> > drivers.
> > 
> > Also note that vblank support is still optional (for virtual drivers,
> > which tend to not have this), check for that.
> > 
> > Cc: Rob Clark <robdclark@gmail.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index ae56d91433ff..cc9c0173e075 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -860,6 +860,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
> >  
> >  	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
> >  		const struct drm_crtc_helper_funcs *funcs;
> > +		int ret;
> >  
> >  		/* Shut down everything that needs a full modeset. */
> >  		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
> > @@ -883,6 +884,14 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
> >  			funcs->disable(crtc);
> >  		else
> >  			funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
> > +
> > +		if (!(dev->irq_enabled && dev->num_crtcs))
> > +			continue;
> > +
> > +		ret = drm_crtc_vblank_get(crtc);
> > +		WARN_ONCE(ret != -EINVAL, "driver forgot to call drm_crtc_vblank_off()\n");
> > +		if (ret)
> 
> Shouldn't this be !ret

Annnd I should check my email before sending. Sorry, v2 is 

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> 
> Sean
> 
> > +			drm_crtc_vblank_put(crtc);
> >  	}
> >  }
> >  
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✓ Fi.CI.IGT: success for drm/atomic-helper: check that drivers call drm_crtc_vblank_off (rev2)

[Patchwork]

== Series Details ==

Series: drm/atomic-helper: check that drivers call drm_crtc_vblank_off (rev2)
URL   : https://patchwork.freedesktop.org/series/32141/
State : success

== Summary ==

Test perf:
        Subgroup oa-exponents:
                pass       -> FAIL       (shard-hsw) fdo#102254
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test kms_cursor_legacy:
        Subgroup cursor-vs-flip-atomic-transitions:
                fail       -> PASS       (shard-hsw)

fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2553 pass:1440 dwarn:0   dfail:0   fail:10  skip:1103 time:9330s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6077/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/atomic-helper: check that drivers call drm_crtc_vblank_off

[Daniel Vetter]

On Tue, Oct 17, 2017 at 06:32:52PM +0300, Ville Syrjälä wrote:
> On Tue, Oct 17, 2017 at 05:27:14PM +0200, Daniel Vetter wrote:
> > At least when they have vblank support they need to call this, or the
> > vblank core will happily call into their crtc->enable_vblank callback
> > even when the crtc is off. Which leads to a boom when the clocks are
> > off on most hardware (besides the inevitable confusion in the
> > book-keeping).
> > 
> > The consistency checks in drm_vblank.c will then make sure that
> > vblank_off/on calls are balanced, and if drivers forget to re-enable
> > it all the commits will stall, so I think we're covered.
> > 
> > It'd be nice to be able to place this check outside of commit helpers,
> > but tha's not really possible (due to nonblocking commits and all
> > that). Placing it into atomic helpers should at least cover most
> > drivers.
> > 
> > Also note that vblank support is still optional (for virtual drivers,
> > which tend to not have this), check for that.
> > 
> > v2: Fixup the handling for vblank_put (Rob).
> > 
> > Cc: Rob Clark <robdclark@gmail.com>
> > Tested-by: Rob Clark <robdclark@gmail.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Applied, thanks for your review.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index ae56d91433ff..029c8c1d97f6 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -860,6 +860,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
> >  
> >  	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
> >  		const struct drm_crtc_helper_funcs *funcs;
> > +		int ret;
> >  
> >  		/* Shut down everything that needs a full modeset. */
> >  		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
> > @@ -883,6 +884,14 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
> >  			funcs->disable(crtc);
> >  		else
> >  			funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
> > +
> > +		if (!(dev->irq_enabled && dev->num_crtcs))
> > +			continue;
> > +
> > +		ret = drm_crtc_vblank_get(crtc);
> > +		WARN_ONCE(ret != -EINVAL, "driver forgot to call drm_crtc_vblank_off()\n");
> > +		if (ret == 0)
> > +			drm_crtc_vblank_put(crtc);
> >  	}
> >  }
> >  
> > -- 
> > 2.14.1
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel