| Message ID | 20180313150759.27620-2-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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ä <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. Would be nice indeed. > 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 > http://blog.ffwll.ch
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ä <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> 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-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
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; }