Bug 94901 - Display corruption due to deferred fbcon pixmap pitch calculation in radeon_set_pixmap_bo
Summary: Display corruption due to deferred fbcon pixmap pitch calculation in radeon_s...
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/Radeon (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: xf86-video-ati maintainers
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-11 22:06 UTC by jpsinthemix
Modified: 2016-05-24 08:06 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Xorg.0.log (36.14 KB, text/plain)
2016-04-11 22:06 UTC, jpsinthemix
no flags Details
Explicitly set the fbcon pixmap pitch again (1.05 KB, patch)
2016-04-12 09:20 UTC, Michel Dänzer
no flags Details | Splinter Review
Explicitly set the fbcon pixmap pitch again (3.47 KB, patch)
2016-05-13 03:16 UTC, Michel Dänzer
no flags Details | Splinter Review

Description jpsinthemix 2016-04-11 22:06:53 UTC
Created attachment 122874 [details]
Xorg.0.log

Hi,

I have the following (rather old) system:

Lenovo Thinkpad T60 with AMD/ATI RV515/M54 [Mobility Radeon X1400] graphics
x86_64-pc-linux-gnu, linux-4.5.0, xorg-server.1.18.3


With xf86-video-ati-7.7.0, on boot into a graphical kde session, I'm getting display distortion on the tty1 console just as the display manager sddm begins to start X. It is quite fleeting so I don't have a screenshot but its as if successive scan lines start with a rapidly increasing offset from the left stretching text and images to illegibility. A moment later the sddm login screen displays and all is well, and the kde session appears to be unaffected. I've attached an Xorg.0.log as well.

The issue is not present in xf86-video-ati-7.6.1. Bisecting from 7.6.1 to 7.7.0, I've determined that the problem is caused by the commit

===
https://cgit.freedesktop.org/xorg/driver/xf86-video-ati/commit/?id=83734317e6bdaeebb4462a63f541e73a1d7c2f77

Update pixmap pitch in radeon_set_pixmap_bo
Stop second guessing it in drmmode_crtc_scanout_create.

Fixes display corruption in some cases with TearFree enabled.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94751

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
===,

and in particular, by the changes in src/drmmode_display.c on lines 97,112, and 401.

Adding some debug printout at line 401, immediately after the drmmode_create_bo_pixmap() call, I get

fbcon->width: 1680, fbcon->height: 1050 fbcon->depth: 24, fbcon->bpp: 32 fbcon->pitch: 6912
   pixmap->devKind: 6720 pixmap->drawable.bitsPerPixel: 32

So, it looks like xorg-server is computing pitch (pixmap->devKind) differently from how it was
computed for fbcon.

It would seem that this could be a problem in xorg-server rather than xf86-video-ati-7.7.0, but
I'm not sure.

The (possibly) relevant code path in from the *pScreen->ModifyPixmapHeader() call w/pitch=-1 in xorg-server is

xorg-server-1.18.3/fb/fb24_32.c
524 Bool
525 fb24_32ModifyPixmapHeader(PixmapPtr pPixmap,
526                           int width,
527                           int height,
528                           int depth,
529                           int bitsPerPixel, int devKind, void *pPixData)
530 {
531     int bpp, w;
532
533     if (!pPixmap)
534         return FALSE;
535     bpp = bitsPerPixel;
536     if (bpp <= 0)
537         bpp = pPixmap->drawable.bitsPerPixel;
538     if (bpp == 24) {
539         if (devKind < 0) {
540             w = width;
541             if (w <= 0)
542                 w = pPixmap->drawable.width;
543             devKind = BitmapBytePad(w * 24);
544         }
545     }
546     return miModifyPixmapHeader(pPixmap, width, height, depth, bitsPerPixel,
547                                 devKind, pPixData);

xorg-server-1.18.3/mi/miscrinit.c
105          * CAVEAT:  Non-SI DDXen may use devKind and devPrivate fields for
106          *          other purposes.
107          */
108         if (devKind > 0)
109             pPixmap->devKind = devKind;
110         else if ((devKind < 0) && ((width > 0) || (depth > 0)))
111             pPixmap->devKind = PixmapBytePad(pPixmap->drawable.width,
112                                              pPixmap->drawable.depth);


xorg-server-1.18.3/include/servermd.h
108 /* The only portable way to get the bpp from the depth is to look it up */
109 #define BitsPerPixel(d) (PixmapWidthPaddingInfo[d].bitsPerPixel)
110
111 #define PixmapWidthInPadUnits(w, d) \
112     (PixmapWidthPaddingInfo[d].notPower2 ? \
113     (((int)(w) * PixmapWidthPaddingInfo[d].bytesPerPixel +  \
114                  PixmapWidthPaddingInfo[d].bytesPerPixel) >> \
115         PixmapWidthPaddingInfo[d].padBytesLog2) : \
116     ((int)((w) + PixmapWidthPaddingInfo[d].padRoundUp) >> \
117         PixmapWidthPaddingInfo[d].padPixelsLog2))
118
119 /*
120  *      Return the number of bytes to which a scanline of the given
121  * depth and width will be padded.
122  */
123 #define PixmapBytePad(w, d) \
124     (PixmapWidthInPadUnits(w, d) << PixmapWidthPaddingInfo[d].padBytesLog2)
125
126 #define BitmapBytePad(w) \
127     (((int)((w) + BITMAP_SCANLINE_PAD - 1) >> LOG2_BITMAP_PAD) << LOG2_BYTES_PER_SCANLINE_PAD)
128
129 #endif                          /* SERVERMD_H */
Comment 1 Michel Dänzer 2016-04-12 09:20:52 UTC
Created attachment 122878 [details] [review]
Explicitly set the fbcon pixmap pitch again

Does this patch fix the problem?
Comment 2 Johannes Hirte 2016-04-12 19:43:38 UTC
(In reply to Michel Dänzer from comment #1)
> Created attachment 122878 [details] [review] [review]
> Explicitly set the fbcon pixmap pitch again
> 
> Does this patch fix the problem?

with this patch I still get the error:

[   12.086479] radeon 0000:00:01.0: evergreen_surface_check_linear_aligned:215 texture pitch 1680 invalid must be aligned with 64
[   12.086491] radeon 0000:00:01.0: evergreen_cs_track_validate_texture:830 texture invalid 0x1a3c3441 0x10000419 0x060a0000 0x00000000 0x00000000 0x8002001a
[   12.086499] [drm:radeon_cs_ioctl] *ERROR* Invalid command stream !


Don't have seen any distortion, but I've not tested much.
Comment 3 jpsinthemix 2016-04-12 20:13:21 UTC
(In reply to Johannes Hirte from comment #2)
> (In reply to Michel Dänzer from comment #1)
> > Created attachment 122878 [details] [review] [review] [review]
> > Explicitly set the fbcon pixmap pitch again
> > 
> > Does this patch fix the problem?
> 
> with this patch I still get the error:
> 
> [   12.086479] radeon 0000:00:01.0:
> evergreen_surface_check_linear_aligned:215 texture pitch 1680 invalid must
> be aligned with 64
> [   12.086491] radeon 0000:00:01.0: evergreen_cs_track_validate_texture:830
> texture invalid 0x1a3c3441 0x10000419 0x060a0000 0x00000000 0x00000000
> 0x8002001a
> [   12.086499] [drm:radeon_cs_ioctl] *ERROR* Invalid command stream !
> 
> 
> Don't have seen any distortion, but I've not tested much.

Yup, looks good. Thanks so much for the speedy response and fix, and for your time.
Comment 4 Michel Dänzer 2016-05-13 03:16:32 UTC
Created attachment 123664 [details] [review]
Explicitly set the fbcon pixmap pitch again

This should be a better fix. Please check that there are no more error messages in dmesg with this one.
Comment 5 jpsinthemix 2016-05-13 07:29:29 UTC
(In reply to Michel Dänzer from comment #4)
> Created attachment 123664 [details] [review] [review]
> Explicitly set the fbcon pixmap pitch again
> 
> This should be a better fix. Please check that there are no more error
> messages in dmesg with this one.

No issues, looks good; thanks again.
Comment 6 Michel Dänzer 2016-05-24 08:06:48 UTC
commit 040a7b80e1fcbaa93ac17f7113d696d9b853cf8a
Author: Michel Dänzer <michel.daenzer@amd.com>
Date:   Tue Apr 12 18:18:43 2016 +0900

    Explicitly set the fbcon pixmap pitch again


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.