[16/16] drm/i915: Perform single buffer plane register updates from vblank worker

Submitted by Ville Syrjälä on Sept. 6, 2019, 7:02 p.m.

Details

Message ID 20190906190247.5878-16-ville.syrjala@linux.intel.com
State New
Headers show
Series "Series without cover letter" ( rev: 2 1 ) in Intel GFX - Try Bot

Not browsing as part of any series.

Commit Message

Ville Syrjälä Sept. 6, 2019, 7:02 p.m.
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../gpu/drm/i915/display/intel_atomic_plane.c |   1 +
 drivers/gpu/drm/i915/display/intel_display.c  |  63 +++++++-
 .../drm/i915/display/intel_display_types.h    |  10 ++
 drivers/gpu/drm/i915/display/intel_sprite.c   | 141 ++++++++++++++++--
 4 files changed, 202 insertions(+), 13 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index d1fcdf206da4..f79f93e597ea 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -94,6 +94,7 @@  intel_plane_duplicate_state(struct drm_plane *plane)
 
 	intel_state->vma = NULL;
 	intel_state->flags = 0;
+	intel_state->need_postvbl_update = false;
 
 	return state;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 488d026aab76..7fcb496989ff 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14051,6 +14051,53 @@  bool intel_crtc_need_lut_load(const struct intel_crtc_state *crtc_state)
 		 crtc_state->update_pipe);
 }
 
+static void intel_update_plane_postvbl(struct intel_plane *plane,
+				       const struct intel_crtc_state *new_crtc_state,
+				       const struct intel_plane_state *new_plane_state)
+{
+	if (new_plane_state->base.visible) {
+		plane->update_plane_postvbl(plane, new_crtc_state, new_plane_state);
+	} else if (new_plane_state->slave) {
+		struct intel_atomic_state *state =
+			to_intel_atomic_state(new_plane_state->base.state);
+		struct intel_plane *master =
+			new_plane_state->linked_plane;
+
+		/*
+		 * We update the slave plane from this function because
+		 * programming it from the master plane's update_plane
+		 * callback runs into issues when the Y plane is
+		 * reassigned, disabled or used by a different plane.
+		 *
+		 * The slave plane is updated with the master plane's
+		 * plane_state.
+		 */
+		new_plane_state =
+			intel_atomic_get_new_plane_state(state, master);
+
+		plane->update_plane_postvbl(plane, new_crtc_state, new_plane_state);
+	}
+}
+
+static bool intel_crtc_need_plane_postvbl_update(struct intel_atomic_state *state,
+						 const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	const struct intel_plane_state *plane_state;
+	struct intel_plane *plane;
+	int i;
+
+	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
+		if (crtc->pipe != plane->pipe ||
+		    !plane_state->need_postvbl_update ||
+		    (crtc_state->update_planes & BIT(plane->id)) == 0)
+			continue;
+		return true;
+	}
+
+	return false;
+}
+
 static void intel_crtc_do_vblank_work(struct intel_atomic_state *state,
 				      struct intel_crtc_state *new_crtc_state,
 				      u64 count)
