From b7a5e9128afbb04f3a2405002ae1380b627e311d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
Date: Fri, 11 Feb 2022 11:26:04 +0200
Subject: drm/i915: Make cursor plane registers unlocked
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Git-commit: 0225a90981c83408e9606b52a65edee1c6891106
Patch-mainline: v5.19-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
Drop the locks around cursor plane register writes. The
lock isn't needed since each plane's register are neatly
contained on their own cachelines.
The locking did have a secondary effect of disabling
interrupts around the cursor registers writes though.
If we drop that then we open outselves up for sceduling
delays and whatnot while on the middle of the register
writes. That increases the chance of not all the register
writes land during the same frame. For normal atomic
commits this is not a concern as the vblank evade mechanism
anyway disables interrupts around the update, but the legacy
cursor codepath does not. Technically we should do a vblank
evade there as well, but so far no one has bothered to hook
that up. So in the meantime let's put an explicit local irq
disable/enable around the legacy cursor update to keep the
race window minimal.
v2: local_irq_{disable,enable}() for legacy cursor ioctl
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220211092604.393-1-ville.syrjala@linux.intel.com
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Acked-by: Patrik Jakobsson <pjakobsson@suse.de>
---
drivers/gpu/drm/i915/display/intel_cursor.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index 2ade8fdd9bdd..b648be744cf2 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -255,7 +255,6 @@ static void i845_cursor_update_arm(struct intel_plane *plane,
{
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
u32 cntl = 0, base = 0, pos = 0, size = 0;
- unsigned long irqflags;
if (plane_state && plane_state->uapi.visible) {
unsigned int width = drm_rect_width(&plane_state->uapi.dst);
@@ -270,8 +269,6 @@ static void i845_cursor_update_arm(struct intel_plane *plane,
pos = intel_cursor_position(plane_state);
}
- spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
/* On these chipsets we can only modify the base/size/stride
* whilst the cursor is disabled.
*/
@@ -290,8 +287,6 @@ static void i845_cursor_update_arm(struct intel_plane *plane,
} else {
intel_de_write_fw(dev_priv, CURPOS(PIPE_A), pos);
}
-
- spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
}
static void i845_cursor_disable_arm(struct intel_plane *plane,
@@ -492,7 +487,6 @@ static void i9xx_cursor_update_arm(struct intel_plane *plane,
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
enum pipe pipe = plane->pipe;
u32 cntl = 0, base = 0, pos = 0, fbc_ctl = 0;
- unsigned long irqflags;
if (plane_state && plane_state->uapi.visible) {
int width = drm_rect_width(&plane_state->uapi.dst);
@@ -508,8 +502,6 @@ static void i9xx_cursor_update_arm(struct intel_plane *plane,
pos = intel_cursor_position(plane_state);
}
- spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
/*
* On some platforms writing CURCNTR first will also
* cause CURPOS to be armed by the CURBASE write.
@@ -555,8 +547,6 @@ static void i9xx_cursor_update_arm(struct intel_plane *plane,
intel_de_write_fw(dev_priv, CURPOS(pipe), pos);
intel_de_write_fw(dev_priv, CURBASE(pipe), base);
}
-
- spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
}
static void i9xx_cursor_disable_arm(struct intel_plane *plane,
@@ -715,6 +705,14 @@ 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.
+ */
+ local_irq_disable();
+
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);
@@ -722,6 +720,8 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
intel_plane_disable_arm(plane, crtc_state);
}
+ local_irq_enable();
+
intel_plane_unpin_fb(old_plane_state);
out_free:
--
2.38.1