Re: [PATCH] drm/amd/display: Clear dc remote sinks on MST disconnect

["Kazlauskas, Nicholas" <nicholas.kazlauskas@amd.com>]

On 2020-11-26 9:31 a.m., Aurabindo Pillai wrote:
> [Why&How]
> Recent changes to upstream mst code remove the callback which
> cleared the internal state for mst. Move the missing functionality
> that was previously called through the destroy call back for mst connector
> destroy
> 
> Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
> Signed-off-by: Eryk Brol <eryk.brol@amd.com>
> ---
>   .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 22 +++++++++++++++++--
>   drivers/gpu/drm/amd/display/dc/dm_helpers.h   |  2 +-
>   2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> index b7d7ec3ba00d..d8b0f07deaf2 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> @@ -418,9 +418,10 @@ bool dm_helpers_dp_mst_start_top_mgr(
>   
>   void dm_helpers_dp_mst_stop_top_mgr(
>   		struct dc_context *ctx,
> -		const struct dc_link *link)
> +		struct dc_link *link)
>   {
>   	struct amdgpu_dm_connector *aconnector = link->priv;
> +	uint8_t i;
>   
>   	if (!aconnector) {
>   		DRM_ERROR("Failed to find connector for link!");
> @@ -430,8 +431,25 @@ void dm_helpers_dp_mst_stop_top_mgr(
>   	DRM_INFO("DM_MST: stopping TM on aconnector: %p [id: %d]\n",
>   			aconnector, aconnector->base.base.id);
>   
> -	if (aconnector->mst_mgr.mst_state == true)
> +	if (aconnector->mst_mgr.mst_state == true) {
>   		drm_dp_mst_topology_mgr_set_mst(&aconnector->mst_mgr, false);
> +
> +		for (i = 0; i < MAX_SINKS_PER_LINK; i++) {
> +			if (link->remote_sinks[i] == NULL)
> +				continue;
> +
> +			if (link->remote_sinks[i]->sink_signal ==
> +			    SIGNAL_TYPE_DISPLAY_PORT_MST) {
> +				dc_link_remove_remote_sink(link, link->remote_sinks[i]);

In general I think this patch looks fine, and you can have the:

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

But I think that this loop is redundant - dc_link_remove_remote_sink 
should be removing all the remote sinks. Not sure if remote_sinks can 
start at an index other than 0 though.

Regards,
Nicholas Kazlauskas

> +
> +				if (aconnector->dc_sink) {
> +					dc_sink_release(aconnector->dc_sink);
> +					aconnector->dc_sink = NULL;
> +					aconnector->dc_link->cur_link_settings.lane_count = 0;
> +				}
> +			}
> +		}
> +	} >   }
>   
>   bool dm_helpers_dp_read_dpcd(
> diff --git a/drivers/gpu/drm/amd/display/dc/dm_helpers.h b/drivers/gpu/drm/amd/display/dc/dm_helpers.h
> index b2cd8491c707..07e349b1067b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dm_helpers.h
> +++ b/drivers/gpu/drm/amd/display/dc/dm_helpers.h
> @@ -113,7 +113,7 @@ bool dm_helpers_dp_mst_start_top_mgr(
>   
>   void dm_helpers_dp_mst_stop_top_mgr(
>   		struct dc_context *ctx,
> -		const struct dc_link *link);
> +		struct dc_link *link);
>   /**
>    * OS specific aux read callback.
>    */
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx