Bug 66663 - broken on big-endian systems
Summary: broken on big-endian systems
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/Radeon (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: xf86-video-ati maintainers
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-07 12:12 UTC by Mark Kettenis
Modified: 2013-09-10 19:35 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
patch (1.58 KB, text/plain)
2013-07-07 12:12 UTC, Mark Kettenis
no flags Details
patch that fixes the accelerated case (1.08 KB, patch)
2013-07-08 18:27 UTC, Mark Kettenis
no flags Details | Splinter Review

Description Mark Kettenis 2013-07-07 12:12:42 UTC
Created attachment 82134 [details]
patch

The current xf86-video-ati is broken on big-endian systems.  The problem is that
RADEON_TILING_SURFACE isn't set on the front buffer since commits ef9bfb262db7004bef3704e5d914687e50d3fca4 and e5bd99faa3b6629a55168386d5dfa936ee4e97ae.

As a result the RADEON_TILING_SWAP_32BIT and RADEON_TILING_SWAP_16BIT flags don't have any effect, and any direct access to the front buffer will not do the necessary byte swapping.  The effect is blatantly obvious if you disable acceleration (option NoAccel "on"), but also happens with (EXA) acceleration enabled.  With acceleration disabled, the colors are all wrong.  With acceleration enabled, the background of my xterms is yellow when it should be white.

The attached patch fixes this, but might reintroduce problems with page flipping as mentioned in https://bugs.freedesktop.org/show_bug.cgi?id=33738 (the bug report that seems to have triggered the removal of the RADEON_TILING_SURFACE flag for the front buffer).  But only on big-endian platforms.
Comment 1 Michel Dänzer 2013-07-08 12:01:50 UTC
Have you tried completely disallowing direct CPU access to the scanout buffer instead? Seems like that should fix this problem without reintroducing the other one.
Comment 2 Mark Kettenis 2013-07-08 18:27:22 UTC
Created attachment 82197 [details] [review]
patch that fixes the accelerated case
Comment 3 Mark Kettenis 2013-07-08 18:41:02 UTC
Looks like I jumped to conclusions here.  For the (EXA) accelerated case, the
problem is that UTS/DFS get bypassed for the "screen pixmap".  That was the right
thing to do when the front buffer had a surface reg allocated to it that made sure
the required byte swapping was done in hardware.  But now that this isn't the case anymore, all pixmaps over 8bpp need byte swapping.  See the attached patch.

Of course this will leave the unaccelerated case broken.  In that case the RADEON_TILING_SURFACE flag really has to be set.  But there should be no interference with page flipping in that case.
Comment 4 Michel Dänzer 2013-07-10 09:06:45 UTC
Patch pushed, thanks.

(In reply to comment #3)
> Of course this will leave the unaccelerated case broken.  In that case the
> RADEON_TILING_SURFACE flag really has to be set.  But there should be no
> interference with page flipping in that case.

Right. Or, the shadow update hook could swap the bytes. Do you want to look into fixing the unaccelerated case as well?
Comment 5 Alex Deucher 2013-07-10 13:03:21 UTC
We only need swapping for r1xx-r5xx asics.  For r6xx+, the display hardware already handles the swapping.
Comment 6 Mark Kettenis 2013-07-10 15:56:30 UTC
Michel, thanks.  Yes I will look at fixing the shadowfb case as well.

Alex, when you say that for r6xx the display hardware already handles the swapping, are you referring to the unaccelerated case?  Or are you saying my change to RADEONPrepareAccess_CS() is wrong?
Comment 7 Alex Deucher 2013-07-10 16:04:21 UTC
(In reply to comment #6)
> Alex, when you say that for r6xx the display hardware already handles the
> swapping, are you referring to the unaccelerated case?  Or are you saying my
> change to RADEONPrepareAccess_CS() is wrong?

For unaccelerated.  RADEONPrepareAccess_CS() only applies to r1xx-r5xx GPUs.  R6xx and newer asics have their own PrepareAccess functions.
Comment 8 Mark Kettenis 2013-09-10 19:35:40 UTC
shadowfb has been fixed now as well


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.