All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ville Syrjala <ville.syrjala@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: [Intel-gfx] [PATCH 2/6] drm/i915: Optimize icl+ universal plane programming
Date: Fri, 21 Jan 2022 13:30:32 +0200	[thread overview]
Message-ID: <20220121113036.23240-3-ville.syrjala@linux.intel.com> (raw)
In-Reply-To: <20220121113036.23240-1-ville.syrjala@linux.intel.com>

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

On icl+ all plane registers are armed by PLANE_SURF, so we can
move almost everything over into the update_noarm() hook.

The PLANE_CTL write has to stay in the icl_update_arm() hook though
as it still exhibits the somewhat annoying self-arming behaviour
when the plane transitioning from disabled to enabled.

We could either do a full split for skl+ vs. icl+, or we could try
some other kind of split where we'd eg. keep most things in the skl+
functions and call them from the icl+ functions. I think a full split
is probably the cleaner approach since we've anyway accumulated quite
a bit of icl+ specific things, so that is what I opted to do.

Some i915_update_info stats for tgl:
before:                             after:
Updates: 5043                       Updates: 5043
       |                                   |
   1us |                               1us |
       |**                                 |***
   4us |******                         4us |********
       |**********                         |***********
  16us |***********                   16us |**********
       |****                               |*
  66us |                              66us |
       |                                   |
 262us |                             262us |
       |                                   |
   1ms |                               1ms |
       |                                   |
   4ms |                               4ms |
       |                                   |
  17ms |                              17ms |
       |                                   |
Min update: 3494ns                  Min update: 2983ns
Max update: 49491ns                 Max update: 39986ns
Average update: 18031ns             Average update: 13423ns
Overruns > 100us: 0                 Overruns > 100us: 0

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../drm/i915/display/skl_universal_plane.c    | 195 ++++++++++++++----
 1 file changed, 155 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 72dcc341bb1f..07c64ea5b362 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -619,6 +619,25 @@ skl_plane_disable_arm(struct intel_plane *plane,
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
+	skl_write_plane_wm(plane, crtc_state);
+
+	intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
+	intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), 0);
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+}
+
+static void
+icl_plane_disable_arm(struct intel_plane *plane,
+		      const struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
 	if (icl_is_hdr_plane(dev_priv, plane_id))
 		intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0);
 
@@ -1064,7 +1083,7 @@ static void icl_plane_csc_load_black(struct intel_plane *plane)
 	intel_de_write_fw(i915, PLANE_CSC_POSTOFF(pipe, plane_id, 2), 0);
 }
 
