On Tue, Feb 4, 2020 at 9:06 AM Kazlauskas, Nicholas <nicholas.kazlauskas@xxxxxxx> wrote: > > Comments inline. > > On 2020-02-03 4:07 p.m., Alex Deucher wrote: > > Ping? > > > > On Fri, Jan 10, 2020 at 3:11 PM Alex Deucher <alexdeucher@xxxxxxxxx> wrote: > >> > >> It looks like we should be reducing the group size when we don't > >> have a plane rather than when we do. > >> > >> Bug: https://gitlab.freedesktop.org/drm/amd/issues/781 > >> Fixes: 5fc0cbfad45648 ("drm/amd/display: determine if a pipe is synced by plane state") > >> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx> > >> --- > >> drivers/gpu/drm/amd/display/dc/core/dc.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c > >> index 3d89904003f0..01b27726d9c5 100644 > >> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c > >> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c > >> @@ -1003,9 +1003,9 @@ static void program_timing_sync( > >> status->timing_sync_info.master = false; > >> > >> } > >> - /* remove any other pipes with plane as they have already been synced */ > >> + /* remove any other pipes without plane as they have already been synced */ > > This took a while to wrap my head around but I think I understand what > this was originally trying to do. > > The original logic seems to have been checking for blanked streams and > trying to remove anything that was blanked from the group to try and > avoid having to enable timing synchronization. > > However, 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. > > The problem is really this iteration below: > > >> for (j = j + 1; j < group_size; j++) { > > There could still be pipes in here (depending on the ordering) that have > planes and could be synchronized with the master OTG. I think starting > at j + 1 is a mistake for this logic as well. > > I wonder if we can just drop this loop altogether. If we add planes or > unblank the OTG later then we'll still want the synchronization. > > Dymtro, Wenjing - feel free to correct my understanding if I'm mistaken > about this. Ping? Any thoughts on this? It would be nice to get this fixed. Alex > > Regards, > Nicholas Kazlauskas > > >> - if (pipe_set[j]->plane_state) { > >> + if (!pipe_set[j]->plane_state) { > >> group_size--; > >> pipe_set[j] = pipe_set[group_size]; > >> j--; > >> -- > >> 2.24.1 > >> > > _______________________________________________ > > amd-gfx mailing list > > amd-gfx@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx > > > _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
