[Mesa-dev] [PATCH v2] egl/dri2: Allow modifiers to add FDs to imports

[Tapani Pälli]

LGTM, now existence of fd is checked foreach plane_n;

Reviewed-by: Tapani Pälli <tapani.palli at intel.com>

On 08/09/2017 01:53 PM, Daniel Stone wrote:
> When using dmabuf import, make sure that the modifier is actually
> allowed to add planes to the base format, as implied by the comment.
> 
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> ---
>   src/egl/drivers/dri2/egl_dri2.c | 38 +++++++++++++++++++-------------------
>   1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index f0d1ded408..14decfed99 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -2120,6 +2120,24 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
>         return 0;
>      }
>   
> +   for (unsigned i = plane_n; i < DMA_BUF_MAX_PLANES; i++) {
> +      /**
> +       * The modifiers extension spec says:
> +       *
> +       * "Modifiers may modify any attribute of a buffer import, including
> +       *  but not limited to adding extra planes to a format which
> +       *  otherwise does not have those planes. As an example, a modifier
> +       *  may add a plane for an external compression buffer to a
> +       *  single-plane format. The exact meaning and effect of any
> +       *  modifier is canonically defined by drm_fourcc.h, not as part of
> +       *  this extension."
> +       */
> +      if (attrs->DMABufPlaneModifiersLo[i].IsPresent &&
> +          attrs->DMABufPlaneModifiersHi[i].IsPresent) {
> +         plane_n = i + 1;
> +      }
> +   }
> +
>      /**
>        * The spec says:
>        *
> @@ -2146,25 +2164,7 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
>      for (unsigned i = plane_n; i < DMA_BUF_MAX_PLANES; ++i) {
>         if (attrs->DMABufPlaneFds[i].IsPresent ||
>             attrs->DMABufPlaneOffsets[i].IsPresent ||
> -          attrs->DMABufPlanePitches[i].IsPresent ||
> -          attrs->DMABufPlaneModifiersLo[i].IsPresent ||
> -          attrs->DMABufPlaneModifiersHi[i].IsPresent) {
> -
> -         /**
> -          * The modifiers extension spec says:
> -          *
> -          * "Modifiers may modify any attribute of a buffer import, including
> -          *  but not limited to adding extra planes to a format which
> -          *  otherwise does not have those planes. As an example, a modifier
> -          *  may add a plane for an external compression buffer to a
> -          *  single-plane format. The exact meaning and effect of any
> -          *  modifier is canonically defined by drm_fourcc.h, not as part of
> -          *  this extension."
> -          */
> -         if (attrs->DMABufPlaneModifiersLo[i].IsPresent &&
> -             attrs->DMABufPlaneModifiersHi[i].IsPresent)
> -            continue;
> -
> +          attrs->DMABufPlanePitches[i].IsPresent) {
>            _eglError(EGL_BAD_ATTRIBUTE, "too many plane attributes");
>            return 0;
>         }
>