On Fri, May 29, 2020 at 9:56 AM Kazlauskas, Nicholas <nicholas.kazlauskas@xxxxxxx> wrote: > > 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@xxxxxxx > > Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx> > --- > > 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); > Hmm, it's not clear to me where this code needs to go. Can you point me in the right direction or provide a quick patch? Thanks, Alex > > > 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( > > > _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
