EXT_image_dma_buf_import FD ownership

[John Sheu]

Hello folks:

About the ownership of dmabuf file descriptors that are passed into EGL.
I'm looking in particular at this blurb from the spec:

* If <target> is EGL_LINUX_DMA_BUF_EXT and eglCreateImageKHR fails,
EGL does not retain ownership of the file descriptor and it is the
responsibility of the application to close it."

My take on this is that this is different from most users of dmabuf, or
even file descriptors in general. For example, mmap() doesn't own the
descriptor passed to it; and more specifically to dmabufs, neither does
(say) DRM_IOCTL_PRIME_HANDLE_TO_FD, or any of the V4L2 entry points that
support dmabuf (e.g. VIDIOC_QBUF). They all increment the refcount on the
descriptor, not own it.

Since we're still iterating drafts on the EXT_image_dma_buf_import spec --
I'd like to see the spec specify that EGL has the similar behavior of
taking a reference, but not owning the descriptor.

As far as I see, in Mesa, only the Intel stack has
implemented EXT_image_dma_buf_import, and the change would be fairly
trivial (removing dri2_take_dma_buf_ownership), since it eventually just
passes the FD down to DRM_IOCTL_PRIME_HANDLE_TO_FD. And as far as I'm
aware, the piglit conformance tests are the only present consumers. (Maybe
the Wayland folks have something to say about this too.) Hopefully the
extension's still at a stage where we can fix up this inconsistency.

-John Sheu

[Eric Anholt]

Post by John Sheu
About the ownership of dmabuf file descriptors that are passed into EGL.
* If <target> is EGL_LINUX_DMA_BUF_EXT and eglCreateImageKHR fails,
EGL does not retain ownership of the file descriptor and it is the
responsibility of the application to close it."
My take on this is that this is different from most users of dmabuf, or
even file descriptors in general. For example, mmap() doesn't own the
descriptor passed to it; and more specifically to dmabufs, neither does
(say) DRM_IOCTL_PRIME_HANDLE_TO_FD, or any of the V4L2 entry points that
support dmabuf (e.g. VIDIOC_QBUF). They all increment the refcount on the
descriptor, not own it.
Since we're still iterating drafts on the EXT_image_dma_buf_import spec --
I'd like to see the spec specify that EGL has the similar behavior of
taking a reference, but not owning the descriptor.

It's still considered a draft? I wasn't aware of this. The file I have
of Version 5 says Status: Complete

I think it would be way more sane for the application to be always
responsible for closing its fd, and the GL had better reference any
resources it needs out of the fd before returning.

[John Sheu]

Post by Eric Anholt
It's still considered a draft? I wasn't aware of this. The file I have
of Version 5 says Status: Complete

Dang. I saw some emails on mesa-dev in October that referred to it as
still a draft. Turns out, they were from October... 2012.

Post by Eric Anholt
I think it would be way more sane for the application to be always
responsible for closing its fd, and the GL had better reference any
resources it needs out of the fd before returning.

Essentially my thoughts, and I'd prefer to get it fixed in the spec if
at all possible, while we still have a handle on all the users.

-John Sheu

[Chad Versace]

About the ownership of dmabuf file descriptors that are passed into EGL. I'm looking in particular at this blurb from
* If <target> is EGL_LINUX_DMA_BUF_EXT and eglCreateImageKHR fails,
EGL does not retain ownership of the file descriptor and it is the
responsibility of the application to close it."
My take on this is that this is different from most users of dmabuf, or even file descriptors in general. For example,
mmap() doesn't own the descriptor passed to it; and more specifically to dmabufs, neither does (say)
DRM_IOCTL_PRIME_HANDLE_TO_FD, or any of the V4L2 entry points that support dmabuf (e.g. VIDIOC_QBUF). They all
increment the refcount on the descriptor, not own it.
Since we're still iterating drafts on the EXT_image_dma_buf_import spec -- I'd like to see the spec specify that EGL has
the similar behavior of taking a reference, but not owning the descriptor.
As far as I see, in Mesa, only the Intel stack has implemented EXT_image_dma_buf_import, and the change would be fairly
trivial (removing dri2_take_dma_buf_ownership), since it eventually just passes the FD down to
DRM_IOCTL_PRIME_HANDLE_TO_FD. And as far as I'm aware, the piglit conformance tests are the only present consumers.
(Maybe the Wayland folks have something to say about this too.) Hopefully the extension's still at a stage where we
can fix up this inconsistency.

Yes. Let's update the spec to have a saner interface before we lose a handle on all the consumers.

To my knowledge, there exists only two implementations.

ARM ships an implementation for Android on Mali. I don't see such
a spec update hurting ARM, because Android devices
are fairly locked down systems with a monolithic source tree for each
device.

As you saw in Mesa, only Intel supports the extension. Today, Piglit
is the only consumer. And there exists no real Mesa release that
supports it. (That is, Mesa 9.2 doesn't support it; only git master
does).

[John Sheu]

On Fri, Oct 11, 2013 at 5:54 PM, Chad Versace

Post by Chad Versace
Yes. Let's update the spec to have a saner interface before we lose a handle
on all the consumers.

Sweet.

Post by Chad Versace
To my knowledge, there exists only two implementations.
ARM ships an implementation for Android on Mali. I don't see such
a spec update hurting ARM, because Android devices
are fairly locked down systems with a monolithic source tree for each
device.

