[libcamera-devel] [PATCH] ipa: vimc: Add configure() function

[Umang Jain]

Hi Laurent,

thanks for the patch.

On 8/5/21 10:31 PM, Laurent Pinchart wrote:
> As part of an effort to make the vimc IPA usable for testing, extend it
> with a configure function. The configuration is currently ignored by the
> IPA.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> Umang, I've had this in my tree for some time, if it helps the work
> you're doing on testing buffer mapping in vimc, please feel free to use
> this patch (as-is, or squashed into anything else).
Ok. :-)
> ---
>   include/libcamera/ipa/vimc.mojom     |  5 +++++
>   src/ipa/vimc/vimc.cpp                | 11 +++++++++++
>   src/libcamera/pipeline/vimc/vimc.cpp | 16 ++++++++++++++++
>   3 files changed, 32 insertions(+)
>
> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom
> index 86bc318a878e..85d25e27ebcd 100644
> --- a/include/libcamera/ipa/vimc.mojom
> +++ b/include/libcamera/ipa/vimc.mojom
> @@ -19,6 +19,11 @@ enum IPAOperationCode {
>   
>   interface IPAVimcInterface {
>   	init(libcamera.IPASettings settings) => (int32 ret);
> +
> +	configure(libcamera.IPACameraSensorInfo sensorInfo,
> +		  map<uint32, libcamera.IPAStream> streamConfig,
> +		  map<uint32, libcamera.ControlInfoMap> entityControls) => ();
> +

We should return int here, as IPU3 does. I'll fixup this and carry it in 
my ongoing series.

We can merge this as part of that series.

Rest looks good:
Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

>   	start() => (int32 ret);
>   	stop();
>   };
> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
> index 0c0ee006fdc7..5da5c6c09d68 100644
> --- a/src/ipa/vimc/vimc.cpp
> +++ b/src/ipa/vimc/vimc.cpp
> @@ -34,6 +34,10 @@ public:
>   	int start() override;
>   	void stop() override;
>   
> +	void configure(const IPACameraSensorInfo &sensorInfo,
> +		       const std::map<unsigned int, IPAStream> &streamConfig,
> +		       const std::map<unsigned int, ControlInfoMap> &entityControls) override;
> +
>   private:
>   	void initTrace();
>   	void trace(enum ipa::vimc::IPAOperationCode operation);
> @@ -86,6 +90,13 @@ void IPAVimc::stop()
>   	LOG(IPAVimc, Debug) << "stop vimc IPA!";
>   }
>   
> +void IPAVimc::configure([[maybe_unused]] const IPACameraSensorInfo &sensorInfo,
> +			[[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
> +			[[maybe_unused]] const std::map<unsigned int, ControlInfoMap> &entityControls)
> +{
> +	LOG(IPAVimc, Debug) << "configure()";
> +}
> +
>   void IPAVimc::initTrace()
>   {
>   	struct stat fifoStat;
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 12f7517fd0ae..b7dd6595d608 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -295,6 +295,22 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
>   
>   	cfg.setStream(&data->stream_);
>   
> +	if (data->ipa_) {
> +		/* Inform IPA of stream configuration and sensor controls. */
> +		std::map<unsigned int, IPAStream> streamConfig;
> +		streamConfig.emplace(std::piecewise_construct,
> +				     std::forward_as_tuple(0),
> +				     std::forward_as_tuple(cfg.pixelFormat, cfg.size));
> +
> +		std::map<unsigned int, ControlInfoMap> entityControls;
> +		entityControls.emplace(0, data->sensor_->controls());
> +
> +		IPACameraSensorInfo sensorInfo;
> +		data->sensor_->sensorInfo(&sensorInfo);
> +
> +		data->ipa_->configure(sensorInfo, streamConfig, entityControls);
> +	}
> +
>   	return 0;
>   }
>