Re: [Mesa-dev] [RFC 1/6] dri: Support 64 bit rgba masks

2019-01-16 Thread Strasser, Kevin 
Adam Jackson wrote:
> On Wed, 2019-01-16 at 17:58 +, Strasser, Kevin wrote:
> > Adam Jackson wrote:
> >
> > > Probably what I'd do is only define those masks to non-zero for <=32bpp
> > > formats,
> >
> > Not sure if I understood this statement. Do you mean if we keep the _HI and
> > _LO  attributes, only make them valid for formats that are <=64bpp, like
> > fp16?
>
> I was thinking:
>
> a) Leave __DRI_ATTRIB_XXX_MASK in place, and simply set it to zero for
> configs that don't fit in 32bpp
>
> b) New attribute(s) for __DRI_ATTRIB_XXX_SHIFT, whose value is the
> right-hand side of the << operator you'd use to find that channel
>
> c) Fix dri2_add_config() and friends to work in terms of shifts not
> masks
>
> This nicely avoids worrying about just how wide a pixel will ever get
> (ie how many _HI attribs you'd need for fp32, fp64, etc), because the
> left-shift is a whole 32-bit integer, and we're almost certainly not
> worried about needing a billion bits per channel. The only other
> flexibility masks would give you is allowing discontiguous bit ranges
> for color channels, but that's not a feature of any hardware we do or
> will care about (and I'm not sure was ever really a thing, tbh).
>
> Setting the MASK attribs to zero appears to magically DTRT when loading
> a newer driver (with fp16 format support) on an older libEGL or
> xserver; since the masks of the winsys' visuals will be non-zero, fp
> formats will never match.

OK, thanks for explaining, I think I get it now. I'll see if I can get this 
working.

Thanks,
Kevin
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC 1/6] dri: Support 64 bit rgba masks

2019-01-16 Thread Adam Jackson 
On Wed, 2019-01-16 at 17:58 +, Strasser, Kevin wrote:
> Adam Jackson wrote:
> 
> > Probably what I'd do is only define those masks to non-zero for <=32bpp 
> > formats,
> 
> Not sure if I understood this statement. Do you mean if we keep the _HI and 
> _LO
> attributes, only make them valid for formats that are <=64bpp, like fp16?

I was thinking:

a) Leave __DRI_ATTRIB_XXX_MASK in place, and simply set it to zero for
configs that don't fit in 32bpp

b) New attribute(s) for __DRI_ATTRIB_XXX_SHIFT, whose value is the
right-hand side of the << operator you'd use to find that channel

c) Fix dri2_add_config() and friends to work in terms of shifts not
masks

This nicely avoids worrying about just how wide a pixel will ever get
(ie how many _HI attribs you'd need for fp32, fp64, etc), because the
left-shift is a whole 32-bit integer, and we're almost certainly not
worried about needing a billion bits per channel. The only other
flexibility masks would give you is allowing discontiguous bit ranges
for color channels, but that's not a feature of any hardware we do or
will care about (and I'm not sure was ever really a thing, tbh).

Setting the MASK attribs to zero appears to magically DTRT when loading
a newer driver (with fp16 format support) on an older libEGL or
xserver; since the masks of the winsys' visuals will be non-zero, fp
formats will never match.

- ajax

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC 1/6] dri: Support 64 bit rgba masks

2019-01-16 Thread Strasser, Kevin 
Adam Jackson wrote:
> On Wed, 2019-01-16 at 04:41 +, Strasser, Kevin wrote:
> > Emil Velikov wrote:
> > >
> > > Please split this up a bit. I'm thinking of:
> > >  - dri_interface
> > >  - mesa
> > >  - egl
> > >  - gbm
> > >  - glx - seems sparse on updates, guessting you're followed in laster
> > >  patches?
> >
> > Sure, I can break it up a bit more. I didn't modify glx much as it doesn't 
> > read the mask attributes directly, hence it can't handle configs with RGBA
> > ordering. I don't know the background of that limitation, but I assume there
> > just hasn't been any use cases for those formats outside of Android, and so
> > handling hasn't been needed for glx... The intention of this series was to
> > get fp16 working first for egl. Can we leave the glx side for if/when
> > someone needs it there?
>
> GLX handles RGBA ordering just fine. GLX doesn't expose channel
> ordering because it inherits that from the X visual. But the only thing
> you could do with fp16 in GLX is make a pbuffer out of it, because X
> doesn't support >8bpc for windows or pixmaps, and there is no visual
> corresponding to a pbuffer, so whatever channel order the hardware
> wants is what you get.

Ah, understood.

> That said, I note that GLX_ARB_color_buffer_float defines not just fp16
> but fp32 formats, and EGL_EXT_pixel_format_float doesn't differ. If we
> did want to support fp32 surfaces then simply widening attributes to 64
> bits (or adding a "high" 32 bits) isn't going to be enough. Probably
> what I'd do is only define those masks to non-zero for <=32bpp formats,
> and add new attributes for channel left-shift instead. I realize this
> contradicts what I said earlier, sorry about that.

Like I said, my immediate goal is to get fp16 working, but I'm all for picking
the design that will work over the long run, especially for hard to change ABI
boundaries.

I suppose *_HI and *_LO attributes don't make any sense if we might be talking
about masks that exceed 64 bits. Is __DRI_ATTRIB_RED_MASK_LSHIFT32 and
__DRI_ATTRIB_RED_MASK_LSHIFT64 and so on what you had in mind?

> Probably what I'd do is only define those masks to non-zero for <=32bpp 
> formats,

Not sure if I understood this statement. Do you mean if we keep the _HI and _LO
attributes, only make them valid for formats that are <=64bpp, like fp16?

Thanks,
Kevin
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC 1/6] dri: Support 64 bit rgba masks

2019-01-16 Thread Adam Jackson 
On Wed, 2019-01-16 at 04:41 +, Strasser, Kevin wrote:
> Emil Velikov wrote:
> > 
> > Please split this up a bit. I'm thinking of:
> >  - dri_interface
> >  - mesa
> >  - egl
> >  - gbm
> >  - glx - seems sparse on updates, guessting you're followed in laster 
> > patches?
> 
> Sure, I can break it up a bit more. I didn't modify glx much as it doesn't 
> read
> the mask attributes directly, hence it can't handle configs with RGBA 
> ordering.
> I don't know the background of that limitation, but I assume there just hasn't
> been any use cases for those formats outside of Android, and so handling 
> hasn't
> been needed for glx... The intention of this series was to get fp16 working
> first for egl. Can we leave the glx side for if/when someone needs it there?

GLX handles RGBA ordering just fine. GLX doesn't expose channel
ordering because it inherits that from the X visual. But the only thing
you could do with fp16 in GLX is make a pbuffer out of it, because X
doesn't support >8bpc for windows or pixmaps, and there is no visual
corresponding to a pbuffer, so whatever channel order the hardware
wants is what you get.

That said, I note that GLX_ARB_color_buffer_float defines not just fp16
but fp32 formats, and EGL_EXT_pixel_format_float doesn't differ. If we
did want to support fp32 surfaces then simply widening attributes to 64
bits (or adding a "high" 32 bits) isn't going to be enough. Probably
what I'd do is only define those masks to non-zero for <=32bpp formats,
and add new attributes for channel left-shift instead. I realize this
contradicts what I said earlier, sorry about that.

- ajax

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC 1/6] dri: Support 64 bit rgba masks

2019-01-15 Thread Strasser, Kevin 
Emil Velikov wrote:
> Hi Kevin,
> 
> Thanks for that massive undertaking in addressing this.

Sure thing!

> On 2019/01/04, Kevin Strasser wrote:
> > The dri core api was written with the assumption that all attribute
> > values would fit into 32 bits. This limitation means the config
> > handlers can't accept 64 bpp formats. Reserve 64 bits for rgba masks
> > and add new attributes that allow access to the upper 32 bits.
> >
> > Signed-off-by: Kevin Strasser 
> > ---
> >  include/GL/internal/dri_interface.h |  6 ++-
> >  src/egl/drivers/dri2/egl_dri2.c | 28 +---
> >  src/egl/drivers/dri2/egl_dri2.h |  6 +--
> >  src/egl/drivers/dri2/platform_android.c |  2 +-
> >  src/egl/drivers/dri2/platform_drm.c | 67 
> > ++---
> >  src/egl/drivers/dri2/platform_surfaceless.c |  2 +-
> >  src/egl/drivers/dri2/platform_wayland.c |  2 +-
> >  src/egl/drivers/dri2/platform_x11.c |  6 +--
> >  src/gbm/backends/dri/gbm_driint.h   |  8 ++--
> >  src/glx/glxconfig.h |  2 +-
> >  src/mesa/drivers/dri/common/utils.c | 16 ++-
> >  src/mesa/main/mtypes.h  |  2 +-
> >  12 files changed, 108 insertions(+), 39 deletions(-)
> >
> Please split this up a bit. I'm thinking of:
>  - dri_interface
>  - mesa
>  - egl
>  - gbm
>  - glx - seems sparse on updates, guessting you're followed in laster patches?

