Re: [Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds
On Wed, 13 Aug 2014 19:46:40 +0300
Pohjolainen, Topi topi.pohjolai...@intel.com wrote:
On Fri, Aug 08, 2014 at 05:28:59PM +0300, Pekka Paalanen wrote:
From: Pekka Paalanen pekka.paala...@collabora.co.uk
The EGL_EXT_image_dma_buf_import specification was revised (according to
its revision history) on Dec 5th, 2013, for EGL to not take ownership of
the file descriptors.
Do not close the file descriptors passed in to eglCreateImageKHR with
EGL_LINUX_DMA_BUF_EXT target.
It is assumed, that the drivers, which ultimately process the file
descriptors, do not close or modify them in any way either. This avoids
the need to dup(), as it seems we would only need to just close the
dup'd file descriptors right after.
Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk
I wrote the current logic based on the older version, and at least to me this
is the right thing to do. Thanks for fixing it as well as taking care of the
piglit test.
Reviewed-by: Topi Pohjolainen topi.pohjolai...@intel.com
I would be happier though if someone else gave his/her approval as well.
Thank you, I have added your R-b, and will wait some more. I think I
want the piglit patch landed first before I try to push this, anyway.
Thanks for the piglit review too, I sent a new version with your R-b
and the comment fix.
- pq
---
Hi,
the corresponding Piglit fix has already been sent to the piglit mailing
list. Both this and that need to be applied to not regress Mesa' piglit run
by one test (ext_image_dma_buf_import-ownership_transfer).
This patch fixes my test case on heavily modified Weston.
Thanks,
pq
---
src/egl/drivers/dri2/egl_dri2.c | 37 ++---
1 file changed, 6 insertions(+), 31 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.c
b/src/egl/drivers/dri2/egl_dri2.c
index 5602ec3..cd85fd3 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1678,36 +1678,13 @@ dri2_check_dma_buf_format(const _EGLImageAttribs
*attrs)
/**
* The spec says:
*
- * 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.
+ * If eglCreateImageKHR is successful for a EGL_LINUX_DMA_BUF_EXT target,
the
+ * EGL will take a reference to the dma_buf(s) which it will release at
any
+ * time while the EGLDisplay is initialized. It is the responsibility of
the
+ * application to close the dma_buf file descriptors.
+ *
+ * Therefore we must never close or otherwise modify the file descriptors.
*/
-static void
-dri2_take_dma_buf_ownership(const int *fds, unsigned num_fds)
-{
- int already_closed[num_fds];
- unsigned num_closed = 0;
- unsigned i, j;
-
- for (i = 0; i num_fds; ++i) {
- /**
- * The same file descriptor can be referenced multiple times in case
more
- * than one plane is found in the same buffer, just with a different
- * offset.
- */
- for (j = 0; j num_closed; ++j) {
- if (already_closed[j] == fds[i])
-break;
- }
-
- if (j == num_closed) {
- close(fds[i]);
- already_closed[num_closed++] = fds[i];
- }
- }
-}
-
static _EGLImage *
dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
EGLClientBuffer buffer, const EGLint *attr_list)
@@ -1770,8 +1747,6 @@ dri2_create_image_dma_buf(_EGLDisplay *disp,
_EGLContext *ctx,
return EGL_NO_IMAGE_KHR;
res = dri2_create_image_from_dri(disp, dri_image);
- if (res)
- dri2_take_dma_buf_ownership(fds, num_fds);
return res;
}
--
1.8.5.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds
On Thu, Aug 14, 2014 at 12:24 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Wed, 13 Aug 2014 19:46:40 +0300 Pohjolainen, Topi topi.pohjolai...@intel.com wrote: On Fri, Aug 08, 2014 at 05:28:59PM +0300, Pekka Paalanen wrote: From: Pekka Paalanen pekka.paala...@collabora.co.uk The EGL_EXT_image_dma_buf_import specification was revised (according to its revision history) on Dec 5th, 2013, for EGL to not take ownership of the file descriptors. Do not close the file descriptors passed in to eglCreateImageKHR with EGL_LINUX_DMA_BUF_EXT target. It is assumed, that the drivers, which ultimately process the file descriptors, do not close or modify them in any way either. This avoids the need to dup(), as it seems we would only need to just close the dup'd file descriptors right after. Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk I wrote the current logic based on the older version, and at least to me this is the right thing to do. Thanks for fixing it as well as taking care of the piglit test. Reviewed-by: Topi Pohjolainen topi.pohjolai...@intel.com I would be happier though if someone else gave his/her approval as well. Thank you, I have added your R-b, and will wait some more. I think I want the piglit patch landed first before I try to push this, anyway. Thanks for the piglit review too, I sent a new version with your R-b and the comment fix. The plan is to make the 10.3 branch tomorrow, so don't wait too long. :) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds
On Thu, Aug 14, 2014 at 08:50:32AM -0700, Matt Turner wrote: On Thu, Aug 14, 2014 at 12:24 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Wed, 13 Aug 2014 19:46:40 +0300 Pohjolainen, Topi topi.pohjolai...@intel.com wrote: On Fri, Aug 08, 2014 at 05:28:59PM +0300, Pekka Paalanen wrote: From: Pekka Paalanen pekka.paala...@collabora.co.uk The EGL_EXT_image_dma_buf_import specification was revised (according to its revision history) on Dec 5th, 2013, for EGL to not take ownership of the file descriptors. Do not close the file descriptors passed in to eglCreateImageKHR with EGL_LINUX_DMA_BUF_EXT target. It is assumed, that the drivers, which ultimately process the file descriptors, do not close or modify them in any way either. This avoids the need to dup(), as it seems we would only need to just close the dup'd file descriptors right after. Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk I wrote the current logic based on the older version, and at least to me this is the right thing to do. Thanks for fixing it as well as taking care of the piglit test. Reviewed-by: Topi Pohjolainen topi.pohjolai...@intel.com I would be happier though if someone else gave his/her approval as well. Thank you, I have added your R-b, and will wait some more. I think I want the piglit patch landed first before I try to push this, anyway. Thanks for the piglit review too, I sent a new version with your R-b and the comment fix. The plan is to make the 10.3 branch tomorrow, so don't wait too long. :) Just had a chat with Matt, better for you to commit the fix. I'll push your piglit patch. There is still time before next release if for some reason there is something amiss. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds
On Thu, 14 Aug 2014 19:43:58 +0300 Pohjolainen, Topi topi.pohjolai...@intel.com wrote: On Thu, Aug 14, 2014 at 08:50:32AM -0700, Matt Turner wrote: On Thu, Aug 14, 2014 at 12:24 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Wed, 13 Aug 2014 19:46:40 +0300 Pohjolainen, Topi topi.pohjolai...@intel.com wrote: On Fri, Aug 08, 2014 at 05:28:59PM +0300, Pekka Paalanen wrote: From: Pekka Paalanen pekka.paala...@collabora.co.uk The EGL_EXT_image_dma_buf_import specification was revised (according to its revision history) on Dec 5th, 2013, for EGL to not take ownership of the file descriptors. Do not close the file descriptors passed in to eglCreateImageKHR with EGL_LINUX_DMA_BUF_EXT target. It is assumed, that the drivers, which ultimately process the file descriptors, do not close or modify them in any way either. This avoids the need to dup(), as it seems we would only need to just close the dup'd file descriptors right after. Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk I wrote the current logic based on the older version, and at least to me this is the right thing to do. Thanks for fixing it as well as taking care of the piglit test. Reviewed-by: Topi Pohjolainen topi.pohjolai...@intel.com I would be happier though if someone else gave his/her approval as well. Thank you, I have added your R-b, and will wait some more. I think I want the piglit patch landed first before I try to push this, anyway. Thanks for the piglit review too, I sent a new version with your R-b and the comment fix. The plan is to make the 10.3 branch tomorrow, so don't wait too long. :) Just had a chat with Matt, better for you to commit the fix. I'll push your piglit patch. There is still time before next release if for some reason there is something amiss. Pushed to Mesa master only. No stable CCs per Matt's reqeust. Thanks, pq ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds
On Wed, 13 Aug 2014 07:41:50 +0200
Gwenole Beauchesne gb.de...@gmail.com wrote:
Hi,
2014-08-08 16:28 GMT+02:00 Pekka Paalanen ppaala...@gmail.com:
From: Pekka Paalanen pekka.paala...@collabora.co.uk
The EGL_EXT_image_dma_buf_import specification was revised (according to
its revision history) on Dec 5th, 2013, for EGL to not take ownership of
the file descriptors.
Do not close the file descriptors passed in to eglCreateImageKHR with
EGL_LINUX_DMA_BUF_EXT target.
It is assumed, that the drivers, which ultimately process the file
descriptors, do not close or modify them in any way either. This avoids
the need to dup(), as it seems we would only need to just close the
dup'd file descriptors right after.
Since this was a so radical change, shouldn't have change the API
string name instead to EXT_image_dma_buf_import2 for instance?
It's like this in the Khronos registry, so how could I change the name?
That is, I can write the code, but I need to know that is actually
wanted and correct, and someone will do the same for the specs at
Khronos.
FWIW, when I wrote experimental dma_buf support in Weston, I was
reading the Khronos spec. I just made a temporary hack to briefly be
able to test on Mesa. And I know of a proprietary EGL implementation
that implements this like the new spec, not like Mesa.
I would like to consider this as just a Mesa bug, and a spec bug that
was already fixed.
Anyway, could you please point to the following bug in your patch?
https://bugs.freedesktop.org/show_bug.cgi?id=76188
Done, thanks for the pointer. I also commented on the bug.
After someone tells me whether or not I should add the stable CC's, I
can send a new version.
Thanks,
pq
Thanks.
the corresponding Piglit fix has already been sent to the piglit mailing
list. Both this and that need to be applied to not regress Mesa' piglit run
by one test (ext_image_dma_buf_import-ownership_transfer).
This patch fixes my test case on heavily modified Weston.
Thanks,
pq
---
src/egl/drivers/dri2/egl_dri2.c | 37 ++---
1 file changed, 6 insertions(+), 31 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.c
b/src/egl/drivers/dri2/egl_dri2.c
index 5602ec3..cd85fd3 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1678,36 +1678,13 @@ dri2_check_dma_buf_format(const _EGLImageAttribs
*attrs)
/**
* The spec says:
*
- * 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.
+ * If eglCreateImageKHR is successful for a EGL_LINUX_DMA_BUF_EXT target,
the
+ * EGL will take a reference to the dma_buf(s) which it will release at
any
+ * time while the EGLDisplay is initialized. It is the responsibility of
the
+ * application to close the dma_buf file descriptors.
+ *
+ * Therefore we must never close or otherwise modify the file descriptors.
*/
-static void
-dri2_take_dma_buf_ownership(const int *fds, unsigned num_fds)
-{
- int already_closed[num_fds];
- unsigned num_closed = 0;
- unsigned i, j;
-
- for (i = 0; i num_fds; ++i) {
- /**
- * The same file descriptor can be referenced multiple times in case
more
- * than one plane is found in the same buffer, just with a different
- * offset.
- */
- for (j = 0; j num_closed; ++j) {
- if (already_closed[j] == fds[i])
-break;
- }
-
- if (j == num_closed) {
- close(fds[i]);
- already_closed[num_closed++] = fds[i];
- }
- }
-}
-
static _EGLImage *
dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
EGLClientBuffer buffer, const EGLint *attr_list)
@@ -1770,8 +1747,6 @@ dri2_create_image_dma_buf(_EGLDisplay *disp,
_EGLContext *ctx,
return EGL_NO_IMAGE_KHR;
res = dri2_create_image_from_dri(disp, dri_image);
- if (res)
- dri2_take_dma_buf_ownership(fds, num_fds);
return res;
}
--
1.8.5.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Regards,
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds
On Fri, Aug 08, 2014 at 05:28:59PM +0300, Pekka Paalanen wrote:
From: Pekka Paalanen pekka.paala...@collabora.co.uk
The EGL_EXT_image_dma_buf_import specification was revised (according to
its revision history) on Dec 5th, 2013, for EGL to not take ownership of
the file descriptors.
Do not close the file descriptors passed in to eglCreateImageKHR with
EGL_LINUX_DMA_BUF_EXT target.
It is assumed, that the drivers, which ultimately process the file
descriptors, do not close or modify them in any way either. This avoids
the need to dup(), as it seems we would only need to just close the
dup'd file descriptors right after.
Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk
I wrote the current logic based on the older version, and at least to me this
is the right thing to do. Thanks for fixing it as well as taking care of the
piglit test.
Reviewed-by: Topi Pohjolainen topi.pohjolai...@intel.com
I would be happier though if someone else gave his/her approval as well.
---
Hi,
the corresponding Piglit fix has already been sent to the piglit mailing
list. Both this and that need to be applied to not regress Mesa' piglit run
by one test (ext_image_dma_buf_import-ownership_transfer).
This patch fixes my test case on heavily modified Weston.
Thanks,
pq
---
src/egl/drivers/dri2/egl_dri2.c | 37 ++---
1 file changed, 6 insertions(+), 31 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 5602ec3..cd85fd3 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1678,36 +1678,13 @@ dri2_check_dma_buf_format(const _EGLImageAttribs
*attrs)
/**
* The spec says:
*
- * 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.
+ * If eglCreateImageKHR is successful for a EGL_LINUX_DMA_BUF_EXT target,
the
+ * EGL will take a reference to the dma_buf(s) which it will release at any
+ * time while the EGLDisplay is initialized. It is the responsibility of the
+ * application to close the dma_buf file descriptors.
+ *
+ * Therefore we must never close or otherwise modify the file descriptors.
*/
-static void
-dri2_take_dma_buf_ownership(const int *fds, unsigned num_fds)
-{
- int already_closed[num_fds];
- unsigned num_closed = 0;
- unsigned i, j;
-
- for (i = 0; i num_fds; ++i) {
- /**
- * The same file descriptor can be referenced multiple times in case
more
- * than one plane is found in the same buffer, just with a different
- * offset.
- */
- for (j = 0; j num_closed; ++j) {
- if (already_closed[j] == fds[i])
-break;
- }
-
- if (j == num_closed) {
- close(fds[i]);
- already_closed[num_closed++] = fds[i];
- }
- }
-}
-
static _EGLImage *
dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
EGLClientBuffer buffer, const EGLint *attr_list)
@@ -1770,8 +1747,6 @@ dri2_create_image_dma_buf(_EGLDisplay *disp,
_EGLContext *ctx,
return EGL_NO_IMAGE_KHR;
res = dri2_create_image_from_dri(disp, dri_image);
- if (res)
- dri2_take_dma_buf_ownership(fds, num_fds);
return res;
}
--
1.8.5.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds
Hi,
2014-08-08 16:28 GMT+02:00 Pekka Paalanen ppaala...@gmail.com:
From: Pekka Paalanen pekka.paala...@collabora.co.uk
The EGL_EXT_image_dma_buf_import specification was revised (according to
its revision history) on Dec 5th, 2013, for EGL to not take ownership of
the file descriptors.
Do not close the file descriptors passed in to eglCreateImageKHR with
EGL_LINUX_DMA_BUF_EXT target.
It is assumed, that the drivers, which ultimately process the file
descriptors, do not close or modify them in any way either. This avoids
the need to dup(), as it seems we would only need to just close the
dup'd file descriptors right after.
Since this was a so radical change, shouldn't have change the API
string name instead to EXT_image_dma_buf_import2 for instance?
Anyway, could you please point to the following bug in your patch?
https://bugs.freedesktop.org/show_bug.cgi?id=76188
Thanks.
the corresponding Piglit fix has already been sent to the piglit mailing
list. Both this and that need to be applied to not regress Mesa' piglit run
by one test (ext_image_dma_buf_import-ownership_transfer).
This patch fixes my test case on heavily modified Weston.
Thanks,
pq
---
src/egl/drivers/dri2/egl_dri2.c | 37 ++---
1 file changed, 6 insertions(+), 31 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 5602ec3..cd85fd3 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1678,36 +1678,13 @@ dri2_check_dma_buf_format(const _EGLImageAttribs
*attrs)
/**
* The spec says:
*
- * 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.
+ * If eglCreateImageKHR is successful for a EGL_LINUX_DMA_BUF_EXT target,
the
+ * EGL will take a reference to the dma_buf(s) which it will release at any
+ * time while the EGLDisplay is initialized. It is the responsibility of the
+ * application to close the dma_buf file descriptors.
+ *
+ * Therefore we must never close or otherwise modify the file descriptors.
*/
-static void
-dri2_take_dma_buf_ownership(const int *fds, unsigned num_fds)
-{
- int already_closed[num_fds];
- unsigned num_closed = 0;
- unsigned i, j;
-
- for (i = 0; i num_fds; ++i) {
- /**
- * The same file descriptor can be referenced multiple times in case
more
- * than one plane is found in the same buffer, just with a different
- * offset.
- */
- for (j = 0; j num_closed; ++j) {
- if (already_closed[j] == fds[i])
-break;
- }
-
- if (j == num_closed) {
- close(fds[i]);
- already_closed[num_closed++] = fds[i];
- }
- }
-}
-
static _EGLImage *
dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
EGLClientBuffer buffer, const EGLint *attr_list)
@@ -1770,8 +1747,6 @@ dri2_create_image_dma_buf(_EGLDisplay *disp,
_EGLContext *ctx,
return EGL_NO_IMAGE_KHR;
res = dri2_create_image_from_dri(disp, dri_image);
- if (res)
- dri2_take_dma_buf_ownership(fds, num_fds);
return res;
}
--
1.8.5.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Regards,
--
Gwenole Beauchesne
Intel Corporation SAS / 2 rue de Paris, 92196 Meudon Cedex, France
Registration Number (RCS): Nanterre B 302 456 199
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds
From: Pekka Paalanen pekka.paala...@collabora.co.uk
The EGL_EXT_image_dma_buf_import specification was revised (according to
its revision history) on Dec 5th, 2013, for EGL to not take ownership of
the file descriptors.
Do not close the file descriptors passed in to eglCreateImageKHR with
EGL_LINUX_DMA_BUF_EXT target.
It is assumed, that the drivers, which ultimately process the file
descriptors, do not close or modify them in any way either. This avoids
the need to dup(), as it seems we would only need to just close the
dup'd file descriptors right after.
Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk
---
Hi,
the corresponding Piglit fix has already been sent to the piglit mailing
list. Both this and that need to be applied to not regress Mesa' piglit run
by one test (ext_image_dma_buf_import-ownership_transfer).
This patch fixes my test case on heavily modified Weston.
Thanks,
pq
---
src/egl/drivers/dri2/egl_dri2.c | 37 ++---
1 file changed, 6 insertions(+), 31 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 5602ec3..cd85fd3 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1678,36 +1678,13 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
/**
* The spec says:
*
- * 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.
+ * If eglCreateImageKHR is successful for a EGL_LINUX_DMA_BUF_EXT target, the
+ * EGL will take a reference to the dma_buf(s) which it will release at any
+ * time while the EGLDisplay is initialized. It is the responsibility of the
+ * application to close the dma_buf file descriptors.
+ *
+ * Therefore we must never close or otherwise modify the file descriptors.
*/
-static void
-dri2_take_dma_buf_ownership(const int *fds, unsigned num_fds)
-{
- int already_closed[num_fds];
- unsigned num_closed = 0;
- unsigned i, j;
-
- for (i = 0; i num_fds; ++i) {
- /**
- * The same file descriptor can be referenced multiple times in case more
- * than one plane is found in the same buffer, just with a different
- * offset.
- */
- for (j = 0; j num_closed; ++j) {
- if (already_closed[j] == fds[i])
-break;
- }
-
- if (j == num_closed) {
- close(fds[i]);
- already_closed[num_closed++] = fds[i];
- }
- }
-}
-
static _EGLImage *
dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
EGLClientBuffer buffer, const EGLint *attr_list)
@@ -1770,8 +1747,6 @@ dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext
*ctx,
return EGL_NO_IMAGE_KHR;
res = dri2_create_image_from_dri(disp, dri_image);
- if (res)
- dri2_take_dma_buf_ownership(fds, num_fds);
return res;
}
--
1.8.5.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds
On Fri, 8 Aug 2014 17:28:59 +0300
Pekka Paalanen ppaala...@gmail.com wrote:
From: Pekka Paalanen pekka.paala...@collabora.co.uk
The EGL_EXT_image_dma_buf_import specification was revised (according to
its revision history) on Dec 5th, 2013, for EGL to not take ownership of
the file descriptors.
Do not close the file descriptors passed in to eglCreateImageKHR with
EGL_LINUX_DMA_BUF_EXT target.
It is assumed, that the drivers, which ultimately process the file
descriptors, do not close or modify them in any way either. This avoids
the need to dup(), as it seems we would only need to just close the
dup'd file descriptors right after.
Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk
---
Hi,
the corresponding Piglit fix has already been sent to the piglit mailing
list. Both this and that need to be applied to not regress Mesa' piglit run
by one test (ext_image_dma_buf_import-ownership_transfer).
This patch fixes my test case on heavily modified Weston.
Sorry, I forgot. This patch would apply also to the 10.0, 10.1, and
10.2 stable branches. Should it be a candidate there?
Thanks,
pq
---
src/egl/drivers/dri2/egl_dri2.c | 37 ++---
1 file changed, 6 insertions(+), 31 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 5602ec3..cd85fd3 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1678,36 +1678,13 @@ dri2_check_dma_buf_format(const _EGLImageAttribs
*attrs)
/**
* The spec says:
*
- * 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.
+ * If eglCreateImageKHR is successful for a EGL_LINUX_DMA_BUF_EXT target,
the
+ * EGL will take a reference to the dma_buf(s) which it will release at any
+ * time while the EGLDisplay is initialized. It is the responsibility of the
+ * application to close the dma_buf file descriptors.
+ *
+ * Therefore we must never close or otherwise modify the file descriptors.
*/
-static void
-dri2_take_dma_buf_ownership(const int *fds, unsigned num_fds)
-{
- int already_closed[num_fds];
- unsigned num_closed = 0;
- unsigned i, j;
-
- for (i = 0; i num_fds; ++i) {
- /**
- * The same file descriptor can be referenced multiple times in case
more
- * than one plane is found in the same buffer, just with a different
- * offset.
- */
- for (j = 0; j num_closed; ++j) {
- if (already_closed[j] == fds[i])
-break;
- }
-
- if (j == num_closed) {
- close(fds[i]);
- already_closed[num_closed++] = fds[i];
- }
- }
-}
-
static _EGLImage *
dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
EGLClientBuffer buffer, const EGLint *attr_list)
@@ -1770,8 +1747,6 @@ dri2_create_image_dma_buf(_EGLDisplay *disp,
_EGLContext *ctx,
return EGL_NO_IMAGE_KHR;
res = dri2_create_image_from_dri(disp, dri_image);
- if (res)
- dri2_take_dma_buf_ownership(fds, num_fds);
return res;
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
