[v1,10/35] pipeline: rkisp1: Add a frameStart function to handle DelayedControls::applyControls
diff mbox series

Message ID 20251024085130.995967-11-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • rkisp1: pipeline rework for PFC
Related show

Commit Message

Stefan Klug Oct. 24, 2025, 8:50 a.m. UTC
Move the call to applyControls into an intermediate function for
upcoming modfications.

While at it, properly disconnect the signal on failure.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Barnabás Pőcze Oct. 24, 2025, 9:17 a.m. UTC | #1
2025. 10. 24. 10:50 keltezéssel, Stefan Klug írta:
> Move the call to applyControls into an intermediate function for
> upcoming modfications.
> 
> While at it, properly disconnect the signal on failure.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>   src/libcamera/pipeline/rkisp1/rkisp1.cpp | 18 ++++++++++++++++--
>   1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 17250cbd5ae6..9be9f44db479 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1540,6 +1540,7 @@ const uint32_t kDefaultExtParamsBlocks = 0xfffff;
> 
>   int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>   {
> +	utils::ScopeExitActions actions;
>   	int ret;
> 
>   	std::unique_ptr<RkISP1CameraData> data =
> @@ -1570,8 +1571,10 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>   	data->delayedCtrls_ =
>   		std::make_unique<DelayedControls>(data->sensor_->device(),
>   						  params);
> -	isp_->frameStart.connect(data->delayedCtrls_.get(),
> -				 &DelayedControls::applyControls);
> +	isp_->frameStart.connect(this,
> +				 &PipelineHandlerRkISP1::frameStart);

I thought the declaration was missing, but it turns out the function is
declared but not defined. (Not removed in 3809fd77463f8f9f30d8da56fc3b103c683fba72.)

I realize that this is kind of unrelated, but why register the signal
handler in every `createCamera()` invocation? Is it not sufficient
to do that once? Especially now that the handler has been completely
decoupled from `RkISP1CameraData`?


> +
> +	actions += [&]() { isp_->frameStart.disconnect(this); };
> 
>   	uint32_t supportedBlocks = kDefaultExtParamsBlocks;
> 
> @@ -1603,9 +1606,20 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> 
>   	registerCamera(std::move(camera));
> 
> +	actions.release();
> +
>   	return 0;
>   }
> 
> +void PipelineHandlerRkISP1::frameStart(uint32_t sequence)
> +{
> +	if (!activeCamera_)
> +		return;
> +
> +	RkISP1CameraData *data = cameraData(activeCamera_);
> +	data->delayedCtrls_->applyControls(sequence);
> +}
> +
>   bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>   {
>   	DeviceMatch dm("rkisp1");
> --
> 2.48.1
>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 17250cbd5ae6..9be9f44db479 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -1540,6 +1540,7 @@  const uint32_t kDefaultExtParamsBlocks = 0xfffff;
 
 int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
 {
+	utils::ScopeExitActions actions;
 	int ret;
 
 	std::unique_ptr<RkISP1CameraData> data =
@@ -1570,8 +1571,10 @@  int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
 	data->delayedCtrls_ =
 		std::make_unique<DelayedControls>(data->sensor_->device(),
 						  params);
-	isp_->frameStart.connect(data->delayedCtrls_.get(),
-				 &DelayedControls::applyControls);
+	isp_->frameStart.connect(this,
+				 &PipelineHandlerRkISP1::frameStart);
+
+	actions += [&]() { isp_->frameStart.disconnect(this); };
 
 	uint32_t supportedBlocks = kDefaultExtParamsBlocks;
 
@@ -1603,9 +1606,20 @@  int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
 
 	registerCamera(std::move(camera));
 
+	actions.release();
+
 	return 0;
 }
 
+void PipelineHandlerRkISP1::frameStart(uint32_t sequence)
+{
+	if (!activeCamera_)
+		return;
+
+	RkISP1CameraData *data = cameraData(activeCamera_);
+	data->delayedCtrls_->applyControls(sequence);
+}
+
 bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
 {
 	DeviceMatch dm("rkisp1");