[Mesa-dev] [PATCH 2/3] mesa/st: Add Gallium support for EXT_texture_sRGB_R8

Gert Wollny gert.wollny at collabora.com
Mon Oct 29 09:45:24 UTC 2018


Am Montag, den 29.10.2018, 04:27 -0400 schrieb Ilia Mirkin:
> On Mon, Oct 29, 2018 at 3:41 AM Gert Wollny <gert.wollny at collabora.co
> m> wrote:
> > 
> > This only adds support on the Gallium core level, for the drivers
> > it is likely that additional changes are needed to support the
> > new texture format.
> > 
> > Enables on softpipe and makes pass:
> >   dEQP-GLES31.functional.srgb_texture_decode.skip_decode.sr8.*
> > 
> > v2: - add include for getting GL_SR8_EXT
> >     - add mapping to linear format for PIPE_FORMATR_R8_SRGB
> > v3: - Add texture format to svga format table since otherwise
> > building
> >       mesa will fail when this driver is enabled. It was not tested
> >       whether the extension actually works.
> > 
> > Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> > ---
> >  src/gallium/auxiliary/util/u_format.csv               |  1 +
> >  src/gallium/auxiliary/util/u_format.h                 |  4 ++++
> >  src/gallium/auxiliary/util/u_format_tests.c           |  4 ++++
> >  src/gallium/drivers/svga/include/svga3d_devcaps.h     |  1 +
> >  src/gallium/drivers/svga/include/svga3d_surfacedefs.h |  5 +++++
> >  src/gallium/drivers/svga/include/svga3d_types.h       |  2 +-
> >  src/gallium/drivers/svga/svga_format.c                |  7 +++++++
> 
> I don't think you can just do this for adding the SVGA3D formats...
> enabling support should be part of a driver-specific change. You
> should add it to the format_conversion table in svga_format probably,
> but nothing else.

Thanks for your comments, I probably started trying to fix compiling
form the wrong end, so that I added some cruft. I'll correct the
patches accordingly.  

BTW: Since you already did a review of the EXT_sRGB_write_control
patches [1], may I kindly ask you to take another look and maybe give
your R-B if you think they are fine (in the end I was able to get virgl
suppport for it without a new cap)

Many thanks, 
Gert

[1] https://patchwork.freedesktop.org/series/51130/


> >  src/gallium/include/pipe/p_format.h                   |  2 ++
> >  src/mesa/state_tracker/st_extensions.c                |  4 ++++
> >  src/mesa/state_tracker/st_format.c                    | 10
> > ++++++++++
> >  10 files changed, 39 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/gallium/auxiliary/util/u_format.csv
> > b/src/gallium/auxiliary/util/u_format.csv
> > index f9e4925f27..911ac07d32 100644
> > --- a/src/gallium/auxiliary/util/u_format.csv
> > +++ b/src/gallium/auxiliary/util/u_format.csv
> > @@ -114,6 +114,7 @@ PIPE_FORMAT_I32_FLOAT             , plain, 1,
> > 1, f32 ,     ,     ,     , xxxx, r
> > 
> >  # SRGB formats
> >  PIPE_FORMAT_L8_SRGB               , plain, 1, 1, un8
> > ,     ,     ,     , xxx1, srgb
> > +PIPE_FORMAT_R8_SRGB               , plain, 1, 1, un8
> > ,     ,     ,     , x001, srgb
> >  PIPE_FORMAT_L8A8_SRGB             , plain, 1, 1, un8 , un8
> > ,     ,     , xxxy, srgb
> >  PIPE_FORMAT_R8G8B8_SRGB           , plain, 1, 1, un8 , un8 , un8
> > ,     , xyz1, srgb
> >  PIPE_FORMAT_R8G8B8A8_SRGB         , plain, 1, 1, un8 , un8 , un8 ,
> > un8 , xyzw, srgb
> > diff --git a/src/gallium/auxiliary/util/u_format.h
> > b/src/gallium/auxiliary/util/u_format.h
> > index e66849c16b..5bcfc1f115 100644
> > --- a/src/gallium/auxiliary/util/u_format.h
> > +++ b/src/gallium/auxiliary/util/u_format.h
> > @@ -925,6 +925,8 @@ util_format_srgb(enum pipe_format format)
> >     switch (format) {
> >     case PIPE_FORMAT_L8_UNORM:
> >        return PIPE_FORMAT_L8_SRGB;
> > +   case PIPE_FORMAT_R8_UNORM:
> > +      return PIPE_FORMAT_R8_SRGB;
> >     case PIPE_FORMAT_L8A8_UNORM:
> >        return PIPE_FORMAT_L8A8_SRGB;
> >     case PIPE_FORMAT_R8G8B8_UNORM:
> > @@ -1001,6 +1003,8 @@ util_format_linear(enum pipe_format format)
> >     switch (format) {
> >     case PIPE_FORMAT_L8_SRGB:
> >        return PIPE_FORMAT_L8_UNORM;
> > +   case PIPE_FORMAT_R8_SRGB:
> > +      return PIPE_FORMAT_R8_UNORM;
> >     case PIPE_FORMAT_L8A8_SRGB:
> >        return PIPE_FORMAT_L8A8_UNORM;
> >     case PIPE_FORMAT_R8G8B8_SRGB:
> > diff --git a/src/gallium/auxiliary/util/u_format_tests.c
> > b/src/gallium/auxiliary/util/u_format_tests.c
> > index 9c9a5838d1..dee52533c1 100644
> > --- a/src/gallium/auxiliary/util/u_format_tests.c
> > +++ b/src/gallium/auxiliary/util/u_format_tests.c
> > @@ -236,6 +236,10 @@ util_format_test_cases[] =
> >     {PIPE_FORMAT_L8_SRGB, PACKED_1x8(0xff), PACKED_1x8(0xbc),
> > UNPACKED_1x1(0.502886458033, 0.502886458033, 0.502886458033, 1.0)},
> >     {PIPE_FORMAT_L8_SRGB, PACKED_1x8(0xff), PACKED_1x8(0xff),
> > UNPACKED_1x1(1.0, 1.0, 1.0, 1.0)},
> > 
> > +   {PIPE_FORMAT_R8_SRGB, PACKED_1x8(0xff), PACKED_1x8(0x00),
> > UNPACKED_1x1(0.0, 0.0, 0.0, 1.0)},
> > +   {PIPE_FORMAT_R8_SRGB, PACKED_1x8(0xff), PACKED_1x8(0xbc),
> > UNPACKED_1x1(0.502886458033, 0.0, 0.0, 1.0)},
> > +   {PIPE_FORMAT_R8_SRGB, PACKED_1x8(0xff), PACKED_1x8(0xff),
> > UNPACKED_1x1(1.0, 0.0, 0.0, 1.0)},
> > +
> >     {PIPE_FORMAT_L8A8_SRGB, PACKED_1x16(0xffff),
> > PACKED_1x16(0x0000), UNPACKED_1x1(0.0, 0.0, 0.0, 0.0)},
> >     {PIPE_FORMAT_L8A8_SRGB, PACKED_1x16(0xffff),
> > PACKED_1x16(0x00bc), UNPACKED_1x1(0.502886458033, 0.502886458033,
> > 0.502886458033, 0.0)},
> >     {PIPE_FORMAT_L8A8_SRGB, PACKED_1x16(0xffff),
> > PACKED_1x16(0x00ff), UNPACKED_1x1(1.0, 1.0, 1.0, 0.0)},
> > diff --git a/src/gallium/drivers/svga/include/svga3d_devcaps.h
> > b/src/gallium/drivers/svga/include/svga3d_devcaps.h
> > index a519198b64..996fe23952 100644
> > --- a/src/gallium/drivers/svga/include/svga3d_devcaps.h
> > +++ b/src/gallium/drivers/svga/include/svga3d_devcaps.h
> > @@ -437,6 +437,7 @@ typedef enum {
> > 
> >     SVGA3D_DEVCAP_MULTISAMPLE_2X                    = 245,
> >     SVGA3D_DEVCAP_MULTISAMPLE_4X                    = 246,
> > +   SVGA3D_DEVCAP_DXFMT_R8_UNORM_SRGB               = 247,
> > 
> >     SVGA3D_DEVCAP_MAX                       /* This must be the
> > last index. */
> >  } SVGA3dDevCapIndex;
> > diff --git a/src/gallium/drivers/svga/include/svga3d_surfacedefs.h
> > b/src/gallium/drivers/svga/include/svga3d_surfacedefs.h
> > index 9054e72207..01e09c31f4 100644
> > --- a/src/gallium/drivers/svga/include/svga3d_surfacedefs.h
> > +++ b/src/gallium/drivers/svga/include/svga3d_surfacedefs.h
> > @@ -900,6 +900,11 @@ static const struct svga3d_surface_desc
> > svga3d_surface_descs[] = {
> >        {4, 4, 1},  16, 16,
> >        128, {{0}, {0}, {128}, {0}},
> >        {{0}, {0}, {0}, {0}}},
> > +   {SVGA3D_R8_UNORM_SRGB, SVGA3DBLOCKDESC_RED,
> > +      {1, 1, 1},  1, 1,
> > +      8, {{0}, {0}, {8}, {0}},
> > +      {{0}, {0}, {0}, {0}}},
> > +
> >  };
> > 
> > 
> > diff --git a/src/gallium/drivers/svga/include/svga3d_types.h
> > b/src/gallium/drivers/svga/include/svga3d_types.h
> > index 6bb99e1cb7..05cafeb1dc 100644
> > --- a/src/gallium/drivers/svga/include/svga3d_types.h
> > +++ b/src/gallium/drivers/svga/include/svga3d_types.h
> > @@ -339,7 +339,7 @@ typedef enum SVGA3dSurfaceFormat {
> >     SVGA3D_B8G8R8X8_UNORM               = 142,
> >     SVGA3D_BC4_UNORM                    = 143,
> >     SVGA3D_BC5_UNORM                    = 144,
> > -
> > +   SVGA3D_R8_UNORM_SRGB                = 145,
> >     SVGA3D_FORMAT_MAX
> >  } SVGA3dSurfaceFormat;
> > 
> > diff --git a/src/gallium/drivers/svga/svga_format.c
> > b/src/gallium/drivers/svga/svga_format.c
> > index 9f6a618706..b4beb5d27f 100644
> > --- a/src/gallium/drivers/svga/svga_format.c
> > +++ b/src/gallium/drivers/svga/svga_format.c
> > @@ -370,6 +370,7 @@ static const struct vgpu10_format_entry
> > format_conversion_table[] =
> >     {
> > PIPE_FORMAT_A1B5G5R5_UNORM,        SVGA3D_FORMAT_INVALID,      SVGA
> > 3D_FORMAT_INVALID,       SVGA3D_FORMAT_INVALID,       0 },
> >     {
> > PIPE_FORMAT_X1B5G5R5_UNORM,        SVGA3D_FORMAT_INVALID,      SVGA
> > 3D_FORMAT_INVALID,       SVGA3D_FORMAT_INVALID,       0 },
> >     {
> > PIPE_FORMAT_A4B4G4R4_UNORM,        SVGA3D_FORMAT_INVALID,      SVGA
> > 3D_FORMAT_INVALID,       SVGA3D_FORMAT_INVALID,       0 },
> > +   {
> > PIPE_FORMAT_R8_SRGB,               SVGA3D_FORMAT_INVALID,      SVGA
> > 3D_FORMAT_INVALID,       SVGA3D_FORMAT_INVALID,       0 },
> 
> This table is indexed by format,
> 
> >  };
> > 
> > 
> > @@ -1520,6 +1521,12 @@ static const struct format_cap
> > format_cap_table[] = {
> >       SVGA3D_BC5_UNORM,
> >       SVGA3D_DEVCAP_DXFMT_BC5_UNORM,
> >       4, 4, 16, 0
> > +   },
> > +   {
> > +       "SVGA3D_R8_UNORM_SRGB",
> > +       SVGA3D_R8_UNORM_SRGB,
> > +       SVGA3D_DEVCAP_DXFMT_R8_UNORM_SRGB,
> > +       1, 1, 1, 0
> >     }
> >  };
> > 
> > diff --git a/src/gallium/include/pipe/p_format.h
> > b/src/gallium/include/pipe/p_format.h
> > index 57399800fa..6fb91222f2 100644
> > --- a/src/gallium/include/pipe/p_format.h
> > +++ b/src/gallium/include/pipe/p_format.h
> > @@ -396,6 +396,8 @@ enum pipe_format {
> >     PIPE_FORMAT_X1B5G5R5_UNORM          = 310,
> >     PIPE_FORMAT_A4B4G4R4_UNORM          = 311,
> > 
> > +   PIPE_FORMAT_R8_SRGB                 = 312,
> > +
> >     PIPE_FORMAT_COUNT
> >  };
> > 
> > diff --git a/src/mesa/state_tracker/st_extensions.c
> > b/src/mesa/state_tracker/st_extensions.c
> > index 798ee60875..16889074f6 100644
> > --- a/src/mesa/state_tracker/st_extensions.c
> > +++ b/src/mesa/state_tracker/st_extensions.c
> > @@ -889,6 +889,10 @@ void st_init_extensions(struct pipe_screen
> > *screen,
> >           PIPE_FORMAT_R8G8B8A8_SRGB},
> >          GL_TRUE }, /* at least one format must be supported */
> > 
> > +      { { o(EXT_texture_sRGB_R8) },
> > +        { PIPE_FORMAT_R8_SRGB },
> > +        GL_TRUE },
> > +
> >        { { o(EXT_texture_type_2_10_10_10_REV) },
> >          { PIPE_FORMAT_R10G10B10A2_UNORM,
> >            PIPE_FORMAT_B10G10R10A2_UNORM },
> > diff --git a/src/mesa/state_tracker/st_format.c
> > b/src/mesa/state_tracker/st_format.c
> > index 16a18c272d..4f4d8062b5 100644
> > --- a/src/mesa/state_tracker/st_format.c
> > +++ b/src/mesa/state_tracker/st_format.c
> > @@ -54,6 +54,8 @@
> >  #include "st_format.h"
> >  #include "st_texture.h"
> > 
> > +#include <GLES2/gl2.h>
> > +#include <GLES2/gl2ext.h>
> > 
> >  /**
> >   * Translate Mesa format to Gallium format.
> > @@ -169,6 +171,8 @@ st_mesa_format_to_pipe_format(const struct
> > st_context *st,
> >        return PIPE_FORMAT_AL88_SRGB;
> >     case MESA_FORMAT_L_SRGB8:
> >        return PIPE_FORMAT_L8_SRGB;
> > +   case MESA_FORMAT_R_SRGB8:
> > +      return PIPE_FORMAT_R8_SRGB;
> >     case MESA_FORMAT_BGR_SRGB8:
> >        return PIPE_FORMAT_R8G8B8_SRGB;
> >     case MESA_FORMAT_A8B8G8R8_SRGB:
> > @@ -719,6 +723,8 @@ st_pipe_format_to_mesa_format(enum pipe_format
> > format)
> >        return MESA_FORMAT_A8L8_SRGB;
> >     case PIPE_FORMAT_L8_SRGB:
> >        return MESA_FORMAT_L_SRGB8;
> > +   case PIPE_FORMAT_R8_SRGB:
> > +      return MESA_FORMAT_R_SRGB8;
> >     case PIPE_FORMAT_R8G8B8_SRGB:
> >        return MESA_FORMAT_BGR_SRGB8;
> >     case PIPE_FORMAT_ABGR8888_SRGB:
> > @@ -1423,6 +1429,10 @@ static const struct format_mapping
> > format_map[] = {
> >          0 },
> >        { PIPE_FORMAT_L8_SRGB, DEFAULT_SRGBA_FORMATS }
> >     },
> > +   {
> > +      { GL_SR8_EXT, 0 },
> > +      { PIPE_FORMAT_R8_SRGB, DEFAULT_SRGBA_FORMATS }
> 
> Do you want DEFAULT_SRGBA_FORMATS here? Since it's not required to be
> supported, why bother with the fallback?
> 
> > +   },
> > 
> >     /* 16-bit float formats */
> >     {
> > --
> > 2.19.1
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list