[libcamera-devel,v6,2/5] ipa: rkisp1: add FrameDurationLimits control
diff mbox series

Message ID 20221030230500.74842-3-nicholas@rothemail.net
State Accepted
Headers show
Series
  • [libcamera-devel,v6,1/5] ipa: workaround libcxx duration limitation
Related show

Commit Message

Nicholas Roth Oct. 30, 2022, 11:04 p.m. UTC 
Currently, the Android HAL does not work on rkisp1-based devices because
required FrameDurationLimits metadata is missing from the IPA
implementation.

This change sets FrameDurationLimits for rkisp1 based on the existing
ipu3 implementation, using the sensor's reported range of vertical
blanking intervals with the minimum reported horizontal blanking
interval.

Signed-off-by: Nicholas Roth <nicholas@rothemail.net>
---
 include/libcamera/ipa/rkisp1.mojom       |  6 ++-
 src/ipa/rkisp1/rkisp1.cpp                | 57 +++++++++++++++++++++---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 +++--
 3 files changed, 64 insertions(+), 11 deletions(-)

Comments

Jacopo Mondi Oct. 31, 2022, 11:42 a.m. UTC | #1 
Hi Nicholas

On Sun, Oct 30, 2022 at 06:04:57PM -0500, Nicholas Roth via libcamera-devel wrote:
> Currently, the Android HAL does not work on rkisp1-based devices because
> required FrameDurationLimits metadata is missing from the IPA
> implementation.
>
> This change sets FrameDurationLimits for rkisp1 based on the existing
> ipu3 implementation, using the sensor's reported range of vertical
> blanking intervals with the minimum reported horizontal blanking
> interval.
>
> Signed-off-by: Nicholas Roth <nicholas@rothemail.net>
> ---
>  include/libcamera/ipa/rkisp1.mojom       |  6 ++-
>  src/ipa/rkisp1/rkisp1.cpp                | 57 +++++++++++++++++++++---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 +++--
>  3 files changed, 64 insertions(+), 11 deletions(-)
>
> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
> index eaf3955e..86ff6d0e 100644
> --- a/include/libcamera/ipa/rkisp1.mojom
> +++ b/include/libcamera/ipa/rkisp1.mojom
> @@ -10,7 +10,9 @@ import "include/libcamera/ipa/core.mojom";
>
>  interface IPARkISP1Interface {
>  	init(libcamera.IPASettings settings,
> -	     uint32 hwRevision)
> +	     uint32 hwRevision,
> +	     libcamera.IPACameraSensorInfo sensorInfo,
> +	     libcamera.ControlInfoMap sensorControls)
>  		=> (int32 ret, libcamera.ControlInfoMap ipaControls);
>  	start() => (int32 ret);
>  	stop();
> @@ -18,7 +20,7 @@ interface IPARkISP1Interface {
>  	configure(libcamera.IPACameraSensorInfo sensorInfo,
>  		  map<uint32, libcamera.IPAStream> streamConfig,
>  		  map<uint32, libcamera.ControlInfoMap> entityControls)
> -		=> (int32 ret);
> +		=> (int32 ret, libcamera.ControlInfoMap ipaControls);
>
>  	mapBuffers(array<libcamera.IPABuffer> buffers);
>  	unmapBuffers(array<uint32> ids);
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 069c901b..49239a87 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -49,13 +49,16 @@ public:
>  	IPARkISP1();
>
>  	int init(const IPASettings &settings, unsigned int hwRevision,
> +		 const IPACameraSensorInfo &sensorInfo,
> +		 const ControlInfoMap &sensorControls,
>  		 ControlInfoMap *ipaControls) override;
>  	int start() override;
>  	void stop() override;
>
>  	int configure(const IPACameraSensorInfo &info,
>  		      const std::map<uint32_t, IPAStream> &streamConfig,
> -		      const std::map<uint32_t, ControlInfoMap> &entityControls) override;
> +		      const std::map<uint32_t, ControlInfoMap> &entityControls,
> +		      ControlInfoMap *ipaControls) override;
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override;
>
> @@ -68,6 +71,9 @@ protected:
>  	std::string logPrefix() const override;
>
>  private:
> +	void updateControls(const IPACameraSensorInfo &sensorInfo,
> +			    const ControlInfoMap &sensorControls,
> +			    ControlInfoMap *ipaControls);
>  	void setControls(unsigned int frame);
>
>  	std::map<unsigned int, FrameBuffer> buffers_;
> @@ -115,6 +121,8 @@ std::string IPARkISP1::logPrefix() const
>  }
>
>  int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
> +		    const IPACameraSensorInfo &sensorInfo,
> +		    const ControlInfoMap &sensorControls,
>  		    ControlInfoMap *ipaControls)
>  {
>  	/* \todo Add support for other revisions */
> @@ -180,9 +188,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
>  	if (ret)
>  		return ret;
>
> -	/* Return the controls handled by the IPA. */
> -	ControlInfoMap::Map ctrlMap = rkisp1Controls;
> -	*ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
> +	/* Initialize controls. */
> +	updateControls(sensorInfo, sensorControls, ipaControls);

Not your fault. The IPU3 IPA does the same here: calls
updateControls() without validating them first in init(), while in
configure it first validates all the required sensor controls are there and
then update the camera controls. RkISP1 equally validates the required
controls are there only in configure().

IPU3 has to be fixed anyhow, but since this code introduces a possible
regression...


>
>  	return 0;
>  }
> @@ -207,7 +214,8 @@ void IPARkISP1::stop()
>   */
>  int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
>  			 [[maybe_unused]] const std::map<uint32_t, IPAStream> &streamConfig,
> -			 const std::map<uint32_t, ControlInfoMap> &entityControls)
> +			 const std::map<uint32_t, ControlInfoMap> &entityControls,
> +			 ControlInfoMap *ipaControls)
>  {
>  	if (entityControls.empty())
>  		return -EINVAL;
> @@ -249,6 +257,9 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
>  	context_.configuration.sensor.size = info.outputSize;
>  	context_.configuration.sensor.lineDuration = info.minLineLength * 1.0s / info.pixelRate;
>
> +	/* Update the camera controls using the new sensor settings. */
> +	updateControls(info, ctrls_, ipaControls);
> +
>  	/*
>  	 * When the AGC computes the new exposure values for a frame, it needs
>  	 * to know the limits for shutter speed and analogue gain.
> @@ -349,6 +360,42 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
>  	metadataReady.emit(frame, metadata);
>  }
>
> +void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo,
> +			       const ControlInfoMap &sensorControls,
> +			       ControlInfoMap *ipaControls)
> +{
> +	ControlInfoMap::Map ctrlMap = rkisp1Controls;
> +
> +	/*
> +	 * Compute the frame duration limits.
> +	 *
> +	 * The frame length is computed assuming a fixed line length combined
> +	 * with the vertical frame sizes.
> +	 */
> +	const ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;
> +	uint32_t hblank = v4l2HBlank.def().get<int32_t>();

... as there you might end up accessing an unexisting control in your
init() call path I would at least copy the sensor controls validation
we have in configure()

	const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);
	if (itExp == ctrls_.end()) {
		LOG(IPARkISP1, Error) << "Can't find exposure control";
		return -EINVAL;
	}

	const auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);
	if (itGain == ctrls_.end()) {
		LOG(IPARkISP1, Error) << "Can't find gain control";
		return -EINVAL;
	}

