[v3,3/3] uvcvideo: Implement acquireDevice() + releaseDevice()
diff mbox series

Message ID 20240830111207.46455-4-hdegoede@redhat.com
State Accepted
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 
The uvcvideo pipeline handler always keeps the uvcvideo /dev/video# node
for a pipeline open after enumerating the camera.

This is a problem for uvcvideo, as keeping the /dev/video# node open stops
the underlying USB device and the USB bus controller from being able to
enter runtime-suspend causing significant unnecessary power-usage.

Implement acquireDevice() + releaseDevice(), openening /dev/video# on
acquire and closing it on release to fix this.

And make validate do a local video_->open() + close() around validate()
when not open yet, to keep validate() working on unacquired cameras.

Bug: https://bugs.libcamera.org/show_bug.cgi?id=168
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
- Put MutexLocker locker(data_->openLock_); in a { } context
  to reduce the time the lock is held to the minimum time necessary

Changes in v2:
- Minor commit msg + code improvements
---
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 54 ++++++++++++++++++--
 1 file changed, 51 insertions(+), 3 deletions(-)

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
index 8a7409fc..a26969e1 100644
--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
@@ -13,6 +13,7 @@ 
 #include <tuple>
 
 #include <libcamera/base/log.h>
+#include <libcamera/base/mutex.h>
 #include <libcamera/base/utils.h>
 
 #include <libcamera/camera.h>
@@ -48,6 +49,7 @@  public:
 
 	const std::string &id() const { return id_; }
 
+	Mutex openLock_;
 	std::unique_ptr<V4L2VideoDevice> video_;
 	Stream stream_;
 	std::map<PixelFormat, std::vector<SizeRange>> formats_;
@@ -93,6 +95,9 @@  private:
 			   const ControlValue &value);
 	int processControls(UVCCameraData *data, Request *request);
 
+	bool acquireDevice(Camera *camera) override;
+	void releaseDevice(Camera *camera) override;
+
 	UVCCameraData *cameraData(Camera *camera)
 	{
 		return static_cast<UVCCameraData *>(camera->_d());
@@ -158,9 +163,29 @@  CameraConfiguration::Status UVCCameraConfiguration::validate()
 	format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
 	format.size = cfg.size;
 
-	int ret = data_->video_->tryFormat(&format);
-	if (ret)
-		return Invalid;
+	/*
+	 * For power-consumption reasons video_ is closed when the camera
+	 * is not acquired. Open it here if necessary.
+	 */
+	{
+		bool opened = false;
+
+		MutexLocker locker(data_->openLock_);
+
+		if (!data_->video_->isOpen()) {
+			int ret = data_->video_->open();
+			if (ret)
+				return Invalid;
+
+			opened = true;
+		}
+
+		int ret = data_->video_->tryFormat(&format);
+		if (opened)
+			data_->video_->close();
+		if (ret)
+			return Invalid;
+	}
 
 	cfg.stride = format.planes[0].bpl;
 	cfg.frameSize = format.planes[0].size;
@@ -411,6 +436,23 @@  bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
 	return true;
 }
 
+bool PipelineHandlerUVC::acquireDevice(Camera *camera)
+{
+	UVCCameraData *data = cameraData(camera);
+
+	MutexLocker locker(data->openLock_);
+
+	return data->video_->open() == 0;
+}
+
+void PipelineHandlerUVC::releaseDevice(Camera *camera)
+{
+	UVCCameraData *data = cameraData(camera);
+
+	MutexLocker locker(data->openLock_);
+	data->video_->close();
+}
+
 int UVCCameraData::init(MediaDevice *media)
 {
 	int ret;
@@ -512,6 +554,12 @@  int UVCCameraData::init(MediaDevice *media)
 
 	controlInfo_ = ControlInfoMap(std::move(ctrls), controls::controls);
 
+	/*
+	 * Close to allow camera to go into runtime-suspend, video_
+	 * will be re-opened from acquireDevice() and validate().
+	 */
+	video_->close();
+
 	return 0;
 }