Re: [PATCH 3/3] drm/amd/display: Set stricter clock dividers on DCE 6-10

["Timur Kristóf" <timur.kristof@gmail.com>]

On 10/9/25 21:38, Alex Deucher wrote:
> On Sun, Sep 28, 2025 at 7:39 PM <timur.kristof@gmail.com> wrote:
>>
>> On Sun, 2025-09-28 at 16:14 +0200, Christian König wrote:
>>>
>>>
>>> On 26.09.25 20:26, Timur Kristóf wrote:
>>>> Set stricter dividers to stabilize the PLL's feedback loop.
>>>> In practice, the actual output isn't exactly the target
>>>> clock, but slowly oscillates around it. This makes it
>>>> more stable.
>>>>
>>>> The values here are taken from the non-DC code.
>>>
>>> There are also a bunch of other restrictions which need to be kept in
>>> mind.
>>>
>>> For example what is the minimum feedback divider value the DC code
>>> uses?
>>
>> As far as I see DC doesn't have minimum / maximum limits for the
>> feedback divider right now, though I can add that in a future patch if
>> necessary.
>>
>>>
>>> We once had a longer discussion with the PLL HW engineers to get this
>>> working because at least the display code we used as reference back
>>> then got it wrong.
>>
>> After this series is reviewed, I can look through the other
>> restrictions in the amdgpu_atombios_get_clock_info function and write
>> another patch series to add those restrictions to DC.
>>
> 
> The PPLL limits from radeon and the non-DC code were based on the PLL
> registers, i.e., what the hardware limit in the register was.  I'm not
> sure what the practical limits really are per se.  I.e., if the whole
> range is really usable or not.  I would tend to defer to DC.
> 
> Alex

Hi Alex,

I'll leave it up to your judgement if you want to include this patch.
I just wrote this for the sake of consistency with the non-DC code but 
the patch actually doesn't fix any actual bugs for me.

It is is based on information I found in the non-DC code, in the 
amdgpu_atombios_get_clock_info function. Maybe the PLL limits in this 
function were meant for even older GPUs and are not relevant to SI/CIK?

Best regards,
Timur


> 
>>>
>>>> ---
>>>>   drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c | 7 +++++++
>>>>   1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>>>> b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>>>> index b4f5b4a6331a..00f25e2ee081 100644
>>>> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>>>> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>>>> @@ -1700,6 +1700,13 @@ bool dce110_clk_src_construct(
>>>>                      clk_src->cs_mask->PLL_POST_DIV_PIXCLK;
>>>>      calc_pll_cs_init_data.min_pll_ref_divider =     1;
>>>>      calc_pll_cs_init_data.max_pll_ref_divider =     clk_src-
>>>>> cs_mask->PLL_REF_DIV;
>>>> +
>>>> +   if (ctx->dce_version <= DCE_VERSION_10_0) {
>>>> +           /* Set stricter dividers to stabilize the PLL's
>>>> feedback loop on old HW. */
>>>> +           calc_pll_cs_init_data.min_pix_clk_pll_post_divider
>>>> = 2;
>>>> +           calc_pll_cs_init_data.min_pll_ref_divider = 2;
>>>> +   }
>>>> +
>>>>      /* when 0 use minInputPxlClkPLLFrequencyInKHz from
>>>> firmwareInfo*/
>>>>      calc_pll_cs_init_data.min_override_input_pxl_clk_pll_freq_
>>>> khz =       0;
>>>>      /* when 0 use maxInputPxlClkPLLFrequencyInKHz from
>>>> firmwareInfo*/