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

[Richard Lane <richardlanedev@gmail.com>]

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.

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
>>