@@ -36,6 +36,158 @@ namespace ipa::rkisp1::algorithms {
LOG_DEFINE_CATEGORY(RkISP1Dpcc)
+namespace {
+
+using MethodParamExtract = uint32_t &(*)(struct rkisp1_cif_isp_dpcc_config &config,
+ struct rkisp1_cif_isp_dpcc_methods_config &set);
+
+struct MethodParam {
+ std::string name;
+ unsigned int indexShift;
+ unsigned int redBlueShift;
+ MethodParamExtract param;
+};
+
+struct Method {
+ std::string name;
+ uint32_t enableGreen;
+ uint32_t enableRedBlue;
+ std::vector<MethodParam> params;
+};
+
+static const Method methods[] = {
+ {
+ "pg", /* Peak Gradient */
+ RKISP1_CIF_ISP_DPCC_METHODS_SET_PG_GREEN_ENABLE,
+ RKISP1_CIF_ISP_DPCC_METHODS_SET_PG_RED_BLUE_ENABLE,
+ {
+ {
+ "", 0, 8,
+ []([[maybe_unused]] struct rkisp1_cif_isp_dpcc_config &config,
+ struct rkisp1_cif_isp_dpcc_methods_config &set) -> uint32_t & {
+ return set.pg_fac;
+ },
+ },
+ },
+ }, {
+ "lc", /* Line Check */
+ RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_GREEN_ENABLE,
+ RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_RED_BLUE_ENABLE,
+ {
+ {
+ "threshold", 0, 8,
+ []([[maybe_unused]] struct rkisp1_cif_isp_dpcc_config &config,
+ struct rkisp1_cif_isp_dpcc_methods_config &set) -> uint32_t & {
+ return set.line_thresh;
+ },
+ }, {
+ "mad-factor", 0, 8,
+ []([[maybe_unused]] struct rkisp1_cif_isp_dpcc_config &config,
+ struct rkisp1_cif_isp_dpcc_methods_config &set) -> uint32_t & {
+ return set.line_mad_fac;
+ },
+ },
+ },
+ }, {
+ "ro", /* Rank Order */
+ RKISP1_CIF_ISP_DPCC_METHODS_SET_RO_GREEN_ENABLE,
+ RKISP1_CIF_ISP_DPCC_METHODS_SET_RO_RED_BLUE_ENABLE,
+ {
+ {
+ "", 4, 2,
+ [](struct rkisp1_cif_isp_dpcc_config &config,
+ [[maybe_unused]] struct rkisp1_cif_isp_dpcc_methods_config &set) -> uint32_t & {
+ return config.ro_limits;
+ },
+ },
+ },
+ }, {
+ "rnd", /* Rank Neighbour Difference */
+ RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_GREEN_ENABLE,
+ RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_RED_BLUE_ENABLE,
+ {
+ {
+ "threshold", 0, 8,
+ []([[maybe_unused]] struct rkisp1_cif_isp_dpcc_config &config,
+ struct rkisp1_cif_isp_dpcc_methods_config &set) -> uint32_t & {
+ return set.rnd_thresh;
+ },
+ }, {
+ "offset", 4, 2,
+ [](struct rkisp1_cif_isp_dpcc_config &config,
+ [[maybe_unused]] struct rkisp1_cif_isp_dpcc_methods_config &set) -> uint32_t & {
+ return config.rnd_offs;
+ },
+ },
+ },
+ }, {
+ "rg", /* Rank Gradient */
+ RKISP1_CIF_ISP_DPCC_METHODS_SET_RG_GREEN_ENABLE,
+ RKISP1_CIF_ISP_DPCC_METHODS_SET_RG_RED_BLUE_ENABLE,
+ {
+ {
+ "", 0, 8,
+ []([[maybe_unused]] struct rkisp1_cif_isp_dpcc_config &config,
+ struct rkisp1_cif_isp_dpcc_methods_config &set) -> uint32_t & {
+ return set.rg_fac;
+ },
+ },
+ },
+ },
+};
+
+bool parseMethodParam(const YamlObject &data, unsigned int setIndex,
+ bool redBlue, const MethodParam ¶m,
+ struct rkisp1_cif_isp_dpcc_config &config,
+ struct rkisp1_cif_isp_dpcc_methods_config &set)
+{
+ uint32_t value;
+
+ if (!param.name.empty()) {
+ if (!data.contains(param.name))
+ return false;
+
+ value = data[param.name].get<uint16_t>(0);
+ } else {
+ value = data.get<uint16_t>(0);
+ }
+
+ uint32_t &field = param.param(config, set);
+ unsigned int shift = param.indexShift * setIndex
+ + (redBlue ? param.redBlueShift : 0);
+ field |= value << shift;
+
+ return true;
+}
+
+bool parseMethod(const YamlObject &yamlMethod, const Method &method,
+ unsigned int setIndex, bool redBlue,
+ struct rkisp1_cif_isp_dpcc_config &config,
+ struct rkisp1_cif_isp_dpcc_methods_config &set)
+{
+ const char *color = redBlue ? "red-blue" : "green";
+
+ if (!yamlMethod.contains(color))
+ return true;
+
+ const YamlObject &yamlParams = yamlMethod[color];
+
+ for (const MethodParam ¶m : method.params) {
+ if (!parseMethodParam(yamlParams, setIndex, redBlue, param, config, set)) {
+ LOG(RkISP1Dpcc, Error)
+ << "Failed to parse " << color << " values for "
+ << method.name << "." << param.name
+ << " in set " << setIndex;
+ return false;
+ }
+ }
+
+ set.method |= method.enableGreen;
+ return true;
+}
+
+} /* namespace */
+
DefectPixelClusterCorrection::DefectPixelClusterCorrection()
: initialized_(false), config_({})
{
@@ -55,171 +207,46 @@ int DefectPixelClusterCorrection::init([[maybe_unused]] IPAContext &context,
? RKISP1_CIF_ISP_DPCC_SET_USE_STAGE1_USE_FIX_SET : 0;
/* Get all defined sets to apply (up to 3). */
- const YamlObject &setsObject = tuningData["sets"];
- if (!setsObject.isList()) {
+ const YamlObject &yamlSets = tuningData["sets"];
+ if (!yamlSets.isList()) {
LOG(RkISP1Dpcc, Error)
<< "'sets' parameter not found in tuning file";
return -EINVAL;
}
- if (setsObject.size() > RKISP1_CIF_ISP_DPCC_METHODS_MAX) {
+ if (yamlSets.size() > RKISP1_CIF_ISP_DPCC_METHODS_MAX) {
LOG(RkISP1Dpcc, Error)
- << "'sets' size in tuning file (" << setsObject.size()
+ << "'sets' size in tuning file (" << yamlSets.size()
<< ") exceeds the maximum hardware capacity (3)";
return -EINVAL;
}
- for (std::size_t i = 0; i < setsObject.size(); ++i) {
- struct rkisp1_cif_isp_dpcc_methods_config &method = config_.methods[i];
- const YamlObject &set = setsObject[i];
- uint16_t value;
+ for (std::size_t i = 0; i < yamlSets.size(); ++i) {
+ struct rkisp1_cif_isp_dpcc_methods_config &set = config_.methods[i];
+ const YamlObject &yamlSet = yamlSets[i];
/* Enable set if described in YAML tuning file. */
config_.set_use |= 1 << i;
- /* PG Method */
- const YamlObject &pgObject = set["pg-factor"];
+ /* Parse the methods contained in the set. */
+ for (const Method &method : methods) {
+ if (!yamlSet.contains(method.name))
+ continue;
- if (pgObject.contains("green")) {
- method.method |=
- RKISP1_CIF_ISP_DPCC_METHODS_SET_PG_GREEN_ENABLE;
+ const YamlObject &yamlMethod = yamlSet[method.name];
- value = pgObject["green"].get<uint16_t>(0);
- method.pg_fac |= RKISP1_CIF_ISP_DPCC_PG_FAC_G(value);
- }
-
- if (pgObject.contains("red-blue")) {
- method.method |=
- RKISP1_CIF_ISP_DPCC_METHODS_SET_PG_RED_BLUE_ENABLE;
-
- value = pgObject["red-blue"].get<uint16_t>(0);
- method.pg_fac |= RKISP1_CIF_ISP_DPCC_PG_FAC_RB(value);
- }
-
- /* RO Method */
- const YamlObject &roObject = set["ro-limits"];
-
- if (roObject.contains("green")) {
- method.method |=
- RKISP1_CIF_ISP_DPCC_METHODS_SET_RO_GREEN_ENABLE;
-
- value = roObject["green"].get<uint16_t>(0);
- config_.ro_limits |=
- RKISP1_CIF_ISP_DPCC_RO_LIMITS_n_G(i, value);
- }
-
- if (roObject.contains("red-blue")) {
- method.method |=
- RKISP1_CIF_ISP_DPCC_METHODS_SET_RO_RED_BLUE_ENABLE;
-
- value = roObject["red-blue"].get<uint16_t>(0);
- config_.ro_limits |=
- RKISP1_CIF_ISP_DPCC_RO_LIMITS_n_RB(i, value);
- }
-
- /* RG Method */
- const YamlObject &rgObject = set["rg-factor"];
- method.rg_fac = 0;
-
- if (rgObject.contains("green")) {
- method.method |=
- RKISP1_CIF_ISP_DPCC_METHODS_SET_RG_GREEN_ENABLE;
-
- value = rgObject["green"].get<uint16_t>(0);
- method.rg_fac |= RKISP1_CIF_ISP_DPCC_RG_FAC_G(value);
- }
-
- if (rgObject.contains("red-blue")) {
- method.method |=
- RKISP1_CIF_ISP_DPCC_METHODS_SET_RG_RED_BLUE_ENABLE;
-
- value = rgObject["red-blue"].get<uint16_t>(0);
- method.rg_fac |= RKISP1_CIF_ISP_DPCC_RG_FAC_RB(value);
- }
-
- /* RND Method */
- const YamlObject &rndOffsetsObject = set["rnd-offsets"];
-
- if (rndOffsetsObject.contains("green")) {
- method.method |=
- RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_GREEN_ENABLE;
-
- value = rndOffsetsObject["green"].get<uint16_t>(0);
- config_.rnd_offs |=
- RKISP1_CIF_ISP_DPCC_RND_OFFS_n_G(i, value);
- }
-
- if (rndOffsetsObject.contains("red-blue")) {
- method.method |=
- RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_RED_BLUE_ENABLE;
-
- value = rndOffsetsObject["red-blue"].get<uint16_t>(0);
- config_.rnd_offs |=
- RKISP1_CIF_ISP_DPCC_RND_OFFS_n_RB(i, value);
- }
-
- const YamlObject &rndThresholdObject = set["rnd-threshold"];
- method.rnd_thresh = 0;
-
- if (rndThresholdObject.contains("green")) {
- method.method |=
- RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_GREEN_ENABLE;
+ /* Parse the green and red-blue params. */
+ if (!parseMethod(yamlMethod, method, i, false, config_, set))
+ return -EINVAL;
- value = rndThresholdObject["green"].get<uint16_t>(0);
- method.rnd_thresh |=
- RKISP1_CIF_ISP_DPCC_RND_THRESH_G(value);
+ if (!parseMethod(yamlMethod, method, i, true, config_, set))
+ return -EINVAL;
}
- if (rndThresholdObject.contains("red-blue")) {
- method.method |=
- RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_RED_BLUE_ENABLE;
-
- value = rndThresholdObject["red-blue"].get<uint16_t>(0);
- method.rnd_thresh |=
- RKISP1_CIF_ISP_DPCC_RND_THRESH_RB(value);
- }
-
- /* LC Method */
- const YamlObject &lcThresholdObject = set["line-threshold"];
- method.line_thresh = 0;
-
- if (lcThresholdObject.contains("green")) {
- method.method |=
- RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_GREEN_ENABLE;
-
- value = lcThresholdObject["green"].get<uint16_t>(0);
- method.line_thresh |=
- RKISP1_CIF_ISP_DPCC_LINE_THRESH_G(value);
- }
-
- if (lcThresholdObject.contains("red-blue")) {
- method.method |=
- RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_RED_BLUE_ENABLE;
-
- value = lcThresholdObject["red-blue"].get<uint16_t>(0);
- method.line_thresh |=
- RKISP1_CIF_ISP_DPCC_LINE_THRESH_RB(value);
- }
-
- const YamlObject &lcTMadFactorObject = set["line-mad-factor"];
- method.line_mad_fac = 0;
-
- if (lcTMadFactorObject.contains("green")) {
- method.method |=
- RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_GREEN_ENABLE;
-
- value = lcTMadFactorObject["green"].get<uint16_t>(0);
- method.line_mad_fac |=
- RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_G(value);
- }
-
- if (lcTMadFactorObject.contains("red-blue")) {
- method.method |=
- RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_RED_BLUE_ENABLE;
-
- value = lcTMadFactorObject["red-blue"].get<uint16_t>(0);
- method.line_mad_fac |=
- RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_RB(value);
+ if (!set.method) {
+ LOG(RkISP1Dpcc, Error)
+ << "No valid method specified in set " << i;
+ return -EINVAL;
}
}
@@ -99,62 +99,64 @@ algorithms:
- DefectPixelClusterCorrection:
fixed-set: false
sets:
- # PG, LC, RO, RND, RG
- - line-threshold:
+ - pg:
green: 8
red-blue: 8
- line-mad-factor:
- green: 4
- red-blue: 4
- pg-factor:
- green: 8
- red-blue: 8
- rnd-threshold:
- green: 10
- red-blue: 10
- rg-factor:
- green: 32
- red-blue: 32
- ro-limits:
+ lc:
+ green:
+ threshold: 8
+ mad-factor: 4
+ red-blue:
+ threshold: 8
+ mad-factor: 4
+ ro:
green: 1
red-blue: 1
- rnd-offsets:
- green: 2
- red-blue: 2
- # PG, LC, RO
- - line-threshold:
- green: 24
- red-blue: 32
- line-mad-factor:
- green: 16
- red-blue: 24
- pg-factor:
- green: 6
- red-blue: 8
- ro-limits:
- green: 2
- red-blue: 2
- # PG, LC, RO, RND, RG
- - line-threshold:
+ rnd:
+ green:
+ threshold: 10
+ offset: 2
+ red-blue:
+ threshold: 10
+ offset: 2
+ rg:
green: 32
red-blue: 32
- line-mad-factor:
- green: 4
- red-blue: 4
- pg-factor:
+ - lc:
+ green:
+ threshold: 24
+ mad-factor: 16
+ red-blue:
+ threshold: 32
+ mad-factor: 24
+ pg:
+ green: 6
+ red-blue: 8
+ ro:
+ green: 2
+ red-blue: 2
+ - pg:
green: 10
red-blue: 10
- rnd-threshold:
- green: 6
- red-blue: 8
- rg-factor:
- green: 4
- red-blue: 4
- ro-limits:
+ lc:
+ green:
+ threshold: 32
+ mad-factor: 4
+ red-blue:
+ threshold: 32
+ mad-factor: 4
+ ro:
green: 1
red-blue: 2
- rnd-offsets:
- green: 2
- red-blue: 2
+ rnd:
+ green:
+ threshold: 6
+ offset: 2
+ red-blue:
+ threshold: 8
+ offset: 2
+ rg:
+ green: 4
+ red-blue: 4
- Filter:
...
The DefectPixelClusterCorrection::init() function contains a large manually written piece of tuning data parsing code, with duplication of similar but slightly different sections. This is error-prone as copy and paste errors easily creep in and can be hard to spot. As an attempt to address this issue, replace that code with two generic functions tbat operate over a data table which describes the structure of the tuning data and where it fits in the ISP configuration structure (which maps directly to registers). At the same time, restructure the tuning data to group parameter per method, to organize the data in a set-method-parameter structure. The line-threshold and line-mad-factor parameters are grouped in a "lc" (line check) method, and the rnd-threshold and rnd-offset parameters in a "rnd" (rank neighbour difference) method. The other parameters (pg-factor, rg-factor and ro-limits) are directly specified in a "pg", "rg" or "ro" method respectively without a parameter name for simplicity. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- This patch applies on top of "[PATCH v3 0/5] Add GSL, LSC and DPCC tuning support for rkisp1" and could be squashed with patch 5/5 in that series, of be left separate. A few questions (hence the RFC): - Should methods that have a single parameter (e.g. "pg") have an explicit parameter name in YAML, to match methods that have multiple parameters ? This would result in pg: green: factor: 10 red-blue: factor: 10 instead of pg: green: 10 red-blue: 10 - Should the method names be spelled out fully in YAML, instead of being abbreviated to 2 or 3 letters ? - Is this actually worth it ? --- src/ipa/rkisp1/algorithms/dpcc.cpp | 319 ++++++++++++++++------------- src/ipa/rkisp1/data/ov5640.yaml | 96 ++++----- 2 files changed, 222 insertions(+), 193 deletions(-)