[Intel-gfx] [PATCH 5/9] drm/i915: Split skl+ plane update into noarm+arm pair

Ville Syrjala ville.syrjala at linux.intel.com
Mon Oct 18 11:50:26 UTC 2021


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

Chop skl_program_plane() into two halves. Fist half becomes
the _noarm() variant, second part the _arm() variant.

Fortunately I have already previously grouped the register
writes into roughtly the correct order, so the split looks
surprisingly clean.

A few notable oddities I did not realize were self arming
are AUX_DIST and COLOR_CTL.

i915_update_info doesn't look too terrible on my cfl running
kms_atomic_transition --r plane-all-transition --extended:
w/o patch                           w/ patch
Updates: 2178                       Updates: 2018
       |                                   |
   1us |                               1us |
       |                                   |
   4us |                               4us |*****
       |*********                          |**********
  16us |**********                    16us |*******
       |***                                |
  66us |                              66us |
       |                                   |
 262us |                             262us |
       |                                   |
   1ms |                               1ms |
       |                                   |
   4ms |                               4ms |
       |                                   |
  17ms |                              17ms |
       |                                   |
Min update: 8332ns                  Min update: 6164ns
Max update: 48758ns                 Max update: 31808ns
Average update: 19959ns             Average update: 13159ns
Overruns > 100us: 0                 Overruns > 100us: 0

And with lockdep enabled:
w/o patch                           w/ patch
Updates: 2177			    Updates: 2172
       |			    	   |
   1us |			       1us |
       |			    	   |
   4us |			       4us |
       |*******			    	   |*********
  16us |**********		      16us |**********
       |*******			    	   |*
  66us |			      66us |
       |			    	   |
 262us |			     262us |
       |			    	   |
   1ms |			       1ms |
       |			    	   |
   4ms |			       4ms |
       |			    	   |
  17ms |			      17ms |
       |			    	   |
Min update: 12645ns		    Min update: 9980ns
Max update: 50153ns		    Max update: 33533ns
Average update: 25337ns		    Average update: 18245ns
Overruns > 250us: 0		    Overruns > 250us: 0

TODO: On icl+ everything seems to be armed by PLANE_SURF, so we
      can optimize this even further on modern platforms. But I
      think there's a bit of refactoring to be done first to
      figure out the best way to go about it (eg. just reusing
      the current skl+ functions, or doing a lower level split).

TODO: Split scaler programming as well, but IIRC the scaler
      has some oddball double buffering behaviour on some
      platforms, so needs proper reverse engineering

Cc: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 .../drm/i915/display/skl_universal_plane.c    | 113 +++++++++++-------
 1 file changed, 72 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 74f3870d39b1..2a822e1e465e 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1050,60 +1050,32 @@ static void icl_plane_csc_load_black(struct intel_plane *plane)
 }
 
 static void
