[libcamera-devel,2/3] ipa: rkisp1: lsc: Enable/disable LSC algorithm
diff mbox series

Message ID 20230308164028.235638-3-jacopo.mondi@ideasonboard.com
State Superseded
Headers show
Series
  • ipa: rkisp1: lsc: Enable/disable LSC
Related show

Commit Message

Jacopo Mondi March 8, 2023, 4:40 p.m. UTC 
Implement LSC algorithm enable/disable by parsing the
"LensShadingEnable" control in queueRequest().

Start with the LSC algorithm enabled by default and disable it
on application request.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/ipa/rkisp1/algorithms/lsc.cpp | 54 ++++++++++++++++++++++++++++++-
 src/ipa/rkisp1/algorithms/lsc.h   |  4 +++
 src/ipa/rkisp1/ipa_context.h      |  5 +++
 src/ipa/rkisp1/rkisp1.cpp         |  1 +
 4 files changed, 63 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart March 12, 2023, 4:01 p.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Wed, Mar 08, 2023 at 05:40:27PM +0100, Jacopo Mondi via libcamera-devel wrote:
> Implement LSC algorithm enable/disable by parsing the
> "LensShadingEnable" control in queueRequest().

I'll assume here that we have use cases for this control, that question
is better discussed in replies to 1/3.

