[libcamera-devel,v3,4/4] ipa: vimc: Send and retrieve FrameBuffers from IPA
diff mbox series

Message ID 20210814050912.15113-5-umang.jain@ideasonboard.com
State Accepted
Delegated to: Umang Jain
Headers show
Series
  • Pass buffers to VIMC IPA
Related show

Commit Message

Umang Jain Aug. 14, 2021, 5:09 a.m. UTC 
Plumb through VIMC mojo interface to enable buffers passing.
VIMC does not have parameters or statistics buffers but we can
mimick the typical case of passing IPA buffers from pipeline
handler to IPA using mock buffers. The mock IPA buffers are
FrameBuffers which are dmabuf backed (in other words, mmap()able
through MappedFramebuffer inside the IPA).

This commits shows:
- Passing the parameter buffer from the pipeline handler to
  the IPA through functions defined in mojom interface.
- Passing request controls ControlList to the IPA.

Any tests using VIMC will now loop in the IPA paths. Any tests running
in isolated mode will help us to test IPA IPC code paths especially
around (de)serialization of data passing from pipeline handlers to the
IPA. Future IPA interface tests can simply extend the vimc mojom
interface to achieve/test a specific use case as required.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 include/libcamera/ipa/vimc.mojom     | 11 ++++++++++-
 src/ipa/vimc/vimc.cpp                | 19 +++++++++++++++++++
 src/libcamera/pipeline/vimc/vimc.cpp | 11 +++++++++++
 3 files changed, 40 insertions(+), 1 deletion(-)

Comments

Paul Elder Aug. 14, 2021, 5:40 a.m. UTC | #1 
Hi Umang,

On Sat, Aug 14, 2021 at 10:39:12AM +0530, Umang Jain wrote:
> Plumb through VIMC mojo interface to enable buffers passing.
> VIMC does not have parameters or statistics buffers but we can
> mimick the typical case of passing IPA buffers from pipeline
> handler to IPA using mock buffers. The mock IPA buffers are
> FrameBuffers which are dmabuf backed (in other words, mmap()able
> through MappedFramebuffer inside the IPA).
> 
> This commits shows:
> - Passing the parameter buffer from the pipeline handler to
>   the IPA through functions defined in mojom interface.
> - Passing request controls ControlList to the IPA.
> 
> Any tests using VIMC will now loop in the IPA paths. Any tests running
> in isolated mode will help us to test IPA IPC code paths especially
> around (de)serialization of data passing from pipeline handlers to the
> IPA. Future IPA interface tests can simply extend the vimc mojom
> interface to achieve/test a specific use case as required.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  include/libcamera/ipa/vimc.mojom     | 11 ++++++++++-
>  src/ipa/vimc/vimc.cpp                | 19 +++++++++++++++++++
>  src/libcamera/pipeline/vimc/vimc.cpp | 11 +++++++++++
>  3 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom
> index 8cb240d3..e3b14e38 100644
> --- a/include/libcamera/ipa/vimc.mojom
> +++ b/include/libcamera/ipa/vimc.mojom
> @@ -29,8 +29,17 @@ interface IPAVimcInterface {
>  
>  	mapBuffers(array<libcamera.IPABuffer> buffers);
>  	unmapBuffers(array<uint32> ids);
> +
> +	/*
> +	 * The vimc driver doesn't use parameters buffers. To maximize coverage
> +	 * of unit tests that rely on the VIMC pipeline handler, we still define
> +	 * interface functions that mimick how other pipeline handlers typically
> +	 * handle parameters at runtime.
> +	 */
> +	[async] fillParams(uint32 frame, uint32 bufferId);
> +	[async] processControls(uint32 frame, libcamera.ControlList controls);
>  };
>  
>  interface IPAVimcEventInterface {
> -	dummyEvent(uint32 val);
> +	paramsFilled(uint32 bufferId);
>  };
> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
> index 7d9d22d0..8481b82b 100644
> --- a/src/ipa/vimc/vimc.cpp
> +++ b/src/ipa/vimc/vimc.cpp
> @@ -43,6 +43,9 @@ public:
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override;
>  
> +	void fillParams(uint32_t frame, uint32_t bufferId) override;
> +	void processControls(uint32_t frame, const ControlList &controls) override;
> +
>  private:
>  	void initTrace();
>  	void trace(enum ipa::vimc::IPAOperationCode operation);
> @@ -126,6 +129,22 @@ void IPAVimc::unmapBuffers(const std::vector<unsigned int> &ids)
>  	}
>  }
>  
> +void IPAVimc::fillParams([[maybe_unused]] uint32_t frame, uint32_t bufferId)
> +{
> +	auto it = buffers_.find(bufferId);
> +	if (it == buffers_.end()) {
> +		LOG(IPAVimc, Error) << "Could not find parameter buffer";
> +		return;
> +	}
> +
> +	paramsFilled.emit(bufferId);
> +}
> +
> +void IPAVimc::processControls([[maybe_unused]] uint32_t frame,
> +			      [[maybe_unused]] const ControlList &controls)
> +{
> +}
> +
>  void IPAVimc::initTrace()
>  {
>  	struct stat fifoStat;
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index b08325c2..b9fa6227 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -52,6 +52,7 @@ public:
>  	int init();
>  	int allocateMockIPABuffers();
>  	void bufferReady(FrameBuffer *buffer);
> +	void paramsFilled(unsigned int id);
>  
>  	MediaDevice *media_;
>  	std::unique_ptr<CameraSensor> sensor_;
> @@ -432,6 +433,8 @@ int PipelineHandlerVimc::queueRequestDevice(Camera *camera, Request *request)
>  	if (ret < 0)
>  		return ret;
>  
> +	data->ipa_->processControls(request->sequence(), request->controls());
> +
>  	return 0;
>  }
>  
> @@ -465,6 +468,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>  		return false;
>  	}
>  
> +	data->ipa_->paramsFilled.connect(data.get(), &VimcCameraData::paramsFilled);
> +
>  	std::string conf = data->ipa_->configurationFile("vimc.conf");
>  	data->ipa_->init(IPASettings{ conf, data->sensor_->model() });
>  
> @@ -569,6 +574,8 @@ void VimcCameraData::bufferReady(FrameBuffer *buffer)
>  
>  	pipe_->completeBuffer(request, buffer);
>  	pipe_->completeRequest(request);
> +
> +	ipa_->fillParams(request->sequence(), mockIPABufs_[0]->cookie());
>  }
>  
>  int VimcCameraData::allocateMockIPABuffers()
> @@ -586,6 +593,10 @@ int VimcCameraData::allocateMockIPABuffers()
>  	return video_->exportBuffers(kBufCount, &mockIPABufs_);
>  }
>  
> +void VimcCameraData::paramsFilled([[maybe_unused]] unsigned int id)
> +{
> +}
> +
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc)
>  
>  } /* namespace libcamera */
> -- 
> 2.31.1
>
Laurent Pinchart Aug. 14, 2021, 7:45 p.m. UTC | #2 
Hi Umang,

Thank you for the patch.

