[libcamera-devel,v3,4/5] libcamera: raspberrypi: Plumb user transform through to IPA

Message ID 20200821155641.11839-5-david.plowman@raspberrypi.com
State Accepted
Headers show
Series
  • 2D transforms
Related show

Commit Message

David Plowman Aug. 21, 2020, 3:56 p.m. UTC
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@raspberrypi.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(-)

Comments

Laurent Pinchart Aug. 23, 2020, 1:55 a.m. UTC | #1
Hi David,

Thank you for the patch.

On Fri, Aug 21, 2020 at 04:56:40PM +0100, 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@raspberrypi.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@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 3747208..0b36d57 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]);
> +
> +	/* 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 a3f8438..4f9aa35 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1159,6 +1159,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>(transform_) };
> +
>  	/* Allocate the lens shading table via dmaHeap and pass to the IPA. */
>  	if (!lsTable_.isValid()) {
>  		lsTable_ = dmaHeap_.alloc("ls_grid", MAX_LS_GRID_SIZE);
> @@ -1167,7 +1170,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 = {};

Patch

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 3747208..0b36d57 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]);
+
+	/* 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 a3f8438..4f9aa35 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -1159,6 +1159,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>(transform_) };
+
 	/* Allocate the lens shading table via dmaHeap and pass to the IPA. */
 	if (!lsTable_.isValid()) {
 		lsTable_ = dmaHeap_.alloc("ls_grid", MAX_LS_GRID_SIZE);
@@ -1167,7 +1170,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 = {};