[v1,1/8] ipa: rpi: Defer initialising AF LensPosition ControlInfo and value
diff mbox series

Message ID 20250620124452.557855-2-naush@raspberrypi.com
State New
Headers show
Series
  • Raspberry Pi: AF improvements
Related show

Commit Message

Naushir Patuck June 20, 2025, 12:42 p.m. UTC
From: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>

This fixes two small bugs:

We previously populated LensPosition's ControlInfo with hard-coded
values, ignoring the tuning file. Now we query the AfAlgorithm to
get limits (over all AF ranges) and default (for AfRangeNormal).

We previously sent a default position to the lens driver, even when
a user-specified starting position would follow. Defer doing this,
to reduce unnecessary lens movement at startup (for some drivers).

Fixes: https://bugs.libcamera.org/show_bug.cgi?id=258
Signed-off-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
---
 src/ipa/rpi/common/ipa_base.cpp       | 53 +++++++++++++++++----------
 src/ipa/rpi/controller/af_algorithm.h |  8 +++-
 src/ipa/rpi/controller/rpi/af.cpp     | 16 +++++++-
 src/ipa/rpi/controller/rpi/af.h       |  4 +-
 4 files changed, 57 insertions(+), 24 deletions(-)

Patch
diff mbox series

diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
index 6565f5366312..a5bdcbb5838c 100644
--- a/src/ipa/rpi/common/ipa_base.cpp
+++ b/src/ipa/rpi/common/ipa_base.cpp
@@ -233,25 +233,6 @@  int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa
 		agcStatus.analogueGain = defaultAnalogueGain;
 		applyAGC(&agcStatus, ctrls);
 
-		/*
-		 * Set the lens to the default (typically hyperfocal) position
-		 * on first start.
-		 */
-		if (lensPresent_) {
-			RPiController::AfAlgorithm *af =
-				dynamic_cast<RPiController::AfAlgorithm *>(controller_.getAlgorithm("af"));
-
-			if (af) {
-				float defaultPos =
-					ipaAfControls.at(&controls::LensPosition).def().get<float>();
-				ControlList lensCtrl(lensCtrls_);
-				int32_t hwpos;
-
-				af->setLensPosition(defaultPos, &hwpos);
-				lensCtrl.set(V4L2_CID_FOCUS_ABSOLUTE, hwpos);
-				result->lensControls = std::move(lensCtrl);
-			}
-		}
 	}
 
 	result->sensorControls = std::move(ctrls);
@@ -281,8 +262,20 @@  int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa
 		ctrlMap.merge(ControlInfoMap::Map(ipaColourControls));
 
 	/* Declare Autofocus controls, only if we have a controllable lens */
-	if (lensPresent_)
+	if (lensPresent_) {
 		ctrlMap.merge(ControlInfoMap::Map(ipaAfControls));
+		RPiController::AfAlgorithm *af =
+			dynamic_cast<RPiController::AfAlgorithm *>(controller_.getAlgorithm("af"));
+		if (af) {
+			double min, max, dflt;
+			af->getLensLimits(min, max);
+			dflt = af->getDefaultLensPosition();
+			ctrlMap[&controls::LensPosition] =
+				ControlInfo(static_cast<float>(min),
+					    static_cast<float>(max),
+					    static_cast<float>(dflt));
+		}
+	}
 
 	result->controlInfo = ControlInfoMap(std::move(ctrlMap), controls::controls);
 
@@ -320,6 +313,26 @@  void IpaBase::start(const ControlList &controls, StartResult *result)
 	/* Make a note of this as it tells us the HDR status of the first few frames. */
 	hdrStatus_ = agcStatus.hdr;
 
