[v1] libcamera: sensor: Disable on-sensor auto-exposure where avilable
diff mbox series

Message ID 20260630082309.804169-1-naush@raspberrypi.com
State New
Headers show
Series
  • [v1] libcamera: sensor: Disable on-sensor auto-exposure where avilable
Related show

Commit Message

Naushir Patuck June 30, 2026, 8:23 a.m. UTC 
Some sensor device drivers (e.g, VD66GY and VD56G3) have sensor auto-
exposure enabled by default. This interferes with the AGC algorithm
running in the IPA.

By default, disable the sensor auto-exposure on startup so that the IPA
can control the sensor shutter time and gain.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 src/libcamera/sensor/camera_sensor_legacy.cpp | 14 ++++++++++++++
 src/libcamera/sensor/camera_sensor_raw.cpp    | 14 ++++++++++++++
 2 files changed, 28 insertions(+)

Comments

Dave Stevenson June 30, 2026, 10:40 a.m. UTC | #1 
Hi Naush

On Tue, 30 Jun 2026 at 09:23, Naushir Patuck <naush@raspberrypi.com> wrote:
>
> Some sensor device drivers (e.g, VD66GY and VD56G3) have sensor auto-
> exposure enabled by default. This interferes with the AGC algorithm
> running in the IPA.
>
> By default, disable the sensor auto-exposure on startup so that the IPA
> can control the sensor shutter time and gain.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>

Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

Works perfectly with vd65g4 through camera_sensor_legacy, so should
work with all the ST sensors. I haven't got a setup using
camera_sensor_raw at present to test that path, but the code is
identical to the legacy one.

Thanks
  Dave

> ---
>  src/libcamera/sensor/camera_sensor_legacy.cpp | 14 ++++++++++++++
>  src/libcamera/sensor/camera_sensor_raw.cpp    | 14 ++++++++++++++
>  2 files changed, 28 insertions(+)
>
> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp
> index 6a683821f219..d86624dc4cc7 100644
> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp
> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp
> @@ -295,6 +295,20 @@ int CameraSensorLegacy::init()
>                         return ret;
>         }
>
> +       /*
> +        * Ensure auto-exposure is disabled in the sensor as the IPAs ought to
> +        * handle that.
> +        */
> +       const struct v4l2_query_ext_ctrl *exposureAuto = subdev_->controlInfo(V4L2_CID_EXPOSURE_AUTO);
> +       if (exposureAuto && !(exposureAuto->flags & V4L2_CTRL_FLAG_READ_ONLY)) {
> +               ControlList ctrl(subdev_->controls());
> +
> +               ctrl.set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_MANUAL);
> +               ret = subdev_->setControls(&ctrl);
> +               if (ret)
> +                       return ret;
> +       }
> +
>         return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
>  }
>
> diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp
> index 10eba0331fe8..1c5ee80281ad 100644
> --- a/src/libcamera/sensor/camera_sensor_raw.cpp
> +++ b/src/libcamera/sensor/camera_sensor_raw.cpp
> @@ -551,6 +551,20 @@ std::optional<int> CameraSensorRaw::init()
>                         return ret;
>         }
>
> +       /*
> +        * Ensure auto-exposure is disabled in the sensor as the IPAs ought to
> +        * handle that.
> +        */
> +       const struct v4l2_query_ext_ctrl *exposureAuto = subdev_->controlInfo(V4L2_CID_EXPOSURE_AUTO);
> +       if (exposureAuto && !(exposureAuto->flags & V4L2_CTRL_FLAG_READ_ONLY)) {
> +               ControlList ctrl(subdev_->controls());
> +
> +               ctrl.set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_MANUAL);
> +               ret = subdev_->setControls(&ctrl);
> +               if (ret)
> +                       return ret;
> +       }
> +
>         ret = applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
>         if (ret)
>                 return { ret };
> --
> 2.53.0
>
Benjamin Mugnier June 30, 2026, 12:07 p.m. UTC | #2 
Hi Naush,

Well that was fast.

Le 30/06/2026 à 12:40, Dave Stevenson a écrit :
> Hi Naush
> 
> On Tue, 30 Jun 2026 at 09:23, Naushir Patuck <naush@raspberrypi.com> wrote:
>>
>> Some sensor device drivers (e.g, VD66GY and VD56G3) have sensor auto-
>> exposure enabled by default. This interferes with the AGC algorithm
>> running in the IPA.
>>
>> By default, disable the sensor auto-exposure on startup so that the IPA
>> can control the sensor shutter time and gain.
>>
>> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> 
> Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> Works perfectly with vd65g4 through camera_sensor_legacy, so should
> work with all the ST sensors. I haven't got a setup using
> camera_sensor_raw at present to test that path, but the code is
> identical to the legacy one.

