[PATCH 17/25] drm/amd/display: Do DC mode-change check when adding CRTCs

Leo Li sunpeng.li at amd.com
Thu Nov 23 20:00:52 UTC 2017



On 2017-11-17 11:46 PM, Andrey Grodzovsky wrote:
> 
> 
> On 2017-11-16 10:32 AM, Harry Wentland wrote:
>> From: "Leo (Sunpeng) Li" <sunpeng.li at amd.com>
>>
>> Within atomic check, dm_update_crtcs_state is called twice. First to
>> remove from the dc_state, and subsequently to add to it.
>>
>> In both calls, a secondary mode-change check is done using dc-level
>> states. We shouldn't be doing this while removing, since a new
>> dc_stream_state has not been created to do the necessary comparison.
>> Because of this, the mode_changed flag within the DRM state can be
>> mistakenly set to false. Doing so only when adding prevents this.
>>
>> We are also guaranteed that a call to add will come after remove, or
>> else the atomic check fails (and a commit will not happen).
>>
>> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li at amd.com>
>> Reviewed-by: Tony Cheng <Tony.Cheng at amd.com>
>> Acked-by: Harry Wentland <harry.wentland at amd.com>
>> ---
>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index d10f8c09469b..05bc98d74a33 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -4535,7 +4535,7 @@ static int dm_update_crtcs_state(struct dc *dc,
>>               }
>>           }
>> -        if (dc_is_stream_unchanged(new_stream, 
>> dm_old_crtc_state->stream) &&
>> +        if (enable && dc_is_stream_unchanged(new_stream, 
>> dm_old_crtc_state->stream) &&
>>                   dc_is_stream_scaling_unchanged(new_stream, 
>> dm_old_crtc_state->stream)) {
>>               new_crtc_state->mode_changed = false;
> 
> Why just not move this logic into the if (aconnector && enable) {} close 
> immediately above ? Seems this call has no meaning unless new_stream is 
> initialized anyway.
> 
> Thanks,
> Andrey

Good point, the intent is clearer that way as well.

I've submitted the change internally, it will pop up in the next promotion.

Thanks,
Leo

> 
> 


More information about the amd-gfx mailing list