[PATCH 16/16] drm/i915: Perform single buffer plane register updates from vblank worker
Ville Syrjala
ville.syrjala at linux.intel.com
Fri Sep 6 19:02:47 UTC 2019
From: Ville Syrjälä <ville.syrjala at linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala at 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(-)
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;
--
2.21.0
More information about the Intel-gfx-trybot
mailing list