From patchwork Wed Jun 24 14:52:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 8401 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 18F75603B8 for ; Wed, 24 Jun 2020 16:53:23 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="QweyTC3f"; dkim-atps=neutral Received: from jade.rasen.tech (unknown [IPv6:2400:4051:61:600:8147:f2a2:a8c6:9087]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 4ED982A8; Wed, 24 Jun 2020 16:53:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1593010402; bh=f9gj0pOEGH1eClO6WWDw87amxovsW33yeoqFhCfrTIE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=QweyTC3f3SQYmfg/TDxU7oG+gbMOVWBlAnbMp/ZCLr/T9QJjP2G9yjwObcw5LvoAB HIOTppF1FoXNQyRgFP38xDYkVUp77o8CxZTs2TjxOPl8LIdf/uvAf3FXpC+5F5jNT2 6cIsFAgyxAqDQ7QIY1JS/erIB1qfkJfHg1zV3//U= From: Paul Elder To: libcamera-devel@lists.libcamera.org Date: Wed, 24 Jun 2020 23:52:37 +0900 Message-Id: <20200624145256.48266-4-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200624145256.48266-1-paul.elder@ideasonboard.com> References: <20200624145256.48266-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 03/22] v4l2: v4l2_compat: Support multiple open X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Jun 2020 14:53:23 -0000 Previously, since we acquired the libcamera camera upon open(), it was impossible to support multiple open, as any subsequent opens would return error because the camera would already be acquired. To fix this, we first initialize the camera in the first call to V4L2CameraProxy::open(), just to heat up the stream format cache. We then add ownership by a V4L2CameraFile of a V4L2Camera via the V4L2CameraProxy. All vidioc ioctls prior to reqbufs > 0 (except for s_fmt) are able to access the camera without ownership. A call to reqbufs > 0 (and s_fmt) will take ownership, and the ownership will be released at reqbufs = 0. While ownership is assigned, the eventfd that should be signaled (and cleared) by V4L2Camera and V4L2CameraProxy is set to the V4L2CameraFile that has ownership, and is cleared when the ownership is released. In case close() is called without a reqbufs = 0 first, the ownership is also released on close(). We also use the V4L2CameraFile to contain all the information specific to an open instance of the file. This removes the need to keep track of such information within V4L2CameraProxy via multiple maps from int fd to info. Since V4L2 does not expect reqbufs 0 to ever return error, make V4L2CameraProxy::freeBuffers() return void. Signed-off-by: Paul Elder Reviewed-by: Laurent Pinchart --- Changes in v4: - return void from V4L2CameraProxy::freeBuffers() - acquire ownership at the end of V4L2CameraProxy::vidioc_reqbufs, instead of at beginning Changes in v3: - split V4L2CompatManager stuff into separate patch - split changing the fd argument to camera file argument in all proxy methods into the separate patch - remove V4L2CameraProxy::efd_ (it's duplicated from V4L2CameraFile::efd()) - rename cf in all the V4L2CameraProxy methods to file - rename V4L2CameraProxy::acquiredCf_ to V4L2CameraProxy::owner_ - remove V4L2CameraProxy::initialized_; just use V4L2CameraProxy::refcount_ - move bufferCount_ > 0 in reqbufs to different patch - move TIMESTAMP_MONOTONIC to different patch - don't take the lock for querybuf - add V4L2CameraProxy::hasOwnership - rename lock() to acquire() and unlock() to release() - only lock on s_fmt and reqbufs > 0, in other ioctls only check for ownership - add explanation about not supporting polling for events from an fd different than the one that owns the device for polling for bufs Changes in v2: - use V4L2CameraFile in V4L2CompatManager to deal with dup, and in V4L2CameraProxy to deal with information specific to the open file (nonblock and priority) - unlock exclusive camera lock on close() --- src/v4l2/v4l2_camera.cpp | 9 ++- src/v4l2/v4l2_camera.h | 1 + src/v4l2/v4l2_camera_proxy.cpp | 109 +++++++++++++++++++++++++++------ src/v4l2/v4l2_camera_proxy.h | 18 +++++- 4 files changed, 113 insertions(+), 24 deletions(-) diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index 9a1ebc8..2557320 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -17,7 +17,8 @@ using namespace libcamera; LOG_DECLARE_CATEGORY(V4L2Compat); V4L2Camera::V4L2Camera(std::shared_ptr camera) - : camera_(camera), isRunning_(false), bufferAllocator_(nullptr) + : camera_(camera), isRunning_(false), bufferAllocator_(nullptr), + efd_(-1) { camera_->requestCompleted.connect(this, &V4L2Camera::requestComplete); } @@ -29,7 +30,6 @@ V4L2Camera::~V4L2Camera() int V4L2Camera::open() { - /* \todo Support multiple open. */ if (camera_->acquire() < 0) { LOG(V4L2Compat, Error) << "Failed to acquire camera"; return -EINVAL; @@ -59,6 +59,11 @@ void V4L2Camera::bind(int efd) efd_ = efd; } +void V4L2Camera::unbind() +{ + efd_ = -1; +} + void V4L2Camera::getStreamConfig(StreamConfiguration *streamConfig) { *streamConfig = config_->at(0); diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h index 33f5eb0..30114ed 100644 --- a/src/v4l2/v4l2_camera.h +++ b/src/v4l2/v4l2_camera.h @@ -40,6 +40,7 @@ public: int open(); void close(); void bind(int efd); + void unbind(); void getStreamConfig(StreamConfiguration *streamConfig); std::vector completedBuffers(); diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index ef3d062..240420c 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -35,7 +35,7 @@ LOG_DECLARE_CATEGORY(V4L2Compat); V4L2CameraProxy::V4L2CameraProxy(unsigned int index, std::shared_ptr camera) : refcount_(0), index_(index), bufferCount_(0), currentBuf_(0), - vcam_(std::make_unique(camera)) + vcam_(std::make_unique(camera)), owner_(nullptr) { querycap(camera); } @@ -44,18 +44,29 @@ int V4L2CameraProxy::open(V4L2CameraFile *file) { LOG(V4L2Compat, Debug) << "Servicing open fd = " << file->efd(); + if (refcount_++) + return 0; + + /* + * We open the camera here, once, and keep it open until the last + * V4L2CameraFile is closed. The proxy is initially not owned by any + * file. The first file that calls reqbufs with count > 0 or s_fmt + * will become the owner, and no other file will be allowed to call + * buffer-related ioctls (except querybuf), set the format, or start or + * stop the stream until ownership is released with a call to reqbufs + * with count = 0. + */ + int ret = vcam_->open(); if (ret < 0) { - errno = -ret; - return -1; + refcount_--; + return ret; } vcam_->getStreamConfig(&streamConfig_); setFmtFromConfig(streamConfig_); sizeimage_ = calculateSizeImage(streamConfig_); - refcount_++; - return 0; } @@ -63,6 +74,7 @@ void V4L2CameraProxy::close(V4L2CameraFile *file) { LOG(V4L2Compat, Debug) << "Servicing close fd = " << file->efd(); + release(file); if (--refcount_ > 0) return; @@ -277,12 +289,16 @@ int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg) if (!validateBufferType(arg->type)) return -EINVAL; + int ret = acquire(file); + if (ret < 0) + return ret; + tryFormat(arg); Size size(arg->fmt.pix.width, arg->fmt.pix.height); - int ret = vcam_->configure(&streamConfig_, size, - v4l2ToDrm(arg->fmt.pix.pixelformat), - bufferCount_); + ret = vcam_->configure(&streamConfig_, size, + v4l2ToDrm(arg->fmt.pix.pixelformat), + bufferCount_); if (ret < 0) return -EINVAL; @@ -309,19 +325,16 @@ int V4L2CameraProxy::vidioc_try_fmt(V4L2CameraFile *file, struct v4l2_format *ar return 0; } -int V4L2CameraProxy::freeBuffers() +void V4L2CameraProxy::freeBuffers() { LOG(V4L2Compat, Debug) << "Freeing libcamera bufs"; int ret = vcam_->streamOff(); - if (ret < 0) { + if (ret < 0) LOG(V4L2Compat, Error) << "Failed to stop stream"; - return ret; - } + vcam_->freeBuffers(); bufferCount_ = 0; - - return 0; } int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuffers *arg) @@ -334,10 +347,17 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf LOG(V4L2Compat, Debug) << arg->count << " buffers requested "; + if (!hasOwnership(file) && owner_) + return -EBUSY; + arg->capabilities = V4L2_BUF_CAP_SUPPORTS_MMAP; - if (arg->count == 0) - return freeBuffers(); + if (arg->count == 0) { + freeBuffers(); + release(file); + + return 0; + } Size size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height); int ret = vcam_->configure(&streamConfig_, size, @@ -386,6 +406,10 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf LOG(V4L2Compat, Debug) << "Allocated " << arg->count << " buffers"; + ret = acquire(file); + if (ret < 0) + return ret; + return 0; } @@ -409,6 +433,9 @@ int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg) LOG(V4L2Compat, Debug) << "Servicing vidioc_qbuf, index = " << arg->index << " fd = " << file->efd(); + if (!hasOwnership(file)) + return -EBUSY; + if (!validateBufferType(arg->type) || !validateMemoryType(arg->memory) || arg->index >= bufferCount_) @@ -428,6 +455,9 @@ int V4L2CameraProxy::vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg) { LOG(V4L2Compat, Debug) << "Servicing vidioc_dqbuf fd = " << file->efd(); + if (!hasOwnership(file)) + return -EBUSY; + if (!validateBufferType(arg->type) || !validateMemoryType(arg->memory)) return -EINVAL; @@ -462,6 +492,9 @@ int V4L2CameraProxy::vidioc_streamon(V4L2CameraFile *file, int *arg) if (!validateBufferType(*arg)) return -EINVAL; + if (!hasOwnership(file)) + return -EBUSY; + currentBuf_ = 0; return vcam_->streamOn(); @@ -474,6 +507,9 @@ int V4L2CameraProxy::vidioc_streamoff(V4L2CameraFile *file, int *arg) if (!validateBufferType(*arg)) return -EINVAL; + if (!hasOwnership(file) && owner_) + return -EBUSY; + int ret = vcam_->streamOff(); for (struct v4l2_buffer &buf : buffers_) @@ -532,10 +568,45 @@ int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *ar return ret; } -void V4L2CameraProxy::bind(int fd) +bool V4L2CameraProxy::hasOwnership(V4L2CameraFile *file) +{ + return owner_ == file; +} + +/** + * \brief Acquire exclusive ownership of the V4L2Camera + * + * \return Zero on success or if already acquired, and negative error on + * failure. + * + * This is sufficient for poll()ing for buffers. Events, however, are signaled + * on the file level, so all fds must be signaled. poll()ing from a different + * fd than the one that locks the device is a corner case, and is currently not + * supported. + */ +int V4L2CameraProxy::acquire(V4L2CameraFile *file) +{ + if (owner_ == file) + return 0; + + if (owner_) + return -EBUSY; + + vcam_->bind(file->efd()); + + owner_ = file; + + return 0; +} + +void V4L2CameraProxy::release(V4L2CameraFile *file) { - efd_ = fd; - vcam_->bind(fd); + if (owner_ != file) + return; + + vcam_->unbind(); + + owner_ = nullptr; } struct PixelFormatPlaneInfo { diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h index b2197ef..36d1dbc 100644 --- a/src/v4l2/v4l2_camera_proxy.h +++ b/src/v4l2/v4l2_camera_proxy.h @@ -33,7 +33,6 @@ public: void *mmap(void *addr, size_t length, int prot, int flags, off64_t offset); int munmap(void *addr, size_t length); - void bind(int fd); int ioctl(V4L2CameraFile *file, unsigned long request, void *arg); private: @@ -44,7 +43,7 @@ private: void querycap(std::shared_ptr camera); void tryFormat(struct v4l2_format *arg); void updateBuffers(); - int freeBuffers(); + void freeBuffers(); int vidioc_querycap(struct v4l2_capability *arg); int vidioc_enum_fmt(V4L2CameraFile *file, struct v4l2_fmtdesc *arg); @@ -58,6 +57,10 @@ private: int vidioc_streamon(V4L2CameraFile *file, int *arg); int vidioc_streamoff(V4L2CameraFile *file, int *arg); + bool hasOwnership(V4L2CameraFile *file); + int acquire(V4L2CameraFile *file); + void release(V4L2CameraFile *file); + static unsigned int bplMultiplier(uint32_t format); static unsigned int imageSize(uint32_t format, unsigned int width, unsigned int height); @@ -80,7 +83,16 @@ private: std::unique_ptr vcam_; - int efd_; + /* + * This is the exclusive owner of this V4L2CameraProxy instance. + * When there is no owner, anybody can call any ioctl before reqbufs. + * The first file to call reqbufs with count > 0 or s_fmt will become + * the owner, and when the owner calls reqbufs with count = 0 it will + * release ownership. Any buffer-related ioctl (except querybuf) or + * s_fmt that is called by a non-owner while there exists an owner + * will return -EBUSY. + */ + V4L2CameraFile *owner_; }; #endif /* __V4L2_CAMERA_PROXY_H__ */