[Mesa-dev] [PATCH] st/mesa: Map packed gallium formats to packed mesa formats.

Oded Gabbay oded.gabbay at gmail.com
Sat Aug 8 15:14:13 PDT 2015


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

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 also run it on ppc64le to make sure there wasn't any regression.

    Oded


More information about the mesa-dev mailing list