Re: [PATCH] drm: Use u64 for intermediate dotclock calculations

From: Daniel Vetter <daniel@ffwll.ch>
To: Alex Deucher <alexdeucher@gmail.com>
Cc: Maling list - DRI developers <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] drm: Use u64 for intermediate dotclock calculations
Date: Fri, 21 Oct 2016 20:23:44 +0200	[thread overview]
Message-ID: <20161021182344.GP20761@phenom.ffwll.local> (raw)
In-Reply-To: <CADnq5_PR-CPDEX3uC_YWaFwsw5u7Unmaf5HRYJXhu1ai7Not2w@mail.gmail.com>

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