[03/13] dri: Add config attributes for color channel shift

Submitted by Strasser, Kevin on Jan. 28, 2019, 6:42 p.m.

Details

Message ID 1548700976-7325-4-git-send-email-kevin.strasser@intel.com
State New
Headers show
Series "Enable fp16 visuals and fbconfigs" ( rev: 2 ) in Mesa

Not browsing as part of any series.

Commit Message

Strasser, Kevin Jan. 28, 2019, 6:42 p.m.
The existing mask attributes can only support up to 32 bpp. Introduce
per-channel SHIFT attributes that indicate how many bits, from lsb towards
msb, the bit field is offset. A shift of -1 will indicate that there is no
bit field set for the channel.

As old loaders will still be looking for masks, we set the masks to 0 for
any formats wider than 32 bpp.

Signed-off-by: Kevin Strasser <kevin.strasser@intel.com>
---
 include/GL/internal/dri_interface.h |  6 +++++-
 src/mesa/drivers/dri/common/utils.c | 41 +++++++++++++++++++++++++++++++++++++
 src/mesa/main/context.c             |  6 +++---
 src/mesa/main/mtypes.h              |  1 +
 4 files changed, 50 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
index f2e46f6..6ffb86e 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -764,7 +764,11 @@  struct __DRIuseInvalidateExtensionRec {
 #define __DRI_ATTRIB_YINVERTED			47
 #define __DRI_ATTRIB_FRAMEBUFFER_SRGB_CAPABLE	48
 #define __DRI_ATTRIB_MUTABLE_RENDER_BUFFER	49 /* EGL_MUTABLE_RENDER_BUFFER_BIT_KHR */
-#define __DRI_ATTRIB_MAX			50
+#define __DRI_ATTRIB_RED_SHIFT			50
+#define __DRI_ATTRIB_GREEN_SHIFT		51
+#define __DRI_ATTRIB_BLUE_SHIFT			52
+#define __DRI_ATTRIB_ALPHA_SHIFT		53
+#define __DRI_ATTRIB_MAX			54
 
 /* __DRI_ATTRIB_RENDER_TYPE */
 #define __DRI_ATTRIB_RGBA_BIT			0x01	
diff --git a/src/mesa/drivers/dri/common/utils.c b/src/mesa/drivers/dri/common/utils.c
index 5a66bcf..1fdc160 100644
--- a/src/mesa/drivers/dri/common/utils.c
+++ b/src/mesa/drivers/dri/common/utils.c
@@ -200,9 +200,33 @@  driCreateConfigs(mesa_format format,
       { 0x000003FF, 0x000FFC00, 0x3FF00000, 0x00000000 },
       /* MESA_FORMAT_R10G10B10A2_UNORM */
       { 0x000003FF, 0x000FFC00, 0x3FF00000, 0xC0000000 },
+      /* For anything wider than 32 bpp */
+      { 0, 0, 0, 0 },
+   };
+
+   static const int shifts_table[][4] = {
+      /* MESA_FORMAT_B5G6R5_UNORM */
+      { 11, 5, 0, -1 },
+      /* MESA_FORMAT_B8G8R8X8_UNORM */
+      { 16, 8, 0, -1 },
+      /* MESA_FORMAT_B8G8R8A8_UNORM */
+      { 16, 8, 0, 24 },
+      /* MESA_FORMAT_B10G10R10X2_UNORM */
+      { 20, 10, 0, -1 },
+      /* MESA_FORMAT_B10G10R10A2_UNORM */
+      { 20, 10, 0, 30 },
+      /* MESA_FORMAT_R8G8B8A8_UNORM */
+      { 0, 8, 16, 24 },
+      /* MESA_FORMAT_R8G8B8X8_UNORM */
+      { 0, 8, 16, -1 },
+      /* MESA_FORMAT_R10G10B10X2_UNORM */
+      { 0, 10, 20, -1 },
+      /* MESA_FORMAT_R10G10B10A2_UNORM */
+      { 0, 10, 20, 30 },
    };
 
    const uint32_t * masks;
+   const int * shifts;
    __DRIconfig **configs, **c;
    struct gl_config *modes;
    unsigned i, j, k, h;
@@ -217,33 +241,42 @@  driCreateConfigs(mesa_format format,
    switch (format) {
    case MESA_FORMAT_B5G6R5_UNORM:
       masks = masks_table[0];
+      shifts = shifts_table[0];
       break;
    case MESA_FORMAT_B8G8R8X8_UNORM:
    case MESA_FORMAT_B8G8R8X8_SRGB:
       masks = masks_table[1];
+      shifts = shifts_table[1];
       break;
    case MESA_FORMAT_B8G8R8A8_UNORM:
    case MESA_FORMAT_B8G8R8A8_SRGB:
       masks = masks_table[2];
+      shifts = shifts_table[2];
       break;
    case MESA_FORMAT_R8G8B8A8_UNORM:
    case MESA_FORMAT_R8G8B8A8_SRGB:
       masks = masks_table[5];
+      shifts = shifts_table[5];
       break;
    case MESA_FORMAT_R8G8B8X8_UNORM:
       masks = masks_table[6];
+      shifts = shifts_table[6];
       break;
    case MESA_FORMAT_B10G10R10X2_UNORM:
       masks = masks_table[3];
+      shifts = shifts_table[3];
       break;
    case MESA_FORMAT_B10G10R10A2_UNORM:
       masks = masks_table[4];
+      shifts = shifts_table[4];
       break;
    case MESA_FORMAT_R10G10B10X2_UNORM:
       masks = masks_table[7];
+      shifts = shifts_table[7];
       break;
    case MESA_FORMAT_R10G10B10A2_UNORM:
       masks = masks_table[8];
+      shifts = shifts_table[8];
       break;
    default:
       fprintf(stderr, "[%s:%u] Unknown framebuffer type %s (%d).\n",
@@ -294,6 +327,10 @@  driCreateConfigs(mesa_format format,
 		    modes->greenMask = masks[1];
 		    modes->blueMask  = masks[2];
 		    modes->alphaMask = masks[3];
+		    modes->redShift   = shifts[0];
+		    modes->greenShift = shifts[1];
+		    modes->blueShift  = shifts[2];
+		    modes->alphaShift = shifts[3];
 		    modes->rgbBits   = modes->redBits + modes->greenBits
 		    	+ modes->blueBits + modes->alphaBits;
 
@@ -414,9 +451,13 @@  static const struct { unsigned int attrib, offset; } attribMap[] = {
     __ATTRIB(__DRI_ATTRIB_TRANSPARENT_BLUE_VALUE,	transparentBlue),
     __ATTRIB(__DRI_ATTRIB_TRANSPARENT_ALPHA_VALUE,	transparentAlpha),
     __ATTRIB(__DRI_ATTRIB_RED_MASK,			redMask),
+    __ATTRIB(__DRI_ATTRIB_RED_SHIFT,			redShift),
     __ATTRIB(__DRI_ATTRIB_GREEN_MASK,			greenMask),
+    __ATTRIB(__DRI_ATTRIB_GREEN_SHIFT,			greenShift),
     __ATTRIB(__DRI_ATTRIB_BLUE_MASK,			blueMask),
+    __ATTRIB(__DRI_ATTRIB_BLUE_SHIFT,			blueShift),
     __ATTRIB(__DRI_ATTRIB_ALPHA_MASK,			alphaMask),
+    __ATTRIB(__DRI_ATTRIB_ALPHA_SHIFT,			alphaShift),
     __ATTRIB(__DRI_ATTRIB_MAX_PBUFFER_WIDTH,		maxPbufferWidth),
     __ATTRIB(__DRI_ATTRIB_MAX_PBUFFER_HEIGHT,		maxPbufferHeight),
     __ATTRIB(__DRI_ATTRIB_MAX_PBUFFER_PIXELS,		maxPbufferPixels),
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 492f01d..d6391a3 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -1537,9 +1537,9 @@  check_compatible(const struct gl_context *ctx,
        ctxvis->foo != bufvis->foo)     \
       return GL_FALSE
 
-   check_component(redMask);
-   check_component(greenMask);
-   check_component(blueMask);
+   check_component(redShift);
+   check_component(greenShift);
+   check_component(blueShift);
    check_component(depthBits);
    check_component(stencilBits);
 
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 0fdeba4..2d749c0 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -170,6 +170,7 @@  struct gl_config
 
    GLint redBits, greenBits, blueBits, alphaBits;	/* bits per comp */
    GLuint redMask, greenMask, blueMask, alphaMask;
+   GLint redShift, greenShift, blueShift, alphaShift;
    GLint rgbBits;		/* total bits for rgb */
    GLint indexBits;		/* total bits for colorindex */
 

Comments

On Mon, 28 Jan 2019 at 18:42, Kevin Strasser <kevin.strasser@intel.com> wrote:
>
> The existing mask attributes can only support up to 32 bpp. Introduce
> per-channel SHIFT attributes that indicate how many bits, from lsb towards
> msb, the bit field is offset. A shift of -1 will indicate that there is no
> bit field set for the channel.
>
Kind of split on the idea of shift instead of masks, yet Adam brought
some good arguments.

> As old loaders will still be looking for masks, we set the masks to 0 for
> any formats wider than 32 bpp.
>
> Signed-off-by: Kevin Strasser <kevin.strasser@intel.com>
> ---
>  include/GL/internal/dri_interface.h |  6 +++++-
>  src/mesa/drivers/dri/common/utils.c | 41 +++++++++++++++++++++++++++++++++++++
>  src/mesa/main/context.c             |  6 +++---
>  src/mesa/main/mtypes.h              |  1 +
>  4 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
> index f2e46f6..6ffb86e 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -764,7 +764,11 @@ struct __DRIuseInvalidateExtensionRec {
>  #define __DRI_ATTRIB_YINVERTED                 47
>  #define __DRI_ATTRIB_FRAMEBUFFER_SRGB_CAPABLE  48
>  #define __DRI_ATTRIB_MUTABLE_RENDER_BUFFER     49 /* EGL_MUTABLE_RENDER_BUFFER_BIT_KHR */
> -#define __DRI_ATTRIB_MAX                       50
> +#define __DRI_ATTRIB_RED_SHIFT                 50
> +#define __DRI_ATTRIB_GREEN_SHIFT               51
> +#define __DRI_ATTRIB_BLUE_SHIFT                        52
> +#define __DRI_ATTRIB_ALPHA_SHIFT               53
> +#define __DRI_ATTRIB_MAX                       54
>
>  /* __DRI_ATTRIB_RENDER_TYPE */
>  #define __DRI_ATTRIB_RGBA_BIT                  0x01
> diff --git a/src/mesa/drivers/dri/common/utils.c b/src/mesa/drivers/dri/common/utils.c
> index 5a66bcf..1fdc160 100644
> --- a/src/mesa/drivers/dri/common/utils.c
> +++ b/src/mesa/drivers/dri/common/utils.c
> @@ -200,9 +200,33 @@ driCreateConfigs(mesa_format format,
>        { 0x000003FF, 0x000FFC00, 0x3FF00000, 0x00000000 },
>        /* MESA_FORMAT_R10G10B10A2_UNORM */
>        { 0x000003FF, 0x000FFC00, 0x3FF00000, 0xC0000000 },
> +      /* For anything wider than 32 bpp */
> +      { 0, 0, 0, 0 },
> +   };
> +
Unused entry?


> +   static const int shifts_table[][4] = {
> +      /* MESA_FORMAT_B5G6R5_UNORM */
> +      { 11, 5, 0, -1 },
> +      /* MESA_FORMAT_B8G8R8X8_UNORM */
> +      { 16, 8, 0, -1 },
> +      /* MESA_FORMAT_B8G8R8A8_UNORM */
> +      { 16, 8, 0, 24 },
> +      /* MESA_FORMAT_B10G10R10X2_UNORM */
> +      { 20, 10, 0, -1 },
> +      /* MESA_FORMAT_B10G10R10A2_UNORM */
> +      { 20, 10, 0, 30 },
> +      /* MESA_FORMAT_R8G8B8A8_UNORM */
> +      { 0, 8, 16, 24 },
> +      /* MESA_FORMAT_R8G8B8X8_UNORM */
> +      { 0, 8, 16, -1 },
> +      /* MESA_FORMAT_R10G10B10X2_UNORM */
> +      { 0, 10, 20, -1 },
> +      /* MESA_FORMAT_R10G10B10A2_UNORM */
> +      { 0, 10, 20, 30 },
>     };
>
At some point in the future we should add the MESA_FORMAT to the
tables above, and simplify the massive switch (below) to a 4-line loop
;-)

>     const uint32_t * masks;
> +   const int * shifts;
>     __DRIconfig **configs, **c;
>     struct gl_config *modes;
>     unsigned i, j, k, h;
> @@ -217,33 +241,42 @@ driCreateConfigs(mesa_format format,
>     switch (format) {
>     case MESA_FORMAT_B5G6R5_UNORM:
>        masks = masks_table[0];
> +      shifts = shifts_table[0];
>        break;
>     case MESA_FORMAT_B8G8R8X8_UNORM:
>     case MESA_FORMAT_B8G8R8X8_SRGB:
>        masks = masks_table[1];
> +      shifts = shifts_table[1];
>        break;
>     case MESA_FORMAT_B8G8R8A8_UNORM:
>     case MESA_FORMAT_B8G8R8A8_SRGB:
>        masks = masks_table[2];
> +      shifts = shifts_table[2];
>        break;
>     case MESA_FORMAT_R8G8B8A8_UNORM:
>     case MESA_FORMAT_R8G8B8A8_SRGB:
>        masks = masks_table[5];
> +      shifts = shifts_table[5];
>        break;
>     case MESA_FORMAT_R8G8B8X8_UNORM:
>        masks = masks_table[6];
> +      shifts = shifts_table[6];
>        break;
>     case MESA_FORMAT_B10G10R10X2_UNORM:
>        masks = masks_table[3];
> +      shifts = shifts_table[3];
>        break;
>     case MESA_FORMAT_B10G10R10A2_UNORM:
>        masks = masks_table[4];
> +      shifts = shifts_table[4];
>        break;
>     case MESA_FORMAT_R10G10B10X2_UNORM:
>        masks = masks_table[7];
> +      shifts = shifts_table[7];
>        break;
>     case MESA_FORMAT_R10G10B10A2_UNORM:
>        masks = masks_table[8];
> +      shifts = shifts_table[8];
>        break;
>     default:
>        fprintf(stderr, "[%s:%u] Unknown framebuffer type %s (%d).\n",
> @@ -294,6 +327,10 @@ driCreateConfigs(mesa_format format,
>                     modes->greenMask = masks[1];
>                     modes->blueMask  = masks[2];
>                     modes->alphaMask = masks[3];
> +                   modes->redShift   = shifts[0];
> +                   modes->greenShift = shifts[1];
> +                   modes->blueShift  = shifts[2];
> +                   modes->alphaShift = shifts[3];
>                     modes->rgbBits   = modes->redBits + modes->greenBits
>                         + modes->blueBits + modes->alphaBits;
>
> @@ -414,9 +451,13 @@ static const struct { unsigned int attrib, offset; } attribMap[] = {
>      __ATTRIB(__DRI_ATTRIB_TRANSPARENT_BLUE_VALUE,      transparentBlue),
>      __ATTRIB(__DRI_ATTRIB_TRANSPARENT_ALPHA_VALUE,     transparentAlpha),
>      __ATTRIB(__DRI_ATTRIB_RED_MASK,                    redMask),
> +    __ATTRIB(__DRI_ATTRIB_RED_SHIFT,                   redShift),
>      __ATTRIB(__DRI_ATTRIB_GREEN_MASK,                  greenMask),
> +    __ATTRIB(__DRI_ATTRIB_GREEN_SHIFT,                 greenShift),
>      __ATTRIB(__DRI_ATTRIB_BLUE_MASK,                   blueMask),
> +    __ATTRIB(__DRI_ATTRIB_BLUE_SHIFT,                  blueShift),
>      __ATTRIB(__DRI_ATTRIB_ALPHA_MASK,                  alphaMask),
> +    __ATTRIB(__DRI_ATTRIB_ALPHA_SHIFT,                 alphaShift),
>      __ATTRIB(__DRI_ATTRIB_MAX_PBUFFER_WIDTH,           maxPbufferWidth),
>      __ATTRIB(__DRI_ATTRIB_MAX_PBUFFER_HEIGHT,          maxPbufferHeight),
>      __ATTRIB(__DRI_ATTRIB_MAX_PBUFFER_PIXELS,          maxPbufferPixels),
> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
> index 492f01d..d6391a3 100644
> --- a/src/mesa/main/context.c
> +++ b/src/mesa/main/context.c
> @@ -1537,9 +1537,9 @@ check_compatible(const struct gl_context *ctx,
>         ctxvis->foo != bufvis->foo)     \
>        return GL_FALSE
>
> -   check_component(redMask);
> -   check_component(greenMask);
> -   check_component(blueMask);
I think these were meant to say.
With the check_component() reinstated (or clarified why they're gone),
the patch is

Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

-Emil
Emil Velikov wrote:
> On Mon, 28 Jan 2019 at 18:42, Kevin Strasser <kevin.strasser@intel.com> wrote:
> > diff --git a/src/mesa/drivers/dri/common/utils.c
> > b/src/mesa/drivers/dri/common/utils.c
> > index 5a66bcf..1fdc160 100644
> > --- a/src/mesa/drivers/dri/common/utils.c
> > +++ b/src/mesa/drivers/dri/common/utils.c
> > @@ -200,9 +200,33 @@ driCreateConfigs(mesa_format format,
> >        { 0x000003FF, 0x000FFC00, 0x3FF00000, 0x00000000 },
> >        /* MESA_FORMAT_R10G10B10A2_UNORM */
> >        { 0x000003FF, 0x000FFC00, 0x3FF00000, 0xC0000000 },
> > +      /* For anything wider than 32 bpp */
> > +      { 0, 0, 0, 0 },
> > +   };
> > +
> Unused entry?

Maybe this hunk belongs in 08/ where the entry gets used...

> > diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
> > index 492f01d..d6391a3 100644
> > --- a/src/mesa/main/context.c
> > +++ b/src/mesa/main/context.c
> > @@ -1537,9 +1537,9 @@ check_compatible(const struct gl_context *ctx,
> >         ctxvis->foo != bufvis->foo)     \
> >        return GL_FALSE
> >
> > -   check_component(redMask);
> > -   check_component(greenMask);
> > -   check_component(blueMask);
> I think these were meant to say.

I didn't see any reason to check both mask and shift values. Can you explain why 
they would still be needed?

Thanks,
Kevin

> With the check_component() reinstated (or clarified why they're gone),
> the patch is

> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>