Hi,
Some MST fixups that landed in 6.3-rc1 were CC to stable and successfully applied to 6.2.y, but failed to apply to 6.1.y even though they are needed there as well.
They fail to apply due to this commit missing in 6.1.y: commit 8c7d980da9ba ("drm/nouveau/disp: move DP MST payload config method")
Backporting that is a rabbit hole of other work and makes this no longer viable for stable, so instead hand modify commit e761cc20946a ("drm/display/dp_mst: Handle old/new payload states in drm_dp_remove_payload()")
for the missing contextual changes in 6.1.y in nouveau.
I've tested that these work on a Rembrandt laptop connected to WD19TB with two monitors connected.
Imre Deak (2): drm/display/dp_mst: Handle old/new payload states in drm_dp_remove_payload() drm/i915/dp_mst: Fix payload removal during output disabling
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 26 ++++++++++--------- drivers/gpu/drm/i915/display/intel_dp_mst.c | 14 +++++++--- drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- include/drm/display/drm_dp_mst_helper.h | 3 ++- 5 files changed, 28 insertions(+), 19 deletions(-)
From: Imre Deak imre.deak@intel.com
Atm, drm_dp_remove_payload() uses the same payload state to both get the vc_start_slot required for the payload removal DPCD message and to deduct time_slots from vc_start_slot of all payloads after the one being removed.
The above isn't always correct, as vc_start_slot must be the up-to-date version contained in the new payload state, but time_slots must be the one used when the payload was previously added, contained in the old payload state. The new payload's time_slots can change vs. the old one if the current atomic commit changes the corresponding mode.
This patch let's drivers pass the old and new payload states to drm_dp_remove_payload(), but keeps these the same for now in all drivers not to change the behavior. A follow-up i915 patch will pass in that driver the correct old and new states to the function.
Cc: Lyude Paul lyude@redhat.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Ben Skeggs bskeggs@redhat.com Cc: Karol Herbst kherbst@redhat.com Cc: Harry Wentland harry.wentland@amd.com Cc: Alex Deucher alexander.deucher@amd.com Cc: Wayne Lin Wayne.Lin@amd.com Cc: stable@vger.kernel.org # 6.1 Cc: dri-devel@lists.freedesktop.org Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Lyude Paul lyude@redhat.com Acked-by: Lyude Paul lyude@redhat.com Acked-by: Daniel Vetter daniel@ffwll.ch Acked-by: Wayne Lin wayne.lin@amd.com Acked-by: Jani Nikula jani.nikula@intel.com Signed-off-by: Imre Deak imre.deak@intel.com Link: https://patchwork.freedesktop.org/patch/msgid/20230206114856.2665066-2-imre.... (cherry picked from commit e761cc20946a0094df71cb31a565a6a0d03bd8be) Hand modified for missing 8c7d980da9ba3eb67a1b40fd4b33bcf49397084b Signed-off-by: Mario Limonciello mario.limonciello@amd.com --- .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 26 ++++++++++--------- drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 ++- drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- include/drm/display/drm_dp_mst_helper.h | 3 ++- 5 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index 57454967617f..6bd7e4537014 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -206,7 +206,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( if (enable) drm_dp_add_payload_part1(mst_mgr, mst_state, payload); else - drm_dp_remove_payload(mst_mgr, mst_state, payload); + drm_dp_remove_payload(mst_mgr, mst_state, payload, payload);
/* mst_mgr->->payloads are VC payload notify MST branch using DPCD or * AUX message. The sequence is slot 1-63 allocated sequence for each diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index e77c674b37ca..38dab76ae69e 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -3342,7 +3342,8 @@ EXPORT_SYMBOL(drm_dp_add_payload_part1); * drm_dp_remove_payload() - Remove an MST payload * @mgr: Manager to use. * @mst_state: The MST atomic state - * @payload: The payload to write + * @old_payload: The payload with its old state + * @new_payload: The payload to write * * Removes a payload from an MST topology if it was successfully assigned a start slot. Also updates * the starting time slots of all other payloads which would have been shifted towards the start of @@ -3350,36 +3351,37 @@ EXPORT_SYMBOL(drm_dp_add_payload_part1); */ void drm_dp_remove_payload(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_topology_state *mst_state, - struct drm_dp_mst_atomic_payload *payload) + const struct drm_dp_mst_atomic_payload *old_payload, + struct drm_dp_mst_atomic_payload *new_payload) { struct drm_dp_mst_atomic_payload *pos; bool send_remove = false;
/* We failed to make the payload, so nothing to do */ - if (payload->vc_start_slot == -1) + if (new_payload->vc_start_slot == -1) return;
mutex_lock(&mgr->lock); - send_remove = drm_dp_mst_port_downstream_of_branch(payload->port, mgr->mst_primary); + send_remove = drm_dp_mst_port_downstream_of_branch(new_payload->port, mgr->mst_primary); mutex_unlock(&mgr->lock);
if (send_remove) - drm_dp_destroy_payload_step1(mgr, mst_state, payload); + drm_dp_destroy_payload_step1(mgr, mst_state, new_payload); else drm_dbg_kms(mgr->dev, "Payload for VCPI %d not in topology, not sending remove\n", - payload->vcpi); + new_payload->vcpi);
list_for_each_entry(pos, &mst_state->payloads, next) { - if (pos != payload && pos->vc_start_slot > payload->vc_start_slot) - pos->vc_start_slot -= payload->time_slots; + if (pos != new_payload && pos->vc_start_slot > new_payload->vc_start_slot) + pos->vc_start_slot -= old_payload->time_slots; } - payload->vc_start_slot = -1; + new_payload->vc_start_slot = -1;
mgr->payload_count--; - mgr->next_start_slot -= payload->time_slots; + mgr->next_start_slot -= old_payload->time_slots;
- if (payload->delete) - drm_dp_mst_put_port_malloc(payload->port); + if (new_payload->delete) + drm_dp_mst_put_port_malloc(new_payload->port); } EXPORT_SYMBOL(drm_dp_remove_payload);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 27c2098dd707..256afff75b0a 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -366,6 +366,8 @@ static void intel_mst_disable_dp(struct intel_atomic_state *state, to_intel_connector(old_conn_state->connector); struct drm_dp_mst_topology_state *mst_state = drm_atomic_get_mst_topology_state(&state->base, &intel_dp->mst_mgr); + struct drm_dp_mst_atomic_payload *payload = + drm_atomic_get_mst_payload_state(mst_state, connector->port); struct drm_i915_private *i915 = to_i915(connector->base.dev);
drm_dbg_kms(&i915->drm, "active links %d\n", @@ -374,7 +376,7 @@ static void intel_mst_disable_dp(struct intel_atomic_state *state, intel_hdcp_disable(intel_mst->connector);
drm_dp_remove_payload(&intel_dp->mst_mgr, mst_state, - drm_atomic_get_mst_payload_state(mst_state, connector->port)); + payload, payload);
intel_audio_codec_disable(encoder, old_crtc_state, old_conn_state); } diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 33c97d510999..4f1bf1cb9e0c 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -996,7 +996,7 @@ nv50_msto_prepare(struct drm_atomic_state *state,
// TODO: Figure out if we want to do a better job of handling VCPI allocation failures here? if (msto->disabled) { - drm_dp_remove_payload(mgr, mst_state, payload); + drm_dp_remove_payload(mgr, mst_state, payload, payload); } else { if (msto->enabled) drm_dp_add_payload_part1(mgr, mst_state, payload); diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h index 6c0c3dc3d3ac..32c764fb9cb5 100644 --- a/include/drm/display/drm_dp_mst_helper.h +++ b/include/drm/display/drm_dp_mst_helper.h @@ -841,7 +841,8 @@ int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_atomic_payload *payload); void drm_dp_remove_payload(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_topology_state *mst_state, - struct drm_dp_mst_atomic_payload *payload); + const struct drm_dp_mst_atomic_payload *old_payload, + struct drm_dp_mst_atomic_payload *new_payload);
int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr);
From: Imre Deak imre.deak@intel.com
Use the correct old/new topology and payload states in intel_mst_disable_dp(). So far drm_atomic_get_mst_topology_state() it used returned either the old state, in case the state was added already earlier during the atomic check phase or otherwise the new state (but the latter could fail, which can't be handled in the enable/disable hooks). After the first patch in the patchset, the state should always get added already during the check phase, so here we can get the old/new states without a failure.
drm_dp_remove_payload() should use time_slots from the old payload state and vc_start_slot in the new one. It should update the new payload states to reflect the sink's current payload table after the payload is removed. Pass the new topology state and the old and new payload states accordingly.
This also fixes a problem where the payload allocations for multiple MST streams on the same link got inconsistent after a few commits, as during payload removal the old instead of the new payload state got updated, so the subsequent enabling sequence and commits used a stale payload state.
v2: Constify the old payload state pointer. (Ville)
Cc: Lyude Paul lyude@redhat.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org # 6.1 Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Lyude Paul lyude@redhat.com Acked-by: Lyude Paul lyude@redhat.com Acked-by: Daniel Vetter daniel@ffwll.ch Acked-by: Wayne Lin wayne.lin@amd.com Acked-by: Jani Nikula jani.nikula@intel.com Signed-off-by: Imre Deak imre.deak@intel.com Link: https://patchwork.freedesktop.org/patch/msgid/20230206114856.2665066-4-imre.... (cherry picked from commit eb50912ec931913e70640cecf75cb993fd26995f) Signed-off-by: Mario Limonciello mario.limonciello@amd.com --- drivers/gpu/drm/i915/display/intel_dp_mst.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 256afff75b0a..9a6822256ddf 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -364,10 +364,14 @@ static void intel_mst_disable_dp(struct intel_atomic_state *state, struct intel_dp *intel_dp = &dig_port->dp; struct intel_connector *connector = to_intel_connector(old_conn_state->connector); - struct drm_dp_mst_topology_state *mst_state = - drm_atomic_get_mst_topology_state(&state->base, &intel_dp->mst_mgr); - struct drm_dp_mst_atomic_payload *payload = - drm_atomic_get_mst_payload_state(mst_state, connector->port); + struct drm_dp_mst_topology_state *old_mst_state = + drm_atomic_get_old_mst_topology_state(&state->base, &intel_dp->mst_mgr); + struct drm_dp_mst_topology_state *new_mst_state = + drm_atomic_get_new_mst_topology_state(&state->base, &intel_dp->mst_mgr); + const struct drm_dp_mst_atomic_payload *old_payload = + drm_atomic_get_mst_payload_state(old_mst_state, connector->port); + struct drm_dp_mst_atomic_payload *new_payload = + drm_atomic_get_mst_payload_state(new_mst_state, connector->port); struct drm_i915_private *i915 = to_i915(connector->base.dev);
drm_dbg_kms(&i915->drm, "active links %d\n", @@ -375,8 +379,8 @@ static void intel_mst_disable_dp(struct intel_atomic_state *state,
intel_hdcp_disable(intel_mst->connector);
- drm_dp_remove_payload(&intel_dp->mst_mgr, mst_state, - payload, payload); + drm_dp_remove_payload(&intel_dp->mst_mgr, new_mst_state, + old_payload, new_payload);
intel_audio_codec_disable(encoder, old_crtc_state, old_conn_state); }
On Wed, Apr 05, 2023 at 02:37:05PM -0500, Mario Limonciello wrote:
Hi,
Some MST fixups that landed in 6.3-rc1 were CC to stable and successfully applied to 6.2.y, but failed to apply to 6.1.y even though they are needed there as well.
They fail to apply due to this commit missing in 6.1.y: commit 8c7d980da9ba ("drm/nouveau/disp: move DP MST payload config method")
Backporting that is a rabbit hole of other work and makes this no longer viable for stable, so instead hand modify commit e761cc20946a ("drm/display/dp_mst: Handle old/new payload states in drm_dp_remove_payload()")
for the missing contextual changes in 6.1.y in nouveau.
I've tested that these work on a Rembrandt laptop connected to WD19TB with two monitors connected.
Imre Deak (2): drm/display/dp_mst: Handle old/new payload states in drm_dp_remove_payload() drm/i915/dp_mst: Fix payload removal during output disabling
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 26 ++++++++++--------- drivers/gpu/drm/i915/display/intel_dp_mst.c | 14 +++++++--- drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- include/drm/display/drm_dp_mst_helper.h | 3 ++- 5 files changed, 28 insertions(+), 19 deletions(-)
-- 2.34.1
Both now queued up, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Mario Limonciello