On Sat, Aug 14, 2021 at 10:39:12AM +0530, Umang Jain wrote:
> Plumb through VIMC mojo interface to enable buffers passing.
> VIMC does not have parameters or statistics buffers but we can
> mimick the typical case of passing IPA buffers from pipeline
> handler to IPA using mock buffers. The mock IPA buffers are
> FrameBuffers which are dmabuf backed (in other words, mmap()able
> through MappedFramebuffer inside the IPA).
> 
> This commits shows:
> - Passing the parameter buffer from the pipeline handler to
>   the IPA through functions defined in mojom interface.
> - Passing request controls ControlList to the IPA.
> 
> Any tests using VIMC will now loop in the IPA paths. Any tests running
> in isolated mode will help us to test IPA IPC code paths especially
> around (de)serialization of data passing from pipeline handlers to the
> IPA. Future IPA interface tests can simply extend the vimc mojom
> interface to achieve/test a specific use case as required.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  include/libcamera/ipa/vimc.mojom     | 11 ++++++++++-
>  src/ipa/vimc/vimc.cpp                | 19 +++++++++++++++++++
>  src/libcamera/pipeline/vimc/vimc.cpp | 11 +++++++++++
>  3 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom
> index 8cb240d3..e3b14e38 100644
> --- a/include/libcamera/ipa/vimc.mojom
> +++ b/include/libcamera/ipa/vimc.mojom
> @@ -29,8 +29,17 @@ interface IPAVimcInterface {
>  
>  	mapBuffers(array<libcamera.IPABuffer> buffers);
>  	unmapBuffers(array<uint32> ids);
> +
> +	/*
> +	 * The vimc driver doesn't use parameters buffers. To maximize coverage
> +	 * of unit tests that rely on the VIMC pipeline handler, we still define
> +	 * interface functions that mimick how other pipeline handlers typically
> +	 * handle parameters at runtime.
> +	 */
> +	[async] fillParams(uint32 frame, uint32 bufferId);
> +	[async] processControls(uint32 frame, libcamera.ControlList controls);
>  };
>  
>  interface IPAVimcEventInterface {
> -	dummyEvent(uint32 val);
> +	paramsFilled(uint32 bufferId);
>  };
> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
> index 7d9d22d0..8481b82b 100644
> --- a/src/ipa/vimc/vimc.cpp
> +++ b/src/ipa/vimc/vimc.cpp
> @@ -43,6 +43,9 @@ public:
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override;
>  
> +	void fillParams(uint32_t frame, uint32_t bufferId) override;
> +	void processControls(uint32_t frame, const ControlList &controls) override;
> +
>  private:
>  	void initTrace();
>  	void trace(enum ipa::vimc::IPAOperationCode operation);
> @@ -126,6 +129,22 @@ void IPAVimc::unmapBuffers(const std::vector<unsigned int> &ids)
>  	}
>  }
>  
> +void IPAVimc::fillParams([[maybe_unused]] uint32_t frame, uint32_t bufferId)
> +{
> +	auto it = buffers_.find(bufferId);
> +	if (it == buffers_.end()) {
> +		LOG(IPAVimc, Error) << "Could not find parameter buffer";
> +		return;
> +	}
> +
> +	paramsFilled.emit(bufferId);
> +}
> +
> +void IPAVimc::processControls([[maybe_unused]] uint32_t frame,
> +			      [[maybe_unused]] const ControlList &controls)
> +{
> +}
> +
>  void IPAVimc::initTrace()
>  {
>  	struct stat fifoStat;
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index b08325c2..b9fa6227 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -52,6 +52,7 @@ public:
>  	int init();
>  	int allocateMockIPABuffers();
>  	void bufferReady(FrameBuffer *buffer);
> +	void paramsFilled(unsigned int id);
>  
>  	MediaDevice *media_;
>  	std::unique_ptr<CameraSensor> sensor_;
> @@ -432,6 +433,8 @@ int PipelineHandlerVimc::queueRequestDevice(Camera *camera, Request *request)
>  	if (ret < 0)
>  		return ret;
>  
> +	data->ipa_->processControls(request->sequence(), request->controls());
> +
>  	return 0;
>  }
>  
> @@ -465,6 +468,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>  		return false;
>  	}
>  
> +	data->ipa_->paramsFilled.connect(data.get(), &VimcCameraData::paramsFilled);
> +
>  	std::string conf = data->ipa_->configurationFile("vimc.conf");
>  	data->ipa_->init(IPASettings{ conf, data->sensor_->model() });
>  
> @@ -569,6 +574,8 @@ void VimcCameraData::bufferReady(FrameBuffer *buffer)
>  
>  	pipe_->completeBuffer(request, buffer);
>  	pipe_->completeRequest(request);
> +
> +	ipa_->fillParams(request->sequence(), mockIPABufs_[0]->cookie());
>  }
>  
>  int VimcCameraData::allocateMockIPABuffers()
> @@ -586,6 +593,10 @@ int VimcCameraData::allocateMockIPABuffers()
>  	return video_->exportBuffers(kBufCount, &mockIPABufs_);
>  }
>  
> +void VimcCameraData::paramsFilled([[maybe_unused]] unsigned int id)
> +{
> +}
> +
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc)
>  
>  } /* namespace libcamera */

