[PATCH] drm/amdgpu/display: drop the reduction loop when setting the sync groups

Kazlauskas, Nicholas nicholas.kazlauskas at amd.com
Fri May 29 13:56:33 UTC 2020


On 2020-05-28 10:06 a.m., Alex Deucher wrote:
> The logic for blanked is not the same as having a plane_state. Technically
> you can drive an OTG without anything connected in the front end and it'll
> just draw out the back color which is distinct from having the OTG be blanked.
> If we add planes or unblank the OTG later then we'll still want the
> synchronization.
> 
> Bug: https://gitlab.freedesktop.org/drm/amd/issues/781
> Fixes: 5fc0cbfad45648 ("drm/amd/display: determine if a pipe is synced by plane state")
> Cc: nicholas.kazlauskas at amd.com
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com> > ---
>   drivers/gpu/drm/amd/display/dc/core/dc.c | 8 --------
>   1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 04c3d9f7e323..6279520f7873 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -1040,14 +1040,6 @@ static void program_timing_sync(
>   				status->timing_sync_info.master = false;
>   
>   		}
> -		/* remove any other pipes with plane as they have already been synced */
> -		for (j = j + 1; j < group_size; j++) {
> -			if (pipe_set[j]->plane_state) {
> -				group_size--;
> -				pipe_set[j] = pipe_set[group_size];
> -				j--;
> -			}
> -		}


Looking at this again, I think I may understand the issue this was 
trying to work around.

If we try to force timing synchronization on displays that are currently 
active then this is going to force reset the vertical position, 
resulting in screen corruption.

So what this logic was attempting to do was ensure that timing 
synchronization only happens when committing two streams at a time 
without any image on the screen.

Maybe it'd be best to just blank these streams out first, but for now, 
let's actually go back to fixing this by applying the actual dpg/tg 
check that Wenjing suggests, something like:

    if (pool->opps[i]->funcs->dpg_is_blanked)
                 s.blank_enabled = 
pool->opps[i]->funcs->dpg_is_blanked(pool->opps[i]);
            else
                 s.blank_enabled = tg->funcs->is_blanked(tg);



The reason why we have this issue in the first place is because 
amdgpu_dm usually commits a dc_state with the planes already in it 
instead of committing them later, so plane_state not being NULL is 
typically true.

Regards,
Nicholas Kazlauskas

>   
>   		if (group_size > 1) {
>   			dc->hwss.enable_timing_synchronization(
> 



More information about the amd-gfx mailing list