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

[Richard Lane <richardlanedev@gmail.com>]

On 13/11/2026 14:05, Alex Deucher wrote:
> On Thu, Nov 12, 2026 at 7:15 PM Richard Lane <richardlanedev@gmail.com> wrote:
>>
>> On 10/11/2026 19:59, Alex Deucher wrote:
>>> On Tue, Nov 10, 2026 at 1:49 PM Richard Lane <richardlanedev@gmail.com> wrote:
>>>>
>>>> On 09/11/2026 20:34, Alex Deucher wrote:
>>>>> On Mon, Nov 2, 2026 at 3:31 AM Richard Lane <richardlanedev@gmail.com> 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
>>>
>>
>> I think this could be limited to DCE 6,8,10 rather than DCE < 12 as in these
>> cases DC uses dce100_validate_bandwdith() which doesn't call bw_calcs() and
>> dce_pplib_apply_display_requirements() in dce_clk_mgr.c so
>> pp_display_cfg->all_displays_in_sync is unset.
> 
> How about adding bw_calcs() to dce100_validate_bandwdith()?  Or if
> there are issues there, just add the all_displays_in_sync() checks to
> dce100_validate_bandwdith().
> 
> Alex
> 

bw_calcs() itself cannot be called in dce100_validate_bandwidth() because
bw_calcs_init() only initializes dc->bw_dceip and dc->bw_vbios on DCE
11.0, 11.2, and 12.0.

However, adding the all_displays_in_sync check directly to
dce100_validate_bandwidth() (which is shared by DCE 6, 8, and 10) and
copying context->bw_ctx.bw.dce.all_displays_in_sync to
pp_display_cfg->all_displays_in_sync in
dce_pplib_apply_display_requirements() works. Is this solution preferable?

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
@@ -320,6 +320,9 @@ static void dce_pplib_apply_display_requirements(
 {
 	struct dm_pp_display_configuration *pp_display_cfg = &context->pp_display_cfg;
 
+	pp_display_cfg->all_displays_in_sync =
+		context->bw_ctx.bw.dce.all_displays_in_sync;
+
 	dce110_fill_display_configs(context, pp_display_cfg);
 
 	if (memcmp(&dc->current_state->pp_display_cfg, pp_display_cfg, sizeof(*pp_display_cfg)) !=  0)
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dce100/dce100_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dce100/dce100_resource.c
--- a/drivers/gpu/drm/amd/display/dc/resource/dce100/dce100_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dce100/dce100_resource.c
@@ -692,6 +692,14 @@ enum dc_status dce100_validate_bandwidth(
 		/* TODO implement when needed but for now hardcode max value*/
 		context->bw_ctx.bw.dce.dispclk_khz = 681000;
 		context->bw_ctx.bw.dce.yclk_khz = 250000 * MEMORY_TYPE_MULTIPLIER_CZ;
+		context->bw_ctx.bw.dce.all_displays_in_sync = context->stream_count > 1;
+		for (i = 1; i < context->stream_count; i++) {
+			if (!resource_are_streams_timing_synchronizable(
+				    context->streams[0], context->streams[i])) {
+				context->bw_ctx.bw.dce.all_displays_in_sync = false;
+				break;
+			}
+		}
 	} else {
 		/* On DCE 6.0 and 6.4 the PLL0 is both the display engine clock and
 		 * the DP clock, and shouldn't be turned off. Just select the display
@@ -703,6 +711,7 @@ enum dc_status dce100_validate_bandwidth(
 			context->bw_ctx.bw.dce.dispclk_khz = 0;
 
 		context->bw_ctx.bw.dce.yclk_khz = 0;
+		context->bw_ctx.bw.dce.all_displays_in_sync = false;
 	}
 
 	return DC_OK;

Richard

>>
>> Richard
>>
>>>>
>>>> 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@gmail.com>
>>>>>> ---
>>>>>>  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
>>>>
>>