[Mesa-dev] [PATCH] st/dri: add 32-bit RGBX/RGBA formats

[Fredrik Höglund]

On Friday 30 June 2017, Rob Herring wrote:
> Add support for 32-bit RGBX/RGBA formats which are required for Android.
> 
> The original patch (commit ccdcf91104a5) was reverted (commit
> c0c6ca40a25e) in mesa as it broke GLX resulting in swapped colors. Based
> on further investigation by Chad Versace, moving the RGBX/RGBA configs
> to the end is enough to prevent breaking GLX.
> 
> Cc: Marek Olšák <marek.olsak at amd.com>
> Cc: Eric Anholt <eric at anholt.net>
> Cc: Chad Versace <chadversary at chromium.org>
> Cc: Mauro Rossi <issor.oruam at gmail.com>
> Signed-off-by: Rob Herring <robh at kernel.org>
> ---
> I've tested only on Android and could use help testing with KDE which 
> broke last time. This has been done on the Intel driver and *should* be 
> okay, but maybe not.

Tested-by: Fredrik Höglund <fredrik at kde.org>

> Mauro, any testing you can do would help. I'm fighting with ADB issues 
> installing the apps you tested...
> 
> Rob
> 
>  src/gallium/state_trackers/dri/dri2.c       |  6 ++++++
>  src/gallium/state_trackers/dri/dri_screen.c | 23 +++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c
> index 60ec38d8e445..4928394306e4 100644
> --- a/src/gallium/state_trackers/dri/dri2.c
> +++ b/src/gallium/state_trackers/dri/dri2.c
> @@ -186,6 +186,9 @@ static enum pipe_format dri2_format_to_pipe_format (int format)
>     case __DRI_IMAGE_FORMAT_ARGB8888:
>        pf = PIPE_FORMAT_BGRA8888_UNORM;
>        break;
> +   case __DRI_IMAGE_FORMAT_XBGR8888:
> +      pf = PIPE_FORMAT_RGBX8888_UNORM;
> +      break;
>     case __DRI_IMAGE_FORMAT_ABGR8888:
>        pf = PIPE_FORMAT_RGBA8888_UNORM;
>        break;
> @@ -434,6 +437,9 @@ dri_image_drawable_get_buffers(struct dri_drawable *drawable,
>        case PIPE_FORMAT_BGRA8888_UNORM:
>           image_format = __DRI_IMAGE_FORMAT_ARGB8888;
>           break;
> +      case PIPE_FORMAT_RGBX8888_UNORM:
> +         image_format = __DRI_IMAGE_FORMAT_XBGR8888;
> +         break;
>        case PIPE_FORMAT_RGBA8888_UNORM:
>           image_format = __DRI_IMAGE_FORMAT_ABGR8888;
>           break;
> diff --git a/src/gallium/state_trackers/dri/dri_screen.c b/src/gallium/state_trackers/dri/dri_screen.c
> index 6b58830e0b42..e3f555561447 100644
> --- a/src/gallium/state_trackers/dri/dri_screen.c
> +++ b/src/gallium/state_trackers/dri/dri_screen.c
> @@ -132,6 +132,27 @@ dri_fill_in_modes(struct dri_screen *screen)
>        MESA_FORMAT_B8G8R8A8_SRGB,
>        MESA_FORMAT_B8G8R8X8_SRGB,
>        MESA_FORMAT_B5G6R5_UNORM,
> +
> +      /* The 32-bit RGBA format must not precede the 32-bit BGRA format.
> +       * Likewise for RGBX and BGRX.  Otherwise, the GLX client and the GLX
> +       * server may disagree on which format the GLXFBConfig represents,
> +       * resulting in swapped color channels.
> +       *
> +       * The problem, as of 2017-05-30:
> +       * When matching a GLXFBConfig to a __DRIconfig, GLX ignores the channel
> +       * order and chooses the first __DRIconfig with the expected channel
> +       * sizes. Specifically, GLX compares the GLXFBConfig's and __DRIconfig's
> +       * __DRI_ATTRIB_{CHANNEL}_SIZE but ignores __DRI_ATTRIB_{CHANNEL}_MASK.
> +       *
> +       * EGL does not suffer from this problem. It correctly compares the
> +       * channel masks when matching EGLConfig to __DRIconfig.
> +       */
> +
> +      /* Required by Android, for HAL_PIXEL_FORMAT_RGBA_8888. */
> +      MESA_FORMAT_R8G8B8A8_UNORM,
> +
> +      /* Required by Android, for HAL_PIXEL_FORMAT_RGBX_8888. */
> +      MESA_FORMAT_R8G8B8X8_UNORM,
>     };
>     static const enum pipe_format pipe_formats[] = {
>        PIPE_FORMAT_BGRA8888_UNORM,
> @@ -139,6 +160,8 @@ dri_fill_in_modes(struct dri_screen *screen)
>        PIPE_FORMAT_BGRA8888_SRGB,
>        PIPE_FORMAT_BGRX8888_SRGB,
>        PIPE_FORMAT_B5G6R5_UNORM,
> +      PIPE_FORMAT_RGBA8888_UNORM,
> +      PIPE_FORMAT_RGBX8888_UNORM,
>     };
>     mesa_format format;
>     __DRIconfig **configs = NULL;
>