Re: [PATCH] drm:Fix the blank screen problem of some 1920x1080 75Hz monitors using R520 graphics card

[Christian König <christian.koenig@xxxxxxx>]

Am 05.09.22 um 05:23 schrieb zhongpei:
> We found that in the scenario of AMD R520 graphics card
> and some 1920x1080 monitors,when we switch the refresh rate
> of the monitor to 75Hz,the monitor will have a blank screen problem,
> and the restart cannot be restored.After testing, it is found that
> when we limit the maximum value of ref_div_max to 128,
> the problem can be solved.In order to keep the previous modification
> to be compatible with other monitors,we added a judgment
> when finding the minimum diff value in the loop of the
> amdgpu_pll_compute/radeon_compute_pll_avivo function.
> If no diff value of 0 is found when the maximum value of ref_div_max
> is limited to 100,continue to search when it is 128,
> and take the parameter with the smallest diff value.

Well that's at least better than what I've seen in previous tries to fix 
this.

But as far as I can see this will certainly break some other monitors, 
so that is pretty much a NAK.

Regards,
Christian.

> Signed-off-by: zhongpei <zhongpei@xxxxxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c | 17 +++++++++++++----
>   drivers/gpu/drm/radeon/radeon_display.c | 15 +++++++++++----
>   2 files changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c
> index 0bb2466d539a..0c298faa0f94 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c
> @@ -84,12 +84,13 @@ static void amdgpu_pll_reduce_ratio(unsigned *nom, unsigned *den,
>   static void amdgpu_pll_get_fb_ref_div(struct amdgpu_device *adev, unsigned int nom,
>   				      unsigned int den, unsigned int post_div,
>   				      unsigned int fb_div_max, unsigned int ref_div_max,
> -				      unsigned int *fb_div, unsigned int *ref_div)
> +				      unsigned int ref_div_limit, unsigned int *fb_div,
> +				      unsigned int *ref_div)
>   {
>   
>   	/* limit reference * post divider to a maximum */
>   	if (adev->family == AMDGPU_FAMILY_SI)
> -		ref_div_max = min(100 / post_div, ref_div_max);
> +		ref_div_max = min(ref_div_limit / post_div, ref_div_max);
>   	else
>   		ref_div_max = min(128 / post_div, ref_div_max);
>   
> @@ -136,6 +137,7 @@ void amdgpu_pll_compute(struct amdgpu_device *adev,
>   	unsigned ref_div_min, ref_div_max, ref_div;
>   	unsigned post_div_best, diff_best;
>   	unsigned nom, den;
> +	unsigned ref_div_limit, ref_limit_best;
>   
>   	/* determine allowed feedback divider range */
>   	fb_div_min = pll->min_feedback_div;
> @@ -204,11 +206,12 @@ void amdgpu_pll_compute(struct amdgpu_device *adev,
>   	else
>   		post_div_best = post_div_max;
>   	diff_best = ~0;
> +	ref_div_limit = ref_limit_best = 100;
>   
>   	for (post_div = post_div_min; post_div <= post_div_max; ++post_div) {
>   		unsigned diff;
>   		amdgpu_pll_get_fb_ref_div(adev, nom, den, post_div, fb_div_max,
> -					  ref_div_max, &fb_div, &ref_div);
> +					  ref_div_max, ref_div_limit, &fb_div, &ref_div);
>   		diff = abs(target_clock - (pll->reference_freq * fb_div) /
>   			(ref_div * post_div));
>   
> @@ -217,13 +220,19 @@ void amdgpu_pll_compute(struct amdgpu_device *adev,
>   
>   			post_div_best = post_div;
>   			diff_best = diff;
> +			ref_limit_best = ref_div_limit;
>   		}
> +		if (post_div >= post_div_max && diff_best != 0 && ref_div_limit != 128) {
> +			ref_div_limit = 128;
> +			post_div = post_div_min - 1;
> +		}
> +
>   	}
>   	post_div = post_div_best;
>   
>   	/* get the feedback and reference divider for the optimal value */
>   	amdgpu_pll_get_fb_ref_div(adev, nom, den, post_div, fb_div_max, ref_div_max,
> -				  &fb_div, &ref_div);
> +				  ref_limit_best, &fb_div, &ref_div);
>   
>   	/* reduce the numbers to a simpler ratio once more */
>   	/* this also makes sure that the reference divider is large enough */
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
> index f12675e3d261..0fcbf45a68db 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -925,10 +925,10 @@ static void avivo_reduce_ratio(unsigned *nom, unsigned *den,
>    */
>   static void avivo_get_fb_ref_div(unsigned nom, unsigned den, unsigned post_div,
>   				 unsigned fb_div_max, unsigned ref_div_max,
> -				 unsigned *fb_div, unsigned *ref_div)
> +				 unsigned ref_div_limit, unsigned *fb_div, unsigned *ref_div)
>   {
>   	/* limit reference * post divider to a maximum */
> -	ref_div_max = max(min(100 / post_div, ref_div_max), 1u);
> +	ref_div_max = max(min(ref_div_limit / post_div, ref_div_max), 1u);
>   
>   	/* get matching reference and feedback divider */
>   	*ref_div = min(max(den/post_div, 1u), ref_div_max);
> @@ -971,6 +971,7 @@ void radeon_compute_pll_avivo(struct radeon_pll *pll,
>   	unsigned ref_div_min, ref_div_max, ref_div;
>   	unsigned post_div_best, diff_best;
>   	unsigned nom, den;
> +	unsigned ref_div_limit, ref_limit_best;
>   
>   	/* determine allowed feedback divider range */
>   	fb_div_min = pll->min_feedback_div;
> @@ -1042,11 +1043,12 @@ void radeon_compute_pll_avivo(struct radeon_pll *pll,
>   	else
>   		post_div_best = post_div_max;
>   	diff_best = ~0;
> +	ref_div_limit = ref_limit_best = 100;
>   
>   	for (post_div = post_div_min; post_div <= post_div_max; ++post_div) {
>   		unsigned diff;
>   		avivo_get_fb_ref_div(nom, den, post_div, fb_div_max,
> -				     ref_div_max, &fb_div, &ref_div);
> +				     ref_div_max, ref_div_limit, &fb_div, &ref_div);
>   		diff = abs(target_clock - (pll->reference_freq * fb_div) /
>   			(ref_div * post_div));
>   
> @@ -1055,13 +1057,18 @@ void radeon_compute_pll_avivo(struct radeon_pll *pll,
>   
>   			post_div_best = post_div;
>   			diff_best = diff;
> +			ref_limit_best = ref_div_limit;
> +		}
> +		if (post_div >= post_div_max && diff_best != 0 && ref_div_limit != 128) {
> +			ref_div_limit = 128;
> +			post_div = post_div_min - 1;
>   		}
>   	}
>   	post_div = post_div_best;
>   
>   	/* get the feedback and reference divider for the optimal value */
>   	avivo_get_fb_ref_div(nom, den, post_div, fb_div_max, ref_div_max,
> -			     &fb_div, &ref_div);
> +			     ref_limit_best, &fb_div, &ref_div);
>   
>   	/* reduce the numbers to a simpler ratio once more */
>   	/* this also makes sure that the reference divider is large enough */