Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Ville Syrjala <ville.syrjala@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Subject: [PATCH 7/9] drm/i915: Include crtc contributed bits in plane_state->ctl
Date: Wed,  2 Apr 2025 03:22:07 +0300	[thread overview]
Message-ID: <20250402002209.24987-8-ville.syrjala@linux.intel.com> (raw)
In-Reply-To: <20250402002209.24987-1-ville.syrjala@linux.intel.com>

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

Currently we precompute plane_state->ctl with just the plane_state
contributed bits, and the crtc contributed bits are added in ad-hoc
fashion in all the places that write the plane control register.
Clean this up by also including the crtc bits in the precomputed
value.

Technically the only place that needs the _ctl() vs. _ctl_crtc()
split after this is i9xx_plane_disable_arm() (since we have to
write the crtc bits even when disabling the plane), but I've opted
to stick to the split on all platforms anyway. I think having clear
areas of responsibility for each function is nice.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/i9xx_plane.c     | 45 +++++++++----------
 drivers/gpu/drm/i915/display/intel_cursor.c   | 14 +++---
 .../drm/i915/display/skl_universal_plane.c    | 31 ++++---------
 3 files changed, 36 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c
index 6a2609402431..ef830644bc44 100644
--- a/drivers/gpu/drm/i915/display/i9xx_plane.c
+++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
@@ -151,6 +151,24 @@ static bool i9xx_plane_has_windowing(struct intel_plane *plane)
 			i9xx_plane == PLANE_C;
 }
 
+static u32 i9xx_plane_ctl_crtc(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	u32 dspcntr = 0;
+
+	if (crtc_state->gamma_enable)
+		dspcntr |= DISP_PIPE_GAMMA_ENABLE;
+
+	if (crtc_state->csc_enable)
+		dspcntr |= DISP_PIPE_CSC_ENABLE;
+
+	if (DISPLAY_VER(display) < 5)
+		dspcntr |= DISP_PIPE_SEL(crtc->pipe);
+
+	return dspcntr;
+}
+
 static u32 i9xx_plane_ctl(const struct intel_plane_state *plane_state)
 {
 	struct intel_display *display = to_intel_display(plane_state);
@@ -350,7 +368,8 @@ i9xx_plane_check(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	plane_state->ctl = i9xx_plane_ctl(plane_state);
+	plane_state->ctl = i9xx_plane_ctl(plane_state) |
+		i9xx_plane_ctl_crtc(crtc_state);
 
 	return 0;
 }
@@ -368,24 +387,6 @@ u32 i965_plane_surf_offset(const struct intel_plane_state *plane_state)
 	return plane_state->view.color_plane[0].offset;
 }
 
-static u32 i9xx_plane_ctl_crtc(const struct intel_crtc_state *crtc_state)
-{
-	struct intel_display *display = to_intel_display(crtc_state);
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	u32 dspcntr = 0;
-
-	if (crtc_state->gamma_enable)
-		dspcntr |= DISP_PIPE_GAMMA_ENABLE;
-
-	if (crtc_state->csc_enable)
-		dspcntr |= DISP_PIPE_CSC_ENABLE;
-
-	if (DISPLAY_VER(display) < 5)
-		dspcntr |= DISP_PIPE_SEL(crtc->pipe);
-
-	return dspcntr;
-}
-
 static void i9xx_plane_ratio(const struct intel_crtc_state *crtc_state,
 			     const struct intel_plane_state *plane_state,
 			     unsigned int *num, unsigned int *den)
