[libcamera-devel,v3,05/33] v4l2: camera: Handle memory mapping of buffers directly
diff mbox series

Message ID 20200110193808.2266294-6-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 
In the upcoming FrameBuffer API the memory mapping of buffers will be
left to the user of the FrameBuffer objects. Prepare the V4L2
compatibility layer to this upcoming change to ease conversion to the
new API.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/v4l2/v4l2_camera.cpp         | 12 ++++++------
 src/v4l2/v4l2_camera.h           |  4 ++--
 src/v4l2/v4l2_camera_proxy.cpp   | 20 +++++++++++++++-----
 src/v4l2/v4l2_camera_proxy.h     |  2 +-
 src/v4l2/v4l2_compat_manager.cpp |  2 +-
 5 files changed, 25 insertions(+), 15 deletions(-)

Comments

Laurent Pinchart Jan. 10, 2020, 11:27 p.m. UTC | #1 
Hi Niklas,

Thank you for the patch.

On Fri, Jan 10, 2020 at 08:37:40PM +0100, Niklas Söderlund wrote:
> In the upcoming FrameBuffer API the memory mapping of buffers will be
> left to the user of the FrameBuffer objects. Prepare the V4L2
> compatibility layer to this upcoming change to ease conversion to the
> new API.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/v4l2/v4l2_camera.cpp         | 12 ++++++------
>  src/v4l2/v4l2_camera.h           |  4 ++--
>  src/v4l2/v4l2_camera_proxy.cpp   | 20 +++++++++++++++-----
>  src/v4l2/v4l2_camera_proxy.h     |  2 +-
>  src/v4l2/v4l2_compat_manager.cpp |  2 +-
>  5 files changed, 25 insertions(+), 15 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 2a507b9bb8318025..ba3783876101b284 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -121,12 +121,6 @@ int V4L2Camera::configure(StreamConfiguration *streamConfigOut,
>  	return 0;
>  }
>  
> -void *V4L2Camera::mmap(unsigned int index)
> -{
> -	Stream *stream = *camera_->streams().begin();
> -	return stream->buffers()[index].planes()[0].mem();
> -}
> -
>  int V4L2Camera::allocBuffers(unsigned int count)
>  {
>  	int ret = camera_->allocateBuffers();
> @@ -138,6 +132,12 @@ void V4L2Camera::freeBuffers()
>  	camera_->freeBuffers();
>  }
>  
> +int V4L2Camera::getBufferFd(unsigned int index)

How about returning a FileDescriptor instance, especially given that
further in the series you will get a FileDescriptor from FrameBuffer.

> +{
> +	Stream *stream = *camera_->streams().begin();
> +	return stream->buffers()[index].planes()[0].dmabuf();
> +}
> +
>  int V4L2Camera::streamOn()
>  {
>  	if (isRunning_)
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index 81f7908e5e8a6beb..afa20a232235e778 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -53,14 +53,14 @@ public:
>  	void getStreamConfig(StreamConfiguration *streamConfig);
>  	std::vector<V4L2FrameMetadata> completedBuffers();
>  
> -	void *mmap(unsigned int index);
> -
>  	int configure(StreamConfiguration *streamConfigOut,
>  		      const Size &size, PixelFormat pixelformat,
>  		      unsigned int bufferCount);
>  
>  	int allocBuffers(unsigned int count);
>  	void freeBuffers();
> +	int getBufferFd(unsigned int index);
> +
>  	int streamOn();
>  	int streamOff();
>  
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 52f8468cdaa06a7a..3c8eba6b29be4811 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -75,7 +75,8 @@ void V4L2CameraProxy::close()
>  	vcam_->invokeMethod(&V4L2Camera::close, ConnectionTypeBlocking);
>  }
>  
> -void *V4L2CameraProxy::mmap(size_t length, int prot, int flags, off_t offset)
> +void *V4L2CameraProxy::mmap(void *addr, size_t length, int prot, int flags,
> +			    off_t offset)
>  {
>  	LOG(V4L2Compat, Debug) << "Servicing mmap";
>  
> @@ -91,13 +92,22 @@ void *V4L2CameraProxy::mmap(size_t length, int prot, int flags, off_t offset)
>  		return MAP_FAILED;
>  	}
>  
> -	void *val = vcam_->invokeMethod(&V4L2Camera::mmap,
> -					ConnectionTypeBlocking, index);
> +	int fd = vcam_->invokeMethod(&V4L2Camera::getBufferFd,
> +				     ConnectionTypeBlocking, index);
> +	if (fd < 0) {
> +		errno = -fd;
> +		return MAP_FAILED;
> +	}
> +
> +	void *map = V4L2CompatManager::instance()->fops().mmap(addr, length, prot,
> +							       flags, fd, 0);
> +	if (map == MAP_FAILED)
> +		return map;
>  
>  	buffers_[index].flags |= V4L2_BUF_FLAG_MAPPED;
> -	mmaps_[val] = index;
> +	mmaps_[map] = index;
>  
> -	return val;
> +	return map;
>  }
>  
>  int V4L2CameraProxy::munmap(void *addr, size_t length)

