Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Hi Hans and Rémi, On Thu, Aug 02, 2012 at 08:35:58AM +0200, Hans Verkuil wrote: ... Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. As far as I understand, V4L1 did have that limitation, as well as videobuf1 and 2 and a number of other drivers, but it's not found in the V4L2 core itself. While I'm not aware of a driver that'd allow creating more buffers than that the changes required to support more would be likely limited to videobuf2. Regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
On Wed 8 August 2012 11:35:38 Sakari Ailus wrote: Hi Hans and Rémi, On Thu, Aug 02, 2012 at 08:35:58AM +0200, Hans Verkuil wrote: ... Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. As far as I understand, V4L1 did have that limitation, as well as videobuf1 and 2 and a number of other drivers, but it's not found in the V4L2 core itself. While I'm not aware of a driver that'd allow creating more buffers than that the changes required to support more would be likely limited to videobuf2. You are correct. It does not touch the v4l2 core, just videobuf and videobuf2. Although the define is in videodev2.h as well, so it's something that apps might use as well. But frankly, 32 is a very generous maximum anyway. Only in special cases can I imagine needing more buffers (frame slicing, high-speed capture). Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote: Le mercredi 1 août 2012 14:35:03 Laurent Pinchart, vous avez écrit : But in general, the V4L element in the pipeline does not know how fast the downstream element(s) will consume the buffers. Thus it has to copy from the MMAP buffers into anonymous user memory pending processing. Then any dequeued buffer can be requeued as soon as possible. In theory, it might also be that, even though the latency is known, the number of required buffers exceeds the maximum MMAP buffers count of the V4L device. Either way, user space ends up doing memory copy from MMAP to custom buffers. This problem does not exist with USERBUF - the V4L2 element can simply allocate a new buffer for each dequeued buffer. What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ? Does CREATE_BUFS always work while already streaming has already started? If it depends on the driver, it's kinda helpless. Yes, it does. It's one of the reasons it exists in the first place. But there are currently only a handful of drivers that implement it. I hope that as more and more drivers are converted to vb2 that the availability of create_bufs will increase. What's the guaranteed minimum buffer count? It seems in any case, MMAP has a hard limit of 32 buffers (at least videobuf2 has), though one might argue this should be more than enough. Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. Although drivers may force a lower maximum if they want. I have no idea whether there are drivers that do that. There probably are. The minimum is usually between 1 and 3, depending on hardware limitations. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit : On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote: What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ? Does CREATE_BUFS always work while already streaming has already started? If it depends on the driver, it's kinda helpless. Yes, it does. It's one of the reasons it exists in the first place. But there are currently only a handful of drivers that implement it. I hope that as more and more drivers are converted to vb2 that the availability of create_bufs will increase. That's contradictory. If most drivers do not support it, then it won't work during streaming. What's the guaranteed minimum buffer count? It seems in any case, MMAP has a hard limit of 32 buffers (at least videobuf2 has), though one might argue this should be more than enough. Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. Although drivers may force a lower maximum if they want. I have no idea whether there are drivers that do that. There probably are. The smallest of the maxima of all drivers. The minimum is usually between 1 and 3, depending on hardware limitations. And that's clearly insufficient without memory copy to userspace buffers. It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for REQBUFS+USERBUF then. -- Rémi Denis-Courmont http://www.remlab.net/ http://fi.linkedin.com/in/remidenis -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
On Thu August 2 2012 08:56:43 Rémi Denis-Courmont wrote: Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit : On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote: What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ? Does CREATE_BUFS always work while already streaming has already started? If it depends on the driver, it's kinda helpless. Yes, it does. It's one of the reasons it exists in the first place. But there are currently only a handful of drivers that implement it. I hope that as more and more drivers are converted to vb2 that the availability of create_bufs will increase. That's contradictory. If most drivers do not support it, then it won't work during streaming. IF create_bufs is implemented in the driver, THEN you can use it during streaming. I.e., it will never return EBUSY as an error due to the fact that streaming is in progress. Obviously it won't work if the driver didn't implement it in the first place. What's the guaranteed minimum buffer count? It seems in any case, MMAP has a hard limit of 32 buffers (at least videobuf2 has), though one might argue this should be more than enough. Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. Although drivers may force a lower maximum if they want. I have no idea whether there are drivers that do that. There probably are. The smallest of the maxima of all drivers. I've no idea. Most will probably abide by the 32 maximum, but without analyzing all drivers I can't guarantee it. The minimum is usually between 1 and 3, depending on hardware limitations. And that's clearly insufficient without memory copy to userspace buffers. It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for REQBUFS+USERBUF then. Just to put your mind at rest: USERPTR mode will *not* disappear or be deprecated in any way. It's been there for a long time, it's in heavy use, it's easy to use and it will not be turned into a second class citizen, because it isn't. Just because there is a new dmabuf mode available doesn't mean that everything should be done as a mmap+dmabuf thing. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Hi Rémi, On Thursday 02 August 2012 09:56:43 Rémi Denis-Courmont wrote: Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit : On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote: What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ? Does CREATE_BUFS always work while already streaming has already started? If it depends on the driver, it's kinda helpless. Yes, it does. It's one of the reasons it exists in the first place. But there are currently only a handful of drivers that implement it. I hope that as more and more drivers are converted to vb2 that the availability of create_bufs will increase. That's contradictory. If most drivers do not support it, then it won't work during streaming. What's the guaranteed minimum buffer count? It seems in any case, MMAP has a hard limit of 32 buffers (at least videobuf2 has), though one might argue this should be more than enough. Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. Although drivers may force a lower maximum if they want. I have no idea whether there are drivers that do that. There probably are. The smallest of the maxima of all drivers. The minimum is usually between 1 and 3, depending on hardware limitations. And that's clearly insufficient without memory copy to userspace buffers. That's the minimum number of buffers *required* by the hardware. You can add up to 32 buffers, I'm not aware of any driver that would prevent that. It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for REQBUFS+USERBUF then. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Hi Hans, On Thursday 02 August 2012 09:08:18 Hans Verkuil wrote: On Thu August 2 2012 08:56:43 Rémi Denis-Courmont wrote: Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit : On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote: What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ? Does CREATE_BUFS always work while already streaming has already started? If it depends on the driver, it's kinda helpless. Yes, it does. It's one of the reasons it exists in the first place. But there are currently only a handful of drivers that implement it. I hope that as more and more drivers are converted to vb2 that the availability of create_bufs will increase. That's contradictory. If most drivers do not support it, then it won't work during streaming. IF create_bufs is implemented in the driver, THEN you can use it during streaming. I.e., it will never return EBUSY as an error due to the fact that streaming is in progress. Obviously it won't work if the driver didn't implement it in the first place. What's the guaranteed minimum buffer count? It seems in any case, MMAP has a hard limit of 32 buffers (at least videobuf2 has), though one might argue this should be more than enough. Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core. Although drivers may force a lower maximum if they want. I have no idea whether there are drivers that do that. There probably are. The smallest of the maxima of all drivers. I've no idea. Most will probably abide by the 32 maximum, but without analyzing all drivers I can't guarantee it. The minimum is usually between 1 and 3, depending on hardware limitations. And that's clearly insufficient without memory copy to userspace buffers. It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for REQBUFS+USERBUF then. Just to put your mind at rest: USERPTR mode will *not* disappear or be deprecated in any way. It's been there for a long time, it's in heavy use, it's easy to use and it will not be turned into a second class citizen, because it isn't. Just because there is a new dmabuf mode available doesn't mean that everything should be done as a mmap+dmabuf thing. I disagree with this. Not everything should obviously be done with MMAP + DMABUF, but for buffer sharing between devices, we should encourage application developers to use DMABUF instead of USERPTR. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Hi Hans,
On 07/31/2012 02:11 PM, Hans Verkuil wrote:
On Tue 31 July 2012 13:56:14 Laurent Pinchart wrote:
Hi Hans,
On Tuesday 31 July 2012 08:33:56 Hans Verkuil wrote:
On Thu June 14 2012 16:32:23 Tomasz Stanislawski wrote:
+/**
+ * struct v4l2_exportbuffer - export of video buffer as DMABUF file
descriptor + *
+ * @fd: file descriptor associated with DMABUF (set by driver)
+ * @mem_offset: buffer memory offset as returned by VIDIOC_QUERYBUF in
struct + * v4l2_buffer::m.offset (for single-plane formats) or
+ *v4l2_plane::m.offset (for multi-planar formats)
+ * @flags:flags for newly created file, currently only O_CLOEXEC
is
+ *supported, refer to manual of open syscall for more
details
+ *
+ * Contains data used for exporting a video buffer as DMABUF file
descriptor. + * The buffer is identified by a 'cookie' returned by
VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to
userspace). All + * reserved fields must be set to zero. The field
reserved0 is expected to + * become a structure 'type' allowing an
alternative layout of the structure + * content. Therefore this field
should not be used for any other extensions. + */
+struct v4l2_exportbuffer {
+ __u32 fd;
+ __u32 reserved0;
+ __u32 mem_offset;
This should be a union identical to the m union in v4l2_plane, together with
a u32 memory field. That way you can just copy memory and m from
v4l2_plane/buffer and call expbuf. If new memory types are added in the
future, then you don't need to change this struct.
OK, reserved0 could be replace by __u32 memory. Could we have other dma-buf
export types in the future not corresponding to a memory type ? I don't see
any use case right now though.
The memory type should be all you need. And the union is also needed since the
userptr value is unsigned long, thus ensuring that you have 64-bits available
for pointer types in the future. That's really my main point: the union should
have the same size as the union in v4l2_buffer/plane, allowing for the same
size pointers or whatever to be added in the future.
I do not see any good point in using v4l2_plane. What would be the meaning
of bytesused, length, data_offset in case of DMABUF exporting?
The field reserved0 was introduced to be replaced by __u32 memory if other means
of buffer description would be needed. The reserved fields at the end of
the structure could be used for auxiliary parameters other then offset and
flags.
The flags field is expected to be used by all exporting types therefore the
layout could be reorganized to:
struct v4l2_exportbuffers {
__u32 fd;
__u32 flags;
__u32 reserved0[2]; /* place for '__u32 memory' + forcing 64 bit
alignment */
__u32 mem_offset; /* what do you thing about using 64-bit field? */
__u32 reserved1[11];
};
What is your opinion about this idea?
For that matter, wouldn't it be useful to support exporting a userptr buffer
at some point in the future?
Shouldn't USERPTR usage be discouraged once we get dma-buf support ?
Why? It's perfectly fine to use it and it's not going away.
I'm not saying that we should support exporting a userptr buffer as a dmabuf
fd,
but I'm just wondering if that is possible at all and how difficult it would
be.
It would be difficult. Currently there is no safe/portable way to transform
a userptr into a scatterlist mappable for other devices. The most trouble
some examples are userspace-mapping of IO memory like framebuffers.
How reference management is going to work if there are no struct pages
describing mapped memory?
The VB2 uses a workaround by keeping a copy of vma that is used to call
open/close callbacks. I am not sure how reliable this solution is.
Who knows, maybe in future someone will introduce a mechanism for creation of
DMABUF descriptor from a userptr without any help of client APIs.
In such a case, it will be the part of DMABUF api not V4L2 :).
Regards,
Tomasz Stanislawski
Regards,
Hans
BTW, this patch series needs to be rebased to the latest for_v3.6. Quite a
few core things have changed when it comes to adding new ioctls.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Hi Tomasz,
On Wednesday 01 August 2012 10:01:45 Tomasz Stanislawski wrote:
On 07/31/2012 02:11 PM, Hans Verkuil wrote:
On Tue 31 July 2012 13:56:14 Laurent Pinchart wrote:
On Tuesday 31 July 2012 08:33:56 Hans Verkuil wrote:
On Thu June 14 2012 16:32:23 Tomasz Stanislawski wrote:
+/**
+ * struct v4l2_exportbuffer - export of video buffer as DMABUF file
descriptor + *
+ * @fd: file descriptor associated with DMABUF (set by driver)
+ * @mem_offset: buffer memory offset as returned by VIDIOC_QUERYBUF
in
struct + * v4l2_buffer::m.offset (for single-plane
formats) or
+ * v4l2_plane::m.offset (for multi-planar formats)
+ * @flags: flags for newly created file, currently only O_CLOEXEC
is
+ * supported, refer to manual of open syscall for more
details
+ *
+ * Contains data used for exporting a video buffer as DMABUF file
descriptor. + * The buffer is identified by a 'cookie' returned by
VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer
to
userspace). All + * reserved fields must be set to zero. The field
reserved0 is expected to + * become a structure 'type' allowing an
alternative layout of the structure + * content. Therefore this field
should not be used for any other extensions. + */
+struct v4l2_exportbuffer {
+__u32 fd;
+__u32 reserved0;
+__u32 mem_offset;
This should be a union identical to the m union in v4l2_plane, together
with a u32 memory field. That way you can just copy memory and m from
v4l2_plane/buffer and call expbuf. If new memory types are added in the
future, then you don't need to change this struct.
OK, reserved0 could be replace by __u32 memory. Could we have other
dma-buf
export types in the future not corresponding to a memory type ? I don't
see
any use case right now though.
The memory type should be all you need. And the union is also needed since
the userptr value is unsigned long, thus ensuring that you have 64-bits
available for pointer types in the future. That's really my main point:
the union should have the same size as the union in v4l2_buffer/plane,
allowing for the same size pointers or whatever to be added in the
future.
I do not see any good point in using v4l2_plane. What would be the meaning
of bytesused, length, data_offset in case of DMABUF exporting?
I don't think Hans meant using v4l2_plane directly, but to use the same union
as in v4l2_plane.
The field reserved0 was introduced to be replaced by __u32 memory if other
means of buffer description would be needed. The reserved fields at the end
of the structure could be used for auxiliary parameters other then offset
and flags. The flags field is expected to be used by all exporting types
therefore the layout could be reorganized to:
struct v4l2_exportbuffers {
__u32 fd;
__u32 flags;
__u32 reserved0[2]; /* place for '__u32 memory' + forcing 64 bit
alignment
*/ __u32 mem_offset; /* what do you thing about using 64-bit field? */
__u32 reserved1[11];
};
What is your opinion about this idea?
For that matter, wouldn't it be useful to support exporting a userptr
buffer at some point in the future?
Shouldn't USERPTR usage be discouraged once we get dma-buf support ?
Why? It's perfectly fine to use it and it's not going away.
I'm not saying that we should support exporting a userptr buffer as a
dmabuf fd, but I'm just wondering if that is possible at all and how
difficult it would be.
It would be difficult. Currently there is no safe/portable way to transform
a userptr into a scatterlist mappable for other devices. The most trouble
some examples are userspace-mapping of IO memory like framebuffers.
How reference management is going to work if there are no struct pages
describing mapped memory?
The VB2 uses a workaround by keeping a copy of vma that is used to call
open/close callbacks. I am not sure how reliable this solution is.
Who knows, maybe in future someone will introduce a mechanism for creation
of DMABUF descriptor from a userptr without any help of client APIs.
In such a case, it will be the part of DMABUF api not V4L2 :).
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
On Wed 1 August 2012 10:01:45 Tomasz Stanislawski wrote:
Hi Hans,
On 07/31/2012 02:11 PM, Hans Verkuil wrote:
On Tue 31 July 2012 13:56:14 Laurent Pinchart wrote:
Hi Hans,
On Tuesday 31 July 2012 08:33:56 Hans Verkuil wrote:
On Thu June 14 2012 16:32:23 Tomasz Stanislawski wrote:
+/**
+ * struct v4l2_exportbuffer - export of video buffer as DMABUF file
descriptor + *
+ * @fd: file descriptor associated with DMABUF (set by driver)
+ * @mem_offset: buffer memory offset as returned by VIDIOC_QUERYBUF in
struct + * v4l2_buffer::m.offset (for single-plane
formats) or
+ * v4l2_plane::m.offset (for multi-planar formats)
+ * @flags: flags for newly created file, currently only O_CLOEXEC
is
+ * supported, refer to manual of open syscall for more
details
+ *
+ * Contains data used for exporting a video buffer as DMABUF file
descriptor. + * The buffer is identified by a 'cookie' returned by
VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to
userspace). All + * reserved fields must be set to zero. The field
reserved0 is expected to + * become a structure 'type' allowing an
alternative layout of the structure + * content. Therefore this field
should not be used for any other extensions. + */
+struct v4l2_exportbuffer {
+__u32 fd;
+__u32 reserved0;
+__u32 mem_offset;
This should be a union identical to the m union in v4l2_plane, together
with
a u32 memory field. That way you can just copy memory and m from
v4l2_plane/buffer and call expbuf. If new memory types are added in the
future, then you don't need to change this struct.
OK, reserved0 could be replace by __u32 memory. Could we have other
dma-buf
export types in the future not corresponding to a memory type ? I don't
see
any use case right now though.
The memory type should be all you need. And the union is also needed since
the
userptr value is unsigned long, thus ensuring that you have 64-bits
available
for pointer types in the future. That's really my main point: the union
should
have the same size as the union in v4l2_buffer/plane, allowing for the same
size pointers or whatever to be added in the future.
I do not see any good point in using v4l2_plane. What would be the meaning
of bytesused, length, data_offset in case of DMABUF exporting?
The field reserved0 was introduced to be replaced by __u32 memory if other
means
of buffer description would be needed. The reserved fields at the end of
the structure could be used for auxiliary parameters other then offset and
flags.
The flags field is expected to be used by all exporting types therefore the
layout could be reorganized to:
struct v4l2_exportbuffers {
__u32 fd;
__u32 flags;
__u32 reserved0[2]; /* place for '__u32 memory' + forcing 64 bit
alignment */
__u32 mem_offset; /* what do you thing about using 64-bit field? */
__u32 reserved1[11];
};
What is your opinion about this idea?
You're missing the point of my argument. How does v4l2_buffer work currently:
you
have a memory field and a union. The memory field determines which field of the
union is to be used. In order to be able to use VIDIOC_EXPBUF you need to be
able to add new memory types in the future. Currently only MMAP makes sense
here,
so all you need is the offset, but in order to be able to support future memory
types you need to make sure that you can extend v4l2_exportbuffers with the
largest possible value that v4l2_buffers can contain in the union, and that's
an unsigned long/pointer. That won't fit in the current proposal since there is
only
a u32.
So I would propose this:
struct v4l2_exportbuffers {
__u32 fd;
__u32 memory;
union {
__u32 mem_offset;
void *reserved; /* ensure that we can handle pointers in the
future */
} m;
__u32 flags;
__u32 reserved1[11];
};
That way an application can just do:
struct v4l2_buffer buf;
struct v4l2_exportbuffers expbuf;
expbuf.memory = buf.memory;
memcpy(expbuf.m, buf.m, sizeof(expbuf.m));
and VIDIOC_EXPBUF will return an error if expbuf.memory != V4L2_MEMORY_MMAP.
I was actually wondering whether it might not be an idea to pass a v4l2_buffer
directly to VIDIOC_EXPBUF. In order to support that you would have to add fd
fields to v4l2_buffer and v4l2_plane and those would be filled in by
VIDIOC_EXPBUF.
For the flags field in exportbuffers you would have to add new V4L2_BUF_FLAG_
flags.
That way you don't need to introduce a new struct and all planes are also
automatically exported. It's just an idea...
For that matter, wouldn't it be useful to support exporting a userptr
buffer
at some point in the future?
Shouldn't USERPTR usage be discouraged once we
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
On Tue, 31 Jul 2012 23:52:35 +0200, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: I want to receive the video buffers in user space for processing. Typically processing is software encoding or conversion. That's what virtually any V4L application does on the desktop... But what prevents you from using MMAP ? As I wrote several times earlier, MMAP uses fixed number of buffers. In some tightly controlled media pipeline with low latency, it might work. But in general, the V4L element in the pipeline does not know how fast the downstream element(s) will consume the buffers. Thus it has to copy from the MMAP buffers into anonymous user memory pending processing. Then any dequeued buffer can be requeued as soon as possible. In theory, it might also be that, even though the latency is known, the number of required buffers exceeds the maximum MMAP buffers count of the V4L device. Either way, user space ends up doing memory copy from MMAP to custom buffers. This problem does not exist with USERBUF - the V4L2 element can simply allocate a new buffer for each dequeued buffer. By the way, this was already discussed a few months ago for the exact same DMABUF patch series... -- Rémi Denis-Courmont Sent from my collocated server -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Hi Hans,
I do not see any good point in using v4l2_plane. What would be the meaning
of bytesused, length, data_offset in case of DMABUF exporting?
The field reserved0 was introduced to be replaced by __u32 memory if other
means
of buffer description would be needed. The reserved fields at the end of
the structure could be used for auxiliary parameters other then offset and
flags.
The flags field is expected to be used by all exporting types therefore the
layout could be reorganized to:
struct v4l2_exportbuffers {
__u32 fd;
__u32 flags;
__u32 reserved0[2]; /* place for '__u32 memory' + forcing 64 bit
alignment */
__u32 mem_offset; /* what do you thing about using 64-bit field? */
__u32 reserved1[11];
};
What is your opinion about this idea?
You're missing the point of my argument. How does v4l2_buffer work currently:
you
have a memory field and a union. The memory field determines which field of
the
union is to be used. In order to be able to use VIDIOC_EXPBUF you need to be
able to add new memory types in the future. Currently only MMAP makes sense
here,
so all you need is the offset, but in order to be able to support future
memory
types you need to make sure that you can extend v4l2_exportbuffers with the
largest possible value that v4l2_buffers can contain in the union, and that's
an unsigned long/pointer. That won't fit in the current proposal since there
is only
a u32.
So I would propose this:
struct v4l2_exportbuffers {
__u32 fd;
__u32 memory;
union {
__u32 mem_offset;
void *reserved; /* ensure that we can handle pointers in the
future */
} m;
__u32 flags;
__u32 reserved1[11];
};
The layout about prevents adding any auxiliary fields other then mem_offset if
expbuf.memory == V4L2_MEMORY_MMAP. This could be fixed by the layout below
(it might be considered ugly by some people):
struct v4l2_exportbuffers {
__u32 fd;
__u32 flags;
__u32 memory;
__u32 reserved;
union {
struct v4l2_exportbyoffset {
__u32 mem_offset;
__u32 reserved[11];
} byoffset;
struct v4l2_exportbyuserptr {
__u64 userptr;
__u32 reserved[10];
} byuserptr;
__u32 reserved[12];
};
};
Notice that the structure above in binary compatible with:
struct v4l2_exportbuffers {
__u32 fd;
__u32 flags;
__u32 reserved0[2];
__u32 mem_offset;
__u32 reserved1[11];
};
That way an application can just do:
struct v4l2_buffer buf;
struct v4l2_exportbuffers expbuf;
expbuf.memory = buf.memory;
memcpy(expbuf.m, buf.m, sizeof(expbuf.m));
and VIDIOC_EXPBUF will return an error if expbuf.memory != V4L2_MEMORY_MMAP.
The other question is if we should use V4L2_MEMORY_MMAP as expbuf.memory.
I think that new enums should be introduced for this purpose. Exporting is
very different from buffer requesting or queuing. One could envision
extension to VIDIOC_EXPBUF for exporting a buffer as entity different then
DMABUF file. In such a case, the fd and flags should go to a separate union.
This argument supports *not* using any v4l2_buffer related structs for
VIDIOC_EXPBUF.
It should use its own structures. Likely, no extra structs are needed at the
moment.
I was actually wondering whether it might not be an idea to pass a v4l2_buffer
directly to VIDIOC_EXPBUF. In order to support that you would have to add fd
fields to v4l2_buffer and v4l2_plane and those would be filled in by
VIDIOC_EXPBUF.
For the flags field in exportbuffers you would have to add new V4L2_BUF_FLAG_
flags.
That way you don't need to introduce a new struct and all planes are also
automatically exported. It's just an idea...
One may not want to create DMABUF descriptors for all the planes.
The number of file descriptors is limited for the process.
Why creating file descriptor if they are going to closed or
(even worse) not used?
The mmap is called on each plane separately. So why VIDIOC_EXPBUF should
behave differently?
Regards,
Tomasz Stanislawski
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Hi Rémi, On Wednesday 01 August 2012 10:37:02 Rémi Denis-Courmont wrote: On Tue, 31 Jul 2012 23:52:35 +0200, Laurent Pinchart wrote: I want to receive the video buffers in user space for processing. Typically processing is software encoding or conversion. That's what virtually any V4L application does on the desktop... But what prevents you from using MMAP ? As I wrote several times earlier, MMAP uses fixed number of buffers. In some tightly controlled media pipeline with low latency, it might work. Sorry about making you repeat. But in general, the V4L element in the pipeline does not know how fast the downstream element(s) will consume the buffers. Thus it has to copy from the MMAP buffers into anonymous user memory pending processing. Then any dequeued buffer can be requeued as soon as possible. In theory, it might also be that, even though the latency is known, the number of required buffers exceeds the maximum MMAP buffers count of the V4L device. Either way, user space ends up doing memory copy from MMAP to custom buffers. This problem does not exist with USERBUF - the V4L2 element can simply allocate a new buffer for each dequeued buffer. What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ? By the way, this was already discussed a few months ago for the exact same DMABUF patch series... -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Le mercredi 1 août 2012 14:35:03 Laurent Pinchart, vous avez écrit : But in general, the V4L element in the pipeline does not know how fast the downstream element(s) will consume the buffers. Thus it has to copy from the MMAP buffers into anonymous user memory pending processing. Then any dequeued buffer can be requeued as soon as possible. In theory, it might also be that, even though the latency is known, the number of required buffers exceeds the maximum MMAP buffers count of the V4L device. Either way, user space ends up doing memory copy from MMAP to custom buffers. This problem does not exist with USERBUF - the V4L2 element can simply allocate a new buffer for each dequeued buffer. What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ? Does CREATE_BUFS always work while already streaming has already started? If it depends on the driver, it's kinda helpless. What's the guaranteed minimum buffer count? It seems in any case, MMAP has a hard limit of 32 buffers (at least videobuf2 has), though one might argue this should be more than enough. -- Rémi Denis-Courmont http://www.remlab.net/ http://fi.linkedin.com/in/remidenis -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
On Thu June 14 2012 16:32:23 Tomasz Stanislawski wrote:
This patch adds extension to V4L2 api. It allow to export a mmap buffer as
file
descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by
mmap and return a file descriptor on success.
Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/video/v4l2-compat-ioctl32.c |1 +
drivers/media/video/v4l2-dev.c|1 +
drivers/media/video/v4l2-ioctl.c |6 ++
include/linux/videodev2.h | 26 ++
include/media/v4l2-ioctl.h|2 ++
5 files changed, 36 insertions(+)
diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
b/drivers/media/video/v4l2-compat-ioctl32.c
index d33ab18..141e745 100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -970,6 +970,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int
cmd, unsigned long arg)
case VIDIOC_S_FBUF32:
case VIDIOC_OVERLAY32:
case VIDIOC_QBUF32:
+ case VIDIOC_EXPBUF:
case VIDIOC_DQBUF32:
case VIDIOC_STREAMON32:
case VIDIOC_STREAMOFF32:
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 5ccbd46..6bf6307 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -597,6 +597,7 @@ static void determine_valid_ioctls(struct video_device
*vdev)
SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
SET_VALID_IOCTL(ops, VIDIOC_QBUF, vidioc_qbuf);
+ SET_VALID_IOCTL(ops, VIDIOC_EXPBUF, vidioc_expbuf);
SET_VALID_IOCTL(ops, VIDIOC_DQBUF, vidioc_dqbuf);
SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
diff --git a/drivers/media/video/v4l2-ioctl.c
b/drivers/media/video/v4l2-ioctl.c
index 31fc2ad..a73b14e 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -212,6 +212,7 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
IOCTL_INFO(VIDIOC_S_FBUF, INFO_FL_PRIO),
IOCTL_INFO(VIDIOC_OVERLAY, INFO_FL_PRIO),
IOCTL_INFO(VIDIOC_QBUF, 0),
+ IOCTL_INFO(VIDIOC_EXPBUF, 0),
IOCTL_INFO(VIDIOC_DQBUF, 0),
IOCTL_INFO(VIDIOC_STREAMON, INFO_FL_PRIO),
IOCTL_INFO(VIDIOC_STREAMOFF, INFO_FL_PRIO),
@@ -957,6 +958,11 @@ static long __video_do_ioctl(struct file *file,
dbgbuf(cmd, vfd, p);
break;
}
+ case VIDIOC_EXPBUF:
+ {
+ ret = ops-vidioc_expbuf(file, fh, arg);
+ break;
+ }
case VIDIOC_DQBUF:
{
struct v4l2_buffer *p = arg;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 51b20f4..e8893a5 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -684,6 +684,31 @@ struct v4l2_buffer {
#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE0x0800
#define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x1000
+/**
+ * struct v4l2_exportbuffer - export of video buffer as DMABUF file
descriptor
+ *
+ * @fd: file descriptor associated with DMABUF (set by driver)
+ * @mem_offset: buffer memory offset as returned by VIDIOC_QUERYBUF in
struct
+ * v4l2_buffer::m.offset (for single-plane formats) or
+ * v4l2_plane::m.offset (for multi-planar formats)
+ * @flags: flags for newly created file, currently only O_CLOEXEC is
+ * supported, refer to manual of open syscall for more details
+ *
+ * Contains data used for exporting a video buffer as DMABUF file descriptor.
+ * The buffer is identified by a 'cookie' returned by VIDIOC_QUERYBUF
+ * (identical to the cookie used to mmap() the buffer to userspace). All
+ * reserved fields must be set to zero. The field reserved0 is expected to
+ * become a structure 'type' allowing an alternative layout of the structure
+ * content. Therefore this field should not be used for any other extensions.
+ */
+struct v4l2_exportbuffer {
+ __u32 fd;
+ __u32 reserved0;
+ __u32 mem_offset;
This should be a union identical to the m union in v4l2_plane, together with a
u32 memory field. That way you can just copy memory and m from v4l2_plane/buffer
and call expbuf. If new memory types are added in the future, then you don't
need
to change this struct.
For that matter, wouldn't it be useful to support exporting a userptr buffer at
some point in the future?
BTW, this patch series needs to be rebased to the latest for_v3.6. Quite a few
core things have changed when it comes to adding new ioctls.
Regards,
Hans
+ __u32 flags;
+ __u32 reserved[12];
+};
+
/*
* O V E R L A Y P R E V I E W
*/
@@ -2553,6
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Hi Hans,
On Tuesday 31 July 2012 08:33:56 Hans Verkuil wrote:
On Thu June 14 2012 16:32:23 Tomasz Stanislawski wrote:
This patch adds extension to V4L2 api. It allow to export a mmap buffer as
file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer
offset used by mmap and return a file descriptor on success.
Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/video/v4l2-compat-ioctl32.c |1 +
drivers/media/video/v4l2-dev.c|1 +
drivers/media/video/v4l2-ioctl.c |6 ++
include/linux/videodev2.h | 26
++
include/media/v4l2-ioctl.h|2 ++
5 files changed, 36 insertions(+)
diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
b/drivers/media/video/v4l2-compat-ioctl32.c index d33ab18..141e745 100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -970,6 +970,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned
int cmd, unsigned long arg)
case VIDIOC_S_FBUF32:
case VIDIOC_OVERLAY32:
case VIDIOC_QBUF32:
+ case VIDIOC_EXPBUF:
case VIDIOC_DQBUF32:
case VIDIOC_STREAMON32:
case VIDIOC_STREAMOFF32:
diff --git a/drivers/media/video/v4l2-dev.c
b/drivers/media/video/v4l2-dev.c index 5ccbd46..6bf6307 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -597,6 +597,7 @@ static void determine_valid_ioctls(struct video_device
*vdev)
SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
SET_VALID_IOCTL(ops, VIDIOC_QBUF, vidioc_qbuf);
+ SET_VALID_IOCTL(ops, VIDIOC_EXPBUF, vidioc_expbuf);
SET_VALID_IOCTL(ops, VIDIOC_DQBUF, vidioc_dqbuf);
SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
diff --git a/drivers/media/video/v4l2-ioctl.c
b/drivers/media/video/v4l2-ioctl.c index 31fc2ad..a73b14e 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -212,6 +212,7 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
IOCTL_INFO(VIDIOC_S_FBUF, INFO_FL_PRIO),
IOCTL_INFO(VIDIOC_OVERLAY, INFO_FL_PRIO),
IOCTL_INFO(VIDIOC_QBUF, 0),
+ IOCTL_INFO(VIDIOC_EXPBUF, 0),
IOCTL_INFO(VIDIOC_DQBUF, 0),
IOCTL_INFO(VIDIOC_STREAMON, INFO_FL_PRIO),
IOCTL_INFO(VIDIOC_STREAMOFF, INFO_FL_PRIO),
@@ -957,6 +958,11 @@ static long __video_do_ioctl(struct file *file,
dbgbuf(cmd, vfd, p);
break;
}
+ case VIDIOC_EXPBUF:
+ {
+ ret = ops-vidioc_expbuf(file, fh, arg);
+ break;
+ }
case VIDIOC_DQBUF:
{
struct v4l2_buffer *p = arg;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 51b20f4..e8893a5 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -684,6 +684,31 @@ struct v4l2_buffer {
#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0800
#define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x1000
+/**
+ * struct v4l2_exportbuffer - export of video buffer as DMABUF file
descriptor + *
+ * @fd:file descriptor associated with DMABUF (set by driver)
+ * @mem_offset:buffer memory offset as returned by VIDIOC_QUERYBUF in
struct + * v4l2_buffer::m.offset (for single-plane formats) or
+ * v4l2_plane::m.offset (for multi-planar formats)
+ * @flags: flags for newly created file, currently only O_CLOEXEC is
+ * supported, refer to manual of open syscall for more details
+ *
+ * Contains data used for exporting a video buffer as DMABUF file
descriptor. + * The buffer is identified by a 'cookie' returned by
VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to
userspace). All + * reserved fields must be set to zero. The field
reserved0 is expected to + * become a structure 'type' allowing an
alternative layout of the structure + * content. Therefore this field
should not be used for any other extensions. + */
+struct v4l2_exportbuffer {
+ __u32 fd;
+ __u32 reserved0;
+ __u32 mem_offset;
This should be a union identical to the m union in v4l2_plane, together with
a u32 memory field. That way you can just copy memory and m from
v4l2_plane/buffer and call expbuf. If new memory types are added in the
future, then you don't need to change this struct.
OK, reserved0 could be replace by __u32 memory. Could we have other dma-buf
export types in the future not corresponding to a memory type ? I don't see
any use case right now though.
For that matter, wouldn't it be useful to support exporting a userptr
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
On Tue 31 July 2012 13:56:14 Laurent Pinchart wrote:
Hi Hans,
On Tuesday 31 July 2012 08:33:56 Hans Verkuil wrote:
On Thu June 14 2012 16:32:23 Tomasz Stanislawski wrote:
+/**
+ * struct v4l2_exportbuffer - export of video buffer as DMABUF file
descriptor + *
+ * @fd: file descriptor associated with DMABUF (set by driver)
+ * @mem_offset: buffer memory offset as returned by VIDIOC_QUERYBUF in
struct + *v4l2_buffer::m.offset (for single-plane
formats) or
+ * v4l2_plane::m.offset (for multi-planar formats)
+ * @flags: flags for newly created file, currently only O_CLOEXEC
is
+ * supported, refer to manual of open syscall for more
details
+ *
+ * Contains data used for exporting a video buffer as DMABUF file
descriptor. + * The buffer is identified by a 'cookie' returned by
VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to
userspace). All + * reserved fields must be set to zero. The field
reserved0 is expected to + * become a structure 'type' allowing an
alternative layout of the structure + * content. Therefore this field
should not be used for any other extensions. + */
+struct v4l2_exportbuffer {
+ __u32 fd;
+ __u32 reserved0;
+ __u32 mem_offset;
This should be a union identical to the m union in v4l2_plane, together with
a u32 memory field. That way you can just copy memory and m from
v4l2_plane/buffer and call expbuf. If new memory types are added in the
future, then you don't need to change this struct.
OK, reserved0 could be replace by __u32 memory. Could we have other dma-buf
export types in the future not corresponding to a memory type ? I don't see
any use case right now though.
The memory type should be all you need. And the union is also needed since the
userptr value is unsigned long, thus ensuring that you have 64-bits available
for pointer types in the future. That's really my main point: the union should
have the same size as the union in v4l2_buffer/plane, allowing for the same
size pointers or whatever to be added in the future.
For that matter, wouldn't it be useful to support exporting a userptr buffer
at some point in the future?
Shouldn't USERPTR usage be discouraged once we get dma-buf support ?
Why? It's perfectly fine to use it and it's not going away.
I'm not saying that we should support exporting a userptr buffer as a dmabuf fd,
but I'm just wondering if that is possible at all and how difficult it would be.
Regards,
Hans
BTW, this patch series needs to be rebased to the latest for_v3.6. Quite a
few core things have changed when it comes to adding new ioctls.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
On Tue, Jul 31, 2012 at 7:11 AM, Hans Verkuil hverk...@xs4all.nl wrote: For that matter, wouldn't it be useful to support exporting a userptr buffer at some point in the future? Shouldn't USERPTR usage be discouraged once we get dma-buf support ? Why? It's perfectly fine to use it and it's not going away. I'm not saying that we should support exporting a userptr buffer as a dmabuf fd, but I'm just wondering if that is possible at all and how difficult it would be. it seems not terribly safe, since you don't really have much control over where the memory comes from w/ userptr. I'm more in favor of discouraging usage of userptr BR, -R -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Le mardi 31 juillet 2012 14:56:14 Laurent Pinchart, vous avez écrit : For that matter, wouldn't it be useful to support exporting a userptr buffer at some point in the future? Shouldn't USERPTR usage be discouraged once we get dma-buf support ? USERPTR, where available, is currently the only way to perform zero-copy from kernel to userspace. READWRITE does not support zero-copy at all. MMAP only supports zero-copy if userspace knows a boundary on the number of concurrent buffers *and* the device can deal with that number of buffers; in general, MMAP requires memory copying. I am not sure DMABUF even supports transmitting data efficiently to userspace. In my understanding, it's meant for transmitting data between DSP's bypassing userspace entirely, in other words the exact opposite of what USERBUF does. -- Rémi Denis-Courmont http://www.remlab.net/ http://fi.linkedin.com/in/remidenis -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
On Tue, Jul 31, 2012 at 8:39 AM, Rémi Denis-Courmont r...@remlab.net wrote: Le mardi 31 juillet 2012 14:56:14 Laurent Pinchart, vous avez écrit : For that matter, wouldn't it be useful to support exporting a userptr buffer at some point in the future? Shouldn't USERPTR usage be discouraged once we get dma-buf support ? USERPTR, where available, is currently the only way to perform zero-copy from kernel to userspace. READWRITE does not support zero-copy at all. MMAP only supports zero-copy if userspace knows a boundary on the number of concurrent buffers *and* the device can deal with that number of buffers; in general, MMAP requires memory copying. hmm, this sounds like the problem is device pre-allocating buffers? Anyways, last time I looked, the vb2 core supported changing dmabuf fd each time you QBUF, in a similar way to what you can do w/ userptr. So that seems to get you the advantages you miss w/ mmap without the pitfalls of userptr. I am not sure DMABUF even supports transmitting data efficiently to userspace. In my understanding, it's meant for transmitting data between DSP's bypassing userspace entirely, in other words the exact opposite of what USERBUF does. well, dmabuf's can be mmap'd.. so it is more a matter of where the buffer gets allocated, malloc() or from some driver (v4l2 or other). There are a *ton* of ways userspace allocated memory can go badly, esp. if the hw has special requirements about memory (GFP_DMA32 in a system w/ PAE/LPAE, certain ranges of memory, certain alignment of memory, etc). BR, -R -- Rémi Denis-Courmont http://www.remlab.net/ http://fi.linkedin.com/in/remidenis -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Le mardi 31 juillet 2012 17:03:52 Rob Clark, vous avez écrit : On Tue, Jul 31, 2012 at 8:39 AM, Rémi Denis-Courmont r...@remlab.net wrote: Le mardi 31 juillet 2012 14:56:14 Laurent Pinchart, vous avez écrit : For that matter, wouldn't it be useful to support exporting a userptr buffer at some point in the future? Shouldn't USERPTR usage be discouraged once we get dma-buf support ? USERPTR, where available, is currently the only way to perform zero-copy from kernel to userspace. READWRITE does not support zero-copy at all. MMAP only supports zero-copy if userspace knows a boundary on the number of concurrent buffers *and* the device can deal with that number of buffers; in general, MMAP requires memory copying. hmm, this sounds like the problem is device pre-allocating buffers? Basically, yes. Anyways, last time I looked, the vb2 core supported changing dmabuf fd each time you QBUF, in a similar way to what you can do w/ userptr. So that seems to get you the advantages you miss w/ mmap without the pitfalls of userptr. It might work albeit with a higher system calls count overhead. But what about libv4l2 transparent format conversion? Emulated USERBUF, with MMAP in the back-end would provide by far the least overhead. I don't see how DMABUF would work there. -- Rémi Denis-Courmont http://www.remlab.net/ http://fi.linkedin.com/in/remidenis -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Hi Rémi, On Tuesday 31 July 2012 16:39:00 Rémi Denis-Courmont wrote: Le mardi 31 juillet 2012 14:56:14 Laurent Pinchart, vous avez écrit : For that matter, wouldn't it be useful to support exporting a userptr buffer at some point in the future? Shouldn't USERPTR usage be discouraged once we get dma-buf support ? USERPTR, where available, is currently the only way to perform zero-copy from kernel to userspace. READWRITE does not support zero-copy at all. MMAP only supports zero-copy if userspace knows a boundary on the number of concurrent buffers *and* the device can deal with that number of buffers; in general, MMAP requires memory copying. Could you please share your use case(s) with us ? I am not sure DMABUF even supports transmitting data efficiently to userspace. In my understanding, it's meant for transmitting data between DSP's bypassing userspace entirely, in other words the exact opposite of what USERBUF does. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Le mardi 31 juillet 2012 19:28:12 Laurent Pinchart, vous avez écrit : Hi Rémi, On Tuesday 31 July 2012 16:39:00 Rémi Denis-Courmont wrote: Le mardi 31 juillet 2012 14:56:14 Laurent Pinchart, vous avez écrit : For that matter, wouldn't it be useful to support exporting a userptr buffer at some point in the future? Shouldn't USERPTR usage be discouraged once we get dma-buf support ? USERPTR, where available, is currently the only way to perform zero-copy from kernel to userspace. READWRITE does not support zero-copy at all. MMAP only supports zero-copy if userspace knows a boundary on the number of concurrent buffers *and* the device can deal with that number of buffers; in general, MMAP requires memory copying. Could you please share your use case(s) with us ? I want to receive the video buffers in user space for processing. Typically processing is software encoding or conversion. That's what virtually any V4L application does on the desktop... -- Rémi Denis-Courmont http://www.remlab.net/ http://fi.linkedin.com/in/remidenis -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf
Hi Rémi, On Tuesday 31 July 2012 21:39:40 Rémi Denis-Courmont wrote: Le mardi 31 juillet 2012 19:28:12 Laurent Pinchart, vous avez écrit : On Tuesday 31 July 2012 16:39:00 Rémi Denis-Courmont wrote: Le mardi 31 juillet 2012 14:56:14 Laurent Pinchart, vous avez écrit : For that matter, wouldn't it be useful to support exporting a userptr buffer at some point in the future? Shouldn't USERPTR usage be discouraged once we get dma-buf support ? USERPTR, where available, is currently the only way to perform zero-copy from kernel to userspace. READWRITE does not support zero-copy at all. MMAP only supports zero-copy if userspace knows a boundary on the number of concurrent buffers *and* the device can deal with that number of buffers; in general, MMAP requires memory copying. Could you please share your use case(s) with us ? I want to receive the video buffers in user space for processing. Typically processing is software encoding or conversion. That's what virtually any V4L application does on the desktop... But what prevents you from using MMAP ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 3/9] v4l: add buffer exporting via dmabuf
This patch adds extension to V4L2 api. It allow to export a mmap buffer as file
descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by
mmap and return a file descriptor on success.
Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/video/v4l2-compat-ioctl32.c |1 +
drivers/media/video/v4l2-dev.c|1 +
drivers/media/video/v4l2-ioctl.c |6 ++
include/linux/videodev2.h | 26 ++
include/media/v4l2-ioctl.h|2 ++
5 files changed, 36 insertions(+)
diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
b/drivers/media/video/v4l2-compat-ioctl32.c
index d33ab18..141e745 100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -970,6 +970,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int
cmd, unsigned long arg)
case VIDIOC_S_FBUF32:
case VIDIOC_OVERLAY32:
case VIDIOC_QBUF32:
+ case VIDIOC_EXPBUF:
case VIDIOC_DQBUF32:
case VIDIOC_STREAMON32:
case VIDIOC_STREAMOFF32:
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 5ccbd46..6bf6307 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -597,6 +597,7 @@ static void determine_valid_ioctls(struct video_device
*vdev)
SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
SET_VALID_IOCTL(ops, VIDIOC_QBUF, vidioc_qbuf);
+ SET_VALID_IOCTL(ops, VIDIOC_EXPBUF, vidioc_expbuf);
SET_VALID_IOCTL(ops, VIDIOC_DQBUF, vidioc_dqbuf);
SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index 31fc2ad..a73b14e 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -212,6 +212,7 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
IOCTL_INFO(VIDIOC_S_FBUF, INFO_FL_PRIO),
IOCTL_INFO(VIDIOC_OVERLAY, INFO_FL_PRIO),
IOCTL_INFO(VIDIOC_QBUF, 0),
+ IOCTL_INFO(VIDIOC_EXPBUF, 0),
IOCTL_INFO(VIDIOC_DQBUF, 0),
IOCTL_INFO(VIDIOC_STREAMON, INFO_FL_PRIO),
IOCTL_INFO(VIDIOC_STREAMOFF, INFO_FL_PRIO),
@@ -957,6 +958,11 @@ static long __video_do_ioctl(struct file *file,
dbgbuf(cmd, vfd, p);
break;
}
+ case VIDIOC_EXPBUF:
+ {
+ ret = ops-vidioc_expbuf(file, fh, arg);
+ break;
+ }
case VIDIOC_DQBUF:
{
struct v4l2_buffer *p = arg;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 51b20f4..e8893a5 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -684,6 +684,31 @@ struct v4l2_buffer {
#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0800
#define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x1000
+/**
+ * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
+ *
+ * @fd:file descriptor associated with DMABUF (set by driver)
+ * @mem_offset:buffer memory offset as returned by VIDIOC_QUERYBUF in
struct
+ * v4l2_buffer::m.offset (for single-plane formats) or
+ * v4l2_plane::m.offset (for multi-planar formats)
+ * @flags: flags for newly created file, currently only O_CLOEXEC is
+ * supported, refer to manual of open syscall for more details
+ *
+ * Contains data used for exporting a video buffer as DMABUF file descriptor.
+ * The buffer is identified by a 'cookie' returned by VIDIOC_QUERYBUF
+ * (identical to the cookie used to mmap() the buffer to userspace). All
+ * reserved fields must be set to zero. The field reserved0 is expected to
+ * become a structure 'type' allowing an alternative layout of the structure
+ * content. Therefore this field should not be used for any other extensions.
+ */
+struct v4l2_exportbuffer {
+ __u32 fd;
+ __u32 reserved0;
+ __u32 mem_offset;
+ __u32 flags;
+ __u32 reserved[12];
+};
+
/*
* O V E R L A Y P R E V I E W
*/
@@ -2553,6 +2578,7 @@ struct v4l2_create_buffers {
#define VIDIOC_S_FBUF _IOW('V', 11, struct v4l2_framebuffer)
#define VIDIOC_OVERLAY _IOW('V', 14, int)
#define VIDIOC_QBUF_IOWR('V', 15, struct v4l2_buffer)
+#define VIDIOC_EXPBUF _IOWR('V', 16, struct v4l2_exportbuffer)
#define VIDIOC_DQBUF _IOWR('V', 17, struct v4l2_buffer)
#define VIDIOC_STREAMON _IOW('V', 18, int)
#define VIDIOC_STREAMOFF_IOW('V', 19, int)
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index d8b76f7..ccd1faa 100644
--- a/include/media/v4l2-ioctl.h
