Ping again? Alex On Thu, Feb 20, 2020 at 8:27 AM Alex Deucher <alexdeucher@xxxxxxxxx> wrote: > > 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
