Bug 77178

Summary: [ivb] Display corruption in Qt5 applications
Product: xorg Reporter: A Rojas <nqn1976>
Component: Driver/intelAssignee: Chris Wilson <chris>
Status: RESOLVED FIXED QA Contact: Intel GFX Bugs mailing list <intel-gfx-bugs>
Severity: normal    
Priority: medium    
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments:
Description Flags
Xorg log
none
Screenshot
none
Refactor CPU migration promotion none

Description A Rojas 2014-04-08 10:55:24 UTC
There is frequent display corruption in Qt5 applications with the latest intel driver. Windows are displayed completely black or with black boxes and horizontal stripes. It is usually fixed by resizing the window.

Using Arch Linux package xf86-video-intel 2.99.911-2, which is upstream 2.99.911 + the patch in http://cgit.freedesktop.org/xorg/driver/xf86-video-intel/patch/?id=3310ee89c1f1a663de5f5b12b8125809a213996f

This was introduced between 2.99.910 and 2.99.911, downgrading to 2.99.910 solves the problem
Comment 1 Chris Wilson 2014-04-08 10:57:56 UTC
Please confirm your bug with upstream and attach an Xorg.log and screenshot if possible.
Comment 2 A Rojas 2014-04-08 11:15:30 UTC
Created attachment 97073 [details]
Xorg log
Comment 3 A Rojas 2014-04-08 11:18:42 UTC
Created attachment 97074 [details]
Screenshot

This is firefox window on Qt5-based kwin. Happens for all windows in Qt5-kwin, and for Qt5 applications in KDE4.
Comment 4 A Rojas 2014-04-08 11:19:38 UTC
Not sure what you mean by upstream, I tried latest git and is still reproducible.
Comment 5 Chris Wilson 2014-04-08 11:30:40 UTC
That's what I needed you to verify, that the bug is still reproducible with git. Can you do a checkout of say 2.99.910 or 2.21.15 and see if it still fails?

Presumably you use KDE as well, what setup? i.e. kwin desktop effects, engines etc.

Are you also using mesa-10.1?
Comment 6 A Rojas 2014-04-08 15:57:29 UTC
Yes, I'm using mesa 10.1, KDE desktop effects enabled and it happens with both openGL 2 and 3 backends.

I've bisected and the guilty commit is:

commit 1de1104064b5898cbed37e836901694a381c1266
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Feb 21 22:43:04 2014 +0000

    sna: Use a hint to do whole image uploads inplace
    
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Comment 7 Chris Wilson 2014-04-08 15:59:51 UTC
That should have been fixed by

commit 3310ee89c1f1a663de5f5b12b8125809a213996f
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Sat Apr 5 12:18:31 2014 +0100

    sna: Avoid discarding damage when applying WHOLE hint to pixmap migration

Please do double check before I start pulling my hair out.
Comment 8 A Rojas 2014-04-08 16:18:33 UTC
Tested again with:

1) Arch linux package 2.99.911-2 (which does contain the mentioned fix)
2) Latest git (commit 617e96f)

They both show the issue.
Comment 9 Chris Wilson 2014-04-08 17:19:08 UTC
Created attachment 97084 [details] [review]
Refactor CPU migration promotion

Please test this patch.
Comment 10 A Rojas 2014-04-08 17:43:14 UTC
(In reply to comment #9)

> Please test this patch.

No improvement.
Comment 11 Chris Wilson 2014-04-08 17:53:59 UTC
Let's disable the two WHOLE hints and see which is the culprit.

A:

diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index 8abfdd0..639985c 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -2525,7 +2525,7 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
                return _sna_pixmap_move_to_cpu(pixmap, flags);
        }
 