Patch
diff mbox series 

diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom
index 8cb240d3..e3b14e38 100644
--- a/include/libcamera/ipa/vimc.mojom
+++ b/include/libcamera/ipa/vimc.mojom
@@ -29,8 +29,17 @@  interface IPAVimcInterface {
 
 	mapBuffers(array<libcamera.IPABuffer> buffers);
 	unmapBuffers(array<uint32> ids);
+
+	/*
+	 * The vimc driver doesn't use parameters buffers. To maximize coverage
+	 * of unit tests that rely on the VIMC pipeline handler, we still define
+	 * interface functions that mimick how other pipeline handlers typically
+	 * handle parameters at runtime.
+	 */
+	[async] fillParams(uint32 frame, uint32 bufferId);
+	[async] processControls(uint32 frame, libcamera.ControlList controls);
 };
 
 interface IPAVimcEventInterface {
-	dummyEvent(uint32 val);
+	paramsFilled(uint32 bufferId);
 };
diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
index 7d9d22d0..8481b82b 100644
--- a/src/ipa/vimc/vimc.cpp
+++ b/src/ipa/vimc/vimc.cpp
@@ -43,6 +43,9 @@  public:
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
 	void unmapBuffers(const std::vector<unsigned int> &ids) override;
 
+	void fillParams(uint32_t frame, uint32_t bufferId) override;
+	void processControls(uint32_t frame, const ControlList &controls) override;
+
 private:
 	void initTrace();
 	void trace(enum ipa::vimc::IPAOperationCode operation);
@@ -126,6 +129,22 @@  void IPAVimc::unmapBuffers(const std::vector<unsigned int> &ids)
 	}
 }
 
+void IPAVimc::fillParams([[maybe_unused]] uint32_t frame, uint32_t bufferId)
+{
+	auto it = buffers_.find(bufferId);
+	if (it == buffers_.end()) {
+		LOG(IPAVimc, Error) << "Could not find parameter buffer";
+		return;
+	}
+
+	paramsFilled.emit(bufferId);
+}
+
+void IPAVimc::processControls([[maybe_unused]] uint32_t frame,
+			      [[maybe_unused]] const ControlList &controls)
+{
+}
+
 void IPAVimc::initTrace()
 {
 	struct stat fifoStat;
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index b08325c2..b9fa6227 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -52,6 +52,7 @@  public:
 	int init();
 	int allocateMockIPABuffers();
 	void bufferReady(FrameBuffer *buffer);
+	void paramsFilled(unsigned int id);
 
 	MediaDevice *media_;
 	std::unique_ptr<CameraSensor> sensor_;
@@ -432,6 +433,8 @@  int PipelineHandlerVimc::queueRequestDevice(Camera *camera, Request *request)
 	if (ret < 0)
 		return ret;
 
+	data->ipa_->processControls(request->sequence(), request->controls());
+
 	return 0;
 }
 
@@ -465,6 +468,8 @@  bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
 		return false;
 	}
 
+	data->ipa_->paramsFilled.connect(data.get(), &VimcCameraData::paramsFilled);
+
 	std::string conf = data->ipa_->configurationFile("vimc.conf");
 	data->ipa_->init(IPASettings{ conf, data->sensor_->model() });
 
@@ -569,6 +574,8 @@  void VimcCameraData::bufferReady(FrameBuffer *buffer)
 
 	pipe_->completeBuffer(request, buffer);
 	pipe_->completeRequest(request);
+
+	ipa_->fillParams(request->sequence(), mockIPABufs_[0]->cookie());
 }
 
 int VimcCameraData::allocateMockIPABuffers()
@@ -586,6 +593,10 @@  int VimcCameraData::allocateMockIPABuffers()
 	return video_->exportBuffers(kBufCount, &mockIPABufs_);
 }
 
+void VimcCameraData::paramsFilled([[maybe_unused]] unsigned int id)
+{
+}
+
 REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc)
 
 } /* namespace libcamera */