[libcamera-devel,v3,14/33] libcamera: v4l2_videodevice: Extract exportDmabufFd()

Message ID 20200110193808.2266294-15-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: Rework buffer API
Related show

Commit Message

Niklas Söderlund Jan. 10, 2020, 7:37 p.m. UTC
The part in createPlane() that exports a dma buffer from a video device
will be used directly by the FrameBuffer interface. Break it out to a
separate function.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
* Changes since v1
- Rename exportDmaBuffer() to exportDmabufFd()
---
 src/libcamera/include/v4l2_videodevice.h |  1 +
 src/libcamera/v4l2_videodevice.cpp       | 41 +++++++++++++++---------
 2 files changed, 27 insertions(+), 15 deletions(-)

Comments

Laurent Pinchart Jan. 11, 2020, 12:17 a.m. UTC | #1
Hi Niklas,

Thank you for the patch.

On Fri, Jan 10, 2020 at 08:37:49PM +0100, Niklas Söderlund wrote:
> The part in createPlane() that exports a dma buffer from a video device
> will be used directly by the FrameBuffer interface. Break it out to a
> separate function.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> * Changes since v1
> - Rename exportDmaBuffer() to exportDmabufFd()
> ---
>  src/libcamera/include/v4l2_videodevice.h |  1 +
>  src/libcamera/v4l2_videodevice.cpp       | 41 +++++++++++++++---------
>  2 files changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index fdf11b3a6ec9b702..1f52fe0120831fa3 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -179,6 +179,7 @@ private:
>  	int requestBuffers(unsigned int count);
>  	int createPlane(BufferMemory *buffer, unsigned int index,
>  			unsigned int plane, unsigned int length);
> +	int exportDmabufFd(unsigned int index, unsigned int plane);

I think you can squash 28/33 with this patch.

>  
>  	Buffer *dequeueBuffer();
>  	void bufferAvailable(EventNotifier *notifier);
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 81d999e354a16bd0..7c9014af4ddea31a 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -902,31 +902,22 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool)
>  int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index,
>  				 unsigned int planeIndex, unsigned int length)
>  {
> -	struct v4l2_exportbuffer expbuf = {};
> -	int ret;
> +	int fd;
>  
>  	LOG(V4L2, Debug)
>  		<< "Buffer " << index
>  		<< " plane " << planeIndex
>  		<< ": length=" << length;
>  
> -	expbuf.type = bufferType_;
> -	expbuf.index = index;
> -	expbuf.plane = planeIndex;
> -	expbuf.flags = O_RDWR;
> -
> -	ret = ioctl(VIDIOC_EXPBUF, &expbuf);
> -	if (ret < 0) {
> -		LOG(V4L2, Error)
> -			<< "Failed to export buffer: " << strerror(-ret);
> -		return ret;
> -	}
> +	fd = exportDmabufFd(index, planeIndex);
> +	if (fd < 0)
> +		return fd;
>  
>  	FrameBuffer::Plane plane;
> -	plane.fd = FileDescriptor(expbuf.fd);
> +	plane.fd = FileDescriptor(fd);
>  	plane.length = length;
>  	buffer->planes().push_back(plane);
> -	::close(expbuf.fd);
> +	::close(fd);
>  
>  	return 0;
>  }
> @@ -952,6 +943,26 @@ int V4L2VideoDevice::importBuffers(BufferPool *pool)
>  	return 0;
>  }
>  
> +int V4L2VideoDevice::exportDmabufFd(unsigned int index, unsigned int plane)
> +{
> +	struct v4l2_exportbuffer expbuf = {};
> +	int ret;
> +
> +	expbuf.type = bufferType_;
> +	expbuf.index = index;
> +	expbuf.plane = plane;
> +	expbuf.flags = O_RDWR;
> +
> +	ret = ioctl(VIDIOC_EXPBUF, &expbuf);
> +	if (ret < 0) {
> +		LOG(V4L2, Error)
> +			<< "Failed to export buffer: " << strerror(-ret);
> +		return ret;
> +	}
> +
> +	return expbuf.fd;
> +}
> +
>  /**
>   * \brief Release all internally allocated buffers
>   */

Patch

diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
index fdf11b3a6ec9b702..1f52fe0120831fa3 100644
--- a/src/libcamera/include/v4l2_videodevice.h
+++ b/src/libcamera/include/v4l2_videodevice.h
@@ -179,6 +179,7 @@  private:
 	int requestBuffers(unsigned int count);
 	int createPlane(BufferMemory *buffer, unsigned int index,
 			unsigned int plane, unsigned int length);
+	int exportDmabufFd(unsigned int index, unsigned int plane);
 
 	Buffer *dequeueBuffer();
 	void bufferAvailable(EventNotifier *notifier);
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index 81d999e354a16bd0..7c9014af4ddea31a 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -902,31 +902,22 @@  int V4L2VideoDevice::exportBuffers(BufferPool *pool)
 int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index,
 				 unsigned int planeIndex, unsigned int length)
 {
-	struct v4l2_exportbuffer expbuf = {};
-	int ret;
+	int fd;
 
 	LOG(V4L2, Debug)
 		<< "Buffer " << index
 		<< " plane " << planeIndex
 		<< ": length=" << length;
 
-	expbuf.type = bufferType_;
-	expbuf.index = index;
-	expbuf.plane = planeIndex;
-	expbuf.flags = O_RDWR;
-
-	ret = ioctl(VIDIOC_EXPBUF, &expbuf);
-	if (ret < 0) {
-		LOG(V4L2, Error)
-			<< "Failed to export buffer: " << strerror(-ret);
-		return ret;
-	}
+	fd = exportDmabufFd(index, planeIndex);
+	if (fd < 0)
+		return fd;
 
 	FrameBuffer::Plane plane;
-	plane.fd = FileDescriptor(expbuf.fd);
+	plane.fd = FileDescriptor(fd);
 	plane.length = length;
 	buffer->planes().push_back(plane);
-	::close(expbuf.fd);
+	::close(fd);
 
 	return 0;
 }
@@ -952,6 +943,26 @@  int V4L2VideoDevice::importBuffers(BufferPool *pool)
 	return 0;
 }
 
+int V4L2VideoDevice::exportDmabufFd(unsigned int index, unsigned int plane)
+{
+	struct v4l2_exportbuffer expbuf = {};
+	int ret;
+
+	expbuf.type = bufferType_;
+	expbuf.index = index;
+	expbuf.plane = plane;
+	expbuf.flags = O_RDWR;
+
+	ret = ioctl(VIDIOC_EXPBUF, &expbuf);
+	if (ret < 0) {
+		LOG(V4L2, Error)
+			<< "Failed to export buffer: " << strerror(-ret);
+		return ret;
+	}
+
+	return expbuf.fd;
+}
+
 /**
  * \brief Release all internally allocated buffers
  */