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

[Harry Wentland]

On 2020-11-26 9:35 a.m., Kazlauskas, Nicholas wrote:
> 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 at amd.com>
>> Signed-off-by: Eryk Brol <eryk.brol at 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 at 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.
> 

I'd prefer we clean this up if it's redundant. Otherwise redundant code 
stays around forever and makes future debug more cumbersome.

Harry

> 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 at lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7CHARRY.WENTLAND%40amd.com%7C7576866a05154b66cdb208d8921898da%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637419981644444920%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=sm7jvbxW%2BgEUevWgjACydnjuiE7SgtunMBUIpLT99r4%3D&reserved=0 
>