[v5,2/2] pipeline: imx8-isi: Delay ISI routes config to acquire() time
diff mbox series

Message ID 20251104075609.94310-3-antoine.bouyer@nxp.com
State Superseded
Headers show
Series
  • imx8-isi: Move isi routing into acquireDevice
Related show

Commit Message

Antoine Bouyer Nov. 4, 2025, 7:56 a.m. UTC 
From: Andrei Gansari <andrei.gansari@nxp.com>

Fixes behavior when calling 'cam -l' during a live stream from a camera
in another process.

Issue is that multiple process should be able to list (match procedure)
the camera supported. But only the unique process that lock the media
devices in order to be able to configure then start the pipeline should
setup the routes, graphs, etc.

Thus, the setRouting() is to be moved to a PipelineHandlerISI::acquireDevice()
implementation to override the default Pipeline::acquireDevice() function.

Fixes: 92df79112fb2 ("pipeline: imx8-isi: Add multicamera support")
Signed-off-by: Andrei Gansari <andrei.gansari@nxp.com>
Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 26 +++++++++++++++-----
 1 file changed, 20 insertions(+), 6 deletions(-)

Comments

Barnabás Pőcze Nov. 4, 2025, 11:33 a.m. UTC | #1 
Hi

2025. 11. 04. 8:56 keltezéssel, Antoine Bouyer írta:
> From: Andrei Gansari <andrei.gansari@nxp.com>
> 
> Fixes behavior when calling 'cam -l' during a live stream from a camera
> in another process.
> 
> Issue is that multiple process should be able to list (match procedure)
> the camera supported. But only the unique process that lock the media
> devices in order to be able to configure then start the pipeline should
> setup the routes, graphs, etc.
> 
> Thus, the setRouting() is to be moved to a PipelineHandlerISI::acquireDevice()
> implementation to override the default Pipeline::acquireDevice() function.
> 
> Fixes: 92df79112fb2 ("pipeline: imx8-isi: Add multicamera support")
> Signed-off-by: Andrei Gansari <andrei.gansari@nxp.com>
> Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>   src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 26 +++++++++++++++-----
>   1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> index de09431cb9b9..ce8d0b35a2a7 100644
> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> @@ -71,6 +71,8 @@ public:
>   
>   	unsigned int xbarSink_ = 0;
>   	unsigned int xbarSourceOffset_ = 0;
> +
> +	const std::string &cameraName() const { return sensor_->entity()->name(); }

Not needed anymore. But this can be removed while applying.

Otherwise looks good to me.

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


>   };
>   
>   class ISICameraConfiguration : public CameraConfiguration
> @@ -117,6 +119,8 @@ protected:
>   
>   	int queueRequestDevice(Camera *camera, Request *request) override;
>   
> +	bool acquireDevice(Camera *camera) override;
> +
>   private:
>   	static constexpr Size kPreviewSize = { 1920, 1080 };
>   	static constexpr Size kMinISISize = { 1, 1 };
> @@ -143,6 +147,8 @@ private:
>   
>   	std::unique_ptr<V4L2Subdevice> crossbar_;
>   	std::vector<Pipe> pipes_;
> +
> +	V4L2Subdevice::Routing routing_ = {};
>   };
>   
>   /* -----------------------------------------------------------------------------
> @@ -950,6 +956,19 @@ int PipelineHandlerISI::queueRequestDevice(Camera *camera, Request *request)
>   	return 0;
>   }
>   
> +bool PipelineHandlerISI::acquireDevice([[maybe_unused]] Camera *camera)
> +{
> +	if (useCount() > 0)
> +		return true;
> +
> +	/* Enable routing for all available sensors once */
> +	int ret = crossbar_->setRouting(&routing_, V4L2Subdevice::ActiveFormat);
> +	if (ret)
> +		return false;
> +
> +	return true;
> +}
> +
>   bool PipelineHandlerISI::match(DeviceEnumerator *enumerator)
>   {
>   	DeviceMatch dm("mxc-isi");
> @@ -1034,7 +1053,6 @@ bool PipelineHandlerISI::match(DeviceEnumerator *enumerator)
>   	unsigned int numSinks = 0;
>   	const unsigned int xbarFirstSource = crossbar_->entity()->pads().size() - pipes_.size();
>   	const unsigned int maxStreams = pipes_.size() / cameraCount;
> -	V4L2Subdevice::Routing routing = {};
>   
>   	for (MediaPad *pad : crossbar_->entity()->pads()) {
>   		unsigned int sink = numSinks;
> @@ -1104,7 +1122,7 @@ bool PipelineHandlerISI::match(DeviceEnumerator *enumerator)
>   		/*  Add routes to the crossbar switch routing table. */
>   		for (unsigned i = 0; i < data->streams_.size(); i++) {
>   			unsigned int sourcePad = xbarFirstSource + data->xbarSourceOffset_ + i;
> -			routing.emplace_back(V4L2Subdevice::Stream{ data->xbarSink_, 0 },
> +			routing_.emplace_back(V4L2Subdevice::Stream{ data->xbarSink_, 0 },
>   					     V4L2Subdevice::Stream{ sourcePad, 0 },
>   					     V4L2_SUBDEV_ROUTE_FL_ACTIVE);
>   		}
> @@ -1116,10 +1134,6 @@ bool PipelineHandlerISI::match(DeviceEnumerator *enumerator)
>   		numCameras++;
>   	}
>   
> -	ret = crossbar_->setRouting(&routing, V4L2Subdevice::ActiveFormat);
> -	if (ret)
> -		return false;
> -
>   	return numCameras > 0;
>   }
>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
index de09431cb9b9..ce8d0b35a2a7 100644
--- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
+++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
@@ -71,6 +71,8 @@  public:
 
 	unsigned int xbarSink_ = 0;
 	unsigned int xbarSourceOffset_ = 0;
+
+	const std::string &cameraName() const { return sensor_->entity()->name(); }
 };
 
 class ISICameraConfiguration : public CameraConfiguration