> Start with the LSC algorithm enabled by default and disable it
> on application request.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/lsc.cpp | 54 ++++++++++++++++++++++++++++++-
>  src/ipa/rkisp1/algorithms/lsc.h   |  4 +++
>  src/ipa/rkisp1/ipa_context.h      |  5 +++
>  src/ipa/rkisp1/rkisp1.cpp         |  1 +
>  4 files changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> index a7ccedb1ed3b..0c9f6cbd1dec 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> @@ -14,6 +14,8 @@
>  #include <libcamera/base/log.h>
>  #include <libcamera/base/utils.h>
>  
> +#include <libcamera/control_ids.h>
> +
>  #include "libcamera/internal/yaml_parser.h"
>  
>  #include "linux/rkisp1-config.h"
> @@ -147,6 +149,8 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
>  		return -EINVAL;
>  	}
>  
> +	context.configuration.lsc.enabled = false;
> +
>  	return 0;
>  }
>  
> @@ -181,10 +185,20 @@ int LensShadingCorrection::configure(IPAContext &context,
>  		yGrad_[i] = std::round(32768 / ySizes_[i]);
>  	}
>  
> -	context.configuration.lsc.enabled = true;
> +	/* Enable LSC on first run unless explicitly disabled by application. */
> +	context.activeState.lsc.enable = true;
> +	context.activeState.lsc.active = false;
> +
>  	return 0;
>  }
>  
> +void LensShadingCorrection::disableLSC(rkisp1_params_cfg *params)
> +{
> +	params->module_en_update |= RKISP1_CIF_ISP_MODULE_LSC;
> +	params->module_ens &= ~RKISP1_CIF_ISP_MODULE_LSC;
> +	params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_LSC;
> +}
> +
>  void LensShadingCorrection::setParameters(rkisp1_params_cfg *params)
>  {
>  	struct rkisp1_cif_isp_lsc_config &config = params->others.lsc_config;
> @@ -242,6 +256,25 @@ void LensShadingCorrection::interpolateTable(rkisp1_cif_isp_lsc_config &config,
>  	}
>  }
>  
> +/**
> + * \copydoc libcamera::ipa::Algorithm::queueRequest
> + */
> +void LensShadingCorrection::queueRequest(IPAContext &context,
> +					 [[maybe_unused]] const uint32_t frame,
> +					 [[maybe_unused]] IPAFrameContext &frameContext,
> +					 const ControlList &controls)
> +{
> +	auto lsc = &context.activeState.lsc;
> +
> +	const auto lscEnable = controls.get(controls::LensShadingEnable);
> +	if (lscEnable && *lscEnable != lsc->enable) {
> +		LOG(RkISP1Lsc, Debug)
> +			<< (*lscEnable ? "Enabling" : "Disabling") << " LSC";
> +
> +		lsc->enable = *lscEnable;
> +	}
> +}
> +
>  /**
>   * \copydoc libcamera::ipa::Algorithm::prepare
>   */
> @@ -250,6 +283,25 @@ void LensShadingCorrection::prepare(IPAContext &context,
>  				    [[maybe_unused]] IPAFrameContext &frameContext,
>  				    rkisp1_params_cfg *params)
>  {
> +	auto lsc = &context.activeState.lsc;

This doesn't look right, you will apply the LSC enable control value of
the very last request that was queued too early. You need to use the
frame context.

> +
> +	if (lsc->enable != lsc->active) {
> +		/* If LSC has to be disabled, disable it and return here. */
> +		if (!lsc->enable) {
> +			disableLSC(params);
> +			lsc->active = false;
> +			context.configuration.lsc.enabled = false;
> +			return;
> +		}
> +
> +		lsc->active = true;
> +		context.configuration.lsc.enabled = true;
> +	}
> +
> +	/* Nothing more to do here if LSC is not active. */
> +	if (!lsc->active)
> +		return;
> +
>  	struct rkisp1_cif_isp_lsc_config &config = params->others.lsc_config;
>  
>  	/*
> diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h
> index e2a93a566973..4708065bcfb7 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.h
> +++ b/src/ipa/rkisp1/algorithms/lsc.h
> @@ -23,6 +23,9 @@ public:
>  
>  	int init(IPAContext &context, const YamlObject &tuningData) override;
>  	int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;
> +	void queueRequest(IPAContext &context, const uint32_t frame,
> +			  IPAFrameContext &frameContext,
> +			  const ControlList &controls) override;
>  	void prepare(IPAContext &context, const uint32_t frame,
>  		     IPAFrameContext &frameContext,
>  		     rkisp1_params_cfg *params) override;
> @@ -36,6 +39,7 @@ private:
>  		std::vector<uint16_t> b;
>  	};
>  
> +	void disableLSC(rkisp1_params_cfg *params);
>  	void setParameters(rkisp1_params_cfg *params);
>  	void copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0);
>  	void interpolateTable(rkisp1_cif_isp_lsc_config &config,
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index b9b2065328d6..ada995274ebf 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -98,6 +98,11 @@ struct IPAActiveState {
>  		uint8_t denoise;
>  		uint8_t sharpness;
>  	} filter;
> +
> +	struct {
> +		bool enable;
> +		bool active;
> +	} lsc;
>  };
>  
>  struct IPAFrameContext : public FrameContext {
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 6544c925ba25..2772592eda66 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -106,6 +106,7 @@ const ControlInfoMap::Map rkisp1Controls{
>  	{ &controls::Saturation, ControlInfo(0.0f, 1.993f, 1.0f) },
>  	{ &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },
>  	{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },
> +	{ &controls::LensShadingEnable, ControlInfo(false, true) },

Shouldn't the default control value be true ?

>  };
>  
>  } /* namespace */

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
index a7ccedb1ed3b..0c9f6cbd1dec 100644
--- a/src/ipa/rkisp1/algorithms/lsc.cpp
+++ b/src/ipa/rkisp1/algorithms/lsc.cpp
@@ -14,6 +14,8 @@ 
 #include <libcamera/base/log.h>
 #include <libcamera/base/utils.h>
 
+#include <libcamera/control_ids.h>
+
 #include "libcamera/internal/yaml_parser.h"
 
 #include "linux/rkisp1-config.h"
@@ -147,6 +149,8 @@  int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
 		return -EINVAL;
 	}
 
+	context.configuration.lsc.enabled = false;
+
 	return 0;
 }
 
@@ -181,10 +185,20 @@  int LensShadingCorrection::configure(IPAContext &context,
 		yGrad_[i] = std::round(32768 / ySizes_[i]);
 	}
 
-	context.configuration.lsc.enabled = true;
+	/* Enable LSC on first run unless explicitly disabled by application. */
+	context.activeState.lsc.enable = true;
+	context.activeState.lsc.active = false;
+
 	return 0;
 }
 