-skl_program_plane(struct intel_plane *plane,
-		  const struct intel_crtc_state *crtc_state,
-		  const struct intel_plane_state *plane_state,
-		  int color_plane)
+skl_program_plane_noarm(struct intel_plane *plane,
+			const struct intel_crtc_state *crtc_state,
+			const struct intel_plane_state *plane_state,
+			int color_plane)
 {
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	enum plane_id plane_id = plane->id;
 	enum pipe pipe = plane->pipe;
-	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	u32 stride = skl_plane_stride(plane_state, color_plane);
 	const struct drm_framebuffer *fb = plane_state->hw.fb;
-	int aux_plane = skl_main_to_aux_plane(fb, color_plane);
 	int crtc_x = plane_state->uapi.dst.x1;
 	int crtc_y = plane_state->uapi.dst.y1;
-	u32 x = plane_state->view.color_plane[color_plane].x;
-	u32 y = plane_state->view.color_plane[color_plane].y;
 	u32 src_w = drm_rect_width(&plane_state->uapi.src) >> 16;
 	u32 src_h = drm_rect_height(&plane_state->uapi.src) >> 16;
-	u8 alpha = plane_state->hw.alpha >> 8;
-	u32 plane_color_ctl = 0, aux_dist = 0;
 	unsigned long irqflags;
-	u32 keymsk, keymax;
-	u32 plane_ctl = plane_state->ctl;
-
-	plane_ctl |= skl_plane_ctl_crtc(crtc_state);
-
-	if (DISPLAY_VER(dev_priv) >= 10)
-		plane_color_ctl = plane_state->color_ctl |
-			glk_plane_color_ctl_crtc(crtc_state);
 
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
 
-	keymax = (key->max_value & 0xffffff) | PLANE_KEYMAX_ALPHA(alpha);
-
-	keymsk = key->channel_mask & 0x7ffffff;
-	if (alpha < 0xff)
-		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
-
 	/* The scaler will handle the output position */
 	if (plane_state->scaler_id >= 0) {
 		crtc_x = 0;
 		crtc_y = 0;
 	}
 
-	if (aux_plane) {
-		aux_dist = skl_surf_address(plane_state, aux_plane) -
-			skl_surf_address(plane_state, color_plane);
-
-		if (DISPLAY_VER(dev_priv) < 12)
-			aux_dist |= skl_plane_stride(plane_state, aux_plane);
-	}
-
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 	/*
@@ -1119,16 +1091,10 @@ skl_program_plane(struct intel_plane *plane,
 	intel_de_write_fw(dev_priv, PLANE_SIZE(pipe, plane_id),
 			  (src_h << 16) | src_w);
 
-	intel_de_write_fw(dev_priv, PLANE_AUX_DIST(pipe, plane_id), aux_dist);
-
 	if (icl_is_hdr_plane(dev_priv, plane_id))
 		intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id),
 				  plane_state->cus_ctl);
 
-	if (DISPLAY_VER(dev_priv) >= 10)
-		intel_de_write_fw(dev_priv, PLANE_COLOR_CTL(pipe, plane_id),
-				  plane_color_ctl);
-
 	if (fb->format->is_yuv && icl_is_hdr_plane(dev_priv, plane_id))
 		icl_program_input_csc(plane, crtc_state, plane_state);
 
@@ -1138,6 +1104,52 @@ skl_program_plane(struct intel_plane *plane,
 
 	skl_write_plane_wm(plane, crtc_state);
 
+	intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, color_plane);
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+}
+
+static void
+skl_program_plane_arm(struct intel_plane *plane,
+		      const struct intel_crtc_state *crtc_state,
+		      const struct intel_plane_state *plane_state,
+		      int color_plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
+	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
+	const struct drm_framebuffer *fb = plane_state->hw.fb;
+	int aux_plane = skl_main_to_aux_plane(fb, color_plane);
+	u32 x = plane_state->view.color_plane[color_plane].x;
+	u32 y = plane_state->view.color_plane[color_plane].y;
+	u32 keymsk, keymax, aux_dist = 0, plane_color_ctl = 0;
+	u8 alpha = plane_state->hw.alpha >> 8;
+	u32 plane_ctl = plane_state->ctl;
+	unsigned long irqflags;
+
+	plane_ctl |= skl_plane_ctl_crtc(crtc_state);
+
+	if (DISPLAY_VER(dev_priv) >= 10)
+		plane_color_ctl = plane_state->color_ctl |
+			glk_plane_color_ctl_crtc(crtc_state);
+
+	keymax = (key->max_value & 0xffffff) | PLANE_KEYMAX_ALPHA(alpha);
+
+	keymsk = key->channel_mask & 0x7ffffff;
+	if (alpha < 0xff)
+		keymsk |= PLANE_KEYMSK_ALPHA_ENABLE;
+
+	if (aux_plane) {
+		aux_dist = skl_surf_address(plane_state, aux_plane) -
+			skl_surf_address(plane_state, color_plane);
+
+		if (DISPLAY_VER(dev_priv) < 12)
+			aux_dist |= skl_plane_stride(plane_state, aux_plane);
+	}
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
 	intel_de_write_fw(dev_priv, PLANE_KEYVAL(pipe, plane_id),
 			  key->min_value);
 	intel_de_write_fw(dev_priv, PLANE_KEYMSK(pipe, plane_id), keymsk);
@@ -1146,17 +1158,22 @@ skl_program_plane(struct intel_plane *plane,
 	intel_de_write_fw(dev_priv, PLANE_OFFSET(pipe, plane_id),
 			  (y << 16) | x);
 
+	intel_de_write_fw(dev_priv, PLANE_AUX_DIST(pipe, plane_id), aux_dist);
+
 	if (DISPLAY_VER(dev_priv) < 11)
 		intel_de_write_fw(dev_priv, PLANE_AUX_OFFSET(pipe, plane_id),
 				  (plane_state->view.color_plane[1].y << 16) |
 				   plane_state->view.color_plane[1].x);
 
-	intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, color_plane);
+	if (DISPLAY_VER(dev_priv) >= 10)
+		intel_de_write_fw(dev_priv, PLANE_COLOR_CTL(pipe, plane_id), plane_color_ctl);
 
 	/*
 	 * Enable the scaler before the plane so that we don't
 	 * get a catastrophic underrun even if the two operations
 	 * end up happening in two different frames.
+	 *
+	 * TODO: split into noarm+arm pair
 	 */
 	if (plane_state->scaler_id >= 0)
 		skl_program_plane_scaler(plane, crtc_state, plane_state);
@@ -1199,7 +1216,20 @@ skl_plane_async_flip(struct intel_plane *plane,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-/* TODO: split into noarm+arm pair */
+static void
+skl_plane_update_noarm(struct intel_plane *plane,
+		       const struct intel_crtc_state *crtc_state,
+		       const struct intel_plane_state *plane_state)
+{
+	int color_plane = 0;
+
+	if (plane_state->planar_linked_plane && !plane_state->planar_slave)
+		/* Program the UV plane on planar master */
+		color_plane = 1;
+
+	skl_program_plane_noarm(plane, crtc_state, plane_state, color_plane);
+}
+
 static void
 skl_plane_update_arm(struct intel_plane *plane,
 		     const struct intel_crtc_state *crtc_state,
@@ -1211,7 +1241,7 @@ skl_plane_update_arm(struct intel_plane *plane,
 		/* Program the UV plane on planar master */
 		color_plane = 1;
 
-	skl_program_plane(plane, crtc_state, plane_state, color_plane);
+	skl_program_plane_arm(plane, crtc_state, plane_state, color_plane);
 }
 
 static bool intel_format_is_p01x(u32 format)
@@ -2159,6 +2189,7 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
 	}
 
 	plane->max_stride = skl_plane_max_stride;
+	plane->update_noarm = skl_plane_update_noarm;
 	plane->update_arm = skl_plane_update_arm;
 	plane->disable_arm = skl_plane_disable_arm;
 	plane->get_hw_state = skl_plane_get_hw_state;
-- 
2.32.0



More information about the Intel-gfx mailing list