[PATCH] drm/i915: Detect the error case for too large CRTC offsets

[PATCH] drm/i915: Detect the error case for too large CRTC offsets

[Chris Wilson]

On gen4+, we program the CRTC offset into the DSPTILEOFF register, which
is limited to 12-bits of integer precision in both the x and the y axes.
Programming a value larger than or equal to 4096 results in a wraparound
of the CRTC location and a distorted output. This is undesirable, so
abort the operation and report the error condition.

We could attempt to mitigate this error by recomputing the DSPSURF base
address to a nearby tile-row and adjusting the offsets relative to the
fake CRTC subsurface. However, the question is whether this is common
enough to merit a workaround as only clients that use a single composite
fb and not per-crtc-fb such as X suffer. Such a design (rendering
directly into a single composite fb across the scanouts) interacts badly
with the power management of the display engine and is strongly
discouraged; future windowing systems are designed around being able to
page-flip per-crtc-fb. Within X itself, the mechanisms are already in
place to avoid making such setcrtc requests, at a slight cost to
efficiency.

Cc: stable@kernel.org
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5708d9b..bd3f84b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1995,6 +1995,11 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 		return -EINVAL;
 	}
 
+	if (INTEL_INFO(dev)->gen >= 4 && (x|y) & ~4095) {
+		DRM_ERROR("CRTC offset too larget (%d, %d)\n", x, y);
+		return -EINVAL;
+	}
+
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
@@ -2069,6 +2074,11 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
 		return -EINVAL;
 	}
 
+	if ((x|y) & ~4095) {
+		DRM_ERROR("CRTC offset too larget (%d, %d)\n", x, y);
+		return -EINVAL;
+	}
+
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
-- 
1.7.10

Re: [PATCH] drm/i915: Detect the error case for too large CRTC offsets

[Olivier Galibert]

On Thu, Jul 05, 2012 at 10:10:10AM +0100, Chris Wilson wrote:
> +	if (INTEL_INFO(dev)->gen >= 4 && (x|y) & ~4095) {
> +		DRM_ERROR("CRTC offset too larget (%d, %d)\n", x, y);

larget?

  OG.

Re: [PATCH] drm/i915: Detect the error case for too large CRTC offsets

[Chris Wilson]

On Thu, 5 Jul 2012 19:22:37 +0200, Olivier Galibert <galibert@pobox.com> wrote:
> On Thu, Jul 05, 2012 at 10:10:10AM +0100, Chris Wilson wrote:
> > +	if (INTEL_INFO(dev)->gen >= 4 && (x|y) & ~4095) {
> > +		DRM_ERROR("CRTC offset too larget (%d, %d)\n", x, y);
> 
> larget?

Not even a humorous typo. Daniel has posted a couple of patches to
actually workaround the limits which obsoletes the need for rejecting
the mode_set_base().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre