[libcamera-devel] [PATCH 1/2] ipa: rkisp1: Split queuing of request and parameter filling

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

On Thu, Mar 31, 2022 at 10:27:24PM +0530, Umang Jain via libcamera-devel wrote:
> Queuing of request (i.e. passing of controls to the IPA)
> and filling of the parameters buffer are two separate operations.
> Treat them as such by splitting them into two functions in
> the rkisp1 IPA interface.
> 
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>

It's a bit useless at the moment as queueRequest() is a stub, but it
prepares for the future.

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

> ---
>  include/libcamera/ipa/rkisp1.mojom       |  4 ++--
>  src/ipa/rkisp1/rkisp1.cpp                | 11 ++++++++---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  5 +++--
>  3 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
> index f50f1e11..e3537385 100644
> --- a/include/libcamera/ipa/rkisp1.mojom
> +++ b/include/libcamera/ipa/rkisp1.mojom
> @@ -23,8 +23,8 @@ interface IPARkISP1Interface {
>  	mapBuffers(array<libcamera.IPABuffer> buffers);
>  	unmapBuffers(array<uint32> ids);
>  
> -	[async] queueRequest(uint32 frame, uint32 bufferId,
> -			     libcamera.ControlList reqControls);
> +	[async] queueRequest(uint32 frame, libcamera.ControlList reqControls);
> +	[async] fillParamsBuffer(uint32 frame, uint32 bufferId);
>  	[async] processStatsBuffer(uint32 frame, uint32 bufferId,
>  				   libcamera.ControlList sensorControls);
>  };
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 13ab1cdc..ef1f0d56 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -54,8 +54,8 @@ public:
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override;
>  
> -	void queueRequest(const uint32_t frame, const uint32_t bufferId,
> -			  const ControlList &controls) override;
> +	void queueRequest(const uint32_t frame, const ControlList &controls) override;
> +	void fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) override;
>  	void processStatsBuffer(const uint32_t frame, const uint32_t bufferId,
>  				const ControlList &sensorControls) override;
>  private:
> @@ -235,8 +235,13 @@ void IPARkISP1::unmapBuffers(const std::vector<unsigned int> &ids)
>  	}
>  }
>  
> -void IPARkISP1::queueRequest(const uint32_t frame, const uint32_t bufferId,
> +void IPARkISP1::queueRequest([[maybe_unused]] const uint32_t frame,
>  			     [[maybe_unused]] const ControlList &controls)
> +{
> +	/* \todo Start processing for 'frame' based on 'controls'. */
> +}
> +
> +void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
>  {
>  	rkisp1_params_cfg *params =
>  		reinterpret_cast<rkisp1_params_cfg *>(
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index e6fc582b..1624e2ec 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -855,8 +855,9 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
>  	if (!info)
>  		return -ENOENT;
>  
> -	data->ipa_->queueRequest(data->frame_, info->paramBuffer->cookie(),
> -				 request->controls());
> +	data->ipa_->queueRequest(data->frame_, request->controls());
> +	data->ipa_->fillParamsBuffer(data->frame_, info->paramBuffer->cookie());
> +
>  	data->frame_++;
>  

-- 
Regards,

Laurent Pinchart