Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/display: Split i845 and i9xx cursor updates into arm/noarm
@ 2026-09-19 17:39 George Karagounis
  2026-09-21  7:11 ` sashiko-bot
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: George Karagounis @ 2026-09-19 17:39 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin, intel-gfx,
	dri-devel, George Karagounis

There was a problem with the i845 and i9xx cursor update functions
they handled both position updates and control/base/size updates in
a single armed sequence. This prevented the cursor position from
being updated asynchronously, which can make the cursor feel laggy

To fix this i split the updates into two phases
1. noarm Calculates and writes the CURPOS register immediately.
2. arm Calculates and writes CURCNTR, CURBASE, and CURSIZE to
   latch the control and memory updates at vblank

Also handled a hardware quirk on i9xx platforms where CURPOS requires
a CURBASE write to arm the update. The arm phase now writes CURBASE
even if only the position changed

This resolves two inline TODOs and improves cursor responsiveness

Signed-off-by: George Karagounis <mail@taterr.org>
---
 drivers/gpu/drm/i915/display/intel_cursor.c | 44 +++++++++++++++------
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index 0673f16f6fd0..c12a7222f2f6 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -270,14 +270,27 @@ static int i845_check_cursor(struct intel_crtc_state *crtc_state,
 	return 0;
 }
 
-/* TODO: split into noarm+arm pair */
+static void i845_cursor_update_noarm(struct intel_dsb *dsb,
+				     struct intel_plane *plane,
+				     const struct intel_crtc_state *crtc_state,
+				     const struct intel_plane_state *plane_state)
+{
+	struct intel_display *display = to_intel_display(plane);
+	u32 pos = 0;
+
+	if (plane_state && plane_state->uapi.visible)
+		pos = intel_cursor_position(crtc_state, plane_state, false);
+
+	intel_de_write_fw(display, CURPOS(display, PIPE_A), pos);
+}
+
 static void i845_cursor_update_arm(struct intel_dsb *dsb,
 				   struct intel_plane *plane,
 				   const struct intel_crtc_state *crtc_state,
 				   const struct intel_plane_state *plane_state)
 {
 	struct intel_display *display = to_intel_display(plane);
-	u32 cntl = 0, base = 0, pos = 0, size = 0;
+	u32 cntl = 0, base = 0, size = 0;
 
 	if (plane_state && plane_state->uapi.visible) {
 		unsigned int width = drm_rect_width(&plane_state->uapi.dst);
@@ -289,7 +302,6 @@ static void i845_cursor_update_arm(struct intel_dsb *dsb,
 		size = CURSOR_HEIGHT(height) | CURSOR_WIDTH(width);
 
 		base = plane_state->surf;
-		pos = intel_cursor_position(crtc_state, plane_state, false);
 	}
 
 	/* On these chipsets we can only modify the base/size/stride
@@ -301,14 +313,11 @@ static void i845_cursor_update_arm(struct intel_dsb *dsb,
 		intel_de_write_fw(display, CURCNTR(display, PIPE_A), 0);
 		intel_de_write_fw(display, CURBASE(display, PIPE_A), base);
 		intel_de_write_fw(display, CURSIZE(display, PIPE_A), size);
-		intel_de_write_fw(display, CURPOS(display, PIPE_A), pos);
 		intel_de_write_fw(display, CURCNTR(display, PIPE_A), cntl);
 
 		plane->cursor.base = base;
 		plane->cursor.size = size;
 		plane->cursor.cntl = cntl;
-	} else {
-		intel_de_write_fw(display, CURPOS(display, PIPE_A), pos);
 	}
 }
 
@@ -645,7 +654,21 @@ static void skl_write_cursor_wm(struct intel_dsb *dsb,
 			   skl_cursor_ddb_reg_val(ddb));
 }
 
-/* TODO: split into noarm+arm pair */
+static void i9xx_cursor_update_noarm(struct intel_dsb *dsb,
+				     struct intel_plane *plane,
+				     const struct intel_crtc_state *crtc_state,
+				     const struct intel_plane_state *plane_state)
+{
+	struct intel_display *display = to_intel_display(plane);
+	enum pipe pipe = plane->pipe;
+	u32 pos = 0;
+
+	if (plane_state && plane_state->uapi.visible)
+		pos = intel_cursor_position(crtc_state, plane_state, false);
+
+	intel_de_write_dsb(display, dsb, CURPOS(display, pipe), pos);
+}
+
 static void i9xx_cursor_update_arm(struct intel_dsb *dsb,
 				   struct intel_plane *plane,
 				   const struct intel_crtc_state *crtc_state,
@@ -653,7 +676,7 @@ static void i9xx_cursor_update_arm(struct intel_dsb *dsb,
 {
 	struct intel_display *display = to_intel_display(plane);
 	enum pipe pipe = plane->pipe;
-	u32 cntl = 0, base = 0, pos = 0, fbc_ctl = 0;
+	u32 cntl = 0, base = 0, fbc_ctl = 0;
 
 	if (plane_state && plane_state->uapi.visible) {
 		int width = drm_rect_width(&plane_state->uapi.dst);
@@ -666,7 +689,6 @@ static void i9xx_cursor_update_arm(struct intel_dsb *dsb,
 			fbc_ctl = CUR_FBC_EN | CUR_FBC_HEIGHT(height - 1);
 
 		base = plane_state->surf;
-		pos = intel_cursor_position(crtc_state, plane_state, false);
 	}
 
 	/*
@@ -703,14 +725,12 @@ static void i9xx_cursor_update_arm(struct intel_dsb *dsb,
 		if (HAS_CUR_FBC(display))
 			intel_de_write_dsb(display, dsb, CUR_FBC_CTL(display, pipe), fbc_ctl);
 		intel_de_write_dsb(display, dsb, CURCNTR(display, pipe), cntl);
-		intel_de_write_dsb(display, dsb, CURPOS(display, pipe), pos);
 		intel_de_write_dsb(display, dsb, CURBASE(display, pipe), base);
 
 		plane->cursor.base = base;
 		plane->cursor.size = fbc_ctl;
 		plane->cursor.cntl = cntl;
 	} else {
-		intel_de_write_dsb(display, dsb, CURPOS(display, pipe), pos);
 		intel_de_write_dsb(display, dsb, CURBASE(display, pipe), base);
 	}
 }
@@ -1019,6 +1039,7 @@ intel_cursor_plane_create(struct intel_display *display,
 	if (display->platform.i845g || display->platform.i865g) {
 		cursor->max_stride = i845_cursor_max_stride;
 		cursor->min_alignment = i845_cursor_min_alignment;
+		cursor->update_noarm = i845_cursor_update_noarm;
 		cursor->update_arm = i845_cursor_update_arm;
 		cursor->disable_arm = i845_cursor_disable_arm;
 		cursor->get_hw_state = i845_cursor_get_hw_state;
@@ -1036,6 +1057,7 @@ intel_cursor_plane_create(struct intel_display *display,
 		if (intel_scanout_needs_vtd_wa(display))
 			cursor->vtd_guard = 2;
 
+		cursor->update_noarm = i9xx_cursor_update_noarm;
 		cursor->update_arm = i9xx_cursor_update_arm;
 		cursor->disable_arm = i9xx_cursor_disable_arm;
 		cursor->get_hw_state = i9xx_cursor_get_hw_state;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-25 19:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-19 17:39 [PATCH] drm/i915/display: Split i845 and i9xx cursor updates into arm/noarm George Karagounis
2026-09-21  7:11 ` sashiko-bot
2026-09-21 15:36 ` ✗ LGCI.VerificationFailed: failure for " Patchwork
2026-09-23 14:10 ` ✓ i915.CI.BAT: success for drm/i915/display: Split i845 and i9xx cursor updates into arm/noarm (rev2) Patchwork
2026-09-23 14:11 ` [PATCH] drm/i915/display: Split i845 and i9xx cursor updates into arm/noarm Ville Syrjälä
2026-09-24 16:25   ` George Karagounis
2026-09-24  8:34 ` ✓ i915.CI.Full: success for drm/i915/display: Split i845 and i9xx cursor updates into arm/noarm (rev2) Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox