[libcamera-devel] [PATCH v3 5/6] ipa: ipu3: Introduce IPAConfigInfo in IPC

[Laurent Pinchart]

Hi Paul,

On Mon, May 24, 2021 at 02:43:54PM +0900, paul.elder at ideasonboard.com wrote:
> On Mon, May 24, 2021 at 04:00:01AM +0300, Laurent Pinchart wrote:
> > On Fri, May 21, 2021 at 06:58:22PM +0530, Umang Jain wrote:
> > > IPAConfigInfo is a consolidated data structure passed from IPU3
> > > pipeline-handler to IPU3 IPA. The structure can be extended with
> > > additional parameters to accommodate the requirements of multiple
> > > IPU3 IPA modules.
> > > 
> > > Adapt the in-tree IPU3 IPA to use IPAConfigInfo as well.
> > > 
> > > Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> > > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> > > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> > > Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> > > ---
> > >  include/libcamera/ipa/ipu3.mojom     | 10 ++++++++--
> > >  src/ipa/ipu3/ipu3.cpp                | 14 ++++++--------
> > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 10 +++++++---
> > >  3 files changed, 21 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
> > > index 9e3cd885..6b6b431f 100644
> > > --- a/include/libcamera/ipa/ipu3.mojom
> > > +++ b/include/libcamera/ipa/ipu3.mojom
> > > @@ -30,13 +30,19 @@ struct IPU3Action {
> > >  	libcamera.ControlList controls;
> > >  };
> > >  
> > > +struct IPAConfigInfo {
> > > +	libcamera.IPACameraSensorInfo sensorInfo;
> > > +	map<uint32, ControlInfoMap> entityControls;
> > 
> > Should this be libcamera.ControlInfoMap ? Paul, what's the rule for the
> > 'libcamera.' namespace prefix ?
> 
> Here it's optional.
> 
> The code generator doesn't actually need the libcamera prefix anywhere
> (because all generated files are in the libcamera namespace anyway), but
> the mojo parser needs it for top-level types.

What do you mean by "top-level types" ?

> > With this address (if needed),
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > 
> > > +	libcamera.Size bdsOutputSize;
> > > +	libcamera.Size iif;
> > > +};
> > > +
> > >  interface IPAIPU3Interface {
> > >  	init(libcamera.IPASettings settings) => (int32 ret);
> > >  	start() => (int32 ret);
> > >  	stop();
> > >  
> > > -	configure(map<uint32, libcamera.ControlInfoMap> entityControls,
> > > -		  libcamera.Size bdsOutputSize) => ();
> > > +	configure(IPAConfigInfo configInfo) => ();
> > >  
> > >  	mapBuffers(array<libcamera.IPABuffer> buffers);
> > >  	unmapBuffers(array<uint32> ids);
> > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> > > index f5343547..769c24d3 100644
> > > --- a/src/ipa/ipu3/ipu3.cpp
> > > +++ b/src/ipa/ipu3/ipu3.cpp
> > > @@ -43,8 +43,7 @@ public:
> > >  	int start() override;
> > >  	void stop() override {}
> > >  
> > > -	void configure(const std::map<uint32_t, ControlInfoMap> &entityControls,
> > > -		       const Size &bdsOutputSize) override;
> > > +	void configure(const IPAConfigInfo &configInfo) override;
> > >  
> > >  	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> > >  	void unmapBuffers(const std::vector<unsigned int> &ids) override;
> > > @@ -139,13 +138,12 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)
> > >  			    << (int)bdsGrid_.height << " << " << (int)bdsGrid_.block_height_log2 << ")";
> > >  }
> > >  
> > > -void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls,
> > > -			const Size &bdsOutputSize)
> > > +void IPAIPU3::configure(const IPAConfigInfo &configInfo)
> > >  {
> > > -	if (entityControls.empty())
> > > +	if (configInfo.entityControls.empty())
> > >  		return;
> > >  
> > > -	ctrls_ = entityControls.at(0);
> > > +	ctrls_ = configInfo.entityControls.at(0);
> > >  
> > >  	const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);
> > >  	if (itExp == ctrls_.end()) {
> > > @@ -169,10 +167,10 @@ void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls
> > >  
> > >  	params_ = {};
> > >  
> > > -	calculateBdsGrid(bdsOutputSize);
> > > +	calculateBdsGrid(configInfo.bdsOutputSize);
> > >  
> > >  	awbAlgo_ = std::make_unique<IPU3Awb>();
> > > -	awbAlgo_->initialise(params_, bdsOutputSize, bdsGrid_);
> > > +	awbAlgo_->initialise(params_, configInfo.bdsOutputSize, bdsGrid_);
> > >  
> > >  	agcAlgo_ = std::make_unique<IPU3Agc>();
> > >  	agcAlgo_->initialise(bdsGrid_);
> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > index 98c6160f..5b15ca90 100644
> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -633,9 +633,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> > >  		return ret;
> > >  	}
> > >  
> > > -	std::map<uint32_t, ControlInfoMap> entityControls;
> > > -	entityControls.emplace(0, data->cio2_.sensor()->controls());
> > > -	data->ipa_->configure(entityControls, config->imguConfig().bds);
> > > +	ipa::ipu3::IPAConfigInfo configInfo;
> > > +	configInfo.entityControls.emplace(0, data->cio2_.sensor()->controls());
> > > +	configInfo.sensorInfo = sensorInfo;
> > > +	configInfo.bdsOutputSize = config->imguConfig().bds;
> > > +	configInfo.iif = config->imguConfig().iif;
> > > +
> > > +	data->ipa_->configure(configInfo);
> > >  
> > >  	return 0;
> > >  }

-- 
Regards,

Laurent Pinchart