to init() by making sure HBLANK/VBLANK are there.

To make it more messy, we have this validation in CameraSensor class

	static constexpr uint32_t mandatoryControls[] = {
		V4L2_CID_EXPOSURE,
		V4L2_CID_HBLANK,
		V4L2_CID_PIXEL_RATE,
		V4L2_CID_VBLANK,
	};

The optimal path would be:
1) Decide how much to validate on the pipeline handler side vs IPA
   side. If have to validate on IPA side for $reasons:
   2) Move validateSensorControls() to IPU3 init()
   3) Copy validateSensorControls in RkISP1
   4) Call validateSensorControls() in RkISP1::init()
   5) Drop custom validation code from RkISP1::configure()

As this is more work for you, and we need to finally make a call on
where sensor control validation should happen, I would be fine merging
this as it is or, if you feel particularly motived, with a check
for HBLANK/VBLANK before calling updateControls() in init() as a
safety measure, with a pinky promise from our side we fix this mess on
both platforms as soon as these patches go in. (not something I'm
generally confortable with as accepting technical debt with the
promise that we'll eventually fix it is usually a recipe for disaster,
but as we're keeping a close eye on the series and we want to progress
it as soon as possible, I think we can take the risk here)


> +	uint32_t lineLength = sensorInfo.outputSize.width + hblank;
> +
> +	const ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;
> +	std::array<uint32_t, 3> frameHeights{
> +		v4l2VBlank.min().get<int32_t>() + sensorInfo.outputSize.height,
> +		v4l2VBlank.max().get<int32_t>() + sensorInfo.outputSize.height,
> +		v4l2VBlank.def().get<int32_t>() + sensorInfo.outputSize.height,
> +	};
> +
> +	std::array<int64_t, 3> frameDurations;
> +	for (unsigned int i = 0; i < frameHeights.size(); ++i) {
> +		uint64_t frameSize = lineLength * frameHeights[i];
> +		frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);
> +	}
> +
> +	ctrlMap[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],
> +							      frameDurations[1],
> +							      frameDurations[2]);
> +
> +	*ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
> +}
> +
>  void IPARkISP1::setControls(unsigned int frame)
>  {
>  	/*
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 455ee2a0..dae29a2c 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -340,15 +340,19 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
>  		/*
>  		 * If the tuning file isn't found, fall back to the
>  		 * 'uncalibrated' configuration file.
> -		 */
> +	 */
>  		if (ipaTuningFile.empty())
>  			ipaTuningFile = ipa_->configurationFile("uncalibrated.yaml");
>  	} else {
>  		ipaTuningFile = std::string(configFromEnv);
>  	}
>
> -	int ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,
> -			     &controlInfo_);
> +	IPACameraSensorInfo sensorInfo{};
> +	int ret = sensor_->sensorInfo(&sensorInfo);
> +	if (ret)
> +		return ret;
> +	ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,
> +			 sensorInfo, sensor_->controls(), &controlInfo_);
>  	if (ret < 0) {
>  		LOG(RkISP1, Error) << "IPA initialization failure";
>  		return ret;
> @@ -725,7 +729,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  	std::map<uint32_t, ControlInfoMap> entityControls;
>  	entityControls.emplace(0, data->sensor_->controls());
>
> -	ret = data->ipa_->configure(sensorInfo, streamConfig, entityControls);
> +	ret = data->ipa_->configure(sensorInfo, streamConfig, entityControls, &data->controlInfo_);
>  	if (ret) {
>  		LOG(RkISP1, Error) << "failed configuring IPA (" << ret << ")";
>  		return ret;
> --
> 2.34.1
>
Jacopo Mondi Oct. 31, 2022, 11:43 a.m. UTC | #2 
Ah sorry one more thing

On Sun, Oct 30, 2022 at 06:04:57PM -0500, Nicholas Roth via libcamera-devel wrote:
> Currently, the Android HAL does not work on rkisp1-based devices because
> required FrameDurationLimits metadata is missing from the IPA
> implementation.
>
> This change sets FrameDurationLimits for rkisp1 based on the existing
> ipu3 implementation, using the sensor's reported range of vertical
> blanking intervals with the minimum reported horizontal blanking
> interval.
>
> Signed-off-by: Nicholas Roth <nicholas@rothemail.net>
> ---
>  include/libcamera/ipa/rkisp1.mojom       |  6 ++-
>  src/ipa/rkisp1/rkisp1.cpp                | 57 +++++++++++++++++++++---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 +++--
>  3 files changed, 64 insertions(+), 11 deletions(-)
>
> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
> index eaf3955e..86ff6d0e 100644
> --- a/include/libcamera/ipa/rkisp1.mojom
> +++ b/include/libcamera/ipa/rkisp1.mojom
> @@ -10,7 +10,9 @@ import "include/libcamera/ipa/core.mojom";
>
>  interface IPARkISP1Interface {
>  	init(libcamera.IPASettings settings,
> -	     uint32 hwRevision)
> +	     uint32 hwRevision,
> +	     libcamera.IPACameraSensorInfo sensorInfo,
> +	     libcamera.ControlInfoMap sensorControls)
>  		=> (int32 ret, libcamera.ControlInfoMap ipaControls);
>  	start() => (int32 ret);
>  	stop();
> @@ -18,7 +20,7 @@ interface IPARkISP1Interface {
>  	configure(libcamera.IPACameraSensorInfo sensorInfo,
>  		  map<uint32, libcamera.IPAStream> streamConfig,
>  		  map<uint32, libcamera.ControlInfoMap> entityControls)
> -		=> (int32 ret);
> +		=> (int32 ret, libcamera.ControlInfoMap ipaControls);
>
>  	mapBuffers(array<libcamera.IPABuffer> buffers);
>  	unmapBuffers(array<uint32> ids);
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 069c901b..49239a87 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -49,13 +49,16 @@ public:
>  	IPARkISP1();
>
>  	int init(const IPASettings &settings, unsigned int hwRevision,
> +		 const IPACameraSensorInfo &sensorInfo,
> +		 const ControlInfoMap &sensorControls,
>  		 ControlInfoMap *ipaControls) override;
>  	int start() override;
>  	void stop() override;
>
>  	int configure(const IPACameraSensorInfo &info,
>  		      const std::map<uint32_t, IPAStream> &streamConfig,
> -		      const std::map<uint32_t, ControlInfoMap> &entityControls) override;
> +		      const std::map<uint32_t, ControlInfoMap> &entityControls,
> +		      ControlInfoMap *ipaControls) override;
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override;
>
> @@ -68,6 +71,9 @@ protected:
>  	std::string logPrefix() const override;
>
>  private:
> +	void updateControls(const IPACameraSensorInfo &sensorInfo,
> +			    const ControlInfoMap &sensorControls,
> +			    ControlInfoMap *ipaControls);
>  	void setControls(unsigned int frame);
>
>  	std::map<unsigned int, FrameBuffer> buffers_;
> @@ -115,6 +121,8 @@ std::string IPARkISP1::logPrefix() const
>  }
>
>  int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
> +		    const IPACameraSensorInfo &sensorInfo,
> +		    const ControlInfoMap &sensorControls,
>  		    ControlInfoMap *ipaControls)
>  {
>  	/* \todo Add support for other revisions */
> @@ -180,9 +188,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
>  	if (ret)
>  		return ret;
>
> -	/* Return the controls handled by the IPA. */
> -	ControlInfoMap::Map ctrlMap = rkisp1Controls;
> -	*ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
> +	/* Initialize controls. */
> +	updateControls(sensorInfo, sensorControls, ipaControls);
>
>  	return 0;
>  }
> @@ -207,7 +214,8 @@ void IPARkISP1::stop()
>   */
>  int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
>  			 [[maybe_unused]] const std::map<uint32_t, IPAStream> &streamConfig,
> -			 const std::map<uint32_t, ControlInfoMap> &entityControls)
> +			 const std::map<uint32_t, ControlInfoMap> &entityControls,
> +			 ControlInfoMap *ipaControls)
>  {
>  	if (entityControls.empty())
>  		return -EINVAL;
> @@ -249,6 +257,9 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
>  	context_.configuration.sensor.size = info.outputSize;
>  	context_.configuration.sensor.lineDuration = info.minLineLength * 1.0s / info.pixelRate;
>
> +	/* Update the camera controls using the new sensor settings. */
> +	updateControls(info, ctrls_, ipaControls);
> +
>  	/*
>  	 * When the AGC computes the new exposure values for a frame, it needs
>  	 * to know the limits for shutter speed and analogue gain.
> @@ -349,6 +360,42 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
>  	metadataReady.emit(frame, metadata);
>  }
>
> +void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo,
> +			       const ControlInfoMap &sensorControls,
> +			       ControlInfoMap *ipaControls)
> +{
> +	ControlInfoMap::Map ctrlMap = rkisp1Controls;
> +
> +	/*
> +	 * Compute the frame duration limits.
> +	 *
> +	 * The frame length is computed assuming a fixed line length combined
> +	 * with the vertical frame sizes.
> +	 */
> +	const ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;
> +	uint32_t hblank = v4l2HBlank.def().get<int32_t>();
> +	uint32_t lineLength = sensorInfo.outputSize.width + hblank;
> +
> +	const ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;
> +	std::array<uint32_t, 3> frameHeights{
> +		v4l2VBlank.min().get<int32_t>() + sensorInfo.outputSize.height,
> +		v4l2VBlank.max().get<int32_t>() + sensorInfo.outputSize.height,
> +		v4l2VBlank.def().get<int32_t>() + sensorInfo.outputSize.height,
> +	};
> +
> +	std::array<int64_t, 3> frameDurations;
> +	for (unsigned int i = 0; i < frameHeights.size(); ++i) {
> +		uint64_t frameSize = lineLength * frameHeights[i];
> +		frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);
> +	}
> +
> +	ctrlMap[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],
> +							      frameDurations[1],
> +							      frameDurations[2]);
> +
> +	*ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
> +}
> +
>  void IPARkISP1::setControls(unsigned int frame)
>  {
>  	/*
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 455ee2a0..dae29a2c 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -340,15 +340,19 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
>  		/*
>  		 * If the tuning file isn't found, fall back to the
>  		 * 'uncalibrated' configuration file.
> -		 */
> +	 */

         ^ Unrelated change

>  		if (ipaTuningFile.empty())
>  			ipaTuningFile = ipa_->configurationFile("uncalibrated.yaml");
>  	} else {
>  		ipaTuningFile = std::string(configFromEnv);
>  	}
>
> -	int ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,
> -			     &controlInfo_);
> +	IPACameraSensorInfo sensorInfo{};
> +	int ret = sensor_->sensorInfo(&sensorInfo);
> +	if (ret)
> +		return ret;

Empty line here maybe

Thanks
  j

> +	ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,
> +			 sensorInfo, sensor_->controls(), &controlInfo_);
>  	if (ret < 0) {
>  		LOG(RkISP1, Error) << "IPA initialization failure";
>  		return ret;
> @@ -725,7 +729,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  	std::map<uint32_t, ControlInfoMap> entityControls;
>  	entityControls.emplace(0, data->sensor_->controls());
>
> -	ret = data->ipa_->configure(sensorInfo, streamConfig, entityControls);
> +	ret = data->ipa_->configure(sensorInfo, streamConfig, entityControls, &data->controlInfo_);
>  	if (ret) {
>  		LOG(RkISP1, Error) << "failed configuring IPA (" << ret << ")";
>  		return ret;
> --
> 2.34.1
>

Patch
diff mbox series 

diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
index eaf3955e..86ff6d0e 100644
--- a/include/libcamera/ipa/rkisp1.mojom
+++ b/include/libcamera/ipa/rkisp1.mojom
@@ -10,7 +10,9 @@  import "include/libcamera/ipa/core.mojom";
 
 interface IPARkISP1Interface {
 	init(libcamera.IPASettings settings,
-	     uint32 hwRevision)
+	     uint32 hwRevision,
+	     libcamera.IPACameraSensorInfo sensorInfo,
+	     libcamera.ControlInfoMap sensorControls)
 		=> (int32 ret, libcamera.ControlInfoMap ipaControls);
 	start() => (int32 ret);
 	stop();
@@ -18,7 +20,7 @@  interface IPARkISP1Interface {
 	configure(libcamera.IPACameraSensorInfo sensorInfo,
 		  map<uint32, libcamera.IPAStream> streamConfig,
 		  map<uint32, libcamera.ControlInfoMap> entityControls)
-		=> (int32 ret);
+		=> (int32 ret, libcamera.ControlInfoMap ipaControls);
 
 	mapBuffers(array<libcamera.IPABuffer> buffers);
 	unmapBuffers(array<uint32> ids);
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 069c901b..49239a87 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -49,13 +49,16 @@  public:
 	IPARkISP1();
 
 	int init(const IPASettings &settings, unsigned int hwRevision,
+		 const IPACameraSensorInfo &sensorInfo,
+		 const ControlInfoMap &sensorControls,
 		 ControlInfoMap *ipaControls) override;
 	int start() override;
 	void stop() override;
 
 	int configure(const IPACameraSensorInfo &info,
 		      const std::map<uint32_t, IPAStream> &streamConfig,
-		      const std::map<uint32_t, ControlInfoMap> &entityControls) override;
+		      const std::map<uint32_t, ControlInfoMap> &entityControls,
+		      ControlInfoMap *ipaControls) override;
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
 	void unmapBuffers(const std::vector<unsigned int> &ids) override;
 
@@ -68,6 +71,9 @@  protected:
 	std::string logPrefix() const override;
 
 private:
+	void updateControls(const IPACameraSensorInfo &sensorInfo,
+			    const ControlInfoMap &sensorControls,
+			    ControlInfoMap *ipaControls);
 	void setControls(unsigned int frame);
 
 	std::map<unsigned int, FrameBuffer> buffers_;
@@ -115,6 +121,8 @@  std::string IPARkISP1::logPrefix() const
 }
 
 int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
+		    const IPACameraSensorInfo &sensorInfo,
+		    const ControlInfoMap &sensorControls,
 		    ControlInfoMap *ipaControls)
 {
 	/* \todo Add support for other revisions */
@@ -180,9 +188,8 @@  int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
 	if (ret)
 		return ret;
 
-	/* Return the controls handled by the IPA. */
-	ControlInfoMap::Map ctrlMap = rkisp1Controls;
-	*ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
+	/* Initialize controls. */
+	updateControls(sensorInfo, sensorControls, ipaControls);
 
 	return 0;
 }
