[Mesa-dev] [PATCH] r600g: Fix ARB_texture_rgb10_a2ui support in big-endian

[Oded Gabbay]

On Mon, Feb 29, 2016 at 11:33 AM, Oded Gabbay <oded.gabbay at gmail.com> wrote:
>
> On Mon, Feb 29, 2016 at 7:44 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> > On Mon, Feb 29, 2016 at 12:31 AM, Oded Gabbay <oded.gabbay at gmail.com> wrote:
> >> On Mon, Feb 29, 2016 at 3:51 AM, Michel Dänzer <michel at daenzer.net> wrote:
> >>> On 28.02.2016 05:48, Oded Gabbay wrote:
> >>>> This patch enables the correct detection of PIPE_FORMAT_R10G10B10A2_UINT
> >>>> and PIPE_FORMAT_B10G10R10A2_UINT formats in r600g in big-endian mode, by
> >>>> adjusting the order of channels in various functions.
> >>>>
> >>>> This enables support for ARB_texture_rgb10_a2ui, which otherwise is not
> >>>> detected as supported.
> >>>>
> >>>> Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
> >>>> ---
> >>>>  src/gallium/drivers/r600/r600_state_common.c | 15 ++++++++++-----
> >>>>  1 file changed, 10 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/src/gallium/drivers/r600/r600_state_common.c b/src/gallium/drivers/r600/r600_state_common.c
> >>>> index b231d1e..b02b6a9 100644
> >>>> --- a/src/gallium/drivers/r600/r600_state_common.c
> >>>> +++ b/src/gallium/drivers/r600/r600_state_common.c
> >>>> @@ -2468,10 +2468,13 @@ uint32_t r600_translate_texformat(struct pipe_screen *screen,
> >>>>                               result = FMT_1_5_5_5;
> >>>>                               goto out_word4;
> >>>>                       }
> >>>> -                     if (desc->channel[0].size == 10 &&
> >>>> -                         desc->channel[1].size == 10 &&
> >>>> -                         desc->channel[2].size == 10 &&
> >>>> -                         desc->channel[3].size == 2) {
> >>>> +                     if ((desc->channel[1].size == 10 &&
> >>>> +                               desc->channel[2].size == 10) &&
> >>>> +                                     ((desc->channel[0].size == 10 &&
> >>>> +                                       desc->channel[3].size == 2) ||
> >>>> +                                      (R600_BIG_ENDIAN &&
> >>>> +                                       desc->channel[0].size == 2 &&
> >>>> +                                       desc->channel[3].size == 10))) {
> >>>>                               result = FMT_2_10_10_10;
> >>>>                               goto out_word4;
> >>>>                       }
> >>>> @@ -2694,9 +2697,11 @@ uint32_t r600_translate_colorformat(enum chip_class chip, enum pipe_format forma
> >>>>                       }
> >>>>               } else if (HAS_SIZE(5,5,5,1)) {
> >>>>                       return V_0280A0_COLOR_1_5_5_5;
> >>>> -             } else if (HAS_SIZE(10,10,10,2)) {
> >>>> +             } else if (HAS_SIZE(10,10,10,2) ||
> >>>> +                             (R600_BIG_ENDIAN && HAS_SIZE(2,10,10,10))) {
> >>>
> >>> Since the components of these formats cross byte boundaries, these
> >>> formats should really be treated as packed by the core Gallium code, in
> >>> which case this change probably wouldn't be necessary. In other words,
> >>> this is probably working around a core Gallium bug.
> >>>
> >>
> >> so ?
> >
> > I think the point here is that RGB10A2 in LE != A2BGR10 in BE. They're
> > both packed integer formats, and the byteswapping that happens between
> > LE/BE does not happen on component boundaries. So there's no way this
> > works, and if it does, it's purely by luck. Instead of breaking this
> > spot, fix the other spot where it's currently broken.
> >
> >   -ilia
>
> Thanks Michel & Ilia,
>
> I'll investigate it and get back with a new patch.
>
> Oded

Indeed, my previous fix didn't work when I checked it with texwrap
piglit test. However, I found out that's because I didn't use the
correct color format to configure the H/W, because as you told me, the
byteswapping can't be the same because it doesn't happen on component
boundaries.

I'm sending now a revised patch.

Oded