Message ID | 20200112010212.2609025-6-niklas.soderlund@ragnatech.se |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
Hi Niklas, Thank you for the patch. On Sun, Jan 12, 2020 at 02:01:45AM +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> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > * Changes since v3 > - Return FileDescriptor from getBufferFd() > - Call fops().munmap() in V4L2CameraProxy::munmap() > --- > src/v4l2/v4l2_camera.cpp | 12 ++++++------ > src/v4l2/v4l2_camera.h | 5 +++-- > src/v4l2/v4l2_camera_proxy.cpp | 24 +++++++++++++++++++----- > src/v4l2/v4l2_camera_proxy.h | 2 +- > src/v4l2/v4l2_compat_manager.cpp | 2 +- > 5 files changed, 30 insertions(+), 15 deletions(-) > > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp > index 2a507b9bb8318025..07f05a31261c1b25 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(); > } > > +FileDescriptor V4L2Camera::getBufferFd(unsigned int index) > +{ > + Stream *stream = *camera_->streams().begin(); > + return FileDescriptor(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..f760316c854fba2f 100644 > --- a/src/v4l2/v4l2_camera.h > +++ b/src/v4l2/v4l2_camera.h > @@ -14,6 +14,7 @@ > > #include <libcamera/buffer.h> > #include <libcamera/camera.h> > +#include <libcamera/file_descriptor.h> > > #include "semaphore.h" > > @@ -53,14 +54,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(); > + FileDescriptor 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..b4a9e2107c0f9f28 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); > + FileDescriptor fd = vcam_->invokeMethod(&V4L2Camera::getBufferFd, > + ConnectionTypeBlocking, index); > + if (!fd.isValid()) { > + errno = EINVAL; > + return MAP_FAILED; > + } > + > + void *map = V4L2CompatManager::instance()->fops().mmap(addr, length, prot, > + flags, fd.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) > @@ -110,6 +120,10 @@ int V4L2CameraProxy::munmap(void *addr, size_t length) > return -1; > } > > + if (V4L2CompatManager::instance()->fops().munmap(addr, length)) > + LOG(V4L2Compat, Error) << "Unmapping " << addr > + << " with length " << length; How about explicitly stating that this is a failure ? LOG(V4L2Compat, Error) << "Failed to unmap " << addr << " with length " << length; Apart from that, you can keep my R-b. > + > buffers_[iter->second].flags &= ~V4L2_BUF_FLAG_MAPPED; > mmaps_.erase(iter); > > 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; >
diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index 2a507b9bb8318025..07f05a31261c1b25 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(); } +FileDescriptor V4L2Camera::getBufferFd(unsigned int index) +{ + Stream *stream = *camera_->streams().begin(); + return FileDescriptor(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..f760316c854fba2f 100644 --- a/src/v4l2/v4l2_camera.h +++ b/src/v4l2/v4l2_camera.h @@ -14,6 +14,7 @@ #include <libcamera/buffer.h> #include <libcamera/camera.h> +#include <libcamera/file_descriptor.h> #include "semaphore.h" @@ -53,14 +54,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(); + FileDescriptor 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..b4a9e2107c0f9f28 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); + FileDescriptor fd = vcam_->invokeMethod(&V4L2Camera::getBufferFd, + ConnectionTypeBlocking, index); + if (!fd.isValid()) { + errno = EINVAL; + return MAP_FAILED; + } + + void *map = V4L2CompatManager::instance()->fops().mmap(addr, length, prot, + flags, fd.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) @@ -110,6 +120,10 @@ int V4L2CameraProxy::munmap(void *addr, size_t length) return -1; } + if (V4L2CompatManager::instance()->fops().munmap(addr, length)) + LOG(V4L2Compat, Error) << "Unmapping " << addr + << " with length " << length; + buffers_[iter->second].flags &= ~V4L2_BUF_FLAG_MAPPED; mmaps_.erase(iter); 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;