@@ -471,9 +472,7 @@ static void i9xx_plane_update_arm(struct intel_dsb *dsb,
 	enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
 	int x = plane_state->view.color_plane[0].x;
 	int y = plane_state->view.color_plane[0].y;
-	u32 dspcntr;
-
-	dspcntr = plane_state->ctl | i9xx_plane_ctl_crtc(crtc_state);
+	u32 dspcntr = plane_state->ctl;
 
 	/* see intel_plane_atomic_calc_changes() */
 	if (plane->need_async_flip_toggle_wa &&
@@ -602,8 +601,8 @@ g4x_primary_async_flip(struct intel_dsb *dsb,
 		       bool async_flip)
 {
 	struct intel_display *display = to_intel_display(plane);
-	u32 dspcntr = plane_state->ctl | i9xx_plane_ctl_crtc(crtc_state);
 	enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
+	u32 dspcntr = plane_state->ctl;
 
 	if (async_flip)
 		dspcntr |= DISP_ASYNC_FLIP;
diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index 8f9c8f0f4b27..debf0f9b7ff6 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -265,7 +265,8 @@ static int i845_check_cursor(struct intel_crtc_state *crtc_state,
 		return -EINVAL;
 	}
 
-	plane_state->ctl = i845_cursor_ctl(plane_state);
+	plane_state->ctl = i845_cursor_ctl(plane_state) |
+		i845_cursor_ctl_crtc(crtc_state);
 
 	return 0;
 }
@@ -283,11 +284,9 @@ static void i845_cursor_update_arm(struct intel_dsb *dsb,
 		unsigned int width = drm_rect_width(&plane_state->uapi.dst);
 		unsigned int height = drm_rect_height(&plane_state->uapi.dst);
 
-		cntl = plane_state->ctl |
-			i845_cursor_ctl_crtc(crtc_state);
-
 		size = CURSOR_HEIGHT(height) | CURSOR_WIDTH(width);
 
+		cntl = plane_state->ctl;
 		base = plane_state->surf;
 		pos = intel_cursor_position(crtc_state, plane_state, false);
 	}
@@ -524,7 +523,8 @@ static int i9xx_check_cursor(struct intel_crtc_state *crtc_state,
 		return -EINVAL;
 	}
 
-	plane_state->ctl = i9xx_cursor_ctl(plane_state);
+	plane_state->ctl = i9xx_cursor_ctl(plane_state) |
+		i9xx_cursor_ctl_crtc(crtc_state);
 
 	return 0;
 }
@@ -659,12 +659,10 @@ static void i9xx_cursor_update_arm(struct intel_dsb *dsb,
 		int width = drm_rect_width(&plane_state->uapi.dst);
 		int height = drm_rect_height(&plane_state->uapi.dst);
 
-		cntl = plane_state->ctl |
-			i9xx_cursor_ctl_crtc(crtc_state);
-
 		if (width != height)
 			fbc_ctl = CUR_FBC_EN | CUR_FBC_HEIGHT(height - 1);
 
+		cntl = plane_state->ctl;
 		base = plane_state->surf;
 		pos = intel_cursor_position(crtc_state, plane_state, false);
 	}
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index cc64d9598c17..2fd7cd76f804 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1414,20 +1414,13 @@ skl_plane_update_arm(struct intel_dsb *dsb,
 	enum pipe pipe = plane->pipe;
 	u32 x = plane_state->view.color_plane[0].x;
 	u32 y = plane_state->view.color_plane[0].y;
-	u32 plane_ctl, plane_color_ctl = 0;
-
-	plane_ctl = plane_state->ctl |
-		skl_plane_ctl_crtc(crtc_state);
+	u32 plane_ctl = plane_state->ctl;
 
 	/* see intel_plane_atomic_calc_changes() */
 	if (plane->need_async_flip_toggle_wa &&
 	    crtc_state->async_flip_planes & BIT(plane->id))
 		plane_ctl |= PLANE_CTL_ASYNC_FLIP;
 
-	if (DISPLAY_VER(display) >= 10)
-		plane_color_ctl = plane_state->color_ctl |
-			glk_plane_color_ctl_crtc(crtc_state);
-
 	intel_de_write_dsb(display, dsb, PLANE_KEYVAL(pipe, plane_id),
 			   skl_plane_keyval(plane_state));
 	intel_de_write_dsb(display, dsb, PLANE_KEYMSK(pipe, plane_id),
@@ -1447,7 +1440,7 @@ skl_plane_update_arm(struct intel_dsb *dsb,
 
 	if (DISPLAY_VER(display) >= 10)
 		intel_de_write_dsb(display, dsb, PLANE_COLOR_CTL(pipe, plane_id),
-				   plane_color_ctl);
+				   plane_state->color_ctl);
 
 	/*
 	 * Enable the scaler before the plane so that we don't
@@ -1534,10 +1527,6 @@ icl_plane_update_noarm(struct intel_dsb *dsb,
 	int y = plane_state->view.color_plane[color_plane].y;
 	int src_w = drm_rect_width(&plane_state->uapi.src) >> 16;
 	int src_h = drm_rect_height(&plane_state->uapi.src) >> 16;
-	u32 plane_color_ctl;
-
-	plane_color_ctl = plane_state->color_ctl |
-		glk_plane_color_ctl_crtc(crtc_state);
 
 	/* The scaler will handle the output position */
 	if (plane_state->scaler_id >= 0) {
@@ -1579,7 +1568,7 @@ icl_plane_update_noarm(struct intel_dsb *dsb,
 				   plane_state->cus_ctl);
 
 	intel_de_write_dsb(display, dsb, PLANE_COLOR_CTL(pipe, plane_id),
-			   plane_color_ctl);
+			   plane_state->color_ctl);
 
 	if (fb->format->is_yuv && icl_is_hdr_plane(display, plane_id))
 		icl_program_input_csc(dsb, plane, plane_state);
@@ -1623,10 +1612,6 @@ icl_plane_update_arm(struct intel_dsb *dsb,
 	struct intel_display *display = to_intel_display(plane);
 	enum plane_id plane_id = plane->id;
 	enum pipe pipe = plane->pipe;
-	u32 plane_ctl;
-
-	plane_ctl = plane_state->ctl |
-		skl_plane_ctl_crtc(crtc_state);
 
 	/*
 	 * Enable the scaler before the plane so that we don't
@@ -1646,7 +1631,7 @@ icl_plane_update_arm(struct intel_dsb *dsb,
 	 * the control register just before the surface register.
 	 */
 	intel_de_write_dsb(display, dsb, PLANE_CTL(pipe, plane_id),
-			   plane_ctl);
+			   plane_state->ctl);
 	intel_de_write_dsb(display, dsb, PLANE_SURF(pipe, plane_id),
 			   plane_state->surf);
 }
@@ -1675,8 +1660,6 @@ skl_plane_async_flip(struct intel_dsb *dsb,
 	u32 plane_ctl = plane_state->ctl;
 	u32 plane_surf = plane_state->surf;
 
-	plane_ctl |= skl_plane_ctl_crtc(crtc_state);
-
 	if (async_flip) {
 		if (DISPLAY_VER(display) >= 30)
 			plane_surf |= PLANE_SURF_ASYNC_UPDATE;
@@ -2353,10 +2336,12 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state,
 		plane_state->damage = DRM_RECT_INIT(0, 0, 0, 0);
 	}
 
-	plane_state->ctl = skl_plane_ctl(plane_state);
+	plane_state->ctl = skl_plane_ctl(plane_state) |
+		skl_plane_ctl_crtc(crtc_state);
 
 	if (DISPLAY_VER(display) >= 10)
-		plane_state->color_ctl = glk_plane_color_ctl(plane_state);
+		plane_state->color_ctl = glk_plane_color_ctl(plane_state) |
+			glk_plane_color_ctl_crtc(crtc_state);
 
 	if (intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier) &&
 	    icl_is_hdr_plane(display, plane->id))
-- 
2.45.3


  parent reply	other threads:[~2025-04-02  0:22 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-02  0:22 [PATCH 0/9] drm/i915: Precompute plane SURF address/etc Ville Syrjala
2025-04-02  0:22 ` [PATCH 1/9] drm/i915: Precompute plane SURF address Ville Syrjala
2025-04-02  0:22 ` [PATCH 2/9] drm/i915: Nuke intel_plane_ggtt_offset() Ville Syrjala
2025-04-02  0:22 ` [PATCH 3/9] drm/i915: Move the intel_dpt_offset() check into intel_plane_pin_fb() Ville Syrjala
2025-04-02  0:22 ` [PATCH 4/9] drm/i915: Use i915_vma_offset() in intel_dpt_offset() Ville Syrjala
2025-04-02  0:22 ` [PATCH 5/9] drm/i915: Remove unused dpt_total_entries() Ville Syrjala
2025-04-02  0:22 ` [PATCH 6/9] drm/i915: Don't pass crtc_state to foo_plane_ctl() & co Ville Syrjala
2025-04-02  0:22 ` Ville Syrjala [this message]
2025-04-02  0:22 ` [PATCH 8/9] drm/i915: Add tracepoint for plane faults Ville Syrjala
2025-04-02  0:22 ` [PATCH 9/9] drm/i915: Include plane ctl/surf registers in the plane update_arm() tracepoint Ville Syrjala
2025-04-02  1:42 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Precompute plane SURF address/etc Patchwork
2025-04-02  1:42 ` ✗ Fi.CI.SPARSE: " Patchwork
2025-04-02  2:02 ` ✓ i915.CI.BAT: success " Patchwork
2025-04-02  4:33 ` ✗ i915.CI.Full: failure " Patchwork
2025-04-02 15:00   ` Ville Syrjälä

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250402002209.24987-8-ville.syrjala@linux.intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox