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

Christian König christian.koenig at amd.com
Mon Sep 5 11:12:52 UTC 2022


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 at amd.comXinhui.Pan@amd.comairlied at linux.iedaniel@ffwll.chisabbasso at 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 at 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 */
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20220905/99b1f2d8/attachment-0001.htm>


More information about the amd-gfx mailing list