<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);"> You might want to talk to the DAL team, they may have some advice. In general, I would say test it as well as you can. It's probably safe as radeon is still the default driver for SI parts and generally seems to be working well there.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Alex</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> </div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Sharma, Shashank <Shashank.Sharma@amd.com> Sent: Wednesday, November 4, 2020 9:11 AM To: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org> Cc: Qin, Eddy <Eddy.Qin@amd.com> Subject: Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 <div> </div> </div> <div> Thanks Alex, Same question here, Should we go through some extensive test routine due to change in PLL values, or is it OK to go ahead based on our experience from Radeon values ? Regards Shashank <div class="x_moz-cite-prefix">On 04/11/20 7:36 pm, Deucher, Alexander wrote: </div> <blockquote type="cite"><style type="text/css" style="display:none">  </style> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)"> Acked-by: Alex Deucher <a class="x_moz-txt-link-rfc2396E" href="mailto:alexander.deucher@amd.com"> <alexander.deucher@amd.com></a> </div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_divRplyFwdMsg" dir="ltr">From: Koenig, Christian <a class="x_moz-txt-link-rfc2396E" href="mailto:Christian.Koenig@amd.com"><Christian.Koenig@amd.com></a> Sent: Wednesday, November 4, 2020 6:54 AM To: Sharma, Shashank <a class="x_moz-txt-link-rfc2396E" href="mailto:Shashank.Sharma@amd.com"> <Shashank.Sharma@amd.com></a>; <a class="x_moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org"> amd-gfx@lists.freedesktop.org</a> <a class="x_moz-txt-link-rfc2396E" href="mailto:amd-gfx@lists.freedesktop.org"> <amd-gfx@lists.freedesktop.org></a> Cc: Deucher, Alexander <a class="x_moz-txt-link-rfc2396E" href="mailto:Alexander.Deucher@amd.com"> <Alexander.Deucher@amd.com></a>; Qin, Eddy <a class="x_moz-txt-link-rfc2396E" href="mailto:Eddy.Qin@amd.com"> <Eddy.Qin@amd.com></a> Subject: Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 <div> </div> </div> <div class="x_BodyFragment"> <div class="x_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 <a class="x_moz-txt-link-rfc2396E" href="mailto:Christian.Koenig@amd.com"> <Christian.Koenig@amd.com></a> > Sent: Wednesday, November 4, 2020 3:35 PM > To: Sharma, Shashank <a class="x_moz-txt-link-rfc2396E" href="mailto:Shashank.Sharma@amd.com"> <Shashank.Sharma@amd.com></a>; <a class="x_moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org"> amd-gfx@lists.freedesktop.org</a> > Cc: Deucher, Alexander <a class="x_moz-txt-link-rfc2396E" href="mailto:Alexander.Deucher@amd.com"> <Alexander.Deucher@amd.com></a>; Qin, Eddy <a class="x_moz-txt-link-rfc2396E" href="mailto:Eddy.Qin@amd.com"> <Eddy.Qin@amd.com></a> > 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 <a class="x_moz-txt-link-rfc2396E" href="mailto:Alexander.Deucher@amd.com"> <Alexander.Deucher@amd.com></a> >> Cc: Christian König <a class="x_moz-txt-link-rfc2396E" href="mailto:christian.koenig@amd.com"> <christian.koenig@amd.com></a> >> Cc: Eddy Qin <a class="x_moz-txt-link-rfc2396E" href="mailto:Eddy.Qin@amd.com"> <Eddy.Qin@amd.com></a> >> >> Signed-off-by: Shashank Sharma <a class="x_moz-txt-link-rfc2396E" href="mailto:shashank.sharma@amd.com"> <shashank.sharma@amd.com></a> >> --- >> 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> </blockquote> </div> </body> </html>