[Mesa-dev] [PATCH v13 07/36] egl: implement eglQueryDmaBufModifiersEXT

[Varad Gautam]

On Mon, 2017-05-22 at 14:03 +0100, Emil Velikov wrote:
> Hi Dan,
> 
> On 19 May 2017 at 10:37, Daniel Stone <daniels at collabora.com> wrote:
> > 
> > From: Varad Gautam <varadgautam at gmail.com>
> > 
> > query and return supported dmabuf format modifiers for
> > EGL_EXT_image_dma_buf_import_modifiers.
> > 
> > v2: move format check to the driver instead of making format queries
> >     here and then checking.
> > v3: Check DRIimageExtension version before query (Daniel Stone)
> > 
> > Signed-off-by: Varad Gautam <varad.gautam at collabora.com>
> > Reviewed-by: Daniel Stone <daniels at collabora.com>
> > Signed-off-by: Daniel Stone <daniels at collabora.com>
> > ---
> >  src/egl/drivers/dri2/egl_dri2.c | 36 ++++++++++++++++++++++++++++++++++++
> >  src/egl/main/eglapi.c           | 20 ++++++++++++++++++++
> >  src/egl/main/eglapi.h           |  5 +++++
> >  src/egl/main/eglentrypoint.h    |  1 +
> >  4 files changed, 62 insertions(+)
> > 
> > diff --git a/src/egl/drivers/dri2/egl_dri2.c
> > b/src/egl/drivers/dri2/egl_dri2.c
> > index 55b6fcf1fc..1e0302359f 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.c
> > +++ b/src/egl/drivers/dri2/egl_dri2.c
> > @@ -2131,6 +2131,41 @@ 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;
> > +   }
> > +
> Thinking out loud: I'm split if these should be part of /dri2 or
> /main. Either way is fine I guess.
> 
> > 
> > +   if (dri2_dpy->image->base.version < 16)
> > +      return EGL_FALSE;
> > +
> > +   if (dri2_dpy->image->queryDmaBufModifiers(dri2_dpy->dri_screen, format,
> As Jason mentioned function pointer can be NULL.
> 
> > 
> > +   if (external_only != NULL) {
> > +      for (i = 0; i < *count && i < max; i++)
> > +         external_only[i] = EGL_TRUE;
> 
> If I'm correctly understanding the spec, each bool represents if the
> respective format/modifier combo.
> Do we want to unconditionally enable all here?
> 

Yes, bool external_only[i] corresponds to <format, modifier[i]> combination.

I am uncertain on what using modifiers with non-external images would mean,
since modifiers are only specific to the dmabufs path, which bind as external
textures.

Would we ever have modifiers with non-GL_TEXTURE_EXTERNAL_OES images? (should
external_only be false perhaps for MOD_INVALID?)

Regards,
Varad

> -Emil