[libcamera-devel,v4,2/2] libcamera: ipu3: Pass the BDS rectangle at IPA configure call
diff mbox series

Message ID 20210316154023.46033-3-jeanmichel.hautbois@ideasonboard.com
State Accepted
Headers show
Series
  • Prepare IPU3 pipeline to pass BDS to IPA
Related show

Commit Message

Jean-Michel Hautbois March 16, 2021, 3:40 p.m. UTC 
The IPU3 IPA will need the BDS configuration when the AWB/AGC algorithm
will be integrated.
In order to do that, the configure() interface needs to be modified.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

---
v4:
- get back to pipeConfig_ beeing private
v3:
- pass bds size to ipa->configure()
v2:
- rename bds => bdsOutputSize
- use a libcamera::Size instead of libcamera::Rectangle
---
 include/libcamera/ipa/ipu3.mojom     | 2 +-
 src/ipa/ipu3/ipu3.cpp                | 7 +++++--
 src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

Comments

Jacopo Mondi March 16, 2021, 3:50 p.m. UTC | #1 
Hello Jean-Michel

On Tue, Mar 16, 2021 at 04:40:23PM +0100, Jean-Michel Hautbois wrote:
> The IPU3 IPA will need the BDS configuration when the AWB/AGC algorithm
> will be integrated.
> In order to do that, the configure() interface needs to be modified.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> ---
> v4:
> - get back to pipeConfig_ beeing private
> v3:
> - pass bds size to ipa->configure()
> v2:
> - rename bds => bdsOutputSize
> - use a libcamera::Size instead of libcamera::Rectangle
> ---
>  include/libcamera/ipa/ipu3.mojom     | 2 +-
>  src/ipa/ipu3/ipu3.cpp                | 7 +++++--
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
>  3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
> index 6ee11333..5d13e7ea 100644
> --- a/include/libcamera/ipa/ipu3.mojom
> +++ b/include/libcamera/ipa/ipu3.mojom
> @@ -30,7 +30,7 @@ interface IPAIPU3Interface {
>  	start() => (int32 ret);
>  	stop();
>
> -	configure(map<uint32, ControlInfoMap> entityControls) => ();
> +	configure(map<uint32, ControlInfoMap> entityControls, Size bdsOutputSize) => ();
>
>  	mapBuffers(array<IPABuffer> buffers);
>  	unmapBuffers(array<uint32> ids);
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index b63e58be..be73a225 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -35,7 +35,9 @@ public:
>  	int start() override { return 0; }
>  	void stop() override {}
>
> -	void configure(const std::map<uint32_t, ControlInfoMap> &entityControls) override;
> +	void configure(
> +		const std::map<uint32_t, ControlInfoMap> &entityControls,

This fits on the previous line

Please retain my tag!

Thanks
   j

> +		const Size &bdsOutputSize) override;
>
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override;
> @@ -62,7 +64,8 @@ private:
>  	uint32_t maxGain_;
>  };
>
> -void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls)
> +void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls,
> +			[[maybe_unused]] const Size &bdsOutputSize)
>  {
>  	if (entityControls.empty())
>  		return;
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index bb61ef4a..c9e3c5ef 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -635,7 +635,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>
>  	std::map<uint32_t, ControlInfoMap> entityControls;
>  	entityControls.emplace(0, data->cio2_.sensor()->controls());
> -	data->ipa_->configure(entityControls);
> +	data->ipa_->configure(entityControls, config->imguConfig().bds);
>
>  	return 0;
>  }
> --
> 2.27.0
>
Laurent Pinchart March 17, 2021, 2:09 p.m. UTC | #2 
Hi Jean-Michel,

