[{"id":28698,"web_url":"https://patchwork.libcamera.org/comment/28698/","msgid":"","date":"2024-02-20T09:56:11","subject":"Re: [PATCH v3 2/5] ipa: rkisp1: Store hardware parameters in IPA\n\tcontext","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Sun, Feb 18, 2024 at 06:49:05PM +0200, Laurent Pinchart wrote:\n> Versions of the ISP differ in the processing blocks they include, as\n> well as in the implementation of some of those blocks. In particular,\n> they have different numbers of histogram bins oe AE statistics cells.\n> The algorithms take these differences into account by checking the ISP\n> version reported by the driver.\n> \n> These checks are currently scattered in multiple places. Centralize them\n> in the IPARkISP1::init() function, and store the version-dependent\n> hardware parameters in the IPA context, accessible by all algorithms.\n> \n> While at it, drop the IPASessionConfiguration::hw member that stores the\n> revision number, unused by the algorithms. It can be added back laer to\n> the IPAHwSettings structure if needed.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Paul Elder \n\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(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 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 weights{ params->meas.hst_config.hist_weight, numHistBins_ };\n> +\tSpan 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 ae{ params->ae.exp_mean, numCells_ };\n> -\tSpan hist{ params->hist.hist_bins, numHistBins_ };\n> +\tSpan ae{ params->ae.exp_mean, context.hw->numAeCells };\n> +\tSpan 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;\n> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> index 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> \n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index 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> *\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 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> \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 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 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(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();\n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","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 D7D05C3257\n\tfor ;\n\tTue, 20 Feb 2024 09:56:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6350B62807;\n\tTue, 20 Feb 2024 10:56:20 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E05562805\n\tfor ;\n\tTue, 20 Feb 2024 10:56:18 +0100 (CET)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 764151536;\n\tTue, 20 Feb 2024 10:56:10 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"VNosc/yn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708422971;\n\tbh=jKPeFbDDtKsyu9KIyl4gjABacAdBMcloSG6tklQ0sCg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VNosc/ynWNmAXiaaglrl0tC0UGyhmjF6Kqa6PGGXcQzLFZP7t4v9AyyOE6FrMzohG\n\tjYl7cnMqEkFvxur/PoBVbnISL77gBB4tx17Ex1QAB1KwRV0wEOKnm4sIyEOFeECftP\n\tsOjxrzk/JbjcB682/EDWsM0vmyhxfLKqL5IkZWzg=","Date":"Tue, 20 Feb 2024 18:56:11 +0900","From":"Paul Elder ","To":"Laurent Pinchart ","Subject":"Re: [PATCH v3 2/5] ipa: rkisp1: Store hardware parameters in IPA\n\tcontext","Message-ID":"","References":"<20240218164908.15921-1-laurent.pinchart@ideasonboard.com>\n\t<20240218164908.15921-3-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240218164908.15921-3-laurent.pinchart@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28718,"web_url":"https://patchwork.libcamera.org/comment/28718/","msgid":"","date":"2024-02-23T13:05:05","subject":"Re: [PATCH v3 2/5] ipa: rkisp1: Store hardware parameters in IPA\n\tcontext","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Am 18.02.24 um 17:49 schrieb Laurent Pinchart:\n> Versions of the ISP differ in the processing blocks they include, as\n> well as in the implementation of some of those blocks. In particular,\n> they have different numbers of histogram bins oe AE statistics cells.\n> The algorithms take these differences into account by checking the ISP\n> version reported by the driver.\n> \n> These checks are currently scattered in multiple places. Centralize them\n> in the IPARkISP1::init() function, and store the version-dependent\n> hardware parameters in the IPA context, accessible by all algorithms.\n> \n> While at it, drop the IPASessionConfiguration::hw member that stores the\n> revision number, unused by the algorithms. It can be added back laer to\n> the IPAHwSettings structure if needed.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Stefan Klug \n\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(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 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 weights{ params->meas.hst_config.hist_weight, numHistBins_ };\n> +\tSpan 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 ae{ params->ae.exp_mean, numCells_ };\n> -\tSpan hist{ params->hist.hist_bins, numHistBins_ };\n> +\tSpan ae{ params->ae.exp_mean, context.hw->numAeCells };\n> +\tSpan 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;\n> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> index 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> \n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index 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> *\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 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> \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 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 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(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();","headers":{"Return-Path":"","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 55768BD80A\n\tfor ;\n\tFri, 23 Feb 2024 13:05:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 93E4A62809;\n\tFri, 23 Feb 2024 14:05:09 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7F0D061CA1\n\tfor ;\n\tFri, 23 Feb 2024 14:05:08 +0100 (CET)","from [IPV6:2a00:6020:448c:6c00:163d:481a:d9f4:bc3d] (unknown\n\t[IPv6:2a00:6020:448c:6c00:163d:481a:d9f4:bc3d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 219A82E7;\n\tFri, 23 Feb 2024 14:04:59 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"E0vAkoye\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708693499;\n\tbh=niJ0iVIwv+ZUZCf/di1xW2cpN1RqCaS74Dt9b+uzec4=;\n\th=Date:Subject:To:References:Cc:From:In-Reply-To:From;\n\tb=E0vAkoyeuWRGsiBmMIMr9bJ/97V/trmeBIsyY46ogvsJ/oaKjxJd2eYG0CAuY/whV\n\taeTHQ1wpPDxVGlZ2t1lLLQ0ckrjQmustCcPkLwZMcDrg0NdwJ1FWV1M7qtiJGqNyfD\n\tfB1V4hvnStaTEp1EhCWylPbbUeQiTjx1UOfznOUA=","Message-ID":"","Date":"Fri, 23 Feb 2024 14:05:05 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 2/5] ipa: rkisp1: Store hardware parameters in IPA\n\tcontext","Content-Language":"en-US","To":"Laurent Pinchart ","References":"<20240218164908.15921-1-laurent.pinchart@ideasonboard.com>\n\t<20240218164908.15921-3-laurent.pinchart@ideasonboard.com>","From":"Stefan Klug ","In-Reply-To":"<20240218164908.15921-3-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]