| Message ID | 20251007-v4l2-params-v5-5-8db451a81398@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo On 07/10/2025 19:17, Jacopo Mondi wrote: > Implement MaliC55Params to derive from V4L2Params and use the new > helpers in the Mali C55 IPA algorithms implementation. > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > Tested-by: Antoine Bouyer <antoine.bouyer@nxp.com> Couple of nits below, but with or without them changed: Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > src/ipa/mali-c55/algorithms/agc.cpp | 87 +++++++++++++++---------------------- > src/ipa/mali-c55/algorithms/agc.h | 14 +++--- > src/ipa/mali-c55/algorithms/awb.cpp | 64 +++++++++++---------------- > src/ipa/mali-c55/algorithms/awb.h | 10 ++--- > src/ipa/mali-c55/algorithms/blc.cpp | 20 +++------ > src/ipa/mali-c55/algorithms/blc.h | 3 +- > src/ipa/mali-c55/algorithms/lsc.cpp | 58 ++++++++++--------------- > src/ipa/mali-c55/algorithms/lsc.h | 8 ++-- > src/ipa/mali-c55/mali-c55.cpp | 20 +++------ > src/ipa/mali-c55/module.h | 3 +- > src/ipa/mali-c55/params.h | 83 +++++++++++++++++++++++++++++++++++ > 11 files changed, 197 insertions(+), 173 deletions(-) > > diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp > index 78e7e07b2348a711e6261bac45e006f49a59513a..5d60cd8c9ff8a90e2e48e5d6939b538135b7ec3d 100644 > --- a/src/ipa/mali-c55/algorithms/agc.cpp > +++ b/src/ipa/mali-c55/algorithms/agc.cpp > @@ -241,8 +241,8 @@ void Agc::queueRequest(IPAContext &context, const uint32_t frame, > } > } > > -size_t Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext, > - mali_c55_params_block block) > +void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext, > + MaliC55Params *params) > { > IPAActiveState &activeState = context.activeState; > double gain; > @@ -252,52 +252,50 @@ size_t Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContex > else > gain = activeState.agc.manual.ispGain; > > - block.header->type = MALI_C55_PARAM_BLOCK_DIGITAL_GAIN; > - block.header->flags = 0; > - block.header->size = sizeof(struct mali_c55_params_digital_gain); > + auto block = params->block<MaliC55Blocks::Dgain>(); > + block->gain = floatingToFixedPoint<5, 8, uint16_t, double>(gain); > > - block.digital_gain->gain = floatingToFixedPoint<5, 8, uint16_t, double>(gain); > frameContext.agc.ispGain = gain; > - > - return block.header->size; > } > > -size_t Agc::fillParamsBuffer(mali_c55_params_block block, > - enum mali_c55_param_block_type type) > +void Agc::fillParamsBuffer(MaliC55Params *params, enum MaliC55Blocks type) > { > - block.header->type = type; > - block.header->flags = 0; > - block.header->size = sizeof(struct mali_c55_params_aexp_hist); > + > + assert(type == MaliC55Blocks::AexpHist || type == MaliC55Blocks::AexpIhist); > + > + auto block = type == MaliC55Blocks::AexpHist ? > + params->block<MaliC55Blocks::AexpHist>() : > + params->block<MaliC55Blocks::AexpIhist>(); > > /* Collect every 3rd pixel horizontally */ > - block.aexp_hist->skip_x = 1; > + block->skip_x = 1; > /* Start from first column */ > - block.aexp_hist->offset_x = 0; > + block->offset_x = 0; > /* Collect every pixel vertically */ > - block.aexp_hist->skip_y = 0; > + block->skip_y = 0; > /* Start from the first row */ > - block.aexp_hist->offset_y = 0; > + block->offset_y = 0; > /* 1x scaling (i.e. none) */ > - block.aexp_hist->scale_bottom = 0; > - block.aexp_hist->scale_top = 0; > + block->scale_bottom = 0; > + block->scale_top = 0; > /* Collect all Bayer planes into 4 separate histograms */ > - block.aexp_hist->plane_mode = 1; > + block->plane_mode = 1; > /* Tap the data immediately after the digital gain block */ > - block.aexp_hist->tap_point = MALI_C55_AEXP_HIST_TAP_FS; > - > - return block.header->size; > + block->tap_point = MALI_C55_AEXP_HIST_TAP_FS; > } > > -size_t Agc::fillWeightsArrayBuffer(mali_c55_params_block block, > - enum mali_c55_param_block_type type) > +void Agc::fillWeightsArrayBuffer(MaliC55Params *params, const enum MaliC55Blocks type) > { > - block.header->type = type; > - block.header->flags = 0; > - block.header->size = sizeof(struct mali_c55_params_aexp_weights); > + assert(type == MaliC55Blocks::AexpHistWeights || > + type == MaliC55Blocks::AexpIhistWeights); > + > + auto block = type == MaliC55Blocks::AexpHistWeights ? > + params->block<MaliC55Blocks::AexpHistWeights>() : > + params->block<MaliC55Blocks::AexpIhistWeights>(); > > /* We use every zone - a 15x15 grid */ > - block.aexp_weights->nodes_used_horiz = 15; > - block.aexp_weights->nodes_used_vert = 15; > + block->nodes_used_horiz = 15; > + block->nodes_used_vert = 15; > > /* > * We uniformly weight the zones to 1 - this results in the collected > @@ -305,40 +303,25 @@ size_t Agc::fillWeightsArrayBuffer(mali_c55_params_block block, > * approximate colour channel averages for the image. > */ > Span<uint8_t> weights{ > - block.aexp_weights->zone_weights, > + block->zone_weights, > MALI_C55_MAX_ZONES > }; > std::fill(weights.begin(), weights.end(), 1); > - > - return block.header->size; > } > > void Agc::prepare(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameContext, v4l2_isp_params_buffer *params) > + IPAFrameContext &frameContext, MaliC55Params *params) > { > - mali_c55_params_block block; > - > - block.data = ¶ms->data[params->data_size]; > - params->data_size += fillGainParamBlock(context, frameContext, block); > + fillGainParamBlock(context, frameContext, params); > > if (frame > 0) > return; > > - block.data = ¶ms->data[params->data_size]; > - params->data_size += fillParamsBuffer(block, > - MALI_C55_PARAM_BLOCK_AEXP_HIST); > - > - block.data = ¶ms->data[params->data_size]; > - params->data_size += fillWeightsArrayBuffer(block, > - MALI_C55_PARAM_BLOCK_AEXP_HIST_WEIGHTS); > - > - block.data = ¶ms->data[params->data_size]; > - params->data_size += fillParamsBuffer(block, > - MALI_C55_PARAM_BLOCK_AEXP_IHIST); > + fillParamsBuffer(params, MaliC55Blocks::AexpHist); > + fillWeightsArrayBuffer(params, MaliC55Blocks::AexpHistWeights); > > - block.data = ¶ms->data[params->data_size]; > - params->data_size += fillWeightsArrayBuffer(block, > - MALI_C55_PARAM_BLOCK_AEXP_IHIST_WEIGHTS); > + fillParamsBuffer(params, MaliC55Blocks::AexpIhist); > + fillWeightsArrayBuffer(params, MaliC55Blocks::AexpIhistWeights); > } > > double Agc::estimateLuminance(const double gain) const > diff --git a/src/ipa/mali-c55/algorithms/agc.h b/src/ipa/mali-c55/algorithms/agc.h > index 4325ef5a9b7dcef36107b64a65db993f194d4167..9684fff664bc67d287bb00f8dc88e238d8dd0cea 100644 > --- a/src/ipa/mali-c55/algorithms/agc.h > +++ b/src/ipa/mali-c55/algorithms/agc.h > @@ -57,7 +57,7 @@ public: > const ControlList &controls) override; > void prepare(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, > - v4l2_isp_params_buffer *params) override; > + MaliC55Params *params) override; > void process(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, > const mali_c55_stats_buffer *stats, > @@ -65,13 +65,11 @@ public: > > private: > double estimateLuminance(const double gain) const override; > - size_t fillGainParamBlock(IPAContext &context, > - IPAFrameContext &frameContext, > - mali_c55_params_block block); > - size_t fillParamsBuffer(mali_c55_params_block block, > - enum mali_c55_param_block_type type); > - size_t fillWeightsArrayBuffer(mali_c55_params_block block, > - enum mali_c55_param_block_type type); > + void fillGainParamBlock(IPAContext &context, > + IPAFrameContext &frameContext, > + MaliC55Params *params); > + void fillParamsBuffer(MaliC55Params *params, enum MaliC55Blocks type); > + void fillWeightsArrayBuffer(MaliC55Params *params, enum MaliC55Blocks type); > > AgcStatistics statistics_; > }; > diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp > index 694c0aaa9c6804bb58e380ba9c744f11c39224fe..964e810882a93cce02f991675d74bbf163d51e7c 100644 > --- a/src/ipa/mali-c55/algorithms/awb.cpp > +++ b/src/ipa/mali-c55/algorithms/awb.cpp > @@ -43,13 +43,9 @@ int Awb::configure([[maybe_unused]] IPAContext &context, > return 0; > } > > -size_t Awb::fillGainsParamBlock(mali_c55_params_block block, IPAContext &context, > +void Awb::fillGainsParamBlock(MaliC55Params *params, IPAContext &context, > IPAFrameContext &frameContext) > { > - block.header->type = MALI_C55_PARAM_BLOCK_AWB_GAINS; > - block.header->flags = 0; > - block.header->size = sizeof(struct mali_c55_params_awb_gains); > - > double rGain = context.activeState.awb.rGain; > double bGain = context.activeState.awb.bGain; > > @@ -63,34 +59,32 @@ size_t Awb::fillGainsParamBlock(mali_c55_params_block block, IPAContext &context > * This holds true regardless of the bayer order of the input data, as > * the mapping is done internally in the ISP. > */ > - block.awb_gains->gain00 = floatingToFixedPoint<4, 8, uint16_t, double>(rGain); > - block.awb_gains->gain01 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0); > - block.awb_gains->gain10 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0); > - block.awb_gains->gain11 = floatingToFixedPoint<4, 8, uint16_t, double>(bGain); > + auto block = params->block<MaliC55Blocks::AwbGains>(); > + > + block->gain00 = floatingToFixedPoint<4, 8, uint16_t, double>(rGain); > + block->gain01 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0); > + block->gain10 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0); > + block->gain11 = floatingToFixedPoint<4, 8, uint16_t, double>(bGain); > > frameContext.awb.rGain = rGain; > frameContext.awb.bGain = bGain; > - > - return sizeof(struct mali_c55_params_awb_gains); > } > > -size_t Awb::fillConfigParamBlock(mali_c55_params_block block) > +void Awb::fillConfigParamBlock(MaliC55Params *params) > { > - block.header->type = MALI_C55_PARAM_BLOCK_AWB_CONFIG; > - block.header->flags = 0; > - block.header->size = sizeof(struct mali_c55_params_awb_config); > + auto block = params->block<MaliC55Blocks::AwbConfig>(); > > /* Tap the stats after the purple fringe block */ > - block.awb_config->tap_point = MALI_C55_AWB_STATS_TAP_PF; > + block->tap_point = MALI_C55_AWB_STATS_TAP_PF; > > /* Get R/G and B/G ratios as statistics */ > - block.awb_config->stats_mode = MALI_C55_AWB_MODE_RGBG; > + block->stats_mode = MALI_C55_AWB_MODE_RGBG; > > /* Default white level */ > - block.awb_config->white_level = 1023; > + block->white_level = 1023; > > /* Default black level */ > - block.awb_config->black_level = 0; > + block->black_level = 0; > > /* > * By default pixels are included who's colour ratios are bounded in a > @@ -104,40 +98,34 @@ size_t Awb::fillConfigParamBlock(mali_c55_params_block block) > * > * \todo should these perhaps be tunable? > */ > - block.awb_config->cr_max = 511; > - block.awb_config->cr_min = 64; > - block.awb_config->cb_max = 511; > - block.awb_config->cb_min = 64; > + block->cr_max = 511; > + block->cr_min = 64; > + block->cb_max = 511; > + block->cb_min = 64; > > /* We use the full 15x15 zoning scheme */ > - block.awb_config->nodes_used_horiz = 15; > - block.awb_config->nodes_used_vert = 15; > + block->nodes_used_horiz = 15; > + block->nodes_used_vert = 15; > > /* > * We set the trimming boundaries equivalent to the main boundaries. In > * other words; no trimming. > */ > - block.awb_config->cr_high = 511; > - block.awb_config->cr_low = 64; > - block.awb_config->cb_high = 511; > - block.awb_config->cb_low = 64; > - > - return sizeof(struct mali_c55_params_awb_config); > + block->cr_high = 511; > + block->cr_low = 64; > + block->cb_high = 511; > + block->cb_low = 64; > } > > void Awb::prepare(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameContext, v4l2_isp_params_buffer *params) > + IPAFrameContext &frameContext, MaliC55Params *params) > { > - mali_c55_params_block block; > - block.data = ¶ms->data[params->data_size]; > - > - params->data_size += fillGainsParamBlock(block, context, frameContext); > + fillGainsParamBlock(params, context, frameContext); > > if (frame > 0) > return; > > - block.data = ¶ms->data[params->data_size]; > - params->data_size += fillConfigParamBlock(block); > + fillConfigParamBlock(params); > } > > void Awb::process(IPAContext &context, const uint32_t frame, > diff --git a/src/ipa/mali-c55/algorithms/awb.h b/src/ipa/mali-c55/algorithms/awb.h > index 647525ff700e8281e3cce63e64ba56a91294bcc2..683a62af263a14d2f5d5b261448953ada6669b2f 100644 > --- a/src/ipa/mali-c55/algorithms/awb.h > +++ b/src/ipa/mali-c55/algorithms/awb.h > @@ -22,17 +22,17 @@ public: > const IPACameraSensorInfo &configInfo) override; > void prepare(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, > - v4l2_isp_params_buffer *params) override; > + MaliC55Params *params) override; > void process(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, > const mali_c55_stats_buffer *stats, > ControlList &metadata) override; > > private: > - size_t fillGainsParamBlock(mali_c55_params_block block, > - IPAContext &context, > - IPAFrameContext &frameContext); > - size_t fillConfigParamBlock(mali_c55_params_block block); > + void fillGainsParamBlock(MaliC55Params *params, > + IPAContext &context, > + IPAFrameContext &frameContext); > + void fillConfigParamBlock(MaliC55Params *params); > }; > > } /* namespace ipa::mali_c55::algorithms */ > diff --git a/src/ipa/mali-c55/algorithms/blc.cpp b/src/ipa/mali-c55/algorithms/blc.cpp > index 543ba96cb57ac0cca2b8f822180d8d8b42f21fc7..d099219c3e43ec96fa452ed13fa46ada2025edc9 100644 > --- a/src/ipa/mali-c55/algorithms/blc.cpp > +++ b/src/ipa/mali-c55/algorithms/blc.cpp > @@ -85,27 +85,19 @@ int BlackLevelCorrection::configure(IPAContext &context, > void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context, > const uint32_t frame, > [[maybe_unused]] IPAFrameContext &frameContext, > - v4l2_isp_params_buffer *params) > + MaliC55Params *params) > { > - mali_c55_params_block block; > - block.data = ¶ms->data[params->data_size]; > - > if (frame > 0) > return; > > if (!tuningParameters_) > return; > > - block.header->type = MALI_C55_PARAM_BLOCK_SENSOR_OFFS; > - block.header->flags = 0; > - block.header->size = sizeof(mali_c55_params_sensor_off_preshading); > - > - block.sensor_offs->chan00 = offset00; > - block.sensor_offs->chan01 = offset01; > - block.sensor_offs->chan10 = offset10; > - block.sensor_offs->chan11 = offset11; > - > - params->data_size += block.header->size; > + auto block = params->block<MaliC55Blocks::Bls>(); > + block->chan00 = offset00; > + block->chan01 = offset01; > + block->chan10 = offset10; > + block->chan11 = offset11; > } > > void BlackLevelCorrection::process([[maybe_unused]] IPAContext &context, > diff --git a/src/ipa/mali-c55/algorithms/blc.h b/src/ipa/mali-c55/algorithms/blc.h > index ee6c889ed89ac2230b231cb58a9fe4412c0ce164..fc5a7ea310cbdbcf271eb26c73b17243aea744cd 100644 > --- a/src/ipa/mali-c55/algorithms/blc.h > +++ b/src/ipa/mali-c55/algorithms/blc.h > @@ -6,6 +6,7 @@ > */ > > #include "algorithm.h" > +#include "params.h" > > namespace libcamera { > > @@ -22,7 +23,7 @@ public: > const IPACameraSensorInfo &configInfo) override; > void prepare(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, > - v4l2_isp_params_buffer *params) override; > + MaliC55Params *params) override; > void process(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, > const mali_c55_stats_buffer *stats, > diff --git a/src/ipa/mali-c55/algorithms/lsc.cpp b/src/ipa/mali-c55/algorithms/lsc.cpp > index cb915c5efd3c22952035f2b03ee659f293942ec9..5b042c757bc70fe4eb4a5aaa848266b3afce9100 100644 > --- a/src/ipa/mali-c55/algorithms/lsc.cpp > +++ b/src/ipa/mali-c55/algorithms/lsc.cpp > @@ -108,41 +108,33 @@ int Lsc::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData > return 0; > } > > -size_t Lsc::fillConfigParamsBlock(mali_c55_params_block block) const > +void Lsc::fillConfigParamsBlock(MaliC55Params *params) const > { > - block.header->type = MALI_C55_PARAM_MESH_SHADING_CONFIG; > - block.header->flags = 0; > - block.header->size = sizeof(struct mali_c55_params_mesh_shading_config); > + auto block = params->block<MaliC55Blocks::MeshShadingConfig>(); > > - block.shading_config->mesh_show = false; > - block.shading_config->mesh_scale = meshScale_; > - block.shading_config->mesh_page_r = 0; > - block.shading_config->mesh_page_g = 1; > - block.shading_config->mesh_page_b = 2; > - block.shading_config->mesh_width = meshSize_; > - block.shading_config->mesh_height = meshSize_; > + block->mesh_show = false; > + block->mesh_scale = meshScale_; > + block->mesh_page_r = 0; > + block->mesh_page_g = 1; > + block->mesh_page_b = 2; > + block->mesh_width = meshSize_; > + block->mesh_height = meshSize_; > > - std::copy(mesh_.begin(), mesh_.end(), block.shading_config->mesh); > - > - return block.header->size; > + std::copy(mesh_.begin(), mesh_.end(), block->mesh); > } > > -size_t Lsc::fillSelectionParamsBlock(mali_c55_params_block block, uint8_t bank, > +void Lsc::fillSelectionParamsBlock(MaliC55Params *params, uint8_t bank, > uint8_t alpha) const > { > - block.header->type = MALI_C55_PARAM_MESH_SHADING_SELECTION; > - block.header->flags = 0; > - block.header->size = sizeof(struct mali_c55_params_mesh_shading_selection); > - > - block.shading_selection->mesh_alpha_bank_r = bank; > - block.shading_selection->mesh_alpha_bank_g = bank; > - block.shading_selection->mesh_alpha_bank_b = bank; > - block.shading_selection->mesh_alpha_r = alpha; > - block.shading_selection->mesh_alpha_g = alpha; > - block.shading_selection->mesh_alpha_b = alpha; > - block.shading_selection->mesh_strength = 0x1000; /* Otherwise known as 1.0 */ > - > - return block.header->size; > + auto block = params->block<MaliC55Blocks::MeshShadingSel>(); > + > + block->mesh_alpha_bank_r = bank; > + block->mesh_alpha_bank_g = bank; > + block->mesh_alpha_bank_b = bank; > + block->mesh_alpha_r = alpha; > + block->mesh_alpha_g = alpha; > + block->mesh_alpha_b = alpha; > + block->mesh_strength = 0x1000; /* Otherwise known as 1.0 */ > } > > std::tuple<uint8_t, uint8_t> Lsc::findBankAndAlpha(uint32_t ct) const > @@ -170,7 +162,7 @@ std::tuple<uint8_t, uint8_t> Lsc::findBankAndAlpha(uint32_t ct) const > > void Lsc::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame, > [[maybe_unused]] IPAFrameContext &frameContext, > - v4l2_isp_params_buffer *params) > + MaliC55Params *params) > { > /* > * For each frame we assess the colour temperature of the **last** frame > @@ -193,10 +185,7 @@ void Lsc::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame, > std::tie(bank, alpha) = findBankAndAlpha(temperatureK); > } > > - mali_c55_params_block block; > - block.data = ¶ms->data[params->data_size]; > - > - params->data_size += fillSelectionParamsBlock(block, bank, alpha); > + fillSelectionParamsBlock(params, bank, alpha); > > if (frame > 0) > return; > @@ -205,8 +194,7 @@ void Lsc::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame, > * If this is the first frame, we need to load the parsed coefficient > * tables from tuning data to the ISP. > */ > - block.data = ¶ms->data[params->data_size]; > - params->data_size += fillConfigParamsBlock(block); > + fillConfigParamsBlock(params); > } > > REGISTER_IPA_ALGORITHM(Lsc, "Lsc") > diff --git a/src/ipa/mali-c55/algorithms/lsc.h b/src/ipa/mali-c55/algorithms/lsc.h > index c287900502528ca82ab6c62d732b6ffc28ad8df6..e7092bc74a0b0301463167d16e1e888696547d10 100644 > --- a/src/ipa/mali-c55/algorithms/lsc.h > +++ b/src/ipa/mali-c55/algorithms/lsc.h > @@ -23,15 +23,15 @@ public: > int init(IPAContext &context, const YamlObject &tuningData) override; > void prepare(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, > - v4l2_isp_params_buffer *params) override; > + MaliC55Params *params) override; > private: > static constexpr unsigned int kRedOffset = 0; > static constexpr unsigned int kGreenOffset = 1024; > static constexpr unsigned int kBlueOffset = 2048; > > - size_t fillConfigParamsBlock(mali_c55_params_block block) const; > - size_t fillSelectionParamsBlock(mali_c55_params_block block, > - uint8_t bank, uint8_t alpha) const; > + void fillConfigParamsBlock(MaliC55Params *params) const; > + void fillSelectionParamsBlock(MaliC55Params *params, > + uint8_t bank, uint8_t alpha) const; > std::tuple<uint8_t, uint8_t> findBankAndAlpha(uint32_t ct) const; > > std::vector<uint32_t> mesh_ = std::vector<uint32_t>(3072); > diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp > index 504d95222074a35b033949a25178f998738e5699..fe36d4445c3ac75abe48dbe2335fa1f3aded9609 100644 > --- a/src/ipa/mali-c55/mali-c55.cpp > +++ b/src/ipa/mali-c55/mali-c55.cpp > @@ -28,6 +28,7 @@ > #include "libipa/camera_sensor_helper.h" > > #include "ipa_context.h" > +#include "params.h" > > namespace libcamera { > > @@ -331,24 +332,13 @@ void IPAMaliC55::queueRequest(const uint32_t request, const ControlList &control > void IPAMaliC55::fillParams(unsigned int request, > [[maybe_unused]] uint32_t bufferId) > { > - struct v4l2_isp_params_buffer *params; > IPAFrameContext &frameContext = context_.frameContexts.get(request); > + MaliC55Params params(buffers_.at(bufferId).planes()[0]); > > - params = reinterpret_cast<v4l2_isp_params_buffer *>( > - buffers_.at(bufferId).planes()[0].data()); > - memset(params, 0, > - buffers_.at(bufferId).planes()[0].size()); > - > - params->version = MALI_C55_PARAM_BUFFER_V1; > - > - for (auto const &algo : algorithms()) { > - algo->prepare(context_, request, frameContext, params); > - > - ASSERT(params->data_size <= MALI_C55_PARAMS_MAX_SIZE); > - } > + for (auto const &algo : algorithms()) > + algo->prepare(context_, request, frameContext, ¶ms); > > - size_t bytesused = offsetof(struct v4l2_isp_params_buffer, data) + params->data_size; > - paramsComputed.emit(request, bytesused); > + paramsComputed.emit(request, params.size()); I think this reinforces my view that size() should be called used() (or even maybe bytesused!)> } > > void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId, > diff --git a/src/ipa/mali-c55/module.h b/src/ipa/mali-c55/module.h > index a8dcb20841a4ebd7bde22cae4ff1f8eef530d8bb..13b34eb2839530a3518340165e6ca895d0f6bcaf 100644 > --- a/src/ipa/mali-c55/module.h > +++ b/src/ipa/mali-c55/module.h > @@ -14,13 +14,14 @@ > #include <libipa/module.h> > > #include "ipa_context.h" > +#include "params.h" > > namespace libcamera { > > namespace ipa::mali_c55 { > > using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo, > - v4l2_isp_params_buffer, mali_c55_stats_buffer>; > + MaliC55Params, mali_c55_stats_buffer>; > > } /* namespace ipa::mali_c55 */ > > diff --git a/src/ipa/mali-c55/params.h b/src/ipa/mali-c55/params.h > new file mode 100644 > index 0000000000000000000000000000000000000000..bb26da19a8f1336eeed91989a6c13e7630c6e5f4 > --- /dev/null > +++ b/src/ipa/mali-c55/params.h > @@ -0,0 +1,83 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board > + * > + * Mali C55 ISP Parameters > + */ > + > +#pragma once > + > +#include <linux/mali-c55-config.h> > +#include <linux/videodev2.h> > + > +#include <libipa/v4l2_params.h> > + > +namespace libcamera { > + > +namespace ipa::mali_c55 { > + > +enum class MaliC55Blocks { > + Bls, > + AexpHist, > + AexpHistWeights, > + AexpIhist, > + AexpIhistWeights, > + Dgain, > + AwbGains, > + AwbConfig, > + MeshShadingConfig, > + MeshShadingSel, > +}; > + > +namespace details { > + > +template<MaliC55Blocks B> > +struct block_type { > +}; > + > +#define MALI_C55_DEFINE_BLOCK_TYPE(id, cfgType, blkType) \ > +template<> \ > +struct block_type<MaliC55Blocks::id> { \ > + using type = struct mali_c55_params_##cfgType; \ > + static constexpr mali_c55_param_block_type blockType = \ > + mali_c55_param_block_type::MALI_C55_PARAM_##blkType; \ > +}; I think dropping the semicolon here and having it on the ends of the lines below would be more in line with what I'd expect to see. > + > +MALI_C55_DEFINE_BLOCK_TYPE(Bls, sensor_off_preshading, BLOCK_SENSOR_OFFS) > +MALI_C55_DEFINE_BLOCK_TYPE(AexpHist, aexp_hist, BLOCK_AEXP_HIST) > +MALI_C55_DEFINE_BLOCK_TYPE(AexpHistWeights, aexp_weights, > + BLOCK_AEXP_HIST_WEIGHTS) > +MALI_C55_DEFINE_BLOCK_TYPE(AexpIhist, aexp_hist, BLOCK_AEXP_IHIST) > +MALI_C55_DEFINE_BLOCK_TYPE(AexpIhistWeights, aexp_weights, > + BLOCK_AEXP_IHIST_WEIGHTS) > +MALI_C55_DEFINE_BLOCK_TYPE(Dgain, digital_gain, BLOCK_DIGITAL_GAIN) > +MALI_C55_DEFINE_BLOCK_TYPE(AwbGains, awb_gains, BLOCK_AWB_GAINS) > +MALI_C55_DEFINE_BLOCK_TYPE(AwbConfig, awb_config, BLOCK_AWB_CONFIG) > +MALI_C55_DEFINE_BLOCK_TYPE(MeshShadingConfig, mesh_shading_config, > + MESH_SHADING_CONFIG) > +MALI_C55_DEFINE_BLOCK_TYPE(MeshShadingSel, mesh_shading_selection, > + MESH_SHADING_SELECTION) Personally I think it would be better to format these so that the fields for each entry are more visibly associated: MALI_C55_DEFINE_BLOCK_TYPE(Bls, sensor_off_preshading, BLOCK_SENSOR_OFFS); MALI_C55_DEFINE_BLOCK_TYPE(AexpHist, aexp_hist, BLOCK_AEXP_HIST); MALI_C55_DEFINE_BLOCK_TYPE(AexpHistWeights, aexp_weights, BLOCK_AEXP_HIST_WEIGHTS); MALI_C55_DEFINE_BLOCK_TYPE(AexpIhist, aexp_hist, BLOCK_AEXP_IHIST); MALI_C55_DEFINE_BLOCK_TYPE(AexpIhistWeights, aexp_weights, BLOCK_AEXP_IHIST_WEIGHTS); MALI_C55_DEFINE_BLOCK_TYPE(Dgain, digital_gain, BLOCK_DIGITAL_GAIN); MALI_C55_DEFINE_BLOCK_TYPE(AwbGains, awb_gains, BLOCK_AWB_GAINS); MALI_C55_DEFINE_BLOCK_TYPE(AwbConfig, awb_config, BLOCK_AWB_CONFIG); MALI_C55_DEFINE_BLOCK_TYPE(MeshShadingConfig, mesh_shading_config, MESH_SHADING_CONFIG) MALI_C55_DEFINE_BLOCK_TYPE(MeshShadingSel, mesh_shading_selection, MESH_SHADING_SELECTION); It's longer, but I think it's a much easier to read style for this kind of thing. Thanks Dan > + > +struct param_traits { > + using id_type = MaliC55Blocks; > + > + template<id_type Id> > + using id_to_details = block_type<Id>; > +}; > + > +} /* namespace details */ > + > +class MaliC55Params : public V4L2Params<details::param_traits> > +{ > +public: > + static constexpr unsigned int kVersion = MALI_C55_PARAM_BUFFER_V1; > + > + MaliC55Params(Span<uint8_t> data) > + : V4L2Params(data, kVersion) > + { > + } > +}; > + > +} /* namespace ipa::mali_c55 */ > + > +} /* namespace libcamera */ >
Hi Dan On Fri, Oct 10, 2025 at 04:16:37PM +0100, Dan Scally wrote: > Hi Jacopo > > On 07/10/2025 19:17, Jacopo Mondi wrote: > > Implement MaliC55Params to derive from V4L2Params and use the new > > helpers in the Mali C55 IPA algorithms implementation. > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > Tested-by: Antoine Bouyer <antoine.bouyer@nxp.com> > > Couple of nits below, but with or without them changed: > > Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> Thanks > > > --- > > src/ipa/mali-c55/algorithms/agc.cpp | 87 +++++++++++++++---------------------- > > src/ipa/mali-c55/algorithms/agc.h | 14 +++--- [snip] > > --- a/src/ipa/mali-c55/mali-c55.cpp > > +++ b/src/ipa/mali-c55/mali-c55.cpp > > @@ -28,6 +28,7 @@ > > #include "libipa/camera_sensor_helper.h" > > #include "ipa_context.h" > > +#include "params.h" > > namespace libcamera { > > @@ -331,24 +332,13 @@ void IPAMaliC55::queueRequest(const uint32_t request, const ControlList &control > > void IPAMaliC55::fillParams(unsigned int request, > > [[maybe_unused]] uint32_t bufferId) > > { > > - struct v4l2_isp_params_buffer *params; > > IPAFrameContext &frameContext = context_.frameContexts.get(request); > > + MaliC55Params params(buffers_.at(bufferId).planes()[0]); > > - params = reinterpret_cast<v4l2_isp_params_buffer *>( > > - buffers_.at(bufferId).planes()[0].data()); > > - memset(params, 0, > > - buffers_.at(bufferId).planes()[0].size()); > > - > > - params->version = MALI_C55_PARAM_BUFFER_V1; > > - > > - for (auto const &algo : algorithms()) { > > - algo->prepare(context_, request, frameContext, params); > > - > > - ASSERT(params->data_size <= MALI_C55_PARAMS_MAX_SIZE); > > - } > > + for (auto const &algo : algorithms()) > > + algo->prepare(context_, request, frameContext, ¶ms); > > - size_t bytesused = offsetof(struct v4l2_isp_params_buffer, data) + params->data_size; > > - paramsComputed.emit(request, bytesused); > > + paramsComputed.emit(request, params.size()); > > I think this reinforces my view that size() should be called used() (or even maybe bytesused!)> } let's use bytesused() > > void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId, > > diff --git a/src/ipa/mali-c55/module.h b/src/ipa/mali-c55/module.h > > index a8dcb20841a4ebd7bde22cae4ff1f8eef530d8bb..13b34eb2839530a3518340165e6ca895d0f6bcaf 100644 > > --- a/src/ipa/mali-c55/module.h > > +++ b/src/ipa/mali-c55/module.h > > @@ -14,13 +14,14 @@ > > #include <libipa/module.h> > > #include "ipa_context.h" > > +#include "params.h" > > namespace libcamera { > > namespace ipa::mali_c55 { > > using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo, > > - v4l2_isp_params_buffer, mali_c55_stats_buffer>; > > + MaliC55Params, mali_c55_stats_buffer>; > > } /* namespace ipa::mali_c55 */ > > diff --git a/src/ipa/mali-c55/params.h b/src/ipa/mali-c55/params.h > > new file mode 100644 > > index 0000000000000000000000000000000000000000..bb26da19a8f1336eeed91989a6c13e7630c6e5f4 > > --- /dev/null > > +++ b/src/ipa/mali-c55/params.h > > @@ -0,0 +1,83 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board > > + * > > + * Mali C55 ISP Parameters > > + */ > > + > > +#pragma once > > + > > +#include <linux/mali-c55-config.h> > > +#include <linux/videodev2.h> > > + > > +#include <libipa/v4l2_params.h> > > + > > +namespace libcamera { > > + > > +namespace ipa::mali_c55 { > > + > > +enum class MaliC55Blocks { > > + Bls, > > + AexpHist, > > + AexpHistWeights, > > + AexpIhist, > > + AexpIhistWeights, > > + Dgain, > > + AwbGains, > > + AwbConfig, > > + MeshShadingConfig, > > + MeshShadingSel, > > +}; > > + > > +namespace details { > > + > > +template<MaliC55Blocks B> > > +struct block_type { > > +}; > > + > > +#define MALI_C55_DEFINE_BLOCK_TYPE(id, cfgType, blkType) \ > > +template<> \ > > +struct block_type<MaliC55Blocks::id> { \ > > + using type = struct mali_c55_params_##cfgType; \ > > + static constexpr mali_c55_param_block_type blockType = \ > > + mali_c55_param_block_type::MALI_C55_PARAM_##blkType; \ > > +}; > > I think dropping the semicolon here and having it on the ends of the lines > below would be more in line with what I'd expect to see. I'm not sure I got what you mean here :) This ? struct block_type<MaliC55Blocks::id> { \ using type = struct mali_c55_params_##cfgType; \ static constexpr mali_c55_param_block_type blockType = \ mali_c55_param_block_type::MALI_C55_PARAM_##blkType; } > > > + > > +MALI_C55_DEFINE_BLOCK_TYPE(Bls, sensor_off_preshading, BLOCK_SENSOR_OFFS) > > +MALI_C55_DEFINE_BLOCK_TYPE(AexpHist, aexp_hist, BLOCK_AEXP_HIST) > > +MALI_C55_DEFINE_BLOCK_TYPE(AexpHistWeights, aexp_weights, > > + BLOCK_AEXP_HIST_WEIGHTS) > > +MALI_C55_DEFINE_BLOCK_TYPE(AexpIhist, aexp_hist, BLOCK_AEXP_IHIST) > > +MALI_C55_DEFINE_BLOCK_TYPE(AexpIhistWeights, aexp_weights, > > + BLOCK_AEXP_IHIST_WEIGHTS) > > +MALI_C55_DEFINE_BLOCK_TYPE(Dgain, digital_gain, BLOCK_DIGITAL_GAIN) > > +MALI_C55_DEFINE_BLOCK_TYPE(AwbGains, awb_gains, BLOCK_AWB_GAINS) > > +MALI_C55_DEFINE_BLOCK_TYPE(AwbConfig, awb_config, BLOCK_AWB_CONFIG) > > +MALI_C55_DEFINE_BLOCK_TYPE(MeshShadingConfig, mesh_shading_config, > > + MESH_SHADING_CONFIG) > > +MALI_C55_DEFINE_BLOCK_TYPE(MeshShadingSel, mesh_shading_selection, > > + MESH_SHADING_SELECTION) > > Personally I think it would be better to format these so that the fields for > each entry are more visibly associated: > > MALI_C55_DEFINE_BLOCK_TYPE(Bls, sensor_off_preshading, BLOCK_SENSOR_OFFS); > MALI_C55_DEFINE_BLOCK_TYPE(AexpHist, aexp_hist, BLOCK_AEXP_HIST); > MALI_C55_DEFINE_BLOCK_TYPE(AexpHistWeights, aexp_weights, BLOCK_AEXP_HIST_WEIGHTS); > MALI_C55_DEFINE_BLOCK_TYPE(AexpIhist, aexp_hist, BLOCK_AEXP_IHIST); > MALI_C55_DEFINE_BLOCK_TYPE(AexpIhistWeights, aexp_weights, BLOCK_AEXP_IHIST_WEIGHTS); > MALI_C55_DEFINE_BLOCK_TYPE(Dgain, digital_gain, BLOCK_DIGITAL_GAIN); > MALI_C55_DEFINE_BLOCK_TYPE(AwbGains, awb_gains, BLOCK_AWB_GAINS); > MALI_C55_DEFINE_BLOCK_TYPE(AwbConfig, awb_config, BLOCK_AWB_CONFIG); > MALI_C55_DEFINE_BLOCK_TYPE(MeshShadingConfig, mesh_shading_config, MESH_SHADING_CONFIG) > MALI_C55_DEFINE_BLOCK_TYPE(MeshShadingSel, mesh_shading_selection, MESH_SHADING_SELECTION); > > It's longer, but I think it's a much easier to read style for this kind of thing. More readable, I agree. Let's see how loud checkstyle complains (I think it already was complaining on this version). Thanks j > > Thanks > Dan > > > + > > +struct param_traits { > > + using id_type = MaliC55Blocks; > > + > > + template<id_type Id> > > + using id_to_details = block_type<Id>; > > +}; > > + > > +} /* namespace details */ > > + > > +class MaliC55Params : public V4L2Params<details::param_traits> > > +{ > > +public: > > + static constexpr unsigned int kVersion = MALI_C55_PARAM_BUFFER_V1; > > + > > + MaliC55Params(Span<uint8_t> data) > > + : V4L2Params(data, kVersion) > > + { > > + } > > +}; > > + > > +} /* namespace ipa::mali_c55 */ > > + > > +} /* namespace libcamera */ > > >
Hi Jacopo On 10/10/2025 17:17, Jacopo Mondi wrote: > Hi Dan > > On Fri, Oct 10, 2025 at 04:16:37PM +0100, Dan Scally wrote: >> Hi Jacopo >> >> On 07/10/2025 19:17, Jacopo Mondi wrote: >>> Implement MaliC55Params to derive from V4L2Params and use the new >>> helpers in the Mali C55 IPA algorithms implementation. >>> >>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >>> Tested-by: Antoine Bouyer <antoine.bouyer@nxp.com> >> >> Couple of nits below, but with or without them changed: >> >> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > > Thanks > >> >>> --- >>> src/ipa/mali-c55/algorithms/agc.cpp | 87 +++++++++++++++---------------------- >>> src/ipa/mali-c55/algorithms/agc.h | 14 +++--- > > [snip] > >>> --- a/src/ipa/mali-c55/mali-c55.cpp >>> +++ b/src/ipa/mali-c55/mali-c55.cpp >>> @@ -28,6 +28,7 @@ >>> #include "libipa/camera_sensor_helper.h" >>> #include "ipa_context.h" >>> +#include "params.h" >>> namespace libcamera { >>> @@ -331,24 +332,13 @@ void IPAMaliC55::queueRequest(const uint32_t request, const ControlList &control >>> void IPAMaliC55::fillParams(unsigned int request, >>> [[maybe_unused]] uint32_t bufferId) >>> { >>> - struct v4l2_isp_params_buffer *params; >>> IPAFrameContext &frameContext = context_.frameContexts.get(request); >>> + MaliC55Params params(buffers_.at(bufferId).planes()[0]); >>> - params = reinterpret_cast<v4l2_isp_params_buffer *>( >>> - buffers_.at(bufferId).planes()[0].data()); >>> - memset(params, 0, >>> - buffers_.at(bufferId).planes()[0].size()); >>> - >>> - params->version = MALI_C55_PARAM_BUFFER_V1; >>> - >>> - for (auto const &algo : algorithms()) { >>> - algo->prepare(context_, request, frameContext, params); >>> - >>> - ASSERT(params->data_size <= MALI_C55_PARAMS_MAX_SIZE); >>> - } >>> + for (auto const &algo : algorithms()) >>> + algo->prepare(context_, request, frameContext, ¶ms); >>> - size_t bytesused = offsetof(struct v4l2_isp_params_buffer, data) + params->data_size; >>> - paramsComputed.emit(request, bytesused); >>> + paramsComputed.emit(request, params.size()); >> >> I think this reinforces my view that size() should be called used() (or even maybe bytesused!)> } > > let's use bytesused() Sounds good > >>> void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId, >>> diff --git a/src/ipa/mali-c55/module.h b/src/ipa/mali-c55/module.h >>> index a8dcb20841a4ebd7bde22cae4ff1f8eef530d8bb..13b34eb2839530a3518340165e6ca895d0f6bcaf 100644 >>> --- a/src/ipa/mali-c55/module.h >>> +++ b/src/ipa/mali-c55/module.h >>> @@ -14,13 +14,14 @@ >>> #include <libipa/module.h> >>> #include "ipa_context.h" >>> +#include "params.h" >>> namespace libcamera { >>> namespace ipa::mali_c55 { >>> using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo, >>> - v4l2_isp_params_buffer, mali_c55_stats_buffer>; >>> + MaliC55Params, mali_c55_stats_buffer>; >>> } /* namespace ipa::mali_c55 */ >>> diff --git a/src/ipa/mali-c55/params.h b/src/ipa/mali-c55/params.h >>> new file mode 100644 >>> index 0000000000000000000000000000000000000000..bb26da19a8f1336eeed91989a6c13e7630c6e5f4 >>> --- /dev/null >>> +++ b/src/ipa/mali-c55/params.h >>> @@ -0,0 +1,83 @@ >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >>> +/* >>> + * Copyright (C) 2025, Ideas On Board >>> + * >>> + * Mali C55 ISP Parameters >>> + */ >>> + >>> +#pragma once >>> + >>> +#include <linux/mali-c55-config.h> >>> +#include <linux/videodev2.h> >>> + >>> +#include <libipa/v4l2_params.h> >>> + >>> +namespace libcamera { >>> + >>> +namespace ipa::mali_c55 { >>> + >>> +enum class MaliC55Blocks { >>> + Bls, >>> + AexpHist, >>> + AexpHistWeights, >>> + AexpIhist, >>> + AexpIhistWeights, >>> + Dgain, >>> + AwbGains, >>> + AwbConfig, >>> + MeshShadingConfig, >>> + MeshShadingSel, >>> +}; >>> + >>> +namespace details { >>> + >>> +template<MaliC55Blocks B> >>> +struct block_type { >>> +}; >>> + >>> +#define MALI_C55_DEFINE_BLOCK_TYPE(id, cfgType, blkType) \ >>> +template<> \ >>> +struct block_type<MaliC55Blocks::id> { \ >>> + using type = struct mali_c55_params_##cfgType; \ >>> + static constexpr mali_c55_param_block_type blockType = \ >>> + mali_c55_param_block_type::MALI_C55_PARAM_##blkType; \ >>> +}; >> >> I think dropping the semicolon here and having it on the ends of the lines >> below would be more in line with what I'd expect to see. > > I'm not sure I got what you mean here :) Instead of "};" at the end of the definition of the macro have "}" so that we do... MALI_C55_DEFINE_BLOCK_TYPE(Bls, sensor_off_preshading, BLOCK_SENSOR_OFFS); ...instead of... MALI_C55_DEFINE_BLOCK_TYPE(Bls, sensor_off_preshading, BLOCK_SENSOR_OFFS) But it doesn't really matter :) > > This ? > > struct block_type<MaliC55Blocks::id> { \ > using type = struct mali_c55_params_##cfgType; \ > static constexpr mali_c55_param_block_type blockType = \ > mali_c55_param_block_type::MALI_C55_PARAM_##blkType; } > >> >>> + >>> +MALI_C55_DEFINE_BLOCK_TYPE(Bls, sensor_off_preshading, BLOCK_SENSOR_OFFS) >>> +MALI_C55_DEFINE_BLOCK_TYPE(AexpHist, aexp_hist, BLOCK_AEXP_HIST) >>> +MALI_C55_DEFINE_BLOCK_TYPE(AexpHistWeights, aexp_weights, >>> + BLOCK_AEXP_HIST_WEIGHTS) >>> +MALI_C55_DEFINE_BLOCK_TYPE(AexpIhist, aexp_hist, BLOCK_AEXP_IHIST) >>> +MALI_C55_DEFINE_BLOCK_TYPE(AexpIhistWeights, aexp_weights, >>> + BLOCK_AEXP_IHIST_WEIGHTS) >>> +MALI_C55_DEFINE_BLOCK_TYPE(Dgain, digital_gain, BLOCK_DIGITAL_GAIN) >>> +MALI_C55_DEFINE_BLOCK_TYPE(AwbGains, awb_gains, BLOCK_AWB_GAINS) >>> +MALI_C55_DEFINE_BLOCK_TYPE(AwbConfig, awb_config, BLOCK_AWB_CONFIG) >>> +MALI_C55_DEFINE_BLOCK_TYPE(MeshShadingConfig, mesh_shading_config, >>> + MESH_SHADING_CONFIG) >>> +MALI_C55_DEFINE_BLOCK_TYPE(MeshShadingSel, mesh_shading_selection, >>> + MESH_SHADING_SELECTION) >> >> Personally I think it would be better to format these so that the fields for >> each entry are more visibly associated: >> >> MALI_C55_DEFINE_BLOCK_TYPE(Bls, sensor_off_preshading, BLOCK_SENSOR_OFFS); >> MALI_C55_DEFINE_BLOCK_TYPE(AexpHist, aexp_hist, BLOCK_AEXP_HIST); >> MALI_C55_DEFINE_BLOCK_TYPE(AexpHistWeights, aexp_weights, BLOCK_AEXP_HIST_WEIGHTS); >> MALI_C55_DEFINE_BLOCK_TYPE(AexpIhist, aexp_hist, BLOCK_AEXP_IHIST); >> MALI_C55_DEFINE_BLOCK_TYPE(AexpIhistWeights, aexp_weights, BLOCK_AEXP_IHIST_WEIGHTS); >> MALI_C55_DEFINE_BLOCK_TYPE(Dgain, digital_gain, BLOCK_DIGITAL_GAIN); >> MALI_C55_DEFINE_BLOCK_TYPE(AwbGains, awb_gains, BLOCK_AWB_GAINS); >> MALI_C55_DEFINE_BLOCK_TYPE(AwbConfig, awb_config, BLOCK_AWB_CONFIG); >> MALI_C55_DEFINE_BLOCK_TYPE(MeshShadingConfig, mesh_shading_config, MESH_SHADING_CONFIG) >> MALI_C55_DEFINE_BLOCK_TYPE(MeshShadingSel, mesh_shading_selection, MESH_SHADING_SELECTION); >> >> It's longer, but I think it's a much easier to read style for this kind of thing. > > More readable, I agree. Let's see how loud checkstyle complains (I > think it already was complaining on this version). Ack > > Thanks > j > >> >> Thanks >> Dan >> >>> + >>> +struct param_traits { >>> + using id_type = MaliC55Blocks; >>> + >>> + template<id_type Id> >>> + using id_to_details = block_type<Id>; >>> +}; >>> + >>> +} /* namespace details */ >>> + >>> +class MaliC55Params : public V4L2Params<details::param_traits> >>> +{ >>> +public: >>> + static constexpr unsigned int kVersion = MALI_C55_PARAM_BUFFER_V1; >>> + >>> + MaliC55Params(Span<uint8_t> data) >>> + : V4L2Params(data, kVersion) >>> + { >>> + } >>> +}; >>> + >>> +} /* namespace ipa::mali_c55 */ >>> + >>> +} /* namespace libcamera */ >>> >>
diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp index 78e7e07b2348a711e6261bac45e006f49a59513a..5d60cd8c9ff8a90e2e48e5d6939b538135b7ec3d 100644 --- a/src/ipa/mali-c55/algorithms/agc.cpp +++ b/src/ipa/mali-c55/algorithms/agc.cpp @@ -241,8 +241,8 @@ void Agc::queueRequest(IPAContext &context, const uint32_t frame, } } -size_t Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext, - mali_c55_params_block block) +void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext, + MaliC55Params *params) { IPAActiveState &activeState = context.activeState; double gain; @@ -252,52 +252,50 @@ size_t Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContex else gain = activeState.agc.manual.ispGain; - block.header->type = MALI_C55_PARAM_BLOCK_DIGITAL_GAIN; - block.header->flags = 0; - block.header->size = sizeof(struct mali_c55_params_digital_gain); + auto block = params->block<MaliC55Blocks::Dgain>(); + block->gain = floatingToFixedPoint<5, 8, uint16_t, double>(gain); - block.digital_gain->gain = floatingToFixedPoint<5, 8, uint16_t, double>(gain); frameContext.agc.ispGain = gain; - - return block.header->size; } -size_t Agc::fillParamsBuffer(mali_c55_params_block block, - enum mali_c55_param_block_type type) +void Agc::fillParamsBuffer(MaliC55Params *params, enum MaliC55Blocks type) { - block.header->type = type; - block.header->flags = 0; - block.header->size = sizeof(struct mali_c55_params_aexp_hist); + + assert(type == MaliC55Blocks::AexpHist || type == MaliC55Blocks::AexpIhist); + + auto block = type == MaliC55Blocks::AexpHist ? + params->block<MaliC55Blocks::AexpHist>() : + params->block<MaliC55Blocks::AexpIhist>(); /* Collect every 3rd pixel horizontally */ - block.aexp_hist->skip_x = 1; + block->skip_x = 1; /* Start from first column */ - block.aexp_hist->offset_x = 0; + block->offset_x = 0; /* Collect every pixel vertically */ - block.aexp_hist->skip_y = 0; + block->skip_y = 0; /* Start from the first row */ - block.aexp_hist->offset_y = 0; + block->offset_y = 0; /* 1x scaling (i.e. none) */ - block.aexp_hist->scale_bottom = 0; - block.aexp_hist->scale_top = 0; + block->scale_bottom = 0; + block->scale_top = 0; /* Collect all Bayer planes into 4 separate histograms */ - block.aexp_hist->plane_mode = 1; + block->plane_mode = 1; /* Tap the data immediately after the digital gain block */ - block.aexp_hist->tap_point = MALI_C55_AEXP_HIST_TAP_FS; - - return block.header->size; + block->tap_point = MALI_C55_AEXP_HIST_TAP_FS; } -size_t Agc::fillWeightsArrayBuffer(mali_c55_params_block block, - enum mali_c55_param_block_type type) +void Agc::fillWeightsArrayBuffer(MaliC55Params *params, const enum MaliC55Blocks type) { - block.header->type = type; - block.header->flags = 0; - block.header->size = sizeof(struct mali_c55_params_aexp_weights); + assert(type == MaliC55Blocks::AexpHistWeights || + type == MaliC55Blocks::AexpIhistWeights); + + auto block = type == MaliC55Blocks::AexpHistWeights ? + params->block<MaliC55Blocks::AexpHistWeights>() : + params->block<MaliC55Blocks::AexpIhistWeights>(); /* We use every zone - a 15x15 grid */ - block.aexp_weights->nodes_used_horiz = 15; - block.aexp_weights->nodes_used_vert = 15; + block->nodes_used_horiz = 15; + block->nodes_used_vert = 15; /* * We uniformly weight the zones to 1 - this results in the collected @@ -305,40 +303,25 @@ size_t Agc::fillWeightsArrayBuffer(mali_c55_params_block block, * approximate colour channel averages for the image. */ Span<uint8_t> weights{ - block.aexp_weights->zone_weights, + block->zone_weights, MALI_C55_MAX_ZONES }; std::fill(weights.begin(), weights.end(), 1); - - return block.header->size; } void Agc::prepare(IPAContext &context, const uint32_t frame, - IPAFrameContext &frameContext, v4l2_isp_params_buffer *params) + IPAFrameContext &frameContext, MaliC55Params *params) { - mali_c55_params_block block; - - block.data = ¶ms->data[params->data_size]; - params->data_size += fillGainParamBlock(context, frameContext, block); + fillGainParamBlock(context, frameContext, params); if (frame > 0) return; - block.data = ¶ms->data[params->data_size]; - params->data_size += fillParamsBuffer(block, - MALI_C55_PARAM_BLOCK_AEXP_HIST); - - block.data = ¶ms->data[params->data_size]; - params->data_size += fillWeightsArrayBuffer(block, - MALI_C55_PARAM_BLOCK_AEXP_HIST_WEIGHTS); - - block.data = ¶ms->data[params->data_size]; - params->data_size += fillParamsBuffer(block, - MALI_C55_PARAM_BLOCK_AEXP_IHIST); + fillParamsBuffer(params, MaliC55Blocks::AexpHist); + fillWeightsArrayBuffer(params, MaliC55Blocks::AexpHistWeights); - block.data = ¶ms->data[params->data_size]; - params->data_size += fillWeightsArrayBuffer(block, - MALI_C55_PARAM_BLOCK_AEXP_IHIST_WEIGHTS); + fillParamsBuffer(params, MaliC55Blocks::AexpIhist); + fillWeightsArrayBuffer(params, MaliC55Blocks::AexpIhistWeights); } double Agc::estimateLuminance(const double gain) const diff --git a/src/ipa/mali-c55/algorithms/agc.h b/src/ipa/mali-c55/algorithms/agc.h index 4325ef5a9b7dcef36107b64a65db993f194d4167..9684fff664bc67d287bb00f8dc88e238d8dd0cea 100644 --- a/src/ipa/mali-c55/algorithms/agc.h +++ b/src/ipa/mali-c55/algorithms/agc.h @@ -57,7 +57,7 @@ public: const ControlList &controls) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - v4l2_isp_params_buffer *params) override; + MaliC55Params *params) override; void process(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, const mali_c55_stats_buffer *stats, @@ -65,13 +65,11 @@ public: private: double estimateLuminance(const double gain) const override; - size_t fillGainParamBlock(IPAContext &context, - IPAFrameContext &frameContext, - mali_c55_params_block block); - size_t fillParamsBuffer(mali_c55_params_block block, - enum mali_c55_param_block_type type); - size_t fillWeightsArrayBuffer(mali_c55_params_block block, - enum mali_c55_param_block_type type); + void fillGainParamBlock(IPAContext &context, + IPAFrameContext &frameContext, + MaliC55Params *params); + void fillParamsBuffer(MaliC55Params *params, enum MaliC55Blocks type); + void fillWeightsArrayBuffer(MaliC55Params *params, enum MaliC55Blocks type); AgcStatistics statistics_; }; diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp index 694c0aaa9c6804bb58e380ba9c744f11c39224fe..964e810882a93cce02f991675d74bbf163d51e7c 100644 --- a/src/ipa/mali-c55/algorithms/awb.cpp +++ b/src/ipa/mali-c55/algorithms/awb.cpp @@ -43,13 +43,9 @@ int Awb::configure([[maybe_unused]] IPAContext &context, return 0; } -size_t Awb::fillGainsParamBlock(mali_c55_params_block block, IPAContext &context, +void Awb::fillGainsParamBlock(MaliC55Params *params, IPAContext &context, IPAFrameContext &frameContext) { - block.header->type = MALI_C55_PARAM_BLOCK_AWB_GAINS; - block.header->flags = 0; - block.header->size = sizeof(struct mali_c55_params_awb_gains); - double rGain = context.activeState.awb.rGain; double bGain = context.activeState.awb.bGain; @@ -63,34 +59,32 @@ size_t Awb::fillGainsParamBlock(mali_c55_params_block block, IPAContext &context * This holds true regardless of the bayer order of the input data, as * the mapping is done internally in the ISP. */ - block.awb_gains->gain00 = floatingToFixedPoint<4, 8, uint16_t, double>(rGain); - block.awb_gains->gain01 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0); - block.awb_gains->gain10 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0); - block.awb_gains->gain11 = floatingToFixedPoint<4, 8, uint16_t, double>(bGain); + auto block = params->block<MaliC55Blocks::AwbGains>(); + + block->gain00 = floatingToFixedPoint<4, 8, uint16_t, double>(rGain); + block->gain01 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0); + block->gain10 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0); + block->gain11 = floatingToFixedPoint<4, 8, uint16_t, double>(bGain); frameContext.awb.rGain = rGain; frameContext.awb.bGain = bGain; - - return sizeof(struct mali_c55_params_awb_gains); } -size_t Awb::fillConfigParamBlock(mali_c55_params_block block) +void Awb::fillConfigParamBlock(MaliC55Params *params) { - block.header->type = MALI_C55_PARAM_BLOCK_AWB_CONFIG; - block.header->flags = 0; - block.header->size = sizeof(struct mali_c55_params_awb_config); + auto block = params->block<MaliC55Blocks::AwbConfig>(); /* Tap the stats after the purple fringe block */ - block.awb_config->tap_point = MALI_C55_AWB_STATS_TAP_PF; + block->tap_point = MALI_C55_AWB_STATS_TAP_PF; /* Get R/G and B/G ratios as statistics */ - block.awb_config->stats_mode = MALI_C55_AWB_MODE_RGBG; + block->stats_mode = MALI_C55_AWB_MODE_RGBG; /* Default white level */ - block.awb_config->white_level = 1023; + block->white_level = 1023; /* Default black level */ - block.awb_config->black_level = 0; + block->black_level = 0; /* * By default pixels are included who's colour ratios are bounded in a @@ -104,40 +98,34 @@ size_t Awb::fillConfigParamBlock(mali_c55_params_block block) * * \todo should these perhaps be tunable? */ - block.awb_config->cr_max = 511; - block.awb_config->cr_min = 64; - block.awb_config->cb_max = 511; - block.awb_config->cb_min = 64; + block->cr_max = 511; + block->cr_min = 64; + block->cb_max = 511; + block->cb_min = 64; /* We use the full 15x15 zoning scheme */ - block.awb_config->nodes_used_horiz = 15; - block.awb_config->nodes_used_vert = 15; + block->nodes_used_horiz = 15; + block->nodes_used_vert = 15; /* * We set the trimming boundaries equivalent to the main boundaries. In * other words; no trimming. */ - block.awb_config->cr_high = 511; - block.awb_config->cr_low = 64; - block.awb_config->cb_high = 511; - block.awb_config->cb_low = 64; - - return sizeof(struct mali_c55_params_awb_config); + block->cr_high = 511; + block->cr_low = 64; + block->cb_high = 511; + block->cb_low = 64; } void Awb::prepare(IPAContext &context, const uint32_t frame, - IPAFrameContext &frameContext, v4l2_isp_params_buffer *params) + IPAFrameContext &frameContext, MaliC55Params *params) { - mali_c55_params_block block; - block.data = ¶ms->data[params->data_size]; - - params->data_size += fillGainsParamBlock(block, context, frameContext); + fillGainsParamBlock(params, context, frameContext); if (frame > 0) return; - block.data = ¶ms->data[params->data_size]; - params->data_size += fillConfigParamBlock(block); + fillConfigParamBlock(params); } void Awb::process(IPAContext &context, const uint32_t frame, diff --git a/src/ipa/mali-c55/algorithms/awb.h b/src/ipa/mali-c55/algorithms/awb.h index 647525ff700e8281e3cce63e64ba56a91294bcc2..683a62af263a14d2f5d5b261448953ada6669b2f 100644 --- a/src/ipa/mali-c55/algorithms/awb.h +++ b/src/ipa/mali-c55/algorithms/awb.h @@ -22,17 +22,17 @@ public: const IPACameraSensorInfo &configInfo) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - v4l2_isp_params_buffer *params) override; + MaliC55Params *params) override; void process(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, const mali_c55_stats_buffer *stats, ControlList &metadata) override; private: - size_t fillGainsParamBlock(mali_c55_params_block block, - IPAContext &context, - IPAFrameContext &frameContext); - size_t fillConfigParamBlock(mali_c55_params_block block); + void fillGainsParamBlock(MaliC55Params *params, + IPAContext &context, + IPAFrameContext &frameContext); + void fillConfigParamBlock(MaliC55Params *params); }; } /* namespace ipa::mali_c55::algorithms */ diff --git a/src/ipa/mali-c55/algorithms/blc.cpp b/src/ipa/mali-c55/algorithms/blc.cpp index 543ba96cb57ac0cca2b8f822180d8d8b42f21fc7..d099219c3e43ec96fa452ed13fa46ada2025edc9 100644 --- a/src/ipa/mali-c55/algorithms/blc.cpp +++ b/src/ipa/mali-c55/algorithms/blc.cpp @@ -85,27 +85,19 @@ int BlackLevelCorrection::configure(IPAContext &context, void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context, const uint32_t frame, [[maybe_unused]] IPAFrameContext &frameContext, - v4l2_isp_params_buffer *params) + MaliC55Params *params) { - mali_c55_params_block block; - block.data = ¶ms->data[params->data_size]; - if (frame > 0) return; if (!tuningParameters_) return; - block.header->type = MALI_C55_PARAM_BLOCK_SENSOR_OFFS; - block.header->flags = 0; - block.header->size = sizeof(mali_c55_params_sensor_off_preshading); - - block.sensor_offs->chan00 = offset00; - block.sensor_offs->chan01 = offset01; - block.sensor_offs->chan10 = offset10; - block.sensor_offs->chan11 = offset11; - - params->data_size += block.header->size; + auto block = params->block<MaliC55Blocks::Bls>(); + block->chan00 = offset00; + block->chan01 = offset01; + block->chan10 = offset10; + block->chan11 = offset11; } void BlackLevelCorrection::process([[maybe_unused]] IPAContext &context, diff --git a/src/ipa/mali-c55/algorithms/blc.h b/src/ipa/mali-c55/algorithms/blc.h index ee6c889ed89ac2230b231cb58a9fe4412c0ce164..fc5a7ea310cbdbcf271eb26c73b17243aea744cd 100644 --- a/src/ipa/mali-c55/algorithms/blc.h +++ b/src/ipa/mali-c55/algorithms/blc.h @@ -6,6 +6,7 @@ */ #include "algorithm.h" +#include "params.h" namespace libcamera { @@ -22,7 +23,7 @@ public: const IPACameraSensorInfo &configInfo) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - v4l2_isp_params_buffer *params) override; + MaliC55Params *params) override; void process(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, const mali_c55_stats_buffer *stats, diff --git a/src/ipa/mali-c55/algorithms/lsc.cpp b/src/ipa/mali-c55/algorithms/lsc.cpp index cb915c5efd3c22952035f2b03ee659f293942ec9..5b042c757bc70fe4eb4a5aaa848266b3afce9100 100644 --- a/src/ipa/mali-c55/algorithms/lsc.cpp +++ b/src/ipa/mali-c55/algorithms/lsc.cpp @@ -108,41 +108,33 @@ int Lsc::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData return 0; } -size_t Lsc::fillConfigParamsBlock(mali_c55_params_block block) const +void Lsc::fillConfigParamsBlock(MaliC55Params *params) const { - block.header->type = MALI_C55_PARAM_MESH_SHADING_CONFIG; - block.header->flags = 0; - block.header->size = sizeof(struct mali_c55_params_mesh_shading_config); + auto block = params->block<MaliC55Blocks::MeshShadingConfig>(); - block.shading_config->mesh_show = false; - block.shading_config->mesh_scale = meshScale_; - block.shading_config->mesh_page_r = 0; - block.shading_config->mesh_page_g = 1; - block.shading_config->mesh_page_b = 2; - block.shading_config->mesh_width = meshSize_; - block.shading_config->mesh_height = meshSize_; + block->mesh_show = false; + block->mesh_scale = meshScale_; + block->mesh_page_r = 0; + block->mesh_page_g = 1; + block->mesh_page_b = 2; + block->mesh_width = meshSize_; + block->mesh_height = meshSize_; - std::copy(mesh_.begin(), mesh_.end(), block.shading_config->mesh); - - return block.header->size; + std::copy(mesh_.begin(), mesh_.end(), block->mesh); } -size_t Lsc::fillSelectionParamsBlock(mali_c55_params_block block, uint8_t bank, +void Lsc::fillSelectionParamsBlock(MaliC55Params *params, uint8_t bank, uint8_t alpha) const { - block.header->type = MALI_C55_PARAM_MESH_SHADING_SELECTION; - block.header->flags = 0; - block.header->size = sizeof(struct mali_c55_params_mesh_shading_selection); - - block.shading_selection->mesh_alpha_bank_r = bank; - block.shading_selection->mesh_alpha_bank_g = bank; - block.shading_selection->mesh_alpha_bank_b = bank; - block.shading_selection->mesh_alpha_r = alpha; - block.shading_selection->mesh_alpha_g = alpha; - block.shading_selection->mesh_alpha_b = alpha; - block.shading_selection->mesh_strength = 0x1000; /* Otherwise known as 1.0 */ - - return block.header->size; + auto block = params->block<MaliC55Blocks::MeshShadingSel>(); + + block->mesh_alpha_bank_r = bank; + block->mesh_alpha_bank_g = bank; + block->mesh_alpha_bank_b = bank; + block->mesh_alpha_r = alpha; + block->mesh_alpha_g = alpha; + block->mesh_alpha_b = alpha; + block->mesh_strength = 0x1000; /* Otherwise known as 1.0 */ } std::tuple<uint8_t, uint8_t> Lsc::findBankAndAlpha(uint32_t ct) const @@ -170,7 +162,7 @@ std::tuple<uint8_t, uint8_t> Lsc::findBankAndAlpha(uint32_t ct) const void Lsc::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame, [[maybe_unused]] IPAFrameContext &frameContext, - v4l2_isp_params_buffer *params) + MaliC55Params *params) { /* * For each frame we assess the colour temperature of the **last** frame @@ -193,10 +185,7 @@ void Lsc::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame, std::tie(bank, alpha) = findBankAndAlpha(temperatureK); } - mali_c55_params_block block; - block.data = ¶ms->data[params->data_size]; - - params->data_size += fillSelectionParamsBlock(block, bank, alpha); + fillSelectionParamsBlock(params, bank, alpha); if (frame > 0) return; @@ -205,8 +194,7 @@ void Lsc::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame, * If this is the first frame, we need to load the parsed coefficient * tables from tuning data to the ISP. */ - block.data = ¶ms->data[params->data_size]; - params->data_size += fillConfigParamsBlock(block); + fillConfigParamsBlock(params); } REGISTER_IPA_ALGORITHM(Lsc, "Lsc") diff --git a/src/ipa/mali-c55/algorithms/lsc.h b/src/ipa/mali-c55/algorithms/lsc.h index c287900502528ca82ab6c62d732b6ffc28ad8df6..e7092bc74a0b0301463167d16e1e888696547d10 100644 --- a/src/ipa/mali-c55/algorithms/lsc.h +++ b/src/ipa/mali-c55/algorithms/lsc.h @@ -23,15 +23,15 @@ public: int init(IPAContext &context, const YamlObject &tuningData) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - v4l2_isp_params_buffer *params) override; + MaliC55Params *params) override; private: static constexpr unsigned int kRedOffset = 0; static constexpr unsigned int kGreenOffset = 1024; static constexpr unsigned int kBlueOffset = 2048; - size_t fillConfigParamsBlock(mali_c55_params_block block) const; - size_t fillSelectionParamsBlock(mali_c55_params_block block, - uint8_t bank, uint8_t alpha) const; + void fillConfigParamsBlock(MaliC55Params *params) const; + void fillSelectionParamsBlock(MaliC55Params *params, + uint8_t bank, uint8_t alpha) const; std::tuple<uint8_t, uint8_t> findBankAndAlpha(uint32_t ct) const; std::vector<uint32_t> mesh_ = std::vector<uint32_t>(3072); diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp index 504d95222074a35b033949a25178f998738e5699..fe36d4445c3ac75abe48dbe2335fa1f3aded9609 100644 --- a/src/ipa/mali-c55/mali-c55.cpp +++ b/src/ipa/mali-c55/mali-c55.cpp @@ -28,6 +28,7 @@ #include "libipa/camera_sensor_helper.h" #include "ipa_context.h" +#include "params.h" namespace libcamera { @@ -331,24 +332,13 @@ void IPAMaliC55::queueRequest(const uint32_t request, const ControlList &control void IPAMaliC55::fillParams(unsigned int request, [[maybe_unused]] uint32_t bufferId) { - struct v4l2_isp_params_buffer *params; IPAFrameContext &frameContext = context_.frameContexts.get(request); + MaliC55Params params(buffers_.at(bufferId).planes()[0]); - params = reinterpret_cast<v4l2_isp_params_buffer *>( - buffers_.at(bufferId).planes()[0].data()); - memset(params, 0, - buffers_.at(bufferId).planes()[0].size()); - - params->version = MALI_C55_PARAM_BUFFER_V1; - - for (auto const &algo : algorithms()) { - algo->prepare(context_, request, frameContext, params); - - ASSERT(params->data_size <= MALI_C55_PARAMS_MAX_SIZE); - } + for (auto const &algo : algorithms()) + algo->prepare(context_, request, frameContext, ¶ms); - size_t bytesused = offsetof(struct v4l2_isp_params_buffer, data) + params->data_size; - paramsComputed.emit(request, bytesused); + paramsComputed.emit(request, params.size()); } void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId, diff --git a/src/ipa/mali-c55/module.h b/src/ipa/mali-c55/module.h index a8dcb20841a4ebd7bde22cae4ff1f8eef530d8bb..13b34eb2839530a3518340165e6ca895d0f6bcaf 100644 --- a/src/ipa/mali-c55/module.h +++ b/src/ipa/mali-c55/module.h @@ -14,13 +14,14 @@ #include <libipa/module.h> #include "ipa_context.h" +#include "params.h" namespace libcamera { namespace ipa::mali_c55 { using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo, - v4l2_isp_params_buffer, mali_c55_stats_buffer>; + MaliC55Params, mali_c55_stats_buffer>; } /* namespace ipa::mali_c55 */ diff --git a/src/ipa/mali-c55/params.h b/src/ipa/mali-c55/params.h new file mode 100644 index 0000000000000000000000000000000000000000..bb26da19a8f1336eeed91989a6c13e7630c6e5f4 --- /dev/null +++ b/src/ipa/mali-c55/params.h @@ -0,0 +1,83 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2025, Ideas On Board + * + * Mali C55 ISP Parameters + */ + +#pragma once + +#include <linux/mali-c55-config.h> +#include <linux/videodev2.h> + +#include <libipa/v4l2_params.h> + +namespace libcamera { + +namespace ipa::mali_c55 { + +enum class MaliC55Blocks { + Bls, + AexpHist, + AexpHistWeights, + AexpIhist, + AexpIhistWeights, + Dgain, + AwbGains, + AwbConfig, + MeshShadingConfig, + MeshShadingSel, +}; + +namespace details { + +template<MaliC55Blocks B> +struct block_type { +}; + +#define MALI_C55_DEFINE_BLOCK_TYPE(id, cfgType, blkType) \ +template<> \ +struct block_type<MaliC55Blocks::id> { \ + using type = struct mali_c55_params_##cfgType; \ + static constexpr mali_c55_param_block_type blockType = \ + mali_c55_param_block_type::MALI_C55_PARAM_##blkType; \ +}; + +MALI_C55_DEFINE_BLOCK_TYPE(Bls, sensor_off_preshading, BLOCK_SENSOR_OFFS) +MALI_C55_DEFINE_BLOCK_TYPE(AexpHist, aexp_hist, BLOCK_AEXP_HIST) +MALI_C55_DEFINE_BLOCK_TYPE(AexpHistWeights, aexp_weights, + BLOCK_AEXP_HIST_WEIGHTS) +MALI_C55_DEFINE_BLOCK_TYPE(AexpIhist, aexp_hist, BLOCK_AEXP_IHIST) +MALI_C55_DEFINE_BLOCK_TYPE(AexpIhistWeights, aexp_weights, + BLOCK_AEXP_IHIST_WEIGHTS) +MALI_C55_DEFINE_BLOCK_TYPE(Dgain, digital_gain, BLOCK_DIGITAL_GAIN) +MALI_C55_DEFINE_BLOCK_TYPE(AwbGains, awb_gains, BLOCK_AWB_GAINS) +MALI_C55_DEFINE_BLOCK_TYPE(AwbConfig, awb_config, BLOCK_AWB_CONFIG) +MALI_C55_DEFINE_BLOCK_TYPE(MeshShadingConfig, mesh_shading_config, + MESH_SHADING_CONFIG) +MALI_C55_DEFINE_BLOCK_TYPE(MeshShadingSel, mesh_shading_selection, + MESH_SHADING_SELECTION) + +struct param_traits { + using id_type = MaliC55Blocks; + + template<id_type Id> + using id_to_details = block_type<Id>; +}; + +} /* namespace details */ + +class MaliC55Params : public V4L2Params<details::param_traits> +{ +public: + static constexpr unsigned int kVersion = MALI_C55_PARAM_BUFFER_V1; + + MaliC55Params(Span<uint8_t> data) + : V4L2Params(data, kVersion) + { + } +}; + +} /* namespace ipa::mali_c55 */ + +} /* namespace libcamera */