<p>Sorry to <span style="font-family: Arial, sans-serif; background-color: rgb(255, 255, 255); font-size: 16px; color: rgb(0, 0, 0);">trouble</span> you, we ran some tests on this patch and <span style="color: rgb(51, 51, 51); font-family: Arial, sans-serif; background-color: rgb(255, 255, 255); font-size: 16px;">want to communicate with you</span>.</p><p><br></p><p><strong><span style="color: rgb(0, 0, 0);">static void amdgpu_pll_get_fb_ref_div(struct amdgpu_device *adev, unsigned nom, unsigned den, unsigned post_div,</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"><span style="color: rgb(86, 156, 214);"> <span style="color: rgb(0, 0, 0);">unsigned</span></span> fb_div_max, unsigned <span style="background-color: rgb(255, 255, 0); color: rgb(0, 0, 0);">ref_div_max</span>,</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"><span style="color: rgb(86, 156, 214);"> <span style="color: rgb(0, 0, 0);">unsigned</span></span> *fb_div, unsigned *ref_div)</span></strong></p><p><strong>{</strong></p><p><br></p><p><strong><span style="color: #6a9955;"> /* limit reference * post divider to a maximum */</span></strong></p><p><strong><span style="color: #c586c0;"> </span><span style="color: rgb(0, 0, 0);">if (adev->family == AMDGPU_FAMILY_SI)</span></strong></p><p><strong><span style="color: #9cdcfe;"> </span><span style="color: rgb(0, 0, 0);"><span style="background-color: rgb(255, 255, 0);">ref_div_max</span> = min(100 / post_div, ref_div_max);</span></strong></p><p><strong><span style="color: #c586c0;"> <span style="color: rgb(0, 0, 0);">else</span></span></strong></p><p><strong><span style="color: #9cdcfe;"> <span style="background-color: rgb(255, 255, 0); color: rgb(0, 0, 0);">ref_div_max</span></span> = <span style="color: rgb(0, 0, 0);">min(128 / post_div, ref_div_max);</span></strong></p><p><br></p><p><strong><span style="color: #6a9955;"> /* get matching reference and feedback divider */</span></strong></p><p><strong> *<span style="color: rgb(0, 0, 0);">ref_div = min(max(DIV_ROUND_CLOSEST(den, post_div), 1u), <span style="color: rgb(0, 0, 0); background-color: rgb(255, 255, 0);">ref_div_max</span>);</span></strong></p><p><strong> *<span style="color: rgb(0, 0, 0);">fb_div = DIV_ROUND_CLOSEST(nom * *ref_div * post_div, den);</span></strong></p><p><br></p><p><strong><span style="color: #6a9955;"> /* limit fb divider to its maximum */</span></strong></p><p><strong><span style="color: #c586c0;"> </span><span style="color: rgb(0, 0, 0);">if (*fb_div > fb_div_max) {</span></strong></p><p><strong> *<span style="color: rgb(0, 0, 0);">ref_div = DIV_ROUND_CLOSEST(*ref_div * fb_div_max, *fb_div);</span></strong></p><p><strong> *<span style="color: rgb(0, 0, 0);">fb_div = fb_div_max;</span></strong></p><p><strong> }</strong></p><p><strong>}</strong></p><p><br></p><p>For R520, the max value of the ref_div_max in this function used to be 128. In order to fix the black screen problem of some monitors, it was changed to 100. From the commit message at the time, we learned that this was a temporary solution. When the value of ref_div_max is 100, it will also cause problems with some monitors.
</p><p><br></p><p><strong><span style="color: rgb(0, 0, 0);">void amdgpu_pll_compute(struct amdgpu_device *adev, struct amdgpu_pll *pll, u32 freq, u32 *dot_clock_p,</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"> u32 *fb_div_p,</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"> u32 *frac_fb_div_p,</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"> u32 *ref_div_p,</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"> u32 *post_div_p)</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);">{</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"> ......<br></span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"> </span></strong></p><p><span style="color: rgb(146, 208, 80);"><strong> <span style="color: rgb(118, 146, 60);">/* now search for a post divider */</span></strong></span></p><p><strong><span style="color: rgb(0, 0, 0);"><span style="color: rgb(197, 134, 192);"> <span style="color: rgb(0, 0, 0);">if</span></span> (pll->flags & AMDGPU_PLL_PREFER_MINM_OVER_MAXP)</span></strong></p><p><span style="color: rgb(0, 0, 0);"><strong><span style="color: rgb(156, 220, 254);"> <span style="color: rgb(0, 0, 0);">post_div_best</span></span> = <span style="color: rgb(0, 0, 0);">post_div_min</span>;</strong></span></p><p><strong><span style="color: rgb(0, 0, 0);"> else</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"><span style="color: rgb(156, 220, 254);"> <span style="color: rgb(0, 0, 0);">post_div_best</span></span> = post_div_max;</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"><span style="color: rgb(156, 220, 254);"> <span style="color: rgb(0, 0, 0);">diff_best</span></span> = ~0;</span></strong></p><p><br></p><p><strong><span style="color: rgb(0, 0, 0);"><span style="color: rgb(197, 134, 192);"> <span style="color: rgb(0, 0, 0);">for</span></span> (post_div = post_div_min; post_div <= post_div_max; ++post_div) {</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"><span style="color: rgb(86, 156, 214);"> <span style="color: rgb(0, 0, 0);">unsigned</span></span> diff;</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"><span style="color: rgb(220, 220, 170);"> <span style="color: rgb(0, 0, 0);">amdgpu_pll_get_fb_ref_div</span></span>(adev, nom, den, post_div, fb_div_max,</span></strong></p><p><span style="color: rgb(0, 0, 0);"><strong><span style="color: rgb(156, 220, 254);"> <span style="color: rgb(0, 0, 0);">ref_div_max</span></span>, &<span style="color: rgb(0, 0, 0); background-color: rgb(255, 255, 0);">fb_div</span>, &<span style="color: rgb(0, 0, 0); background-color: rgb(255, 255, 0);">ref_div</span></strong></span><strong><span style="color: rgb(0, 0, 0);"><span style="color: rgb(156, 220, 254);"></span>);</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"><span style="color: rgb(156, 220, 254);"> </span><span style="color: rgb(0, 0, 0); background-color: rgb(255, 255, 0);"><span style="background-color: rgb(255, 255, 0); color: rgb(0, 0, 0);">diff</span> = abs(target_clock - (pll->reference_freq * fb_div) /</span></span></strong></p><p><span style="background-color: rgb(255, 255, 0);"><strong><span style="background-color: rgb(255, 255, 0); color: rgb(0, 0, 0);"> (ref_div * post_div));</span></strong></span></p><p><br></p><p><strong><span style="color: rgb(0, 0, 0);"><span style="color: rgb(197, 134, 192);"> <span style="color: rgb(0, 0, 0);">if</span></span> (diff < diff_best || (diff == diff_best &&</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"> !(pll->flags & AMDGPU_PLL_PREFER_MINM_OVER_MAXP))) {</span></strong></p><p><br></p><p><strong><span style="color: rgb(0, 0, 0);"><span style="color: rgb(156, 220, 254);"> <span style="color: rgb(0, 0, 0);">post_div_best</span></span> = post_div;</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"><span style="color: rgb(156, 220, 254);"> <span style="color: rgb(0, 0, 0);">diff_best</span></span> = diff;</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"> }</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"> }</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"><span style="color: rgb(156, 220, 254);"> <span style="color: rgb(0, 0, 0);">post_div</span></span> = post_div_best;</span></strong></p><p><strong><span style="color: rgb(0, 0, 0);"> ......<br></span></strong></p><p><strong><span style="color: rgb(0, 0, 0);">}</span></strong></p><p><br></p><p>This piece of code in the above function is to find a post_div, so that the value of <strong><span style="background-color: rgb(255, 255, 0);">diff = abs(target_clock - (pll->reference_freq * fb_div) /(ref_div * post_div))</span></strong> is the smallest. The pixel clock closest to the target frequency when the value of the <strong>diff</strong> is the smallest. The values of the reference divider(<strong>ref_div</strong>) and feedback divider(<strong>fb_div</strong>) in the above formula are affected by <strong>ref_div_max</strong>, so when the value of <strong>ref_div_max</strong> is different, the value of <strong>diff</strong> may also be different. The larger value of <strong>diff</strong> may cause the blank screen problem of some monitors.</p><p><br></p><p>A value of 100 or 128 for <strong>ref_div_max</strong> is not suitable for all monitors. So we set it to 100 and 128 in turn to calculate the corresponding ref_div and fb_div values, and then calculate <span style="background-color: rgb(255, 255, 0);"><strong>diff = abs(target_clock - (pll->reference_freq * fb_div) /(ref_div * post_div))</strong></span>. If the <strong>diff</strong> is smaller when the value of <strong>ref_div_max</strong> is 100, set the value of <strong>ref_div_max</strong> to 100. Otherwise, set the value of <strong style="white-space: normal;">ref_div_max</strong> to 128. In this way, the value of <strong>ref_div_max</strong> can be determined according to the value of <strong>diff</strong>, in order to adapt to all monitors. After our verification on different monitors, those monitors that have a blank screen problem when the value of <strong>ref_div_max</strong> is 100 or 128 can work normally under this scheme.</p><p><br></p><p>We believe that temporarily adopting this method to replace the previous method can improve the compatibility of the graphics card.</p><p><br></p><p><span style="color: rgb(18, 18, 18); font-family: -apple-system, BlinkMacSystemFont, "Helvetica Neue", "PingFang SC", "Microsoft YaHei", "Source Han Sans SC", "Noto Sans CJK SC", "WenQuanYi Micro Hei", sans-serif; font-size: medium; background-color: rgb(255, 255, 255);">Best Regards.</span><br></p><p>----</p><p><br></p><p><br></p><p><br></p><p><br><br><br>
</p><p><strong>主 题:</strong><span id="subject">Re: 回复: Re: [PATCH] drm:Fix the blank screen problem of some 1920x1080 75Hz monitors using R520 graphics card</span>
<br><strong>日 期:</strong><span id="date">2022-09-05 19:12</span>
<br><strong>发件人:</strong><span id="from">Christian König</span>
<br><strong>收件人:</strong><span id="to" style="">钟沛alexander.deucher@amd.comXinhui.Pan@amd.comairlied@linux.iedaniel@ffwll.chisabbasso@riseup.net</span>
</p><p><br>
</p><p>Am 05.09.22 um 10:10 schrieb 钟沛:<br>
</p><p>Thanks for your reply!</p><p><br>
</p><p>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.</p><p><br>
</p><p>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?</p><p><br>
Nope, exactly that's just utterly nonsense.<br>
<br>
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.<br>
<br>
Regards,<br>
Christian.<br>
<br>
<br>
Thank you for taking the time to read my email.</p><p><span class="qkunPe" style="color: rgb(234, 67, 53);
font-family: arial, sans-serif; font-size: 14px;
background-color: rgb(255, 255, 255);"></span></p><p><br>
</p><p><span style="color: rgb(18, 18, 18); font-family:
-apple-system, BlinkMacSystemFont, "Helvetica Neue",
"PingFang SC", "Microsoft YaHei",
"Source Han Sans SC", "Noto Sans CJK SC",
"WenQuanYi Micro Hei", sans-serif; font-size:
medium; background-color: rgb(255, 255, 255);">Best Regards.</span></p><p>----</p><p><br>
</p><p><br>
</p><p><br>
</p><p><br>
<br>
<br>
</p><p><strong>主 题:</strong><span id="subject">Re: [PATCH] drm:Fix the
blank screen problem of some 1920x1080 75Hz monitors using
R520 graphics card</span> <br>
<strong>日 期:</strong><span id="date">2022-09-05 14:05</span>
<br>
<strong>发件人:</strong><span id="from">Christian König</span>
<br>
<strong>收件人:</strong><span id="to">钟沛<a class="moz-txt-link-abbreviated" href="http://mailto:alexander.deucher@amd.comXinhui.Pan@amd.comairlied@linux.iedaniel@ffwll.chisabbasso@riseup.net" target="_blank">alexander.deucher@amd.comXinhui.Pan@amd.comairlied@linux.iedaniel@ffwll.chisabbasso@riseup.net</a></span>
</p><p><br>
</p><p>Am 05.09.22 um 05:23 schrieb zhongpei:<br>
> We found that in the scenario of AMD R520 graphics card<br>
> and some 1920x1080 monitors,when we switch the refresh rate<br>
> of the monitor to 75Hz,the monitor will have a blank screen
problem,<br>
> and the restart cannot be restored.After testing, it is
found that<br>
> when we limit the maximum value of ref_div_max to 128,<br>
> the problem can be solved.In order to keep the previous
modification<br>
> to be compatible with other monitors,we added a judgment<br>
> when finding the minimum diff value in the loop of the<br>
> amdgpu_pll_compute/radeon_compute_pll_avivo function.<br>
> If no diff value of 0 is found when the maximum value of
ref_div_max<br>
> is limited to 100,continue to search when it is 128,<br>
> and take the parameter with the smallest diff value.<br>
<br>
Well that's at least better than what I've seen in previous
tries to fix <br>
this.<br>
<br>
But as far as I can see this will certainly break some other
monitors, <br>
so that is pretty much a NAK.<br>
<br>
Regards,<br>
Christian.<br>
<br>
><br>
> Signed-off-by: zhongpei <a class="moz-txt-link-rfc2396E" href="http://mailto:zhongpei@kylinos.cn" target="_blank"><zhongpei@kylinos.cn></a><br>
> ---<br>
> drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c | 17
+++++++++++++----<br>
> drivers/gpu/drm/radeon/radeon_display.c | 15
+++++++++++----<br>
> 2 files changed, 24 insertions(+), 8 deletions(-)<br>
><br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c<br>
> index 0bb2466d539a..0c298faa0f94 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c<br>
> @@ -84,12 +84,13 @@ static void
amdgpu_pll_reduce_ratio(unsigned *nom, unsigned *den,<br>
> static void amdgpu_pll_get_fb_ref_div(struct
amdgpu_device *adev, unsigned int nom,<br>
> unsigned int den, unsigned int post_div,<br>
> unsigned int fb_div_max, unsigned int ref_div_max,<br>
> - unsigned int *fb_div, unsigned int *ref_div)<br>
> + unsigned int ref_div_limit, unsigned int *fb_div,<br>
> + unsigned int *ref_div)<br>
> {<br>
> <br>
> /* limit reference * post divider to a maximum */<br>
> if (adev->family == AMDGPU_FAMILY_SI)<br>
> - ref_div_max = min(100 / post_div, ref_div_max);<br>
> + ref_div_max = min(ref_div_limit / post_div, ref_div_max);<br>
> else<br>
> ref_div_max = min(128 / post_div, ref_div_max);<br>
> <br>
> @@ -136,6 +137,7 @@ void amdgpu_pll_compute(struct
amdgpu_device *adev,<br>
> unsigned ref_div_min, ref_div_max, ref_div;<br>
> unsigned post_div_best, diff_best;<br>
> unsigned nom, den;<br>
> + unsigned ref_div_limit, ref_limit_best;<br>
> <br>
> /* determine allowed feedback divider range */<br>
> fb_div_min = pll->min_feedback_div;<br>
> @@ -204,11 +206,12 @@ void amdgpu_pll_compute(struct
amdgpu_device *adev,<br>
> else<br>
> post_div_best = post_div_max;<br>
> diff_best = ~0;<br>
> + ref_div_limit = ref_limit_best = 100;<br>
> <br>
> for (post_div = post_div_min; post_div <=
post_div_max; ++post_div) {<br>
> unsigned diff;<br>
> amdgpu_pll_get_fb_ref_div(adev, nom, den, post_div,
fb_div_max,<br>
> - ref_div_max, &fb_div, &ref_div);<br>
> + ref_div_max, ref_div_limit, &fb_div, &ref_div);<br>
> diff = abs(target_clock - (pll->reference_freq *
fb_div) /<br>
> (ref_div * post_div));<br>
> <br>
> @@ -217,13 +220,19 @@ void amdgpu_pll_compute(struct
amdgpu_device *adev,<br>
> <br>
> post_div_best = post_div;<br>
> diff_best = diff;<br>
> + ref_limit_best = ref_div_limit;<br>
> }<br>
> + if (post_div >= post_div_max && diff_best != 0
&& ref_div_limit != 128) {<br>
> + ref_div_limit = 128;<br>
> + post_div = post_div_min - 1;<br>
> + }<br>
> +<br>
> }<br>
> post_div = post_div_best;<br>
> <br>
> /* get the feedback and reference divider for the optimal
value */<br>
> amdgpu_pll_get_fb_ref_div(adev, nom, den, post_div,
fb_div_max, ref_div_max,<br>
> - &fb_div, &ref_div);<br>
> + ref_limit_best, &fb_div, &ref_div);<br>
> <br>
> /* reduce the numbers to a simpler ratio once more */<br>
> /* this also makes sure that the reference divider is
large enough */<br>
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c
b/drivers/gpu/drm/radeon/radeon_display.c<br>
> index f12675e3d261..0fcbf45a68db 100644<br>
> --- a/drivers/gpu/drm/radeon/radeon_display.c<br>
> +++ b/drivers/gpu/drm/radeon/radeon_display.c<br>
> @@ -925,10 +925,10 @@ static void
avivo_reduce_ratio(unsigned *nom, unsigned *den,<br>
> */<br>
> static void avivo_get_fb_ref_div(unsigned nom, unsigned
den, unsigned post_div,<br>
> unsigned fb_div_max, unsigned ref_div_max,<br>
> - unsigned *fb_div, unsigned *ref_div)<br>
> + unsigned ref_div_limit, unsigned *fb_div, unsigned
*ref_div)<br>
> {<br>
> /* limit reference * post divider to a maximum */<br>
> - ref_div_max = max(min(100 / post_div, ref_div_max), 1u);<br>
> + ref_div_max = max(min(ref_div_limit / post_div,
ref_div_max), 1u);<br>
> <br>
> /* get matching reference and feedback divider */<br>
> *ref_div = min(max(den/post_div, 1u), ref_div_max);<br>
> @@ -971,6 +971,7 @@ void radeon_compute_pll_avivo(struct
radeon_pll *pll,<br>
> unsigned ref_div_min, ref_div_max, ref_div;<br>
> unsigned post_div_best, diff_best;<br>
> unsigned nom, den;<br>
> + unsigned ref_div_limit, ref_limit_best;<br>
> <br>
> /* determine allowed feedback divider range */<br>
> fb_div_min = pll->min_feedback_div;<br>
> @@ -1042,11 +1043,12 @@ void
radeon_compute_pll_avivo(struct radeon_pll *pll,<br>
> else<br>
> post_div_best = post_div_max;<br>
> diff_best = ~0;<br>
> + ref_div_limit = ref_limit_best = 100;<br>
> <br>
> for (post_div = post_div_min; post_div <=
post_div_max; ++post_div) {<br>
> unsigned diff;<br>
> avivo_get_fb_ref_div(nom, den, post_div, fb_div_max,<br>
> - ref_div_max, &fb_div, &ref_div);<br>
> + ref_div_max, ref_div_limit, &fb_div,
&ref_div);<br>
> diff = abs(target_clock - (pll->reference_freq *
fb_div) /<br>
> (ref_div * post_div));<br>
> <br>
> @@ -1055,13 +1057,18 @@ void
radeon_compute_pll_avivo(struct radeon_pll *pll,<br>
> <br>
> post_div_best = post_div;<br>
> diff_best = diff;<br>
> + ref_limit_best = ref_div_limit;<br>
> + }<br>
> + if (post_div >= post_div_max && diff_best != 0
&& ref_div_limit != 128) {<br>
> + ref_div_limit = 128;<br>
> + post_div = post_div_min - 1;<br>
> }<br>
> }<br>
> post_div = post_div_best;<br>
> <br>
> /* get the feedback and reference divider for the optimal
value */<br>
> avivo_get_fb_ref_div(nom, den, post_div, fb_div_max,
ref_div_max,<br>
> - &fb_div, &ref_div);<br>
> + ref_limit_best, &fb_div, &ref_div);<br>
> <br>
> /* reduce the numbers to a simpler ratio once more */<br>
> /* this also makes sure that the reference divider is
large enough */<br>
<br>
</p><p><br>
</p>