Sure, I can break it up a bit more. I didn't modify glx much as it doesn't read
the mask attributes directly, hence it can't handle configs with RGBA ordering.
I don't know the background of that limitation, but I assume there just hasn't
been any use cases for those formats outside of Android, and so handling hasn't
been needed for glx... The intention of this series was to get fp16 working
first for egl. Can we leave the glx side for if/when someone needs it there?

> > diff --git a/include/GL/internal/dri_interface.h
> > b/include/GL/internal/dri_interface.h
> > index 072f379..c5761c4 100644
> > --- a/include/GL/internal/dri_interface.h
> > +++ b/include/GL/internal/dri_interface.h
> > @@ -747,7 +747,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_MASK_HI   50
> > +#define __DRI_ATTRIB_GREEN_MASK_HI 51
> > +#define __DRI_ATTRIB_BLUE_MASK_HI  52
> > +#define __DRI_ATTRIB_ALPHA_MASK_HI 53
> > +#define __DRI_ATTRIB_MAX   54
> 
> Worth adding some defines as below for clarity/consistency sake and updating
> the existing code to use them?
> 
> #define __DRI_ATTRIB_RED_MASK_LO __DRI_ATTRIB_RED_MASK ...

Sounds like a good idea.

> >  /* __DRI_ATTRIB_RENDER_TYPE */
> >  #define __DRI_ATTRIB_RGBA_BIT  0x01
> > diff --git a/src/egl/drivers/dri2/egl_dri2.c
> > b/src/egl/drivers/dri2/egl_dri2.c index 0be9235..d19950d 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.c
> > +++ b/src/egl/drivers/dri2/egl_dri2.c
> > @@ -179,7 +179,7 @@ dri2_match_config(const _EGLConfig *conf, const
> > _EGLConfig *criteria)  struct dri2_egl_config *
> > dri2_add_config(_EGLDisplay *disp, const __DRIconfig *dri_config, int id,
> >  EGLint surface_type, const EGLint *attr_list,
> > -const unsigned int *rgba_masks)
> > +const unsigned long long int *rgba_masks)
> I'm slightly inclided towards uint64_t since it's a little more explicit and 
> clear.
> IIRC sizeof(long long) varies across platforms and/or compilers so uint64_t 
> will
> avoid all the potential fun ;-)

Sure, I'm all for using explicit width types.