-static int skl_plane_color_plane(const struct intel_plane_state *plane_state)
+static int icl_plane_color_plane(const struct intel_plane_state *plane_state)
 {
 	/* Program the UV plane on planar master */
 	if (plane_state->planar_linked_plane && !plane_state->planar_slave)
@@ -1081,9 +1100,7 @@ skl_plane_update_noarm(struct intel_plane *plane,
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	enum plane_id plane_id = plane->id;
 	enum pipe pipe = plane->pipe;
-	int color_plane = skl_plane_color_plane(plane_state);
-	u32 stride = skl_plane_stride(plane_state, color_plane);
-	const struct drm_framebuffer *fb = plane_state->hw.fb;
+	u32 stride = skl_plane_stride(plane_state, 0);
 	int crtc_x = plane_state->uapi.dst.x1;
 	int crtc_y = plane_state->uapi.dst.y1;
 	u32 src_w = drm_rect_width(&plane_state->uapi.src) >> 16;
@@ -1098,12 +1115,109 @@ skl_plane_update_noarm(struct intel_plane *plane,
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
+	intel_de_write_fw(dev_priv, PLANE_STRIDE(pipe, plane_id),
+			  PLANE_STRIDE_(stride));
+	intel_de_write_fw(dev_priv, PLANE_POS(pipe, plane_id),
+			  PLANE_POS_Y(crtc_y) | PLANE_POS_X(crtc_x));
+	intel_de_write_fw(dev_priv, PLANE_SIZE(pipe, plane_id),
+			  PLANE_HEIGHT(src_h - 1) | PLANE_WIDTH(src_w - 1));
+
+	skl_write_plane_wm(plane, crtc_state);
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+}
+
+static void
+skl_plane_update_arm(struct intel_plane *plane,
+		     const struct intel_crtc_state *crtc_state,
+		     const struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum plane_id plane_id = plane->id;
+	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;
+	unsigned long irqflags;
+
+	plane_ctl = plane_state->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);
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
+	intel_de_write_fw(dev_priv, PLANE_KEYVAL(pipe, plane_id), skl_plane_keyval(plane_state));
+	intel_de_write_fw(dev_priv, PLANE_KEYMSK(pipe, plane_id), skl_plane_keymsk(plane_state));
+	intel_de_write_fw(dev_priv, PLANE_KEYMAX(pipe, plane_id), skl_plane_keymax(plane_state));
+
+	intel_de_write_fw(dev_priv, PLANE_OFFSET(pipe, plane_id),
+			  PLANE_OFFSET_Y(y) | PLANE_OFFSET_X(x));
+
+	intel_de_write_fw(dev_priv, PLANE_AUX_DIST(pipe, plane_id),
+			  skl_plane_aux_dist(plane_state, 0));
+
+	intel_de_write_fw(dev_priv, PLANE_AUX_OFFSET(pipe, plane_id),
+			  PLANE_OFFSET_Y(plane_state->view.color_plane[1].y) |
+			  PLANE_OFFSET_X(plane_state->view.color_plane[1].x));
+
+	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);
+
 	/*
-	 * FIXME: pxp session invalidation can hit any time even at time of commit
-	 * or after the commit, display content will be garbage.
+	 * The control register self-arms if the plane was previously
+	 * disabled. Try to make the plane enable atomic by writing
+	 * the control register just before the surface register.
 	 */
-	if (plane_state->force_black)
-		icl_plane_csc_load_black(plane);
+	intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl);
+	intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id),
+			  skl_plane_surf(plane_state, 0));
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+}
+
+static void
+icl_plane_update_noarm(struct intel_plane *plane,
+		       const struct intel_crtc_state *crtc_state,
+		       const struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
+	int color_plane = icl_plane_color_plane(plane_state);
+	u32 stride = skl_plane_stride(plane_state, color_plane);
+	const struct drm_framebuffer *fb = plane_state->hw.fb;
+	int crtc_x = plane_state->uapi.dst.x1;
+	int crtc_y = plane_state->uapi.dst.y1;
+	int x = plane_state->view.color_plane[color_plane].x;
+	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;
+	unsigned long irqflags;
+
+	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) {
+		crtc_x = 0;
+		crtc_y = 0;
+	}
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 	intel_de_write_fw(dev_priv, PLANE_STRIDE(pipe, plane_id),
 			  PLANE_STRIDE_(stride));
@@ -1112,6 +1226,13 @@ skl_plane_update_noarm(struct intel_plane *plane,
 	intel_de_write_fw(dev_priv, PLANE_SIZE(pipe, plane_id),
 			  PLANE_HEIGHT(src_h - 1) | PLANE_WIDTH(src_w - 1));
 
+	intel_de_write_fw(dev_priv, PLANE_KEYVAL(pipe, plane_id), skl_plane_keyval(plane_state));
+	intel_de_write_fw(dev_priv, PLANE_KEYMSK(pipe, plane_id), skl_plane_keymsk(plane_state));
+	intel_de_write_fw(dev_priv, PLANE_KEYMAX(pipe, plane_id), skl_plane_keymax(plane_state));
+
+	intel_de_write_fw(dev_priv, PLANE_OFFSET(pipe, plane_id),
+			  PLANE_OFFSET_Y(y) | PLANE_OFFSET_X(x));
+
 	if (intel_fb_is_rc_ccs_cc_modifier(fb->modifier)) {
 		intel_de_write_fw(dev_priv, PLANE_CC_VAL(pipe, plane_id, 0),
 				  lower_32_bits(plane_state->ccval));
@@ -1119,61 +1240,49 @@ skl_plane_update_noarm(struct intel_plane *plane,
 				  upper_32_bits(plane_state->ccval));
 	}
 
+	intel_de_write_fw(dev_priv, PLANE_AUX_DIST(pipe, plane_id),
+			  skl_plane_aux_dist(plane_state, color_plane));
+
 	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);
 
