[v2,4/4] ipa: rpi: pisp: Allow an initial decompand curve to be set on the FE

24561 diff mbox series

In the current code, decompand will only set a curve in the prepare
phase, which will only run after 1-2 frames pass through the FE. This
is fixed by adding an initialValues() member function to the decompand
algorithm, which will be called in the IPA before we start the hardware
streaming.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>
---
 src/ipa/rpi/controller/decompand_algorithm.h |  2 ++
 src/ipa/rpi/controller/rpi/decompand.cpp     |  8 ++++++++
 src/ipa/rpi/controller/rpi/decompand.h       |  1 +
 src/ipa/rpi/pisp/pisp.cpp                    | 15 +++++++++++++++
 4 files changed, 26 insertions(+)

Hi Naush

Thanks for the patch.

On Fri, 3 Oct 2025 at 13:18, Naushir Patuck <naush@raspberrypi.com> wrote:
>
> In the current code, decompand will only set a curve in the prepare
> phase, which will only run after 1-2 frames pass through the FE. This
> is fixed by adding an initialValues() member function to the decompand
> algorithm, which will be called in the IPA before we start the hardware
> streaming.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Tested-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>
> ---
>  src/ipa/rpi/controller/decompand_algorithm.h |  2 ++
>  src/ipa/rpi/controller/rpi/decompand.cpp     |  8 ++++++++
>  src/ipa/rpi/controller/rpi/decompand.h       |  1 +
>  src/ipa/rpi/pisp/pisp.cpp                    | 15 +++++++++++++++
>  4 files changed, 26 insertions(+)
>
> diff --git a/src/ipa/rpi/controller/decompand_algorithm.h b/src/ipa/rpi/controller/decompand_algorithm.h
> index f19f8c109323..6d2467490106 100644
> --- a/src/ipa/rpi/controller/decompand_algorithm.h
> +++ b/src/ipa/rpi/controller/decompand_algorithm.h
> @@ -19,6 +19,8 @@ public:
>                 : Algorithm(controller)
>         {
>         }
> +       /* A decompand algorithm must provide the following: */
> +       virtual void initialValues(libcamera::ipa::Pwl &decompandCurve) = 0;
>  };
>
>  } /* namespace RPiController */
> diff --git a/src/ipa/rpi/controller/rpi/decompand.cpp b/src/ipa/rpi/controller/rpi/decompand.cpp
> index 5b4c2e5524af..2d457926c060 100644
> --- a/src/ipa/rpi/controller/rpi/decompand.cpp
> +++ b/src/ipa/rpi/controller/rpi/decompand.cpp
> @@ -39,6 +39,14 @@ void Decompand::switchMode(CameraMode const &cameraMode,
>         mode_ = cameraMode;
>  }
>
> +void Decompand::initialValues(libcamera::ipa::Pwl &decompandCurve)
> +{
> +       if (config_.bitdepth == 0 || mode_.bitdepth == config_.bitdepth) {
> +               decompandCurve = config_.decompandCurve;
> +       } else
> +               decompandCurve = {};
> +}
> +
>

I guess it makes me wonder a bit whether we could just return the PWL
out of switchMode, could we then dispense with both initialValues and
prepare? But I don't think any other algorithms do this, so what is
here seems fine too.

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks!
David

 void Decompand::prepare(Metadata *imageMetadata)
>  {
>         DecompandStatus decompandStatus;
> diff --git a/src/ipa/rpi/controller/rpi/decompand.h b/src/ipa/rpi/controller/rpi/decompand.h
> index 0f2a9b3bbdc4..6db779c359a8 100644
> --- a/src/ipa/rpi/controller/rpi/decompand.h
> +++ b/src/ipa/rpi/controller/rpi/decompand.h
> @@ -20,6 +20,7 @@ public:
>         int read(const libcamera::YamlObject &params) override;
>         void initialise() override;
>         void switchMode(CameraMode const &cameraMode, Metadata *metadata) override;
> +       void initialValues(libcamera::ipa::Pwl &decompandCurve) override;
>         void prepare(Metadata *imageMetadata) override;
>
>  private:
> diff --git a/src/ipa/rpi/pisp/pisp.cpp b/src/ipa/rpi/pisp/pisp.cpp
> index f5be39aca5ee..01baebcd2bb6 100644
> --- a/src/ipa/rpi/pisp/pisp.cpp
> +++ b/src/ipa/rpi/pisp/pisp.cpp
> @@ -32,6 +32,7 @@
>  #include "controller/cac_status.h"
>  #include "controller/ccm_status.h"
>  #include "controller/contrast_status.h"
> +#include "controller/decompand_algorithm.h"
>  #include "controller/decompand_status.h"
>  #include "controller/denoise_algorithm.h"
>  #include "controller/denoise_status.h"
> @@ -335,6 +336,20 @@ int32_t IpaPiSP::platformStart([[maybe_unused]] const ControlList &controls,
>         /* Cause the stitch block to be reset correctly. */
>         lastStitchHdrStatus_ = HdrStatus();
>
> +       /* Setup a default decompand curve on startup if needed. */
> +       RPiController::DecompandAlgorithm *decompand = dynamic_cast<RPiController::DecompandAlgorithm *>(
> +               controller_.getAlgorithm("decompand"));
> +       if (decompand) {
> +               std::scoped_lock<FrontEnd> l(*fe_);
> +               pisp_fe_global_config feGlobal;
> +               DecompandStatus decompandStatus;
> +
> +               fe_->GetGlobal(feGlobal);
> +               decompand->initialValues(decompandStatus.decompandCurve);
> +               applyDecompand(&decompandStatus, feGlobal);
> +               fe_->SetGlobal(feGlobal);
> +       }
> +
>         return 0;
>  }
>
> --
> 2.43.0
>