Thanks for your reply!
We found that in the amdgpu_pll_compute function, when the target_clock is the value contained in the drm_dmt_modes defined in drm_edid.c, the diff is 0. When target_clock is some special value, we cannot find a diff value of 0, so we need to find the smallest diff value to fit the current target_clock. For the monitor that has the blank screen problem here, we found that when the ref_div_max is 128, the diff value is smaller and the blank screen problem can be solved. We tested some other monitors and added log printing to the code. We found that this change did not affect those monitors, and in the analysis of the logs, we found that the solution with a smaller diff value always displayed normally.
Changing the value of ref_div_max from 128 to 100 can solve the blank screen problem of some monitors, but it will also cause some normal monitors to go black, so is it a more reasonable solution to determine the value of ref_div_max according to the value of diff?
Thank you for taking the time to read my email.
Best Regards.
----
主 题：Re: [PATCH] drm:Fix the blank screen problem of some 1920x1080 75Hz monitors using R520 graphics card
日 期：2022-09-05 14:05
发件人：Christian König
收件人：钟沛alexander.deucher@xxxxxxxxxxxxxxxxx@amd.comairlied@linux.iedaniel@ffwll.chisabbasso@xxxxxxxxxx
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 */