@@ -117,6 +119,8 @@  protected:
 
 	int queueRequestDevice(Camera *camera, Request *request) override;
 
+	bool acquireDevice(Camera *camera) override;
+
 private:
 	static constexpr Size kPreviewSize = { 1920, 1080 };
 	static constexpr Size kMinISISize = { 1, 1 };
@@ -143,6 +147,8 @@  private:
 
 	std::unique_ptr<V4L2Subdevice> crossbar_;
 	std::vector<Pipe> pipes_;
+
+	V4L2Subdevice::Routing routing_ = {};
 };
 
 /* -----------------------------------------------------------------------------
@@ -950,6 +956,19 @@  int PipelineHandlerISI::queueRequestDevice(Camera *camera, Request *request)
 	return 0;
 }
 
+bool PipelineHandlerISI::acquireDevice([[maybe_unused]] Camera *camera)
+{
+	if (useCount() > 0)
+		return true;
+
+	/* Enable routing for all available sensors once */
+	int ret = crossbar_->setRouting(&routing_, V4L2Subdevice::ActiveFormat);
+	if (ret)
+		return false;
+
+	return true;
+}
+
 bool PipelineHandlerISI::match(DeviceEnumerator *enumerator)
 {
 	DeviceMatch dm("mxc-isi");
@@ -1034,7 +1053,6 @@  bool PipelineHandlerISI::match(DeviceEnumerator *enumerator)
 	unsigned int numSinks = 0;
 	const unsigned int xbarFirstSource = crossbar_->entity()->pads().size() - pipes_.size();
 	const unsigned int maxStreams = pipes_.size() / cameraCount;
-	V4L2Subdevice::Routing routing = {};
 
 	for (MediaPad *pad : crossbar_->entity()->pads()) {
 		unsigned int sink = numSinks;
@@ -1104,7 +1122,7 @@  bool PipelineHandlerISI::match(DeviceEnumerator *enumerator)
 		/*  Add routes to the crossbar switch routing table. */
 		for (unsigned i = 0; i < data->streams_.size(); i++) {
 			unsigned int sourcePad = xbarFirstSource + data->xbarSourceOffset_ + i;
-			routing.emplace_back(V4L2Subdevice::Stream{ data->xbarSink_, 0 },
+			routing_.emplace_back(V4L2Subdevice::Stream{ data->xbarSink_, 0 },
 					     V4L2Subdevice::Stream{ sourcePad, 0 },
 					     V4L2_SUBDEV_ROUTE_FL_ACTIVE);
 		}
@@ -1116,10 +1134,6 @@  bool PipelineHandlerISI::match(DeviceEnumerator *enumerator)
 		numCameras++;
 	}
 
-	ret = crossbar_->setRouting(&routing, V4L2Subdevice::ActiveFormat);
-	if (ret)
-		return false;
-
 	return numCameras > 0;
 }