[v1] ipa: rpi: Improve control over which sensor modes require "debinning"

26767 diff mbox series

"Debinning" is the process that corrects the spatial sampling of
pixels when this has been disturbed by the standard binning process.

However, debinning is not required for sensors that do it themselves,
or for quad Bayer sensors at 2x2 binning where the spatial sampling is
already correct.

Here we add a getMinDebinFactor() to the CamHelper which allows us to
customise at what binning (or downscaling) factor we need to apply the
correction. This value is then used to decide whether to enable the
relevant PiSP block.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/rpi/cam_helper/cam_helper.cpp        | 6 ++++++
 src/ipa/rpi/cam_helper/cam_helper.h          | 6 ++++++
 src/ipa/rpi/cam_helper/cam_helper_imx708.cpp | 7 +++++++
 src/ipa/rpi/pisp/pisp.cpp                    | 8 +++++---
 4 files changed, 24 insertions(+), 3 deletions(-)

A small correction of my own...

On Mon, 18 May 2026 at 15:26, David Plowman
<david.plowman@raspberrypi.com> wrote:
>
> "Debinning" is the process that corrects the spatial sampling of
> pixels when this has been disturbed by the standard binning process.
>
> However, debinning is not required for sensors that do it themselves,
> or for quad Bayer sensors at 2x2 binning where the spatial sampling is
> already correct.
>
> Here we add a getMinDebinFactor() to the CamHelper which allows us to
> customise at what binning (or downscaling) factor we need to apply the
> correction. This value is then used to decide whether to enable the
> relevant PiSP block.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/ipa/rpi/cam_helper/cam_helper.cpp        | 6 ++++++
>  src/ipa/rpi/cam_helper/cam_helper.h          | 6 ++++++
>  src/ipa/rpi/cam_helper/cam_helper_imx708.cpp | 7 +++++++
>  src/ipa/rpi/pisp/pisp.cpp                    | 8 +++++---
>  4 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/src/ipa/rpi/cam_helper/cam_helper.cpp b/src/ipa/rpi/cam_helper/cam_helper.cpp
> index a78db9c1..dfdcd167 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper.cpp
> +++ b/src/ipa/rpi/cam_helper/cam_helper.cpp
> @@ -204,6 +204,12 @@ unsigned int CamHelper::mistrustFramesModeSwitch() const
>         return 0;
>  }
>
> +unsigned int CamHelper::getMinDebinFactor() const
> +{
> +       /* Most cameras require debinning from 2x2 binning upwards. */
> +       return 2;
> +}
> +
>  void CamHelper::parseEmbeddedData(Span<const uint8_t> buffer,
>                                   Metadata &metadata)
>  {
> diff --git a/src/ipa/rpi/cam_helper/cam_helper.h b/src/ipa/rpi/cam_helper/cam_helper.h
> index 4a826690..349891c4 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper.h
> +++ b/src/ipa/rpi/cam_helper/cam_helper.h
> @@ -62,6 +62,11 @@ namespace RPiController {
>   * MistrustFramesModeSwitch(): The number of frames, after a mode switch
>   *    (other than start-up), for which control algorithms should not run
>   *    (for example, metadata may be unreliable).
> + * getMinDebinFactor(): the binning factor after which we should apply
> + *    "debinning", which corrects for the uneven spatial sampling of the
> + *    standard binning process. A return value of 2 means to enable
> + *    debinning for camera modes using 2x2, or larger, binning. A return
> + *    value of zero means never to enable binning.

s/binning/debinning/

>   */
>
>  class CamHelper
> @@ -93,6 +98,7 @@ public:
>         virtual unsigned int hideFramesModeSwitch() const;
>         virtual unsigned int mistrustFramesStartup() const;
>         virtual unsigned int mistrustFramesModeSwitch() const;
> +       virtual unsigned int getMinDebinFactor() const;
>
>  protected:
>         void parseEmbeddedData(libcamera::Span<const uint8_t> buffer,
> diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
> index 6150909c..e7b8d671 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
> +++ b/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp
> @@ -58,6 +58,7 @@ public:
>         double getModeSensitivity(const CameraMode &mode) const override;
>         unsigned int hideFramesModeSwitch() const override;
>         unsigned int hideFramesStartup() const override;
> +       unsigned int getMinDebinFactor() const override;
>
>  private:
>         /*
> @@ -237,6 +238,12 @@ unsigned int CamHelperImx708::hideFramesStartup() const
>         return hideFramesModeSwitch();
>  }
>
> +unsigned int CamHelperImx708::getMinDebinFactor() const
> +{
> +       /* Quad-Bayer sensor, so debinning required only at 4x4 binning. */
> +       return 4;
> +}
> +
>  void CamHelperImx708::populateMetadata(const MdParser::RegisterMap &registers,
>                                        Metadata &metadata) const
>  {
> diff --git a/src/ipa/rpi/pisp/pisp.cpp b/src/ipa/rpi/pisp/pisp.cpp
> index de2a6afe..fd28fc38 100644
> --- a/src/ipa/rpi/pisp/pisp.cpp
> +++ b/src/ipa/rpi/pisp/pisp.cpp
> @@ -1078,12 +1078,14 @@ void IpaPiSP::setStatsAndDebin()
>         pisp_be_global_config beGlobal;
>         be_->GetGlobal(beGlobal);
>
> -       if (mode_.binX > 1 || mode_.binY > 1) {
> +       unsigned int minDebinFactor = helper_->getMinDebinFactor();
> +       if (minDebinFactor &&
> +           (mode_.binX >= minDebinFactor || mode_.binY >= minDebinFactor)) {
>                 pisp_be_debin_config debin;
>
>                 be_->GetDebin(debin);
> -               debin.h_enable = (mode_.binX > 1);
> -               debin.v_enable = (mode_.binY > 1);
> +               debin.h_enable = (mode_.binX >= minDebinFactor);
> +               debin.v_enable = (mode_.binY >= minDebinFactor);
>                 be_->SetDebin(debin);
>                 beGlobal.bayer_enables |= PISP_BE_BAYER_ENABLE_DEBIN;
>         } else
> --
> 2.47.3
>