* Re: 回复: Re: [PATCH] drm:Fix the blank screen problem of some 1920x1080 75Hz monitors using R520 graphics card
2022-09-05 8:10 回复: Re: [PATCH] drm:Fix the blank screen problem of some 1920x1080 75Hz monitors using R520 graphics card 钟沛
@ 2022-09-05 11:12 ` Christian König
0 siblings, 0 replies; 2+ messages in thread
From: Christian König @ 2022-09-05 11:12 UTC (permalink / raw)
To: 钟沛, alexander.deucher, Xinhui.Pan, airlied, daniel,
isabbasso
Cc: dri-devel, amd-gfx, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 8275 bytes --]
Am 05.09.22 um 10:10 schrieb 钟沛:
>
> 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?
>
Nope, exactly that's just utterly nonsense.
What we could maybe do is to prefer a smaller ref_div over a larger
ref_div, but I don't see how this will help you.
Regards,
Christian.
>
> 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@amd.comXinhui.Pan@amd.comairlied@linux.iedaniel@ffwll.chisabbasso@riseup.net
>
>
>
> 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@kylinos.cn>
> > ---
> > 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 */
>
[-- Attachment #2: Type: text/html, Size: 13120 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread