RE: [PATCH] drm/amdgpu: clip the ref divider max value at 100

["Sharma, Shashank" <Shashank.Sharma@xxxxxxx>]

[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 <Alexander.Deucher@xxxxxxx>
Sent: Wednesday, November 4, 2020 8:03 PM
To: Koenig, Christian <Christian.Koenig@xxxxxxx>; Sharma, Shashank <Shashank.Sharma@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Qin, Eddy <Eddy.Qin@xxxxxxx>
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 <Christian.Koenig@xxxxxxx>
Sent: Wednesday, November 4, 2020 9:31 AM
To: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Sharma, Shashank <Shashank.Sharma@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Cc: Qin, Eddy <Eddy.Qin@xxxxxxx>
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 <Shashank.Sharma@xxxxxxx>
Sent: Wednesday, November 4, 2020 9:11 AM
To: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, Christian <Christian.Koenig@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Cc: Qin, Eddy <Eddy.Qin@xxxxxxx>
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 <alexander.deucher@xxxxxxx>

---

From: Koenig, Christian <Christian.Koenig@xxxxxxx>
Sent: Wednesday, November 4, 2020 6:54 AM
To: Sharma, Shashank <Shashank.Sharma@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Qin, Eddy <Eddy.Qin@xxxxxxx>
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 <Christian.Koenig@xxxxxxx>
> Sent: Wednesday, November 4, 2020 3:35 PM
> To: Sharma, Shashank <Shashank.Sharma@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Qin, Eddy <Eddy.Qin@xxxxxxx>
> 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 <Alexander.Deucher@xxxxxxx>
>> Cc: Christian König <christian.koenig@xxxxxxx>
>> Cc: Eddy Qin <Eddy.Qin@xxxxxxx>
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxx>
>> ---
>>    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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx