[PATCH 19/24] drm/i915: Link planes in a bigjoiner configuration, v2.
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Tue Oct 8 09:28:41 UTC 2019
Make sure that when a plane is set in a bigjoiner mode, we will add
their counterpart to the atomic state as well. This will allow us to
make sure all state is available when planes are checked.
Because of the funny interactions with bigjoiner and planar YUV
formats, we may end up adding a lot of planes, so we have to keep
iterating until we no longer add any planes.
Also fix the atomic intel plane iterator, so things watermarks start
working automagically.
Changes since v1:
- Rebase on top of plane_state split, cleaning up the code a lot.
- Make intel_atomic_crtc_state_for_each_plane_state() bigjoiner capable.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
---
.../gpu/drm/i915/display/intel_atomic_plane.c | 47 +++++-
.../gpu/drm/i915/display/intel_atomic_plane.h | 3 +-
drivers/gpu/drm/i915/display/intel_display.c | 158 ++++++++++++++++--
drivers/gpu/drm/i915/display/intel_display.h | 16 +-
.../drm/i915/display/intel_display_types.h | 11 ++
5 files changed, 211 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 9947c08afb77..08860be6d9ac 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -143,10 +143,15 @@ unsigned int intel_plane_data_rate(const struct intel_crtc_state *crtc_state,
return cpp * crtc_state->pixel_rate;
}
-void intel_plane_copy_uapi_to_hw_state(struct intel_plane_state *plane_state,
+void intel_plane_copy_uapi_to_hw_state(const struct intel_crtc_state *crtc_state,
+ struct intel_plane_state *plane_state,
const struct intel_plane_state *from_plane_state)
{
- plane_state->hw.crtc = from_plane_state->uapi.crtc;
+ if (from_plane_state->uapi.crtc)
+ plane_state->hw.crtc = crtc_state->uapi.crtc;
+ else
+ plane_state->hw.crtc = NULL;
+
plane_state->hw.fb = from_plane_state->uapi.fb;
if (plane_state->hw.fb)
drm_framebuffer_get(plane_state->hw.fb);
@@ -165,10 +170,20 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
struct intel_plane_state *new_plane_state)
{
struct intel_plane *plane = to_intel_plane(new_plane_state->uapi.plane);
+ struct intel_plane_state *new_master_plane_state = new_plane_state;
+ struct intel_atomic_state *state =
+ to_intel_atomic_state(new_plane_state->uapi.state);
const struct drm_framebuffer *fb;
int ret;
- intel_plane_copy_uapi_to_hw_state(new_plane_state, new_plane_state);
+ if (new_plane_state->bigjoiner_slave)
+ new_master_plane_state =
+ intel_atomic_get_new_plane_state(state,
+ new_plane_state->bigjoiner_plane);
+
+ intel_plane_copy_uapi_to_hw_state(new_crtc_state,
+ new_plane_state,
+ new_master_plane_state);
fb = new_plane_state->hw.fb;
new_crtc_state->active_planes &= ~BIT(plane->id);
@@ -207,15 +222,36 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
}
static struct intel_crtc *
-get_crtc_from_states(const struct intel_plane_state *old_plane_state,
+get_crtc_from_states(struct intel_atomic_state *state,
+ const struct intel_plane_state *old_plane_state,
const struct intel_plane_state *new_plane_state)
{
+ struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+ struct intel_plane *plane = to_intel_plane(new_plane_state->uapi.plane);
+
if (new_plane_state->uapi.crtc)
return to_intel_crtc(new_plane_state->uapi.crtc);
if (old_plane_state->uapi.crtc)
return to_intel_crtc(old_plane_state->uapi.crtc);
+ if (new_plane_state->bigjoiner_slave) {
+ const struct intel_plane_state *new_master_plane_state =
+ intel_atomic_get_new_plane_state(state, new_plane_state->bigjoiner_plane);
+
+ /* need to use uapi here, new_master_plane_state might not be copied to hw yet */
+ if (new_master_plane_state->uapi.crtc)
+ return intel_get_crtc_for_pipe(dev_priv, plane->pipe);
+ }
+
+ if (old_plane_state->bigjoiner_slave) {
+ const struct intel_plane_state *old_master_plane_state =
+ intel_atomic_get_old_plane_state(state, old_plane_state->bigjoiner_plane);
+
+ if (old_master_plane_state->uapi.crtc)
+ return intel_get_crtc_for_pipe(dev_priv, plane->pipe);
+ }
+
return NULL;
}
@@ -227,7 +263,8 @@ int intel_plane_atomic_check(struct intel_atomic_state *state,
const struct intel_plane_state *old_plane_state =
intel_atomic_get_old_plane_state(state, plane);
struct intel_crtc *crtc =
- get_crtc_from_states(old_plane_state, new_plane_state);
+ get_crtc_from_states(state, old_plane_state,
+ new_plane_state);
const struct intel_crtc_state *old_crtc_state;
struct intel_crtc_state *new_crtc_state;
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
index 726ececd6abd..e40d6e4e913b 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
@@ -20,7 +20,8 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
unsigned int intel_plane_data_rate(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state);
-void intel_plane_copy_uapi_to_hw_state(struct intel_plane_state *plane_state,
+void intel_plane_copy_uapi_to_hw_state(const struct intel_crtc_state *crtc_state,
+ struct intel_plane_state *plane_state,
const struct intel_plane_state *from_plane_state);
void intel_update_plane(struct intel_plane *plane,
const struct intel_crtc_state *crtc_state,
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 667005b2583c..d7df3712d0c3 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3268,7 +3268,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
plane_state->fb = fb;
plane_state->crtc = &intel_crtc->base;
- intel_plane_copy_uapi_to_hw_state(intel_state, intel_state);
+ intel_plane_copy_uapi_to_hw_state(crtc_state, intel_state, intel_state);
atomic_or(to_intel_plane(primary)->frontbuffer_bit,
&to_intel_frontbuffer(fb)->bits);
@@ -11832,24 +11832,101 @@ static bool check_single_encoder_cloning(struct drm_atomic_state *state,
return true;
}
+static int icl_add_dependent_planes(struct intel_atomic_state *state,
+ struct intel_plane_state *plane_state)
+{
+ struct intel_plane_state *new_plane_state;
+ struct intel_plane *plane;
+ int ret = 0;
+
+ plane = plane_state->bigjoiner_plane;
+ if (plane && !intel_atomic_get_new_plane_state(state, plane)) {
+ new_plane_state = intel_atomic_get_plane_state(state, plane);
+ if (IS_ERR(new_plane_state))
+ return PTR_ERR(new_plane_state);
+
+ ret = 1;
+ }
+
+ plane = plane_state->planar_linked_plane;
+ if (plane && !intel_atomic_get_new_plane_state(state, plane)) {
+ new_plane_state = intel_atomic_get_plane_state(state, plane);
+ if (IS_ERR(new_plane_state))
+ return PTR_ERR(new_plane_state);
+
+ ret = 1;
+ }
+
+ return ret;
+}
+
static int icl_add_linked_planes(struct intel_atomic_state *state)
{
- struct intel_plane *plane, *linked;
- struct intel_plane_state *plane_state, *linked_plane_state;
+ struct intel_plane *plane;
+ struct intel_plane_state *old_plane_state, *new_plane_state;
+ struct intel_crtc *crtc, *linked_crtc;
+ struct intel_crtc_state *old_crtc_state, *new_crtc_state, *linked_crtc_state;
+ bool added;
int i;
- for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
- linked = plane_state->planar_linked_plane;
+ /*
+ * Iteratively add plane_state->linked_plane and plane_state->bigjoiner_plane
+ *
+ * This needs to be done repeatedly, because of is a funny interaction;
+ * the Y-plane may be assigned differently on the other bigjoiner crtc,
+ * and we could end up with the following evil recursion, when only adding a
+ * single plane to state:
+ *
+ * XRGB8888 master plane 6 adds NV12 slave Y-plane 6, which adds slave UV plane 0,
+ * which adds master UV plane 0, which adds master Y-plane 7, which adds XRGB8888
+ * slave plane 7.
+ *
+ * We could pull in even more because of old_plane_state vs new_plane_state.
+ *
+ * Max depth = 5 (or 7 for evil case) in this case.
+ * Number of passes will be less, because newly added planes show up in the
+ * same iteration round when added_plane->index > plane->index.
+ */
+ do {
+ added = false;
- if (!linked)
- continue;
+ for_each_oldnew_intel_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
+ int ret, ret2;
- linked_plane_state = intel_atomic_get_plane_state(state, linked);
- if (IS_ERR(linked_plane_state))
- return PTR_ERR(linked_plane_state);
+ ret = icl_add_dependent_planes(state, old_plane_state);
+ if (ret < 0)
+ return ret;
+
+ ret2 = icl_add_dependent_planes(state, new_plane_state);
+ if (ret2 < 0)
+ return ret2;
+
+ added |= ret || ret2;
+ }
+ } while (added);
- WARN_ON(linked_plane_state->planar_linked_plane != plane);
- WARN_ON(linked_plane_state->planar_slave == plane_state->planar_slave);
+ /*
+ * Make sure bigjoiner slave crtc's are also pulled in. This is not done automatically
+ * when adding slave planes, because plane_state->crtc is null.
+ */
+ for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+ linked_crtc = old_crtc_state->bigjoiner_linked_crtc;
+ if (linked_crtc) {
+ linked_crtc_state =
+ intel_atomic_get_crtc_state(&state->base, linked_crtc);
+
+ if (IS_ERR(linked_crtc_state))
+ return PTR_ERR(linked_crtc_state);
+ }
+
+ linked_crtc = new_crtc_state->bigjoiner_linked_crtc;
+ if (linked_crtc && linked_crtc != old_crtc_state->bigjoiner_linked_crtc) {
+ linked_crtc_state =
+ intel_atomic_get_crtc_state(&state->base, linked_crtc);
+
+ if (IS_ERR(linked_crtc_state))
+ return PTR_ERR(linked_crtc_state);
+ }
}
return 0;
@@ -11889,6 +11966,7 @@ static int icl_check_nv12_planes(struct intel_crtc_state *crtc_state)
for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
struct intel_plane_state *linked_state = NULL;
+ struct intel_plane_state *master_plane_state;
if (plane->pipe != crtc->pipe ||
!(crtc_state->nv12_planes & BIT(plane->id)))
@@ -11928,9 +12006,16 @@ static int icl_check_nv12_planes(struct intel_crtc_state *crtc_state)
linked_state->color_ctl = plane_state->color_ctl;
linked_state->color_plane[0] = plane_state->color_plane[0];
- linked_state->uapi.src = plane_state->uapi.src;
- linked_state->uapi.dst = plane_state->uapi.dst;
- intel_plane_copy_uapi_to_hw_state(linked_state, plane_state);
+ master_plane_state = plane_state;
+ if (plane_state->bigjoiner_slave)
+ master_plane_state =
+ intel_atomic_get_new_plane_state(state,
+ plane_state->bigjoiner_plane);
+
+ linked_state->uapi.src = master_plane_state->uapi.src;
+ linked_state->uapi.dst = master_plane_state->uapi.dst;
+ intel_plane_copy_uapi_to_hw_state(crtc_state, linked_state,
+ master_plane_state);
if (icl_is_hdr_plane(dev_priv, plane->id)) {
if (linked->id == PLANE_SPRITE5)
@@ -13842,6 +13927,7 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state)
struct drm_i915_private *dev_priv = to_i915(state->base.dev);
struct intel_crtc_state *old_crtc_state, *new_crtc_state, *slave_crtc_state, *master_crtc_state;
struct intel_crtc *crtc, *slave, *master;
+ struct intel_plane *plane;
int i, ret = 0;
if (INTEL_GEN(dev_priv) < 11)
@@ -13937,6 +14023,48 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state)
return ret;
}
+ /*
+ * Setup and teardown the new bigjoiner plane mappings.
+ */
+ for_each_intel_plane(&dev_priv->drm, plane) {
+ struct intel_plane_state *plane_state;
+ struct intel_plane *other_plane = NULL;
+
+ crtc = intel_get_crtc_for_pipe(dev_priv, plane->pipe);
+ old_crtc_state = intel_atomic_get_old_crtc_state(state, crtc);
+ new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
+
+ if (!new_crtc_state || !needs_modeset(new_crtc_state))
+ continue;
+
+ if (new_crtc_state->bigjoiner) {
+ struct intel_crtc *other_crtc =
+ new_crtc_state->bigjoiner_linked_crtc;
+ bool found = false;
+
+ for_each_intel_plane_on_crtc(&dev_priv->drm, other_crtc, other_plane) {
+ if (other_plane->id != plane->id)
+ continue;
+
+ found = true;
+ break;
+ }
+
+ /* All pipes should have identical planes. */
+ if (WARN_ON(!found))
+ return -EINVAL;
+ } else if (!old_crtc_state->bigjoiner) {
+ continue;
+ }
+
+ plane_state = intel_atomic_get_plane_state(state, plane);
+ if (IS_ERR(plane_state))
+ return PTR_ERR(plane_state);
+
+ plane_state->bigjoiner_plane = other_plane;
+ plane_state->bigjoiner_slave = new_crtc_state->bigjoiner_slave;
+ }
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 44a1c331d6b9..505d6fd1377e 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -444,9 +444,19 @@ enum phy_fia {
plane, plane_state, \
crtc_state) \
for_each_intel_plane_mask(((crtc_state)->uapi.state->dev), (plane), \
- ((crtc_state)->uapi.plane_mask)) \
- for_each_if ((plane_state = \
- to_intel_plane_state(__drm_atomic_get_current_plane_state((crtc_state)->uapi.state, &plane->base))))
+ (((crtc_state)->bigjoiner_slave ? \
+ intel_atomic_get_new_crtc_state( \
+ to_intel_atomic_state((crtc_state)->uapi.state), \
+ (crtc_state)->bigjoiner_linked_crtc) : \
+ (crtc_state))->uapi.plane_mask)) \
+ for_each_if ((((plane_state) = \
+ to_intel_plane_state(__drm_atomic_get_current_plane_state((crtc_state)->uapi.state, &plane->base))), \
+ ((plane) = (plane_state)->bigjoiner_slave ? \
+ (plane_state)->bigjoiner_plane : \
+ (plane)), \
+ ((plane_state) = (plane_state)->bigjoiner_slave ? \
+ to_intel_plane_state(__drm_atomic_get_current_plane_state((crtc_state)->uapi.state, &plane->base)) : \
+ (plane_state))))
void intel_link_compute_m_n(u16 bpp, int nlanes,
int pixel_clock, int link_clock,
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 5bbc708437aa..81699d0508c0 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -588,6 +588,17 @@ struct intel_plane_state {
*/
struct intel_plane *planar_linked_plane;
+ /*
+ * bigjoiner_plane:
+ *
+ * When 2 pipes are joined in a bigjoiner configuration,
+ * points to the same plane on the other pipe.
+ *
+ * bigjoiner_slave is set on the slave pipe.
+ */
+ struct intel_plane *bigjoiner_plane;
+ u32 bigjoiner_slave;
+
/*
* planar_slave:
* If set don't update use the linked plane's state for updating
--
2.23.0
More information about the Intel-gfx-trybot
mailing list