[Mesa-dev] [PATCH v3 3/4] r600g: use do_endian_swap in color swapping functions

[Marek Olšák]

On Thu, Apr 21, 2016 at 4:39 PM, Oded Gabbay <oded.gabbay at gmail.com> wrote:
> For some formats we need to take "do_endian_swap" into account when
> configuring swapping for color buffers.
>
> Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
> ---
>  src/gallium/drivers/r600/r600_state_common.c |  2 +-
>  src/gallium/drivers/radeon/r600_texture.c    | 20 +++++++++++++-------
>  2 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/src/gallium/drivers/r600/r600_state_common.c b/src/gallium/drivers/r600/r600_state_common.c
> index 1d44ef0..fea2bcb 100644
> --- a/src/gallium/drivers/r600/r600_state_common.c
> +++ b/src/gallium/drivers/r600/r600_state_common.c
> @@ -2639,7 +2639,7 @@ uint32_t r600_translate_colorformat(enum chip_class chip, enum pipe_format forma
>                                         return V_0280A0_COLOR_32_32;
>                         }
>                 } else if (HAS_SIZE(8,24,0,0)) {
> -                       return V_0280A0_COLOR_24_8;
> +                       return (do_endian_swap ? V_0280A0_COLOR_8_24 : V_0280A0_COLOR_24_8);
>                 } else if (HAS_SIZE(24,8,0,0)) {
>                         return V_0280A0_COLOR_8_24;
>                 }
> diff --git a/src/gallium/drivers/radeon/r600_texture.c b/src/gallium/drivers/radeon/r600_texture.c
> index d446061..ead4dd7 100644
> --- a/src/gallium/drivers/radeon/r600_texture.c
> +++ b/src/gallium/drivers/radeon/r600_texture.c
> @@ -1419,7 +1419,8 @@ unsigned r600_translate_colorswap(enum pipe_format format, bool do_endian_swap)
>                 else if ((HAS_SWIZZLE(0,Y) && HAS_SWIZZLE(1,X)) ||
>                          (HAS_SWIZZLE(0,Y) && HAS_SWIZZLE(1,NONE)) ||
>                          (HAS_SWIZZLE(0,NONE) && HAS_SWIZZLE(1,X)))
> -                       return V_0280A0_SWAP_STD_REV; /* YX__ */
> +                       /* YX__ */
> +                       return (do_endian_swap ? V_0280A0_SWAP_STD : V_0280A0_SWAP_STD_REV);
>                 else if (HAS_SWIZZLE(0,X) && HAS_SWIZZLE(3,Y))
>                         return V_0280A0_SWAP_ALT; /* X__Y */
>                 else if (HAS_SWIZZLE(0,Y) && HAS_SWIZZLE(3,X))
> @@ -1427,20 +1428,25 @@ unsigned r600_translate_colorswap(enum pipe_format format, bool do_endian_swap)
>                 break;
>         case 3:
>                 if (HAS_SWIZZLE(0,X))
> -                       return V_0280A0_SWAP_STD; /* XYZ */
> +                       return (do_endian_swap ? V_0280A0_SWAP_STD_REV : V_0280A0_SWAP_STD);
>                 else if (HAS_SWIZZLE(0,Z))
>                         return V_0280A0_SWAP_STD_REV; /* ZYX */
>                 break;
>         case 4:
>                 /* check the middle channels, the 1st and 4th channel can be NONE */
> -               if (HAS_SWIZZLE(1,Y) && HAS_SWIZZLE(2,Z))
> +               if (HAS_SWIZZLE(1,Y) && HAS_SWIZZLE(2,Z)) {
>                         return V_0280A0_SWAP_STD; /* XYZW */
> -               else if (HAS_SWIZZLE(1,Z) && HAS_SWIZZLE(2,Y))
> +               } else if (HAS_SWIZZLE(1,Z) && HAS_SWIZZLE(2,Y)) {
>                         return V_0280A0_SWAP_STD_REV; /* WZYX */
> -               else if (HAS_SWIZZLE(1,Y) && HAS_SWIZZLE(2,X))
> +               } else if (HAS_SWIZZLE(1,Y) && HAS_SWIZZLE(2,X)) {
>                         return V_0280A0_SWAP_ALT; /* ZYXW */

These braces don't have to be there.

I don't understand the logic behind this patch, but I'm okay with it
if nobody else comments.

Marek