@@ -14059,6 +14106,19 @@  static void intel_crtc_do_vblank_work(struct intel_atomic_state *state,
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_crtc_state *old_crtc_state =
 		intel_atomic_get_old_crtc_state(state, crtc);
+	const struct intel_plane_state *new_plane_state;
+	struct intel_plane *plane;
+	int i;
+
+	for_each_new_intel_plane_in_state(state, plane, new_plane_state, i) {
+		if (crtc->pipe != plane->pipe ||
+		    !new_plane_state->need_postvbl_update ||
+		    (new_crtc_state->update_planes & BIT(plane->id)) == 0)
+			continue;
+
+		intel_update_plane_postvbl(plane, new_crtc_state,
+					   new_plane_state);
+	}
 
 	if (new_crtc_state->base.active &&
 	    intel_crtc_need_lut_load(new_crtc_state))
@@ -14671,7 +14731,8 @@  static void intel_begin_crtc_commit(struct intel_atomic_state *state,
 	 * regiser update can get delayed until we're already
 	 * in the active portion of the new frame.
 	 */
-	if (intel_crtc_need_lut_load(new_crtc_state))
+	if (intel_crtc_need_lut_load(new_crtc_state) ||
+	    intel_crtc_need_plane_postvbl_update(state, new_crtc_state))
 		pm_qos_update_request(&crtc->pm_qos, 0);
 
 	/* Perform vblank evasion around commit operation */
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 9117f0a51dde..366f8052a871 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -581,6 +581,13 @@  struct intel_plane_state {
 	u32 slave;
 
 	struct drm_intel_sprite_colorkey ckey;
+
+	/*
+	 * Some plane registers are single buffered and thus
+	 * may require programming after the vblank when the
+	 * double buffered registers have latched.
+	 */
+	bool need_postvbl_update;
 };
 
 struct intel_initial_plane_config {
@@ -1071,6 +1078,9 @@  struct intel_plane {
 	void (*update_slave)(struct intel_plane *plane,
 			     const struct intel_crtc_state *crtc_state,
 			     const struct intel_plane_state *plane_state);
+	void (*update_plane_postvbl)(struct intel_plane *plane,
+				     const struct intel_crtc_state *crtc_state,
+				     const struct intel_plane_state *plane_state);
 	void (*disable_plane)(struct intel_plane *plane,
 			      const struct intel_crtc_state *crtc_state);
 	bool (*get_hw_state)(struct intel_plane *plane, enum pipe *pipe);
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index b6c301c75b9b..2c8aa6a94ebd 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -77,9 +77,23 @@  bool intel_crtc_need_lut_load(const struct intel_crtc_state *crtc_state);
 static bool intel_crtc_need_vblank_work(struct intel_atomic_state *state,
 					const struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	const struct intel_plane_state *plane_state;
+	struct intel_plane *plane;
+	int i;
+
 	if (!crtc_state->base.active)
 		return false;
 
+	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
+		if (crtc->pipe != plane->pipe ||
+		    !plane_state->need_postvbl_update ||
+		    (crtc_state->update_planes & BIT(plane->id)) == 0)
+			continue;
+
+		return true;
+	}
+
 	return intel_crtc_need_lut_load(crtc_state) ||
 		crtc_state->wm.need_postvbl_update;
 }
@@ -833,6 +847,31 @@  chv_update_csc(const struct intel_plane_state *plane_state)
 	I915_WRITE_FW(SPCSCCROCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0));
 }
 
+static bool vlv_sprite_needs_postvbl(const struct intel_crtc_state *new_crtc_state,
+				     const struct intel_plane_state *new_plane_state)
+{
+	struct intel_plane *plane = to_intel_plane(new_plane_state->base.plane);
+	struct intel_atomic_state *state =
+		to_intel_atomic_state(new_plane_state->base.state);
+	const struct intel_plane_state *old_plane_state =
+		intel_atomic_get_old_plane_state(state, plane);
+	bool old_is_yuv = old_plane_state->base.visible &&
+		old_plane_state->base.fb->format->is_yuv;
+	bool new_is_yuv = new_plane_state->base.visible &&
+		new_plane_state->base.fb->format->is_yuv;
+	bool old_limited_range = old_is_yuv &&
+		old_plane_state->base.color_range == DRM_COLOR_YCBCR_LIMITED_RANGE;
+	bool new_limited_range = new_is_yuv &&
+		new_plane_state->base.color_range == DRM_COLOR_YCBCR_LIMITED_RANGE;
+
+	if (!new_crtc_state->base.active ||
+	    drm_atomic_crtc_needs_modeset(&new_crtc_state->base))
+		return false;
+
+	return old_is_yuv && new_is_yuv &&
+		old_limited_range != new_limited_range;
+}
+
 #define SIN_0 0
 #define COS_0 1
 
@@ -980,6 +1019,21 @@  static void vlv_update_gamma(const struct intel_plane_state *plane_state)
 			      gamma[i]);
 }
 
