Bug 61152

Before duplicate the issue, please configure intel driver working under linear frame buffer mode.

Created attachment 75153 [details]
Secondary monitor shows corruptions

We need to migrate away from Option LinearFramebuffer; it is a power/performance nightmare for us.

Thanks Wilson for the reply.
I have offline talked with jesse.barnes@intel about the linear frame buffer support issue under extended mode.

I will assign the bug to him once get his confirmation.

The code reads fine according to the bspec - it mentions no restrictions on PRI_OFFSET to tiled modes. So the only thing I could come up with was that we broke the ordering of latching:

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4179780..86edbb2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2175,10 +2175,10 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 		      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 + intel_crtc->dspaddr_offset);
 		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
 		I915_WRITE(DSPLINOFF(plane), linear_offset);
+		I915_MODIFY_DISPBASE(DSPSURF(plane),
+				     obj->gtt_offset + intel_crtc->dspaddr_offset);
 	} else
 		I915_WRITE(DSPADDR(plane), obj->gtt_offset + linear_offset);
 	POSTING_READ(reg);
@@ -2264,14 +2264,14 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
 	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 + intel_crtc->dspaddr_offset);
 	if (IS_HASWELL(dev)) {
 		I915_WRITE(DSPOFFSET(plane), (y << 16) | x);
 	} else {
 		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
 		I915_WRITE(DSPLINOFF(plane), linear_offset);
 	}
+	I915_MODIFY_DISPBASE(DSPSURF(plane),
+			     obj->gtt_offset + intel_crtc->dspaddr_offset);
 	POSTING_READ(reg);
 
 	return 0;

Wilson, would you like me to try your patch?
Btw, I don't have bspec you mentioned, could you share it with me ?

Kevin, I think Chris was trying to get at another possible solution.

Rather than relying on a linear framebuffer, maybe we could invent some new interfaces between our driver and the AMD driver that might allow for higher performance.

I still think we need to fix the LinearFramebuffer option, but we should explore something better as well.

Does your driver blit directly into the framebuffer we use for scanout?  Or do you blit from it to get current screen contents?  Either way, we may be able to be more efficient if we can leave the scanout buffer in tiled format and use the Intel BLT engine to do tile conversion for us.

Can't we do the right thing and use dma-buf/prime here? It like ... works ;-)

Maybe, but we still need to accommodate the formats required by the Intel and AMD chips to get good performance (not to mention power efficiency).

Jesse,

AMD driver render the desktop and blit the content to intel framebuffer.
However, AMD blit engine could not recognize intel tiling mode, so upon on preInit, we force intel driver working under linear frame buffer mode to do the right bliting.

(In reply to comment #7)
> Kevin, I think Chris was trying to get at another possible solution.

Rather
> than relying on a linear framebuffer, maybe we could invent some new
> interfaces between our driver and the AMD driver that might allow for higher
> performance.

I still think we need to fix the LinearFramebuffer option, but
> we should explore something better as well.

Does your driver blit directly
> into the framebuffer we use for scanout?  Or do you blit from it to get
> current screen contents?  Either way, we may be able to be more efficient if
> we can leave the scanout buffer in tiled format and use the Intel BLT engine
> to do tile conversion for us.

Created attachment 75254 [details] [review]
Fix LinearFramebuffer

(In reply to comment #10)
> AMD driver render the desktop and blit the content to intel framebuffer.
> However, AMD blit engine could not recognize intel tiling mode, so upon on
> preInit, we force intel driver working under linear frame buffer mode to do
> the right bliting.

You could have staged that transfer through a linear back buffer rather than force the framebuffer to be linear. Or compute the swizzling in a shader.

With the master/slave pixmap introduced for PRIME, we should be able to create an interface that is analogous to dma-buf so that we do not have to apply so many contortions.

Created attachment 75265 [details] [review]
Fix LinearFramebuffer

Patch merged to drm-intel-fixes as:

commit bc752862170c135d6c09fb22d79eeb451023568e
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu Feb 21 20:04:31 2013 +0000

    drm/i915: Handle untiled planes when computing their offsets

Guys, thank you for the fast fixing.
However, it is costly to manage Intel driver tiling information in our driver.

Besides, how much faster will Intel Crtc scanout Tiled frame buffer other than Linear frame buffer ?

Depending on your exact usage, if you have a screen wider than 1024 pixels across, the difference in performance is between 3-30x in favour of tiling. Considering that the Intel driver is preferrable in all cases except for gaming, it seems that you are penalising the major use case of those machines.

I guess the performance you are mentioning is render + surface scanout, is that correct?

Yes, the minimum access pattern of the render pipeline is a 2x2, and if the stride is such that cross a page boundary, you incur a TLB miss on every pixel.

The BLT engine doesn't have the same access pattern and so is not as adversely affected.

For the operations you want to perform the render pipeline is typically much faster (as that uses the L3 cache, which even has a notable impact for simple copies). Conversely the BLT has lower latency...

Forcing a LinearFramebuffer is a hack. Using a linear intermediate is still a hack, but at least a step forward.

Original bug is fixed, the the design discussions around why we can't get rid of the discussion is something different entirely imo.

I've applied the patch to the Ubuntu 3.5 kernel, but it causes even more problems. I had to apply 'ca320ac4 drm/i915: Use pixel size for computing linear offsets into a sprite' from drm-fixes as well since this patch depended on it.

Are there any other commits from drm-next that I need to incorporate?

I should add that I did this test on a haswell only machine. You can try the kernel for yourself here:

http://people.canonical.com/~jmleddy/.private/linearfb/

which I built by taking the latest quantal kernel from here:

git://kernel.ubuntu.com/ubuntu/ubuntu-quantal.git

with the addition of these patches:

b6f4aeac298cb891805591ada44757451d0c14f5 drm/i915: Handle untiled planes when computing their offsets
e3f9b83dbe0947a29dbaee5b6a43f0330cb94991 drm/i915: Use pixel size for computing linear offsets into a sprite

Additionally, symptoms of the failure are:

1) No plymouth boot up screen
2) black screen, with the cursor, and either:
   a) no console text
   b) get chip id failed: -1 [13]
      param:4, val:0
3) If I switch consoles to vt1 for example, the cursor stays
4) I can get in to unity by killing lightdm and manually restarting it.

james: that's the infamous (ubuntu) boot race bug, most likely not related to the commit since it's been there for some time..