On Thu, Jan 26, 2023 at 08:38:16PM +0200, Ville Syrjälä wrote: > 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?
Yes, will change this. > > 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