Tested successfully on my side too with all ST sensor having auto
exposure enabled at boot time, i.e vd55g1, vd55g4, vd65g4, vd56g3 and
vd66gy. Thanks a ton.

Tested-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>

All comments further down are minor, feel free to ignore them.

> 
> Thanks
>   Dave
> 
>> ---
>>  src/libcamera/sensor/camera_sensor_legacy.cpp | 14 ++++++++++++++
>>  src/libcamera/sensor/camera_sensor_raw.cpp    | 14 ++++++++++++++
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp
>> index 6a683821f219..d86624dc4cc7 100644
>> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp
>> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp
>> @@ -295,6 +295,20 @@ int CameraSensorLegacy::init()
>>                         return ret;
>>         }
>>
>> +       /*
>> +        * Ensure auto-exposure is disabled in the sensor as the IPAs ought to
>> +        * handle that.
>> +        */
>> +       const struct v4l2_query_ext_ctrl *exposureAuto = subdev_->controlInfo(V4L2_CID_EXPOSURE_AUTO);
>> +       if (exposureAuto && !(exposureAuto->flags & V4L2_CTRL_FLAG_READ_ONLY)) {

If the control is available and happens to be read only, should we print
a warning message stating that we can't disable auto exposure here ?
Something alongside :

  if (exposureAuto) {
    if (exposureAuto->flags & V4L2_CTRL_FLAG_READ_ONLY)
      LOG(CameraSensor, Warning) << "Can't disable auto exposure";
    else
      [...]
  }

In that case it may also be worth checking if the control is set to
V4L2_EXPOSURE_AUTO to avoid printing the warning if it's set to
V4L2_EXPOSURE_MANUAL already.

>> +               ControlList ctrl(subdev_->controls());
>> +
>> +               ctrl.set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_MANUAL);
>> +               ret = subdev_->setControls(&ctrl);
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +>>         return
applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
>>  }
>>
>> diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp
>> index 10eba0331fe8..1c5ee80281ad 100644
>> --- a/src/libcamera/sensor/camera_sensor_raw.cpp
>> +++ b/src/libcamera/sensor/camera_sensor_raw.cpp
>> @@ -551,6 +551,20 @@ std::optional<int> CameraSensorRaw::init()
>>                         return ret;
>>         }
>>
>> +       /*
>> +        * Ensure auto-exposure is disabled in the sensor as the IPAs ought to
>> +        * handle that.
>> +        */
>> +       const struct v4l2_query_ext_ctrl *exposureAuto = subdev_->controlInfo(V4L2_CID_EXPOSURE_AUTO);
>> +       if (exposureAuto && !(exposureAuto->flags & V4L2_CTRL_FLAG_READ_ONLY)) {

Ditto.

>> +               ControlList ctrl(subdev_->controls());>> +
>> +               ctrl.set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_MANUAL);
>> +               ret = subdev_->setControls(&ctrl);
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>>         ret = applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
>>         if (ret)
>>                 return { ret };
>> --
>> 2.53.0
>>
Naushir Patuck June 30, 2026, 12:13 p.m. UTC | #3 
On Tue, 30 Jun 2026 at 13:07, Benjamin Mugnier
<benjamin.mugnier@foss.st.com> wrote:
>
> Hi Naush,
>
> Well that was fast.
>
> Le 30/06/2026 à 12:40, Dave Stevenson a écrit :
> > Hi Naush
> >
> > On Tue, 30 Jun 2026 at 09:23, Naushir Patuck <naush@raspberrypi.com> wrote:
> >>
> >> Some sensor device drivers (e.g, VD66GY and VD56G3) have sensor auto-
> >> exposure enabled by default. This interferes with the AGC algorithm
> >> running in the IPA.
> >>
> >> By default, disable the sensor auto-exposure on startup so that the IPA
> >> can control the sensor shutter time and gain.
> >>
> >> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> >
> > Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> >
> > Works perfectly with vd65g4 through camera_sensor_legacy, so should
> > work with all the ST sensors. I haven't got a setup using
> > camera_sensor_raw at present to test that path, but the code is
> > identical to the legacy one.
>
> Tested successfully on my side too with all ST sensor having auto
> exposure enabled at boot time, i.e vd55g1, vd55g4, vd65g4, vd56g3 and
> vd66gy. Thanks a ton.
>
> Tested-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
>
> All comments further down are minor, feel free to ignore them.
>
> >
> > Thanks
> >   Dave
> >
> >> ---
> >>  src/libcamera/sensor/camera_sensor_legacy.cpp | 14 ++++++++++++++
> >>  src/libcamera/sensor/camera_sensor_raw.cpp    | 14 ++++++++++++++
> >>  2 files changed, 28 insertions(+)
> >>
> >> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp
> >> index 6a683821f219..d86624dc4cc7 100644
> >> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp
> >> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp
> >> @@ -295,6 +295,20 @@ int CameraSensorLegacy::init()
> >>                         return ret;
> >>         }
> >>
> >> +       /*
> >> +        * Ensure auto-exposure is disabled in the sensor as the IPAs ought to
> >> +        * handle that.
> >> +        */
> >> +       const struct v4l2_query_ext_ctrl *exposureAuto = subdev_->controlInfo(V4L2_CID_EXPOSURE_AUTO);
> >> +       if (exposureAuto && !(exposureAuto->flags & V4L2_CTRL_FLAG_READ_ONLY)) {
>
> If the control is available and happens to be read only, should we print
> a warning message stating that we can't disable auto exposure here ?
> Something alongside :
>
>   if (exposureAuto) {
>     if (exposureAuto->flags & V4L2_CTRL_FLAG_READ_ONLY)
>       LOG(CameraSensor, Warning) << "Can't disable auto exposure";
>     else
>       [...]
>   }
>
> In that case it may also be worth checking if the control is set to
> V4L2_EXPOSURE_AUTO to avoid printing the warning if it's set to
> V4L2_EXPOSURE_MANUAL already.

I can do this, but let me defer to Laurent to see if that's desirable.

Naush

>
> >> +               ControlList ctrl(subdev_->controls());
> >> +
> >> +               ctrl.set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_MANUAL);
> >> +               ret = subdev_->setControls(&ctrl);
> >> +               if (ret)
> >> +                       return ret;
> >> +       }
> >> +>>         return
> applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
> >>  }
> >>
> >> diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp
> >> index 10eba0331fe8..1c5ee80281ad 100644
> >> --- a/src/libcamera/sensor/camera_sensor_raw.cpp
> >> +++ b/src/libcamera/sensor/camera_sensor_raw.cpp
> >> @@ -551,6 +551,20 @@ std::optional<int> CameraSensorRaw::init()
> >>                         return ret;
> >>         }
> >>
> >> +       /*
> >> +        * Ensure auto-exposure is disabled in the sensor as the IPAs ought to
> >> +        * handle that.
> >> +        */
> >> +       const struct v4l2_query_ext_ctrl *exposureAuto = subdev_->controlInfo(V4L2_CID_EXPOSURE_AUTO);
> >> +       if (exposureAuto && !(exposureAuto->flags & V4L2_CTRL_FLAG_READ_ONLY)) {
>
> Ditto.
>
> >> +               ControlList ctrl(subdev_->controls());>> +
> >> +               ctrl.set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_MANUAL);
> >> +               ret = subdev_->setControls(&ctrl);
> >> +               if (ret)
> >> +                       return ret;
> >> +       }
> >> +
> >>         ret = applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
> >>         if (ret)
> >>                 return { ret };
> >> --
> >> 2.53.0
> >>
>
> --
> Regards,
> Benjamin
>

