[Mesa-dev] [v7 8/9] egl/dri2: support for creating images out of dma buffers

Chad Versace chad.versace at linux.intel.com
Fri Jul 19 14:32:25 PDT 2013


On 07/10/2013 01:24 AM, Topi Pohjolainen wrote:
> v2:
>     - upon success close the given file descriptors
>
> v3:
>     - use specific entry for dma buffers instead of the basic for
>       primes, and enable the extension based on the availability
>       of the hook
>
> v4 (Chad):
>     - use ARRAY_SIZE
>     - improve the comment about the number of file descriptors
>     - in case of invalid format report EGL_BAD_ATTRIBUTE instead
>       of EGL_BAD_MATCH
>     - take into account specific error set by the driver.
>
> v5:
>     - fix error handling
>
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> ---
>   src/egl/drivers/dri2/egl_dri2.c | 278 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 278 insertions(+)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 1bce314..c4139cb 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>
> @@ -507,6 +508,10 @@ dri2_setup_screen(_EGLDisplay *disp)
>            disp->Extensions.KHR_gl_texture_2D_image = EGL_TRUE;
>            disp->Extensions.KHR_gl_texture_cubemap_image = EGL_TRUE;
>         }
> +      if (dri2_dpy->image->base.version >= 8 &&
> +          dri2_dpy->image->createImageFromDmaBufs) {
> +         disp->Extensions.EXT_image_dma_buf_import = EGL_TRUE;
> +      }
>      }
>   }
>
> @@ -1340,6 +1345,277 @@ dri2_create_image_khr_texture(_EGLDisplay *disp, _EGLContext *ctx,
>      return &dri2_img->base;
>   }
>
> +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."


Below, in dri2_check_dma_buf_format(), the checks for incomplete fd/offset/pitch
attributes emit EGL_BAD_ATTRIBUTE instead of EGL_BAD_PARAMETER. If you fix that
function to fully check for incompleteness, then the check for fd/offset/pitch
immediately below would be redundant.


> +     */
> +   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 < ARRAY_SIZE(attrs->DMABufPlanePitches); ++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 indicates 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;


Here, in the case of 2 planes, EGL_BAD_ATTRIBUTE is correctly emitted
when EGL_DMA_BUF_PLANE2_FD/OFFSET/PITCH is specified.

          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.

But EGL_BAD_ATTRIBUTE is also, and incorrectly, emitted when DMA_BUF_PLANE${N}_FD/OFFSET/PITCH
is absent. Instead, EGL_BAD_PARAMETER should be emitted.

        * If <target> is EGL_LINUX_DMA_BUF_EXT, and the list of attributes is
          incomplete, EGL_BAD_PARAMETER is generated.

Breaking the above if-statement into two should fix the problem.


> +   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:
> +      _eglError(EGL_BAD_ATTRIBUTE, "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;
> +}


Also, git complained about whitespace errors.

/home/chad/proj/linux-gfx/master/system/mesa/.git/rebase-apply/patch:53: trailing whitespace.
      *
warning: 1 line adds whitespace errors.


More information about the mesa-dev mailing list