[v3,1/3] pipeline_handler: Add acquireDevice() function to mirror existing releaseDevice()
diff mbox series

Message ID 20240830111207.46455-2-hdegoede@redhat.com
State Superseded
Headers show
Series
  • Fix uvcvideo pipelinehandler keeping /dev/video# open
Related show

Commit Message

Hans de Goede Aug. 30, 2024, 11:12 a.m. UTC 
libcamera always keeps all /dev/video# and /dev/v4l2_subdev# nodes
for a pipeline open after enumerating the camera.

This is a problem for the uvcvideo pipeline handler. Keeping /dev/video#
open stops the UVC USB device from being able to enter runtime-suspend
causing significant unnecessary power-usage.

Add a stub acquireDevice() function to the PipelineHandler class which
pipeline handlers can override.

The uvcvideo pipeline handler will use this to delay opening /dev/video#
until the device is acquired. This is a special case because the kernel
uvcvideo driver powers on the USB device as soon as /dev/video# is opened.
This behavior should *not* be copied by other pipeline handlers.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Harvey Yang <chenghaoyang@chromium.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
- s/method/function/ in commit msg
- Add Reviewed-by-s

Changes in v2:
- Add a note to both the doxygen documentation as well as to the commit
  message that opening/closing /dev/video# from acquire()/release()
  as done by the uvcvideo pipeline handler is an exception and that this
  behavior should not be copied by other pipeline handlers
- Other doxygen doc fixes / improvements
- Only unlock media devices on acquireDevice() failure if useCount_ == 0
---
 include/libcamera/internal/pipeline_handler.h |  3 +-
 src/libcamera/camera.cpp                      |  2 +-
 src/libcamera/pipeline_handler.cpp            | 48 +++++++++++++++----
 3 files changed, 43 insertions(+), 10 deletions(-)

Patch
diff mbox series 

diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
index cad5812f..597f7c3e 100644
--- a/include/libcamera/internal/pipeline_handler.h
+++ b/include/libcamera/internal/pipeline_handler.h
@@ -45,7 +45,7 @@  public:
 	MediaDevice *acquireMediaDevice(DeviceEnumerator *enumerator,
 					const DeviceMatch &dm);
 
-	bool acquire();
+	bool acquire(Camera *camera);
 	void release(Camera *camera);
 
 	virtual std::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,
@@ -79,6 +79,7 @@  protected:
 	virtual int queueRequestDevice(Camera *camera, Request *request) = 0;
 	virtual void stopDevice(Camera *camera) = 0;
 
+	virtual bool acquireDevice(Camera *camera);
 	virtual void releaseDevice(Camera *camera);
 
 	CameraManager *manager_;
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 382a68f7..4e393f89 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -995,7 +995,7 @@  int Camera::acquire()
 	if (ret < 0)
 		return ret == -EACCES ? -EBUSY : ret;
 
-	if (!d->pipe_->acquire()) {
+	if (!d->pipe_->acquire(this)) {
 		LOG(Camera, Info)
 			<< "Pipeline handler in use by another process";
 		return -EBUSY;
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 1fc22d6a..861815cb 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -163,20 +163,24 @@  MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator,
  * has already acquired it
  * \sa release()
  */
-bool PipelineHandler::acquire()
+bool PipelineHandler::acquire(Camera *camera)
 {
 	MutexLocker locker(lock_);
 
-	if (useCount_) {
-		++useCount_;
-		return true;
+	if (useCount_ == 0) {
+		for (std::shared_ptr<MediaDevice> &media : mediaDevices_) {
+			if (!media->lock()) {
+				unlockMediaDevices();
+				return false;
+			}
+		}
 	}
 
-	for (std::shared_ptr<MediaDevice> &media : mediaDevices_) {
-		if (!media->lock()) {
+	if (!acquireDevice(camera)) {
+		if (useCount_ == 0)
 			unlockMediaDevices();
-			return false;
-		}
+
+		return false;
 	}
 
 	++useCount_;
@@ -213,12 +217,40 @@  void PipelineHandler::release(Camera *camera)
 	--useCount_;
 }
 
+/**
+ * \brief Acquire resources associated with this camera
+ * \param[in] camera The camera for which to acquire resources
+ *
+ * Pipeline handlers may override this in order to get resources such as opening
+ * devices and allocating buffers when a camera is acquired.
+ *
+ * This is used by the uvcvideo pipeline handler to delay opening /dev/video#
+ * until the camera is acquired to avoid excess power consumption. The delayed
+ * opening of /dev/video# is a special case because the kernel uvcvideo driver
+ * powers on the USB device as soon as /dev/video# is opened. This behavior
+ * should *not* be copied by other pipeline handlers.
+ *
+ * \return True on success, false on failure
+ * \sa releaseDevice()
+ */
+bool PipelineHandler::acquireDevice([[maybe_unused]] Camera *camera)
+{
+	return true;
+}
+
 /**
  * \brief Release resources associated with this camera
  * \param[in] camera The camera for which to release resources
  *
  * Pipeline handlers may override this in order to perform cleanup operations
  * when a camera is released, such as freeing memory.
+ *
+ * This is called once for every camera that is released. If there are resources
+ * shared by multiple cameras then the pipeline handler must take care to not
+ * release them until releaseDevice() has been called for all previously
+ * acquired cameras.
+ *
+ * \sa acquireDevice()
  */
 void PipelineHandler::releaseDevice([[maybe_unused]] Camera *camera)
 {