[Intel-gfx] [PATCH 04/10] drm/i915: Clean up the bigjoiner state copy logic
Ville Syrjala
ville.syrjala at linux.intel.com
Thu Feb 3 18:38:17 UTC 2022
From: Ville Syrjälä <ville.syrjala at linux.intel.com>
Currently the bigjoiner state copy logic is kind of
a byzantine mess.
Clean it up to operate in the following manner during a full
modeset:
1) master uapi -> hw state copy
2) master hw -> slave hw state copy
And during a non-modeset update we do:
1) master uapi -> hw state light copy
2) master hw -> slave hw state light copy
I think that is now easier to reason about since we never do
any kind of master uapi -> slave hw state copy short circuit
that could happen previously.
Obviously this does now depend on the master uapi->hw copy
always happening before the master hw -> slave hw copy, but
that is guaranteed by the fact that we always add both crtcs
to the state early, the crtcs are registered in pipe
order (so the compute_config loop happens in pipe order),
and the hardware requires the master pipe has to be lower
than the slave pipe as well. And for good measure we shall
add a check+WARN for this before doing the bigjoiner crtc
assignment.
Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
drivers/gpu/drm/i915/display/intel_atomic.c | 11 --
drivers/gpu/drm/i915/display/intel_atomic.h | 2 -
drivers/gpu/drm/i915/display/intel_display.c | 170 ++++++++++++-------
3 files changed, 108 insertions(+), 75 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
index 093904065112..e0667d163266 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -281,17 +281,6 @@ void intel_crtc_free_hw_state(struct intel_crtc_state *crtc_state)
intel_crtc_put_color_blobs(crtc_state);
}
-void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state,
- const struct intel_crtc_state *from_crtc_state)
-{
- drm_property_replace_blob(&crtc_state->hw.degamma_lut,
- from_crtc_state->uapi.degamma_lut);
- drm_property_replace_blob(&crtc_state->hw.gamma_lut,
- from_crtc_state->uapi.gamma_lut);
- drm_property_replace_blob(&crtc_state->hw.ctm,
- from_crtc_state->uapi.ctm);
-}
-
/**
* intel_crtc_destroy_state - destroy crtc state
* @crtc: drm crtc
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h b/drivers/gpu/drm/i915/display/intel_atomic.h
index d2700c74c9da..1dc439983dd9 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic.h
@@ -44,8 +44,6 @@ struct drm_crtc_state *intel_crtc_duplicate_state(struct drm_crtc *crtc);
void intel_crtc_destroy_state(struct drm_crtc *crtc,
struct drm_crtc_state *state);
void intel_crtc_free_hw_state(struct intel_crtc_state *crtc_state);
-void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state,
- const struct intel_crtc_state *from_crtc_state);
struct drm_atomic_state *intel_atomic_state_alloc(struct drm_device *dev);
void intel_atomic_state_free(struct drm_atomic_state *state);
void intel_atomic_state_clear(struct drm_atomic_state *state);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b5701ca57889..48869478efc2 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5818,32 +5818,37 @@ static bool check_digital_port_conflicts(struct intel_atomic_state *state)
static void
intel_crtc_copy_uapi_to_hw_state_nomodeset(struct intel_atomic_state *state,
- struct intel_crtc_state *crtc_state)
+ struct intel_crtc *crtc)
{
- const struct intel_crtc_state *master_crtc_state;
- struct intel_crtc *master_crtc;
+ struct intel_crtc_state *crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
- master_crtc = intel_master_crtc(crtc_state);
- master_crtc_state = intel_atomic_get_new_crtc_state(state, master_crtc);
+ WARN_ON(crtc_state->bigjoiner_slave);
- /* No need to copy state if the master state is unchanged */
- if (master_crtc_state) {
- crtc_state->uapi.color_mgmt_changed = master_crtc_state->uapi.color_mgmt_changed;
- intel_crtc_copy_color_blobs(crtc_state, master_crtc_state);
- }
+ drm_property_replace_blob(&crtc_state->hw.degamma_lut,
+ crtc_state->uapi.degamma_lut);
+ drm_property_replace_blob(&crtc_state->hw.gamma_lut,
+ crtc_state->uapi.gamma_lut);
+ drm_property_replace_blob(&crtc_state->hw.ctm,
+ crtc_state->uapi.ctm);
}
static void
-intel_crtc_copy_uapi_to_hw_state(struct intel_atomic_state *state,
- struct intel_crtc_state *crtc_state)
+intel_crtc_copy_uapi_to_hw_state_modeset(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
+ struct intel_crtc_state *crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
+
+ WARN_ON(crtc_state->bigjoiner_slave);
+
crtc_state->hw.enable = crtc_state->uapi.enable;
crtc_state->hw.active = crtc_state->uapi.active;
crtc_state->hw.mode = crtc_state->uapi.mode;
crtc_state->hw.adjusted_mode = crtc_state->uapi.adjusted_mode;
crtc_state->hw.scaling_filter = crtc_state->uapi.scaling_filter;
- intel_crtc_copy_uapi_to_hw_state_nomodeset(state, crtc_state);
+ intel_crtc_copy_uapi_to_hw_state_nomodeset(state, crtc);
}
static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state)
@@ -5859,7 +5864,6 @@ static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state
crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
crtc_state->uapi.scaling_filter = crtc_state->hw.scaling_filter;
- /* copy color blobs to uapi */
drm_property_replace_blob(&crtc_state->uapi.degamma_lut,
crtc_state->hw.degamma_lut);
drm_property_replace_blob(&crtc_state->uapi.gamma_lut,
@@ -5868,61 +5872,79 @@ static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state
crtc_state->hw.ctm);
}
+static void
+copy_bigjoiner_crtc_state_nomodeset(struct intel_atomic_state *state,
+ struct intel_crtc *slave_crtc)
+{
+ struct intel_crtc_state *slave_crtc_state =
+ intel_atomic_get_new_crtc_state(state, slave_crtc);
+ struct intel_crtc *master_crtc = intel_master_crtc(slave_crtc_state);
+ const struct intel_crtc_state *master_crtc_state =
+ intel_atomic_get_new_crtc_state(state, master_crtc);
+
+ drm_property_replace_blob(&slave_crtc_state->hw.degamma_lut,
+ master_crtc_state->hw.degamma_lut);
+ drm_property_replace_blob(&slave_crtc_state->hw.gamma_lut,
+ master_crtc_state->hw.gamma_lut);
+ drm_property_replace_blob(&slave_crtc_state->uapi.ctm,
+ master_crtc_state->hw.ctm);
+
+ slave_crtc_state->uapi.color_mgmt_changed = master_crtc_state->uapi.color_mgmt_changed;
+}
+
static int
-copy_bigjoiner_crtc_state(struct intel_crtc_state *crtc_state,
- const struct intel_crtc_state *from_crtc_state)
+copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state,
+ struct intel_crtc *slave_crtc)
{
+ struct intel_crtc_state *slave_crtc_state =
+ intel_atomic_get_new_crtc_state(state, slave_crtc);
+ struct intel_crtc *master_crtc = intel_master_crtc(slave_crtc_state);
+ const struct intel_crtc_state *master_crtc_state =
+ intel_atomic_get_new_crtc_state(state, master_crtc);
struct intel_crtc_state *saved_state;
- saved_state = kmemdup(from_crtc_state, sizeof(*saved_state), GFP_KERNEL);
+ saved_state = kmemdup(master_crtc_state, sizeof(*saved_state), GFP_KERNEL);
if (!saved_state)
return -ENOMEM;
- saved_state->uapi = crtc_state->uapi;
- saved_state->scaler_state = crtc_state->scaler_state;
- saved_state->shared_dpll = crtc_state->shared_dpll;
- saved_state->dpll_hw_state = crtc_state->dpll_hw_state;
- saved_state->crc_enabled = crtc_state->crc_enabled;
+ /* preserve some things from the slave's original crtc state */
+ saved_state->uapi = slave_crtc_state->uapi;
+ saved_state->scaler_state = slave_crtc_state->scaler_state;
+ saved_state->shared_dpll = slave_crtc_state->shared_dpll;
+ saved_state->dpll_hw_state = slave_crtc_state->dpll_hw_state;
+ saved_state->crc_enabled = slave_crtc_state->crc_enabled;
- intel_crtc_free_hw_state(crtc_state);
- memcpy(crtc_state, saved_state, sizeof(*crtc_state));
+ intel_crtc_free_hw_state(slave_crtc_state);
+ memcpy(slave_crtc_state, saved_state, sizeof(*slave_crtc_state));
kfree(saved_state);
/* Re-init hw state */
- memset(&crtc_state->hw, 0, sizeof(saved_state->hw));
- crtc_state->hw.enable = from_crtc_state->hw.enable;
- crtc_state->hw.active = from_crtc_state->hw.active;
- crtc_state->hw.mode = from_crtc_state->hw.mode;
- crtc_state->hw.pipe_mode = from_crtc_state->hw.pipe_mode;
- crtc_state->hw.adjusted_mode = from_crtc_state->hw.adjusted_mode;
- crtc_state->hw.scaling_filter = from_crtc_state->hw.scaling_filter;
+ memset(&slave_crtc_state->hw, 0, sizeof(slave_crtc_state->hw));
+ slave_crtc_state->hw.enable = master_crtc_state->hw.enable;
+ slave_crtc_state->hw.active = master_crtc_state->hw.active;
+ slave_crtc_state->hw.mode = master_crtc_state->hw.mode;
+ slave_crtc_state->hw.pipe_mode = master_crtc_state->hw.pipe_mode;
+ slave_crtc_state->hw.adjusted_mode = master_crtc_state->hw.adjusted_mode;
+ slave_crtc_state->hw.scaling_filter = master_crtc_state->hw.scaling_filter;
- drm_property_replace_blob(&crtc_state->hw.degamma_lut,
- from_crtc_state->hw.degamma_lut);
- drm_property_replace_blob(&crtc_state->hw.gamma_lut,
- from_crtc_state->hw.gamma_lut);
- drm_property_replace_blob(&crtc_state->uapi.ctm,
- from_crtc_state->hw.ctm);
+ copy_bigjoiner_crtc_state_nomodeset(state, slave_crtc);
/* Some fixups */
- crtc_state->uapi.mode_changed = from_crtc_state->uapi.mode_changed;
- crtc_state->uapi.connectors_changed = from_crtc_state->uapi.connectors_changed;
- crtc_state->uapi.active_changed = from_crtc_state->uapi.active_changed;
- crtc_state->uapi.color_mgmt_changed = from_crtc_state->uapi.color_mgmt_changed;
- crtc_state->nv12_planes = crtc_state->c8_planes = crtc_state->update_planes = 0;
- crtc_state->bigjoiner_linked_crtc = to_intel_crtc(from_crtc_state->uapi.crtc);
- crtc_state->bigjoiner_slave = true;
- crtc_state->cpu_transcoder = from_crtc_state->cpu_transcoder;
- crtc_state->has_audio = from_crtc_state->has_audio;
+ slave_crtc_state->uapi.mode_changed = master_crtc_state->uapi.mode_changed;
+ slave_crtc_state->uapi.connectors_changed = master_crtc_state->uapi.connectors_changed;
+ slave_crtc_state->uapi.active_changed = master_crtc_state->uapi.active_changed;
+ slave_crtc_state->cpu_transcoder = master_crtc_state->cpu_transcoder;
+ slave_crtc_state->has_audio = master_crtc_state->has_audio;
return 0;
}
static int
intel_crtc_prepare_cleared_state(struct intel_atomic_state *state,
- struct intel_crtc_state *crtc_state)
+ struct intel_crtc *crtc)
{
- struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+ struct intel_crtc_state *crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
struct intel_crtc_state *saved_state;
@@ -5952,7 +5974,7 @@ intel_crtc_prepare_cleared_state(struct intel_atomic_state *state,
memcpy(crtc_state, saved_state, sizeof(*crtc_state));
kfree(saved_state);
- intel_crtc_copy_uapi_to_hw_state(state, crtc_state);
+ intel_crtc_copy_uapi_to_hw_state_modeset(state, crtc);
return 0;
}
@@ -7592,6 +7614,9 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state,
struct intel_crtc_state *slave_crtc_state;
struct intel_crtc *slave_crtc;
+ WARN_ON(master_crtc_state->bigjoiner_linked_crtc);
+ WARN_ON(master_crtc_state->bigjoiner_slave);
+
if (!master_crtc_state->bigjoiner)
return 0;
@@ -7604,7 +7629,6 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state,
return -EINVAL;
}
- master_crtc_state->bigjoiner_linked_crtc = slave_crtc;
slave_crtc_state = intel_atomic_get_crtc_state(&state->base, slave_crtc);
if (IS_ERR(slave_crtc_state))
return PTR_ERR(slave_crtc_state);
@@ -7613,11 +7637,28 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state,
if (slave_crtc_state->uapi.enable)
goto claimed;
+ /*
+ * The state copy logic assumes the master crtc gets processed
+ * before the slave crtc during the main compute_config loop.
+ * This works because the crtcs are created in pipe order,
+ * and the hardware requires master pipe < slave pipe as well.
+ * Should that change we need to rethink the logic.
+ */
+ if (WARN_ON(drm_crtc_index(&master_crtc->base) > drm_crtc_index(&slave_crtc->base)))
+ return -EINVAL;
+
drm_dbg_kms(&i915->drm,
- "[CRTC:%d:%s] Used as slave for big joiner\n",
- slave_crtc->base.base.id, slave_crtc->base.name);
+ "[CRTC:%d:%s] Used as slave for big joiner master [CRTC:%d:%s]\n",
+ slave_crtc->base.base.id, slave_crtc->base.name,
+ master_crtc->base.base.id, master_crtc->base.name);
- return copy_bigjoiner_crtc_state(slave_crtc_state, master_crtc_state);
+ master_crtc_state->bigjoiner_linked_crtc = slave_crtc;
+ master_crtc_state->bigjoiner_slave = false;
+
+ slave_crtc_state->bigjoiner_linked_crtc = master_crtc;
+ slave_crtc_state->bigjoiner_slave = true;
+
+ return copy_bigjoiner_crtc_state_modeset(state, slave_crtc);
claimed:
drm_dbg_kms(&i915->drm,
@@ -7629,15 +7670,19 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state,
}
static void kill_bigjoiner_slave(struct intel_atomic_state *state,
- struct intel_crtc_state *master_crtc_state)
+ struct intel_crtc *master_crtc)
{
+ struct intel_crtc_state *master_crtc_state =
+ intel_atomic_get_new_crtc_state(state, master_crtc);
+ struct intel_crtc *slave_crtc = master_crtc_state->bigjoiner_linked_crtc;
struct intel_crtc_state *slave_crtc_state =
- intel_atomic_get_new_crtc_state(state, master_crtc_state->bigjoiner_linked_crtc);
+ intel_atomic_get_new_crtc_state(state, slave_crtc);
slave_crtc_state->bigjoiner = master_crtc_state->bigjoiner = false;
slave_crtc_state->bigjoiner_slave = master_crtc_state->bigjoiner_slave = false;
slave_crtc_state->bigjoiner_linked_crtc = master_crtc_state->bigjoiner_linked_crtc = NULL;
- intel_crtc_copy_uapi_to_hw_state(state, slave_crtc_state);
+
+ intel_crtc_copy_uapi_to_hw_state_modeset(state, slave_crtc);
}
/**
@@ -7823,7 +7868,7 @@ static int intel_bigjoiner_add_affected_crtcs(struct intel_atomic_state *state)
/* Kill old bigjoiner link, we may re-establish afterwards */
if (intel_crtc_needs_modeset(crtc_state) &&
crtc_state->bigjoiner && !crtc_state->bigjoiner_slave)
- kill_bigjoiner_slave(state, crtc_state);
+ kill_bigjoiner_slave(state, crtc);
}
return 0;
@@ -7867,21 +7912,22 @@ static int intel_atomic_check(struct drm_device *dev,
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
if (!intel_crtc_needs_modeset(new_crtc_state)) {
- /* Light copy */
- intel_crtc_copy_uapi_to_hw_state_nomodeset(state, new_crtc_state);
-
+ if (new_crtc_state->bigjoiner_slave)
+ copy_bigjoiner_crtc_state_nomodeset(state, crtc);
+ else
+ intel_crtc_copy_uapi_to_hw_state_nomodeset(state, crtc);
continue;
}
if (!new_crtc_state->uapi.enable) {
if (!new_crtc_state->bigjoiner_slave) {
- intel_crtc_copy_uapi_to_hw_state(state, new_crtc_state);
+ intel_crtc_copy_uapi_to_hw_state_modeset(state, crtc);
any_ms = true;
}
continue;
}
- ret = intel_crtc_prepare_cleared_state(state, new_crtc_state);
+ ret = intel_crtc_prepare_cleared_state(state, crtc);
if (ret)
goto fail;
--
2.34.1
More information about the Intel-gfx
mailing list