[RFC,2/3] camss: Use finer grained locking / disableLinks()
diff mbox series

Message ID 20260408115645.12487-3-johannes.goede@oss.qualcomm.com
State New
Headers show
Series
  • camss: Add CAMSS pipeline handler
Related show

Commit Message

Hans de Goede April 8, 2026, 11:56 a.m. UTC
Use finer grained locking / disableLinks() calls to allow 2 separate
libcamera instances to stream from 2 different cameras at the same time.

Note this does not work when mixing (e.g. from containers) older libcamera
instances still using the simple pipeline handler with libcamera instances
using the new camss pipeline handler. The simple pipeline always picks
csid0 for all cameras. When using a camera on phy1 this sets up a
'msm_csiphy1'[1] -> 'msm_csid0'[0] link which does not get disabled by
the camss pipeline handler when trying to use a phy0 -> csid0 -> vfe0
pipeline, leading to a "Failed to setup link 'msm_csiphy0'[1] ->
'msm_csid0'[0]: Device or resource busy" error.

Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 src/libcamera/pipeline/camss/camss.cpp     | 17 ++++++++++
 src/libcamera/pipeline/camss/camss_csi.cpp | 36 ++++++++++++++--------
 src/libcamera/pipeline/camss/camss_csi.h   |  3 ++
 3 files changed, 44 insertions(+), 12 deletions(-)

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/camss/camss.cpp b/src/libcamera/pipeline/camss/camss.cpp
index 6939ac115..90d7b681e 100644
--- a/src/libcamera/pipeline/camss/camss.cpp
+++ b/src/libcamera/pipeline/camss/camss.cpp
@@ -119,6 +119,8 @@  private:
 		return static_cast<CamssCameraData *>(camera->_d());
 	}
 
+	bool acquireDevice(Camera *camera) override;
+	void releaseDevice(Camera *camera) override;
 	int allocateBuffers(Camera *camera);
 	void freeBuffers(Camera *camera);
 	static int validateConfigMatchesV4L2DeviceFormat(const StreamConfiguration &cfg,
@@ -293,6 +295,7 @@  CameraConfiguration::Status CamssCameraConfiguration::validate()
 PipelineHandlerCamss::PipelineHandlerCamss(CameraManager *manager)
 	: PipelineHandler(manager), csi_()
 {
+	lockOnAcquire_ = false;
 }
 
 std::unique_ptr<CameraConfiguration>
@@ -419,6 +422,20 @@  void PipelineHandlerCamss::freeBuffers(Camera *camera)
 	data->isp_->freeBuffers();
 }
 
+bool PipelineHandlerCamss::acquireDevice(Camera *camera)
+{
+	CamssCameraData *data = cameraData(camera);
+
+	return data->csi_->acquireDevice();
+}
+
+void PipelineHandlerCamss::releaseDevice(Camera *camera)
+{
+	CamssCameraData *data = cameraData(camera);
+
+	data->csi_->releaseDevice();
+}
+
 int PipelineHandlerCamss::start(Camera *camera, [[maybe_unused]] const ControlList *controls)
 {
 	CamssCameraData *data = cameraData(camera);
diff --git a/src/libcamera/pipeline/camss/camss_csi.cpp b/src/libcamera/pipeline/camss/camss_csi.cpp
index b7191a8e9..327196992 100644
--- a/src/libcamera/pipeline/camss/camss_csi.cpp
+++ b/src/libcamera/pipeline/camss/camss_csi.cpp
@@ -268,6 +268,12 @@  int CamssCsiCamera::configure(const StreamConfiguration &cfg, const Transform &t
 	V4L2SubdeviceFormat sensorFormat;
 	int ret;
 
+	for (auto &link : links_) {
+		ret = link.sinkSubdev->entity()->disableLinks();
+		if (ret)
+			return ret;
+	}
+
 	sensorFormat = getSensorFormat(cfg.size, cfg.pixelFormat);
 	/* This updates sensorFormat with the actual established format */
 	ret = sensor_->setFormat(&sensorFormat, transform);
@@ -308,6 +314,24 @@  int CamssCsiCamera::exportBuffers(unsigned int count,
 	return output_->exportBuffers(count, buffers);
 }
 
+bool CamssCsiCamera::acquireDevice()
+{
+	for (auto &link : links_) {
+		if (!link.sinkSubdev->lock()) {
+			releaseDevice();
+			return false;
+		}
+	}
+
+	return true;
+}
+
+void CamssCsiCamera::releaseDevice()
+{
+	for (auto &link : links_)
+		link.sinkSubdev->unlock();
+}
+
 int CamssCsiCamera::start()
 {
 	int ret = output_->exportBuffers(bufferCount_, &buffers_);
@@ -427,18 +451,6 @@  CamssCsi::Cameras CamssCsi::match(PipelineHandler *pipe, DeviceEnumerator *enume
 	if (!camssMediaDev_)
 		return {};
 
-	/*
-	 * Disable all links that are enabled to start with a clean state,
-	 * CamssCsiCamera::configure() enables links as necessary.
-	 * \todo instead only disable links on used entities, to allow
-	 * 2 separate libcamera instances to drive 2 different sensors.
-	 * This will also require changes to PipelineHandler::acquire() to
-	 * allow a more fine grained version of that locking a list of
-	 * subdevs associated with a Camera instead of the mediactl node.
-	 */
-	if (camssMediaDev_->disableLinks())
-		return {};
-
 	getEntities(phys_, "msm_csiphy%d", kMaxCsiPhys);
 	getEntities(csids_, "msm_csid%d", kMaxCsiDecoders);
 	/* Only RDI0 is used for now */
diff --git a/src/libcamera/pipeline/camss/camss_csi.h b/src/libcamera/pipeline/camss/camss_csi.h
index 4b1b0f83b..cc853b889 100644
--- a/src/libcamera/pipeline/camss/camss_csi.h
+++ b/src/libcamera/pipeline/camss/camss_csi.h
@@ -50,6 +50,9 @@  public:
 	PixelFormat mbusCodeToPixelFormat(unsigned int code) const;
 	unsigned int PixelFormatToMbusCode(const PixelFormat &format) const;
 
+	bool acquireDevice();
+	void releaseDevice();
+
 	int start();
 	void stop();