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 <[email protected]>
---
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_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
...
/* __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 in some way.
[snip]
@@ -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?
Thanks
Emil
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
