Bug 61152 - Ubuntu12.04.2 LinearFramebuffer under extended mode broken by intel_gen4_compute_offset_xtiled()
Summary: Ubuntu12.04.2 LinearFramebuffer under extended mode broken by intel_gen4_comp...
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/intel (show other bugs)
Version: unspecified
Hardware: x86-64 (AMD64) Linux (All)
: medium blocker
Assignee: Chris Wilson
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-20 04:27 UTC by kevin
Modified: 2013-03-06 14:24 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
corruption shows on the secondary monitor (1.01 MB, image/jpeg)
2013-02-20 04:27 UTC, kevin
no flags Details
Secondary monitor shows corruptions (1.01 MB, image/jpeg)
2013-02-20 04:46 UTC, kevin
no flags Details
Fix LinearFramebuffer (4.56 KB, patch)
2013-02-21 14:46 UTC, Chris Wilson
no flags Details | Splinter Review
Fix LinearFramebuffer (4.84 KB, patch)
2013-02-21 19:06 UTC, Chris Wilson
no flags Details | Splinter Review

Description kevin 2013-02-20 04:27:50 UTC
Created attachment 75152 [details]
corruption shows on the secondary monitor

Steps to reproduce:
1. Ubuntu12.04.2 image(getting it from Canonical side), with i915_hsw to support intel haswell asic
2. Connect the second monitor and desktop would configured into extended mode, screen corruption will appear shown as the attached picture.
Comment 1 kevin 2013-02-20 04:32:04 UTC
Before duplicate the issue, please configure intel driver working under linear frame buffer mode.
Comment 2 kevin 2013-02-20 04:46:36 UTC
Created attachment 75153 [details]
Secondary monitor shows corruptions
Comment 3 Chris Wilson 2013-02-20 09:24:17 UTC
We need to migrate away from Option LinearFramebuffer; it is a power/performance nightmare for us.
Comment 4 kevin 2013-02-20 09:39:57 UTC
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.
Comment 5 Chris Wilson 2013-02-20 09:52:36 UTC
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;
Comment 6 kevin 2013-02-20 14:07:27 UTC
Wilson, would you like me to try your patch?
Btw, I don't have bspec you mentioned, could you share it with me ?
Comment 7 Jesse Barnes 2013-02-20 16:37:06 UTC
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.
Comment 8 Daniel Vetter 2013-02-20 17:38:20 UTC
Can't we do the right thing and use dma-buf/prime here? It like ... works ;-)
Comment 9 Jesse Barnes 2013-02-20 17:59:40 UTC
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).
Comment 10 kevin 2013-02-21 00:01:14 UTC
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.
Comment 11 Chris Wilson 2013-02-21 14:46:26 UTC
Created attachment 75254 [details] [review]
Fix LinearFramebuffer
Comment 12 Chris Wilson 2013-02-21 14:51:31 UTC
(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.
Comment 13 Chris Wilson 2013-02-21 19:06:25 UTC
Created attachment 75265 [details] [review]
Fix LinearFramebuffer
Comment 14 Daniel Vetter 2013-02-21 21:23:30 UTC
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
Comment 15 kevin 2013-02-22 03:25:49 UTC
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 ?
Comment 16 Chris Wilson 2013-02-22 08:12:10 UTC
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.
Comment 17 kevin 2013-02-22 09:50:10 UTC
I guess the performance you are mentioning is render + surface scanout, is that correct?
Comment 18 Chris Wilson 2013-02-22 10:04:37 UTC
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.
Comment 19 Daniel Vetter 2013-02-22 11:12:54 UTC
Original bug is fixed, the the design discussions around why we can't get rid of the discussion is something different entirely imo.
Comment 20 James M. Leddy 2013-03-05 18:39:06 UTC
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?
Comment 21 James M. Leddy 2013-03-05 18:46:15 UTC
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
Comment 22 James M. Leddy 2013-03-05 18:52:39 UTC
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.
Comment 23 Timo Aaltonen 2013-03-06 14:24:55 UTC
james: that's the infamous (ubuntu) boot race bug, most likely not related to the commit since it's been there for some time..


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.