From 9fc3097585a1adf659851d13ae78bd971c06a2ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
Date: Wed, 17 Nov 2021 20:31:01 +0200
Subject: drm/i915: Move vrr push after the frame counter sampling again
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Git-commit: 4765d061d50559ce3addc9a86433c35f48d76085
Patch-mainline: v5.17-rc1
References: jsc#PED-1166 jsc#PED-1168 jsc#PED-1170 jsc#PED-1218 jsc#PED-1220 jsc#PED-1222 jsc#PED-1223 jsc#PED-1225
Moving the vrr push to happen before sampling the frame counter
was wrong. If we are already in vblank when the push is sent
the vblank exit will start immediately which causes the sampled
frame counter to correspond to the next frame instead of the current
frame.
So put things back into the original order (except we should
keep the vrr push within the irq disable section to avoid
pointless irq related delays here).
We'll just have to accept the tiny race that exists between
sampling the frame counter vs. vrr push. And let's at least
document said race properly in a comment.
I suppose we could try to minimize the race by sampling the frame
counter just before sending the push, but that would require
changing drm_crtc_arm_vblank_event() to accept a caller provided
vblank counter value, so leave it be for now. Another thing we
could do is change the vblank evasion to account for the case
where a push was already sent. That would anyway be required
for mailbox style updates. Currently mailbox updates are only
used by the legacy cursor, but we don't do a vrr push for those.
Cc: Manasi Navare <manasi.d.navare@intel.com>
Fixes: 6f9976bd1310 ("drm/i915: Do vrr push before sampling the frame counter")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20211117183103.27418-1-ville.syrjala@linux.intel.com
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Acked-by: Patrik Jakobsson <pjakobsson@suse.de>
---
drivers/gpu/drm/i915/display/intel_crtc.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index f09df2cf052b..cf403be7736c 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -610,9 +610,6 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
trace_intel_pipe_update_end(crtc, end_vbl_count, scanline_end);
- /* Send VRR Push to terminate Vblank */
- intel_vrr_send_push(new_crtc_state);
-
/*
* Incase of mipi dsi command mode, we need to set frame update
* request for every commit.
@@ -641,6 +638,22 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
new_crtc_state->uapi.event = NULL;
}
+ /*
+ * Send VRR Push to terminate Vblank. If we are already in vblank
+ * this has to be done _after_ sampling the frame counter, as
+ * otherwise the push would immediately terminate the vblank and
+ * the sampled frame counter would correspond to the next frame
+ * instead of the current frame.
+ *
+ * There is a tiny race here (iff vblank evasion failed us) where
+ * we might sample the frame counter just before vmax vblank start
+ * but the push would be sent just after it. That would cause the
+ * push to affect the next frame instead of the current frame,
+ * which would cause the next frame to terminate already at vmin
+ * vblank start instead of vmax vblank start.
+ */
+ intel_vrr_send_push(new_crtc_state);
+
local_irq_enable();
if (intel_vgpu_active(dev_priv))
--
2.38.1