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

[Ville Syrjälä]

On Wed, Mar 14, 2018 at 02:56:28PM +0100, Daniel Vetter wrote:
> On Tue, Mar 13, 2018 at 05:07:58PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > 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ä 
> 
> I kinda want special integers here that Oops on overflow, I thought
> they're coming. That aside:
> 
> Reviewed-by: Daniel Vetter 

Thanks. Patches 1-2 pushed to drm-misc-next.

> > ---
> >  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-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Wed, Mar 14, 2018 at 02:56:28PM +0100, Daniel Vetter wrote:
> On Tue, Mar 13, 2018 at 05:07:58PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > 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ä 
> 
> I kinda want special integers here that Oops on overflow, I thought
> they're coming.

Would be nice indeed.

> That aside:
> 
> Reviewed-by: Daniel Vetter 
> > ---
> >  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-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

On Tue, Mar 13, 2018 at 05:07:58PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> 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ä 

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

Reviewed-by: Daniel Vetter 
> ---
>  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-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjala]

From: Ville Syrjälä 

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ä 
---
 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

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx