[PATCH 8/9] drm/i915: Perform vblank evasion around legacy cursor updates

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala at linux.intel.com>

Our legacy cursor updates are actually mailbox updates.
Ie. the hardware latches things once per frame on start of
vblank, but we issue an number of updates per frame,
withough any attempt to synchronize against the vblank
in software. So in theory only the last update issued
during the frame will latch, and the previous ones are
discarded.

However this can lead to problems with maintaining the
ggtt/iommu mappings as we have no idea which updates
will actually latch.

The problem is exacerbated by the hardware's annoying disarming
behaviour; any non-arming register write will disarm an already
armed update, only to be rearmed later by the arming register
(CURBASE in case of cursors). If a disarming write happens
just before the start of vblank, and the arming write happens
after start of vblank we have effectively prevented the hardware
from latching anything. And if we manage to straddle multiple
sequential vblank starts in this manner we effectively prevent
the hardware from latching any new registers for an arbitrary
amount of time. This provides more time for the (potentially
still in use by the hardware) gtt/iommu mappings to be torn
down.

A partial solution, of course, is to use vblank evasion to
avoid the register writes from spreading on both sides of
the start of vblank.

I've previously highlighted this problem as a general issue
affecting mailbox updates. I even added some notes to the
{i9xx,skl}_crtc_planes_update_arm() to remind us that the noarm
and arm phases both need to pulled into the vblank evasion
critical section if we actually decided to implement mailbox
updates in general. But as I never impelemented the noarm+arm
split for cursors we don't have to worry about that for the
moment.

We've been lucky enough so far that this hasn't really caused
problems. One thing that does help is that Xorg generally
sticks to the same cursor BO. But igt seems pretty good at
hitting this on MTL now, so apparently we have to start
thinking about this.

Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_cursor.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index 926e2de00eb5..77531838001f 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -22,6 +22,7 @@
 #include "intel_frontbuffer.h"
 #include "intel_psr.h"
 #include "intel_psr_regs.h"
+#include "intel_vblank.h"
 #include "skl_watermark.h"
 
 #include "gem/i915_gem_object.h"
@@ -647,12 +648,14 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
 {
 	struct intel_plane *plane = to_intel_plane(_plane);
 	struct intel_crtc *crtc = to_intel_crtc(_crtc);
+	struct drm_i915_private *i915 = to_i915(plane->base.dev);
 	struct intel_plane_state *old_plane_state =
 		to_intel_plane_state(plane->base.state);
 	struct intel_plane_state *new_plane_state;
 	struct intel_crtc_state *crtc_state =
 		to_intel_crtc_state(crtc->base.state);
 	struct intel_crtc_state *new_crtc_state;
+	struct intel_vblank_evade_ctx evade;
 	int ret;
 
 	/*
@@ -745,14 +748,15 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
 	 */
 	crtc_state->active_planes = new_crtc_state->active_planes;
 
-	/*
-	 * Technically we should do a vblank evasion here to make
-	 * sure all the cursor registers update on the same frame.
-	 * For now just make sure the register writes happen as
-	 * quickly as possible to minimize the race window.
-	 */
+	intel_vblank_evade_init(crtc_state, crtc_state, &evade);
+
 	local_irq_disable();
 
+	if (!drm_WARN_ON(&i915->drm, drm_crtc_vblank_get(&crtc->base))) {
+		intel_vblank_evade(&evade);
+		drm_crtc_vblank_put(&crtc->base);
+	}
+
 	if (new_plane_state->uapi.visible) {
 		intel_plane_update_noarm(plane, crtc_state, new_plane_state);
 		intel_plane_update_arm(plane, crtc_state, new_plane_state);
-- 
2.41.0