Patch
diff mbox series 

diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp
index 6a683821f219..d86624dc4cc7 100644
--- a/src/libcamera/sensor/camera_sensor_legacy.cpp
+++ b/src/libcamera/sensor/camera_sensor_legacy.cpp
@@ -295,6 +295,20 @@  int CameraSensorLegacy::init()
 			return ret;
 	}
 
+	/*
+	 * Ensure auto-exposure is disabled in the sensor as the IPAs ought to
+	 * handle that.
+	 */
+	const struct v4l2_query_ext_ctrl *exposureAuto = subdev_->controlInfo(V4L2_CID_EXPOSURE_AUTO);
+	if (exposureAuto && !(exposureAuto->flags & V4L2_CTRL_FLAG_READ_ONLY)) {
+		ControlList ctrl(subdev_->controls());
+
+		ctrl.set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_MANUAL);
+		ret = subdev_->setControls(&ctrl);
+		if (ret)
+			return ret;
+	}
+
 	return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
 }
 
diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp
index 10eba0331fe8..1c5ee80281ad 100644
--- a/src/libcamera/sensor/camera_sensor_raw.cpp
+++ b/src/libcamera/sensor/camera_sensor_raw.cpp
@@ -551,6 +551,20 @@  std::optional<int> CameraSensorRaw::init()
 			return ret;
 	}
 
+	/*
+	 * Ensure auto-exposure is disabled in the sensor as the IPAs ought to
+	 * handle that.
+	 */
+	const struct v4l2_query_ext_ctrl *exposureAuto = subdev_->controlInfo(V4L2_CID_EXPOSURE_AUTO);
+	if (exposureAuto && !(exposureAuto->flags & V4L2_CTRL_FLAG_READ_ONLY)) {
+		ControlList ctrl(subdev_->controls());
+
+		ctrl.set(V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_MANUAL);
+		ret = subdev_->setControls(&ctrl);
+		if (ret)
+			return ret;
+	}
+
 	ret = applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
 	if (ret)
 		return { ret };