[libcamera-devel] [PATCH v6 7/8] libcamera: raspberrypi: Plumb user transform through to IPA

[Kieran Bingham]

Hi David,

On 02/09/2020 11:44, David Plowman wrote:
> This commit plumbs the user transform from the Raspberry Pi pipeline
> handler through to the IPA. Note that the transform is actually
> handled in the sensor (by setting the h/v flip bits), so the IPAs need
> to understand the orientation of the image they receive.
> 
> Once in the IPA we add it to the CameraMode description, so that it
> becomes automatically available to all the individual control
> algorithms.
> 
> The IPA configure method has to be reordered just a little so as to
> fill in the transform in the camera mode before calling SwitchMode.
> 
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> ---
>  src/ipa/raspberrypi/controller/camera_mode.h  |  4 ++
>  src/ipa/raspberrypi/raspberrypi.cpp           | 48 +++++++++++--------
>  .../pipeline/raspberrypi/raspberrypi.cpp      |  5 +-
>  3 files changed, 35 insertions(+), 22 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/controller/camera_mode.h b/src/ipa/raspberrypi/controller/camera_mode.h
> index 875bab3..920f11b 100644
> --- a/src/ipa/raspberrypi/controller/camera_mode.h
> +++ b/src/ipa/raspberrypi/controller/camera_mode.h
> @@ -6,6 +6,8 @@
>   */
>  #pragma once
>  
> +#include <libcamera/transform.h>
> +
>  // Description of a "camera mode", holding enough information for control
>  // algorithms to adapt their behaviour to the different modes of the camera,
>  // including binning, scaling, cropping etc.
> @@ -33,6 +35,8 @@ struct CameraMode {
>  	double noise_factor;
>  	// line time in nanoseconds
>  	double line_length;
> +	// any camera transform *not* reflected already in the camera tuning
> +	libcamera::Transform transform;
>  };
>  
>  #ifdef __cplusplus
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 4557016..c730326 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -232,6 +232,33 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>  	/* Re-assemble camera mode using the sensor info. */
>  	setMode(sensorInfo);
>  
> +	/*
> +	 * The ipaConfig.data always gives us the user transform first. Note that
> +	 * this will always make the LS table pointer (if present) element 1.
> +	 */
> +	mode_.transform = static_cast<libcamera::Transform>(ipaConfig.data[0]);

I hope Paul's IPC/IDL work will help clarify how data is passed from
Pipeline handler to IPA, so I'll take this as a given for now.


> +
> +	/* Store the lens shading table pointer and handle if available. */
> +	if (ipaConfig.operation & RPI_IPA_CONFIG_LS_TABLE) {
> +		/* Remove any previous table, if there was one. */
> +		if (lsTable_) {
> +			munmap(lsTable_, MAX_LS_GRID_SIZE);
> +			lsTable_ = nullptr;
> +		}
> +
> +		/* Map the LS table buffer into user space (now element 1). */
> +		lsTableHandle_ = FileDescriptor(ipaConfig.data[1]);
> +		if (lsTableHandle_.isValid()) {
> +			lsTable_ = mmap(nullptr, MAX_LS_GRID_SIZE, PROT_READ | PROT_WRITE,
> +					MAP_SHARED, lsTableHandle_.fd(), 0);
> +
> +			if (lsTable_ == MAP_FAILED) {
> +				LOG(IPARPI, Error) << "dmaHeap mmap failure for LS table.";
> +				lsTable_ = nullptr;
> +			}
> +		}
> +	}
> +
>  	/* Pass the camera mode to the CamHelper to setup algorithms. */
>  	helper_->SetCameraMode(mode_);
>  
> @@ -280,27 +307,6 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>  	}
>  
>  	lastMode_ = mode_;
> -
> -	/* Store the lens shading table pointer and handle if available. */
> -	if (ipaConfig.operation & RPI_IPA_CONFIG_LS_TABLE) {
> -		/* Remove any previous table, if there was one. */
> -		if (lsTable_) {
> -			munmap(lsTable_, MAX_LS_GRID_SIZE);
> -			lsTable_ = nullptr;
> -		}
> -
> -		/* Map the LS table buffer into user space. */
> -		lsTableHandle_ = FileDescriptor(ipaConfig.data[0]);
> -		if (lsTableHandle_.isValid()) {
> -			lsTable_ = mmap(nullptr, MAX_LS_GRID_SIZE, PROT_READ | PROT_WRITE,
> -					MAP_SHARED, lsTableHandle_.fd(), 0);
> -
> -			if (lsTable_ == MAP_FAILED) {
> -				LOG(IPARPI, Error) << "dmaHeap mmap failure for LS table.";
> -				lsTable_ = nullptr;
> -			}
> -		}
> -	}
>  }
>  
>  void IPARPi::mapBuffers(const std::vector<IPABuffer> &buffers)
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 29112da..728973a 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1252,6 +1252,9 @@ int RPiCameraData::configureIPA()
>  	entityControls.emplace(0, unicam_[Unicam::Image].dev()->controls());
>  	entityControls.emplace(1, isp_[Isp::Input].dev()->controls());
>  
> +	/* Always send the user transform to the IPA. */
> +	ipaConfig.data = { static_cast<unsigned int>(userTransform_) };
> +
>  	/* Allocate the lens shading table via dmaHeap and pass to the IPA. */
>  	if (!lsTable_.isValid()) {
>  		lsTable_ = dmaHeap_.alloc("ls_grid", MAX_LS_GRID_SIZE);
> @@ -1260,7 +1263,7 @@ int RPiCameraData::configureIPA()
>  
>  		/* Allow the IPA to mmap the LS table via the file descriptor. */
>  		ipaConfig.operation = RPI_IPA_CONFIG_LS_TABLE;
> -		ipaConfig.data = { static_cast<unsigned int>(lsTable_.fd()) };
> +		ipaConfig.data.push_back(static_cast<unsigned int>(lsTable_.fd()));
>  	}
>  
>  	CameraSensorInfo sensorInfo = {};
> 

-- 
Regards
--
Kieran