[libcamera-devel,v2,3/3] ipa: ipu3: Calculate frame duration from minimum VBLANK value
diff mbox series

Message ID 20210608074225.59862-4-umang.jain@ideasonboard.com
State Accepted
Headers show
Series
  • IPAIPU3 drive-by improvements
Related show

Commit Message

Umang Jain June 8, 2021, 7:42 a.m. UTC
Frame duration is hard-coded for CTS as per [1]. Ideally, to accurately
calculate the frame duration, it needs the VBLANK value from every
frame's exposure. However, this particular bit is yet to be implemented
in IPAIPU3.

Meanwhile, we can atleast head in the right direction by not hard
coding the value, instead using the default VBLANK value as reported
by the sensor. Update the existing \todo, to use the derived VBLANK
value as and when it's available from each frame exposure.

[1] 6c5f3fe6ced7 ("ipa: ipu3: Set output frame duration metadata")

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Tested-by: Paul Elder <paul.elder@ideasonboard.com>
Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
---
 src/ipa/ipu3/ipu3.cpp | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Comments

Paul Elder June 8, 2021, 7:49 a.m. UTC | #1
Hi Umang,

On Tue, Jun 08, 2021 at 01:12:25PM +0530, Umang Jain wrote:
> Frame duration is hard-coded for CTS as per [1]. Ideally, to accurately
> calculate the frame duration, it needs the VBLANK value from every
> frame's exposure. However, this particular bit is yet to be implemented
> in IPAIPU3.
> 
> Meanwhile, we can atleast head in the right direction by not hard

s/atleast/at least/

> coding the value, instead using the default VBLANK value as reported
> by the sensor. Update the existing \todo, to use the derived VBLANK
> value as and when it's available from each frame exposure.
> 
> [1] 6c5f3fe6ced7 ("ipa: ipu3: Set output frame duration metadata")
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> Tested-by: Paul Elder <paul.elder@ideasonboard.com>
> Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/ipa/ipu3/ipu3.cpp | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 97ddb863..415ea9e5 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -66,6 +66,7 @@ private:
>  	IPACameraSensorInfo sensorInfo_;
>  
>  	/* Camera sensor controls. */
> +	uint32_t defVBlank_;
>  	uint32_t exposure_;
>  	uint32_t minExposure_;
>  	uint32_t maxExposure_;
> @@ -162,6 +163,12 @@ void IPAIPU3::configure(const IPAConfigInfo &configInfo)
>  		return;
>  	}
>  
> +	const auto itVBlank = ctrls_.find(V4L2_CID_VBLANK);
> +	if (itVBlank == ctrls_.end()) {
> +		LOG(IPAIPU3, Error) << "Can't find VBLANK control";
> +		return;
> +	}
> +
>  	minExposure_ = std::max(itExp->second.min().get<int32_t>(), 1);
>  	maxExposure_ = itExp->second.max().get<int32_t>();
>  	exposure_ = minExposure_;
> @@ -170,6 +177,8 @@ void IPAIPU3::configure(const IPAConfigInfo &configInfo)
>  	maxGain_ = itGain->second.max().get<int32_t>();
>  	gain_ = minGain_;
>  
> +	defVBlank_ = itVBlank->second.def().get<int32_t>();
> +
>  	params_ = {};
>  
>  	calculateBdsGrid(configInfo.bdsOutputSize);
> @@ -273,9 +282,10 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>  	if (agcAlgo_->updateControls())
>  		setControls(frame);
>  
> -	/* \todo Populate this with real values */
> -	ctrls.set(controls::FrameDuration,
> -		  static_cast<int64_t>(33334));
> +	/* \todo Use VBlank value calculated from each frame exposure. */
> +	int64_t frameDuration = sensorInfo_.lineLength * (defVBlank_ + sensorInfo_.outputSize.height) /
> +				(sensorInfo_.pixelRate / 1e6);
> +	ctrls.set(controls::FrameDuration, frameDuration);
>  
>  	IPU3Action op;
>  	op.op = ActionMetadataReady;
> -- 
> 2.31.1
>

Patch
diff mbox series

diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index 97ddb863..415ea9e5 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -66,6 +66,7 @@  private:
 	IPACameraSensorInfo sensorInfo_;
 
 	/* Camera sensor controls. */
+	uint32_t defVBlank_;
 	uint32_t exposure_;
 	uint32_t minExposure_;
 	uint32_t maxExposure_;
@@ -162,6 +163,12 @@  void IPAIPU3::configure(const IPAConfigInfo &configInfo)
 		return;
 	}
 
+	const auto itVBlank = ctrls_.find(V4L2_CID_VBLANK);
+	if (itVBlank == ctrls_.end()) {
+		LOG(IPAIPU3, Error) << "Can't find VBLANK control";
+		return;
+	}
+
 	minExposure_ = std::max(itExp->second.min().get<int32_t>(), 1);
 	maxExposure_ = itExp->second.max().get<int32_t>();
 	exposure_ = minExposure_;
@@ -170,6 +177,8 @@  void IPAIPU3::configure(const IPAConfigInfo &configInfo)
 	maxGain_ = itGain->second.max().get<int32_t>();
 	gain_ = minGain_;
 
+	defVBlank_ = itVBlank->second.def().get<int32_t>();
+
 	params_ = {};
 
 	calculateBdsGrid(configInfo.bdsOutputSize);
@@ -273,9 +282,10 @@  void IPAIPU3::parseStatistics(unsigned int frame,
 	if (agcAlgo_->updateControls())
 		setControls(frame);
 
-	/* \todo Populate this with real values */
-	ctrls.set(controls::FrameDuration,
-		  static_cast<int64_t>(33334));
+	/* \todo Use VBlank value calculated from each frame exposure. */
+	int64_t frameDuration = sensorInfo_.lineLength * (defVBlank_ + sensorInfo_.outputSize.height) /
+				(sensorInfo_.pixelRate / 1e6);
+	ctrls.set(controls::FrameDuration, frameDuration);
 
 	IPU3Action op;
 	op.op = ActionMetadataReady;