> > +  const __DRIconfig *config = dri2_dpy->driver_configs[i];
> > +  unsigned long long int red, green, blue, alpha;
> > +  unsigned int mask_hi = 0, mask_lo;
> > +
> > +  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_RED_MASK_HI,
> > +  &mask_hi);
> > +  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_RED_MASK,
> > +  &mask_lo);
> > +  red = (unsigned long long int)mask_hi << 32 | mask_lo;
> > +
> > +  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_GREEN_MASK_HI,
> > +  &mask_hi);
> > +  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_GREEN_MASK,
> > +  &mask_lo);
> > +  green = (unsigned long long int)mask_hi << 32 | mask_lo;
> > +
> > +  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_BLUE_MASK_HI,
> > +  &mask_hi);
> > +  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_BLUE_MASK,
> > +  &mask_lo);
> > +  blue = (

Re: [Mesa-dev] [RFC 1/6] dri: Support 64 bit rgba masks

2019-01-15 Thread Strasser, Kevin 
Adam Jackson wrote:
> > On Fri, 2019-01-11 at 15:01 +, Emil Velikov wrote:
> > 
> > > > @@ -460,6 +464,14 @@ driGetConfigAttribIndex(const __DRIconfig
> > *config,
> > > > else
> > > > *value = 0;
> > > > break;
> > > > +case __DRI_ATTRIB_RED_MASK_HI:
> > > > +case __DRI_ATTRIB_GREEN_MASK_HI:
> > > > +case __DRI_ATTRIB_BLUE_MASK_HI:
> > > > +case __DRI_ATTRIB_ALPHA_MASK_HI:
> > > > +/* upper 32 bits of 64 bit fields */
> > > > +*value = *(unsigned int *)
> > > > +((char *) &config->modes + attribMap[index].offset +
> > > > + 4);
> > >
> > > Is the "+ 4" going to work on big endian systems?
> > 
> > No.
> > 
> > I think I'd prefer to just expand config attribute values to 64-bit across 
> > the board
> > internally, rather than have paired 32-bit attributes like this.

Emil is right, big endian wouldn't work with my patch as-is, but I suppose I
could incorporate some macros to make it work. I experimented with changing
getConfigAttrib/indexConfigAttrib to allow for 64-bit values, like you suggest,
but doing so without breaking ABI got pretty ugly - required adding new
functions (getConfigAttrib2/indexConfigAttrib2), and checking the driver's
extension version level at every call site. However, given the issue with
endianness, I'm now more inclined to go in that direction.

Thanks,
Kevin
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC 1/6] dri: Support 64 bit rgba masks

2019-01-14 Thread Adam Jackson 
On Fri, 2019-01-11 at 15:01 +, Emil Velikov wrote:

> > @@ -460,6 +464,14 @@ driGetConfigAttribIndex(const __DRIconfig *config,
> > else
> > *value = 0;
> > break;
> > +case __DRI_ATTRIB_RED_MASK_HI:
> > +case __DRI_ATTRIB_GREEN_MASK_HI:
> > +case __DRI_ATTRIB_BLUE_MASK_HI:
> > +case __DRI_ATTRIB_ALPHA_MASK_HI:
> > +/* upper 32 bits of 64 bit fields */
> > +*value = *(unsigned int *)
> > +((char *) &config->modes + attribMap[index].offset + 4);
> 
> Is the "+ 4" going to work on big endian systems?

No.

I think I'd prefer to just expand config attribute values to 64-bit
across the board internally, rather than have paired 32-bit attributes
like this.

- ajax
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC 1/6] dri: Support 64 bit rgba masks

2019-01-13 Thread Tapani Pälli 



On 1/11/19 5:01 PM, Emil Velikov wrote:

Hi Kevin,

Thanks for that massive undertaking in addressing this.

On 2019/01/04, Kevin Strasser wrote:

The dri core api was written with the assumption that all attribute values
would fit into 32 bits. This limitation means the config handlers can't
accept 64 bpp formats. Reserve 64 bits for rgba masks and add new
attributes that allow access to the upper 32 bits.

Signed-off-by: Kevin Strasser 
---
  include/GL/internal/dri_interface.h |  6 ++-
  src/egl/drivers/dri2/egl_dri2.c | 28 +---
  src/egl/drivers/dri2/egl_dri2.h |  6 +--
  src/egl/drivers/dri2/platform_android.c |  2 +-
  src/egl/drivers/dri2/platform_drm.c | 67 ++---
  src/egl/drivers/dri2/platform_surfaceless.c |  2 +-
  src/egl/drivers/dri2/platform_wayland.c |  2 +-
  src/egl/drivers/dri2/platform_x11.c |  6 +--
  src/gbm/backends/dri/gbm_driint.h   |  8 ++--
  src/glx/glxconfig.h |  2 +-
  src/mesa/drivers/dri/common/utils.c | 16 ++-
  src/mesa/main/mtypes.h  |  2 +-
  12 files changed, 108 insertions(+), 39 deletions(-)


Please split this up a bit. I'm thinking of:
  - dri_interface
  - mesa
  - egl
  - gbm
  - glx - seems sparse on updates, guessting you're followed in laster
patches?


diff --git a/include/GL/internal/dri_interface.h 
b/include/GL/internal/dri_interface.h
index 072f379..c5761c4 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -747,7 +747,11 @@ struct __DRIuseInvalidateExtensionRec {
  #define __DRI_ATTRIB_YINVERTED47
  #define __DRI_ATTRIB_FRAMEBUFFER_SRGB_CAPABLE 48
  #define __DRI_ATTRIB_MUTABLE_RENDER_BUFFER49 /* 
EGL_MUTABLE_RENDER_BUFFER_BIT_KHR */
-#define __DRI_ATTRIB_MAX   50
+#define __DRI_ATTRIB_RED_MASK_HI   50
+#define __DRI_ATTRIB_GREEN_MASK_HI 51
+#define __DRI_ATTRIB_BLUE_MASK_HI  52
+#define __DRI_ATTRIB_ALPHA_MASK_HI 53
+#define __DRI_ATTRIB_MAX   54


Worth adding some defines as below for clarity/consistency sake and
updating the existing code to use them?

#define __DRI_ATTRIB_RED_MASK_LO __DRI_ATTRIB_RED_MASK
...


  
  /* __DRI_ATTRIB_RENDER_TYPE */

  #define __DRI_ATTRIB_RGBA_BIT 0x01
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 0be9235..d19950d 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -179,7 +179,7 @@ dri2_match_config(const _EGLConfig *conf, const _EGLConfig 
*criteria)
  struct dri2_egl_config *
  dri2_add_config(_EGLDisplay *disp, const __DRIconfig *dri_config, int id,
  EGLint surface_type, const EGLint *attr_list,
-const unsigned int *rgba_masks)
+const unsigned long long int *rgba_masks)

I'm slightly inclided towards uint64_t since it's a little more explicit
and clear. IIRC sizeof(long long) varies across platforms and/or
compilers so uint64_t will avoid all the potential fun ;-)


