On Wed, Jan 25, 2023 at 01:48:47PM +0200, Imre Deak wrote: > 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. > > Cc: Lyude Paul <ly...@redhat.com> > Cc: sta...@vger.kernel.org # 6.1 > Signed-off-by: Imre Deak <imre.d...@intel.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 5f7bcb5c14847..800fa12a61d93 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -524,10 +524,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); > + 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);
old states could be const no? > struct drm_i915_private *i915 = to_i915(connector->base.dev); > > drm_dbg_kms(&i915->drm, "active links %d\n", > @@ -535,8 +539,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, Right that one needs to be 'new' to update the start_slots Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com> > + old_payload, new_payload); > > intel_audio_codec_disable(encoder, old_crtc_state, old_conn_state); > } > -- > 2.37.1 -- Ville Syrjälä Intel