On Tue, Mar 16, 2021 at 04:40:23PM +0100, Jean-Michel Hautbois wrote:
> The IPU3 IPA will need the BDS configuration when the AWB/AGC algorithm
> will be integrated.
> In order to do that, the configure() interface needs to be modified.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> ---
> v4:
> - get back to pipeConfig_ beeing private
> v3:
> - pass bds size to ipa->configure()
> v2:
> - rename bds => bdsOutputSize
> - use a libcamera::Size instead of libcamera::Rectangle
> ---
>  include/libcamera/ipa/ipu3.mojom     | 2 +-
>  src/ipa/ipu3/ipu3.cpp                | 7 +++++--
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
>  3 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
> index 6ee11333..5d13e7ea 100644
> --- a/include/libcamera/ipa/ipu3.mojom
> +++ b/include/libcamera/ipa/ipu3.mojom
> @@ -30,7 +30,7 @@ interface IPAIPU3Interface {
>  	start() => (int32 ret);
>  	stop();
>  
> -	configure(map<uint32, ControlInfoMap> entityControls) => ();
> +	configure(map<uint32, ControlInfoMap> entityControls, Size bdsOutputSize) => ();
>  
>  	mapBuffers(array<IPABuffer> buffers);
>  	unmapBuffers(array<uint32> ids);
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index b63e58be..be73a225 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -35,7 +35,9 @@ public:
>  	int start() override { return 0; }
>  	void stop() override {}
>  
> -	void configure(const std::map<uint32_t, ControlInfoMap> &entityControls) override;
> +	void configure(
> +		const std::map<uint32_t, ControlInfoMap> &entityControls,
> +		const Size &bdsOutputSize) override;

I forgot to mention, this should be

	void configure(const std::map<uint32_t, ControlInfoMap> &entityControls,
		       const Size &bdsOutputSize) override;

to match the usual coding style. The rest looks good.

>  
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override;
> @@ -62,7 +64,8 @@ private:
>  	uint32_t maxGain_;
>  };
>  
> -void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls)
> +void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls,
> +			[[maybe_unused]] const Size &bdsOutputSize)
>  {
>  	if (entityControls.empty())
>  		return;
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index bb61ef4a..c9e3c5ef 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -635,7 +635,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  
>  	std::map<uint32_t, ControlInfoMap> entityControls;
>  	entityControls.emplace(0, data->cio2_.sensor()->controls());
> -	data->ipa_->configure(entityControls);
> +	data->ipa_->configure(entityControls, config->imguConfig().bds);
>  
>  	return 0;
>  }

Patch
diff mbox series 

diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
index 6ee11333..5d13e7ea 100644
--- a/include/libcamera/ipa/ipu3.mojom
+++ b/include/libcamera/ipa/ipu3.mojom
@@ -30,7 +30,7 @@  interface IPAIPU3Interface {
 	start() => (int32 ret);
 	stop();
 
-	configure(map<uint32, ControlInfoMap> entityControls) => ();
+	configure(map<uint32, ControlInfoMap> entityControls, Size bdsOutputSize) => ();
 
 	mapBuffers(array<IPABuffer> buffers);
 	unmapBuffers(array<uint32> ids);
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index b63e58be..be73a225 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -35,7 +35,9 @@  public:
 	int start() override { return 0; }
 	void stop() override {}
 
-	void configure(const std::map<uint32_t, ControlInfoMap> &entityControls) override;
+	void configure(
+		const std::map<uint32_t, ControlInfoMap> &entityControls,
+		const Size &bdsOutputSize) override;
 
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
 	void unmapBuffers(const std::vector<unsigned int> &ids) override;
@@ -62,7 +64,8 @@  private:
 	uint32_t maxGain_;
 };
 
-void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls)
+void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls,
+			[[maybe_unused]] const Size &bdsOutputSize)
 {
 	if (entityControls.empty())
 		return;
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index bb61ef4a..c9e3c5ef 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -635,7 +635,7 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 
 	std::map<uint32_t, ControlInfoMap> entityControls;
 	entityControls.emplace(0, data->cio2_.sensor()->controls());
-	data->ipa_->configure(entityControls);
+	data->ipa_->configure(entityControls, config->imguConfig().bds);
 
 	return 0;
 }