This was my thinking as well, we use uint64_t in other places. I've gone 
briefly through the series and overall these changes look good to me.




[snip]


+  const __DRIconfig *config = dri2_dpy->driver_configs[i];
+  unsigned long long int red, green, blue, alpha;
+  unsigned int mask_hi = 0, mask_lo;
+
+  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_RED_MASK_HI,
+  &mask_hi);
+  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_RED_MASK,
+  &mask_lo);
+  red = (unsigned long long int)mask_hi << 32 | mask_lo;
+
+  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_GREEN_MASK_HI,
+  &mask_hi);
+  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_GREEN_MASK,
+  &mask_lo);
+  green = (unsigned long long int)mask_hi << 32 | mask_lo;
+
+  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_BLUE_MASK_HI,
+  &mask_hi);
+  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_BLUE_MASK,
+  &mask_lo);
+  blue = (unsigned long long int)mask_hi << 32 | mask_lo;
+
+  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_ALPHA_MASK_HI,
+  &mask_hi);
+  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_ALPHA_MASK,
+  &mask_lo);
+  alpha = (unsigned long long int)mask_hi << 32 | mask_lo;
  

Would be great to fold this into a helper since we have it in three
places already.

Speaking of which did you forget to git add the platform_wayland.c hunk?

Note: getConfigAttrib returns 0 (GL_FALSE) on error (think new libEGL,
old i965_dri.so) so we want to handle that i

Re: [Mesa-dev] [RFC 1/6] dri: Support 64 bit rgba masks

2019-01-11 Thread Emil Velikov 
Hi Kevin,

Thanks for that massive undertaking in addressing this.

On 2019/01/04, Kevin Strasser wrote:
> The dri core api was written with the assumption that all attribute values
> would fit into 32 bits. This limitation means the config handlers can't
> accept 64 bpp formats. Reserve 64 bits for rgba masks and add new
> attributes that allow access to the upper 32 bits.
> 
> Signed-off-by: Kevin Strasser 
> ---
>  include/GL/internal/dri_interface.h |  6 ++-
>  src/egl/drivers/dri2/egl_dri2.c | 28 +---
>  src/egl/drivers/dri2/egl_dri2.h |  6 +--
>  src/egl/drivers/dri2/platform_android.c |  2 +-
>  src/egl/drivers/dri2/platform_drm.c | 67 
> ++---
>  src/egl/drivers/dri2/platform_surfaceless.c |  2 +-
>  src/egl/drivers/dri2/platform_wayland.c |  2 +-
>  src/egl/drivers/dri2/platform_x11.c |  6 +--
>  src/gbm/backends/dri/gbm_driint.h   |  8 ++--
>  src/glx/glxconfig.h |  2 +-
>  src/mesa/drivers/dri/common/utils.c | 16 ++-
>  src/mesa/main/mtypes.h  |  2 +-
>  12 files changed, 108 insertions(+), 39 deletions(-)
> 
Please split this up a bit. I'm thinking of:
 - dri_interface
 - mesa
 - egl
 - gbm
 - glx - seems sparse on updates, guessting you're followed in laster
patches?