+static void vlv_update_plane_postvbl(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);
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
+	vlv_update_clrc(plane_state);
+	vlv_update_gamma(plane_state);
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+}
+
 static void
 vlv_update_plane(struct intel_plane *plane,
 		 const struct intel_crtc_state *crtc_state,
@@ -1010,6 +1064,16 @@  vlv_update_plane(struct intel_plane *plane,
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
+	/*
+	 * If the new gamma/clrc register values don't conflict with
+	 * the old state let's let's update them before the double
+	 * buffered registers, thus guaranteeing no visual artifacts.
+	 */
+	if (!plane_state->need_postvbl_update) {
+		vlv_update_clrc(plane_state);
+		vlv_update_gamma(plane_state);
+	}
+
 	I915_WRITE_FW(SPSTRIDE(pipe, plane_id),
 		      plane_state->color_plane[0].stride);
 	I915_WRITE_FW(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x);
@@ -1037,9 +1101,6 @@  vlv_update_plane(struct intel_plane *plane,
 	I915_WRITE_FW(SPSURF(pipe, plane_id),
 		      intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
 
-	vlv_update_clrc(plane_state);
-	vlv_update_gamma(plane_state);
-
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
@@ -1084,6 +1145,33 @@  vlv_plane_get_hw_state(struct intel_plane *plane,
 	return ret;
 }
 
+static bool ivb_sprite_needs_postvbl(const struct intel_crtc_state *new_crtc_state,
+				     const struct intel_plane_state *new_plane_state)
+{
+	struct intel_plane *plane = to_intel_plane(new_plane_state->base.plane);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	struct intel_atomic_state *state =
+		to_intel_atomic_state(new_plane_state->base.state);
+	const struct intel_plane_state *old_plane_state =
+		intel_atomic_get_old_plane_state(state, plane);
+	bool old_visible = old_plane_state->base.visible;
+	bool new_visible = new_plane_state->base.visible;
+	bool old_is_fp16 = old_visible &&
+		old_plane_state->base.fb->format->cpp[0] == 8;
+	bool new_is_fp16 = new_visible &&
+		new_plane_state->base.fb->format->cpp[0] == 8;
+
+	if (IS_BROADWELL(dev_priv))
+		return false;
+
+	if (!new_crtc_state->base.active ||
+	    drm_atomic_crtc_needs_modeset(&new_crtc_state->base))
+		return false;
+
+	return old_visible && new_visible &&
+		old_is_fp16 != new_is_fp16;
+}
+
 static u32 ivb_sprite_ctl_crtc(const struct intel_crtc_state *crtc_state)
 {
 	u32 sprctl = 0;
@@ -1197,6 +1285,20 @@  static void ivb_update_gamma(const struct intel_plane_state *plane_state)
 	i++;
 }
 
+static void ivb_update_plane_postvbl(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);
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
+	ivb_update_gamma(plane_state);
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+}
+
 static void
 ivb_update_plane(struct intel_plane *plane,
 		 const struct intel_crtc_state *crtc_state,
@@ -1233,6 +1335,14 @@  ivb_update_plane(struct intel_plane *plane,
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
+	/*
+	 * If the new gamma register values don't conflict with
+	 * the old state let's let's update them before the double
+	 * buffered registers, thus guaranteeing no visual artifacts.
+	 */
+	if (!plane_state->need_postvbl_update)
+		ivb_update_gamma(plane_state);
+
 	I915_WRITE_FW(SPRSTRIDE(pipe), plane_state->color_plane[0].stride);
 	I915_WRITE_FW(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
 	I915_WRITE_FW(SPRSIZE(pipe), (crtc_h << 16) | crtc_w);
@@ -1263,8 +1373,6 @@  ivb_update_plane(struct intel_plane *plane,
 	I915_WRITE_FW(SPRSURF(pipe),
 		      intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
 
-	ivb_update_gamma(plane_state);
-
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
@@ -1486,6 +1594,11 @@  g4x_update_plane(struct intel_plane *plane,
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
+	if (IS_G4X(dev_priv))
+		g4x_update_gamma(plane_state);
+	else
+		ilk_update_gamma(plane_state);
+
 	I915_WRITE_FW(DVSSTRIDE(pipe), plane_state->color_plane[0].stride);
 	I915_WRITE_FW(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
 	I915_WRITE_FW(DVSSIZE(pipe), (crtc_h << 16) | crtc_w);
@@ -1509,11 +1622,6 @@  g4x_update_plane(struct intel_plane *plane,
 	I915_WRITE_FW(DVSSURF(pipe),
 		      intel_plane_ggtt_offset(plane_state) + dvssurf_offset);
 
-	if (IS_G4X(dev_priv))
-		g4x_update_gamma(plane_state);
-	else
-		ilk_update_gamma(plane_state);
-
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
@@ -1673,10 +1781,14 @@  g4x_sprite_check(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	if (INTEL_GEN(dev_priv) >= 7)
+	if (INTEL_GEN(dev_priv) >= 7) {
 		plane_state->ctl = ivb_sprite_ctl(crtc_state, plane_state);
-	else
+
+		plane_state->need_postvbl_update =
+			ivb_sprite_needs_postvbl(crtc_state, plane_state);
+	} else {
 		plane_state->ctl = g4x_sprite_ctl(crtc_state, plane_state);
+	}
 
 	return 0;
 }
@@ -1729,6 +1841,9 @@  vlv_sprite_check(struct intel_crtc_state *crtc_state,
 
 	plane_state->ctl = vlv_sprite_ctl(crtc_state, plane_state);
 
+	plane_state->need_postvbl_update =
+		vlv_sprite_needs_postvbl(crtc_state, plane_state);
+
 	return 0;
 }
 
@@ -2681,6 +2796,7 @@  intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		plane->max_stride = i9xx_plane_max_stride;
 		plane->update_plane = vlv_update_plane;
+		plane->update_plane_postvbl = vlv_update_plane_postvbl;
 		plane->disable_plane = vlv_disable_plane;
 		plane->get_hw_state = vlv_plane_get_hw_state;
 		plane->check_plane = vlv_sprite_check;
@@ -2693,6 +2809,7 @@  intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 	} else if (INTEL_GEN(dev_priv) >= 7) {
 		plane->max_stride = g4x_sprite_max_stride;
 		plane->update_plane = ivb_update_plane;
+		plane->update_plane_postvbl = ivb_update_plane_postvbl;
 		plane->disable_plane = ivb_disable_plane;
 		plane->get_hw_state = ivb_plane_get_hw_state;
 		plane->check_plane = g4x_sprite_check;