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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux