[libcamera-devel] [PATCH v1 4/9] pipeline: ipa: raspberrypi: Add HBLANK control to DelayedControls
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Oct 4 19:05:52 CEST 2022
Hi Naush,
Thank you for the patch.
On Mon, Oct 03, 2022 at 09:39:30AM +0100, Naushir Patuck via libcamera-devel wrote:
> Update CamHelper::getDelays() to return the sensor HBLANK delay. The HBLANK
> delay is set to the same value as VBLANK delay for all sensors in the Raspberry
> Pi IPA.
>
> Return the HBLANK gain delay from the IPA to the pipeline handler, and
> initialise DelayedControls to handle V4L2_CID_HBLANK with this delay value.
>
> As a drive-by, check that the V4L2_CID_HBLANK control is aviailable when calling
s/aviailable/available/
> IPARPi::configure().
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> include/libcamera/ipa/raspberrypi.mojom | 1 +
> src/ipa/raspberrypi/cam_helper.cpp | 3 ++-
> src/ipa/raspberrypi/cam_helper.h | 2 +-
> src/ipa/raspberrypi/cam_helper_imx290.cpp | 5 +++--
You will need to also address imx296, as another patch has been merged
in the meantime.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> src/ipa/raspberrypi/cam_helper_imx477.cpp | 5 +++--
> src/ipa/raspberrypi/cam_helper_imx519.cpp | 5 +++--
> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 5 +++--
> src/ipa/raspberrypi/cam_helper_ov9281.cpp | 5 +++--
> src/ipa/raspberrypi/raspberrypi.cpp | 6 ++++--
> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 1 +
> 10 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
> index c0de435b7b33..40f78d9e3b3f 100644
> --- a/include/libcamera/ipa/raspberrypi.mojom
> +++ b/include/libcamera/ipa/raspberrypi.mojom
> @@ -23,6 +23,7 @@ struct SensorConfig {
> uint32 gainDelay;
> uint32 exposureDelay;
> uint32 vblankDelay;
> + uint32 hblankDelay;
> uint32 sensorMetadata;
> };
>
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
> index fd3527b94501..916632f83037 100644
> --- a/src/ipa/raspberrypi/cam_helper.cpp
> +++ b/src/ipa/raspberrypi/cam_helper.cpp
> @@ -114,7 +114,7 @@ void CamHelper::setCameraMode(const CameraMode &mode)
> }
>
> void CamHelper::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay) const
> + int &vblankDelay, int &hblankDelay) const
> {
> /*
> * These values are correct for many sensors. Other sensors will
> @@ -123,6 +123,7 @@ void CamHelper::getDelays(int &exposureDelay, int &gainDelay,
> exposureDelay = 2;
> gainDelay = 1;
> vblankDelay = 2;
> + hblankDelay = 2;
> }
>
> bool CamHelper::sensorEmbeddedDataPresent() const
> diff --git a/src/ipa/raspberrypi/cam_helper.h b/src/ipa/raspberrypi/cam_helper.h
> index 9b5e602689f3..1bbdd715d2b1 100644
> --- a/src/ipa/raspberrypi/cam_helper.h
> +++ b/src/ipa/raspberrypi/cam_helper.h
> @@ -88,7 +88,7 @@ public:
> virtual uint32_t gainCode(double gain) const = 0;
> virtual double gain(uint32_t gainCode) const = 0;
> virtual void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay) const;
> + int &vblankDelay, int &hblankDelay) const;
> virtual bool sensorEmbeddedDataPresent() const;
> virtual double getModeSensitivity(const CameraMode &mode) const;
> virtual unsigned int hideFramesStartup() const;
> diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> index 25f23d531c72..7d6f5b549a73 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx290.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> @@ -18,7 +18,7 @@ public:
> uint32_t gainCode(double gain) const override;
> double gain(uint32_t gainCode) const override;
> void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay) const override;
> + int &vblankDelay, int &hblankDelay) const override;
> unsigned int hideFramesModeSwitch() const override;
>
> private:
> @@ -46,11 +46,12 @@ double CamHelperImx290::gain(uint32_t gainCode) const
> }
>
> void CamHelperImx290::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay) const
> + int &vblankDelay, int &hblankDelay) const
> {
> exposureDelay = 2;
> gainDelay = 2;
> vblankDelay = 2;
> + hblankDelay = 2;
> }
>
> unsigned int CamHelperImx290::hideFramesModeSwitch() const
> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> index 71529bdd3034..76a82cc51378 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> @@ -49,7 +49,7 @@ public:
> uint32_t getVBlanking(Duration &exposure, Duration minFrameDuration,
> Duration maxFrameDuration) const override;
> void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay) const override;
> + int &vblankDelay, int &hblankDelay) const override;
> bool sensorEmbeddedDataPresent() const override;
>
> private:
> @@ -153,11 +153,12 @@ uint32_t CamHelperImx477::getVBlanking(Duration &exposure,
> }
>
> void CamHelperImx477::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay) const
> + int &vblankDelay, int &hblankDelay) const
> {
> exposureDelay = 2;
> gainDelay = 2;
> vblankDelay = 3;
> + hblankDelay = 3;
> }
>
> bool CamHelperImx477::sensorEmbeddedDataPresent() const
> diff --git a/src/ipa/raspberrypi/cam_helper_imx519.cpp b/src/ipa/raspberrypi/cam_helper_imx519.cpp
> index 2c120dad1680..9dff1eeb899f 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx519.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx519.cpp
> @@ -49,7 +49,7 @@ public:
> uint32_t getVBlanking(Duration &exposure, Duration minFrameDuration,
> Duration maxFrameDuration) const override;
> void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay) const override;
> + int &vblankDelay, int &hblankDelay) const override;
> bool sensorEmbeddedDataPresent() const override;
>
> private:
> @@ -153,11 +153,12 @@ uint32_t CamHelperImx519::getVBlanking(Duration &exposure,
> }
>
> void CamHelperImx519::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay) const
> + int &vblankDelay, int &hblankDelay) const
> {
> exposureDelay = 2;
> gainDelay = 2;
> vblankDelay = 3;
> + hblankDelay = 3;
> }
>
> bool CamHelperImx519::sensorEmbeddedDataPresent() const
> diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> index 04fb725d42db..5a99083dee78 100644
> --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> @@ -18,7 +18,7 @@ public:
> uint32_t gainCode(double gain) const override;
> double gain(uint32_t gainCode) const override;
> void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay) const override;
> + int &vblankDelay, int &hblankDelay) const override;
> unsigned int hideFramesStartup() const override;
> unsigned int hideFramesModeSwitch() const override;
> unsigned int mistrustFramesStartup() const override;
> @@ -53,7 +53,7 @@ double CamHelperOv5647::gain(uint32_t gainCode) const
> }
>
> void CamHelperOv5647::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay) const
> + int &vblankDelay, int &hblankDelay) const
> {
> /*
> * We run this sensor in a mode where the gain delay is bumped up to
> @@ -62,6 +62,7 @@ void CamHelperOv5647::getDelays(int &exposureDelay, int &gainDelay,
> exposureDelay = 2;
> gainDelay = 2;
> vblankDelay = 2;
> + hblankDelay = 2;
> }
>
> unsigned int CamHelperOv5647::hideFramesStartup() const
> diff --git a/src/ipa/raspberrypi/cam_helper_ov9281.cpp b/src/ipa/raspberrypi/cam_helper_ov9281.cpp
> index 66f56a31e1b8..86c5bc4c8fda 100644
> --- a/src/ipa/raspberrypi/cam_helper_ov9281.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_ov9281.cpp
> @@ -18,7 +18,7 @@ public:
> uint32_t gainCode(double gain) const override;
> double gain(uint32_t gainCode) const override;
> void getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay) const override;
> + int &vblankDelay, int &hblankDelay) const override;
>
> private:
> /*
> @@ -49,12 +49,13 @@ double CamHelperOv9281::gain(uint32_t gainCode) const
> }
>
> void CamHelperOv9281::getDelays(int &exposureDelay, int &gainDelay,
> - int &vblankDelay) const
> + int &vblankDelay, int &hblankDelay) const
> {
> /* The driver appears to behave as follows: */
> exposureDelay = 2;
> gainDelay = 2;
> vblankDelay = 2;
> + hblankDelay = 2;
> }
>
> static CamHelper *create()
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 13807f4d47f7..9b0ad4361c97 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -219,13 +219,14 @@ int IPARPi::init(const IPASettings &settings, IPAInitResult *result)
> * Pass out the sensor config to the pipeline handler in order
> * to setup the staggered writer class.
> */
> - int gainDelay, exposureDelay, vblankDelay, sensorMetadata;
> - helper_->getDelays(exposureDelay, gainDelay, vblankDelay);
> + int gainDelay, exposureDelay, vblankDelay, hblankDelay, sensorMetadata;
> + helper_->getDelays(exposureDelay, gainDelay, vblankDelay, hblankDelay);
> sensorMetadata = helper_->sensorEmbeddedDataPresent();
>
> result->sensorConfig.gainDelay = gainDelay;
> result->sensorConfig.exposureDelay = exposureDelay;
> result->sensorConfig.vblankDelay = vblankDelay;
> + result->sensorConfig.hblankDelay = hblankDelay;
> result->sensorConfig.sensorMetadata = sensorMetadata;
>
> /* Load the tuning file for this sensor. */
> @@ -607,6 +608,7 @@ bool IPARPi::validateSensorControls()
> V4L2_CID_ANALOGUE_GAIN,
> V4L2_CID_EXPOSURE,
> V4L2_CID_VBLANK,
> + V4L2_CID_HBLANK,
> };
>
> for (auto c : ctrls) {
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index dcd81650c32d..623bec6bea3c 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1299,6 +1299,7 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me
> std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
> { V4L2_CID_ANALOGUE_GAIN, { result.sensorConfig.gainDelay, false } },
> { V4L2_CID_EXPOSURE, { result.sensorConfig.exposureDelay, false } },
> + { V4L2_CID_HBLANK, { result.sensorConfig.hblankDelay, false } },
> { V4L2_CID_VBLANK, { result.sensorConfig.vblankDelay, true } }
> };
> data->delayedCtrls_ = std::make_unique<DelayedControls>(data->sensor_->device(), params);
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list