[AMD Public Use] > -----Original Message----- > From: Siqueira, Rodrigo <Rodrigo.Siqueira@xxxxxxx> > Sent: Wednesday, August 5, 2020 10:55 PM > To: Lin, Wayne <Wayne.Lin@xxxxxxx> > Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Lakha, Bhawanpreet > <Bhawanpreet.Lakha@xxxxxxx>; Wu, Hersen <hersenxs.wu@xxxxxxx>; > Kazlauskas, Nicholas <Nicholas.Kazlauskas@xxxxxxx>; Zuo, Jerry > <Jerry.Zuo@xxxxxxx> > Subject: Re: [PATCH] drm/amd/mst: clean DP main link status only when > unplug mst 1st link > > On 08/04, Wayne Lin wrote: > > [Why] > > Under DP daisy chain scenario as below: > > > > Src - Monitor_1 - Monitor_2 > > > > If unplug 2nd Monitor_2 and plug in again, observe that Monitor_1 > > doesn't light up. > > > > When unplug 2nd monitor, we clear the > > dc_link->cur_link_settings.lane_count in dm_dp_destroy_mst_connector(). > > However this link status is a shared data structure by all connected > > mst monitors. Although the 2nd monitor is gone, this link status > > should still be retained for other connected mst monitors. > > Otherwise, when we plug the 2nd monitor in again, we find out that > > main link is not trained and do link training again. Payload ID > > Table for Monitor_1 is ruined and we don't reallocate it. > > > > [How] > > In dm_dp_destroy_mst_connector(), only clean the cur_link_settings > > when we no longer do mst mode. > > > > Signed-off-by: Wayne Lin <Wayne.Lin@xxxxxxx> > > --- > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 5 > ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git > > a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > > index 2c10352fa514..526f29598403 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > > @@ -415,7 +415,10 @@ static void dm_dp_destroy_mst_connector(struct > drm_dp_mst_topology_mgr *mgr, > > aconnector->dc_sink); > > dc_sink_release(aconnector->dc_sink); > > aconnector->dc_sink = NULL; > > - aconnector->dc_link->cur_link_settings.lane_count = 0; > > + mutex_lock(&mgr->lock); > > + if (!mgr->mst_state) > > + aconnector->dc_link->cur_link_settings.lane_count = 0; > > + mutex_unlock(&mgr->lock); > Hi Wayne, > > The change looks good to me. > > Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@xxxxxxx> > > Just for curiosity, do you know why we use a mutex instead of > spin_lock_irq for this case? FWIU, the spin_lock_irq behavior looks > better for this sort of manipulation. Hi Siqueira, Thanks for your time. AFAIK, changing mst_state (e.g. enabling MST mode) involves some reading/writing steps on DPCD which might cost some time. > > Thanks > > > } > > > > drm_connector_unregister(connector); > > -- > > 2.17.1 > > > > -- > Rodrigo Siqueira > https://siqueira.tech -- Wayne Lin _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
