* [PATCH] drm: Use u64 for intermediate dotclock calculations
@ 2016-10-21 14:15 Chris Wilson
2016-10-21 14:22 ` Alex Deucher
0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2016-10-21 14:15 UTC (permalink / raw)
To: dri-devel
We have reached the era where monitor bandwidths now exceed 31bits in
frequency calculations, though as we stored them in kHz units we are
safe from overflow in the modelines for some time.
[ 48.723720] UBSAN: Undefined behaviour in ../drivers/gpu/drm/drm_modes.c:325:49
[ 48.726943] signed integer overflow:
[ 48.728503] 2240 * 1000000 cannot be represented in type 'int'
Reported-by: Martin Liška <marxin.liska@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98372
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/drm_modes.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 173b7d335834..f64ac86deb84 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -165,6 +165,7 @@ struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, int hdisplay,
unsigned int vfieldrate, hperiod;
int hdisplay_rnd, hmargin, vdisplay_rnd, vmargin, vsync;
int interlace;
+ u64 tmp;
/* allocate the drm_display_mode structure. If failure, we will
* return directly
@@ -322,8 +323,11 @@ struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, int hdisplay,
drm_mode->vsync_end = drm_mode->vsync_start + vsync;
}
/* 15/13. Find pixel clock frequency (kHz for xf86) */
- drm_mode->clock = drm_mode->htotal * HV_FACTOR * 1000 / hperiod;
- drm_mode->clock -= drm_mode->clock % CVT_CLOCK_STEP;
+ tmp = drm_mode->htotal; /* perform intermediate calcs in u64 */
+ tmp *= HV_FACTOR * 1000;
+ do_div(tmp, hperiod);
+ tmp -= drm_mode->clock % CVT_CLOCK_STEP;
+ drm_mode->clock = tmp;
/* 18/16. Find actual vertical frame frequency */
/* ignore - just set the mode flag for interlaced */
if (interlaced) {
--
2.9.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm: Use u64 for intermediate dotclock calculations
2016-10-21 14:15 [PATCH] drm: Use u64 for intermediate dotclock calculations Chris Wilson
@ 2016-10-21 14:22 ` Alex Deucher
2016-10-21 18:23 ` Daniel Vetter
0 siblings, 1 reply; 3+ messages in thread
From: Alex Deucher @ 2016-10-21 14:22 UTC (permalink / raw)
To: Chris Wilson; +Cc: Maling list - DRI developers
On Fri, Oct 21, 2016 at 10:15 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> We have reached the era where monitor bandwidths now exceed 31bits in
> frequency calculations, though as we stored them in kHz units we are
> safe from overflow in the modelines for some time.
>
> [ 48.723720] UBSAN: Undefined behaviour in ../drivers/gpu/drm/drm_modes.c:325:49
> [ 48.726943] signed integer overflow:
> [ 48.728503] 2240 * 1000000 cannot be represented in type 'int'
>
> Reported-by: Martin Liška <marxin.liska@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98372
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/drm_modes.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 173b7d335834..f64ac86deb84 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -165,6 +165,7 @@ struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, int hdisplay,
> unsigned int vfieldrate, hperiod;
> int hdisplay_rnd, hmargin, vdisplay_rnd, vmargin, vsync;
> int interlace;
> + u64 tmp;
>
> /* allocate the drm_display_mode structure. If failure, we will
> * return directly
> @@ -322,8 +323,11 @@ struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, int hdisplay,
> drm_mode->vsync_end = drm_mode->vsync_start + vsync;
> }
> /* 15/13. Find pixel clock frequency (kHz for xf86) */
> - drm_mode->clock = drm_mode->htotal * HV_FACTOR * 1000 / hperiod;
> - drm_mode->clock -= drm_mode->clock % CVT_CLOCK_STEP;
> + tmp = drm_mode->htotal; /* perform intermediate calcs in u64 */
> + tmp *= HV_FACTOR * 1000;
> + do_div(tmp, hperiod);
> + tmp -= drm_mode->clock % CVT_CLOCK_STEP;
> + drm_mode->clock = tmp;
> /* 18/16. Find actual vertical frame frequency */
> /* ignore - just set the mode flag for interlaced */
> if (interlaced) {
> --
> 2.9.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm: Use u64 for intermediate dotclock calculations
2016-10-21 14:22 ` Alex Deucher
@ 2016-10-21 18:23 ` Daniel Vetter
0 siblings, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2016-10-21 18:23 UTC (permalink / raw)
To: Alex Deucher; +Cc: Maling list - DRI developers
On Fri, Oct 21, 2016 at 10:22:04AM -0400, Alex Deucher wrote:
> On Fri, Oct 21, 2016 at 10:15 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > We have reached the era where monitor bandwidths now exceed 31bits in
> > frequency calculations, though as we stored them in kHz units we are
> > safe from overflow in the modelines for some time.
> >
> > [ 48.723720] UBSAN: Undefined behaviour in ../drivers/gpu/drm/drm_modes.c:325:49
> > [ 48.726943] signed integer overflow:
> > [ 48.728503] 2240 * 1000000 cannot be represented in type 'int'
> >
> > Reported-by: Martin Liška <marxin.liska@gmail.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98372
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Applied to drm-misc.
-Daniel
>
> > ---
> > drivers/gpu/drm/drm_modes.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index 173b7d335834..f64ac86deb84 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -165,6 +165,7 @@ struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, int hdisplay,
> > unsigned int vfieldrate, hperiod;
> > int hdisplay_rnd, hmargin, vdisplay_rnd, vmargin, vsync;
> > int interlace;
> > + u64 tmp;
> >
> > /* allocate the drm_display_mode structure. If failure, we will
> > * return directly
> > @@ -322,8 +323,11 @@ struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, int hdisplay,
> > drm_mode->vsync_end = drm_mode->vsync_start + vsync;
> > }
> > /* 15/13. Find pixel clock frequency (kHz for xf86) */
> > - drm_mode->clock = drm_mode->htotal * HV_FACTOR * 1000 / hperiod;
> > - drm_mode->clock -= drm_mode->clock % CVT_CLOCK_STEP;
> > + tmp = drm_mode->htotal; /* perform intermediate calcs in u64 */
> > + tmp *= HV_FACTOR * 1000;
> > + do_div(tmp, hperiod);
> > + tmp -= drm_mode->clock % CVT_CLOCK_STEP;
> > + drm_mode->clock = tmp;
> > /* 18/16. Find actual vertical frame frequency */
> > /* ignore - just set the mode flag for interlaced */
> > if (interlaced) {
> > --
> > 2.9.3
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-10-21 18:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-21 14:15 [PATCH] drm: Use u64 for intermediate dotclock calculations Chris Wilson
2016-10-21 14:22 ` Alex Deucher
2016-10-21 18:23 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.