+	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);
 
 	skl_write_plane_wm(plane, crtc_state);
 
+	/*
+	 * FIXME: pxp session invalidation can hit any time even at time of commit
+	 * or after the commit, display content will be garbage.
+	 */
+	if (plane_state->force_black)
+		icl_plane_csc_load_black(plane);
+
 	intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, color_plane);
 
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void
-skl_plane_update_arm(struct intel_plane *plane,
+icl_plane_update_arm(struct intel_plane *plane,
 		     const struct intel_crtc_state *crtc_state,
 		     const struct intel_plane_state *plane_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	enum plane_id plane_id = plane->id;
 	enum pipe pipe = plane->pipe;
-	int color_plane = skl_plane_color_plane(plane_state);
-	u32 x = plane_state->view.color_plane[color_plane].x;
-	u32 y = plane_state->view.color_plane[color_plane].y;
-	u32 plane_color_ctl = 0;
-	u32 plane_ctl = plane_state->ctl;
+	int color_plane = icl_plane_color_plane(plane_state);
+	u32 plane_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);
+	plane_ctl = plane_state->ctl |
+		skl_plane_ctl_crtc(crtc_state);
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-	intel_de_write_fw(dev_priv, PLANE_KEYVAL(pipe, plane_id), skl_plane_keyval(plane_state));
-	intel_de_write_fw(dev_priv, PLANE_KEYMSK(pipe, plane_id), skl_plane_keymsk(plane_state));
-	intel_de_write_fw(dev_priv, PLANE_KEYMAX(pipe, plane_id), skl_plane_keymax(plane_state));
-
-	intel_de_write_fw(dev_priv, PLANE_OFFSET(pipe, plane_id),
-			  PLANE_OFFSET_Y(y) | PLANE_OFFSET_X(x));
-
-	intel_de_write_fw(dev_priv, PLANE_AUX_DIST(pipe, plane_id),
-			  skl_plane_aux_dist(plane_state, color_plane));
-
-	if (DISPLAY_VER(dev_priv) < 11)
-		intel_de_write_fw(dev_priv, PLANE_AUX_OFFSET(pipe, plane_id),
-				  PLANE_OFFSET_Y(plane_state->view.color_plane[1].y) |
-				  PLANE_OFFSET_X(plane_state->view.color_plane[1].x));
-
-	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
@@ -2160,9 +2269,15 @@ 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;
+	if (DISPLAY_VER(dev_priv) >= 11) {
+		plane->update_noarm = icl_plane_update_noarm;
+		plane->update_arm = icl_plane_update_arm;
+		plane->disable_arm = icl_plane_disable_arm;
+	} else {
+		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;
 	plane->check_plane = skl_plane_check;
 
-- 
2.32.0


  parent reply	other threads:[~2022-01-21 11:30 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-21 11:30 [Intel-gfx] [PATCH 0/6] drm/i915: Futher optimize plane updates Ville Syrjala
2022-01-21 11:30 ` [Intel-gfx] [PATCH 1/6] drm/i915: Clean up pre-skl primary plane registers Ville Syrjala
2022-01-21 17:23   ` Souza, Jose
2022-01-21 11:30 ` Ville Syrjala [this message]
2022-01-21 11:30 ` [Intel-gfx] [PATCH 3/6] drm/i915: Make skl+ universal plane registers unlocked Ville Syrjala
2022-01-21 11:30 ` [Intel-gfx] [PATCH 4/6] drm/i915: Make cursor " Ville Syrjala
2022-01-21 11:30 ` [Intel-gfx] [PATCH 5/6] drm/i915: Make most pre-skl primary " Ville Syrjala
2022-01-21 11:30 ` [Intel-gfx] [PATCH 6/6] drm/i915: Make pre-skl sprite " Ville Syrjala
2022-01-21 12:44 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Futher optimize plane updates Patchwork
2022-01-21 13:22 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-01-21 16:00 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2022-02-09 16:14 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Futher optimize plane updates (rev2) Patchwork

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=20220121113036.23240-3-ville.syrjala@linux.intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.