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 <[email protected]> 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*/
>
