{"id":19507,"url":"https://patchwork.libcamera.org/api/patches/19507/?format=json","web_url":"https://patchwork.libcamera.org/patch/19507/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20240218164908.15921-3-laurent.pinchart@ideasonboard.com>","date":"2024-02-18T16:49:05","name":"[v3,2/5] ipa: rkisp1: Store hardware parameters in IPA context","commit_ref":"528dc21b09cc4c0e202c09677bc742db5a5e484d","pull_url":null,"state":"accepted","archived":false,"hash":"4681ffaba3488223603a2fe995200610fb221b37","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/19507/mbox/","series":[{"id":4171,"url":"https://patchwork.libcamera.org/api/series/4171/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4171","date":"2024-02-18T16:49:03","name":"ipa: rkisp1: Support the i.MX8MP ISP","version":3,"mbox":"https://patchwork.libcamera.org/series/4171/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/19507/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/19507/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 329F7BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 18 Feb 2024 16:49:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E7F7862811;\n\tSun, 18 Feb 2024 17:49:10 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 31ABD62806\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 18 Feb 2024 17:49:09 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CD28D18A2;\n\tSun, 18 Feb 2024 17:49:01 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mEiEle7+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708274942;\n\tbh=8GD33Hd/J+ExySh7bmlvv+a70wTUsR/zgvb6BPVhBQE=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=mEiEle7+XxsbJz93Aqjf5xtQ+ogQS5i8xpT5Dkvbt2ZY19mgeuOytkz0SSDxG/Brx\n\tiwNhMkjvs7uZwKL7aH1vTAqRdJBtQa5F9aNNU/ZkNy0dKynn2xBc3Um9Kno/Rnjr1q\n\tr30c86SmcGW93CJmYtqqG3+CxSuuqej2m4XIQmiI=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Subject":"[PATCH v3 2/5] ipa: rkisp1: Store hardware parameters in IPA context","Date":"Sun, 18 Feb 2024 18:49:05 +0200","Message-ID":"<20240218164908.15921-3-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.43.0","In-Reply-To":"<20240218164908.15921-1-laurent.pinchart@ideasonboard.com>","References":"<20240218164908.15921-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"Versions of the ISP differ in the processing blocks they include, as\nwell as in the implementation of some of those blocks. In particular,\nthey have different numbers of histogram bins oe AE statistics cells.\nThe algorithms take these differences into account by checking the ISP\nversion reported by the driver.\n\nThese checks are currently scattered in multiple places. Centralize them\nin the IPARkISP1::init() function, and store the version-dependent\nhardware parameters in the IPA context, accessible by all algorithms.\n\nWhile at it, drop the IPASessionConfiguration::hw member that stores the\nrevision number, unused by the algorithms. It can be added back laer to\nthe IPAHwSettings structure if needed.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/ipa/rkisp1/algorithms/agc.cpp | 27 +++++++++--------------\n src/ipa/rkisp1/algorithms/agc.h   |  3 ---\n src/ipa/rkisp1/ipa_context.cpp    | 30 +++++++++++++++++++-------\n src/ipa/rkisp1/ipa_context.h      | 12 +++++++----\n src/ipa/rkisp1/rkisp1.cpp         | 36 +++++++++++++++----------------\n 5 files changed, 57 insertions(+), 51 deletions(-)","diff":"diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\nindex f83a9ba1686a..da705b14754c 100644\n--- a/src/ipa/rkisp1/algorithms/agc.cpp\n+++ b/src/ipa/rkisp1/algorithms/agc.cpp\n@@ -59,7 +59,7 @@ static constexpr double kEvGainTarget = 0.5;\n static constexpr double kRelativeLuminanceTarget = 0.4;\n \n Agc::Agc()\n-\t: frameCount_(0), numCells_(0), numHistBins_(0), filteredExposure_(0s)\n+\t: frameCount_(0), filteredExposure_(0s)\n {\n \tsupportsRaw_ = true;\n }\n@@ -81,19 +81,6 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n \tcontext.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure;\n \tcontext.activeState.agc.autoEnabled = !context.configuration.raw;\n \n-\t/*\n-\t * According to the RkISP1 documentation:\n-\t * - versions < V12 have RKISP1_CIF_ISP_AE_MEAN_MAX_V10 entries,\n-\t * - versions >= V12 have RKISP1_CIF_ISP_AE_MEAN_MAX_V12 entries.\n-\t */\n-\tif (context.configuration.hw.revision < RKISP1_V12) {\n-\t\tnumCells_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;\n-\t\tnumHistBins_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;\n-\t} else {\n-\t\tnumCells_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n-\t\tnumHistBins_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12;\n-\t}\n-\n \t/*\n \t * Define the measurement window for AGC as a centered rectangle\n \t * covering 3/4 of the image width and height.\n@@ -186,7 +173,10 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,\n \t/* Produce the luminance histogram. */\n \tparams->meas.hst_config.mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM;\n \t/* Set an average weighted histogram. */\n-\tSpan<uint8_t> weights{ params->meas.hst_config.hist_weight, numHistBins_ };\n+\tSpan<uint8_t> weights{\n+\t\tparams->meas.hst_config.hist_weight,\n+\t\tcontext.hw->numHistogramBins\n+\t};\n \tstd::fill(weights.begin(), weights.end(), 1);\n \t/* Step size can't be less than 3. */\n \tparams->meas.hst_config.histogram_predivider = 4;\n@@ -414,8 +404,11 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n \tconst rkisp1_cif_isp_stat *params = &stats->params;\n \tASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);\n \n-\tSpan<const uint8_t> ae{ params->ae.exp_mean, numCells_ };\n-\tSpan<const uint32_t> hist{ params->hist.hist_bins, numHistBins_ };\n+\tSpan<const uint8_t> ae{ params->ae.exp_mean, context.hw->numAeCells };\n+\tSpan<const uint32_t> hist{\n+\t\tparams->hist.hist_bins,\n+\t\tcontext.hw->numHistogramBins\n+\t};\n \n \tdouble iqMean = measureBrightness(hist);\n \tdouble iqMeanGain = kEvGainTarget * hist.size() / iqMean;\ndiff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\nindex ce8594f393ab..fb82a33fc1bf 100644\n--- a/src/ipa/rkisp1/algorithms/agc.h\n+++ b/src/ipa/rkisp1/algorithms/agc.h\n@@ -50,9 +50,6 @@ private:\n \n \tuint64_t frameCount_;\n \n-\tuint32_t numCells_;\n-\tuint32_t numHistBins_;\n-\n \tutils::Duration filteredExposure_;\n };\n \ndiff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\nindex 9bbf368432fa..070834fa682d 100644\n--- a/src/ipa/rkisp1/ipa_context.cpp\n+++ b/src/ipa/rkisp1/ipa_context.cpp\n@@ -14,6 +14,25 @@\n \n namespace libcamera::ipa::rkisp1 {\n \n+/**\n+ * \\struct IPAHwSettings\n+ * \\brief RkISP1 version-specific hardware parameters\n+ */\n+\n+/**\n+ * \\var IPAHwSettings::numAeCells\n+ * \\brief Number of cells in the AE exposure means grid\n+ *\n+ * \\var IPAHwSettings::numHistogramBins\n+ * \\brief Number of bins in the histogram\n+ *\n+ * \\var IPAHwSettings::numHistogramWeights\n+ * \\brief Number of weights in the histogram grid\n+ *\n+ * \\var IPAHwSettings::numGammaOutSamples\n+ * \\brief Number of samples in the gamma out table\n+ */\n+\n /**\n  * \\struct IPASessionConfiguration\n  * \\brief Session configuration for the IPA module\n@@ -32,14 +51,6 @@ namespace libcamera::ipa::rkisp1 {\n  * \\brief AGC measure window\n  */\n \n-/**\n- * \\var IPASessionConfiguration::hw\n- * \\brief RkISP1-specific hardware information\n- *\n- * \\var IPASessionConfiguration::hw.revision\n- * \\brief Hardware revision of the ISP\n- */\n-\n /**\n  * \\var IPASessionConfiguration::awb\n  * \\brief AWB parameters configuration of the IPA\n@@ -337,6 +348,9 @@ namespace libcamera::ipa::rkisp1 {\n  * \\struct IPAContext\n  * \\brief Global IPA context data shared between all algorithms\n  *\n+ * \\var IPAContext::hw\n+ * \\brief RkISP1 version-specific hardware parameters\n+ *\n  * \\var IPAContext::configuration\n  * \\brief The IPA session configuration, immutable during the session\n  *\ndiff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\nindex b9b2065328d6..10d8f38cad5d 100644\n--- a/src/ipa/rkisp1/ipa_context.h\n+++ b/src/ipa/rkisp1/ipa_context.h\n@@ -20,6 +20,13 @@ namespace libcamera {\n \n namespace ipa::rkisp1 {\n \n+struct IPAHwSettings {\n+\tunsigned int numAeCells;\n+\tunsigned int numHistogramBins;\n+\tunsigned int numHistogramWeights;\n+\tunsigned int numGammaOutSamples;\n+};\n+\n struct IPASessionConfiguration {\n \tstruct {\n \t\tstruct rkisp1_cif_isp_window measureWindow;\n@@ -45,10 +52,6 @@ struct IPASessionConfiguration {\n \t\tSize size;\n \t} sensor;\n \n-\tstruct {\n-\t\trkisp1_cif_isp_version revision;\n-\t} hw;\n-\n \tbool raw;\n };\n \n@@ -143,6 +146,7 @@ struct IPAFrameContext : public FrameContext {\n };\n \n struct IPAContext {\n+\tconst IPAHwSettings *hw;\n \tIPASessionConfiguration configuration;\n \tIPAActiveState activeState;\n \ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex 6544c925ba25..44401af8910d 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -81,12 +81,6 @@ private:\n \n \tControlInfoMap sensorControls_;\n \n-\t/* revision-specific data */\n-\trkisp1_cif_isp_version hwRevision_;\n-\tunsigned int hwHistBinNMax_;\n-\tunsigned int hwGammaOutMaxSamples_;\n-\tunsigned int hwHistogramWeightGridsSize_;\n-\n \t/* Interface to the Camera Helper */\n \tstd::unique_ptr<CameraSensorHelper> camHelper_;\n \n@@ -96,6 +90,20 @@ private:\n \n namespace {\n \n+const IPAHwSettings ipaHwSettingsV10{\n+\tRKISP1_CIF_ISP_AE_MEAN_MAX_V10,\n+\tRKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n+\tRKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n+\tRKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n+};\n+\n+const IPAHwSettings ipaHwSettingsV12{\n+\tRKISP1_CIF_ISP_AE_MEAN_MAX_V12,\n+\tRKISP1_CIF_ISP_HIST_BIN_N_MAX_V12,\n+\tRKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12,\n+\tRKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12,\n+};\n+\n /* List of controls handled by the RkISP1 IPA */\n const ControlInfoMap::Map rkisp1Controls{\n \t{ &controls::AeEnable, ControlInfo(false, true) },\n@@ -111,7 +119,7 @@ const ControlInfoMap::Map rkisp1Controls{\n } /* namespace */\n \n IPARkISP1::IPARkISP1()\n-\t: context_({ {}, {}, { kMaxFrameContexts } })\n+\t: context_({ {}, {}, {}, { kMaxFrameContexts } })\n {\n }\n \n@@ -128,14 +136,10 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n \t/* \\todo Add support for other revisions */\n \tswitch (hwRevision) {\n \tcase RKISP1_V10:\n-\t\thwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;\n-\t\thwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10;\n-\t\thwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10;\n+\t\tcontext_.hw = &ipaHwSettingsV10;\n \t\tbreak;\n \tcase RKISP1_V12:\n-\t\thwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12;\n-\t\thwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12;\n-\t\thwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12;\n+\t\tcontext_.hw = &ipaHwSettingsV12;\n \t\tbreak;\n \tdefault:\n \t\tLOG(IPARkISP1, Error)\n@@ -146,9 +150,6 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n \n \tLOG(IPARkISP1, Debug) << \"Hardware revision is \" << hwRevision;\n \n-\t/* Cache the value to set it in configure. */\n-\thwRevision_ = static_cast<rkisp1_cif_isp_version>(hwRevision);\n-\n \tcamHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);\n \tif (!camHelper_) {\n \t\tLOG(IPARkISP1, Error)\n@@ -232,9 +233,6 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,\n \tcontext_.activeState = {};\n \tcontext_.frameContexts.clear();\n \n-\t/* Set the hardware revision for the algorithms. */\n-\tcontext_.configuration.hw.revision = hwRevision_;\n-\n \tconst IPACameraSensorInfo &info = ipaConfig.sensorInfo;\n \tconst ControlInfo vBlank = sensorControls_.find(V4L2_CID_VBLANK)->second;\n \tcontext_.configuration.sensor.defVBlank = vBlank.def().get<int32_t>();\n","prefixes":["v3","2/5"]}