Re: [Mesa-dev] [PATCH v2] egl/dri2: Allow modifiers to add FDs to imports
Hi, On 11 August 2017 at 08:39, Philipp Zabel wrote: > Nice, this makes sure that all planes up to the last modifier have fds > present. And since all fds are guaranteed to be present, the modifier > equality check in dri2_check_dma_buf_attribs also makes sure that there > are no missing modifiers. Thanks both for the review! Pushed now. Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] egl/dri2: Allow modifiers to add FDs to imports
On Wed, 2017-08-09 at 11:53 +0100, 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
---
> 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;
> + }
> + }
> +
Nice, this makes sure that all planes up to the last modifier have fds
present. And since all fds are guaranteed to be present, the modifier
equality check in dri2_check_dma_buf_attribs also makes sure that there
are no missing modifiers.
Reviewed-by: Philipp Zabel
regards
Philipp
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] egl/dri2: Allow modifiers to add FDs to imports
LGTM, now existence of fd is checked foreach plane_n;
Reviewed-by: Tapani Pälli
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
---
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;
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] egl/dri2: Allow modifiers to add FDs to imports
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
---
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;
}
--
2.13.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
