[Mesa-dev] [PATCH] st/mesa: Map packed gallium formats to packed mesa formats.
Oded Gabbay
oded.gabbay at gmail.com
Sun Aug 9 03:14:00 PDT 2015
On Sun, Aug 9, 2015 at 12:56 PM, Marek Olšák <maraeo at gmail.com> wrote:
> He was probably talking about the gallium swrast (softpipe/llvmpipe).
> I don't know of anybody using classic swrast (except classic drivers
> as software fallback).
>
> All Gallium formats are array-based except those which can't be arrays
> like 332, 565, 5551, 4444, 10_10_10_2, 11_11_10, 24_8, 8_24, 32_8_24,
> and oddities like mixed signed/unsigned components in
> R8SG8SB8UX8U_NORM.
>
> The function "is_array" in u_format_parse.py is pretty clear about.
>
> Marek
Hi Marek,
No, I was actually talking about classic swrast. Jason's patches fix
llvmpipe/softpipe, but classic swrast is still broken. I just checked
it for completeness, as I know it is not in use much.
I do think it may be in use (although I'm not sure) in 32-bit power
systems, as they don't support llvm.
Oded
>
> On Sun, Aug 9, 2015 at 1:43 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>> 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.
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list