RE: [PATCH] drm/amdgpu: clip the ref divider max value at 100
[AMD Public Use] Thanks Alex, Christian, Let me quickly run this patch through someone in DAL team, and see how it looks. Regards Shashank From: Deucher, Alexander Sent: Wednesday, November 4, 2020 8:03 PM To: Koenig, Christian ; Sharma, Shashank ; amd-gfx@lists.freedesktop.org Cc: Qin, Eddy Subject: Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 yeah, ideally. Just need to get support for analog encoders. Alex From: Koenig, Christian mailto:christian.koe...@amd.com>> Sent: Wednesday, November 4, 2020 9:31 AM To: Deucher, Alexander mailto:alexander.deuc...@amd.com>>; Sharma, Shashank mailto:shashank.sha...@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> mailto:amd-gfx@lists.freedesktop.org>> Cc: Qin, Eddy mailto:eddy@amd.com>> Subject: Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 In the long term we probably want to nuke this code anyway and switch to the DC code, don't we? Christian. Am 04.11.20 um 15:23 schrieb Deucher, Alexander: 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. Alex From: Sharma, Shashank <mailto:shashank.sha...@amd.com> Sent: Wednesday, November 4, 2020 9:11 AM To: Deucher, Alexander <mailto:alexander.deuc...@amd.com>; Koenig, Christian <mailto:christian.koe...@amd.com>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <mailto:amd-gfx@lists.freedesktop.org> Cc: Qin, Eddy <mailto:eddy....@amd.com> Subject: Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 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 On 04/11/20 7:36 pm, Deucher, Alexander wrote: Acked-by: Alex Deucher <mailto:alexander.deuc...@amd.com> From: Koenig, Christian <mailto:christian.koe...@amd.com> Sent: Wednesday, November 4, 2020 6:54 AM To: Sharma, Shashank <mailto:shashank.sha...@amd.com>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <mailto:amd-gfx@lists.freedesktop.org> Cc: Deucher, Alexander <mailto:alexander.deuc...@amd.com>; Qin, Eddy <mailto:eddy@amd.com> Subject: Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 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 > <mailto:christian.koe...@amd.com> > Sent: Wednesday, November 4, 2020 3:35 PM > To: Sharma, Shashank > <mailto:shashank.sha...@amd.com>; > amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> > Cc: Deucher, Alexander > <mailto:alexander.deuc...@amd.com>; Qin, Eddy > <mailto:eddy@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
Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100
Then feel free to stick an Acked-by: Christian König to the code and get it committed. Ideally somebody should have it on the TODO list to get rid of that code. Christian. Am 04.11.20 um 15:33 schrieb Deucher, Alexander: yeah, ideally. Just need to get support for analog encoders. Alex *From:* Koenig, Christian *Sent:* Wednesday, November 4, 2020 9:31 AM *To:* Deucher, Alexander ; Sharma, Shashank ; amd-gfx@lists.freedesktop.org *Cc:* Qin, Eddy *Subject:* Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 In the long term we probably want to nuke this code anyway and switch to the DC code, don't we? Christian. Am 04.11.20 um 15:23 schrieb Deucher, Alexander: 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. Alex *From:* Sharma, Shashank <mailto:shashank.sha...@amd.com> *Sent:* Wednesday, November 4, 2020 9:11 AM *To:* Deucher, Alexander <mailto:alexander.deuc...@amd.com>; Koenig, Christian <mailto:christian.koe...@amd.com>; amd-gfx@lists.freedesktop.org <mailto:amd-gfx@lists.freedesktop.org> <mailto:amd-gfx@lists.freedesktop.org> *Cc:* Qin, Eddy <mailto:eddy@amd.com> *Subject:* Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 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 On 04/11/20 7:36 pm, Deucher, Alexander wrote: Acked-by: Alex Deucher <mailto:alexander.deuc...@amd.com> *From:* Koenig, Christian <mailto:christian.koe...@amd.com> *Sent:* Wednesday, November 4, 2020 6:54 AM *To:* Sharma, Shashank <mailto:shashank.sha...@amd.com>; amd-gfx@lists.freedesktop.org <mailto:amd-gfx@lists.freedesktop.org> <mailto:amd-gfx@lists.freedesktop.org> *Cc:* Deucher, Alexander <mailto:alexander.deuc...@amd.com>; Qin, Eddy <mailto:eddy....@amd.com> *Subject:* Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 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 <mailto:christian.koe...@amd.com> > Sent: Wednesday, November 4, 2020 3:35 PM > To: Sharma, Shashank <mailto:shashank.sha...@amd.com>; amd-gfx@lists.freedesktop.org <mailto:amd-gfx@lists.freedesktop.org> > Cc: Deucher, Alexander <mailto:alexander.deuc...@amd.com>; Qin, Eddy <mailto:eddy@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 | | | >> |Â
Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100
yeah, ideally. Just need to get support for analog encoders. Alex From: Koenig, Christian Sent: Wednesday, November 4, 2020 9:31 AM To: Deucher, Alexander ; Sharma, Shashank ; amd-gfx@lists.freedesktop.org Cc: Qin, Eddy Subject: Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 In the long term we probably want to nuke this code anyway and switch to the DC code, don't we? Christian. Am 04.11.20 um 15:23 schrieb Deucher, Alexander: 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. Alex From: Sharma, Shashank <mailto:shashank.sha...@amd.com> Sent: Wednesday, November 4, 2020 9:11 AM To: Deucher, Alexander <mailto:alexander.deuc...@amd.com>; Koenig, Christian <mailto:christian.koe...@amd.com>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <mailto:amd-gfx@lists.freedesktop.org> Cc: Qin, Eddy <mailto:eddy....@amd.com> Subject: Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 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 On 04/11/20 7:36 pm, Deucher, Alexander wrote: Acked-by: Alex Deucher <mailto:alexander.deuc...@amd.com> From: Koenig, Christian <mailto:christian.koe...@amd.com> Sent: Wednesday, November 4, 2020 6:54 AM To: Sharma, Shashank <mailto:shashank.sha...@amd.com>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <mailto:amd-gfx@lists.freedesktop.org> Cc: Deucher, Alexander <mailto:alexander.deuc...@amd.com>; Qin, Eddy <mailto:eddy@amd.com> Subject: Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 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 > <mailto:christian.koe...@amd.com> > Sent: Wednesday, November 4, 2020 3:35 PM > To: Sharma, Shashank > <mailto:shashank.sha...@amd.com>; > amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> > Cc: Deucher, Alexander > <mailto:alexander.deuc...@amd.com>; Qin, Eddy > <mailto:eddy@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
Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100
In the long term we probably want to nuke this code anyway and switch to the DC code, don't we? Christian. Am 04.11.20 um 15:23 schrieb Deucher, Alexander: 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. Alex *From:* Sharma, Shashank *Sent:* Wednesday, November 4, 2020 9:11 AM *To:* Deucher, Alexander ; Koenig, Christian ; amd-gfx@lists.freedesktop.org *Cc:* Qin, Eddy *Subject:* Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 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 On 04/11/20 7:36 pm, Deucher, Alexander wrote: Acked-by: Alex Deucher <mailto:alexander.deuc...@amd.com> *From:* Koenig, Christian <mailto:christian.koe...@amd.com> *Sent:* Wednesday, November 4, 2020 6:54 AM *To:* Sharma, Shashank <mailto:shashank.sha...@amd.com>; amd-gfx@lists.freedesktop.org <mailto:amd-gfx@lists.freedesktop.org> <mailto:amd-gfx@lists.freedesktop.org> *Cc:* Deucher, Alexander <mailto:alexander.deuc...@amd.com>; Qin, Eddy <mailto:eddy....@amd.com> *Subject:* Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 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 <mailto:christian.koe...@amd.com> > Sent: Wednesday, November 4, 2020 3:35 PM > To: Sharma, Shashank <mailto:shashank.sha...@amd.com>; amd-gfx@lists.freedesktop.org <mailto:amd-gfx@lists.freedesktop.org> > Cc: Deucher, Alexander <mailto:alexander.deuc...@amd.com>; Qin, Eddy <mailto:eddy@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 d
Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100
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. Alex From: Sharma, Shashank Sent: Wednesday, November 4, 2020 9:11 AM To: Deucher, Alexander ; Koenig, Christian ; amd-gfx@lists.freedesktop.org Cc: Qin, Eddy Subject: Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 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 On 04/11/20 7:36 pm, Deucher, Alexander wrote: Acked-by: Alex Deucher <mailto:alexander.deuc...@amd.com> From: Koenig, Christian <mailto:christian.koe...@amd.com> Sent: Wednesday, November 4, 2020 6:54 AM To: Sharma, Shashank <mailto:shashank.sha...@amd.com>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <mailto:amd-gfx@lists.freedesktop.org> Cc: Deucher, Alexander <mailto:alexander.deuc...@amd.com>; Qin, Eddy <mailto:eddy....@amd.com> Subject: Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 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 > <mailto:christian.koe...@amd.com> > Sent: Wednesday, November 4, 2020 3:35 PM > To: Sharma, Shashank > <mailto:shashank.sha...@amd.com>; > amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> > Cc: Deucher, Alexander > <mailto:alexander.deuc...@amd.com>; Qin, Eddy > <mailto:eddy@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: >&g
Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100
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 On 04/11/20 7:36 pm, Deucher, Alexander wrote: > Acked-by: Alex Deucher > -- > *From:* Koenig, Christian > *Sent:* Wednesday, November 4, 2020 6:54 AM > *To:* Sharma, Shashank ; > amd-gfx@lists.freedesktop.org > *Cc:* Deucher, Alexander ; Qin, Eddy > > *Subject:* Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100 >  > 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 > > Sent: Wednesday, November 4, 2020 3:35 PM > > To: Sharma, Shashank ; > > amd-gfx@lists.freedesktop.org > > Cc: Deucher, Alexander ; Qin, Eddy > > > > 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|Â
Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100
On 04/11/20 5:24 pm, Christian König wrote:
> 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.
Cool, I was wondering if we should let this patch go through some elaborated
Pixel clock testing on different resolution, or is it ok to go ahead with this
patch based on the experience from Radeon driver change ?
Regards
Shashank
>> Regards
>> Shashank
>> -Original Message-
>> From: Koenig, Christian
>> Sent: Wednesday, November 4, 2020 3:35 PM
>> To: Sharma, Shashank ; amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander ; Qin, Eddy
>>
>> 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:
>>> https://github.com/freedesktop/drm-tip/commit/4b21ce1b4b5d262e7d4656b8
>>> ececc891fc3cb806
>>>
>>> Cc: Alex Deucher
>>> Cc: Christian König
>>> Cc: Eddy Qin
>>>
>>> Signed-off-by: Shashank Sharma
>>> ---
>>>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);
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100
Acked-by: Alex Deucher
From: Koenig, Christian
Sent: Wednesday, November 4, 2020 6:54 AM
To: Sharma, Shashank ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Qin, Eddy
Subject: Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100
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
> Sent: Wednesday, November 4, 2020 3:35 PM
> To: Sharma, Shashank ; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Qin, Eddy
>
> 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:
>> https://github.com/freedesktop/drm-tip/commit/4b21ce1b4b5d262e7d4656b8
>> ececc891fc3cb806
>>
>> Cc: Alex Deucher
>> Cc: Christian König
>> Cc: Eddy Qin
>>
>> Signed-off-by: Shashank Sharma
>> ---
>>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);
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: clip the ref divider max value at 100
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
Sent: Wednesday, November 4, 2020 3:35 PM
To: Sharma, Shashank ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Qin, Eddy
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:
https://github.com/freedesktop/drm-tip/commit/4b21ce1b4b5d262e7d4656b8
ececc891fc3cb806
Cc: Alex Deucher
Cc: Christian König
Cc: Eddy Qin
Signed-off-by: Shashank Sharma
---
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);
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: clip the ref divider max value at 100
[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 ?
Regards
Shashank
-Original Message-
From: Koenig, Christian
Sent: Wednesday, November 4, 2020 3:35 PM
To: Sharma, Shashank ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Qin, Eddy
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:
> https://github.com/freedesktop/drm-tip/commit/4b21ce1b4b5d262e7d4656b8
> ececc891fc3cb806
>
> Cc: Alex Deucher
> Cc: Christian König
> Cc: Eddy Qin
>
> Signed-off-by: Shashank Sharma
> ---
> 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);
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
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:
https://github.com/freedesktop/drm-tip/commit/4b21ce1b4b5d262e7d4656b8ececc891fc3cb806
Cc: Alex Deucher
Cc: Christian König
Cc: Eddy Qin
Signed-off-by: Shashank Sharma
---
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);
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
