<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="blue" vlink="purple"> <div class="WordSection1"> <p class="MsoNormal">Thanks for the comments.<o:p></o:p></p> <p class="MsoNormal"><o:p> </o:p></p> <p class="MsoNormal">The input parameters are intended as uHz, but units do get translated within the function. We did not want to create new unnecessary parameters on the stack, which led up to the slightly confusing units. Maybe a better comment here would also help.<o:p></o:p></p> <p class="MsoNormal">And indeed, the comment about ceil needs to be fixed up. <o:p> </o:p></p> <p class="MsoNormal"><o:p> </o:p></p> <p class="MsoNormal">Thanks,<o:p></o:p></p> <p class="MsoNormal">Anthony<o:p></o:p></p> <p class="MsoNormal"><o:p> </o:p></p> <p class="MsoNormal"><b>From:</b> Nils Wallménius [mailto:nils.wallmenius@gmail.com] <br> <b>Sent:</b> Friday, April 13, 2018 1:05 AM<br> <b>To:</b> Wentland, Harry <Harry.Wentland@amd.com><br> <b>Cc:</b> amd-gfx@lists.freedesktop.org; Koo, Anthony <Anthony.Koo@amd.com><br> <b>Subject:</b> Re: [PATCH 10/21] drm/amd/display: Add back code to allow for rounding error<o:p></o:p></p> <p class="MsoNormal"><o:p> </o:p></p> <div> <div> <p class="MsoNormal" style="margin-bottom:12.0pt"><o:p> </o:p></p> <div> <div> <p class="MsoNormal">Den tis 10 apr. 2018 23:11Harry Wentland <<a href="mailto:harry.wentland@amd.com">harry.wentland@amd.com</a>> skrev:<o:p></o:p></p> </div> <blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in"> <p class="MsoNormal">From: Anthony Koo <<a href="mailto:Anthony.Koo@amd.com" target="_blank">Anthony.Koo@amd.com</a>><br> <br> Signed-off-by: Anthony Koo <<a href="mailto:Anthony.Koo@amd.com" target="_blank">Anthony.Koo@amd.com</a>><br> Reviewed-by: Aric Cyr <<a href="mailto:Aric.Cyr@amd.com" target="_blank">Aric.Cyr@amd.com</a>><br> Acked-by: Harry Wentland <<a href="mailto:harry.wentland@amd.com" target="_blank">harry.wentland@amd.com</a>><br> ---<br> drivers/gpu/drm/amd/display/modules/freesync/freesync.c | 13 ++++++++++++-<br> 1 file changed, 12 insertions(+), 1 deletion(-)<br> <br> diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c<br> index 4887c888bbe7..abd5c9374eb3 100644<br> --- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c<br> +++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c<br> @@ -896,6 +896,17 @@ bool mod_freesync_is_valid_range(struct mod_freesync *mod_freesync,<br> unsigned long long nominal_field_rate_in_uhz =<br> mod_freesync_calc_nominal_field_rate(stream);<br> <br> + /* Allow for some rounding error of actual video timing by taking ceil.<br> + * For example, 144 Hz mode timing may actually be 143.xxx Hz when<br> + * calculated from pixel rate and vertical/horizontal totals, but<br> + * this should be allowed instead of blocking FreeSync.<br> + */<o:p></o:p></p> </blockquote> </div> </div> <div> <p class="MsoNormal"><o:p> </o:p></p> </div> <div> <p class="MsoNormal">Hi, with this change the var names ending in *_in_uhz are a bit confusing, also the integer division will not round up (take ceil) as seems to be the intention from the above comment. Perhaps the comment needs to be improved?<o:p></o:p></p> </div> <div> <p class="MsoNormal"><o:p> </o:p></p> </div> <div> <p class="MsoNormal">BR<o:p></o:p></p> </div> <div> <p class="MsoNormal">Nils<o:p></o:p></p> </div> <div> <p class="MsoNormal"><o:p> </o:p></p> </div> <div> <p class="MsoNormal"><o:p> </o:p></p> </div> <div> <div> <blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in"> <p class="MsoNormal">+ nominal_field_rate_in_uhz = div_u64(nominal_field_rate_in_uhz, 1000000);<br> + min_refresh_cap_in_uhz /= 1000000;<br> + max_refresh_cap_in_uhz /= 1000000;<br> + min_refresh_request_in_uhz /= 1000000;<br> + max_refresh_request_in_uhz /= 1000000;<br> +<br> // Check nominal is within range<br> if (nominal_field_rate_in_uhz > max_refresh_cap_in_uhz ||<br> nominal_field_rate_in_uhz < min_refresh_cap_in_uhz)<br> @@ -921,7 +932,7 @@ bool mod_freesync_is_valid_range(struct mod_freesync *mod_freesync,<br> <br> // For variable range, check for at least 10 Hz range<br> if ((max_refresh_request_in_uhz != min_refresh_request_in_uhz) &&<br> - (max_refresh_request_in_uhz - min_refresh_request_in_uhz < 10000000))<br> + (max_refresh_request_in_uhz - min_refresh_request_in_uhz < 10))<br> return false;<br> <br> return true;<br> -- <br> 2.15.1<br> <br> _______________________________________________<br> amd-gfx mailing list<br> <a href="mailto:amd-gfx@lists.freedesktop.org" target="_blank">amd-gfx@lists.freedesktop.org</a><br> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" target="_blank">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><o:p></o:p></p> </blockquote> </div> </div> </div> </div> </body> </html>