[Mesa-dev] [PATCH] st/mesa: Map packed gallium formats to packed mesa formats.
Jason Ekstrand
jason at jlekstrand.net
Sat Aug 8 16:43:16 PDT 2015
On Sat, Aug 8, 2015 at 3:14 PM, Oded Gabbay <oded.gabbay at gmail.com> wrote:
> On Sun, Aug 9, 2015 at 12:26 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>> On Sat, Aug 8, 2015 at 2:14 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>>> Am 08.08.2015 um 22:45 schrieb Jason Ekstrand:
>>>> Mesa formats and gallium formats are defined a bit differently. In mesa
>>>> there are "packed" formats which are based on byte-order within a 8, 16, or
>>>> 32-bit word and there are "array" formats which are simply an array of 8,
>>>> 16, or 32-bit values. In gallium, they do something different called
>>>> "plain" which I've never been able to fully understand. However, for those
>>>> formats which mesa defines as "packed", they should match up 1-1 with the
>>>> corresponding "plane" gallium format.
>>> "plain" I guess.
>>> Though plain in gallium really just means the layout and components can
>>> be defined in generic terms.
>>>
>>> FWIW I think gallium's definition of array formats makes more sense. If
>>> all channels are the same width and the channel width is divisible by 8,
>>> then it is an array format. That is kinda obvious, you could easily
>>> address that in c (with byte, short or int members) as an array. The
>>
>> Right. mesa is more explicit; It's either an array format or a packed format.
>>
>> Are you saying that the gallium A8R8G8B8 format is an array format
>> with the same array-based channel ordering on both LE and BE? If so
>> then I have, once again, failed to understand gallium formats and this
>> patch is bogus.
>>
>> Just to be 100% clear MESA_FORMAT_R8G8B8A8 is defined as ((A << 24) |
>> (B << 16) | (G << 8) | (R << 0)).
>>
>>> auto-generated format handling code obviously uses this - if you define
>>> such formats as packed instead the code would fetch all elements at once
>>> and shuffle things into place.
>>> I have no idea what array format means in core mesa, why exactly isn't
>>> the standard rgba8 stuff not an array format? Maybe that comes from some
>>
>> Good question. It certainly could be done either way for 8888-style
>> things. To be honest, I don't really know. Probably some history in
>> there.
>>
>>> OpenGL oddities (the UNSIGNED_INT_8_8_8_8 vs. UNSIGNED_INT_8_8_8_8_REV
>>> stuff is always fun stuff, and one of them will match UNSIGNED_BYTE on
>>> big endian the other on little endian...) which gallium ultimately
>>> doesn't care much about.
>>>
>>> (As for the patch I can't review that, thinking about endianness issues
>>> just gives me a headache.)
>>
>> Me too.
>>
> Same here but I don't have a choice as its my job ;)
>
>>> Roland
>>>
>>>
>>>
>>>>
>>>> Unfortunately, st_format.c was using the endian-dependent wrappers such as
>>>> PIPE_FORMAT_ARGB_8888 that are defined in p_format.h and mapping them to
>>>> MESA_FORMAT_A8R8G8B8. On little-endian systems, this is fine but on
>>>> big-endian systems, it breaks because it introduces one extra byte-swap.
>>>>
>>>> Cc: Brian Paul <brianp at vmware.com>
>>>> Cc: Oded Gabbay <oded.gabbay at gmail.com>
>>>> ---
>>>> src/mesa/state_tracker/st_format.c | 94 +++++++++++++++++++-------------------
>>>> 1 file changed, 47 insertions(+), 47 deletions(-)
>>>>
>>>> diff --git a/src/mesa/state_tracker/st_format.c b/src/mesa/state_tracker/st_format.c
>>>> index db7b5b7..f946f24 100644
>>>> --- a/src/mesa/state_tracker/st_format.c
>>>> +++ b/src/mesa/state_tracker/st_format.c
>>>> @@ -58,21 +58,21 @@ st_mesa_format_to_pipe_format(struct st_context *st, mesa_format mesaFormat)
>>>> {
>>>> switch (mesaFormat) {
>>>> case MESA_FORMAT_A8B8G8R8_UNORM:
>>>> - return PIPE_FORMAT_ABGR8888_UNORM;
>>>> + return PIPE_FORMAT_A8B8G8R8_UNORM;
>>>> case MESA_FORMAT_R8G8B8A8_UNORM:
>>>> - return PIPE_FORMAT_RGBA8888_UNORM;
>>>> + return PIPE_FORMAT_R8G8B8A8_UNORM;
>>>> case MESA_FORMAT_B8G8R8A8_UNORM:
>>>> - return PIPE_FORMAT_BGRA8888_UNORM;
>>>> + return PIPE_FORMAT_B8G8R8A8_UNORM;
>>>> case MESA_FORMAT_A8R8G8B8_UNORM:
>>>> - return PIPE_FORMAT_ARGB8888_UNORM;
>>>> + return PIPE_FORMAT_A8R8G8B8_UNORM;
>>>> case MESA_FORMAT_X8B8G8R8_UNORM:
>>>> - return PIPE_FORMAT_XBGR8888_UNORM;
>>>> + return PIPE_FORMAT_X8B8G8R8_UNORM;
>>>> case MESA_FORMAT_R8G8B8X8_UNORM:
>>>> - return PIPE_FORMAT_RGBX8888_UNORM;
>>>> + return PIPE_FORMAT_R8G8B8X8_UNORM;
>>>> case MESA_FORMAT_B8G8R8X8_UNORM:
>>>> - return PIPE_FORMAT_BGRX8888_UNORM;
>>>> + return PIPE_FORMAT_B8G8R8X8_UNORM;
>>>> case MESA_FORMAT_X8R8G8B8_UNORM:
>>>> - return PIPE_FORMAT_XRGB8888_UNORM;
>>>> + return PIPE_FORMAT_X8R8G8B8_UNORM;
>>>> case MESA_FORMAT_B5G5R5A1_UNORM:
>>>> return PIPE_FORMAT_B5G5R5A1_UNORM;
>>>> case MESA_FORMAT_B4G4R4A4_UNORM:
>>>> @@ -154,13 +154,13 @@ st_mesa_format_to_pipe_format(struct st_context *st, mesa_format mesaFormat)
>>>> case MESA_FORMAT_BGR_SRGB8:
>>>> return PIPE_FORMAT_R8G8B8_SRGB;
>>>> case MESA_FORMAT_A8B8G8R8_SRGB:
>>>> - return PIPE_FORMAT_ABGR8888_SRGB;
>>>> + return PIPE_FORMAT_A8B8G8R8_SRGB;
>>>> case MESA_FORMAT_R8G8B8A8_SRGB:
>>>> - return PIPE_FORMAT_RGBA8888_SRGB;
>>>> + return PIPE_FORMAT_R8G8B8A8_SRGB;
>>>> case MESA_FORMAT_B8G8R8A8_SRGB:
>>>> - return PIPE_FORMAT_BGRA8888_SRGB;
>>>> + return PIPE_FORMAT_B8G8R8A8_SRGB;
>>>> case MESA_FORMAT_A8R8G8B8_SRGB:
>>>> - return PIPE_FORMAT_ARGB8888_SRGB;
>>>> + return PIPE_FORMAT_A8R8G8B8_SRGB;
>>>> case MESA_FORMAT_RGBA_FLOAT32:
>>>> return PIPE_FORMAT_R32G32B32A32_FLOAT;
>>>> case MESA_FORMAT_RGBA_FLOAT16:
>>>> @@ -199,13 +199,13 @@ st_mesa_format_to_pipe_format(struct st_context *st, mesa_format mesaFormat)
>>>> case MESA_FORMAT_R_UNORM16:
>>>> return PIPE_FORMAT_R16_UNORM;
>>>> case MESA_FORMAT_R8G8_UNORM:
>>>> - return PIPE_FORMAT_RG88_UNORM;
>>>> + return PIPE_FORMAT_R8G8_UNORM;
>>>> case MESA_FORMAT_G8R8_UNORM:
>>>> - return PIPE_FORMAT_GR88_UNORM;
>>>> + return PIPE_FORMAT_G8R8_UNORM;
>>>> case MESA_FORMAT_R16G16_UNORM:
>>>> - return PIPE_FORMAT_RG1616_UNORM;
>>>> + return PIPE_FORMAT_R16G16_UNORM;
>>>> case MESA_FORMAT_G16R16_UNORM:
>>>> - return PIPE_FORMAT_GR1616_UNORM;
>>>> + return PIPE_FORMAT_G16R16_UNORM;
>>>> case MESA_FORMAT_RGBA_UNORM16:
>>>> return PIPE_FORMAT_R16G16B16A16_UNORM;
>>>>
>>>> @@ -357,7 +357,7 @@ st_mesa_format_to_pipe_format(struct st_context *st, mesa_format mesaFormat)
>>>> case MESA_FORMAT_G8R8_SNORM:
>>>> return PIPE_FORMAT_GR88_SNORM;
>>>> case MESA_FORMAT_R8G8B8A8_SNORM:
>>>> - return PIPE_FORMAT_RGBA8888_SNORM;
>>>> + return PIPE_FORMAT_R8G8B8A8_SNORM;
>>>> case MESA_FORMAT_A8B8G8R8_SNORM:
>>>> return PIPE_FORMAT_ABGR8888_SNORM;
>>>>
>>>> @@ -476,21 +476,21 @@ mesa_format
>>>> st_pipe_format_to_mesa_format(enum pipe_format format)
>>>> {
>>>> switch (format) {
>>>> - case PIPE_FORMAT_ABGR8888_UNORM:
>>>> + case PIPE_FORMAT_A8B8G8R8_UNORM:
>>>> return MESA_FORMAT_A8B8G8R8_UNORM;
>>>> - case PIPE_FORMAT_RGBA8888_UNORM:
>>>> + case PIPE_FORMAT_R8G8B8A8_UNORM:
>>>> return MESA_FORMAT_R8G8B8A8_UNORM;
>>>> - case PIPE_FORMAT_BGRA8888_UNORM:
>>>> + case PIPE_FORMAT_B8G8R8A8_UNORM:
>>>> return MESA_FORMAT_B8G8R8A8_UNORM;
>>>> - case PIPE_FORMAT_ARGB8888_UNORM:
>>>> + case PIPE_FORMAT_A8R8G8B8_UNORM:
>>>> return MESA_FORMAT_A8R8G8B8_UNORM;
>>>> - case PIPE_FORMAT_XBGR8888_UNORM:
>>>> + case PIPE_FORMAT_X8B8G8R8_UNORM:
>>>> return MESA_FORMAT_X8B8G8R8_UNORM;
>>>> - case PIPE_FORMAT_RGBX8888_UNORM:
>>>> + case PIPE_FORMAT_R8G8B8X8_UNORM:
>>>> return MESA_FORMAT_R8G8B8X8_UNORM;
>>>> - case PIPE_FORMAT_BGRX8888_UNORM:
>>>> + case PIPE_FORMAT_B8G8R8X8_UNORM:
>>>> return MESA_FORMAT_B8G8R8X8_UNORM;
>>>> - case PIPE_FORMAT_XRGB8888_UNORM:
>>>> + case PIPE_FORMAT_X8R8G8B8_UNORM:
>>>> return MESA_FORMAT_X8R8G8B8_UNORM;
>>>> case PIPE_FORMAT_B5G5R5A1_UNORM:
>>>> return MESA_FORMAT_B5G5R5A1_UNORM;
>>>> @@ -506,13 +506,13 @@ st_pipe_format_to_mesa_format(enum pipe_format format)
>>>> return MESA_FORMAT_R10G10B10A2_UNORM;
>>>> case PIPE_FORMAT_L4A4_UNORM:
>>>> return MESA_FORMAT_L4A4_UNORM;
>>>> - case PIPE_FORMAT_LA88_UNORM:
>>>> + case PIPE_FORMAT_L8A8_UNORM:
>>>> return MESA_FORMAT_L8A8_UNORM;
>>>> - case PIPE_FORMAT_AL88_UNORM:
>>>> + case PIPE_FORMAT_A8L8_UNORM:
>>>> return MESA_FORMAT_A8L8_UNORM;
>>>> - case PIPE_FORMAT_LA1616_UNORM:
>>>> + case PIPE_FORMAT_L16A16_UNORM:
>>>> return MESA_FORMAT_L16A16_UNORM;
>>>> - case PIPE_FORMAT_AL1616_UNORM:
>>>> + case PIPE_FORMAT_A16L16_UNORM:
>>>> return MESA_FORMAT_A16L16_UNORM;
>>>> case PIPE_FORMAT_A8_UNORM:
>>>> return MESA_FORMAT_A_UNORM8;
>>>> @@ -570,21 +570,21 @@ st_pipe_format_to_mesa_format(enum pipe_format format)
>>>> return MESA_FORMAT_SRGBA_DXT3;
>>>> case PIPE_FORMAT_DXT5_SRGBA:
>>>> return MESA_FORMAT_SRGBA_DXT5;
>>>> - case PIPE_FORMAT_LA88_SRGB:
>>>> + case PIPE_FORMAT_L8A8_SRGB:
>>>> return MESA_FORMAT_L8A8_SRGB;
>>>> - case PIPE_FORMAT_AL88_SRGB:
>>>> + case PIPE_FORMAT_A8L8_SRGB:
>>>> return MESA_FORMAT_A8L8_SRGB;
>>>> case PIPE_FORMAT_L8_SRGB:
>>>> return MESA_FORMAT_L_SRGB8;
>>>> case PIPE_FORMAT_R8G8B8_SRGB:
>>>> return MESA_FORMAT_BGR_SRGB8;
>>>> - case PIPE_FORMAT_ABGR8888_SRGB:
>>>> + case PIPE_FORMAT_A8B8G8R8_SRGB:
>>>> return MESA_FORMAT_A8B8G8R8_SRGB;
>>>> - case PIPE_FORMAT_RGBA8888_SRGB:
>>>> + case PIPE_FORMAT_R8G8B8A8_SRGB:
>>>> return MESA_FORMAT_R8G8B8A8_SRGB;
>>>> - case PIPE_FORMAT_BGRA8888_SRGB:
>>>> + case PIPE_FORMAT_B8G8R8A8_SRGB:
>>>> return MESA_FORMAT_B8G8R8A8_SRGB;
>>>> - case PIPE_FORMAT_ARGB8888_SRGB:
>>>> + case PIPE_FORMAT_A8R8G8B8_SRGB:
>>>> return MESA_FORMAT_A8R8G8B8_SRGB;
>>>> case PIPE_FORMAT_R32G32B32A32_FLOAT:
>>>> return MESA_FORMAT_RGBA_FLOAT32;
>>>> @@ -623,13 +623,13 @@ st_pipe_format_to_mesa_format(enum pipe_format format)
>>>> return MESA_FORMAT_R_UNORM8;
>>>> case PIPE_FORMAT_R16_UNORM:
>>>> return MESA_FORMAT_R_UNORM16;
>>>> - case PIPE_FORMAT_RG88_UNORM:
>>>> + case PIPE_FORMAT_R8G8_UNORM:
>>>> return MESA_FORMAT_R8G8_UNORM;
>>>> - case PIPE_FORMAT_GR88_UNORM:
>>>> + case PIPE_FORMAT_G8R8_UNORM:
>>>> return MESA_FORMAT_G8R8_UNORM;
>>>> - case PIPE_FORMAT_RG1616_UNORM:
>>>> + case PIPE_FORMAT_R16G16_UNORM:
>>>> return MESA_FORMAT_R16G16_UNORM;
>>>> - case PIPE_FORMAT_GR1616_UNORM:
>>>> + case PIPE_FORMAT_G16R16_UNORM:
>>>> return MESA_FORMAT_G16R16_UNORM;
>>>>
>>>> case PIPE_FORMAT_A8_UINT:
>>>> @@ -772,31 +772,31 @@ st_pipe_format_to_mesa_format(enum pipe_format format)
>>>> /* signed normalized formats */
>>>> case PIPE_FORMAT_R8_SNORM:
>>>> return MESA_FORMAT_R_SNORM8;
>>>> - case PIPE_FORMAT_RG88_SNORM:
>>>> + case PIPE_FORMAT_R8G8_SNORM:
>>>> return MESA_FORMAT_R8G8_SNORM;
>>>> - case PIPE_FORMAT_GR88_SNORM:
>>>> + case PIPE_FORMAT_G8R8_SNORM:
>>>> return MESA_FORMAT_G8R8_SNORM;
>>>> - case PIPE_FORMAT_RGBA8888_SNORM:
>>>> + case PIPE_FORMAT_R8G8B8A8_SNORM:
>>>> return MESA_FORMAT_R8G8B8A8_SNORM;
>>>> - case PIPE_FORMAT_ABGR8888_SNORM:
>>>> + case PIPE_FORMAT_A8B8G8R8_SNORM:
>>>> return MESA_FORMAT_A8B8G8R8_SNORM;
>>>>
>>>> case PIPE_FORMAT_A8_SNORM:
>>>> return MESA_FORMAT_A_SNORM8;
>>>> case PIPE_FORMAT_L8_SNORM:
>>>> return MESA_FORMAT_L_SNORM8;
>>>> - case PIPE_FORMAT_LA88_SNORM:
>>>> + case PIPE_FORMAT_L8A8_SNORM:
>>>> return MESA_FORMAT_L8A8_SNORM;
>>>> - case PIPE_FORMAT_AL88_SNORM:
>>>> + case PIPE_FORMAT_A8L8_SNORM:
>>>> return MESA_FORMAT_A8L8_SNORM;
>>>> case PIPE_FORMAT_I8_SNORM:
>>>> return MESA_FORMAT_I_SNORM8;
>>>>
>>>> case PIPE_FORMAT_R16_SNORM:
>>>> return MESA_FORMAT_R_SNORM16;
>>>> - case PIPE_FORMAT_RG1616_SNORM:
>>>> + case PIPE_FORMAT_R16G16_SNORM:
>>>> return MESA_FORMAT_R16G16_SNORM;
>>>> - case PIPE_FORMAT_GR1616_SNORM:
>>>> + case PIPE_FORMAT_G16R16_SNORM:
>>>> return MESA_FORMAT_G16R16_SNORM;
>>>> case PIPE_FORMAT_R16G16B16A16_SNORM:
>>>> return MESA_FORMAT_RGBA_SNORM16;
>>>>
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> If I understand your patch, instead of using the endian-dependent
> defines, you use the LE defines all the time when converting
> pipe<-->mesa formats
>
> I'm not saying its wrong, but these endian-dependent defines, e.g.
> PIPE_FORMAT_ARGB8888_UNORM, are used in more places in the gallium
> code than in the above two functions. Why change only here and not in
> all places, i.e removing them ?
>
> Also, this fixes only llvmpipe/softpipe. It doesn't fix sw_rast. That
> is still broken
The fact that sw_rast is broken bothers me. The sw_rast code doesn't
leave core mesa so it shouldn't be affected by any gallium changes.
>From that perspective, this makes sense. However, sw_rast should be
using the auto-generated packing/unpacking functions in core mesa.
Did sw_rast work prior to the readpixels changes? It's possible it
was already broken.
> I tested this patch on llvmpipe, combined with the previous patch you
> sent on POWER8 machine running ppc64 (BE)
> First of all, as I said above, it fixes the piglit sanity test.
> Second, it seems that it improves piglit gpu.py results much more than
> my patch. "Only" 1368 tests failed, vs. a bit more than 2000 with my
> patch. Still a lot higher than the ~600 tests that fail on ppc64le
I'm not surprised that it didn't get to zero regressions vs. ppc64le.
This patch was just kind of thrown together and I didn't check all of
the formats. Also, as per my conversation with rolland, I may be
misunderstanding the PIPE_FORMAT enums.
Really, this is a problem that is going to have to be chased by
someone other than me. I have no BE hardware, so I can't really chase
it down. I I also have very limited gallium knowledge so I'm probably
not the best person to start chasing it through gallium. All I've
done so far is throw darts at the wall. I'm sure this patch is
incomplete and, to be honest, I didn't even compile-test it as I don't
usually build gallium. What i do know is that, for the most part, the
core mesa code is consistent with itself as far as endianness goes.
If you want to see what a given format means in a more explicit
fashion, just look at the auto-generated format_pack.c and
format_unpack.c files. gallium has similar auto-generated format
packing functions. If you can digest what each of them means on BE,
then it should be pretty obvious how to match things up.
--Jason
P.S. Thanks for bringing this up and taking the time to look into it.
Mesa claims to work on BE but, as you've clearly demonstrated, it's
completely broken. I'm sure we'd all like to see it working again.
More information about the mesa-dev
mailing list