Bug 76176

Summary: [BYT] Page flip doesn't change tiling mode
Product: DRI Reporter: Ander Conselvan de Oliveira <conselvan2>
Component: DRM/IntelAssignee: Chris Wilson <chris>
Status: CLOSED FIXED QA Contact: Intel GFX Bugs mailing list <intel-gfx-bugs>
Severity: normal    
Priority: medium CC: intel-gfx-bugs, tiago.vignatti, ullysses.a.eoff
Version: unspecified   
Hardware: x86 (IA32)   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments:
Description Flags
Simple test case that reproduces the bug.
none
dmesg output
none
Patch for intel-gpu-tools none

Description Ander Conselvan de Oliveira 2014-03-14 15:04:44 UTC
Created attachment 95812 [details]
Simple test case that reproduces the bug.

On BayTrail, page flipping to a bo that has a different tiling format than the current front buffer does not work properly. The new buffer is scanned out using the tiling format of the fb used in the drmModeSetCrtc() call.

The attached program reproduces the bug. It works properly on SandyBridge. The expected behavior is for the even scanlines of the display to be colored, and the odd ones black.
Comment 1 Ander Conselvan de Oliveira 2014-03-14 15:14:39 UTC
Created attachment 95814 [details]
dmesg output
Comment 2 Ander Conselvan de Oliveira 2014-03-14 15:24:20 UTC
I forgot to mention, I saw the issue with a 3.14.0-rc3+ kernel. There are reports of the same issue with 3.12.8 and 3.13.6.
Comment 3 Jesse Barnes 2014-03-14 23:08:52 UTC
I see the same thing here, but going from untiled to a tiled flip seems to work.  Maybe the CS msg to the display block is incorrect, or the tile bit isn't getting cleared when the msg is received.  I've asked the hw guys to check into it.
Comment 4 Daniel Vetter 2014-03-27 09:55:54 UTC
We don't have a tiled->untiled->tiled pageflip testcase anywhere. No wonder this is broken and untested.

Ander, can you please convert your testcase into an igt test using the crc infrastructure to make sure we scan out the right stuff? That way we can at least validate whether this works and if needed escalate early.

The local guys in Helsinki should be able to help you out and since very recently we have neat docs for igt:

http://people.freedesktop.org/~danvet/igt/
Comment 5 Ander Conselvan de Oliveira 2014-04-02 12:34:05 UTC
Created attachment 96779 [details] [review]
Patch for intel-gpu-tools
Comment 6 Daniel Vetter 2014-04-11 14:18:43 UTC
Ok, I've applied the testcase with a bit of a fixup.
Comment 7 Jesse Barnes 2014-06-24 19:33:59 UTC
We're moving to mmio based flips, which should address this:
http://lists.freedesktop.org/archives/intel-gfx/2014-January/038067.html
Comment 8 Chris Wilson 2014-06-24 19:38:24 UTC
It doesn't fix it yet...
Comment 9 Chris Wilson 2014-06-25 16:44:20 UTC
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 97432d2..e3fba41 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9763,6 +9763,10 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 
        if (IS_VALLEYVIEW(dev)) {
                ring = &dev_priv->ring[BCS];
+               if (obj->tiling_mode != to_intel_framebuffer(old_fb)->obj->tiling_mode)
+                       ring = NULL;
+       } else if (IS_IVYBRIDGE(dev)) {
+               ring = &dev_priv->ring[BCS];
        } else if (INTEL_INFO(dev)->gen >= 7) {
Comment 10 Jesse Barnes 2014-06-25 19:46:05 UTC
I guess Chris can tilt at this windmill. :)

Another thing I don't think I tried is to write the tile reg and surf reg *after* the flip in the CS.  Writing it before didn't seem to work, probably because the tile bit got clobbered by the message from the GT side.  But writing it after, while racy, might work.
Comment 11 Ville Syrjala 2014-06-25 19:53:37 UTC
(In reply to comment #10)
> I guess Chris can tilt at this windmill. :)
> 
> Another thing I don't think I tried is to write the tile reg and surf reg
> *after* the flip in the CS.  Writing it before didn't seem to work, probably
> because the tile bit got clobbered by the message from the GT side.  But
> writing it after, while racy, might work.

Yeah mean with LRI? AFAIK LRIs targeting display registers end up in /dev/null on byt.
Comment 12 Chris Wilson 2014-06-25 20:05:01 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > I guess Chris can tilt at this windmill. :)
> > 
> > Another thing I don't think I tried is to write the tile reg and surf reg
> > *after* the flip in the CS.  Writing it before didn't seem to work, probably
> > because the tile bit got clobbered by the message from the GT side.  But
> > writing it after, while racy, might work.
> 
> Yeah mean with LRI? AFAIK LRIs targeting display registers end up in
> /dev/null on byt.

Yes. All we need to do is to make sure we do actually engage the mmio flip. Sadly the same applies for #77104
Comment 13 Chris Wilson 2014-07-19 10:44:53 UTC
commit 8e09bf837f8c6b09784bf22c3a8c597df3c20b79
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Jul 8 10:40:30 2014 +0100

    drm/i915: Use mmio flips to change tiling mode on Baytrail
    
    For whatever reason, MI_DISPLAY_FLIP fails to change tiling mode on
    Baytrail, so just use CPU driven mmio flips instead.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76176
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

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.