We also need to patch munmap() to call fops().munmap(). This should be
fairly trivial as V4L2CompatManager::munmap() doesn't allow partial
unmapping, so we don't need to care about that. You just need to call
fops().munmap() in V4L2CameraProxy::munmap() without real error handling
(but logging an Error message would be useful).

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> index b59d19d707590d88..c8e61adf80f1b93b 100644
> --- a/src/v4l2/v4l2_camera_proxy.h
> +++ b/src/v4l2/v4l2_camera_proxy.h
> @@ -28,7 +28,7 @@ public:
>  	int open(bool nonBlocking);
>  	void dup();
>  	void close();
> -	void *mmap(size_t length, int prot, int flags, off_t offset);
> +	void *mmap(void *addr, size_t length, int prot, int flags, off_t offset);
>  	int munmap(void *addr, size_t length);
>  
>  	int ioctl(unsigned long request, void *arg);
> diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp
> index fe453445a9fb2cdd..f5a7b2ac4229d5d5 100644
> --- a/src/v4l2/v4l2_compat_manager.cpp
> +++ b/src/v4l2/v4l2_compat_manager.cpp
> @@ -223,7 +223,7 @@ void *V4L2CompatManager::mmap(void *addr, size_t length, int prot, int flags,
>  	if (!proxy)
>  		return fops_.mmap(addr, length, prot, flags, fd, offset);
>  
> -	void *map = proxy->mmap(length, prot, flags, offset);
> +	void *map = proxy->mmap(addr, length, prot, flags, offset);
>  	if (map == MAP_FAILED)
>  		return map;
>

Patch
diff mbox series 

diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
index 2a507b9bb8318025..ba3783876101b284 100644
--- a/src/v4l2/v4l2_camera.cpp
+++ b/src/v4l2/v4l2_camera.cpp
@@ -121,12 +121,6 @@  int V4L2Camera::configure(StreamConfiguration *streamConfigOut,
 	return 0;
 }
 
-void *V4L2Camera::mmap(unsigned int index)
-{
-	Stream *stream = *camera_->streams().begin();
-	return stream->buffers()[index].planes()[0].mem();
-}
-
 int V4L2Camera::allocBuffers(unsigned int count)
 {
 	int ret = camera_->allocateBuffers();
@@ -138,6 +132,12 @@  void V4L2Camera::freeBuffers()
 	camera_->freeBuffers();
 }
 
+int V4L2Camera::getBufferFd(unsigned int index)
+{
+	Stream *stream = *camera_->streams().begin();
+	return stream->buffers()[index].planes()[0].dmabuf();
+}
+
 int V4L2Camera::streamOn()
 {
 	if (isRunning_)
diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
index 81f7908e5e8a6beb..afa20a232235e778 100644
--- a/src/v4l2/v4l2_camera.h
+++ b/src/v4l2/v4l2_camera.h
@@ -53,14 +53,14 @@  public:
 	void getStreamConfig(StreamConfiguration *streamConfig);
 	std::vector<V4L2FrameMetadata> completedBuffers();
 
-	void *mmap(unsigned int index);
-
 	int configure(StreamConfiguration *streamConfigOut,
 		      const Size &size, PixelFormat pixelformat,
 		      unsigned int bufferCount);
 
 	int allocBuffers(unsigned int count);
 	void freeBuffers();
+	int getBufferFd(unsigned int index);
+
 	int streamOn();
 	int streamOff();
 
diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
index 52f8468cdaa06a7a..3c8eba6b29be4811 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -75,7 +75,8 @@  void V4L2CameraProxy::close()
 	vcam_->invokeMethod(&V4L2Camera::close, ConnectionTypeBlocking);
 }
 
-void *V4L2CameraProxy::mmap(size_t length, int prot, int flags, off_t offset)
+void *V4L2CameraProxy::mmap(void *addr, size_t length, int prot, int flags,
+			    off_t offset)
 {
 	LOG(V4L2Compat, Debug) << "Servicing mmap";
 
@@ -91,13 +92,22 @@  void *V4L2CameraProxy::mmap(size_t length, int prot, int flags, off_t offset)
 		return MAP_FAILED;
 	}
 
-	void *val = vcam_->invokeMethod(&V4L2Camera::mmap,
-					ConnectionTypeBlocking, index);
+	int fd = vcam_->invokeMethod(&V4L2Camera::getBufferFd,
+				     ConnectionTypeBlocking, index);
+	if (fd < 0) {
+		errno = -fd;
+		return MAP_FAILED;
+	}
+
+	void *map = V4L2CompatManager::instance()->fops().mmap(addr, length, prot,
+							       flags, fd, 0);
+	if (map == MAP_FAILED)
+		return map;
 
 	buffers_[index].flags |= V4L2_BUF_FLAG_MAPPED;
-	mmaps_[val] = index;
+	mmaps_[map] = index;
 
-	return val;
+	return map;
 }
 
 int V4L2CameraProxy::munmap(void *addr, size_t length)
diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
index b59d19d707590d88..c8e61adf80f1b93b 100644
--- a/src/v4l2/v4l2_camera_proxy.h
+++ b/src/v4l2/v4l2_camera_proxy.h
@@ -28,7 +28,7 @@  public:
 	int open(bool nonBlocking);
 	void dup();
 	void close();
-	void *mmap(size_t length, int prot, int flags, off_t offset);
+	void *mmap(void *addr, size_t length, int prot, int flags, off_t offset);
 	int munmap(void *addr, size_t length);
 
 	int ioctl(unsigned long request, void *arg);
diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp
index fe453445a9fb2cdd..f5a7b2ac4229d5d5 100644
--- a/src/v4l2/v4l2_compat_manager.cpp
+++ b/src/v4l2/v4l2_compat_manager.cpp
@@ -223,7 +223,7 @@  void *V4L2CompatManager::mmap(void *addr, size_t length, int prot, int flags,
 	if (!proxy)
 		return fops_.mmap(addr, length, prot, flags, fd, offset);
 
-	void *map = proxy->mmap(length, prot, flags, offset);
+	void *map = proxy->mmap(addr, length, prot, flags, offset);
 	if (map == MAP_FAILED)
 		return map;