[libcamera-devel] [PATCH 07/20] ipa: rpi: Add statsInline to the Controller hardware description

[Jacopo Mondi]

On Fri, Oct 06, 2023 at 02:19:47PM +0100, Naushir Patuck via libcamera-devel wrote:
> Add a new boolean field (statsInline) to Controller::HardwareConfigMap.
> This field indicates where the statistics are generated in the hardware
> ISP pipeline. For statsInline == true, statistics are generated before
> the frame is processed (e.g. the PiSP case), and statsInline == false
> indicates statistics are generated after the frame is processed (e.g.
> the VC4 case).
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  src/ipa/rpi/common/ipa_base.cpp       | 19 ++++++++++++++-----
>  src/ipa/rpi/controller/controller.cpp |  3 ++-
>  src/ipa/rpi/controller/controller.h   |  1 +
>  3 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index 97f647a9e53e..f28eb36b7a44 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -429,11 +429,10 @@ void IpaBase::prepareIsp(const PrepareParams &params)
>  	}
>
>  	/*
> -	 * If a statistics buffer has been passed in, call processStats
> -	 * directly now before prepare() since the statistics are available in-line
> -	 * with the Bayer frame.
> +	 * If the statistics are inline (i.e. already available with the Bayer
> +	 * frame), call processStats() now before prepare().
>  	 */
> -	if (params.buffers.stats)
> +	if (controller_.getHardwareConfig().statsInline)
>  		processStats({ params.buffers, params.ipaContext });
>
>  	/* Do we need/want to call prepare? */
> @@ -445,6 +444,10 @@ void IpaBase::prepareIsp(const PrepareParams &params)
>
>  	frameCount_++;
>
> +	/* If the statistics are inline the metadata can be returned early. */
> +	if (controller_.getHardwareConfig().statsInline)
> +		reportMetadata(ipaContext);
> +

Can't you drop this one and unconditionally call reportMetadata() in
processStats() ?

>  	/* Ready to push the input buffer into the ISP. */
>  	prepareIspComplete.emit(params.buffers, false);
>  }
> @@ -479,7 +482,13 @@ void IpaBase::processStats(const ProcessParams &params)
>  		}
>  	}
>
> -	reportMetadata(ipaContext);
> +	/*
> +	 * If the statistics are not inline the metadata must be returned now,
> +	 * before the processStatsComplete signal.
> +	 */
> +	if (!controller_.getHardwareConfig().statsInline)
> +		reportMetadata(ipaContext);
> +
>  	processStatsComplete.emit(params.buffers);
>  }
>
> diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp
> index 14d245da2ce7..4b6f82b41916 100644
> --- a/src/ipa/rpi/controller/controller.cpp
> +++ b/src/ipa/rpi/controller/controller.cpp
> @@ -34,7 +34,8 @@ static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap
>  			.focusRegions = { 4, 3 },
>  			.numHistogramBins = 128,
>  			.numGammaPoints = 33,
> -			.pipelineWidth = 13
> +			.pipelineWidth = 13,
> +			.statsInline = false,
>  		}
>  	},
>  };
> diff --git a/src/ipa/rpi/controller/controller.h b/src/ipa/rpi/controller/controller.h
> index c6af5cd6c7d4..a8bc61880ab4 100644
> --- a/src/ipa/rpi/controller/controller.h
> +++ b/src/ipa/rpi/controller/controller.h
> @@ -45,6 +45,7 @@ public:
>  		unsigned int numHistogramBins;
>  		unsigned int numGammaPoints;
>  		unsigned int pipelineWidth;
> +		bool statsInline;
>  	};
>
>  	Controller();
> --
> 2.34.1
>