Commit 2bdba9d4 authored by Brian Norris's avatar Brian Norris Committed by Sean Paul

drm/rockchip: vop: Leave vblank enabled in self-refresh

If we disable vblank when entering self-refresh, vblank APIs (like
DRM_IOCTL_WAIT_VBLANK) no longer work. But user space is not aware when
we enter self-refresh, so this appears to be an API violation -- that
DRM_IOCTL_WAIT_VBLANK fails with EINVAL whenever the display is idle and
enters self-refresh.

The downstream driver used by many of these systems never used to
disable vblank for PSR, and in fact, even upstream, we didn't do that
until radically redesigning the state machine in commit 6c836d96
("drm/rockchip: Use the helpers for PSR").

Thus, it seems like a reasonable API fix to simply restore that
behavior, and leave vblank enabled.

Note that this appears to potentially unbalance the
drm_crtc_vblank_{off,on}() calls in some cases, but:
(a) drm_crtc_vblank_on() documents this as OK and
(b) if I do the naive balancing, I find state machine issues such that
    we're not in sync properly; so it's easier to take advantage of (a).

This issue was exposed by IGT's kms_vblank tests, and reported by
KernelCI. The bug has been around a while (longer than KernelCI
noticed), but was only exposed once self-refresh was bugfixed more
recently, and so KernelCI could properly test it. Some other notes in:

  https://lore.kernel.org/dri-devel/Y6OCg9BPnJvimQLT@google.com/
  Re: renesas/master bisection: igt-kms-rockchip.kms_vblank.pipe-A-wait-forked on rk3399-gru-kevin

== Backporting notes: ==

Marking as 'Fixes' commit 6c836d96 ("drm/rockchip: Use the helpers
for PSR"), but it probably depends on commit bed030a4
("drm/rockchip: Don't fully disable vop on self refresh") as well.

We also need the previous patch ("drm/atomic: Allow vblank-enabled +
self-refresh "disable""), of course.

v3:
 * no update

v2:
 * skip unnecessary lock/unlock

Fixes: 6c836d96 ("drm/rockchip: Use the helpers for PSR")
Cc: <stable@vger.kernel.org>
Reported-by: default avatar"kernelci.org bot" <bot@kernelci.org>
Link: https://lore.kernel.org/dri-devel/Y5itf0+yNIQa6fU4@sirena.org.uk/Signed-off-by: default avatarBrian Norris <briannorris@chromium.org>
Signed-off-by: default avatarSean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20230109171809.v3.2.Ic07cba4ab9a7bd3618a9e4258b8f92ea7d10ae5a@changeid
parent 9d0e3cac
......@@ -714,13 +714,13 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc,
if (crtc->state->self_refresh_active)
rockchip_drm_set_win_enabled(crtc, false);
if (crtc->state->self_refresh_active)
goto out;
mutex_lock(&vop->vop_lock);
drm_crtc_vblank_off(crtc);
if (crtc->state->self_refresh_active)
goto out;
/*
* Vop standby will take effect at end of current frame,
* if dsp hold valid irq happen, it means standby complete.
......@@ -754,9 +754,9 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc,
vop_core_clks_disable(vop);
pm_runtime_put(vop->dev);
out:
mutex_unlock(&vop->vop_lock);
out:
if (crtc->state->event && !crtc->state->active) {
spin_lock_irq(&crtc->dev->event_lock);
drm_crtc_send_vblank_event(crtc, crtc->state->event);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment