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@vger.kernel.org # dependency for subsequent patch Signed-off-by: Brian Norris briannorris@chromium.org
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