[Mesa-dev] [v2 09/10] egl: dri2: support for creating images out of dma buffers

Pohjolainen, Topi topi.pohjolainen at intel.com
Mon Apr 29 23:16:25 PDT 2013


On Mon, Apr 29, 2013 at 11:48:28PM +0200, Daniel Vetter wrote:
> On Mon, Apr 29, 2013 at 02:08:03PM +0300, Topi Pohjolainen wrote:
> > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> 
> Shouldn't we close the imported dma_buf fds somewhere after the call to
> createImageFromFds? Or have I missed something?

You are correct, the spec clearly says so:

"If eglCreateImageKHR is successful for a EGL_LINUX_DMA_BUF_EXT target, the
 EGL takes ownership of the file descriptor and is responsible for closing
 it, which it may do at any time while the EGLDisplay is initialized."

I some how just kept thinking about the gem-manager reference counting and
ignored the clearly written "file descriptor". While the reference counting got
down to zero in the piglit testing, the file descriptors remained open. I'll
revise this, and the test cases.

> 
> The wayland platform code seems to do that in create_wl_buffer.
> -Daniel
> 
> > ---
> >  src/egl/drivers/dri2/egl_dri2.c | 238 ++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 238 insertions(+)
> > 
> > diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> > index 10fdcef..2a8c614 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.c
> > +++ b/src/egl/drivers/dri2/egl_dri2.c
> > @@ -34,6 +34,7 @@
> >  #include <errno.h>
> >  #include <unistd.h>
> >  #include <xf86drm.h>
> > +#include <drm_fourcc.h>
> >  #include <GL/gl.h>
> >  #include <GL/internal/dri_interface.h>
> >  #include <sys/types.h>
> > @@ -1170,6 +1171,241 @@ dri2_create_image_mesa_drm_buffer(_EGLDisplay *disp, _EGLContext *ctx,
> >     return dri2_create_image(disp, dri_image);
> >  }
> >  
> > +static EGLBoolean
> > +dri2_check_dma_buf_attribs(const _EGLImageAttribs *attrs)
> > +{
> > +   unsigned i;
> > +
> > +   /**
> > +     * The spec says:
> > +     *
> > +     * "Required attributes and their values are as follows:
> > +     *
> > +     *  * EGL_WIDTH & EGL_HEIGHT: The logical dimensions of the buffer in pixels
> > +     *
> > +     *  * EGL_LINUX_DRM_FOURCC_EXT: The pixel format of the buffer, as specified
> > +     *    by drm_fourcc.h and used as the pixel_format parameter of the
> > +     *    drm_mode_fb_cmd2 ioctl."
> > +     *
> > +     *  * EGL_DMA_BUF_PLANE0_FD_EXT: The dma_buf file descriptor of plane 0 of
> > +     *    the image.
> > +     *
> > +     *  * EGL_DMA_BUF_PLANE0_OFFSET_EXT: The offset from the start of the
> > +     *    dma_buf of the first sample in plane 0, in bytes.
> > +     * 
> > +     *  * EGL_DMA_BUF_PLANE0_PITCH_EXT: The number of bytes between the start of
> > +     *    subsequent rows of samples in plane 0. May have special meaning for
> > +     *    non-linear formats."
> > +     *
> > +     * "* If <target> is EGL_LINUX_DMA_BUF_EXT, and the list of attributes is
> > +     *    incomplete, EGL_BAD_PARAMETER is generated."
> > +     */
> > +   if (attrs->Width <= 0 || attrs->Height <= 0 ||
> > +       !attrs->DMABufFourCC.IsPresent ||
> > +       !attrs->DMABufPlaneFds[0].IsPresent ||
> > +       !attrs->DMABufPlaneOffsets[0].IsPresent ||
> > +       !attrs->DMABufPlanePitches[0].IsPresent) {
> > +      _eglError(EGL_BAD_PARAMETER, "attribute(s) missing");
> > +      return EGL_FALSE;
> > +   }
> > +
> > +   /**
> > +    * Also:
> > +    *
> > +    * "If <target> is EGL_LINUX_DMA_BUF_EXT and one or more of the values
> > +    *  specified for a plane's pitch or offset isn't supported by EGL,
> > +    *  EGL_BAD_ACCESS is generated."
> > +    */
> > +   for (i = 0; i < sizeof(attrs->DMABufPlanePitches) /
> > +                   sizeof(attrs->DMABufPlanePitches[0]); ++i) {
> > +      if (attrs->DMABufPlanePitches[i].IsPresent &&
> > +          attrs->DMABufPlanePitches[i].Value <= 0) {
> > +         _eglError(EGL_BAD_ACCESS, "invalid pitch");
> > +         return EGL_FALSE;
> > +      }
> > +   }
> > +
> > +   return EGL_TRUE;
> > +}
> > +
> > +/* Returns the total number of file descriptors zero indicating an error. */
> > +static unsigned
> > +dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
> > +{
> > +   switch (attrs->DMABufFourCC.Value) {
> > +   case DRM_FORMAT_RGB332:
> > +   case DRM_FORMAT_BGR233:
> > +   case DRM_FORMAT_XRGB4444:
> > +   case DRM_FORMAT_XBGR4444:
> > +   case DRM_FORMAT_RGBX4444:
> > +   case DRM_FORMAT_BGRX4444:
> > +   case DRM_FORMAT_ARGB4444:
> > +   case DRM_FORMAT_ABGR4444:
> > +   case DRM_FORMAT_RGBA4444:
> > +   case DRM_FORMAT_BGRA4444:
> > +   case DRM_FORMAT_XRGB1555:
> > +   case DRM_FORMAT_XBGR1555:
> > +   case DRM_FORMAT_RGBX5551:
> > +   case DRM_FORMAT_BGRX5551:
> > +   case DRM_FORMAT_ARGB1555:
> > +   case DRM_FORMAT_ABGR1555:
> > +   case DRM_FORMAT_RGBA5551:
> > +   case DRM_FORMAT_BGRA5551:
> > +   case DRM_FORMAT_RGB565:
> > +   case DRM_FORMAT_BGR565:
> > +   case DRM_FORMAT_RGB888:
> > +   case DRM_FORMAT_BGR888:
> > +   case DRM_FORMAT_XRGB8888:
> > +   case DRM_FORMAT_XBGR8888:
> > +   case DRM_FORMAT_RGBX8888:
> > +   case DRM_FORMAT_BGRX8888:
> > +   case DRM_FORMAT_ARGB8888:
> > +   case DRM_FORMAT_ABGR8888:
> > +   case DRM_FORMAT_RGBA8888:
> > +   case DRM_FORMAT_BGRA8888:
> > +   case DRM_FORMAT_XRGB2101010:
> > +   case DRM_FORMAT_XBGR2101010:
> > +   case DRM_FORMAT_RGBX1010102:
> > +   case DRM_FORMAT_BGRX1010102:
> > +   case DRM_FORMAT_ARGB2101010:
> > +   case DRM_FORMAT_ABGR2101010:
> > +   case DRM_FORMAT_RGBA1010102:
> > +   case DRM_FORMAT_BGRA1010102:
> > +   case DRM_FORMAT_YUYV:
> > +   case DRM_FORMAT_YVYU:
> > +   case DRM_FORMAT_UYVY:
> > +   case DRM_FORMAT_VYUY:
> > +      /* There must be one and only one plane present */
> > +      if (attrs->DMABufPlaneFds[0].IsPresent &&
> > +          attrs->DMABufPlaneOffsets[0].IsPresent &&
> > +          attrs->DMABufPlanePitches[0].IsPresent &&
> > +          !attrs->DMABufPlaneFds[1].IsPresent &&
> > +          !attrs->DMABufPlaneOffsets[1].IsPresent &&
> > +          !attrs->DMABufPlanePitches[1].IsPresent &&
> > +          !attrs->DMABufPlaneFds[2].IsPresent &&
> > +          !attrs->DMABufPlaneOffsets[2].IsPresent &&
> > +          !attrs->DMABufPlanePitches[2].IsPresent)
> > +      return 1;
> > +   case DRM_FORMAT_NV12:
> > +   case DRM_FORMAT_NV21:
> > +   case DRM_FORMAT_NV16:
> > +   case DRM_FORMAT_NV61:
> > +      /* There must be two and only two planes present */
> > +      if (attrs->DMABufPlaneFds[0].IsPresent &&
> > +          attrs->DMABufPlaneOffsets[0].IsPresent &&
> > +          attrs->DMABufPlanePitches[0].IsPresent &&
> > +          attrs->DMABufPlaneFds[1].IsPresent &&
> > +          attrs->DMABufPlaneOffsets[1].IsPresent &&
> > +          attrs->DMABufPlanePitches[1].IsPresent &&
> > +          !attrs->DMABufPlaneFds[2].IsPresent &&
> > +          !attrs->DMABufPlaneOffsets[2].IsPresent &&
> > +          !attrs->DMABufPlanePitches[2].IsPresent)
> > +         return 2;
> > +      break;
> > +   case DRM_FORMAT_YUV410:
> > +   case DRM_FORMAT_YVU410:
> > +   case DRM_FORMAT_YUV411:
> > +   case DRM_FORMAT_YVU411:
> > +   case DRM_FORMAT_YUV420:
> > +   case DRM_FORMAT_YVU420:
> > +   case DRM_FORMAT_YUV422:
> > +   case DRM_FORMAT_YVU422:
> > +   case DRM_FORMAT_YUV444:
> > +   case DRM_FORMAT_YVU444:
> > +      /* All three planes must be specified */
> > +      if (attrs->DMABufPlaneFds[0].IsPresent &&
> > +          attrs->DMABufPlaneOffsets[0].IsPresent &&
> > +          attrs->DMABufPlanePitches[0].IsPresent &&
> > +          attrs->DMABufPlaneFds[1].IsPresent &&
> > +          attrs->DMABufPlaneOffsets[1].IsPresent &&
> > +          attrs->DMABufPlanePitches[1].IsPresent &&
> > +          attrs->DMABufPlaneFds[2].IsPresent &&
> > +          attrs->DMABufPlaneOffsets[2].IsPresent &&
> > +          attrs->DMABufPlanePitches[2].IsPresent)
> > +         return 3;
> > +      break;
> > +   default:
> > +     /*
> > +      * The spec says:
> > +      *
> > +      * "If <target> is EGL_LINUX_DMA_BUF_EXT, and the EGL_LINUX_DRM_FOURCC_EXT
> > +      *  attribute is set to a format not supported by the EGL, EGL_BAD_MATCH
> > +      *  is generated."
> > +      */
> > +      _eglError(EGL_BAD_MATCH, "invalid format");
> > +      return 0;
> > +   }
> > +
> > +   /**
> > +    * The spec says:
> > +    *
> > +    * "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."
> > +    */
> > +   _eglError(EGL_BAD_ATTRIBUTE, "invalid number of planes");
> > +
> > +   return 0;
> > +}
> > +
> > +static _EGLImage *
> > +dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
> > +			  EGLClientBuffer buffer, const EGLint *attr_list)
> > +{
> > +   struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> > +   EGLint err;
> > +   _EGLImageAttribs attrs;
> > +   __DRIimage *dri_image;
> > +   unsigned num_fds;
> > +   unsigned i;
> > +   int fds[3];
> > +   int pitches[3];
> > +   int offsets[3];
> > +
> > +   /**
> > +    * The spec says:
> > +    *
> > +    * ""* If <target> is EGL_LINUX_DMA_BUF_EXT and <buffer> is not NULL, the
> > +    *     error EGL_BAD_PARAMETER is generated."
> > +    */
> > +   if (buffer != NULL) {
> > +      _eglError(EGL_BAD_PARAMETER, "buffer not NULL");
> > +      return NULL;
> > +   }
> > +
> > +   err = _eglParseImageAttribList(&attrs, disp, attr_list);
> > +   if (err != EGL_SUCCESS) {
> > +      _eglError(err, "bad attribute");
> > +      return NULL;
> > +   }
> > +
> > +   if (!dri2_check_dma_buf_attribs(&attrs))
> > +      return NULL;
> > +
> > +   num_fds = dri2_check_dma_buf_format(&attrs);
> > +   if (!num_fds)
> > +      return NULL;
> > +
> > +   for (i = 0; i < num_fds; ++i) {
> > +      fds[i] = attrs.DMABufPlaneFds[i].Value;
> > +      pitches[i] = attrs.DMABufPlanePitches[i].Value;
> > +      offsets[i] = attrs.DMABufPlaneOffsets[i].Value;
> > +   }
> > +
> > +   dri_image =
> > +      dri2_dpy->image->createImageFromFds(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,
> > +         NULL);
> > +
> > +   return dri2_create_image(disp, dri_image);
> > +}
> > +
> >  #ifdef HAVE_WAYLAND_PLATFORM
> >  
> >  /* This structure describes how a wl_buffer maps to one or more
> > @@ -1364,6 +1600,8 @@ dri2_create_image_khr(_EGLDriver *drv, _EGLDisplay *disp,
> >     case EGL_WAYLAND_BUFFER_WL:
> >        return dri2_create_image_wayland_wl_buffer(disp, ctx, buffer, attr_list);
> >  #endif
> > +   case EGL_LINUX_DMA_BUF_EXT:
> > +      return dri2_create_image_dma_buf(disp, ctx, buffer, attr_list);
> >     default:
> >        _eglError(EGL_BAD_PARAMETER, "dri2_create_image_khr");
> >        return EGL_NO_IMAGE_KHR;
> > -- 
> > 1.8.1.2
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch


More information about the mesa-dev mailing list