+void LensShadingCorrection::disableLSC(rkisp1_params_cfg *params)
+{
+	params->module_en_update |= RKISP1_CIF_ISP_MODULE_LSC;
+	params->module_ens &= ~RKISP1_CIF_ISP_MODULE_LSC;
+	params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_LSC;
+}
+
 void LensShadingCorrection::setParameters(rkisp1_params_cfg *params)
 {
 	struct rkisp1_cif_isp_lsc_config &config = params->others.lsc_config;
@@ -242,6 +256,25 @@  void LensShadingCorrection::interpolateTable(rkisp1_cif_isp_lsc_config &config,
 	}
 }
 
+/**
+ * \copydoc libcamera::ipa::Algorithm::queueRequest
+ */
+void LensShadingCorrection::queueRequest(IPAContext &context,
+					 [[maybe_unused]] const uint32_t frame,
+					 [[maybe_unused]] IPAFrameContext &frameContext,
+					 const ControlList &controls)
+{
+	auto lsc = &context.activeState.lsc;
+
+	const auto lscEnable = controls.get(controls::LensShadingEnable);
+	if (lscEnable && *lscEnable != lsc->enable) {
+		LOG(RkISP1Lsc, Debug)
+			<< (*lscEnable ? "Enabling" : "Disabling") << " LSC";
+
+		lsc->enable = *lscEnable;
+	}
+}
+
 /**
  * \copydoc libcamera::ipa::Algorithm::prepare
  */
@@ -250,6 +283,25 @@  void LensShadingCorrection::prepare(IPAContext &context,
 				    [[maybe_unused]] IPAFrameContext &frameContext,
 				    rkisp1_params_cfg *params)
 {
+	auto lsc = &context.activeState.lsc;
+
+	if (lsc->enable != lsc->active) {
+		/* If LSC has to be disabled, disable it and return here. */
+		if (!lsc->enable) {
+			disableLSC(params);
+			lsc->active = false;
+			context.configuration.lsc.enabled = false;
+			return;
+		}
+
+		lsc->active = true;
+		context.configuration.lsc.enabled = true;
+	}
+
+	/* Nothing more to do here if LSC is not active. */
+	if (!lsc->active)
+		return;
+
 	struct rkisp1_cif_isp_lsc_config &config = params->others.lsc_config;
 
 	/*
diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h
index e2a93a566973..4708065bcfb7 100644
--- a/src/ipa/rkisp1/algorithms/lsc.h
+++ b/src/ipa/rkisp1/algorithms/lsc.h
@@ -23,6 +23,9 @@  public:
 
 	int init(IPAContext &context, const YamlObject &tuningData) override;
 	int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;
+	void queueRequest(IPAContext &context, const uint32_t frame,
+			  IPAFrameContext &frameContext,
+			  const ControlList &controls) override;
 	void prepare(IPAContext &context, const uint32_t frame,
 		     IPAFrameContext &frameContext,
 		     rkisp1_params_cfg *params) override;
@@ -36,6 +39,7 @@  private:
 		std::vector<uint16_t> b;
 	};
 
+	void disableLSC(rkisp1_params_cfg *params);
 	void setParameters(rkisp1_params_cfg *params);
 	void copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0);
 	void interpolateTable(rkisp1_cif_isp_lsc_config &config,
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
index b9b2065328d6..ada995274ebf 100644
--- a/src/ipa/rkisp1/ipa_context.h
+++ b/src/ipa/rkisp1/ipa_context.h
@@ -98,6 +98,11 @@  struct IPAActiveState {
 		uint8_t denoise;
 		uint8_t sharpness;
 	} filter;
+
+	struct {
+		bool enable;
+		bool active;
+	} lsc;
 };
 
 struct IPAFrameContext : public FrameContext {
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 6544c925ba25..2772592eda66 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -106,6 +106,7 @@  const ControlInfoMap::Map rkisp1Controls{
 	{ &controls::Saturation, ControlInfo(0.0f, 1.993f, 1.0f) },
 	{ &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },
 	{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },
+	{ &controls::LensShadingEnable, ControlInfo(false, true) },
 };
 
 } /* namespace */