+	/*
+	 * AF: If no lens position was specified, drive lens to a default position.
+	 * This had to be deferred (not initialised by a constructor) until here
+	 * to ensure that exactly ONE starting position is sent to the lens driver.
+	 * It should be the static API default, not dependent on AF range or mode.
+	 */
+	if (firstStart_ && lensPresent_) {
+		RPiController::AfAlgorithm *af = dynamic_cast<RPiController::AfAlgorithm *>(
+			controller_.getAlgorithm("af"));
+		if (af && !af->getLensPosition()) {
+			int32_t hwpos;
+			double pos = af->getDefaultLensPosition();
+			if (af->setLensPosition(pos, &hwpos, true)) {
+				ControlList lensCtrls(lensCtrls_);
+				lensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE, hwpos);
+				setLensControls.emit(lensCtrls);
+			}
+		}
+	}
+
 	/*
 	 * Initialise frame counts, and decide how many frames must be hidden or
 	 * "mistrusted", which depends on whether this is a startup from cold,
diff --git a/src/ipa/rpi/controller/af_algorithm.h b/src/ipa/rpi/controller/af_algorithm.h
index ad9b575450e3..382609f9b2d8 100644
--- a/src/ipa/rpi/controller/af_algorithm.h
+++ b/src/ipa/rpi/controller/af_algorithm.h
@@ -33,6 +33,10 @@  public:
 	 *
 	 * getMode() is provided mainly for validating controls.
 	 * getLensPosition() is provided for populating DeviceStatus.
+	 *
+	 * getDefaultlensPosition() and getLensLimits() were added for
+	 * populating ControlInfoMap. They return the static API limits
+	 * which should be independent of the current range or mode.
 	 */
 
 	enum AfRange { AfRangeNormal = 0,
@@ -66,7 +70,9 @@  public:
 	}
 	virtual void setMode(AfMode mode) = 0;
 	virtual AfMode getMode() const = 0;
-	virtual bool setLensPosition(double dioptres, int32_t *hwpos) = 0;
+	virtual double getDefaultLensPosition() const = 0;
+	virtual void getLensLimits(double &min, double &max) const = 0;
+	virtual bool setLensPosition(double dioptres, int32_t *hwpos, bool force = false) = 0;
 	virtual std::optional<double> getLensPosition() const = 0;
 	virtual void triggerScan() = 0;
 	virtual void cancelScan() = 0;
diff --git a/src/ipa/rpi/controller/rpi/af.cpp b/src/ipa/rpi/controller/rpi/af.cpp
index 2157eb94f427..041cb51db277 100644
--- a/src/ipa/rpi/controller/rpi/af.cpp
+++ b/src/ipa/rpi/controller/rpi/af.cpp
@@ -715,11 +715,23 @@  void Af::setWindows(libcamera::Span<libcamera::Rectangle const> const &wins)
 		invalidateWeights();
 }
 
-bool Af::setLensPosition(double dioptres, int *hwpos)
+double Af::getDefaultLensPosition() const
+{
+	return cfg_.ranges[AfRangeNormal].focusDefault;
+}
+
+void Af::getLensLimits(double &min, double &max) const
+{
+	/* Limits for manual focus are set by map, not by ranges */
+	min = cfg_.map.domain().start;
+	max = cfg_.map.domain().end;
+}
+
+bool Af::setLensPosition(double dioptres, int *hwpos, bool force)
 {
 	bool changed = false;
 
-	if (mode_ == AfModeManual) {
+	if (mode_ == AfModeManual || force) {
 		LOG(RPiAf, Debug) << "setLensPosition: " << dioptres;
 		ftarget_ = cfg_.map.domain().clamp(dioptres);
 		changed = !(initted_ && fsmooth_ == ftarget_);
diff --git a/src/ipa/rpi/controller/rpi/af.h b/src/ipa/rpi/controller/rpi/af.h
index 317a51f3ecc3..d8ece1ab119c 100644
--- a/src/ipa/rpi/controller/rpi/af.h
+++ b/src/ipa/rpi/controller/rpi/af.h
@@ -54,7 +54,9 @@  public:
 	void setWindows(libcamera::Span<libcamera::Rectangle const> const &wins) override;
 	void setMode(AfMode mode) override;
 	AfMode getMode() const override;
-	bool setLensPosition(double dioptres, int32_t *hwpos) override;
+	double getDefaultLensPosition() const override;
+	void getLensLimits(double &min, double &max) const override;
+	bool setLensPosition(double dioptres, int32_t *hwpos, bool force) override;
 	std::optional<double> getLensPosition() const override;
 	void triggerScan() override;
 	void cancelScan() override;