As far as I know, the Mali driver dropped to Android doesn't have an
implementation yet, so we're clear here. The Mali driver dropped to
ChromeOS does -- because I implemented it :-). So I think we're good
on spec changes with respect to Mali. If the Intel folks are fine
too, then we can do it.

I should probably let Tom Cooksey chime in on this though.

Post by Chad Versace
As you saw in Mesa, only Intel supports the extension. Today, Piglit
is the only consumer. And there exists no real Mesa release that
supports it. (That is, Mesa 9.2 doesn't support it; only git master
does).

Even better. We'll break almost nothing.

-John Sheu

[Chad Versace]

Post by John Sheu
On Fri, Oct 11, 2013 at 5:54 PM, Chad Versace

Post by Chad Versace
Yes. Let's update the spec to have a saner interface before we lose a handle
on all the consumers.

Sweet.

Post by Chad Versace
To my knowledge, there exists only two implementations.
ARM ships an implementation for Android on Mali. I don't see such
a spec update hurting ARM, because Android devices
are fairly locked down systems with a monolithic source tree for each
device.

As far as I know, the Mali driver dropped to Android doesn't have an
implementation yet, so we're clear here. The Mali driver dropped to
ChromeOS does -- because I implemented it :-). So I think we're good
on spec changes with respect to Mali. If the Intel folks are fine
too, then we can do it.

That's a surprise! I knew ARM shipped an implementation, but I always
assumed it was for Android.

Can give us some details on how ChromeOS/ARM uses it? As you may
have heard through other channels, I've proposed that ChromeOS/Intel
use it to enable cross-process zero-copy texture uploads.

[John Sheu]

On Fri, Oct 11, 2013 at 6:32 PM, Chad Versace

Post by Chad Versace
That's a surprise! I knew ARM shipped an implementation, but I always
assumed it was for Android.
Can give us some details on how ChromeOS/ARM uses it? As you may
have heard through other channels, I've proposed that ChromeOS/Intel
use it to enable cross-process zero-copy texture uploads.

ChromeOS/ARM doesn't use it... yet. I've got a Chrome-side commit,
not in the tree yet, that would use it for zero-copying HW video
decoder textures, but it's pending resolution of a few issues
(including this one). This has actually been the driver for me to
getting the spec/implementation nailed down for
EXT_image_dma_buf_import.

-John Sheu

[Tom Cooksey]

Post by John Sheu

Post by Chad Versace
To my knowledge, there exists only two implementations.
ARM ships an implementation for Android on Mali. I don't see such
a spec update hurting ARM, because Android devices
are fairly locked down systems with a monolithic source tree for each
device.

As far as I know, the Mali driver dropped to Android doesn't have an
implementation yet, so we're clear here. The Mali driver dropped to
ChromeOS does -- because I implemented it :-). So I think we're good
on spec changes with respect to Mali. If the Intel folks are fine
too, then we can do it.
I should probably let Tom Cooksey chime in on this though.

Hiya! :-)

Yes we're fine making this change to the spec and the implementation
in the upstream Mali driver. As you say, we don't expose this on Android
as it has its own EGL_ANDROID_image_native_buffer which achieves much
the same thing. So it is really only X11 & fbdev.

I think Dave G (ARM) will raise a Khronos bug on the spec and propose
some new language. This will give other vendors a chance to comment,
though as you say, I believe ARM & Mesa are the only implementations
so doubt there will be any objections.


Cheers,

Tom

[Kristian Høgsberg]

Post by John Sheu
About the ownership of dmabuf file descriptors that are passed into EGL.
* If <target> is EGL_LINUX_DMA_BUF_EXT and eglCreateImageKHR fails,
EGL does not retain ownership of the file descriptor and it is the
responsibility of the application to close it."
My take on this is that this is different from most users of dmabuf, or even
file descriptors in general. For example, mmap() doesn't own the descriptor
passed to it; and more specifically to dmabufs, neither does (say)
DRM_IOCTL_PRIME_HANDLE_TO_FD, or any of the V4L2 entry points that support
dmabuf (e.g. VIDIOC_QBUF). They all increment the refcount on the
descriptor, not own it.
Since we're still iterating drafts on the EXT_image_dma_buf_import spec --
I'd like to see the spec specify that EGL has the similar behavior of taking
a reference, but not owning the descriptor.
As far as I see, in Mesa, only the Intel stack has implemented
EXT_image_dma_buf_import, and the change would be fairly trivial (removing
dri2_take_dma_buf_ownership), since it eventually just passes the FD down to
DRM_IOCTL_PRIME_HANDLE_TO_FD. And as far as I'm aware, the piglit
conformance tests are the only present consumers. (Maybe the Wayland folks
have something to say about this too.)

We don't use the EXT_image_dma_buf_import extension, we hide all the
details in a Wayland specific extensions. But I do agree that the fd
life-cycle semantics in the dma_buf extensions are non-standard and it
would be cleaner for it to not take ownership of the fd.

Kristian

Post by John Sheu
Hopefully the extension's still at a
stage where we can fix up this inconsistency.
-John Sheu
_______________________________________________
mesa-dev mailing list
http://lists.freedesktop.org/mailman/listinfo/mesa-dev