RE: [PATCH] drm/amd/mst: clean DP main link status only when unplug mst 1st link
[AMD Public Use] > -Original Message- > From: Siqueira, Rodrigo > Sent: Wednesday, August 5, 2020 10:55 PM > To: Lin, Wayne > Cc: amd-gfx@lists.freedesktop.org; Lakha, Bhawanpreet > ; Wu, Hersen ; > Kazlauskas, Nicholas ; Zuo, Jerry > > 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 > > --- > > 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(>lock); > > + if (!mgr->mst_state) > > + aconnector->dc_link->cur_link_settings.lane_count = 0; > > + mutex_unlock(>lock); > Hi Wayne, > > The change looks good to me. > > Reviewed-by: Rodrigo Siqueira > > 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
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 > --- > 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(>lock); > + if (!mgr->mst_state) > + aconnector->dc_link->cur_link_settings.lane_count = 0; > + mutex_unlock(>lock); Hi Wayne, The change looks good to me. Reviewed-by: Rodrigo Siqueira 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. Thanks > } > > drm_connector_unregister(connector); > -- > 2.17.1 > -- Rodrigo Siqueira https://siqueira.tech signature.asc Description: PGP signature ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amd/mst: clean DP main link status only when unplug mst 1st link
[AMD Public Use] Sorry for spamming. It's a duplicate one and should be ignored. > -Original Message- > From: Wayne Lin > Sent: Tuesday, August 4, 2020 11:37 AM > To: amd-gfx@lists.freedesktop.org > Cc: Pillai, Aurabindo ; Wu, Hersen > ; Kazlauskas, Nicholas > ; Siqueira, Rodrigo > ; Zuo, Jerry ; Lin, Wayne > > Subject: [PATCH] drm/amd/mst: clean DP main link status only when unplug > mst 1st link > > [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 > --- > 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(>lock); > + if (!mgr->mst_state) > + aconnector->dc_link->cur_link_settings.lane_count = 0; > + mutex_unlock(>lock); > } > > drm_connector_unregister(connector); > -- > 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx