Re: [Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds
On Thu, 14 Aug 2014 19:43:58 +0300 "Pohjolainen, Topi" 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 > > wrote: > > > On Wed, 13 Aug 2014 19:46:40 +0300 > > > "Pohjolainen, Topi" wrote: > > > > > >> On Fri, Aug 08, 2014 at 05:28:59PM +0300, Pekka Paalanen wrote: > > >> > From: Pekka Paalanen > > >> > > > >> > 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 > > >> > > >> 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 > > >> > > >> 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 Thu, Aug 14, 2014 at 08:50:32AM -0700, Matt Turner wrote: > On Thu, Aug 14, 2014 at 12:24 AM, Pekka Paalanen wrote: > > On Wed, 13 Aug 2014 19:46:40 +0300 > > "Pohjolainen, Topi" wrote: > > > >> On Fri, Aug 08, 2014 at 05:28:59PM +0300, Pekka Paalanen wrote: > >> > From: Pekka Paalanen > >> > > >> > 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 > >> > >> 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 > >> > >> 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, Aug 14, 2014 at 12:24 AM, Pekka Paalanen wrote: > On Wed, 13 Aug 2014 19:46:40 +0300 > "Pohjolainen, Topi" wrote: > >> On Fri, Aug 08, 2014 at 05:28:59PM +0300, Pekka Paalanen wrote: >> > From: Pekka Paalanen >> > >> > 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 >> >> 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 >> >> 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 Wed, 13 Aug 2014 19:46:40 +0300
"Pohjolainen, Topi" wrote:
> On Fri, Aug 08, 2014 at 05:28:59PM +0300, Pekka Paalanen wrote:
> > From: Pekka Paalanen
> >
> > 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
>
> 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
>
> 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 Fri, Aug 08, 2014 at 05:28:59PM +0300, Pekka Paalanen wrote:
> From: Pekka Paalanen
>
> 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
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
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
On Wed, 13 Aug 2014 07:41:50 +0200
Gwenole Beauchesne wrote:
> Hi,
>
> 2014-08-08 16:28 GMT+02:00 Pekka Paalanen :
> > From: Pekka Paalanen
> >
> > 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
Hi,
2014-08-08 16:28 GMT+02:00 Pekka Paalanen :
> From: Pekka Paalanen
>
> 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
Re: [Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds
On Fri, 8 Aug 2014 17:28:59 +0300
Pekka Paalanen wrote:
> From: Pekka Paalanen
>
> 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
> ---
>
> 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
[Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds
From: Pekka Paalanen
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
---
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
