[PATCH v2 16/17] drm/i915: Perform single buffer plane register updates from vblank worker
Ville Syrjala
ville.syrjala at linux.intel.com
Tue Sep 10 16:08:18 UTC 2019
From: Ville Syrjälä <ville.syrjala at linux.intel.com>
Like we did for pipe gamma let's move the single buffered plane
register updates to occur from a vblank work.
As the vblank works are a bit on the expensive side (with the qos and
everything) let's try to do this only when we absolutely have to. If
we know that the registers we're about to update don't have any
effect on the old frame we can just directly write the registers
while the old frame is still being scanned out.
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 8dd9cc1107f3..ea6281218a53 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 9a6e07512a40..277b08bb7738 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;
}
@@ -835,6 +849,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
@@ -982,6 +1021,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,
@@ -1012,6 +1066,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);
@@ -1039,9 +1103,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);
}
@@ -1086,6 +1147,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;
@@ -1199,6 +1287,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,
@@ -1235,6 +1337,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);
@@ -1265,8 +1375,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);
}
@@ -1488,6 +1596,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);
@@ -1511,11 +1624,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);
}
@@ -1675,10 +1783,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;
}
@@ -1731,6 +1843,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;
}
@@ -2683,6 +2798,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;
@@ -2695,6 +2811,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