On 8/22/19 11:36 AM, Alex Deucher wrote: > Properly set all_displays_in_sync so that when the data is > propagated to powerplay, it's set properly and we can enable > mclk switching when all monitors are in sync. > > v2: fix logic, clean up > > Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx> > --- > .../gpu/drm/amd/display/dc/calcs/dce_calcs.c | 49 ++++++++++++++++++- > 1 file changed, 47 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c > index 9f12e21f8b9b..8d904647fb0f 100644 > --- a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c > +++ b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c > @@ -25,6 +25,7 @@ > > #include <linux/slab.h> > > +#include "resource.h" > #include "dm_services.h" > #include "dce_calcs.h" > #include "dc.h" > @@ -2977,6 +2978,50 @@ static void populate_initial_data( > data->number_of_displays = num_displays; > } > > +static bool all_displays_in_sync(const struct pipe_ctx pipe[], > + int pipe_count, > + uint32_t number_of_displays) > +{ > + const struct pipe_ctx *unsynced_pipes[MAX_PIPES] = { NULL }; > + int group_size = 1; > + int i, j; > + > + for (i = 0; i < pipe_count; i++) { > + if (!pipe[i].stream) This bit differs from program_timing_sync, but since this is for dce and we don't do pipe split or MPO I think it's probably fine that you're not checking top_pipe here. Wouldn't hurt to have that logic here though. > + continue; > + > + unsynced_pipes[i] = &pipe[i]; > + } > + > + for (i = 0; i < pipe_count; i++) { > + const struct pipe_ctx *pipe_set[MAX_PIPES]; > + > + if (!unsynced_pipes[i]) > + continue; > + > + pipe_set[0] = unsynced_pipes[i]; > + unsynced_pipes[i] = NULL; > + > + /* Add tg to the set, search rest of the tg's for ones with > + * same timing, add all tgs with same timing to the group > + */ > + for (j = i + 1; j < pipe_count; j++) { > + if (!unsynced_pipes[j]) > + continue; > + > + if (resource_are_streams_timing_synchronizable( > + unsynced_pipes[j]->stream, > + pipe_set[0]->stream)) { > + pipe_set[group_size] = unsynced_pipes[j]; > + unsynced_pipes[j] = NULL; > + group_size++; > + } > + } > + } > + > + return (group_size == number_of_displays) ? true : false; I think this logic is functional but it looks incorrect at first glance because group_size doesn't get reset. What ends up happening is the first pipe of each group doesn't get added to group_size. I feel that this would be more clear as: static bool all_displays_in_sync(const struct pipe_ctx pipe[], int pipe_count) { const struct pipe_ctx *active_pipes[MAX_PIPES]; int i, num_active_pipes = 0; for (i = 0; i < pipe_count; i++) { if (!pipe[i].stream || pipe[i].top_pipe) continue; active_pipes[num_active_pipes++] = &pipe[i]; } if (!num_active_pipes) return false; for (i = 1; i < num_active_pipes; ++i) if (!resource_are_streams_timing_synchronizable( active_pipes[0]->stream, active_pipes[i]->stream)) return false; return true; } But I haven't tested this. Nicholas Kazlauskas > +} > + > /** > * Return: > * true - Display(s) configuration supported. > @@ -2998,8 +3043,8 @@ bool bw_calcs(struct dc_context *ctx, > > populate_initial_data(pipe, pipe_count, data); > > - /*TODO: this should be taken out calcs output and assigned during timing sync for pplib use*/ > - calcs_output->all_displays_in_sync = false; > + calcs_output->all_displays_in_sync = all_displays_in_sync(pipe, pipe_count, > + data->number_of_displays); > > if (data->number_of_displays != 0) { > uint8_t yclk_lvl, sclk_lvl; > _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx