[PATCH] drm/amd/mst: clean DP main link status only when unplug mst 1st link

[Rodrigo Siqueira]

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 at amd.com>
> ---
>  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 at amd.com>

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20200805/fa5b7961/attachment.sig>