[Mesa-dev] [PATCH 00/13] implement EGL_EXT_image_dma_buf_import_modifiers

[Varad Gautam]

Hello,

This series implements EGL_EXT_image_dma_buf_import_modifiers [1] which makes
it possible to pass drm fourcc modifiers to EGL when importing dmabufs.

[1] https://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_import_modifiers.txt

Pekka Paalanen (5):
egl: return error for unknown EGLImage attributes
egl: introduce DMA_BUF_MAX_PLANES
egl/main: add support for fourth plane tokens
dri: support DRIimage creation from dmabufs with modifiers
egl_dri2: add support for using modifier attributes in
eglCreateImageKHR

Varad Gautam (8):
egl: update eglext.h
st/dri: implement DRIimage creation from dmabufs with modifiers
egl: implement eglQueryDmaBufFormatsEXT
dri: add queryDmaBufModifiers to DRIimage
gallium: introduce format modifier querying
st/dri: support format modifier queries
egl: implement eglQueryDmaBufModifiersEXT
egl: advertise EGL_EXT_image_dma_buf_import_modifiers

include/EGL/eglext.h | 60 ++++++-
include/GL/internal/dri_interface.h | 31 +++-
src/egl/drivers/dri2/egl_dri2.c | 209 +++++++++++++++++++++--
src/egl/main/eglapi.c | 41 +++++
src/egl/main/eglapi.h | 9 +
src/egl/main/egldisplay.h | 1 +
src/egl/main/eglimage.c | 63 ++++++-
src/egl/main/eglimage.h | 10 +-
src/gallium/docs/source/screen.rst | 2 +
src/gallium/drivers/freedreno/freedreno_screen.c | 1 +
src/gallium/drivers/i915/i915_screen.c | 1 +
src/gallium/drivers/ilo/ilo_screen.c | 1 +
src/gallium/drivers/llvmpipe/lp_screen.c | 1 +
src/gallium/drivers/nouveau/nv30/nv30_screen.c | 1 +
src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 +
src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 1 +
src/gallium/drivers/r300/r300_screen.c | 1 +
src/gallium/drivers/r600/r600_pipe.c | 1 +
src/gallium/drivers/radeonsi/si_pipe.c | 1 +
src/gallium/drivers/softpipe/sp_screen.c | 1 +
src/gallium/drivers/svga/svga_screen.c | 1 +
src/gallium/drivers/swr/swr_screen.cpp | 1 +
src/gallium/drivers/vc4/vc4_screen.c | 1 +
src/gallium/drivers/virgl/virgl_screen.c | 1 +
src/gallium/include/pipe/p_defines.h | 1 +
src/gallium/include/pipe/p_screen.h | 7 +
src/gallium/include/state_tracker/drm_driver.h | 2 +
src/gallium/state_trackers/dri/dri2.c | 63 ++++++-
28 files changed, 487 insertions(+), 27 deletions(-)

[Varad Gautam]

From: Pekka Paalanen <***@collabora.co.uk>

Commit 0135e5d6c83add5e539492a4899504e33f3f2434 "egl: Add support for
more EGLImage extensions to EGL core." removed an error code for unknown
EGLImage attributes without explaining why.

EGL_KHR_image_base says:

* If an attribute specified in <attrib_list> is not one of the
attributes listed in Table bbb, the error EGL_BAD_PARAMETER is
generated.

Implement this.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
Signed-off-by: Varad Gautam <***@collabora.com>
CC: <mesa-***@lists.freedesktop.org>
---
src/egl/main/eglimage.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/egl/main/eglimage.c b/src/egl/main/eglimage.c
index 818b597..411d1ca 100644
--- a/src/egl/main/eglimage.c
+++ b/src/egl/main/eglimage.c
@@ -170,7 +170,7 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
break;

default:
- /* unknown attrs are ignored */
+ err = EGL_BAD_PARAMETER;
break;
}

[Varad Gautam]

From: Pekka Paalanen <***@collabora.co.uk>

