Bug 92900

Summary:	[regression bisected] About 700 piglit regressions is what could go wrong
Product:	Mesa	Reporter:	Ian Romanick <idr>
Component:	Drivers/DRI/R100	Assignee:	Default DRI bug account <dri-devel>
Status:	RESOLVED FIXED	QA Contact:	Default DRI bug account <dri-devel>
Severity:	normal
Priority:	medium	CC:	idr, maraeo, sroland
Version:	11.0
Hardware:	x86-64 (AMD64)
OS:	All
Whiteboard:
i915 platform:		i915 features:

Description Ian Romanick 2015-11-11 08:46:26 UTC

About 700 piglit tests regressed on r100.  This includes, but is not limited to:

All 648 subcases of clipflat
fbo-1d
fbo-clearmipmap
fbo-sys-sub-blit
gen-nonzero-unit
...

At least fbo-clearmipmap bisected to the following commit.  I did this bisect using 'git bisect run' with that one test, and I haven't verified all the others yet.

Tested on:

01:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] RV200 [Radeon 7500/7500 LE]


This commit is in Mesa 11.0.

d21320f6258b2e1780a15c1ca718963d8a15ca18 is the first bad commit
commit d21320f6258b2e1780a15c1ca718963d8a15ca18
Author: Roland Scheidegger <sroland@vmware.com>
Date:   Thu Jul 16 03:18:20 2015 +0200

    radeon: fix some potential big endian issues
    
    The formats chosen (both by texture format choser, fbo storage allocation)
    are different for big endian not just for rgba8 but also lower bit width
    formats (why I don't actually know). Even the function to test for renderable
    formats used different formats, however the actual colorbuffer setup did not.
    And the blitter did not take that into account neither.
    Untested (what could possibly go wrong...).
    
    Acked-by: Marek Olšák <marek.olsak@amd.com>

Comment 1 Roland Scheidegger 2015-11-11 14:35:24 UTC

Oops, my bad...
This commit was based on code analysis (and some reports that things don't quite work on big endian) - texturing and backend doing completely different things wrt le/be. But for the LE case I don't really see any actual differences. Except maybe the missing case for MESA_FORMAT_B8G8R8X8_UNORM in blitter setup - the tx_table doesn't contain it because it's never chosen by the texture format chooser (albeit the same blit code still tries to handle it for backend setup, and we don't skip it above, so that's slightly buggy). Maybe we still get this by the winsys buffers? If so adding it to the tx_table might be necessary. I can't actually test this, though...

Comment 2 Marek Olšák 2015-11-11 17:25:23 UTC

If there are no simple way to fix this, the best course of action is to revert the commit.

Comment 3 Roland Scheidegger 2015-11-11 17:53:39 UTC

(In reply to Marek Olšák from comment #2)
> If there are no simple way to fix this, the best course of action is to
> revert the commit.

Well if it's the missing B8G8R8X8 stuff this could be added trivially to the table like so:
[ MESA_FORMAT_B8G8R8X8_UNORM ] = { RADEON_TXFORMAT_ARGB8888, 0 },
[ MESA_FORMAT_X8R8G8B8_UNORM ] = { RADEON_TXFORMAT_ARGB8888, 0 },

Other than that, I really don't see anything which could make a difference on le systems.

Roland

Comment 4 Ian Romanick 2015-11-12 00:07:01 UTC

(In reply to Roland Scheidegger from comment #3)
> (In reply to Marek Olšák from comment #2)
> > If there are no simple way to fix this, the best course of action is to
> > revert the commit.
> 
> Well if it's the missing B8G8R8X8 stuff this could be added trivially to the
> table like so:
> [ MESA_FORMAT_B8G8R8X8_UNORM ] = { RADEON_TXFORMAT_ARGB8888, 0 },
> [ MESA_FORMAT_X8R8G8B8_UNORM ] = { RADEON_TXFORMAT_ARGB8888, 0 },

That fixes the problem here.  If you want to send that as a patch, you can put

Tested-by: Ian Romanick <ian.d.romanick@intel.com>
Cc: "11.0" <mesa-stable@lists.freedesktop.org>

on it.

> Other than that, I really don't see anything which could make a difference
> on le systems.
> 
> Roland

Comment 5 Roland Scheidegger 2015-11-12 19:02:27 UTC

(In reply to Ian Romanick from comment #4)
> 
> That fixes the problem here.  If you want to send that as a patch, you can
> put
> 
> Tested-by: Ian Romanick <ian.d.romanick@intel.com>
> Cc: "11.0" <mesa-stable@lists.freedesktop.org>
> 
> on it.


Thanks for testing! Patches sent.

Comment 6 Roland Scheidegger 2015-11-17 00:12:10 UTC

Fixed by 983614dbede7b94cba1bad9f3e8627fc5e14bb91.
Sorry for that regression...

Comment 7 Ian Romanick 2015-11-17 16:15:30 UTC

(In reply to Roland Scheidegger from comment #6)
> Fixed by 983614dbede7b94cba1bad9f3e8627fc5e14bb91.
> Sorry for that regression...

It happens.  You can CC me on future radeon and r200 patches, and I'll try to test them.

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.