> diff --git a/include/GL/internal/dri_interface.h 
> b/include/GL/internal/dri_interface.h
> index 072f379..c5761c4 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -747,7 +747,11 @@ struct __DRIuseInvalidateExtensionRec {
>  #define __DRI_ATTRIB_YINVERTED   47
>  #define __DRI_ATTRIB_FRAMEBUFFER_SRGB_CAPABLE48
>  #define __DRI_ATTRIB_MUTABLE_RENDER_BUFFER   49 /* 
> EGL_MUTABLE_RENDER_BUFFER_BIT_KHR */
> -#define __DRI_ATTRIB_MAX 50
> +#define __DRI_ATTRIB_RED_MASK_HI 50
> +#define __DRI_ATTRIB_GREEN_MASK_HI   51
> +#define __DRI_ATTRIB_BLUE_MASK_HI52
> +#define __DRI_ATTRIB_ALPHA_MASK_HI   53
> +#define __DRI_ATTRIB_MAX 54

Worth adding some defines as below for clarity/consistency sake and
updating the existing code to use them?

#define __DRI_ATTRIB_RED_MASK_LO __DRI_ATTRIB_RED_MASK
...


>  
>  /* __DRI_ATTRIB_RENDER_TYPE */
>  #define __DRI_ATTRIB_RGBA_BIT0x01
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 0be9235..d19950d 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -179,7 +179,7 @@ dri2_match_config(const _EGLConfig *conf, const 
> _EGLConfig *criteria)
>  struct dri2_egl_config *
>  dri2_add_config(_EGLDisplay *disp, const __DRIconfig *dri_config, int id,
>  EGLint surface_type, const EGLint *attr_list,
> -const unsigned int *rgba_masks)
> +const unsigned long long int *rgba_masks)
I'm slightly inclided towards uint64_t since it's a little more explicit
and clear. IIRC sizeof(long long) varies across platforms and/or
compilers so uint64_t will avoid all the potential fun ;-)


[snip]

> +  const __DRIconfig *config = dri2_dpy->driver_configs[i];
> +  unsigned long long int red, green, blue, alpha;
> +  unsigned int mask_hi = 0, mask_lo;
> +
> +  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_RED_MASK_HI,
> +  &mask_hi);
> +  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_RED_MASK,
> +  &mask_lo);
> +  red = (unsigned long long int)mask_hi << 32 | mask_lo;
> +
> +  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_GREEN_MASK_HI,
> +  &mask_hi);
> +  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_GREEN_MASK,
> +  &mask_lo);
> +  green = (unsigned long long int)mask_hi << 32 | mask_lo;
> +
> +  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_BLUE_MASK_HI,
> +  &mask_hi);
> +  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_BLUE_MASK,
> +  &mask_lo);
> +  blue = (unsigned long long int)mask_hi << 32 | mask_lo;
> +
> +  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_ALPHA_MASK_HI,
> +  &mask_hi);
> +  dri2_dpy->core->getConfigAttrib(config, __DRI_ATTRIB_ALPHA_MASK,
> +  &mask_lo);
> +  alpha = (unsigned long long int)mask_hi << 32 | mask_lo;
>  
Would be great to fold this into a helper since we have it in three
places already.

Speaking of which did you forget to git add the platform_wayland.c hunk?

Note: getConfigAttrib returns 0 (GL_FALSE) on error (think new libEGL,
old i965_dri.so) so we want to handle that in some way.

[snip]

> @@ -460,6 +464,14 @@ driGetConfigAttribIndex

[Mesa-dev] [RFC 1/6] dri: Support 64 bit rgba masks

2019-01-04 Thread Kevin Strasser 
The dri core api was written with the assumption that all attribute values
would fit into 32 bits. This limitation means the config handlers can't
accept 64 bpp formats. Reserve 64 bits for rgba masks and add new
attributes that allow access to the upper 32 bits.

Signed-off-by: Kevin Strasser 
---
 include/GL/internal/dri_interface.h |  6 ++-
 src/egl/drivers/dri2/egl_dri2.c | 28 +---
 src/egl/drivers/dri2/egl_dri2.h |  6 +--
 src/egl/drivers/dri2/platform_android.c |  2 +-
 src/egl/drivers/dri2/platform_drm.c | 67 ++---
 src/egl/drivers/dri2/platform_surfaceless.c |  2 +-
 src/egl/drivers/dri2/platform_wayland.c |  2 +-
 src/egl/drivers/dri2/platform_x11.c |  6 +--
 src/gbm/backends/dri/gbm_driint.h   |  8 ++--
 src/glx/glxconfig.h |  2 +-
 src/mesa/drivers/dri/common/utils.c | 16 ++-
 src/mesa/main/mtypes.h  |  2 +-
 12 files changed, 108 insertions(+), 39 deletions(-)