Rather than hardcoding 3, use a #define. Makes it easier to bump this
later to 4.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
Signed-off-by: Varad Gautam <***@collabora.com>
---
src/egl/drivers/dri2/egl_dri2.c | 8 ++++----
src/egl/main/eglimage.h | 8 +++++---
2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index d9e2ad7..9a41ad0 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -2040,7 +2040,7 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
* generated if any of the EGL_DMA_BUF_PLANE1_* or EGL_DMA_BUF_PLANE2_*
* attributes are specified."
*/
- for (i = plane_n; i < 3; ++i) {
+ for (i = plane_n; i < DMA_BUF_MAX_PLANES; ++i) {
if (attrs->DMABufPlaneFds[i].IsPresent ||
attrs->DMABufPlaneOffsets[i].IsPresent ||
attrs->DMABufPlanePitches[i].IsPresent) {
@@ -2073,9 +2073,9 @@ dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
__DRIimage *dri_image;
unsigned num_fds;
unsigned i;
- int fds[3];
- int pitches[3];
- int offsets[3];
+ int fds[DMA_BUF_MAX_PLANES];
+ int pitches[DMA_BUF_MAX_PLANES];
+ int offsets[DMA_BUF_MAX_PLANES];
unsigned error;

/**
diff --git a/src/egl/main/eglimage.h b/src/egl/main/eglimage.h
index 0dd5e12..9a75d0c 100644
--- a/src/egl/main/eglimage.h
+++ b/src/egl/main/eglimage.h
@@ -46,6 +46,8 @@ struct _egl_image_attrib_int
EGLBoolean IsPresent;
};

+#define DMA_BUF_MAX_PLANES 3
+
struct _egl_image_attribs
{
/* EGL_KHR_image_base */
@@ -67,9 +69,9 @@ struct _egl_image_attribs

/* EGL_EXT_image_dma_buf_import */
struct _egl_image_attrib_int DMABufFourCC;
- struct _egl_image_attrib_int DMABufPlaneFds[3];
- struct _egl_image_attrib_int DMABufPlaneOffsets[3];
- struct _egl_image_attrib_int DMABufPlanePitches[3];
+ struct _egl_image_attrib_int DMABufPlaneFds[DMA_BUF_MAX_PLANES];
+ struct _egl_image_attrib_int DMABufPlaneOffsets[DMA_BUF_MAX_PLANES];
+ struct _egl_image_attrib_int DMABufPlanePitches[DMA_BUF_MAX_PLANES];
struct _egl_image_attrib_int DMABufYuvColorSpaceHint;
struct _egl_image_attrib_int DMABufSampleRangeHint;
struct _egl_image_attrib_int DMABufChromaHorizontalSiting;

[Varad Gautam]

From: Varad Gautam <***@collabora.com>

update eglext.h to revision 33288 from Khronos

Signed-off-by: Varad Gautam <***@collabora.com>
---
include/EGL/eglext.h | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 58 insertions(+), 2 deletions(-)

diff --git a/include/EGL/eglext.h b/include/EGL/eglext.h
index 4ccbab8..762b5a6 100644
--- a/include/EGL/eglext.h
+++ b/include/EGL/eglext.h
@@ -33,12 +33,12 @@ extern "C" {
** used to make the header, and the header can be found at
** http://www.opengl.org/registry/
**
-** Khronos $Revision$ on $Date$
+** Khronos $Revision: 33288 $ on $Date: 2016-11-09 17:46:01 -0800 (Wed, 09 Nov 2016) $
*/

#include <EGL/eglplatform.h>

-#define EGL_EGLEXT_VERSION 20160809
+#define EGL_EGLEXT_VERSION 20161109

/* Generated C header for:
* API: egl
@@ -77,6 +77,13 @@ EGLAPI EGLSyncKHR EGLAPIENTRY eglCreateSync64KHR (EGLDisplay dpy, EGLenum type,
#define EGL_VG_ALPHA_FORMAT_PRE_BIT_KHR 0x0040
#endif /* EGL_KHR_config_attribs */

+#ifndef EGL_KHR_context_flush_control
+#define EGL_KHR_context_flush_control 1
+#define EGL_CONTEXT_RELEASE_BEHAVIOR_NONE_KHR 0
+#define EGL_CONTEXT_RELEASE_BEHAVIOR_KHR 0x2097
+#define EGL_CONTEXT_RELEASE_BEHAVIOR_FLUSH_KHR 0x2098
+#endif /* EGL_KHR_context_flush_control */
+
#ifndef EGL_KHR_create_context
#define EGL_KHR_create_context 1
#define EGL_CONTEXT_MAJOR_VERSION_KHR 0x3098
@@ -343,6 +350,24 @@ EGLAPI EGLBoolean EGLAPIENTRY eglQueryStreamu64KHR (EGLDisplay dpy, EGLStreamKHR
#endif /* KHRONOS_SUPPORT_INT64 */
#endif /* EGL_KHR_stream */

+#ifndef EGL_KHR_stream_attrib
+#define EGL_KHR_stream_attrib 1
+#ifdef KHRONOS_SUPPORT_INT64
+typedef EGLStreamKHR (EGLAPIENTRYP PFNEGLCREATESTREAMATTRIBKHRPROC) (EGLDisplay dpy, const EGLAttrib *attrib_list);
+typedef EGLBoolean (EGLAPIENTRYP PFNEGLSETSTREAMATTRIBKHRPROC) (EGLDisplay dpy, EGLStreamKHR stream, EGLenum attribute, EGLAttrib value);
+typedef EGLBoolean (EGLAPIENTRYP PFNEGLQUERYSTREAMATTRIBKHRPROC) (EGLDisplay dpy, EGLStreamKHR stream, EGLenum attribute, EGLAttrib *value);
+typedef EGLBoolean (EGLAPIENTRYP PFNEGLSTREAMCONSUMERACQUIREATTRIBKHRPROC) (EGLDisplay dpy, EGLStreamKHR stream, const EGLAttrib *attrib_list);
+typedef EGLBoolean (EGLAPIENTRYP PFNEGLSTREAMCONSUMERRELEASEATTRIBKHRPROC) (EGLDisplay dpy, EGLStreamKHR stream, const EGLAttrib *attrib_list);
+#ifdef EGL_EGLEXT_PROTOTYPES
+EGLAPI EGLStreamKHR EGLAPIENTRY eglCreateStreamAttribKHR (EGLDisplay dpy, const EGLAttrib *attrib_list);
+EGLAPI EGLBoolean EGLAPIENTRY eglSetStreamAttribKHR (EGLDisplay dpy, EGLStreamKHR stream, EGLenum attribute, EGLAttrib value);
+EGLAPI EGLBoolean EGLAPIENTRY eglQueryStreamAttribKHR (EGLDisplay dpy, EGLStreamKHR stream, EGLenum attribute, EGLAttrib *value);
+EGLAPI EGLBoolean EGLAPIENTRY eglStreamConsumerAcquireAttribKHR (EGLDisplay dpy, EGLStreamKHR stream, const EGLAttrib *attrib_list);
+EGLAPI EGLBoolean EGLAPIENTRY eglStreamConsumerReleaseAttribKHR (EGLDisplay dpy, EGLStreamKHR stream, const EGLAttrib *attrib_list);
+#endif
+#endif /* KHRONOS_SUPPORT_INT64 */
+#endif /* EGL_KHR_stream_attrib */
+
#ifndef EGL_KHR_stream_consumer_gltexture
#define EGL_KHR_stream_consumer_gltexture 1
#ifdef EGL_KHR_stream
@@ -520,6 +545,11 @@ EGLAPI EGLBoolean EGLAPIENTRY eglQuerySurfacePointerANGLE (EGLDisplay dpy, EGLSu
#define EGL_FIXED_SIZE_ANGLE 0x3201
#endif /* EGL_ANGLE_window_fixed_size */

+#ifndef EGL_ARM_implicit_external_sync
+#define EGL_ARM_implicit_external_sync 1
+#define EGL_SYNC_PRIOR_COMMANDS_IMPLICIT_EXTERNAL_ARM 0x328A
+#endif /* EGL_ARM_implicit_external_sync */
+
#ifndef EGL_ARM_pixmap_multisample_discard
#define EGL_ARM_pixmap_multisample_discard 1
#define EGL_DISCARD_SAMPLES_ARM 0x3286
@@ -604,6 +634,27 @@ EGLAPI EGLBoolean EGLAPIENTRY eglQueryDisplayAttribEXT (EGLDisplay dpy, EGLint a
#define EGL_YUV_CHROMA_SITING_0_5_EXT 0x3285
#endif /* EGL_EXT_image_dma_buf_import */

+#ifndef EGL_EXT_image_dma_buf_import_modifiers
+#define EGL_EXT_image_dma_buf_import_modifiers 1
+#define EGL_DMA_BUF_PLANE3_FD_EXT 0x3440
+#define EGL_DMA_BUF_PLANE3_OFFSET_EXT 0x3441
+#define EGL_DMA_BUF_PLANE3_PITCH_EXT 0x3442
+#define EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT 0x3443
+#define EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT 0x3444
+#define EGL_DMA_BUF_PLANE1_MODIFIER_LO_EXT 0x3445
+#define EGL_DMA_BUF_PLANE1_MODIFIER_HI_EXT 0x3446
+#define EGL_DMA_BUF_PLANE2_MODIFIER_LO_EXT 0x3447
+#define EGL_DMA_BUF_PLANE2_MODIFIER_HI_EXT 0x3448
+#define EGL_DMA_BUF_PLANE3_MODIFIER_LO_EXT 0x3449
+#define EGL_DMA_BUF_PLANE3_MODIFIER_HI_EXT 0x344A
+typedef EGLBoolean (EGLAPIENTRYP PFNEGLQUERYDMABUFFORMATSEXTPROC) (EGLDisplay dpy, EGLint max_formats, EGLint *formats, EGLint *num_formats);
+typedef EGLBoolean (EGLAPIENTRYP PFNEGLQUERYDMABUFMODIFIERSEXTPROC) (EGLDisplay dpy, EGLint format, EGLint max_modifiers, EGLuint64KHR *modifiers, EGLBoolean *external_only, EGLint *num_modifiers);
+#ifdef EGL_EGLEXT_PROTOTYPES
+EGLAPI EGLBoolean EGLAPIENTRY eglQueryDmaBufFormatsEXT (EGLDisplay dpy, EGLint max_formats, EGLint *formats, EGLint *num_formats);
+EGLAPI EGLBoolean EGLAPIENTRY eglQueryDmaBufModifiersEXT (EGLDisplay dpy, EGLint format, EGLint max_modifiers, EGLuint64KHR *modifiers, EGLBoolean *external_only, EGLint *num_modifiers);
+#endif
+#endif /* EGL_EXT_image_dma_buf_import_modifiers */
+
#ifndef EGL_EXT_multiview_window
#define EGL_EXT_multiview_window 1
#define EGL_MULTIVIEW_VIEW_COUNT_EXT 0x3134
@@ -802,6 +853,11 @@ EGLAPI EGLBoolean EGLAPIENTRY eglExportDMABUFImageMESA (EGLDisplay dpy, EGLImage
#define EGL_PLATFORM_GBM_MESA 0x31D7
#endif /* EGL_MESA_platform_gbm */

+#ifndef EGL_MESA_platform_surfaceless
+#define EGL_MESA_platform_surfaceless 1
+#define EGL_PLATFORM_SURFACELESS_MESA 0x31DD
+#endif /* EGL_MESA_platform_surfaceless */
+
#ifndef EGL_NOK_swap_region
#define EGL_NOK_swap_region 1
typedef EGLBoolean (EGLAPIENTRYP PFNEGLSWAPBUFFERSREGIONNOKPROC) (EGLDisplay dpy, EGLSurface surface, EGLint numRects, const EGLint *rects);

[Emil Velikov]

Post by Varad Gautam
+#ifndef EGL_MESA_platform_surfaceless
+#define EGL_MESA_platform_surfaceless 1
+#define EGL_PLATFORM_SURFACELESS_MESA 0x31DD
+#endif /* EGL_MESA_platform_surfaceless */
+

Thinking out loud: is dropping the similar hunk from
include/EGL/eglmesaext.h a wise move ?
Either way - not something that should be addressed here/now.

-Emil

[Varad Gautam]

From: Pekka Paalanen <***@collabora.co.uk>

The EGL_EXT_dma_buf_import_modifiers extension adds support for a
fourth plane, just like DRM KMS API does.

Bump maximum dma_buf plane count to four.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
Signed-off-by: Varad Gautam <***@collabora.com>
---
src/egl/drivers/dri2/egl_dri2.c | 2 +-
src/egl/main/eglimage.c | 12 ++++++++++++
src/egl/main/eglimage.h | 2 +-
3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 9a41ad0..58d16e1 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -2038,7 +2038,7 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
* "If <target> is EGL_LINUX_DMA_BUF_EXT, and the EGL_LINUX_DRM_FOURCC_EXT
* attribute indicates a single-plane format, EGL_BAD_ATTRIBUTE is
* generated if any of the EGL_DMA_BUF_PLANE1_* or EGL_DMA_BUF_PLANE2_*
- * attributes are specified."
+ * or EGL_DMA_BUF_PLANE3_* attributes are specified."
*/
for (i = plane_n; i < DMA_BUF_MAX_PLANES; ++i) {
if (attrs->DMABufPlaneFds[i].IsPresent ||
diff --git a/src/egl/main/eglimage.c b/src/egl/main/eglimage.c
index 411d1ca..cd170c6 100644
--- a/src/egl/main/eglimage.c
+++ b/src/egl/main/eglimage.c
@@ -133,6 +133,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[2].Value = val;
attrs->DMABufPlanePitches[2].IsPresent = EGL_TRUE;
break;
+ case EGL_DMA_BUF_PLANE3_FD_EXT:
+ attrs->DMABufPlaneFds[3].Value = val;
+ attrs->DMABufPlaneFds[3].IsPresent = EGL_TRUE;
+ break;
+ case EGL_DMA_BUF_PLANE3_OFFSET_EXT:
+ attrs->DMABufPlaneOffsets[3].Value = val;
+ attrs->DMABufPlaneOffsets[3].IsPresent = EGL_TRUE;
+ break;
+ case EGL_DMA_BUF_PLANE3_PITCH_EXT:
+ attrs->DMABufPlanePitches[3].Value = val;
+ attrs->DMABufPlanePitches[3].IsPresent = EGL_TRUE;
+ break;
case EGL_YUV_COLOR_SPACE_HINT_EXT:
if (val != EGL_ITU_REC601_EXT && val != EGL_ITU_REC709_EXT &&
val != EGL_ITU_REC2020_EXT) {
diff --git a/src/egl/main/eglimage.h b/src/egl/main/eglimage.h
index 9a75d0c..2c110c7 100644
--- a/src/egl/main/eglimage.h
+++ b/src/egl/main/eglimage.h
@@ -46,7 +46,7 @@ struct _egl_image_attrib_int
EGLBoolean IsPresent;
};

-#define DMA_BUF_MAX_PLANES 3
+#define DMA_BUF_MAX_PLANES 4

struct _egl_image_attribs
{

[Emil Velikov]

Post by Varad Gautam
The EGL_EXT_dma_buf_import_modifiers extension adds support for a
fourth plane, just like DRM KMS API does.
Bump maximum dma_buf plane count to four.
---
src/egl/drivers/dri2/egl_dri2.c | 2 +-
src/egl/main/eglimage.c | 12 ++++++++++++
src/egl/main/eglimage.h | 2 +-
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 9a41ad0..58d16e1 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -2038,7 +2038,7 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
* "If <target> is EGL_LINUX_DMA_BUF_EXT, and the EGL_LINUX_DRM_FOURCC_EXT
* attribute indicates a single-plane format, EGL_BAD_ATTRIBUTE is
* generated if any of the EGL_DMA_BUF_PLANE1_* or EGL_DMA_BUF_PLANE2_*
- * attributes are specified."
+ * or EGL_DMA_BUF_PLANE3_* attributes are specified."
*/
for (i = plane_n; i < DMA_BUF_MAX_PLANES; ++i) {
if (attrs->DMABufPlaneFds[i].IsPresent ||
diff --git a/src/egl/main/eglimage.c b/src/egl/main/eglimage.c
index 411d1ca..cd170c6 100644
--- a/src/egl/main/eglimage.c
+++ b/src/egl/main/eglimage.c
@@ -133,6 +133,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[2].Value = val;
attrs->DMABufPlanePitches[2].IsPresent = EGL_TRUE;
break;
+ attrs->DMABufPlaneFds[3].Value = val;
+ attrs->DMABufPlaneFds[3].IsPresent = EGL_TRUE;
+ break;
+ attrs->DMABufPlaneOffsets[3].Value = val;
+ attrs->DMABufPlaneOffsets[3].IsPresent = EGL_TRUE;
+ break;
+ attrs->DMABufPlanePitches[3].Value = val;
+ attrs->DMABufPlanePitches[3].IsPresent = EGL_TRUE;
+ break;

These should be within an extension guard. Otherwise we'll parse them
(and try to push down) even if we don't support them.

Something line the following should do it. Either squashed here or
separate patch is fine.

src/egl/main/egldisplay.h
+ EGLBoolean EGL_EXT_dma_buf_import_modifiers;

src/egl/main/eglapi.c
+ _EGL_CHECK_EXTENSION(EGL_EXT_dma_buf_import_modifiers);

src/egl/main/eglimage.h
- /* EGL_EXT_image_dma_buf_import */
+ /* EGL_EXT_image_dma_buf_import and EGL_EXT_dma_buf_import_modifiers */

src/egl/main/eglimage.c
+ case EGL_DMA_BUF_PLANE3_FD_EXT:
+ if (!disp->Extensions.EXT_dma_buf_import_modifiers) {
+ err = EGL_BAD_ATTRIBUTE;
+ break;
+ }
+ attrs->DMABufPlaneFds[3].Value = val;
+ attrs->DMABufPlaneFds[3].IsPresent = EGL_TRUE;
+ break;
and same for OFFSET and PITCH.

IMHO we want to keep the new code relatively bug free, so it's better
to address those irrespective of the bugs/extra work mentioned below.

Afaict none of the existing attribs honour their respective extension
(bool). Some of them are kind of ok like EGL_EXT_image_dma_buf_import
were we don't have the API/vfunc so even if we parse the values we
cannot push them further down. Either way correct extensions' attrib
parsing can be addressed, as independent work at a later point in
time.

Thanks
Emil

[Ben Widawsky]

Post by Emil Velikov

Post by Varad Gautam
The EGL_EXT_dma_buf_import_modifiers extension adds support for a
fourth plane, just like DRM KMS API does.
Bump maximum dma_buf plane count to four.
---
src/egl/drivers/dri2/egl_dri2.c | 2 +-
src/egl/main/eglimage.c | 12 ++++++++++++
src/egl/main/eglimage.h | 2 +-
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 9a41ad0..58d16e1 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -2038,7 +2038,7 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
* "If <target> is EGL_LINUX_DMA_BUF_EXT, and the EGL_LINUX_DRM_FOURCC_EXT
* attribute indicates a single-plane format, EGL_BAD_ATTRIBUTE is
* generated if any of the EGL_DMA_BUF_PLANE1_* or EGL_DMA_BUF_PLANE2_*
- * attributes are specified."
+ * or EGL_DMA_BUF_PLANE3_* attributes are specified."
*/
for (i = plane_n; i < DMA_BUF_MAX_PLANES; ++i) {
if (attrs->DMABufPlaneFds[i].IsPresent ||
diff --git a/src/egl/main/eglimage.c b/src/egl/main/eglimage.c
index 411d1ca..cd170c6 100644
--- a/src/egl/main/eglimage.c
+++ b/src/egl/main/eglimage.c
@@ -133,6 +133,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[2].Value = val;
attrs->DMABufPlanePitches[2].IsPresent = EGL_TRUE;
break;
+ attrs->DMABufPlaneFds[3].Value = val;
+ attrs->DMABufPlaneFds[3].IsPresent = EGL_TRUE;
+ break;
+ attrs->DMABufPlaneOffsets[3].Value = val;
+ attrs->DMABufPlaneOffsets[3].IsPresent = EGL_TRUE;
+ break;
+ attrs->DMABufPlanePitches[3].Value = val;
+ attrs->DMABufPlanePitches[3].IsPresent = EGL_TRUE;
+ break;

These should be within an extension guard. Otherwise we'll parse them
(and try to push down) even if we don't support them.
Something line the following should do it. Either squashed here or
separate patch is fine.
src/egl/main/egldisplay.h
+ EGLBoolean EGL_EXT_dma_buf_import_modifiers;
src/egl/main/eglapi.c
+ _EGL_CHECK_EXTENSION(EGL_EXT_dma_buf_import_modifiers);
src/egl/main/eglimage.h
- /* EGL_EXT_image_dma_buf_import */
+ /* EGL_EXT_image_dma_buf_import and EGL_EXT_dma_buf_import_modifiers */
src/egl/main/eglimage.c
+ if (!disp->Extensions.EXT_dma_buf_import_modifiers) {
+ err = EGL_BAD_ATTRIBUTE;
+ break;
+ }
+ attrs->DMABufPlaneFds[3].Value = val;
+ attrs->DMABufPlaneFds[3].IsPresent = EGL_TRUE;
+ break;
and same for OFFSET and PITCH.
IMHO we want to keep the new code relatively bug free, so it's better
to address those irrespective of the bugs/extra work mentioned below.
Afaict none of the existing attribs honour their respective extension
(bool). Some of them are kind of ok like EGL_EXT_image_dma_buf_import
were we don't have the API/vfunc so even if we parse the values we
cannot push them further down. Either way correct extensions' attrib
parsing can be addressed, as independent work at a later point in
time.
Thanks
Emil

I never bothered to understand how the attrib_list gets populated, so I'll
assume you're correct about this. So with Emil's suggestion, everything up to
here is:
Reviewed-by: Ben Widawsky <***@bwidawsk.net>

[Varad Gautam]

From: Pekka Paalanen <***@collabora.co.uk>

allow creating EGLImages with dmabuf format modifiers when target is
EGL_LINUX_DMA_BUF_EXT for EGL_EXT_image_dma_buf_import_modifiers.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
Signed-off-by: Varad Gautam <***@collabora.com>
---
src/egl/drivers/dri2/egl_dri2.c | 68 ++++++++++++++++++++++++++++++++++-------
src/egl/main/egldisplay.h | 1 +
src/egl/main/eglimage.c | 49 +++++++++++++++++++++++++++++
src/egl/main/eglimage.h | 2 ++
4 files changed, 109 insertions(+), 11 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 58d16e1..4eb1861 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1937,6 +1937,21 @@ dri2_check_dma_buf_attribs(const _EGLImageAttribs *attrs)
}
}

+ /**
+ * If <target> is EGL_LINUX_DMA_BUF_EXT, both or neither of the following
+ * attribute values may be given.
+ *
+ * This is referring to EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT and
+ * EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT, and the same for other planes.
+ */
+ for (i = 0; i < DMA_BUF_MAX_PLANES; ++i) {
+ if (attrs->DMABufPlaneModifiersLo[i].IsPresent !=
+ attrs->DMABufPlaneModifiersHi[i].IsPresent) {
+ _eglError(EGL_BAD_PARAMETER, "modifier attribute lo or hi missing");
+ return EGL_FALSE;
+ }
+ }
+
return EGL_TRUE;
}

@@ -2043,7 +2058,9 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
for (i = plane_n; i < DMA_BUF_MAX_PLANES; ++i) {
if (attrs->DMABufPlaneFds[i].IsPresent ||
attrs->DMABufPlaneOffsets[i].IsPresent ||
- attrs->DMABufPlanePitches[i].IsPresent) {
+ attrs->DMABufPlanePitches[i].IsPresent ||
+ attrs->DMABufPlaneModifiersLo[i].IsPresent ||
+ attrs->DMABufPlaneModifiersHi[i].IsPresent) {
_eglError(EGL_BAD_ATTRIBUTE, "too many plane attributes");
return 0;
}
@@ -2076,6 +2093,8 @@ dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
int fds[DMA_BUF_MAX_PLANES];
int pitches[DMA_BUF_MAX_PLANES];
int offsets[DMA_BUF_MAX_PLANES];
+ uint64_t modifiers[DMA_BUF_MAX_PLANES];
+ int nonzero_modifier_found = 0;
unsigned error;

/**
@@ -2106,18 +2125,45 @@ dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
fds[i] = attrs.DMABufPlaneFds[i].Value;
pitches[i] = attrs.DMABufPlanePitches[i].Value;
offsets[i] = attrs.DMABufPlaneOffsets[i].Value;
+ if (attrs.DMABufPlaneModifiersLo[i].IsPresent) {
+ modifiers[i] = attrs.DMABufPlaneModifiersHi[i].Value;
+ modifiers[i] = (modifiers[i] << 32) |
+ attrs.DMABufPlaneModifiersLo[i].Value;
+ if (modifiers[i] != 0)
+ nonzero_modifier_found = EGL_TRUE;
+ } else {
+ modifiers[i] = 0;
+ }
}

- dri_image =
- dri2_dpy->image->createImageFromDmaBufs(dri2_dpy->dri_screen,
- attrs.Width, attrs.Height, attrs.DMABufFourCC.Value,
- fds, num_fds, pitches, offsets,
- attrs.DMABufYuvColorSpaceHint.Value,
- attrs.DMABufSampleRangeHint.Value,
- attrs.DMABufChromaHorizontalSiting.Value,
- attrs.DMABufChromaVerticalSiting.Value,
- &error,
- NULL);
+ if (nonzero_modifier_found && dri2_dpy->image->createImageFromDmaBufs2) {
+ dri_image =
+ dri2_dpy->image->createImageFromDmaBufs2(dri2_dpy->dri_screen,
+ attrs.Width, attrs.Height, attrs.DMABufFourCC.Value,
+ fds, num_fds, pitches, offsets, modifiers,
+ attrs.DMABufYuvColorSpaceHint.Value,
+ attrs.DMABufSampleRangeHint.Value,
+ attrs.DMABufChromaHorizontalSiting.Value,
+ attrs.DMABufChromaVerticalSiting.Value,
+ &error,
+ NULL);
+ } else {
+ if (nonzero_modifier_found) {
+ _eglError(EGL_BAD_MATCH, "unsupported dma_buf format modifier");
+ return EGL_NO_IMAGE_KHR;
+ }
+
+ dri_image =
+ dri2_dpy->image->createImageFromDmaBufs(dri2_dpy->dri_screen,
+ attrs.Width, attrs.Height, attrs.DMABufFourCC.Value,
+ fds, num_fds, pitches, offsets,
+ attrs.DMABufYuvColorSpaceHint.Value,
+ attrs.DMABufSampleRangeHint.Value,
+ attrs.DMABufChromaHorizontalSiting.Value,
+ attrs.DMABufChromaVerticalSiting.Value,
+ &error,
+ NULL);
+ }
dri2_create_image_khr_texture_error(error);

if (!dri_image)
diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
index 62d9a11..7a59dc5 100644
--- a/src/egl/main/egldisplay.h
+++ b/src/egl/main/egldisplay.h
@@ -101,6 +101,7 @@ struct _egl_extensions
EGLBoolean EXT_buffer_age;
EGLBoolean EXT_create_context_robustness;
EGLBoolean EXT_image_dma_buf_import;
+ EGLBoolean EXT_image_dma_buf_import_modifiers;
EGLBoolean EXT_swap_buffers_with_damage;

EGLBoolean KHR_cl_event2;
diff --git a/src/egl/main/eglimage.c b/src/egl/main/eglimage.c
index cd170c6..f98b937 100644
--- a/src/egl/main/eglimage.c
+++ b/src/egl/main/eglimage.c
@@ -109,6 +109,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[0].Value = val;
attrs->DMABufPlanePitches[0].IsPresent = EGL_TRUE;
break;
+ case EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersLo[0].Value = val;
+ attrs->DMABufPlaneModifiersLo[0].IsPresent = EGL_TRUE;
+ break;
+ case EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersHi[0].Value = val;
+ attrs->DMABufPlaneModifiersHi[0].IsPresent = EGL_TRUE;
+ break;
case EGL_DMA_BUF_PLANE1_FD_EXT:
attrs->DMABufPlaneFds[1].Value = val;
attrs->DMABufPlaneFds[1].IsPresent = EGL_TRUE;
@@ -121,6 +133,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[1].Value = val;
attrs->DMABufPlanePitches[1].IsPresent = EGL_TRUE;
break;
+ case EGL_DMA_BUF_PLANE1_MODIFIER_LO_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersLo[1].Value = val;
+ attrs->DMABufPlaneModifiersLo[1].IsPresent = EGL_TRUE;
+ break;
+ case EGL_DMA_BUF_PLANE1_MODIFIER_HI_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersHi[1].Value = val;
+ attrs->DMABufPlaneModifiersHi[1].IsPresent = EGL_TRUE;
+ break;
case EGL_DMA_BUF_PLANE2_FD_EXT:
attrs->DMABufPlaneFds[2].Value = val;
attrs->DMABufPlaneFds[2].IsPresent = EGL_TRUE;
@@ -133,6 +157,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[2].Value = val;
attrs->DMABufPlanePitches[2].IsPresent = EGL_TRUE;
break;
+ case EGL_DMA_BUF_PLANE2_MODIFIER_LO_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersLo[2].Value = val;
+ attrs->DMABufPlaneModifiersLo[2].IsPresent = EGL_TRUE;
+ break;
+ case EGL_DMA_BUF_PLANE2_MODIFIER_HI_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersHi[2].Value = val;
+ attrs->DMABufPlaneModifiersHi[2].IsPresent = EGL_TRUE;
+ break;
case EGL_DMA_BUF_PLANE3_FD_EXT:
attrs->DMABufPlaneFds[3].Value = val;
attrs->DMABufPlaneFds[3].IsPresent = EGL_TRUE;
@@ -145,6 +181,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[3].Value = val;
attrs->DMABufPlanePitches[3].IsPresent = EGL_TRUE;
break;
+ case EGL_DMA_BUF_PLANE3_MODIFIER_LO_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersLo[3].Value = val;
+ attrs->DMABufPlaneModifiersLo[3].IsPresent = EGL_TRUE;
+ break;
+ case EGL_DMA_BUF_PLANE3_MODIFIER_HI_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersHi[3].Value = val;
+ attrs->DMABufPlaneModifiersHi[3].IsPresent = EGL_TRUE;
+ break;
case EGL_YUV_COLOR_SPACE_HINT_EXT:
if (val != EGL_ITU_REC601_EXT && val != EGL_ITU_REC709_EXT &&
val != EGL_ITU_REC2020_EXT) {
@@ -181,6 +229,7 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
}
break;

+out:
default:
err = EGL_BAD_PARAMETER;
break;
diff --git a/src/egl/main/eglimage.h b/src/egl/main/eglimage.h
index 2c110c7..98acd88 100644
--- a/src/egl/main/eglimage.h
+++ b/src/egl/main/eglimage.h
@@ -72,6 +72,8 @@ struct _egl_image_attribs
struct _egl_image_attrib_int DMABufPlaneFds[DMA_BUF_MAX_PLANES];
struct _egl_image_attrib_int DMABufPlaneOffsets[DMA_BUF_MAX_PLANES];
struct _egl_image_attrib_int DMABufPlanePitches[DMA_BUF_MAX_PLANES];
+ struct _egl_image_attrib_int DMABufPlaneModifiersLo[DMA_BUF_MAX_PLANES];
+ struct _egl_image_attrib_int DMABufPlaneModifiersHi[DMA_BUF_MAX_PLANES];
struct _egl_image_attrib_int DMABufYuvColorSpaceHint;
struct _egl_image_attrib_int DMABufSampleRangeHint;
struct _egl_image_attrib_int DMABufChromaHorizontalSiting;

[Eric Engestrom]

Post by Varad Gautam
allow creating EGLImages with dmabuf format modifiers when target is
EGL_LINUX_DMA_BUF_EXT for EGL_EXT_image_dma_buf_import_modifiers.
---
src/egl/drivers/dri2/egl_dri2.c | 68 ++++++++++++++++++++++++++++++++++-------
src/egl/main/egldisplay.h | 1 +
src/egl/main/eglimage.c | 49 +++++++++++++++++++++++++++++
src/egl/main/eglimage.h | 2 ++
4 files changed, 109 insertions(+), 11 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 58d16e1..4eb1861 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1937,6 +1937,21 @@ dri2_check_dma_buf_attribs(const _EGLImageAttribs *attrs)
}
}
+ /**
+ * If <target> is EGL_LINUX_DMA_BUF_EXT, both or neither of the following
+ * attribute values may be given.
+ *
+ * This is referring to EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT and
+ * EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT, and the same for other planes.
+ */
+ for (i = 0; i < DMA_BUF_MAX_PLANES; ++i) {
+ if (attrs->DMABufPlaneModifiersLo[i].IsPresent !=
+ attrs->DMABufPlaneModifiersHi[i].IsPresent) {
+ _eglError(EGL_BAD_PARAMETER, "modifier attribute lo or hi missing");
+ return EGL_FALSE;
+ }
+ }
+
return EGL_TRUE;
}
@@ -2043,7 +2058,9 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
for (i = plane_n; i < DMA_BUF_MAX_PLANES; ++i) {
if (attrs->DMABufPlaneFds[i].IsPresent ||
attrs->DMABufPlaneOffsets[i].IsPresent ||
- attrs->DMABufPlanePitches[i].IsPresent) {
+ attrs->DMABufPlanePitches[i].IsPresent ||
+ attrs->DMABufPlaneModifiersLo[i].IsPresent ||
+ attrs->DMABufPlaneModifiersHi[i].IsPresent) {
_eglError(EGL_BAD_ATTRIBUTE, "too many plane attributes");
return 0;
}
@@ -2076,6 +2093,8 @@ dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
int fds[DMA_BUF_MAX_PLANES];
int pitches[DMA_BUF_MAX_PLANES];
int offsets[DMA_BUF_MAX_PLANES];
+ uint64_t modifiers[DMA_BUF_MAX_PLANES];
+ int nonzero_modifier_found = 0;
unsigned error;
/**
@@ -2106,18 +2125,45 @@ dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
fds[i] = attrs.DMABufPlaneFds[i].Value;
pitches[i] = attrs.DMABufPlanePitches[i].Value;
offsets[i] = attrs.DMABufPlaneOffsets[i].Value;
+ if (attrs.DMABufPlaneModifiersLo[i].IsPresent) {
+ modifiers[i] = attrs.DMABufPlaneModifiersHi[i].Value;
+ modifiers[i] = (modifiers[i] << 32) |
+ attrs.DMABufPlaneModifiersLo[i].Value;

I'd prefer this, it's clearer IMO, and avoids temporarily invalid values
(eg. `hi` unshifted):

modifiers[i] = attrs.DMABufPlaneModifiersHi[i].Value << 32 |
attrs.DMABufPlaneModifiersLo[i].Value;

Otherwise, it all looks good to me (one nit-pick below), but I don't
know Gallium enough, so I'm only r-b'ing the EGL bits.

Post by Varad Gautam
+ if (modifiers[i] != 0)
+ nonzero_modifier_found = EGL_TRUE;
+ } else {
+ modifiers[i] = 0;
+ }
}
- dri_image =
- dri2_dpy->image->createImageFromDmaBufs(dri2_dpy->dri_screen,
- attrs.Width, attrs.Height, attrs.DMABufFourCC.Value,
- fds, num_fds, pitches, offsets,
- attrs.DMABufYuvColorSpaceHint.Value,
- attrs.DMABufSampleRangeHint.Value,
- attrs.DMABufChromaHorizontalSiting.Value,
- attrs.DMABufChromaVerticalSiting.Value,
- &error,
- NULL);
+ if (nonzero_modifier_found && dri2_dpy->image->createImageFromDmaBufs2) {
+ dri_image =
+ dri2_dpy->image->createImageFromDmaBufs2(dri2_dpy->dri_screen,
+ attrs.Width, attrs.Height, attrs.DMABufFourCC.Value,
+ fds, num_fds, pitches, offsets, modifiers,
+ attrs.DMABufYuvColorSpaceHint.Value,
+ attrs.DMABufSampleRangeHint.Value,
+ attrs.DMABufChromaHorizontalSiting.Value,
+ attrs.DMABufChromaVerticalSiting.Value,
+ &error,
+ NULL);
+ } else {
+ if (nonzero_modifier_found) {
+ _eglError(EGL_BAD_MATCH, "unsupported dma_buf format modifier");
+ return EGL_NO_IMAGE_KHR;
+ }
+
+ dri_image =
+ dri2_dpy->image->createImageFromDmaBufs(dri2_dpy->dri_screen,
+ attrs.Width, attrs.Height, attrs.DMABufFourCC.Value,
+ fds, num_fds, pitches, offsets,
+ attrs.DMABufYuvColorSpaceHint.Value,
+ attrs.DMABufSampleRangeHint.Value,
+ attrs.DMABufChromaHorizontalSiting.Value,
+ attrs.DMABufChromaVerticalSiting.Value,
+ &error,
+ NULL);
+ }
dri2_create_image_khr_texture_error(error);
if (!dri_image)
diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
index 62d9a11..7a59dc5 100644
--- a/src/egl/main/egldisplay.h
+++ b/src/egl/main/egldisplay.h
@@ -101,6 +101,7 @@ struct _egl_extensions
EGLBoolean EXT_buffer_age;
EGLBoolean EXT_create_context_robustness;
EGLBoolean EXT_image_dma_buf_import;
+ EGLBoolean EXT_image_dma_buf_import_modifiers;
EGLBoolean EXT_swap_buffers_with_damage;
EGLBoolean KHR_cl_event2;
diff --git a/src/egl/main/eglimage.c b/src/egl/main/eglimage.c
index cd170c6..f98b937 100644
--- a/src/egl/main/eglimage.c
+++ b/src/egl/main/eglimage.c
@@ -109,6 +109,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[0].Value = val;
attrs->DMABufPlanePitches[0].IsPresent = EGL_TRUE;
break;
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersLo[0].Value = val;
+ attrs->DMABufPlaneModifiersLo[0].IsPresent = EGL_TRUE;
+ break;
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersHi[0].Value = val;
+ attrs->DMABufPlaneModifiersHi[0].IsPresent = EGL_TRUE;
+ break;
attrs->DMABufPlaneFds[1].Value = val;
attrs->DMABufPlaneFds[1].IsPresent = EGL_TRUE;
@@ -121,6 +133,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[1].Value = val;
attrs->DMABufPlanePitches[1].IsPresent = EGL_TRUE;
break;
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersLo[1].Value = val;
+ attrs->DMABufPlaneModifiersLo[1].IsPresent = EGL_TRUE;
+ break;
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersHi[1].Value = val;
+ attrs->DMABufPlaneModifiersHi[1].IsPresent = EGL_TRUE;
+ break;
attrs->DMABufPlaneFds[2].Value = val;
attrs->DMABufPlaneFds[2].IsPresent = EGL_TRUE;
@@ -133,6 +157,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[2].Value = val;
attrs->DMABufPlanePitches[2].IsPresent = EGL_TRUE;
break;
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersLo[2].Value = val;
+ attrs->DMABufPlaneModifiersLo[2].IsPresent = EGL_TRUE;
+ break;
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersHi[2].Value = val;
+ attrs->DMABufPlaneModifiersHi[2].IsPresent = EGL_TRUE;
+ break;
attrs->DMABufPlaneFds[3].Value = val;
attrs->DMABufPlaneFds[3].IsPresent = EGL_TRUE;
@@ -145,6 +181,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[3].Value = val;
attrs->DMABufPlanePitches[3].IsPresent = EGL_TRUE;
break;
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersLo[3].Value = val;
+ attrs->DMABufPlaneModifiersLo[3].IsPresent = EGL_TRUE;
+ break;
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto out;
+ attrs->DMABufPlaneModifiersHi[3].Value = val;
+ attrs->DMABufPlaneModifiersHi[3].IsPresent = EGL_TRUE;
+ break;
if (val != EGL_ITU_REC601_EXT && val != EGL_ITU_REC709_EXT &&
val != EGL_ITU_REC2020_EXT) {
@@ -181,6 +229,7 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
}
break;

Nit: call this `fail`, `error`, `bad_param`, or anything else that
indicates that it's an error path?

Cheers,
Eric

[Varad Gautam]

From: Varad Gautam <***@collabora.com>

queryDmaBufModifiers function allows querying implementation
supported dmabuf format modifiers for a given format.

Signed-off-by: Varad Gautam <***@collabora.com>
---
include/GL/internal/dri_interface.h | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
index 4874e59..c3a6b3d 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -1094,7 +1094,7 @@ struct __DRIdri2ExtensionRec {
* extensions.
*/
#define __DRI_IMAGE "DRI_IMAGE"
-#define __DRI_IMAGE_VERSION 14
+#define __DRI_IMAGE_VERSION 15


/**
@@ -1439,6 +1439,16 @@ struct __DRIimageExtensionRec {
enum __DRIChromaSiting vert_siting,
unsigned *error,
void *loaderPrivate);
+
+ /*
+ * Returns modifiers supported by the driver for a given format.
+ *
+ * For EGL_EXT_image_dma_buf_import_modifiers.
+ *
+ * \since 15
+ */
+ void (*queryDmaBufModifiers)(__DRIscreen *screen, int fourcc, int max,
+ uint64_t *modifiers, int *count);
};

[Varad Gautam]

From: Pekka Paalanen <***@collabora.co.uk>

add createImageFromDmaBufs2 function which accepts per-plane dmabuf
format modifiers.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
Signed-off-by: Varad Gautam <***@collabora.com>
---
include/GL/internal/dri_interface.h | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
index d0b1bc6..4874e59 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -1094,7 +1094,8 @@ struct __DRIdri2ExtensionRec {
* extensions.
*/
#define __DRI_IMAGE "DRI_IMAGE"
-#define __DRI_IMAGE_VERSION 13
+#define __DRI_IMAGE_VERSION 14
+

/**
* These formats correspond to the similarly named MESA_FORMAT_*
@@ -1420,6 +1421,24 @@ struct __DRIimageExtensionRec {
*/
void (*unmapImage)(__DRIcontext *context, __DRIimage *image, void *data);

+ /*
+ * Like createImageFromDmaBufs, but takes also format modifiers.
+ *
+ * For EGL_EXT_image_dma_buf_import_modifiers.
+ *
+ * \since 14
+ */
+ __DRIimage *(*createImageFromDmaBufs2)(__DRIscreen *screen,
+ int width, int height, int fourcc,
+ int *fds, int num_fds,
+ int *strides, int *offsets,
+ uint64_t *modifiers,
+ enum __DRIYUVColorSpace color_space,
+ enum __DRISampleRange sample_range,
+ enum __DRIChromaSiting horiz_siting,
+ enum __DRIChromaSiting vert_siting,
+ unsigned *error,
+ void *loaderPrivate);
};

[Varad Gautam]

From: Varad Gautam <***@collabora.com>

allow egl clients to query the dmabuf formats supported on this platform.

Signed-off-by: Louis-Francis Ratté-Boulianne <***@collabora.com>
Signed-off-by: Varad Gautam <***@collabora.com>
---
src/egl/drivers/dri2/egl_dri2.c | 87 +++++++++++++++++++++++++++++++++++++++++
src/egl/main/eglapi.c | 19 +++++++++
src/egl/main/eglapi.h | 4 ++
3 files changed, 110 insertions(+)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 4eb1861..de2d4df 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -78,6 +78,68 @@ const __DRIuseInvalidateExtension use_invalidate = {
.base = { __DRI_USE_INVALIDATE, 1 }
};

+static const EGLint dma_buf_formats[] = {
+ DRM_FORMAT_R8,
+ DRM_FORMAT_RG88,
+ DRM_FORMAT_GR88,
+ DRM_FORMAT_RGB332,
+ DRM_FORMAT_BGR233,
+ DRM_FORMAT_XRGB4444,
+ DRM_FORMAT_XBGR4444,
+ DRM_FORMAT_RGBX4444,
+ DRM_FORMAT_BGRX4444,
+ DRM_FORMAT_ARGB4444,
+ DRM_FORMAT_ABGR4444,
+ DRM_FORMAT_RGBA4444,
+ DRM_FORMAT_BGRA4444,
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_XBGR1555,
+ DRM_FORMAT_RGBX5551,
+ DRM_FORMAT_BGRX5551,
+ DRM_FORMAT_ARGB1555,
+ DRM_FORMAT_ABGR1555,
+ DRM_FORMAT_RGBA5551,
+ DRM_FORMAT_BGRA5551,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_BGR565,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_XRGB8888,
+ DRM_FORMAT_XBGR8888,
+ DRM_FORMAT_RGBX8888,
+ DRM_FORMAT_BGRX8888,
+ DRM_FORMAT_ARGB8888,
+ DRM_FORMAT_ABGR8888,
+ DRM_FORMAT_RGBA8888,
+ DRM_FORMAT_BGRA8888,
+ DRM_FORMAT_XRGB2101010,
+ DRM_FORMAT_XBGR2101010,
+ DRM_FORMAT_RGBX1010102,
+ DRM_FORMAT_BGRX1010102,
+ DRM_FORMAT_ARGB2101010,
+ DRM_FORMAT_ABGR2101010,
+ DRM_FORMAT_RGBA1010102,
+ DRM_FORMAT_BGRA1010102,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_VYUY,
+ DRM_FORMAT_NV12,
+ DRM_FORMAT_NV21,
+ DRM_FORMAT_NV16,
+ DRM_FORMAT_NV61,
+ DRM_FORMAT_YUV410,
+ DRM_FORMAT_YVU410,
+ DRM_FORMAT_YUV411,
+ DRM_FORMAT_YVU411,
+ DRM_FORMAT_YUV420,
+ DRM_FORMAT_YVU420,
+ DRM_FORMAT_YUV422,
+ DRM_FORMAT_YVU422,
+ DRM_FORMAT_YUV444,
+ DRM_FORMAT_YVU444
+};
+
EGLint dri2_to_egl_attribute_map[] = {
0,
EGL_BUFFER_SIZE, /* __DRI_ATTRIB_BUFFER_SIZE */
@@ -2069,6 +2131,30 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
return plane_n;
}

+static EGLBoolean
+dri2_query_dma_buf_formats(_EGLDriver *drv, _EGLDisplay *disp,
+ EGLint max, EGLint *formats, EGLint *count)
+{
+ EGLint i;
+
+ if (max < 0 || (max > 0 && formats == NULL)) {
+ _eglError(EGL_BAD_PARAMETER, "invalid value for max count of formats");
+ return EGL_FALSE;
+ }
+
+ if (max == 0) {
+ *count = ARRAY_SIZE(dma_buf_formats);
+ return EGL_TRUE;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(dma_buf_formats) && i < max; i++) {
+ formats[i] = dma_buf_formats[i];
+ }
+ *count = i;
+
+ return EGL_TRUE;
+}
+
/**
* The spec says:
*
@@ -2953,6 +3039,7 @@ _eglBuiltInDriverDRI2(const char *args)
dri2_drv->base.API.ExportDRMImageMESA = dri2_export_drm_image_mesa;
dri2_drv->base.API.ExportDMABUFImageQueryMESA = dri2_export_dma_buf_image_query_mesa;
dri2_drv->base.API.ExportDMABUFImageMESA = dri2_export_dma_buf_image_mesa;
+ dri2_drv->base.API.QueryDmaBufFormatsEXT = dri2_query_dma_buf_formats;
#endif
#ifdef HAVE_WAYLAND_PLATFORM
dri2_drv->base.API.BindWaylandDisplayWL = dri2_bind_wayland_display_wl;
diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index 53340bf..53d34d8 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -2257,6 +2257,24 @@ eglQueryDebugKHR(EGLint attribute, EGLAttrib *value)
return EGL_TRUE;
}

+static EGLBoolean EGLAPIENTRY
+eglQueryDmaBufFormatsEXT(EGLDisplay dpy, EGLint max_formats,
+ EGLint *formats, EGLint *num_formats)
+{
+ _EGLDisplay *disp = _eglLockDisplay(dpy);
+ _EGLDriver *drv;
+ EGLBoolean ret;
+
+ _EGL_FUNC_START(NULL, EGL_NONE, NULL, EGL_FALSE);
+
+ _EGL_CHECK_DISPLAY(disp, EGL_FALSE, drv);
+
+ ret = drv->API.QueryDmaBufFormatsEXT(drv, disp, max_formats, formats,
+ num_formats);
+
+ RETURN_EGL_EVAL(disp, ret);
+}
+
__eglMustCastToProperFunctionPointerType EGLAPIENTRY
eglGetProcAddress(const char *procname)
{
@@ -2340,6 +2358,7 @@ eglGetProcAddress(const char *procname)
{ "eglLabelObjectKHR", (_EGLProc) eglLabelObjectKHR },
{ "eglDebugMessageControlKHR", (_EGLProc) eglDebugMessageControlKHR },
{ "eglQueryDebugKHR", (_EGLProc) eglQueryDebugKHR },
+ { "eglQueryDmaBufFormatsEXT", (_EGLProc) eglQueryDmaBufFormatsEXT },
{ NULL, NULL }
};
EGLint i;
diff --git a/src/egl/main/eglapi.h b/src/egl/main/eglapi.h
index b9bcc8e..13388b1 100644
--- a/src/egl/main/eglapi.h
+++ b/src/egl/main/eglapi.h
@@ -196,6 +196,10 @@ struct _egl_api
int (*GLInteropExportObject)(_EGLDisplay *dpy, _EGLContext *ctx,
struct mesa_glinterop_export_in *in,
struct mesa_glinterop_export_out *out);
+
+ EGLBoolean (*QueryDmaBufFormatsEXT)(_EGLDriver *drv, _EGLDisplay *dpy,
+ EGLint max_formats, EGLint *formats,
+ EGLint *num_formats);
};

EGLint _eglConvertIntsToAttribs(const EGLint *int_list,

[Emil Velikov]

Post by Varad Gautam
allow egl clients to query the dmabuf formats supported on this platform.
---
src/egl/drivers/dri2/egl_dri2.c | 87 +++++++++++++++++++++++++++++++++++++++++
src/egl/main/eglapi.c | 19 +++++++++
src/egl/main/eglapi.h | 4 ++
3 files changed, 110 insertions(+)
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 4eb1861..de2d4df 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -78,6 +78,68 @@ const __DRIuseInvalidateExtension use_invalidate = {
.base = { __DRI_USE_INVALIDATE, 1 }
};
+static const EGLint dma_buf_formats[] = {
+ DRM_FORMAT_R8,
+ DRM_FORMAT_RG88,
+ DRM_FORMAT_GR88,
+ DRM_FORMAT_RGB332,
+ DRM_FORMAT_BGR233,
+ DRM_FORMAT_XRGB4444,
+ DRM_FORMAT_XBGR4444,
+ DRM_FORMAT_RGBX4444,
+ DRM_FORMAT_BGRX4444,
+ DRM_FORMAT_ARGB4444,
+ DRM_FORMAT_ABGR4444,
+ DRM_FORMAT_RGBA4444,
+ DRM_FORMAT_BGRA4444,
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_XBGR1555,
+ DRM_FORMAT_RGBX5551,
+ DRM_FORMAT_BGRX5551,
+ DRM_FORMAT_ARGB1555,
+ DRM_FORMAT_ABGR1555,
+ DRM_FORMAT_RGBA5551,
+ DRM_FORMAT_BGRA5551,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_BGR565,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_XRGB8888,
+ DRM_FORMAT_XBGR8888,
+ DRM_FORMAT_RGBX8888,
+ DRM_FORMAT_BGRX8888,
+ DRM_FORMAT_ARGB8888,
+ DRM_FORMAT_ABGR8888,
+ DRM_FORMAT_RGBA8888,
+ DRM_FORMAT_BGRA8888,
+ DRM_FORMAT_XRGB2101010,
+ DRM_FORMAT_XBGR2101010,
+ DRM_FORMAT_RGBX1010102,
+ DRM_FORMAT_BGRX1010102,
+ DRM_FORMAT_ARGB2101010,
+ DRM_FORMAT_ABGR2101010,
+ DRM_FORMAT_RGBA1010102,
+ DRM_FORMAT_BGRA1010102,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_VYUY,
+ DRM_FORMAT_NV12,
+ DRM_FORMAT_NV21,
+ DRM_FORMAT_NV16,
+ DRM_FORMAT_NV61,
+ DRM_FORMAT_YUV410,
+ DRM_FORMAT_YVU410,
+ DRM_FORMAT_YUV411,
+ DRM_FORMAT_YVU411,
+ DRM_FORMAT_YUV420,
+ DRM_FORMAT_YVU420,
+ DRM_FORMAT_YUV422,
+ DRM_FORMAT_YVU422,
+ DRM_FORMAT_YUV444,
+ DRM_FORMAT_YVU444
+};
+
EGLint dri2_to_egl_attribute_map[] = {
0,
EGL_BUFFER_SIZE, /* __DRI_ATTRIB_BUFFER_SIZE */
@@ -2069,6 +2131,30 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
return plane_n;
}
+static EGLBoolean
+dri2_query_dma_buf_formats(_EGLDriver *drv, _EGLDisplay *disp,
+ EGLint max, EGLint *formats, EGLint *count)
+{
+ EGLint i;
+
+ if (max < 0 || (max > 0 && formats == NULL)) {
+ _eglError(EGL_BAD_PARAMETER, "invalid value for max count of formats");
+ return EGL_FALSE;
+ }
+
+ if (max == 0) {
+ *count = ARRAY_SIZE(dma_buf_formats);
+ return EGL_TRUE;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(dma_buf_formats) && i < max; i++) {
+ formats[i] = dma_buf_formats[i];
+ }

Returning every format imaginable as supported then most drivers
(currently) support up-to half of them is very misleading.
Worth adding another DRIimage callback, reuse existing one(s) and/or
add some other heuristics ?

Emil

[Varad Gautam]

From: Varad Gautam <***@collabora.com>

query and return supported dmabuf format modifiers for
EGL_EXT_image_dma_buf_import_modifiers.

Signed-off-by: Varad Gautam <***@collabora.com>
---
src/egl/drivers/dri2/egl_dri2.c | 39 +++++++++++++++++++++++++++++++++++++++
src/egl/main/eglapi.c | 21 +++++++++++++++++++++
src/egl/main/eglapi.h | 5 +++++
3 files changed, 65 insertions(+)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index de2d4df..443e0a3 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -2155,6 +2155,44 @@ dri2_query_dma_buf_formats(_EGLDriver *drv, _EGLDisplay *disp,
return EGL_TRUE;
}

+static EGLBoolean
+dri2_query_dma_buf_modifiers(_EGLDriver *drv, _EGLDisplay *disp, EGLint format,
+ EGLint max, EGLuint64KHR *modifiers,
+ EGLBoolean *external_only, EGLint *count)
+{
+ struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
+ EGLint i;
+
+ if (max < 0) {
+ _eglError(EGL_BAD_PARAMETER, "invalid value for max count of formats");
+ return EGL_FALSE;
+ }
+
+ if (max > 0 && modifiers == NULL) {
+ _eglError(EGL_BAD_PARAMETER, "invalid modifiers array");
+ return EGL_FALSE;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(dma_buf_formats); i++) {
+ if (format == dma_buf_formats[i])
+ break;
+ }
+ if (i == ARRAY_SIZE(dma_buf_formats)) {
+ _eglError(EGL_BAD_PARAMETER, "invalid format");
+ return EGL_FALSE;
+ }
+
+ dri2_dpy->image->queryDmaBufModifiers(dri2_dpy->dri_screen, format, max,
+ modifiers, count);
+
+ if (external_only != NULL) {
+ for (i = 0; i < *count && i < max; i++)
+ external_only[i] = EGL_TRUE;
+ }
+
+ return EGL_TRUE;
+}
+
/**
* The spec says:
*
@@ -3040,6 +3078,7 @@ _eglBuiltInDriverDRI2(const char *args)
dri2_drv->base.API.ExportDMABUFImageQueryMESA = dri2_export_dma_buf_image_query_mesa;
dri2_drv->base.API.ExportDMABUFImageMESA = dri2_export_dma_buf_image_mesa;
dri2_drv->base.API.QueryDmaBufFormatsEXT = dri2_query_dma_buf_formats;
+ dri2_drv->base.API.QueryDmaBufModifiersEXT = dri2_query_dma_buf_modifiers;
#endif
#ifdef HAVE_WAYLAND_PLATFORM
dri2_drv->base.API.BindWaylandDisplayWL = dri2_bind_wayland_display_wl;
diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index 53d34d8..cd65115 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -2275,6 +2275,26 @@ eglQueryDmaBufFormatsEXT(EGLDisplay dpy, EGLint max_formats,
RETURN_EGL_EVAL(disp, ret);
}

+static EGLBoolean EGLAPIENTRY
+eglQueryDmaBufModifiersEXT(EGLDisplay dpy, EGLint format, EGLint max_modifiers,
+ EGLuint64KHR *modifiers, EGLBoolean *external_only,
+ EGLint *num_modifiers)
+{
+ _EGLDisplay *disp = _eglLockDisplay(dpy);
+ _EGLDriver *drv;
+ EGLBoolean ret;
+
+ _EGL_FUNC_START(NULL, EGL_NONE, NULL, EGL_FALSE);
+
+ _EGL_CHECK_DISPLAY(disp, EGL_FALSE, drv);
+
+ ret = drv->API.QueryDmaBufModifiersEXT(drv, disp, format, max_modifiers,
+ modifiers, external_only,
+ num_modifiers);
+
+ RETURN_EGL_EVAL(disp, ret);
+}
+
__eglMustCastToProperFunctionPointerType EGLAPIENTRY
eglGetProcAddress(const char *procname)
{
@@ -2359,6 +2379,7 @@ eglGetProcAddress(const char *procname)
{ "eglDebugMessageControlKHR", (_EGLProc) eglDebugMessageControlKHR },
{ "eglQueryDebugKHR", (_EGLProc) eglQueryDebugKHR },
{ "eglQueryDmaBufFormatsEXT", (_EGLProc) eglQueryDmaBufFormatsEXT },
+ { "eglQueryDmaBufModifiersEXT", (_EGLProc) eglQueryDmaBufModifiersEXT },
{ NULL, NULL }
};
EGLint i;
diff --git a/src/egl/main/eglapi.h b/src/egl/main/eglapi.h
index 13388b1..3428195 100644
--- a/src/egl/main/eglapi.h
+++ b/src/egl/main/eglapi.h
@@ -200,6 +200,11 @@ struct _egl_api
EGLBoolean (*QueryDmaBufFormatsEXT)(_EGLDriver *drv, _EGLDisplay *dpy,
EGLint max_formats, EGLint *formats,
EGLint *num_formats);
+ EGLBoolean (*QueryDmaBufModifiersEXT) (_EGLDriver *drv, _EGLDisplay *dpy,
+ EGLint format, EGLint max_modifiers,
+ EGLuint64KHR *modifiers,
+ EGLBoolean *external_only,
+ EGLint *num_modifiers);
};

EGLint _eglConvertIntsToAttribs(const EGLint *int_list,

[Varad Gautam]

From: Varad Gautam <***@collabora.com>

Signed-off-by: Varad Gautam <***@collabora.com>
---
src/egl/drivers/dri2/egl_dri2.c | 5 +++++
src/egl/main/eglapi.c | 1 +
2 files changed, 6 insertions(+)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 443e0a3..a9a6f95 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -754,6 +754,11 @@ dri2_setup_screen(_EGLDisplay *disp)
dri2_dpy->image->createImageFromDmaBufs) {
disp->Extensions.EXT_image_dma_buf_import = EGL_TRUE;
}
+ if (dri2_dpy->image->base.version >= 15 &&
+ dri2_dpy->image->createImageFromDmaBufs2 &&
+ dri2_dpy->image->queryDmaBufModifiers) {
+ disp->Extensions.EXT_image_dma_buf_import_modifiers = EGL_TRUE;
+ }
#endif
}
}
diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index cd65115..44d7935 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -481,6 +481,7 @@ _eglCreateExtensionsString(_EGLDisplay *dpy)
_EGL_CHECK_EXTENSION(EXT_buffer_age);
_EGL_CHECK_EXTENSION(EXT_create_context_robustness);
_EGL_CHECK_EXTENSION(EXT_image_dma_buf_import);
+ _EGL_CHECK_EXTENSION(EXT_image_dma_buf_import_modifiers);
_EGL_CHECK_EXTENSION(EXT_swap_buffers_with_damage);

_EGL_CHECK_EXTENSION(KHR_cl_event2);

[Varad Gautam]

From: Varad Gautam <***@collabora.com>

support importing dmabufs into DRIimage taking format modifiers in
account, as per DRIimage extension version 14.

Signed-off-by: Varad Gautam <***@collabora.com>
---
src/gallium/include/state_tracker/drm_driver.h | 2 ++
src/gallium/state_trackers/dri/dri2.c | 45 +++++++++++++++++++++++---
2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/src/gallium/include/state_tracker/drm_driver.h b/src/gallium/include/state_tracker/drm_driver.h
index c80fb09..8b9d6bc 100644
--- a/src/gallium/include/state_tracker/drm_driver.h
+++ b/src/gallium/include/state_tracker/drm_driver.h
@@ -45,6 +45,8 @@ struct winsys_handle
* Output for texture_get_handle.
*/
unsigned offset;
+
+ uint64_t modifier;
};


diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c
index 9ec069b..a2b87a7 100644
--- a/src/gallium/state_trackers/dri/dri2.c
+++ b/src/gallium/state_trackers/dri/dri2.c
@@ -884,7 +884,7 @@ static __DRIimage *
dri2_create_image_from_fd(__DRIscreen *_screen,
int width, int height, int fourcc,
int *fds, int num_fds, int *strides,
- int *offsets, unsigned *error,
+ int *offsets, uint64_t *modifiers, unsigned *error,
int *dri_components, void *loaderPrivate)
{
struct winsys_handle whandles[3];
@@ -929,6 +929,8 @@ dri2_create_image_from_fd(__DRIscreen *_screen,
whandles[i].handle = (unsigned)fds[i];
whandles[i].stride = (unsigned)strides[i];
whandles[i].offset = (unsigned)offsets[i];
+ if (modifiers)
+ whandles[i].modifier = modifiers[i];
}

if (fourcc == __DRI_IMAGE_FOURCC_YVU420) {
@@ -1248,7 +1250,7 @@ dri2_from_fds(__DRIscreen *screen, int width, int height, int fourcc,
int dri_components;

img = dri2_create_image_from_fd(screen, width, height, fourcc,
- fds, num_fds, strides, offsets, NULL,
+ fds, num_fds, strides, offsets, NULL, NULL,
&dri_components, loaderPrivate);
if (img == NULL)
return NULL;
@@ -1273,7 +1275,7 @@ dri2_from_dma_bufs(__DRIscreen *screen,
int dri_components;

img = dri2_create_image_from_fd(screen, width, height, fourcc,
- fds, num_fds, strides, offsets, error,
+ fds, num_fds, strides, offsets, NULL, error,
&dri_components, loaderPrivate);
if (img == NULL)
return NULL;
@@ -1288,6 +1290,38 @@ dri2_from_dma_bufs(__DRIscreen *screen,
return img;
}

+static __DRIimage *
+dri2_from_dma_bufs2(__DRIscreen *screen,
+ int width, int height, int fourcc,
+ int *fds, int num_fds,
+ int *strides, int *offsets,
+ uint64_t *modifiers,
+ enum __DRIYUVColorSpace yuv_color_space,
+ enum __DRISampleRange sample_range,
+ enum __DRIChromaSiting horizontal_siting,
+ enum __DRIChromaSiting vertical_siting,
+ unsigned *error,
+ void *loaderPrivate)
+{
+ __DRIimage *img;
+ int dri_components;
+
+ img = dri2_create_image_from_fd(screen, width, height, fourcc,
+ fds, num_fds, strides, offsets, modifiers,
+ error, &dri_components, loaderPrivate);
+ if (img == NULL)
+ return NULL;
+
+ img->yuv_color_space = yuv_color_space;
+ img->sample_range = sample_range;
+ img->horizontal_siting = horizontal_siting;
+ img->vertical_siting = vertical_siting;
+ img->dri_components = dri_components;
+
+ *error = __DRI_IMAGE_ERROR_SUCCESS;
+ return img;
+}
+
static void
dri2_blit_image(__DRIcontext *context, __DRIimage *dst, __DRIimage *src,
int dstx0, int dsty0, int dstwidth, int dstheight,
@@ -1391,7 +1425,7 @@ dri2_get_capabilities(__DRIscreen *_screen)

/* The extension is modified during runtime if DRI_PRIME is detected */
static __DRIimageExtension dri2ImageExtension = {
- .base = { __DRI_IMAGE, 12 },
+ .base = { __DRI_IMAGE, 14 },

.createImageFromName = dri2_create_image_from_name,
.createImageFromRenderbuffer = dri2_create_image_from_renderbuffer,
@@ -1409,6 +1443,7 @@ static __DRIimageExtension dri2ImageExtension = {
.getCapabilities = dri2_get_capabilities,
.mapImage = dri2_map_image,
.unmapImage = dri2_unmap_image,
+ .createImageFromDmaBufs2 = NULL,
};


@@ -1902,6 +1937,7 @@ dri2_init_screen(__DRIscreen * sPriv)
(cap & DRM_PRIME_CAP_IMPORT)) {
dri2ImageExtension.createImageFromFds = dri2_from_fds;
dri2ImageExtension.createImageFromDmaBufs = dri2_from_dma_bufs;
+ dri2ImageExtension.createImageFromDmaBufs2 = dri2_from_dma_bufs2;
}
}

@@ -1974,6 +2010,7 @@ dri_kms_init_screen(__DRIscreen * sPriv)
(cap & DRM_PRIME_CAP_IMPORT)) {
dri2ImageExtension.createImageFromFds = dri2_from_fds;
dri2ImageExtension.createImageFromDmaBufs = dri2_from_dma_bufs;
+ dri2ImageExtension.createImageFromDmaBufs2 = dri2_from_dma_bufs2;
}

sPriv->extensions = dri_screen_extensions;

[Emil Velikov]

Post by Varad Gautam
support importing dmabufs into DRIimage taking format modifiers in
account, as per DRIimage extension version 14.

With the following discussion in mind [1] I'm wondering if we don't
want to rework things to pass/store a single modifier.
I'm leaning that implementation good as-is. One small catch though -
we would need a check at the EGL API level to check if the modifier
provided is identical across the board.

-Emil
[1] https://lists.freedesktop.org/archives/dri-devel/2016-November/123922.html

[Varad Gautam]

From: Varad Gautam <***@collabora.com>

ask the driver for supported modifiers for a given format.

Signed-off-by: Varad Gautam <***@collabora.com>
---
src/gallium/state_trackers/dri/dri2.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c
index a2b87a7..2684481 100644
--- a/src/gallium/state_trackers/dri/dri2.c
+++ b/src/gallium/state_trackers/dri/dri2.c
@@ -1259,6 +1259,21 @@ dri2_from_fds(__DRIscreen *screen, int width, int height, int fourcc,
return img;
}

+static void
+dri2_query_dma_buf_modifiers(__DRIscreen *_screen, int fourcc, int max,
+ uint64_t *modifiers, int *count)
+{
+ struct dri_screen *screen = dri_screen(_screen);
+ struct pipe_screen *pscreen = screen->base.screen;
+ int dri_components;
+ enum pipe_format format = dri2_format_to_pipe_format(
+ convert_fourcc(fourcc,&dri_components));
+
+ if (pscreen->get_param(pscreen, PIPE_CAP_QUERY_DMABUF_MODIFIERS))
+ pscreen->get_modifiers_for_format(pscreen, format, max, modifiers,
+ count);
+}
+
static __DRIimage *
dri2_from_dma_bufs(__DRIscreen *screen,
int width, int height, int fourcc,
@@ -1425,7 +1440,7 @@ dri2_get_capabilities(__DRIscreen *_screen)

/* The extension is modified during runtime if DRI_PRIME is detected */
static __DRIimageExtension dri2ImageExtension = {
- .base = { __DRI_IMAGE, 14 },
+ .base = { __DRI_IMAGE, 15 },

.createImageFromName = dri2_create_image_from_name,
.createImageFromRenderbuffer = dri2_create_image_from_renderbuffer,
@@ -1938,6 +1953,8 @@ dri2_init_screen(__DRIscreen * sPriv)
dri2ImageExtension.createImageFromFds = dri2_from_fds;
dri2ImageExtension.createImageFromDmaBufs = dri2_from_dma_bufs;
dri2ImageExtension.createImageFromDmaBufs2 = dri2_from_dma_bufs2;
+ dri2ImageExtension.queryDmaBufModifiers =
+ dri2_query_dma_buf_modifiers;
}
}

@@ -2011,6 +2028,7 @@ dri_kms_init_screen(__DRIscreen * sPriv)
dri2ImageExtension.createImageFromFds = dri2_from_fds;
dri2ImageExtension.createImageFromDmaBufs = dri2_from_dma_bufs;
dri2ImageExtension.createImageFromDmaBufs2 = dri2_from_dma_bufs2;
+ dri2ImageExtension.queryDmaBufModifiers = dri2_query_dma_buf_modifiers;
}

sPriv->extensions = dri_screen_extensions;

[Varad Gautam]

From: Varad Gautam <***@collabora.com>

drivers should implement pipe_screen->get_modifiers_for_format and
advertise it with PIPE_CAP_QUERY_DMABUF_MODIFIERS to support format
modifier queries.

Signed-off-by: Varad Gautam <***@collabora.com>
---
src/gallium/docs/source/screen.rst | 2 ++
src/gallium/drivers/freedreno/freedreno_screen.c | 1 +
src/gallium/drivers/i915/i915_screen.c | 1 +
src/gallium/drivers/ilo/ilo_screen.c | 1 +
src/gallium/drivers/llvmpipe/lp_screen.c | 1 +
src/gallium/drivers/nouveau/nv30/nv30_screen.c | 1 +
src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 +
src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 1 +
src/gallium/drivers/r300/r300_screen.c | 1 +
src/gallium/drivers/r600/r600_pipe.c | 1 +
src/gallium/drivers/radeonsi/si_pipe.c | 1 +
src/gallium/drivers/softpipe/sp_screen.c | 1 +
src/gallium/drivers/svga/svga_screen.c | 1 +
src/gallium/drivers/swr/swr_screen.cpp | 1 +
src/gallium/drivers/vc4/vc4_screen.c | 1 +
src/gallium/drivers/virgl/virgl_screen.c | 1 +
src/gallium/include/pipe/p_defines.h | 1 +
src/gallium/include/pipe/p_screen.h | 7 +++++++
18 files changed, 25 insertions(+)

diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst
index 6ad2bec..1775fe3 100644
--- a/src/gallium/docs/source/screen.rst
+++ b/src/gallium/docs/source/screen.rst
@@ -364,6 +364,8 @@ The integer capabilities:
* ``PIPE_CAP_STREAM_OUTPUT_INTERLEAVE_BUFFERS``: Whether interleaved stream
output mode is able to interleave across buffers. This is required for
ARB_transform_feedback3.
+* ``PIPE_CAP_QUERY_DMABUF_MODIFIERS``: Whether the driver supports modifier
+ queries for a given format.


.. _pipe_capf:
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c b/src/gallium/drivers/freedreno/freedreno_screen.c
index 97da0d7..2786d45 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -288,6 +288,7 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
case PIPE_CAP_TGSI_ARRAY_COMPONENTS:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;

case PIPE_CAP_MAX_VIEWPORTS:
diff --git a/src/gallium/drivers/i915/i915_screen.c b/src/gallium/drivers/i915/i915_screen.c
index bfadca3..ce42e04 100644
--- a/src/gallium/drivers/i915/i915_screen.c
+++ b/src/gallium/drivers/i915/i915_screen.c
@@ -278,6 +278,7 @@ i915_get_param(struct pipe_screen *screen, enum pipe_cap cap)
case PIPE_CAP_MAX_WINDOW_RECTANGLES:
case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
case PIPE_CAP_TGSI_ARRAY_COMPONENTS:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;

case PIPE_CAP_MAX_DUAL_SOURCE_RENDER_TARGETS:
diff --git a/src/gallium/drivers/ilo/ilo_screen.c b/src/gallium/drivers/ilo/ilo_screen.c
index f3f182c..05fc906 100644
--- a/src/gallium/drivers/ilo/ilo_screen.c
+++ b/src/gallium/drivers/ilo/ilo_screen.c
@@ -517,6 +517,7 @@ ilo_get_param(struct pipe_screen *screen, enum pipe_cap param)
case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
case PIPE_CAP_TGSI_ARRAY_COMPONENTS:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;

case PIPE_CAP_VENDOR_ID:
diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c b/src/gallium/drivers/llvmpipe/lp_screen.c
index 4b502f0..2a8fa76 100644
--- a/src/gallium/drivers/llvmpipe/lp_screen.c
+++ b/src/gallium/drivers/llvmpipe/lp_screen.c
@@ -338,6 +338,7 @@ llvmpipe_get_param(struct pipe_screen *screen, enum pipe_cap param)
case PIPE_CAP_MAX_WINDOW_RECTANGLES:
case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;
}
/* should only get here on unhandled cases */
diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
index 96708c0..6ba768b 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
@@ -203,6 +203,7 @@ nv30_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
case PIPE_CAP_MIXED_COLOR_DEPTH_BITS:
case PIPE_CAP_TGSI_ARRAY_COMPONENTS:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;

case PIPE_CAP_VENDOR_ID:
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
index 50cdeda..5c922ed 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
@@ -255,6 +255,7 @@ nv50_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
case PIPE_CAP_STREAM_OUTPUT_INTERLEAVE_BUFFERS:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;

case PIPE_CAP_VENDOR_ID:
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index 6361e9e..02216a9 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -274,6 +274,7 @@ nvc0_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
case PIPE_CAP_PCI_DEVICE:
case PIPE_CAP_PCI_FUNCTION:
case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;

case PIPE_CAP_VENDOR_ID:
diff --git a/src/gallium/drivers/r300/r300_screen.c b/src/gallium/drivers/r300/r300_screen.c
index 1e1dc87..e8efb1f 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -225,6 +225,7 @@ static int r300_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
case PIPE_CAP_TGSI_ARRAY_COMPONENTS:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;

/* SWTCL-only features. */
diff --git a/src/gallium/drivers/r600/r600_pipe.c b/src/gallium/drivers/r600/r600_pipe.c
index 970a20e..39230cf 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c
@@ -374,6 +374,7 @@ static int r600_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
case PIPE_CAP_TGSI_VOTE:
case PIPE_CAP_MAX_WINDOW_RECTANGLES:
case PIPE_CAP_TGSI_ARRAY_COMPONENTS:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;

case PIPE_CAP_MAX_SHADER_PATCH_VARYINGS:
diff --git a/src/gallium/drivers/radeonsi/si_pipe.c b/src/gallium/drivers/radeonsi/si_pipe.c
index f0cd9ca..1b60e81 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -458,6 +458,7 @@ static int si_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
case PIPE_CAP_PRIMITIVE_RESTART_FOR_PATCHES:
case PIPE_CAP_TGSI_VOTE:
case PIPE_CAP_MAX_WINDOW_RECTANGLES:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;

case PIPE_CAP_QUERY_BUFFER_OBJECT:
diff --git a/src/gallium/drivers/softpipe/sp_screen.c b/src/gallium/drivers/softpipe/sp_screen.c
index 275adab..aeab080 100644
--- a/src/gallium/drivers/softpipe/sp_screen.c
+++ b/src/gallium/drivers/softpipe/sp_screen.c
@@ -288,6 +288,7 @@ softpipe_get_param(struct pipe_screen *screen, enum pipe_cap param)
case PIPE_CAP_MAX_WINDOW_RECTANGLES:
case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;
case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
return 4;
diff --git a/src/gallium/drivers/svga/svga_screen.c b/src/gallium/drivers/svga/svga_screen.c
index 67a35cf..0d19eb4 100644
--- a/src/gallium/drivers/svga/svga_screen.c
+++ b/src/gallium/drivers/svga/svga_screen.c
@@ -419,6 +419,7 @@ svga_get_param(struct pipe_screen *screen, enum pipe_cap param)
case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
case PIPE_CAP_TGSI_ARRAY_COMPONENTS:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;
}

diff --git a/src/gallium/drivers/swr/swr_screen.cpp b/src/gallium/drivers/swr/swr_screen.cpp
index fa16edd..3d0b2f4 100644
--- a/src/gallium/drivers/swr/swr_screen.cpp
+++ b/src/gallium/drivers/swr/swr_screen.cpp
@@ -369,6 +369,7 @@ swr_get_param(struct pipe_screen *screen, enum pipe_cap param)
case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
case PIPE_CAP_TGSI_ARRAY_COMPONENTS:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;
}

diff --git a/src/gallium/drivers/vc4/vc4_screen.c b/src/gallium/drivers/vc4/vc4_screen.c
index 9aa80ca..70c9e3a 100644
--- a/src/gallium/drivers/vc4/vc4_screen.c
+++ b/src/gallium/drivers/vc4/vc4_screen.c
@@ -239,6 +239,7 @@ vc4_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
case PIPE_CAP_TGSI_ARRAY_COMPONENTS:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;

/* Stream output. */
diff --git a/src/gallium/drivers/virgl/virgl_screen.c b/src/gallium/drivers/virgl/virgl_screen.c
index 2817809..f41a078 100644
--- a/src/gallium/drivers/virgl/virgl_screen.c
+++ b/src/gallium/drivers/virgl/virgl_screen.c
@@ -250,6 +250,7 @@ virgl_get_param(struct pipe_screen *screen, enum pipe_cap param)
case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
case PIPE_CAP_TGSI_ARRAY_COMPONENTS:
+ case PIPE_CAP_QUERY_DMABUF_MODIFIERS:
return 0;
case PIPE_CAP_VENDOR_ID:
return 0x1af4;
diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
index 69d290c..53d041c 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -740,6 +740,7 @@ enum pipe_cap
PIPE_CAP_MIXED_COLOR_DEPTH_BITS,
PIPE_CAP_TGSI_ARRAY_COMPONENTS,
PIPE_CAP_STREAM_OUTPUT_INTERLEAVE_BUFFERS,
+ PIPE_CAP_QUERY_DMABUF_MODIFIERS,
};

#define PIPE_QUIRK_TEXTURE_BORDER_COLOR_SWIZZLE_NV50 (1 << 0)
diff --git a/src/gallium/include/pipe/p_screen.h b/src/gallium/include/pipe/p_screen.h
index 255647e..91b3434 100644
--- a/src/gallium/include/pipe/p_screen.h
+++ b/src/gallium/include/pipe/p_screen.h
@@ -300,6 +300,13 @@ struct pipe_screen {
const void *(*get_compiler_options)(struct pipe_screen *screen,
enum pipe_shader_ir ir,
unsigned shader);
+
+ /**
+ * Get supported modifiers for a format.
+ */
+ void (*get_modifiers_for_format)(struct pipe_screen *screen,
+ enum pipe_format format, int max,
+ uint64_t *modifiers, int *count);
};

[Roland Scheidegger]

It looks like there aren't any possible modifier bits defined, so how is
this supposed to work?

Roland

Post by Varad Gautam
drivers should implement pipe_screen->get_modifiers_for_format and
advertise it with PIPE_CAP_QUERY_DMABUF_MODIFIERS to support format
modifier queries.
---
src/gallium/docs/source/screen.rst | 2 ++
src/gallium/drivers/freedreno/freedreno_screen.c | 1 +
src/gallium/drivers/i915/i915_screen.c | 1 +
src/gallium/drivers/ilo/ilo_screen.c | 1 +
src/gallium/drivers/llvmpipe/lp_screen.c | 1 +
src/gallium/drivers/nouveau/nv30/nv30_screen.c | 1 +
src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 +
src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 1 +
src/gallium/drivers/r300/r300_screen.c | 1 +
src/gallium/drivers/r600/r600_pipe.c | 1 +
src/gallium/drivers/radeonsi/si_pipe.c | 1 +
src/gallium/drivers/softpipe/sp_screen.c | 1 +
src/gallium/drivers/svga/svga_screen.c | 1 +
src/gallium/drivers/swr/swr_screen.cpp | 1 +
src/gallium/drivers/vc4/vc4_screen.c | 1 +
src/gallium/drivers/virgl/virgl_screen.c | 1 +
src/gallium/include/pipe/p_defines.h | 1 +
src/gallium/include/pipe/p_screen.h | 7 +++++++
18 files changed, 25 insertions(+)
diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst
index 6ad2bec..1775fe3 100644
--- a/src/gallium/docs/source/screen.rst
+++ b/src/gallium/docs/source/screen.rst
* ``PIPE_CAP_STREAM_OUTPUT_INTERLEAVE_BUFFERS``: Whether interleaved stream
output mode is able to interleave across buffers. This is required for
ARB_transform_feedback3.
+* ``PIPE_CAP_QUERY_DMABUF_MODIFIERS``: Whether the driver supports modifier
+ queries for a given format.
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c b/src/gallium/drivers/freedreno/freedreno_screen.c
index 97da0d7..2786d45 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -288,6 +288,7 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
return 0;
diff --git a/src/gallium/drivers/i915/i915_screen.c b/src/gallium/drivers/i915/i915_screen.c
index bfadca3..ce42e04 100644
--- a/src/gallium/drivers/i915/i915_screen.c
+++ b/src/gallium/drivers/i915/i915_screen.c
@@ -278,6 +278,7 @@ i915_get_param(struct pipe_screen *screen, enum pipe_cap cap)
return 0;
diff --git a/src/gallium/drivers/ilo/ilo_screen.c b/src/gallium/drivers/ilo/ilo_screen.c
index f3f182c..05fc906 100644
--- a/src/gallium/drivers/ilo/ilo_screen.c
+++ b/src/gallium/drivers/ilo/ilo_screen.c
@@ -517,6 +517,7 @@ ilo_get_param(struct pipe_screen *screen, enum pipe_cap param)
return 0;
diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c b/src/gallium/drivers/llvmpipe/lp_screen.c
index 4b502f0..2a8fa76 100644
--- a/src/gallium/drivers/llvmpipe/lp_screen.c
+++ b/src/gallium/drivers/llvmpipe/lp_screen.c
@@ -338,6 +338,7 @@ llvmpipe_get_param(struct pipe_screen *screen, enum pipe_cap param)
return 0;
}
/* should only get here on unhandled cases */
diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
index 96708c0..6ba768b 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
@@ -203,6 +203,7 @@ nv30_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
return 0;
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
index 50cdeda..5c922ed 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
@@ -255,6 +255,7 @@ nv50_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
return 0;
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index 6361e9e..02216a9 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -274,6 +274,7 @@ nvc0_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
return 0;
diff --git a/src/gallium/drivers/r300/r300_screen.c b/src/gallium/drivers/r300/r300_screen.c
index 1e1dc87..e8efb1f 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -225,6 +225,7 @@ static int r300_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
return 0;
/* SWTCL-only features. */
diff --git a/src/gallium/drivers/r600/r600_pipe.c b/src/gallium/drivers/r600/r600_pipe.c
index 970a20e..39230cf 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c
@@ -374,6 +374,7 @@ static int r600_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
return 0;
diff --git a/src/gallium/drivers/radeonsi/si_pipe.c b/src/gallium/drivers/radeonsi/si_pipe.c
index f0cd9ca..1b60e81 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -458,6 +458,7 @@ static int si_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
return 0;
diff --git a/src/gallium/drivers/softpipe/sp_screen.c b/src/gallium/drivers/softpipe/sp_screen.c
index 275adab..aeab080 100644
--- a/src/gallium/drivers/softpipe/sp_screen.c
+++ b/src/gallium/drivers/softpipe/sp_screen.c
@@ -288,6 +288,7 @@ softpipe_get_param(struct pipe_screen *screen, enum pipe_cap param)
return 0;
return 4;
diff --git a/src/gallium/drivers/svga/svga_screen.c b/src/gallium/drivers/svga/svga_screen.c
index 67a35cf..0d19eb4 100644
--- a/src/gallium/drivers/svga/svga_screen.c
+++ b/src/gallium/drivers/svga/svga_screen.c
@@ -419,6 +419,7 @@ svga_get_param(struct pipe_screen *screen, enum pipe_cap param)
return 0;
}
diff --git a/src/gallium/drivers/swr/swr_screen.cpp b/src/gallium/drivers/swr/swr_screen.cpp
index fa16edd..3d0b2f4 100644
--- a/src/gallium/drivers/swr/swr_screen.cpp
+++ b/src/gallium/drivers/swr/swr_screen.cpp
@@ -369,6 +369,7 @@ swr_get_param(struct pipe_screen *screen, enum pipe_cap param)
return 0;
}
diff --git a/src/gallium/drivers/vc4/vc4_screen.c b/src/gallium/drivers/vc4/vc4_screen.c
index 9aa80ca..70c9e3a 100644
--- a/src/gallium/drivers/vc4/vc4_screen.c
+++ b/src/gallium/drivers/vc4/vc4_screen.c
@@ -239,6 +239,7 @@ vc4_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
return 0;
/* Stream output. */
diff --git a/src/gallium/drivers/virgl/virgl_screen.c b/src/gallium/drivers/virgl/virgl_screen.c
index 2817809..f41a078 100644
--- a/src/gallium/drivers/virgl/virgl_screen.c
+++ b/src/gallium/drivers/virgl/virgl_screen.c
@@ -250,6 +250,7 @@ virgl_get_param(struct pipe_screen *screen, enum pipe_cap param)
return 0;
return 0x1af4;
diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
index 69d290c..53d041c 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -740,6 +740,7 @@ enum pipe_cap
PIPE_CAP_MIXED_COLOR_DEPTH_BITS,
PIPE_CAP_TGSI_ARRAY_COMPONENTS,
PIPE_CAP_STREAM_OUTPUT_INTERLEAVE_BUFFERS,
+ PIPE_CAP_QUERY_DMABUF_MODIFIERS,
};
#define PIPE_QUIRK_TEXTURE_BORDER_COLOR_SWIZZLE_NV50 (1 << 0)
diff --git a/src/gallium/include/pipe/p_screen.h b/src/gallium/include/pipe/p_screen.h
index 255647e..91b3434 100644
--- a/src/gallium/include/pipe/p_screen.h
+++ b/src/gallium/include/pipe/p_screen.h
@@ -300,6 +300,13 @@ struct pipe_screen {
const void *(*get_compiler_options)(struct pipe_screen *screen,
enum pipe_shader_ir ir,
unsigned shader);
+
+ /**
+ * Get supported modifiers for a format.
+ */
+ void (*get_modifiers_for_format)(struct pipe_screen *screen,
+ enum pipe_format format, int max,
+ uint64_t *modifiers, int *count);
};

[Rob Clark]

see drm_fourcc.h (if the extension txt didn't mention that, perhaps it should)

BR,
-R

Post by Roland Scheidegger
It looks like there aren't any possible modifier bits defined, so how is
this supposed to work?
Roland

Post by Varad Gautam
drivers should implement pipe_screen->get_modifiers_for_format and
advertise it with PIPE_CAP_QUERY_DMABUF_MODIFIERS to support format
modifier queries.
---
src/gallium/docs/source/screen.rst | 2 ++
src/gallium/drivers/freedreno/freedreno_screen.c | 1 +
src/gallium/drivers/i915/i915_screen.c | 1 +
src/gallium/drivers/ilo/ilo_screen.c | 1 +
src/gallium/drivers/llvmpipe/lp_screen.c | 1 +
src/gallium/drivers/nouveau/nv30/nv30_screen.c | 1 +
src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 +
src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 1 +
src/gallium/drivers/r300/r300_screen.c | 1 +
src/gallium/drivers/r600/r600_pipe.c | 1 +
src/gallium/drivers/radeonsi/si_pipe.c | 1 +
src/gallium/drivers/softpipe/sp_screen.c | 1 +
src/gallium/drivers/svga/svga_screen.c | 1 +
src/gallium/drivers/swr/swr_screen.cpp | 1 +
src/gallium/drivers/vc4/vc4_screen.c | 1 +
src/gallium/drivers/virgl/virgl_screen.c | 1 +
src/gallium/include/pipe/p_defines.h | 1 +
src/gallium/include/pipe/p_screen.h | 7 +++++++
18 files changed, 25 insertions(+)
diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst
index 6ad2bec..1775fe3 100644
--- a/src/gallium/docs/source/screen.rst
+++ b/src/gallium/docs/source/screen.rst
* ``PIPE_CAP_STREAM_OUTPUT_INTERLEAVE_BUFFERS``: Whether interleaved stream
output mode is able to interleave across buffers. This is required for
ARB_transform_feedback3.
+* ``PIPE_CAP_QUERY_DMABUF_MODIFIERS``: Whether the driver supports modifier
+ queries for a given format.
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c b/src/gallium/drivers/freedreno/freedreno_screen.c
index 97da0d7..2786d45 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -288,6 +288,7 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
return 0;
diff --git a/src/gallium/drivers/i915/i915_screen.c b/src/gallium/drivers/i915/i915_screen.c
index bfadca3..ce42e04 100644
--- a/src/gallium/drivers/i915/i915_screen.c
+++ b/src/gallium/drivers/i915/i915_screen.c
@@ -278,6 +278,7 @@ i915_get_param(struct pipe_screen *screen, enum pipe_cap cap)
return 0;
diff --git a/src/gallium/drivers/ilo/ilo_screen.c b/src/gallium/drivers/ilo/ilo_screen.c
index f3f182c..05fc906 100644
--- a/src/gallium/drivers/ilo/ilo_screen.c
+++ b/src/gallium/drivers/ilo/ilo_screen.c
@@ -517,6 +517,7 @@ ilo_get_param(struct pipe_screen *screen, enum pipe_cap param)
return 0;
diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c b/src/gallium/drivers/llvmpipe/lp_screen.c
index 4b502f0..2a8fa76 100644
--- a/src/gallium/drivers/llvmpipe/lp_screen.c
+++ b/src/gallium/drivers/llvmpipe/lp_screen.c
@@ -338,6 +338,7 @@ llvmpipe_get_param(struct pipe_screen *screen, enum pipe_cap param)
return 0;
}
/* should only get here on unhandled cases */
diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
index 96708c0..6ba768b 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
@@ -203,6 +203,7 @@ nv30_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
return 0;
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
index 50cdeda..5c922ed 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
@@ -255,6 +255,7 @@ nv50_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
return 0;
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index 6361e9e..02216a9 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -274,6 +274,7 @@ nvc0_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
return 0;
diff --git a/src/gallium/drivers/r300/r300_screen.c b/src/gallium/drivers/r300/r300_screen.c
index 1e1dc87..e8efb1f 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -225,6 +225,7 @@ static int r300_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
return 0;
/* SWTCL-only features. */
diff --git a/src/gallium/drivers/r600/r600_pipe.c b/src/gallium/drivers/r600/r600_pipe.c
index 970a20e..39230cf 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c
@@ -374,6 +374,7 @@ static int r600_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
return 0;
diff --git a/src/gallium/drivers/radeonsi/si_pipe.c b/src/gallium/drivers/radeonsi/si_pipe.c
index f0cd9ca..1b60e81 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -458,6 +458,7 @@ static int si_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
return 0;
diff --git a/src/gallium/drivers/softpipe/sp_screen.c b/src/gallium/drivers/softpipe/sp_screen.c
index 275adab..aeab080 100644
--- a/src/gallium/drivers/softpipe/sp_screen.c
+++ b/src/gallium/drivers/softpipe/sp_screen.c
@@ -288,6 +288,7 @@ softpipe_get_param(struct pipe_screen *screen, enum pipe_cap param)
return 0;
return 4;
diff --git a/src/gallium/drivers/svga/svga_screen.c b/src/gallium/drivers/svga/svga_screen.c
index 67a35cf..0d19eb4 100644
--- a/src/gallium/drivers/svga/svga_screen.c
+++ b/src/gallium/drivers/svga/svga_screen.c
@@ -419,6 +419,7 @@ svga_get_param(struct pipe_screen *screen, enum pipe_cap param)
return 0;
}
diff --git a/src/gallium/drivers/swr/swr_screen.cpp b/src/gallium/drivers/swr/swr_screen.cpp
index fa16edd..3d0b2f4 100644
--- a/src/gallium/drivers/swr/swr_screen.cpp
+++ b/src/gallium/drivers/swr/swr_screen.cpp
@@ -369,6 +369,7 @@ swr_get_param(struct pipe_screen *screen, enum pipe_cap param)
return 0;
}
diff --git a/src/gallium/drivers/vc4/vc4_screen.c b/src/gallium/drivers/vc4/vc4_screen.c
index 9aa80ca..70c9e3a 100644
--- a/src/gallium/drivers/vc4/vc4_screen.c
+++ b/src/gallium/drivers/vc4/vc4_screen.c
@@ -239,6 +239,7 @@ vc4_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
return 0;
/* Stream output. */
diff --git a/src/gallium/drivers/virgl/virgl_screen.c b/src/gallium/drivers/virgl/virgl_screen.c
index 2817809..f41a078 100644
--- a/src/gallium/drivers/virgl/virgl_screen.c
+++ b/src/gallium/drivers/virgl/virgl_screen.c
@@ -250,6 +250,7 @@ virgl_get_param(struct pipe_screen *screen, enum pipe_cap param)
return 0;
return 0x1af4;
diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
index 69d290c..53d041c 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -740,6 +740,7 @@ enum pipe_cap
PIPE_CAP_MIXED_COLOR_DEPTH_BITS,
PIPE_CAP_TGSI_ARRAY_COMPONENTS,
PIPE_CAP_STREAM_OUTPUT_INTERLEAVE_BUFFERS,
+ PIPE_CAP_QUERY_DMABUF_MODIFIERS,
};
#define PIPE_QUIRK_TEXTURE_BORDER_COLOR_SWIZZLE_NV50 (1 << 0)
diff --git a/src/gallium/include/pipe/p_screen.h b/src/gallium/include/pipe/p_screen.h
index 255647e..91b3434 100644
--- a/src/gallium/include/pipe/p_screen.h
+++ b/src/gallium/include/pipe/p_screen.h
@@ -300,6 +300,13 @@ struct pipe_screen {
const void *(*get_compiler_options)(struct pipe_screen *screen,
enum pipe_shader_ir ir,
unsigned shader);
+
+ /**
+ * Get supported modifiers for a format.
+ */
+ void (*get_modifiers_for_format)(struct pipe_screen *screen,
+ enum pipe_format format, int max,
+ uint64_t *modifiers, int *count);
};

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

[Eric Engestrom]

Post by Rob Clark
see drm_fourcc.h (if the extension txt didn't mention that, perhaps it should)

It does :)

These attribute values together form an unsigned 64-bit value called
a format modifier. Format modifiers are specified by drm_fourcc.h and
used as the modifier parameter of the drm_mode_fb_cmd2 ioctl.

The exact meaning and effect of any modifier is canonically defined by
drm_fourcc.h, not as part of this extension.

Cheers,
Eric

Post by Rob Clark
BR,
-R

Post by Roland Scheidegger
It looks like there aren't any possible modifier bits defined, so how is
this supposed to work?
Roland

[Michel Dänzer]

Post by Varad Gautam
drivers should implement pipe_screen->get_modifiers_for_format and
advertise it with PIPE_CAP_QUERY_DMABUF_MODIFIERS to support format
modifier queries.

Is there any point in having the PIPE_CAP_QUERY_DMABUF_MODIFIERS cap, vs
just testing if pipe_screen->get_modifiers_for_format != NULL ?

[Varad Gautam]

Post by Michel Dänzer

Post by Varad Gautam
drivers should implement pipe_screen->get_modifiers_for_format and
advertise it with PIPE_CAP_QUERY_DMABUF_MODIFIERS to support format
modifier queries.

Is there any point in having the PIPE_CAP_QUERY_DMABUF_MODIFIERS cap, vs
just testing if pipe_screen->get_modifiers_for_format != NULL ?

The PIPE_CAP is to convey that the driver supports modifier queries as
compatible with the egl ext (64b), as compared to drivers that wish to treat
modifiers differently. If this isn't a concern, I can replace it with the NULL
check.

Thanks,
Varad

Post by Michel Dänzer
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer

[Marek Olšák]

Hi,

Is the modifier just a driver-specific description of the tiling
layout and compression?

What makes you think that 8 bytes is enough to describe that? What if
I need 9 bytes just to program the display hardware?

Drivers importing DMABUFs still have to invoke the texture tiling
calculator to get all necessary parameters for rendering (not just
display), which may even be 128 bytes per plane.

Marek

Post by Varad Gautam
Hello,
This series implements EGL_EXT_image_dma_buf_import_modifiers [1] which makes
it possible to pass drm fourcc modifiers to EGL when importing dmabufs.
[1] https://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_import_modifiers.txt
egl: return error for unknown EGLImage attributes
egl: introduce DMA_BUF_MAX_PLANES
egl/main: add support for fourth plane tokens
dri: support DRIimage creation from dmabufs with modifiers
egl_dri2: add support for using modifier attributes in
eglCreateImageKHR
egl: update eglext.h
st/dri: implement DRIimage creation from dmabufs with modifiers
egl: implement eglQueryDmaBufFormatsEXT
dri: add queryDmaBufModifiers to DRIimage
gallium: introduce format modifier querying
st/dri: support format modifier queries
egl: implement eglQueryDmaBufModifiersEXT
egl: advertise EGL_EXT_image_dma_buf_import_modifiers
include/EGL/eglext.h | 60 ++++++-
include/GL/internal/dri_interface.h | 31 +++-
src/egl/drivers/dri2/egl_dri2.c | 209 +++++++++++++++++++++--
src/egl/main/eglapi.c | 41 +++++
src/egl/main/eglapi.h | 9 +
src/egl/main/egldisplay.h | 1 +
src/egl/main/eglimage.c | 63 ++++++-
src/egl/main/eglimage.h | 10 +-
src/gallium/docs/source/screen.rst | 2 +
src/gallium/drivers/freedreno/freedreno_screen.c | 1 +
src/gallium/drivers/i915/i915_screen.c | 1 +
src/gallium/drivers/ilo/ilo_screen.c | 1 +
src/gallium/drivers/llvmpipe/lp_screen.c | 1 +
src/gallium/drivers/nouveau/nv30/nv30_screen.c | 1 +
src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 +
src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 1 +
src/gallium/drivers/r300/r300_screen.c | 1 +
src/gallium/drivers/r600/r600_pipe.c | 1 +
src/gallium/drivers/radeonsi/si_pipe.c | 1 +
src/gallium/drivers/softpipe/sp_screen.c | 1 +
src/gallium/drivers/svga/svga_screen.c | 1 +
src/gallium/drivers/swr/swr_screen.cpp | 1 +
src/gallium/drivers/vc4/vc4_screen.c | 1 +
src/gallium/drivers/virgl/virgl_screen.c | 1 +
src/gallium/include/pipe/p_defines.h | 1 +
src/gallium/include/pipe/p_screen.h | 7 +
src/gallium/include/state_tracker/drm_driver.h | 2 +
src/gallium/state_trackers/dri/dri2.c | 63 ++++++-
28 files changed, 487 insertions(+), 27 deletions(-)
--
2.6.2
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Rob Clark]

Post by Marek Olšák
Hi,
Is the modifier just a driver-specific description of the tiling
layout and compression?
What makes you think that 8 bytes is enough to describe that? What if
I need 9 bytes just to program the display hardware?
Drivers importing DMABUFs still have to invoke the texture tiling
calculator to get all necessary parameters for rendering (not just
display), which may even be 128 bytes per plane.

fwiw, this maps 1:1 to addfb2 ioctl, and just brings egl to parity with kms.

Maybe the addfb2 approach wasn't the best idea compared to some of the
ideas proposed for hypothetical "liballoc" for being ultra-generic.
OTOH perhaps you can just treat it like an enum? I mean maybe the set
of tiled formats that you would actually exchange with another device
is less than 2^^72. It seems reasonable to restrict the possible
tiled formats supported by this extension to only things that can be
exchanged with i965..

BR,
-R

Post by Marek Olšák
Marek

Post by Varad Gautam
Hello,
This series implements EGL_EXT_image_dma_buf_import_modifiers [1] which makes
it possible to pass drm fourcc modifiers to EGL when importing dmabufs.
[1] https://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_import_modifiers.txt
egl: return error for unknown EGLImage attributes
egl: introduce DMA_BUF_MAX_PLANES
egl/main: add support for fourth plane tokens
dri: support DRIimage creation from dmabufs with modifiers
egl_dri2: add support for using modifier attributes in
eglCreateImageKHR
egl: update eglext.h
st/dri: implement DRIimage creation from dmabufs with modifiers
egl: implement eglQueryDmaBufFormatsEXT
dri: add queryDmaBufModifiers to DRIimage
gallium: introduce format modifier querying
st/dri: support format modifier queries
egl: implement eglQueryDmaBufModifiersEXT
egl: advertise EGL_EXT_image_dma_buf_import_modifiers
include/EGL/eglext.h | 60 ++++++-
include/GL/internal/dri_interface.h | 31 +++-
src/egl/drivers/dri2/egl_dri2.c | 209 +++++++++++++++++++++--
src/egl/main/eglapi.c | 41 +++++
src/egl/main/eglapi.h | 9 +
src/egl/main/egldisplay.h | 1 +
src/egl/main/eglimage.c | 63 ++++++-
src/egl/main/eglimage.h | 10 +-
src/gallium/docs/source/screen.rst | 2 +
src/gallium/drivers/freedreno/freedreno_screen.c | 1 +
src/gallium/drivers/i915/i915_screen.c | 1 +
src/gallium/drivers/ilo/ilo_screen.c | 1 +
src/gallium/drivers/llvmpipe/lp_screen.c | 1 +
src/gallium/drivers/nouveau/nv30/nv30_screen.c | 1 +
src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 +
src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 1 +
src/gallium/drivers/r300/r300_screen.c | 1 +
src/gallium/drivers/r600/r600_pipe.c | 1 +
src/gallium/drivers/radeonsi/si_pipe.c | 1 +
src/gallium/drivers/softpipe/sp_screen.c | 1 +
src/gallium/drivers/svga/svga_screen.c | 1 +
src/gallium/drivers/swr/swr_screen.cpp | 1 +
src/gallium/drivers/vc4/vc4_screen.c | 1 +
src/gallium/drivers/virgl/virgl_screen.c | 1 +
src/gallium/include/pipe/p_defines.h | 1 +
src/gallium/include/pipe/p_screen.h | 7 +
src/gallium/include/state_tracker/drm_driver.h | 2 +
src/gallium/state_trackers/dri/dri2.c | 63 ++++++-
28 files changed, 487 insertions(+), 27 deletions(-)
--
2.6.2
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Marek Olšák]

Post by Rob Clark

Post by Marek Olšák
Hi,
Is the modifier just a driver-specific description of the tiling
layout and compression?
What makes you think that 8 bytes is enough to describe that? What if
I need 9 bytes just to program the display hardware?
Drivers importing DMABUFs still have to invoke the texture tiling
calculator to get all necessary parameters for rendering (not just
display), which may even be 128 bytes per plane.

fwiw, this maps 1:1 to addfb2 ioctl, and just brings egl to parity with kms.
Maybe the addfb2 approach wasn't the best idea compared to some of the
ideas proposed for hypothetical "liballoc" for being ultra-generic.
OTOH perhaps you can just treat it like an enum? I mean maybe the set
of tiled formats that you would actually exchange with another device
is less than 2^^72. It seems reasonable to restrict the possible
tiled formats supported by this extension to only things that can be
exchanged with i965..

For tile modes alone, 64 bits seem enough.

For compression, you may need a separate buffer to hold compression
data, so now you have to encode the tile mode and other parameters of
the plane, parameters of the compression buffer, and possibly also the
pitch and offset of the compression buffer.

I'm not saying it can't be done, but it wouldn't be nice.

Marek

[Rob Clark]

Post by Marek Olšák

Post by Rob Clark

Post by Marek Olšák
Hi,
Is the modifier just a driver-specific description of the tiling
layout and compression?
What makes you think that 8 bytes is enough to describe that? What if
I need 9 bytes just to program the display hardware?
Drivers importing DMABUFs still have to invoke the texture tiling
calculator to get all necessary parameters for rendering (not just
display), which may even be 128 bytes per plane.

fwiw, this maps 1:1 to addfb2 ioctl, and just brings egl to parity with kms.
Maybe the addfb2 approach wasn't the best idea compared to some of the
ideas proposed for hypothetical "liballoc" for being ultra-generic.
OTOH perhaps you can just treat it like an enum? I mean maybe the set
of tiled formats that you would actually exchange with another device
is less than 2^^72. It seems reasonable to restrict the possible
tiled formats supported by this extension to only things that can be
exchanged with i965..

For tile modes alone, 64 bits seem enough.
For compression, you may need a separate buffer to hold compression
data, so now you have to encode the tile mode and other parameters of
the plane, parameters of the compression buffer, and possibly also the
pitch and offset of the compression buffer.
I'm not saying it can't be done, but it wouldn't be nice.

Perhaps pass the compression buffer as a separate plane? Then it gets
it's own dmabuf fd, and stride (and offset incase it really is a
single buffer) plus 64b modifier..

BR,
-R

Post by Marek Olšák
Marek

[Marek Olšák]

Post by Rob Clark

Post by Marek Olšák

Post by Rob Clark

Post by Marek Olšák
Hi,
Is the modifier just a driver-specific description of the tiling
layout and compression?
What makes you think that 8 bytes is enough to describe that? What if
I need 9 bytes just to program the display hardware?
Drivers importing DMABUFs still have to invoke the texture tiling
calculator to get all necessary parameters for rendering (not just
display), which may even be 128 bytes per plane.

fwiw, this maps 1:1 to addfb2 ioctl, and just brings egl to parity with kms.
Maybe the addfb2 approach wasn't the best idea compared to some of the
ideas proposed for hypothetical "liballoc" for being ultra-generic.
OTOH perhaps you can just treat it like an enum? I mean maybe the set
of tiled formats that you would actually exchange with another device
is less than 2^^72. It seems reasonable to restrict the possible
tiled formats supported by this extension to only things that can be
exchanged with i965..

For tile modes alone, 64 bits seem enough.
For compression, you may need a separate buffer to hold compression
data, so now you have to encode the tile mode and other parameters of
the plane, parameters of the compression buffer, and possibly also the
pitch and offset of the compression buffer.
I'm not saying it can't be done, but it wouldn't be nice.

Perhaps pass the compression buffer as a separate plane? Then it gets
it's own dmabuf fd, and stride (and offset incase it really is a
single buffer) plus 64b modifier..

And then I'd have to update all the window system protocols to get the
second plane for basic RGBA formats. Thank you very much, I'm not
interested.

Immutable metadata (modifiers) stored in the kernel is the only
scalable (and thus usable) solution here. There was an argument
against _mutable_ metadata attached to BOs and the synchronization
hell it can cause, but I've not seen any argument against _immutable_
metadata. Trying to push the metadata (modifiers) through window
system protocols seems like a horrible idea to me, not just because of
that fact that window system protocols shouldn't care about
driver-specific stuff, but also because of the immense burden once you
realize that you have to fix all window system protocols and KMS apps
because 64 bits of metadata is not enough to support your hardware.
It's clearly not economically sustainable.

That said, I'm OK with the patch series (I didn't read all of it - you
still need an ack from someone else), but widespread adoption of this
feature is unlikely to happen.

Marek

[Rob Clark]

Post by Marek Olšák

Post by Rob Clark

Post by Marek Olšák

Post by Rob Clark

Post by Marek Olšák
Hi,
Is the modifier just a driver-specific description of the tiling
layout and compression?
What makes you think that 8 bytes is enough to describe that? What if
I need 9 bytes just to program the display hardware?
Drivers importing DMABUFs still have to invoke the texture tiling
calculator to get all necessary parameters for rendering (not just
display), which may even be 128 bytes per plane.

fwiw, this maps 1:1 to addfb2 ioctl, and just brings egl to parity with kms.
Maybe the addfb2 approach wasn't the best idea compared to some of the
ideas proposed for hypothetical "liballoc" for being ultra-generic.
OTOH perhaps you can just treat it like an enum? I mean maybe the set
of tiled formats that you would actually exchange with another device
is less than 2^^72. It seems reasonable to restrict the possible
tiled formats supported by this extension to only things that can be
exchanged with i965..

For tile modes alone, 64 bits seem enough.
For compression, you may need a separate buffer to hold compression
data, so now you have to encode the tile mode and other parameters of
the plane, parameters of the compression buffer, and possibly also the
pitch and offset of the compression buffer.
I'm not saying it can't be done, but it wouldn't be nice.

Perhaps pass the compression buffer as a separate plane? Then it gets
it's own dmabuf fd, and stride (and offset incase it really is a
single buffer) plus 64b modifier..

And then I'd have to update all the window system protocols to get the
second plane for basic RGBA formats. Thank you very much, I'm not
interested.

afaiu weston already supports planar formats, and I know android does.
So really only a matter of fixing x11 ;-)

Post by Marek Olšák
Immutable metadata (modifiers) stored in the kernel is the only
scalable (and thus usable) solution here. There was an argument
against _mutable_ metadata attached to BOs and the synchronization
hell it can cause, but I've not seen any argument against _immutable_
metadata. Trying to push the metadata (modifiers) through window
system protocols seems like a horrible idea to me, not just because of
that fact that window system protocols shouldn't care about
driver-specific stuff, but also because of the immense burden once you
realize that you have to fix all window system protocols and KMS apps
because 64 bits of metadata is not enough to support your hardware.
It's clearly not economically sustainable.
That said, I'm OK with the patch series (I didn't read all of it - you
still need an ack from someone else), but widespread adoption of this
feature is unlikely to happen.

I'm needing something like this for sharing tiled yuv buffers w/ video
decoder, so even outside of window-system it has some utility.

BR,
-R

[Emil Velikov]

[Pardon for dropping in uninvited]

Post by Marek Olšák
Immutable metadata (modifiers) stored in the kernel is the only
scalable (and thus usable) solution here. There was an argument
against _mutable_ metadata attached to BOs and the synchronization
hell it can cause, but I've not seen any argument against _immutable_
metadata. Trying to push the metadata (modifiers) through window
system protocols seems like a horrible idea to me, not just because of
that fact that window system protocols shouldn't care about
driver-specific stuff, but also because of the immense burden once you
realize that you have to fix all window system protocols and KMS apps
because 64 bits of metadata is not enough to support your hardware.
It's clearly not economically sustainable.

Wasn't this one of the things that were [supposed to be] discussed at
XDC as part of the gbm2/liballoc ?
Not too sure on the topic, so a simple yes/no would be appreciated.

-Emil

[Marek Olšák]

Post by Emil Velikov
[Pardon for dropping in uninvited]

Post by Marek Olšák
Immutable metadata (modifiers) stored in the kernel is the only
scalable (and thus usable) solution here. There was an argument
against _mutable_ metadata attached to BOs and the synchronization
hell it can cause, but I've not seen any argument against _immutable_
metadata. Trying to push the metadata (modifiers) through window
system protocols seems like a horrible idea to me, not just because of
that fact that window system protocols shouldn't care about
driver-specific stuff, but also because of the immense burden once you
realize that you have to fix all window system protocols and KMS apps
because 64 bits of metadata is not enough to support your hardware.
It's clearly not economically sustainable.

Wasn't this one of the things that were [supposed to be] discussed at
XDC as part of the gbm2/liballoc ?
Not too sure on the topic, so a simple yes/no would be appreciated.

Yes. There is also a thread on dri-devel About it.

Marek

Post by Emil Velikov
-Emil

[Emil Velikov]

Post by Marek Olšák

Post by Emil Velikov
[Pardon for dropping in uninvited]

Post by Marek Olšák
Immutable metadata (modifiers) stored in the kernel is the only
scalable (and thus usable) solution here. There was an argument
against _mutable_ metadata attached to BOs and the synchronization
hell it can cause, but I've not seen any argument against _immutable_
metadata. Trying to push the metadata (modifiers) through window
system protocols seems like a horrible idea to me, not just because of
that fact that window system protocols shouldn't care about
driver-specific stuff, but also because of the immense burden once you
realize that you have to fix all window system protocols and KMS apps
because 64 bits of metadata is not enough to support your hardware.
It's clearly not economically sustainable.

Wasn't this one of the things that were [supposed to be] discussed at
XDC as part of the gbm2/liballoc ?
Not too sure on the topic, so a simple yes/no would be appreciated.

Yes. There is also a thread on dri-devel About it.

Afaict the dri-devel thread started after XDC. Seemingly you/others
did not had the chance to have a productive brainstorming discussion
and/or reach a consensus ?
Either way, I won't deviate the thread any more.

Thanks
Emil

[Varad Gautam]

From: Pekka Paalanen <***@collabora.co.uk>

allow creating EGLImages with dmabuf format modifiers when target is
EGL_LINUX_DMA_BUF_EXT for EGL_EXT_image_dma_buf_import_modifiers.

v2: clear modifier assembling and error label name (Eric Engestrom)

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>
Signed-off-by: Varad Gautam <***@collabora.com>
Reviewed-by: Eric Engestrom <***@imgtec.com>
---
src/egl/drivers/dri2/egl_dri2.c | 67 ++++++++++++++++++++++++++++++++++-------
src/egl/main/egldisplay.h | 1 +
src/egl/main/eglimage.c | 49 ++++++++++++++++++++++++++++++
src/egl/main/eglimage.h | 2 ++
4 files changed, 108 insertions(+), 11 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 58d16e1..f98416c 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1937,6 +1937,21 @@ dri2_check_dma_buf_attribs(const _EGLImageAttribs *attrs)
}
}

+ /**
+ * If <target> is EGL_LINUX_DMA_BUF_EXT, both or neither of the following
+ * attribute values may be given.
+ *
+ * This is referring to EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT and
+ * EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT, and the same for other planes.
+ */
+ for (i = 0; i < DMA_BUF_MAX_PLANES; ++i) {
+ if (attrs->DMABufPlaneModifiersLo[i].IsPresent !=
+ attrs->DMABufPlaneModifiersHi[i].IsPresent) {
+ _eglError(EGL_BAD_PARAMETER, "modifier attribute lo or hi missing");
+ return EGL_FALSE;
+ }
+ }
+
return EGL_TRUE;
}

@@ -2043,7 +2058,9 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
for (i = plane_n; i < DMA_BUF_MAX_PLANES; ++i) {
if (attrs->DMABufPlaneFds[i].IsPresent ||
attrs->DMABufPlaneOffsets[i].IsPresent ||
- attrs->DMABufPlanePitches[i].IsPresent) {
+ attrs->DMABufPlanePitches[i].IsPresent ||
+ attrs->DMABufPlaneModifiersLo[i].IsPresent ||
+ attrs->DMABufPlaneModifiersHi[i].IsPresent) {
_eglError(EGL_BAD_ATTRIBUTE, "too many plane attributes");
return 0;
}
@@ -2076,6 +2093,8 @@ dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
int fds[DMA_BUF_MAX_PLANES];
int pitches[DMA_BUF_MAX_PLANES];
int offsets[DMA_BUF_MAX_PLANES];
+ uint64_t modifiers[DMA_BUF_MAX_PLANES];
+ int nonzero_modifier_found = 0;
unsigned error;

/**
@@ -2106,18 +2125,44 @@ dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
fds[i] = attrs.DMABufPlaneFds[i].Value;
pitches[i] = attrs.DMABufPlanePitches[i].Value;
offsets[i] = attrs.DMABufPlaneOffsets[i].Value;
+ if (attrs.DMABufPlaneModifiersLo[i].IsPresent) {
+ modifiers[i] = (attrs.DMABufPlaneModifiersHi[i].Value << 32) |
+ attrs.DMABufPlaneModifiersLo[i].Value;
+ if (modifiers[i] != 0)
+ nonzero_modifier_found = EGL_TRUE;
+ } else {
+ modifiers[i] = 0;
+ }
}

- dri_image =
- dri2_dpy->image->createImageFromDmaBufs(dri2_dpy->dri_screen,
- attrs.Width, attrs.Height, attrs.DMABufFourCC.Value,
- fds, num_fds, pitches, offsets,
- attrs.DMABufYuvColorSpaceHint.Value,
- attrs.DMABufSampleRangeHint.Value,
- attrs.DMABufChromaHorizontalSiting.Value,
- attrs.DMABufChromaVerticalSiting.Value,
- &error,
- NULL);
+ if (nonzero_modifier_found && dri2_dpy->image->createImageFromDmaBufs2) {
+ dri_image =
+ dri2_dpy->image->createImageFromDmaBufs2(dri2_dpy->dri_screen,
+ attrs.Width, attrs.Height, attrs.DMABufFourCC.Value,
+ fds, num_fds, pitches, offsets, modifiers,
+ attrs.DMABufYuvColorSpaceHint.Value,
+ attrs.DMABufSampleRangeHint.Value,
+ attrs.DMABufChromaHorizontalSiting.Value,
+ attrs.DMABufChromaVerticalSiting.Value,
+ &error,
+ NULL);
+ } else {
+ if (nonzero_modifier_found) {
+ _eglError(EGL_BAD_MATCH, "unsupported dma_buf format modifier");
+ return EGL_NO_IMAGE_KHR;
+ }
+
+ dri_image =
+ dri2_dpy->image->createImageFromDmaBufs(dri2_dpy->dri_screen,
+ attrs.Width, attrs.Height, attrs.DMABufFourCC.Value,
+ fds, num_fds, pitches, offsets,
+ attrs.DMABufYuvColorSpaceHint.Value,
+ attrs.DMABufSampleRangeHint.Value,
+ attrs.DMABufChromaHorizontalSiting.Value,
+ attrs.DMABufChromaVerticalSiting.Value,
+ &error,
+ NULL);
+ }
dri2_create_image_khr_texture_error(error);

if (!dri_image)
diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
index 62d9a11..7a59dc5 100644
--- a/src/egl/main/egldisplay.h
+++ b/src/egl/main/egldisplay.h
@@ -101,6 +101,7 @@ struct _egl_extensions
EGLBoolean EXT_buffer_age;
EGLBoolean EXT_create_context_robustness;
EGLBoolean EXT_image_dma_buf_import;
+ EGLBoolean EXT_image_dma_buf_import_modifiers;
EGLBoolean EXT_swap_buffers_with_damage;

EGLBoolean KHR_cl_event2;
diff --git a/src/egl/main/eglimage.c b/src/egl/main/eglimage.c
index cd170c6..f8e99d1 100644
--- a/src/egl/main/eglimage.c
+++ b/src/egl/main/eglimage.c
@@ -109,6 +109,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[0].Value = val;
attrs->DMABufPlanePitches[0].IsPresent = EGL_TRUE;
break;
+ case EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto bad_param;
+ attrs->DMABufPlaneModifiersLo[0].Value = val;
+ attrs->DMABufPlaneModifiersLo[0].IsPresent = EGL_TRUE;
+ break;
+ case EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto bad_param;
+ attrs->DMABufPlaneModifiersHi[0].Value = val;
+ attrs->DMABufPlaneModifiersHi[0].IsPresent = EGL_TRUE;
+ break;
case EGL_DMA_BUF_PLANE1_FD_EXT:
attrs->DMABufPlaneFds[1].Value = val;
attrs->DMABufPlaneFds[1].IsPresent = EGL_TRUE;
@@ -121,6 +133,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[1].Value = val;
attrs->DMABufPlanePitches[1].IsPresent = EGL_TRUE;
break;
+ case EGL_DMA_BUF_PLANE1_MODIFIER_LO_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto bad_param;
+ attrs->DMABufPlaneModifiersLo[1].Value = val;
+ attrs->DMABufPlaneModifiersLo[1].IsPresent = EGL_TRUE;
+ break;
+ case EGL_DMA_BUF_PLANE1_MODIFIER_HI_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto bad_param;
+ attrs->DMABufPlaneModifiersHi[1].Value = val;
+ attrs->DMABufPlaneModifiersHi[1].IsPresent = EGL_TRUE;
+ break;
case EGL_DMA_BUF_PLANE2_FD_EXT:
attrs->DMABufPlaneFds[2].Value = val;
attrs->DMABufPlaneFds[2].IsPresent = EGL_TRUE;
@@ -133,6 +157,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[2].Value = val;
attrs->DMABufPlanePitches[2].IsPresent = EGL_TRUE;
break;
+ case EGL_DMA_BUF_PLANE2_MODIFIER_LO_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto bad_param;
+ attrs->DMABufPlaneModifiersLo[2].Value = val;
+ attrs->DMABufPlaneModifiersLo[2].IsPresent = EGL_TRUE;
+ break;
+ case EGL_DMA_BUF_PLANE2_MODIFIER_HI_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto bad_param;
+ attrs->DMABufPlaneModifiersHi[2].Value = val;
+ attrs->DMABufPlaneModifiersHi[2].IsPresent = EGL_TRUE;
+ break;
case EGL_DMA_BUF_PLANE3_FD_EXT:
attrs->DMABufPlaneFds[3].Value = val;
attrs->DMABufPlaneFds[3].IsPresent = EGL_TRUE;
@@ -145,6 +181,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
attrs->DMABufPlanePitches[3].Value = val;
attrs->DMABufPlanePitches[3].IsPresent = EGL_TRUE;
break;
+ case EGL_DMA_BUF_PLANE3_MODIFIER_LO_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto bad_param;
+ attrs->DMABufPlaneModifiersLo[3].Value = val;
+ attrs->DMABufPlaneModifiersLo[3].IsPresent = EGL_TRUE;
+ break;
+ case EGL_DMA_BUF_PLANE3_MODIFIER_HI_EXT:
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto bad_param;
+ attrs->DMABufPlaneModifiersHi[3].Value = val;
+ attrs->DMABufPlaneModifiersHi[3].IsPresent = EGL_TRUE;
+ break;
case EGL_YUV_COLOR_SPACE_HINT_EXT:
if (val != EGL_ITU_REC601_EXT && val != EGL_ITU_REC709_EXT &&
val != EGL_ITU_REC2020_EXT) {
@@ -181,6 +229,7 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
}
break;

+bad_param:
default:
err = EGL_BAD_PARAMETER;
break;
diff --git a/src/egl/main/eglimage.h b/src/egl/main/eglimage.h
index 2c110c7..98acd88 100644
--- a/src/egl/main/eglimage.h
+++ b/src/egl/main/eglimage.h
@@ -72,6 +72,8 @@ struct _egl_image_attribs
struct _egl_image_attrib_int DMABufPlaneFds[DMA_BUF_MAX_PLANES];
struct _egl_image_attrib_int DMABufPlaneOffsets[DMA_BUF_MAX_PLANES];
struct _egl_image_attrib_int DMABufPlanePitches[DMA_BUF_MAX_PLANES];
+ struct _egl_image_attrib_int DMABufPlaneModifiersLo[DMA_BUF_MAX_PLANES];
+ struct _egl_image_attrib_int DMABufPlaneModifiersHi[DMA_BUF_MAX_PLANES];
struct _egl_image_attrib_int DMABufYuvColorSpaceHint;
struct _egl_image_attrib_int DMABufSampleRangeHint;
struct _egl_image_attrib_int DMABufChromaHorizontalSiting;

[Emil Velikov]

Post by Varad Gautam
allow creating EGLImages with dmabuf format modifiers when target is
EGL_LINUX_DMA_BUF_EXT for EGL_EXT_image_dma_buf_import_modifiers.
v2: clear modifier assembling and error label name (Eric Engestrom)
---
+ int nonzero_modifier_found = 0;
unsigned error;
/**
@@ -2106,18 +2125,44 @@ dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
fds[i] = attrs.DMABufPlaneFds[i].Value;
pitches[i] = attrs.DMABufPlanePitches[i].Value;
offsets[i] = attrs.DMABufPlaneOffsets[i].Value;
+ if (attrs.DMABufPlaneModifiersLo[i].IsPresent) {
+ modifiers[i] = (attrs.DMABufPlaneModifiersHi[i].Value << 32) |
+ attrs.DMABufPlaneModifiersLo[i].Value;
+ if (modifiers[i] != 0)
+ nonzero_modifier_found = EGL_TRUE;

integer storage and EGL_TRUE -> bool and true/false ?

Post by Varad Gautam
+ if (nonzero_modifier_found && dri2_dpy->image->createImageFromDmaBufs2) {
+ dri_image =
+ dri2_dpy->image->createImageFromDmaBufs2(dri2_dpy->dri_screen,
+ attrs.Width, attrs.Height, attrs.DMABufFourCC.Value,
+ fds, num_fds, pitches, offsets, modifiers,
+ attrs.DMABufYuvColorSpaceHint.Value,
+ attrs.DMABufSampleRangeHint.Value,
+ attrs.DMABufChromaHorizontalSiting.Value,
+ attrs.DMABufChromaVerticalSiting.Value,
+ &error,
+ NULL);
+ } else {
+ if (nonzero_modifier_found) {
+ _eglError(EGL_BAD_MATCH, "unsupported dma_buf format modifier");
+ return EGL_NO_IMAGE_KHR;
+ }
+

Using something like the following might be better?

if (nonzero_modifier_found) {
if (!dri2_dpy->image->createImageFromDmaBufs2)
# assert should never reach here, since the extension should be
advertised only if the API is available.
use new API
else
use old API

Post by Varad Gautam
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto bad_param;
+ attrs->DMABufPlaneModifiersLo[3].Value = val;
+ attrs->DMABufPlaneModifiersLo[3].IsPresent = EGL_TRUE;
+ break;
+ if (!dpy->Extensions.EXT_image_dma_buf_import_modifiers)
+ goto bad_param;
+ attrs->DMABufPlaneModifiersHi[3].Value = val;
+ attrs->DMABufPlaneModifiersHi[3].IsPresent = EGL_TRUE;
+ break;
if (val != EGL_ITU_REC601_EXT && val != EGL_ITU_REC709_EXT &&
val != EGL_ITU_REC2020_EXT) {
@@ -181,6 +229,7 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
}
break;

Using goto to jump to another case statement is "evil". Please don't use them.


Thanks
Emil

[Daniel Stone]

Hi,

Post by Emil Velikov

Post by Varad Gautam
+ if (nonzero_modifier_found && dri2_dpy->image->createImageFromDmaBufs2) {
+ dri_image =
+ dri2_dpy->image->createImageFromDmaBufs2(dri2_dpy->dri_screen,
+ attrs.Width, attrs.Height, attrs.DMABufFourCC.Value,
+ fds, num_fds, pitches, offsets, modifiers,
+ attrs.DMABufYuvColorSpaceHint.Value,
+ attrs.DMABufSampleRangeHint.Value,
+ attrs.DMABufChromaHorizontalSiting.Value,
+ attrs.DMABufChromaVerticalSiting.Value,
+ &error,
+ NULL);
+ } else {
+ if (nonzero_modifier_found) {
+ _eglError(EGL_BAD_MATCH, "unsupported dma_buf format modifier");
+ return EGL_NO_IMAGE_KHR;
+ }
+

Using something like the following might be better?
if (nonzero_modifier_found) {
if (!dri2_dpy->image->createImageFromDmaBufs2)
# assert should never reach here, since the extension should be
advertised only if the API is available.
use new API
else
use old API

Actually, present-and-zero modifier has a very well-defined meaning:
it _forces_ linear interpretation of the buffer, whereas a non-present
modifier may cause a kernel query (e.g. i915_gem_get_tiling) to
discover a hidden tiling mode. So, if present, the modifier should be
passed.

Cheers,
Daniel

[Emil Velikov]

Post by Daniel Stone
Hi,

Post by Emil Velikov

Post by Varad Gautam
+ if (nonzero_modifier_found && dri2_dpy->image->createImageFromDmaBufs2) {
+ dri_image =
+ dri2_dpy->image->createImageFromDmaBufs2(dri2_dpy->dri_screen,
+ attrs.Width, attrs.Height, attrs.DMABufFourCC.Value,
+ fds, num_fds, pitches, offsets, modifiers,
+ attrs.DMABufYuvColorSpaceHint.Value,
+ attrs.DMABufSampleRangeHint.Value,
+ attrs.DMABufChromaHorizontalSiting.Value,
+ attrs.DMABufChromaVerticalSiting.Value,
+ &error,
+ NULL);
+ } else {
+ if (nonzero_modifier_found) {
+ _eglError(EGL_BAD_MATCH, "unsupported dma_buf format modifier");
+ return EGL_NO_IMAGE_KHR;
+ }
+

Using something like the following might be better?
if (nonzero_modifier_found) {
if (!dri2_dpy->image->createImageFromDmaBufs2)
# assert should never reach here, since the extension should be
advertised only if the API is available.
use new API
else
use old API

it _forces_ linear interpretation of the buffer, whereas a non-present
modifier may cause a kernel query (e.g. i915_gem_get_tiling) to
discover a hidden tiling mode. So, if present, the modifier should be
passed.

You are suggesting that we should track "has_modifier" (as opposed to
nonzero_modifier_found) and pass it to DmaBuf2 regardless of the
contents, right ?

Just double-checking.
Emil

[Daniel Stone]

Hi Emil,

Post by Emil Velikov

Post by Daniel Stone
it _forces_ linear interpretation of the buffer, whereas a non-present
modifier may cause a kernel query (e.g. i915_gem_get_tiling) to
discover a hidden tiling mode. So, if present, the modifier should be
passed.

You are suggesting that we should track "has_modifier" (as opposed to
nonzero_modifier_found) and pass it to DmaBuf2 regardless of the
contents, right ?

Yep, exactly that.

Cheers,
Daniel