<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Acked-by: Alex Deucher <alexander.deucher@amd.com> </div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Koenig, Christian <Christian.Koenig@amd.com> Sent: Wednesday, November 4, 2020 6:54 AM To: Sharma, Shashank <Shashank.Sharma@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Qin, Eddy <Eddy.Qin@amd.com> Subject: Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">Am 04.11.20 um 11:40 schrieb Sharma, Shashank: > [AMD Public Use] > > Hello Christian, > Yes, that 100 is hardcoded in Radeon, and git blame says it was one of your patches which made it 100 from 128 😊. > Would you mind having a look at commit id: 4b21ce1b4b5d262e7d4656b8ececc891fc3cb806 ? Ah, yes that one :) Yeah the background is that this was just an educated guess because I couldn't find anybody which could tell me what the real limits of the PLL is. Looks like we just forgot to apply that patch to amdgpu. Regards, Christian. > > Regards > Shashank > -----Original Message----- > From: Koenig, Christian <Christian.Koenig@amd.com> > Sent: Wednesday, November 4, 2020 3:35 PM > To: Sharma, Shashank <Shashank.Sharma@amd.com>; amd-gfx@lists.freedesktop.org > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Qin, Eddy <Eddy.Qin@amd.com> > Subject: Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 > > Am 03.11.20 um 18:13 schrieb Shashank Sharma: >> This patch limits the ref_div_max value to 100, during the calculation >> of PLL feedback reference divider. With current value (128), the >> produced fb_ref_div value generates unstable output at particular >> frequencies. Radeon driver limits this value at 100. > Mhm, is that 100 hard coded in radeon? I have no idea where that is coming from. > > Best would probably to grab a hardware engineer and try to figure out what the real maximums for the PLL is to still produce a stable signal. > > Christian. > >> On Oland, when we try to setup mode 2048x1280@60 (a bit weird, I >> know), it demands a clock of 221270 Khz. It's been observed that the >> PLL calculations using values 128 and 100 are vastly different, and >> look like this: >> >> +------------------------------------------+ >> |Parameter |AMDGPU |Radeon | >> | | | | >> +-------------+----------------------------+ >> |Clock feedback | | >> |divider max | 128 | 100 | >> |cap value | | | >> | | | | >> | | | | >> +------------------------------------------+ >> |ref_div_max | | | >> | | 42 | 20 | >> | | | | >> | | | | >> +------------------------------------------+ >> |ref_div | 42 | 20 | >> | | | | >> +------------------------------------------+ >> |fb_div | 10326 | 8195 | >> +------------------------------------------+ >> |fb_div | 1024 | 163 | >> +------------------------------------------+ >> |fb_dev_p | 4 | 9 | >> |frac fb_de^_p| | | >> +----------------------------+-------------+ >> >> With ref_div_max value clipped at 100, AMDGPU driver can also drive >> videmode 2048x1280@60 (221Mhz) and produce proper output without any >> blanking and distortion on the screen. >> >> PS: This value was changed from 128 to 100 in Radeon driver also, here: >> <a href="https://github.com/freedesktop/drm-tip/commit/4b21ce1b4b5d262e7d4656b8"> https://github.com/freedesktop/drm-tip/commit/4b21ce1b4b5d262e7d4656b8</a> >> ececc891fc3cb806 >> >> Cc: Alex Deucher <Alexander.Deucher@amd.com> >> Cc: Christian König <christian.koenig@amd.com> >> Cc: Eddy Qin <Eddy.Qin@amd.com> >> >> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c >> index 1f2305b7bd13..23a2e1ebf78a 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c >> @@ -85,7 +85,7 @@ static void amdgpu_pll_get_fb_ref_div(unsigned nom, unsigned den, unsigned post_ >> unsigned *fb_div, unsigned *ref_div) >> { >> /* limit reference * post divider to a maximum */ >> - ref_div_max = min(128 / post_div, ref_div_max); >> + ref_div_max = min(100 / post_div, ref_div_max); >> >> /* get matching reference and feedback divider */ >> *ref_div = min(max(DIV_ROUND_CLOSEST(den, post_div), 1u), >> ref_div_max); </div> </div> </body> </html>