[PATCH] drm/i915: Avoid div by zero when pixel clock is large

[PATCH] drm/i915: Avoid div by zero when pixel clock is large

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Make sure the line_time_us isn't zero in the gmch watermarks code as
that would cause a div by zero. This can be triggered by specifying
a very fast pixel clock for the mode.

At some point we should probably just switch over to using the same
math we use on PCH platforms which avoids such intermediate rounded
results.

Also we should verify the user provided mode much more rigorously.
At the moment we accept pretty much anything.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e4a0c9c..b84836f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1131,7 +1131,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
 		*plane_wm = display->max_wm;
 
 	/* Use the large buffer method to calculate cursor watermark */
-	line_time_us = ((htotal * 1000) / clock);
+	line_time_us = max(htotal * 1000 / clock, 1);
 	line_count = (cursor_latency_ns / line_time_us + 1000) / 1000;
 	entries = line_count * 64 * pixel_size;
 	tlb_miss = cursor->fifo_size*cursor->cacheline_size - hdisplay * 8;
@@ -1207,7 +1207,7 @@ static bool g4x_compute_srwm(struct drm_device *dev,
 	hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
 	pixel_size = crtc->fb->bits_per_pixel / 8;
 
-	line_time_us = (htotal * 1000) / clock;
+	line_time_us = max(htotal * 1000 / clock, 1);
 	line_count = (latency_ns / line_time_us + 1000) / 1000;
 	line_size = hdisplay * pixel_size;
 
@@ -1440,7 +1440,7 @@ static void i965_update_wm(struct drm_crtc *unused_crtc)
 		unsigned long line_time_us;
 		int entries;
 
-		line_time_us = ((htotal * 1000) / clock);
+		line_time_us = max(htotal * 1000 / clock, 1);
 
 		/* Use ns/us then divide to preserve precision */
 		entries = (((sr_latency_ns / line_time_us) + 1000) / 1000) *
@@ -1566,7 +1566,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
 		unsigned long line_time_us;
 		int entries;
 
-		line_time_us = (htotal * 1000) / clock;
+		line_time_us = max(htotal * 1000 / clock, 1);
 
 		/* Use ns/us then divide to preserve precision */
 		entries = (((sr_latency_ns / line_time_us) + 1000) / 1000) *
-- 
1.8.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Avoid div by zero when pixel clock is large

[Chris Wilson]

On Fri, Feb 14, 2014 at 02:18:57PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Make sure the line_time_us isn't zero in the gmch watermarks code as
> that would cause a div by zero. This can be triggered by specifying
> a very fast pixel clock for the mode.

Very fast but valid? If it was just very fast, we should have rejected
the mode line. On the other hand, if it was valid, maybe we should
adjust the bias (* 1000 / 1000) so that the computation remains
reasonably accurate because we will need a larger FIFO to accommodate
the higher clock rate.

Anyway, even with the larger bias, such a safe guard is sensisble. I
can't argue against this patch, but I just wonder if you opened a can of
worms.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Avoid div by zero when pixel clock is large

[Ville Syrjälä]

On Fri, Feb 14, 2014 at 12:28:29PM +0000, Chris Wilson wrote:
> On Fri, Feb 14, 2014 at 02:18:57PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Make sure the line_time_us isn't zero in the gmch watermarks code as
> > that would cause a div by zero. This can be triggered by specifying
> > a very fast pixel clock for the mode.
> 
> Very fast but valid?

74.25 GHz. I'm not sure which way you would classify that :)

> If it was just very fast, we should have rejected
> the mode line.

Except we do more or less zero checks on the mode supplied to the
setcrtc ioctl. Something we should really get fixed at some point.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: Avoid div by zero when pixel clock is large

[Daniel Vetter]

On Fri, Feb 14, 2014 at 02:40:39PM +0200, Ville Syrjälä wrote:
> On Fri, Feb 14, 2014 at 12:28:29PM +0000, Chris Wilson wrote:
> > On Fri, Feb 14, 2014 at 02:18:57PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Make sure the line_time_us isn't zero in the gmch watermarks code as
> > > that would cause a div by zero. This can be triggered by specifying
> > > a very fast pixel clock for the mode.
> > 
> > Very fast but valid?
> 
> 74.25 GHz. I'm not sure which way you would classify that :)

I've added this to the commit message ...

> > If it was just very fast, we should have rejected
> > the mode line.
> 
> Except we do more or less zero checks on the mode supplied to the
> setcrtc ioctl. Something we should really get fixed at some point.

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch