diff mbox

[2/2] drm/i915: adjust framebuffer base address on gen4+

Message ID 1341483450-6385-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter July 5, 2012, 10:17 a.m. UTC 
The tileoffset register only supports a limited offset in x/y of 4096,
so for giant screen configuration with a shared fb we wrap around.

Fix this by computing a linear offset in tiles (pages) and only use
the tileoffset register to offset within the tile.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h      |    2 +-
 drivers/gpu/drm/i915/intel_display.c |   47 +++++++++++++++++++++++++++-------
 2 files changed, 39 insertions(+), 10 deletions(-)

Comments

Chris Wilson July 5, 2012, 10:29 a.m. UTC | #1 
On Thu,  5 Jul 2012 12:17:30 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> The tileoffset register only supports a limited offset in x/y of 4096,
> so for giant screen configuration with a shared fb we wrap around.
> 
> Fix this by computing a linear offset in tiles (pages) and only use
> the tileoffset register to offset within the tile.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Both Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

We could do with validating that the CRTC is within the fb->obj,
otherwise we run foul of hanging the hardware. (Since this is a
pre-existing condition it doesn't mar these patches.)

So the only question is whether we indicate to userspace that the kernel
is fixed? Or just kill the w/a in userspace and for reasons of sanity
strongly encourage everyone who hits this to upgrade? Since this is no
stable material, having keeping the w/a seems to make sense...
-Chris
Chris Wilson July 5, 2012, 10:37 a.m. UTC | #2 
On Thu,  5 Jul 2012 12:17:30 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> The tileoffset register only supports a limited offset in x/y of 4096,
> so for giant screen configuration with a shared fb we wrap around.
> 
> Fix this by computing a linear offset in tiles (pages) and only use
> the tileoffset register to offset within the tile.

One thing I did spot after hitting send, is that we don't enforce that
the new_fb has the same pitch as the old_fb when flipping. Whilst there
are patching floating around to do so in the midlayer, we need to
express that constraint now for this patch to be valid.
-Chris
Daniel Vetter July 5, 2012, 11:38 a.m. UTC | #3 
On Thu, Jul 05, 2012 at 11:29:10AM +0100, Chris Wilson wrote:
> On Thu,  5 Jul 2012 12:17:30 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > The tileoffset register only supports a limited offset in x/y of 4096,
> > so for giant screen configuration with a shared fb we wrap around.
> > 
> > Fix this by computing a linear offset in tiles (pages) and only use
> > the tileoffset register to offset within the tile.
> > 
> > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Both Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks for the review, I've addressed your pitch concern from the other
mail by merging one of Ville's patches.

> We could do with validating that the CRTC is within the fb->obj,
> otherwise we run foul of hanging the hardware. (Since this is a
> pre-existing condition it doesn't mar these patches.)
> 
> So the only question is whether we indicate to userspace that the kernel
> is fixed? Or just kill the w/a in userspace and for reasons of sanity
> strongly encourage everyone who hits this to upgrade? Since this is no
> stable material, having keeping the w/a seems to make sense...

As discussed I think we'll just ask ppl to upgrade their kernel and should
rip out the hack in userspace ...
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 871bedc..95d87e3 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2904,7 +2904,7 @@ 
 #define I915_LO_DISPBASE(val)	(val & ~DISP_BASEADDR_MASK)
 #define I915_HI_DISPBASE(val)	(val & DISP_BASEADDR_MASK)
 #define I915_MODIFY_DISPBASE(reg, gfx_addr) \
-		(I915_WRITE(reg, gfx_addr | I915_LO_DISPBASE(I915_READ(reg))))
+		(I915_WRITE((reg), (gfx_addr) | I915_LO_DISPBASE(I915_READ(reg))))
 
 /* VBIOS flags */
 #define SWF00			0x71410
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 839633c..08d9f8b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1824,6 +1824,22 @@  void intel_unpin_fb_obj(struct drm_i915_gem_object *obj)
 	i915_gem_object_unpin(obj);
 }
 
+/* Computes the linear offset to the base tile and adjusts x, y. bytes per pixel
+ * is assumed to be a power-of-two. */
+static unsigned long gen4_compute_dspaddr_offset_xtiled(int *x, int *y,
+							unsigned int bpp,
+							unsigned int pitch)
+{
+	int tile_rows, tiles;
+
+	tile_rows = *y / 8;
+	*y %= 8;
+	tiles = *x / (512/bpp);
+	*x %= 512/bpp;
+
+	return tile_rows * pitch * 8 + tiles * 4096;
+}
+
 static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 			     int x, int y)
 {
@@ -1882,16 +1898,22 @@  static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 
 	linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
 
-	if (INTEL_INFO(dev)->gen >= 4)
-		intel_crtc->dspaddr_offset = 0;
-	else
+	if (INTEL_INFO(dev)->gen >= 4) {
+		intel_crtc->dspaddr_offset =
+			gen4_compute_dspaddr_offset_xtiled(&x, &y,
+							   fb->bits_per_pixel / 8,
+							   fb->pitches[0]);
+		linear_offset -= intel_crtc->dspaddr_offset;
+	} else {
 		intel_crtc->dspaddr_offset = linear_offset;
+	}
 
 	DRM_DEBUG_KMS("Writing base %08X %08lX %d %d %d\n",
 		      obj->gtt_offset, linear_offset, x, y, fb->pitches[0]);
 	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
 	if (INTEL_INFO(dev)->gen >= 4) {
-		I915_MODIFY_DISPBASE(DSPSURF(plane), obj->gtt_offset);
+		I915_MODIFY_DISPBASE(DSPSURF(plane),
+				     obj->gtt_offset + intel_crtc->dspaddr_offset);
 		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
 		I915_WRITE(DSPLINOFF(plane), linear_offset);
 	} else
@@ -1966,12 +1988,17 @@  static int ironlake_update_plane(struct drm_crtc *crtc,
 	I915_WRITE(reg, dspcntr);
 
 	linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
-	intel_crtc->dspaddr_offset = 0;
+	intel_crtc->dspaddr_offset =
+		gen4_compute_dspaddr_offset_xtiled(&x, &y,
+						   fb->bits_per_pixel / 8,
+						   fb->pitches[0]);
+	linear_offset -= intel_crtc->dspaddr_offset;
 
 	DRM_DEBUG_KMS("Writing base %08X %08lX %d %d %d\n",
 		      obj->gtt_offset, linear_offset, x, y, fb->pitches[0]);
 	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
-	I915_MODIFY_DISPBASE(DSPSURF(plane), obj->gtt_offset);
+	I915_MODIFY_DISPBASE(DSPSURF(plane),
+			     obj->gtt_offset + intel_crtc->dspaddr_offset);
 	I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
 	I915_WRITE(DSPLINOFF(plane), linear_offset);
 	POSTING_READ(reg);
@@ -6080,7 +6107,9 @@  static int intel_gen4_queue_flip(struct drm_device *dev,
 	intel_ring_emit(ring, MI_DISPLAY_FLIP |
 			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 	intel_ring_emit(ring, fb->pitches[0]);
-	intel_ring_emit(ring, obj->gtt_offset | obj->tiling_mode);
+	intel_ring_emit(ring,
+			(obj->gtt_offset + intel_crtc->dspaddr_offset) |
+			obj->tiling_mode);
 
 	/* XXX Enabling the panel-fitter across page-flip is so far
 	 * untested on non-native modes, so ignore it for now.
@@ -6120,7 +6149,7 @@  static int intel_gen6_queue_flip(struct drm_device *dev,
 	intel_ring_emit(ring, MI_DISPLAY_FLIP |
 			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 	intel_ring_emit(ring, fb->pitches[0] | obj->tiling_mode);
-	intel_ring_emit(ring, obj->gtt_offset);
+	intel_ring_emit(ring, obj->gtt_offset + intel_crtc->dspaddr_offset);
 
 	/* Contrary to the suggestions in the documentation,
 	 * "Enable Panel Fitter" does not seem to be required when page
@@ -6183,7 +6212,7 @@  static int intel_gen7_queue_flip(struct drm_device *dev,
 
 	intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane_bit);
 	intel_ring_emit(ring, (fb->pitches[0] | obj->tiling_mode));
-	intel_ring_emit(ring, (obj->gtt_offset));
+	intel_ring_emit(ring, obj->gtt_offset + intel_crtc->dspaddr_offset);
 	intel_ring_emit(ring, (MI_NOOP));
 	intel_ring_advance(ring);
 	return 0;