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

13247 diff mbox series

Plumb through actions and events in the VIMC mojo interface.
Events are send from pipeline handler to IPA and actions can be
emitted from IPA and handled in the pipeline handler. The plumbing
ensures that, we can send/retrieve FrameBuffers from the vimc IPA.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 include/libcamera/ipa/vimc.mojom     | 23 ++++++++++++++-
 src/ipa/vimc/vimc.cpp                | 42 +++++++++++++++++++++++++++-
 src/libcamera/pipeline/vimc/vimc.cpp | 27 ++++++++++++++++++
 3 files changed, 90 insertions(+), 2 deletions(-)

Hi Umang,

Thank you for the patch.

On Fri, Aug 06, 2021 at 03:44:09PM +0530, Umang Jain wrote:
> Plumb through actions and events in the VIMC mojo interface.
> Events are send from pipeline handler to IPA and actions can be
> emitted from IPA and handled in the pipeline handler. The plumbing
> ensures that, we can send/retrieve FrameBuffers from the vimc IPA.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  include/libcamera/ipa/vimc.mojom     | 23 ++++++++++++++-
>  src/ipa/vimc/vimc.cpp                | 42 +++++++++++++++++++++++++++-
>  src/libcamera/pipeline/vimc/vimc.cpp | 27 ++++++++++++++++++
>  3 files changed, 90 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom
> index 8452461d..85a87227 100644
> --- a/include/libcamera/ipa/vimc.mojom
> +++ b/include/libcamera/ipa/vimc.mojom
> @@ -17,6 +17,25 @@ enum IPATraceCode {
>  	IPATraceStop,
>  };
>  
> +enum VimcOperations {
> +        ActionParamFilled = 1,
> +        EventFillParams = 2,
> +        EventProcessControls = 3,
> +};

IPA operations are legacy, they come from a time when we didn't have
mojo to generate the IPA interface. Today it's best to create dedicated
functions in the IPA interface.

> +
> +struct VimcEvent {
> +        VimcOperations op;
> +        uint32 frame;
> +        int64 frameTimestamp;
> +        uint32 bufferId;
> +        libcamera.ControlList controls;
> +};
> +
> +struct VimcAction {
> +        VimcOperations op;
> +        libcamera.ControlList controls;
> +};

Same for these data types. You can still create structures to group
related data together, but there's no need anymore to create structures
that group all data that any operation may ever need.

> +
>  interface IPAVimcInterface {
>  	init(libcamera.IPASettings settings) => (int32 ret);
>  
> @@ -29,8 +48,10 @@ interface IPAVimcInterface {
>  
>  	mapBuffers(array<libcamera.IPABuffer> buffers);
>  	unmapBuffers(array<uint32> ids);
> +
> +	[async] processEvent(VimcEvent ev);
>  };
>  
>  interface IPAVimcEventInterface {
> -	dummyEvent(uint32 val);
> +	queueFrameAction(uint32 frame, VimcAction action);
>  };
> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
> index 2b00687f..784d7993 100644
> --- a/src/ipa/vimc/vimc.cpp
> +++ b/src/ipa/vimc/vimc.cpp
> @@ -25,6 +25,8 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(IPAVimc)
>  
> +namespace ipa::vimc {
> +
>  class IPAVimc : public ipa::vimc::IPAVimcInterface
>  {
>  public:
> @@ -43,7 +45,11 @@ public:
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override;
>  
> +	void processEvent(const VimcEvent &event) override;
> +
>  private:
> +	void processControls(unsigned int frame, const ControlList &controls);
> +
>  	void initTrace();
>  	void trace(enum ipa::vimc::IPATraceCode operation);
>  
> @@ -125,6 +131,38 @@ void IPAVimc::unmapBuffers(const std::vector<unsigned int> &ids)
>  	}
>  }
>  
> +void IPAVimc::processControls([[maybe_unused]] unsigned int frame,
> +			      [[maybe_unused]] const ControlList &controls)
> +{
> +        /* \todo Start processing for 'frame' based on 'controls'. */
> +}
> +
> +void IPAVimc::processEvent(const VimcEvent &event)
> +{
> +	switch (event.op) {
> +	case EventFillParams: {
> +		auto it = buffers_.find(event.bufferId);
> +		if (it == buffers_.end()) {
> +			LOG(IPAVimc, Error) << "Could not find param buffer!";
> +			return;
> +		}
> +
> +		/* \todo Fill parameters with actual parameter buffer */
> +		VimcAction op;
> +		op.op = ActionParamFilled;
> +		queueFrameAction.emit(event.frame, op);
> +		break;
> +	}
> +	case EventProcessControls: {
> +		processControls(event.frame, event.controls);
> +		break;
> +	}
> +	default:
> +		LOG(IPAVimc, Error) << "Unknown event " << event.op;
> +		break;
> +	}
> +}
> +
>  void IPAVimc::initTrace()
>  {
>  	struct stat fifoStat;
> @@ -156,6 +194,8 @@ void IPAVimc::trace(enum ipa::vimc::IPATraceCode operation)
>  	}
>  }
>  
> +} /* namespace ipa::vimc */
> +
>  /*
>   * External IPA module interface
>   */
> @@ -170,7 +210,7 @@ const struct IPAModuleInfo ipaModuleInfo = {
>  
>  IPAInterface *ipaCreate()
>  {
> -	return new IPAVimc();
> +	return new ipa::vimc::IPAVimc();
>  }
>  }
>  
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index fa21128d..4b7a542d 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -51,6 +51,8 @@ public:
>  
>  	int init();
>  	void bufferReady(FrameBuffer *buffer);
> +	void queueFrameAction(unsigned int id,
> +			      const ipa::vimc::VimcAction &action);
>  
>  	MediaDevice *media_;
>  	std::unique_ptr<CameraSensor> sensor_;
> @@ -438,6 +440,12 @@ int PipelineHandlerVimc::queueRequestDevice(Camera *camera, Request *request)
>  	if (ret < 0)
>  		return ret;
>  
> +	ipa::vimc::VimcEvent ev;
> +	ev.op = ipa::vimc::EventProcessControls;
> +	ev.frame = request->sequence();
> +	ev.controls = request->controls();
> +	data->ipa_->processEvent(ev);
> +
>  	return 0;
>  }
>  
> @@ -471,6 +479,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>  		return false;
>  	}
>  
> +	data->ipa_->queueFrameAction.connect(data.get(), &VimcCameraData::queueFrameAction);
> +
>  	std::string conf = data->ipa_->configurationFile("vimc.conf");
>  	data->ipa_->init(IPASettings{ conf, data->sensor_->model() });
>  
> @@ -569,6 +579,23 @@ void VimcCameraData::bufferReady(FrameBuffer *buffer)
>  
>  	pipe_->completeBuffer(request, buffer);
>  	pipe_->completeRequest(request);
> +
> +	ipa::vimc::VimcEvent ev;
> +	ev.op = ipa::vimc::EventFillParams;
> +	ev.frame = request->sequence();
> +	ev.bufferId = buffer->cookie();
> +	ipa_->processEvent(ev);
> +}
> +
> +void VimcCameraData::queueFrameAction([[maybe_unused]] unsigned int id,
> +				      const ipa::vimc::VimcAction &action)
> +{
> +	switch (action.op) {
> +	case ipa::vimc::ActionParamFilled:
> +		break;
> +	default:
> +		LOG(VIMC, Error) << "unknown action: " << action.op;
> +	}
>  }
>  
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc)