[libcamera-devel] [PATCH v1 1/3] pipeline: ipa: raspberrypi: Change Unicam timeout handling

[Jacopo Mondi]

Hi Naush

On Thu, Feb 23, 2023 at 12:49:55PM +0000, Naushir Patuck via libcamera-devel wrote:
> Date: Thu, 23 Feb 2023 12:49:55 +0000
> From: Naushir Patuck <naush at raspberrypi.com>
> To: libcamera-devel at lists.libcamera.org
> Subject: [PATCH v1 1/3] pipeline: ipa: raspberrypi: Change Unicam timeout
>  handling
> X-Mailer: git-send-email 2.25.1
>
> Add an explicit helper function setCameraTimeout() in the pipeline
> handler to set the Unicam timeout value. This function is signalled from
> the IPA to set up an appropriate timeout. This replaces the
> maxSensorFrameLengthMs value parameter returned back from
> IPARPi::start().
>
> Adjust the timeout to be 5x the maximum frame duration reported by the
> IPA.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks

   j
> ---
>  include/libcamera/ipa/raspberrypi.mojom       |  2 +-
>  src/ipa/raspberrypi/raspberrypi.cpp           |  2 +-
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 24 ++++++++++++-------
>  3 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
> index 8e78f167f179..80e0126618c8 100644
> --- a/include/libcamera/ipa/raspberrypi.mojom
> +++ b/include/libcamera/ipa/raspberrypi.mojom
> @@ -49,7 +49,6 @@ struct IPAConfigResult {
>  struct StartConfig {
>  	libcamera.ControlList controls;
>  	int32 dropFrameCount;
> -	uint32 maxSensorFrameLengthMs;
>  };
>
>  interface IPARPiInterface {
> @@ -132,4 +131,5 @@ interface IPARPiEventInterface {
>  	setIspControls(libcamera.ControlList controls);
>  	setDelayedControls(libcamera.ControlList controls, uint32 delayContext);
>  	setLensControls(libcamera.ControlList controls);
> +	setCameraTimeout(uint32 maxFrameLengthMs);
>  };
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 9b08ae4ca622..f6826bf27fe1 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -341,7 +341,7 @@ void IPARPi::start(const ControlList &controls, StartConfig *startConfig)
>
>  	startConfig->dropFrameCount = dropFrameCount_;
>  	const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.maxLineLength;
> -	startConfig->maxSensorFrameLengthMs = maxSensorFrameDuration.get<std::milli>();
> +	setCameraTimeout.emit(maxSensorFrameDuration.get<std::milli>());
>
>  	firstStart_ = false;
>  	lastRunTimestamp_ = 0;
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 841209548350..3d04842a2440 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -212,6 +212,7 @@ public:
>  	void setIspControls(const ControlList &controls);
>  	void setDelayedControls(const ControlList &controls, uint32_t delayContext);
>  	void setLensControls(const ControlList &controls);
> +	void setCameraTimeout(uint32_t maxExposureTimeMs);
>  	void setSensorControls(ControlList &controls);
>  	void unicamTimeout();
>
> @@ -1166,14 +1167,6 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)
>  		}
>  	}
>
> -	/*
> -	 * Set the dequeue timeout to the larger of 2x the maximum possible
> -	 * frame duration or 1 second.
> -	 */
> -	utils::Duration timeout =
> -		std::max<utils::Duration>(1s, 2 * startConfig.maxSensorFrameLengthMs * 1ms);
> -	data->unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);
> -
>  	return 0;
>  }
>
> @@ -1645,6 +1638,7 @@ int RPiCameraData::loadIPA(ipa::RPi::IPAInitResult *result)
>  	ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);
>  	ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);
>  	ipa_->setLensControls.connect(this, &RPiCameraData::setLensControls);
> +	ipa_->setCameraTimeout.connect(this, &RPiCameraData::setCameraTimeout);
>
>  	/*
>  	 * The configuration (tuning file) is made from the sensor name unless
> @@ -1957,6 +1951,20 @@ void RPiCameraData::setLensControls(const ControlList &controls)
>  	}
>  }
>
> +void RPiCameraData::setCameraTimeout(uint32_t maxFrameLengthMs)
> +{
> +	/*
> +	 * Set the dequeue timeout to the larger of 5x the maximum reported
> +	 * frame length advertised by the IPA over a number of frames. Allow
> +	 * a minimum timeout value of 1s.
> +	 */
> +	utils::Duration timeout =
> +		std::max<utils::Duration>(1s, 5 * maxFrameLengthMs * 1ms);
> +
> +	LOG(RPI, Debug) << "Setting Unicam timeout to " << timeout;
> +	unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout);
> +}
> +
>  void RPiCameraData::setSensorControls(ControlList &controls)
>  {
>  	/*
> --
> 2.25.1
>