Re: [PATCH v2 3/3] drm/amd/display: report synced displays to PowerPlay for MCLK switch

[Alex Deucher]

On Tue, Nov 10, 2026 at 1:49 PM Richard Lane <richardlanedev@xxxxxxxxx> wrote:
>
> On 09/11/2026 20:34, Alex Deucher wrote:
> > On Mon, Nov 2, 2026 at 3:31 AM Richard Lane <richardlanedev@xxxxxxxxx> wrote:
> >>
> >> When driving multiple displays or tiled panels with identical timings
> >> (such as the two 2560x2880 tiles of an Apple 5K iMac panel), Display
> >> Core's program_timing_sync() aligns the CRTCs into a timing sync group
> >> in hardware during dc_commit_streams().
> >>
> >> However, on DCE < 12, Display Core leaves
> >> pp_display_cfg.all_displays_in_sync unset. Because PowerPlay is not
> >> informed that the streams have synchronized vertical blanking intervals,
> >> SMU7 prevents MCLK switching at idle or causes modeset underflows.
> >>
> >> If all active streams belong to the same timing sync group
> >> (timing_sync_info.group_size == params.stream_count), report
> >> all_displays_in_sync = true to PowerPlay after committing the streams so
> >> SMU7 can safely downclock MCLK at idle.
> >>
> >
> > Looking at this a little deeper, the function bw_calcs() in
> > dce_calcs.c already calculates all_displays_in_sync using the helper
> > all_displays_in_sync().  Can you look into this and figure out why
> > that is not working as expected?  My concern with this patch is that
> > DCE and DCN have different logic for handling mclk switching so I'm
> > not sure it's safe to call dm_pp_apply_display_requirements() directly
> > from common code.
> >
> > Thanks,
> >
> > Alex
>
> Moving it to dce_clk_mgr.c and using resource_are_streams_timing_synchronizable() also works. Is that a solution?

I think that would be preferred, but in general, why doesn't the
current flow I described work?  The function bw_calcs() dce_calcs.c
already calculates the all_displays_in_sync flag using the helper
all_displays_in_sync().  Is all_displays_in_sync() not checking
something properly or is the overall call timing wrong such that
all_displays_in_sync() can't properly calculate whether all of the
displays are synced at the point that it gets called?

Alex

>
> diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dce100/dce_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dce100/dce_clk_mgr.c
> --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dce100/dce_clk_mgr.c
> +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dce100/dce_clk_mgr.c
> @@ -31,6 +31,7 @@
>  #include "dmcu.h"
>  #include "core_types.h"
>  #include "dal_asic_id.h"
> +#include "resource.h"
>
>  /*
>   * Currently the register shifts and masks in this file are used for dce100 and dce80
> @@ -319,8 +320,17 @@ static void dce_pplib_apply_display_requirements(
>         struct dc_state *context)
>  {
>         struct dm_pp_display_configuration *pp_display_cfg = &context->pp_display_cfg;
> +       int j;
>
>         dce110_fill_display_configs(context, pp_display_cfg);
> +       pp_display_cfg->all_displays_in_sync = pp_display_cfg->display_count > 1;
> +       for (j = 1; j < context->stream_count; j++) {
> +               if (!resource_are_streams_timing_synchronizable(
> +                           context->streams[0], context->streams[j])) {
> +                       pp_display_cfg->all_displays_in_sync = false;
> +                       break;
> +               }
> +       }
>
>         if (memcmp(&dc->current_state->pp_display_cfg, pp_display_cfg, sizeof(*pp_display_cfg)) !=  0)
>                 dm_pp_apply_display_requirements(dc->ctx, pp_display_cfg);
>
> Richard
>
> >
> >> Signed-off-by: Richard Lane <richardlanedev@xxxxxxxxx>
> >> ---
> >>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> index 74cd88d8d580..b4531383437e 100644
> >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> @@ -4777,6 +4777,16 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_commit *state,
> >>         dc_exit_ips_for_hw_access(dm->dc);
> >>         WARN_ON(!dc_commit_streams(dm->dc, &params));
> >>
> >> +       if (params.stream_count > 1 &&
> >> +           dm->dc->current_state->stream_status[0].timing_sync_info.group_size ==
> >> +           params.stream_count) {
> >> +               struct dm_pp_display_configuration *pp_cfg =
> >> +                       &dm->dc->current_state->pp_display_cfg;
> >> +
> >> +               pp_cfg->all_displays_in_sync = true;
> >> +               dm_pp_apply_display_requirements(dm->dc->ctx, pp_cfg);
> >> +       }
> >> +
> >>         bool frl_stream_found = false;
> >>
> >>         for (i = 0; i < params.stream_count; i++) {
> >> --
> >> 2.55.0
>