On Thu, Jan 05, 2023 at 05:40:17PM -0800, Brian Norris wrote: > The self-refresh helper framework overloads "disable" to sometimes mean > "go into self-refresh mode," and this mode activates automatically > (e.g., after some period of unchanging display output). In such cases, > the display pipe is still considered "on", and user-space is not aware > that we went into self-refresh mode. Thus, users may expect that > vblank-related features (such as DRM_IOCTL_WAIT_VBLANK) still work > properly. > > However, we trigger the WARN_ONCE() here if a CRTC driver tries to leave > vblank enabled here. > > Add a new exception, such that we allow CRTCs to be "disabled" (with > self-refresh active) with vblank interrupts still enabled. > > Cc: <stable@xxxxxxxxxxxxxxx> # dependency for subsequent patch > Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx> > --- > > drivers/gpu/drm/drm_atomic_helper.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index d579fd8f7cb8..7b5eddadebd5 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1207,6 +1207,12 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) > > if (!drm_dev_has_vblank(dev)) > continue; > + /* > + * Self-refresh is not a true "disable"; let vblank remain > + * enabled. > + */ > + if (new_crtc_state->self_refresh_active) > + continue; This very fishy, because we check in crtc_needs_disable whether this output should stay on due to self-refresh. Which means you should never end up in here. And yes vblank better work in self refresh :-) If it doesn't, then you need to fake it with a timer, that's at least what i915 has done for transparent self-refresh. We might need a few more helpers. Also, probably more igt, or is this something igt testing has uncovered? If so, please cite the igt testcase which hits this. -Daniel > > ret = drm_crtc_vblank_get(crtc); > WARN_ONCE(ret != -EINVAL, "driver forgot to call drm_crtc_vblank_off()\n"); > -- > 2.39.0.314.g84b9a713c41-goog > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch