[Mesa-dev] [PATCH 04/13] egl/main: add support for fourth plane tokens

[Emil Velikov]

On 15 November 2016 at 14:24, Varad Gautam <varadgautam at gmail.com> wrote:
> From: Pekka Paalanen <pekka.paalanen at 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 <pekka.paalanen at collabora.co.uk>
> Signed-off-by: Varad Gautam <varad.gautam at 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;
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