Re: [PATCH 2/3] drm: Make drm_mode_vrefresh() a bit more accurate

Daniel Vetter
2018-03-14T13:56:32Z

On Tue, Mar 13, 2018 at 05:07:58PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Do the refresh rate calculation with a single division. This gives
> us slightly more accurate results, especially for interlaced since
> we don't just double the final truncated result.
>
> We do lose one bit compared to the old way, so with an interlaced
> mode the new code can only handle ~2GHz instead of the ~4GHz the
> old code handeled.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I kinda want special integers here that Oops on overflow, I thought
they're coming. That aside:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/drm_modes.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 4157250140b0..f6b7c0e36a1a 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -773,24 +773,23 @@ EXPORT_SYMBOL(drm_mode_hsync);
> int drm_mode_vrefresh(const struct drm_display_mode *mode)
> {
> int refresh = 0;
> - unsigned int calc_val;
>
> if (mode->vrefresh > 0)
> refresh = mode->vrefresh;
> else if (mode->htotal > 0 && mode->vtotal > 0) {
> - int vtotal;
> - vtotal = mode->vtotal;
> - /* work out vrefresh the value will be x1000 */
> - calc_val = (mode->clock * 1000);
> - calc_val /= mode->htotal;
> - refresh = (calc_val + vtotal / 2) / vtotal;
> + unsigned int num, den;
> +
> + num = mode->clock * 1000;
> + den = mode->htotal * mode->vtotal;
>
> if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> - refresh *= 2;
> + num *= 2;
> if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> - refresh /= 2;
> + den *= 2;
> if (mode->vscan > 1)
> - refresh /= mode->vscan;
> + den *= mode->vscan;
> +
> + refresh = DIV_ROUND_CLOSEST(num, den);
> }
> return refresh;
> }
> --
> 2.16.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Daniel Vetter
Software Engineer, Intel Corporation