-       if (flags & MOVE_WHOLE_HINT) {
+       if (flags & MOVE_WHOLE_HINT && 0) {
                DBG(("%s: region (%d, %d), (%d, %d) marked with WHOLE hint, pixmap %dx%d\n",
                       __FUNCTION__,
                       region->extents.x1,


B (on top of the attached patch):
diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index 8abfdd0..7a7499a 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -2500,11 +2500,13 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
                flags |= MOVE_INPLACE_HINT;
        }
 
+#if 0
        if (flags & MOVE_READ || (priv->gpu_damage == NULL && priv->cpu_damage == NULL)) {
                if (flags & MOVE_WHOLE_HINT ||
                    (flags & MOVE_WRITE && (priv->create & KGEM_CAN_CREATE_GPU) == 0))
                        return _sna_pixmap_move_to_cpu(pixmap, flags);
        }
+#endif
 
        if (get_drawable_deltas(drawable, pixmap, &dx, &dy)) {
                DBG(("%s: delta=(%d, %d)\n", __FUNCTION__, dx, dy));
Comment 12 A Rojas 2014-04-09 09:17:07 UTC
Do these patches go on top of git master? patch A doesn't apply here.
Comment 13 Chris Wilson 2014-04-09 09:26:31 UTC
Yes, they should both apply to git master.
Comment 14 A Rojas 2014-04-09 09:56:33 UTC
(In reply to comment #13)
> Yes, they should both apply to git master.

Was a whitespace issue, they apply with patch -l

So:
patch A alone -> bad
patch A + patch B -> good
Comment 15 Chris Wilson 2014-04-09 10:46:01 UTC
(In reply to comment #14)
> (In reply to comment #13)
> > Yes, they should both apply to git master.
> 
> Was a whitespace issue, they apply with patch -l

Apologies, I should learn not to paste diffs into bugzilla.

> So:
> patch A alone -> bad
> patch A + patch B -> good

That implies that is that first chunk, the promotion of the empty pixmap to whole that is troublesome. Thanks, time to mull it over.
Comment 16 Chris Wilson 2014-04-09 10:56:10 UTC
Is there any chance you can reproduce this issue with --enable-debug=full and send me the Xorg.0.log file? (Please grab the latest DBG patch from xf86-video-intel.git first.)
Comment 18 Chris Wilson 2014-04-09 11:43:05 UTC
I think

commit 510e5cf3fa4e1db8297be62461b602b13d18d8fc
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed Apr 9 12:26:53 2014 +0100

    sna: Tweak application of WHOLE hint for uploads
    
    This help with the continuing saga of
    commit 1de1104064b5898cbed37e836901694a381c1266 [2.99.911]
    Author: Chris Wilson <chris@chris-wilson.co.uk>
    Date:   Fri Feb 21 22:43:04 2014 +0000
    
        sna: Use a hint to do whole image uploads inplace
    
    References: https://bugs.freedesktop.org/show_bug.cgi?id=77178
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

should do the trick, but I have not finished digesting your debug log.
Comment 19 A Rojas 2014-04-09 11:47:51 UTC
(In reply to comment #18)

> should do the trick, but I have not finished digesting your debug log.

So, should I test it, or are there more fixes coming?
Comment 20 Chris Wilson 2014-04-09 11:54:38 UTC
That should fix the regression identified, I'm just not finding evidence of it in the debug log. :|
Comment 21 Chris Wilson 2014-04-09 11:57:03 UTC
Not to worry, my grepping was wrong.
Comment 22 Chris Wilson 2014-04-09 12:01:50 UTC
As, I see where my assumptions broke down. Here it is uploading the whole image, but the drawable is clipped.
Comment 23 Chris Wilson 2014-04-09 12:18:29 UTC
commit f14d6cbff3b9766f5fa834282a4eca6636b6fd7d
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed Apr 9 13:14:37 2014 +0100

    sna: Handle clipped PutImage uploads more carefully
    
    If the upload is clipped, we do not want to apply the WHOLE migration hints.
    
    References: https://bugs.freedesktop.org/show_bug.cgi?id=77178
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

That should behave better by not blindly applying the optimisation in cases where we are uploading less than the whole image.
Comment 24 A Rojas 2014-04-09 14:30:02 UTC
Everything OK in f14d6cb, thanks!

So which commits need to be backported to fix this downstream?
Comment 25 Chris Wilson 2014-04-09 15:04:38 UTC
The fix is 510e5cf3fa4e1db8297be62461b602b13d18d8fc.

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.