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

[Aurabindo Pillai <aurabindo.pillai@amd.com>]

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@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.

dc_link_remove_remote_sink() will only remove one sink. It returns as
soon as it is done removing the sink we asked for.


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