@@ -207,7 +214,8 @@  void IPARkISP1::stop()
  */
 int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
 			 [[maybe_unused]] const std::map<uint32_t, IPAStream> &streamConfig,
-			 const std::map<uint32_t, ControlInfoMap> &entityControls)
+			 const std::map<uint32_t, ControlInfoMap> &entityControls,
+			 ControlInfoMap *ipaControls)
 {
 	if (entityControls.empty())
 		return -EINVAL;
@@ -249,6 +257,9 @@  int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
 	context_.configuration.sensor.size = info.outputSize;
 	context_.configuration.sensor.lineDuration = info.minLineLength * 1.0s / info.pixelRate;
 
+	/* Update the camera controls using the new sensor settings. */
+	updateControls(info, ctrls_, ipaControls);
+
 	/*
 	 * When the AGC computes the new exposure values for a frame, it needs
 	 * to know the limits for shutter speed and analogue gain.
@@ -349,6 +360,42 @@  void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
 	metadataReady.emit(frame, metadata);
 }
 
+void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo,
+			       const ControlInfoMap &sensorControls,
+			       ControlInfoMap *ipaControls)
+{
+	ControlInfoMap::Map ctrlMap = rkisp1Controls;
+
+	/*
+	 * Compute the frame duration limits.
+	 *
+	 * The frame length is computed assuming a fixed line length combined
+	 * with the vertical frame sizes.
+	 */
+	const ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;
+	uint32_t hblank = v4l2HBlank.def().get<int32_t>();
+	uint32_t lineLength = sensorInfo.outputSize.width + hblank;
+
+	const ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;
+	std::array<uint32_t, 3> frameHeights{
+		v4l2VBlank.min().get<int32_t>() + sensorInfo.outputSize.height,
+		v4l2VBlank.max().get<int32_t>() + sensorInfo.outputSize.height,
+		v4l2VBlank.def().get<int32_t>() + sensorInfo.outputSize.height,
+	};
+
+	std::array<int64_t, 3> frameDurations;
+	for (unsigned int i = 0; i < frameHeights.size(); ++i) {
+		uint64_t frameSize = lineLength * frameHeights[i];
+		frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);
+	}
+
+	ctrlMap[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],
+							      frameDurations[1],
+							      frameDurations[2]);
+
+	*ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
+}
+
 void IPARkISP1::setControls(unsigned int frame)
 {
 	/*
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 455ee2a0..dae29a2c 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -340,15 +340,19 @@  int RkISP1CameraData::loadIPA(unsigned int hwRevision)
 		/*
 		 * If the tuning file isn't found, fall back to the
 		 * 'uncalibrated' configuration file.
-		 */
+	 */
 		if (ipaTuningFile.empty())
 			ipaTuningFile = ipa_->configurationFile("uncalibrated.yaml");
 	} else {
 		ipaTuningFile = std::string(configFromEnv);
 	}
 
-	int ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,
-			     &controlInfo_);
+	IPACameraSensorInfo sensorInfo{};
+	int ret = sensor_->sensorInfo(&sensorInfo);
+	if (ret)
+		return ret;
+	ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,
+			 sensorInfo, sensor_->controls(), &controlInfo_);
 	if (ret < 0) {
 		LOG(RkISP1, Error) << "IPA initialization failure";
 		return ret;
@@ -725,7 +729,7 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 	std::map<uint32_t, ControlInfoMap> entityControls;
 	entityControls.emplace(0, data->sensor_->controls());
 
-	ret = data->ipa_->configure(sensorInfo, streamConfig, entityControls);
+	ret = data->ipa_->configure(sensorInfo, streamConfig, entityControls, &data->controlInfo_);
 	if (ret) {
 		LOG(RkISP1, Error) << "failed configuring IPA (" << ret << ")";
 		return ret;