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

["Christian König" <christian.koenig@amd.com>]

On 13.10.25 08:15, Timur Kristóf wrote:
> 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?

The DC and even the atombios tables got the clock divider ranges quite wrong sometimes. For example as far as I know the 1/10th handling for the feedback divider minimum was never correct in those.

But I'm not sure where the ref and post divider minimums came from of hand.

Regards,
Christian.

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