diff --git a/include/GL/internal/dri_interface.h 
b/include/GL/internal/dri_interface.h
index 072f379..c5761c4 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -747,7 +747,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_MASK_HI   50
+#define __DRI_ATTRIB_GREEN_MASK_HI 51
+#define __DRI_ATTRIB_BLUE_MASK_HI  52
+#define __DRI_ATTRIB_ALPHA_MASK_HI 53
+#define __DRI_ATTRIB_MAX   54
 
 /* __DRI_ATTRIB_RENDER_TYPE */
 #define __DRI_ATTRIB_RGBA_BIT  0x01
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 0be9235..d19950d 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -179,7 +179,7 @@ dri2_match_config(const _EGLConfig *conf, const _EGLConfig 
*criteria)
 struct dri2_egl_config *
 dri2_add_config(_EGLDisplay *disp, const __DRIconfig *dri_config, int id,
 EGLint surface_type, const EGLint *attr_list,
-const unsigned int *rgba_masks)
+const unsigned long long int *rgba_masks)
 {
struct dri2_egl_config *conf;
struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
@@ -187,7 +187,7 @@ dri2_add_config(_EGLDisplay *disp, const __DRIconfig 
*dri_config, int id,
unsigned int attrib, value, double_buffer;
bool srgb = false;
EGLint key, bind_to_texture_rgb, bind_to_texture_rgba;
-   unsigned int dri_masks[4] = { 0, 0, 0, 0 };
+   unsigned long long int dri_masks[4] = { 0, 0, 0, 0 };
_EGLConfig *matching_config;
EGLint num_configs = 0;
EGLint config_id;
@@ -234,19 +234,35 @@ dri2_add_config(_EGLDisplay *disp, const __DRIconfig 
*dri_config, int id,
  break;
 
   case __DRI_ATTRIB_RED_MASK:
- dri_masks[0] = value;
+ dri_masks[0] |= value;
+ break;
+
+  case __DRI_ATTRIB_RED_MASK_HI:
+ dri_masks[0] |= (unsigned long long int)value << 32;
  break;
 
   case __DRI_ATTRIB_GREEN_MASK:
- dri_masks[1] = value;
+ dri_masks[1] |= value;
+ break;
+
+  case __DRI_ATTRIB_GREEN_MASK_HI:
+ dri_masks[1] |= (unsigned long long int)value << 32;
  break;
 
   case __DRI_ATTRIB_BLUE_MASK:
- dri_masks[2] = value;
+ dri_masks[2] |= value;
+ break;
+
+  case __DRI_ATTRIB_BLUE_MASK_HI:
+ dri_masks[2] |= (unsigned long long int)value << 32;
  break;
 
   case __DRI_ATTRIB_ALPHA_MASK:
- dri_masks[3] = value;
+ dri_masks[3] |= value;
+ break;
+
+  case __DRI_ATTRIB_ALPHA_MASK_HI:
+ dri_masks[3] |= (unsigned long long int)value << 32;
  break;
 
   case __DRI_ATTRIB_ACCUM_RED_SIZE:
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index 4abe1ba..3916c68 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -404,7 +404,7 @@ dri2_lookup_egl_image(__DRIscreen *screen, void *image, 
void *data);
 struct dri2_egl_config *
 dri2_add_config(_EGLDisplay *disp, const __DRIconfig *dri_config, int id,
 EGLint surface_type, const EGLint *attr_list,
-const unsigned int *rgba_masks);
+const unsigned long long int *rgba_masks);
 
 _EGLImage *
 dri2_create_image_khr(_EGLDriver *drv, _EGLDisplay *disp,
@@ -420,7 +420,7 @@ EGLBoolean
 dri2_initialize_x11(_EGLDriver *drv, _EGLDisplay *disp);
 void
 dri2_teardown_x11(struct dri2_egl_display *dri2_dpy);
-unsigned int
+unsigned long long int
 dri2_x11_get_red_mask_for_depth(struct dri2_egl_display *dri2_dpy, int depth);
 #else
 static inline EGLBoolea