[libcamera-devel] [PATCH v2 04/10] ipa: ipu3: Introduce a prepare() call to Algorithm
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Mon Aug 16 10:01:14 CEST 2021
Hi Laurent,
On 15/08/2021 20:02, Laurent Pinchart wrote:
> Hi Jean-Michel,
>
> Thank you for the patch.
>
> On Thu, Aug 12, 2021 at 06:52:37PM +0200, Jean-Michel Hautbois wrote:
>> When a new ipu3_uapi_params buffer is ready, IPAIPU3 receives a
>> EventFillParams. When this is called, the algorithms should fill every
>> field in the parameter structure they need to update.
>>
>> Demonstrate it by adding a loop on the prepare() call in IPAIPU3 and let
>> the Grid algorithm update the grid field.
>> This leads to removing this same update from the Awb algorithm.
>>
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>> ---
>> src/ipa/ipu3/algorithms/algorithm.h | 2 ++
>> src/ipa/ipu3/algorithms/grid.cpp | 6 ++++++
>> src/ipa/ipu3/algorithms/grid.h | 1 +
>> src/ipa/ipu3/ipu3.cpp | 3 +++
>> src/ipa/ipu3/ipu3_awb.cpp | 1 -
>> 5 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h
>> index c1b37276..b571f55a 100644
>> --- a/src/ipa/ipu3/algorithms/algorithm.h
>> +++ b/src/ipa/ipu3/algorithms/algorithm.h
>> @@ -26,6 +26,8 @@ public:
>> {
>> return 0;
>> }
>> +
>> + virtual void prepare([[maybe_unused]] IPAContext &context, [[maybe_unused]] ipu3_uapi_params ¶ms) {}
>
> Line wrap.
>
> Could you add a patch after 02/10 that adds all functions (configure(),
> prepare() and process()) to the Algorithm class, and call them in the
> IPAIPU3 class as appropriate ? That patch should include the
> documentation of the functions, to explain how they're used. After that
> the rest of the series can begin moving the algorithms to the new class.
>
I did that in the first place, and it was a bit of a heavy patch. So I
decided to split it to introduce each of configure/prepare/process calls
step by step...
>> };
>>
>> } /* namespace ipa::ipu3 */
>> diff --git a/src/ipa/ipu3/algorithms/grid.cpp b/src/ipa/ipu3/algorithms/grid.cpp
>> index 3578f41b..e42a760d 100644
>> --- a/src/ipa/ipu3/algorithms/grid.cpp
>> +++ b/src/ipa/ipu3/algorithms/grid.cpp
>> @@ -78,6 +78,12 @@ int Grid::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>> return 0;
>> }
>>
>> +void Grid::prepare(IPAContext &context, ipu3_uapi_params ¶ms)
>> +{
>> + /* Update the IPU3 parameters with new calculated grid */
>> + params.acc_param.awb.config.grid = context.configuration.grid.bdsGrid;
>
> And this is where the spaghetti dish begins :-( It should be the
> responsibility of the AWB algorithm to set params.acc_param.awb.
>
Yes, I moved it at least 5 times while writing the patches as I could
not decide where it should be :-p.
>> +}
>> +
>> } /* namespace ipa::ipu3::algorithms */
>>
>> } /* namespace libcamera */
>> diff --git a/src/ipa/ipu3/algorithms/grid.h b/src/ipa/ipu3/algorithms/grid.h
>> index b4a51b42..89399bd2 100644
>> --- a/src/ipa/ipu3/algorithms/grid.h
>> +++ b/src/ipa/ipu3/algorithms/grid.h
>> @@ -19,6 +19,7 @@ public:
>> ~Grid() = default;
>>
>> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
>> + void prepare(IPAContext &context, ipu3_uapi_params ¶ms) override;
>> };
>>
>> } /* namespace ipa::ipu3::algorithms */
>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>> index ef7fec86..394a7a45 100644
>> --- a/src/ipa/ipu3/ipu3.cpp
>> +++ b/src/ipa/ipu3/ipu3.cpp
>> @@ -305,6 +305,9 @@ void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,
>>
>> void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
>> {
>> + for (auto const &algo : algorithms_)
>> + algo->prepare(context_, params_);
>> +
>> if (agcAlgo_->updateControls())
>> awbAlgo_->updateWbParameters(params_, agcAlgo_->gamma());
>>
>> diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/ipu3_awb.cpp
>> index 4ee5ee6f..911760b3 100644
>> --- a/src/ipa/ipu3/ipu3_awb.cpp
>> +++ b/src/ipa/ipu3/ipu3_awb.cpp
>> @@ -159,7 +159,6 @@ void IPU3Awb::initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, st
>> params.acc_param.awb.config.rgbs_thr_r = 8191;
>> params.acc_param.awb.config.rgbs_thr_gb = 8191;
>> params.acc_param.awb.config.rgbs_thr_b = 8191 | IPU3_UAPI_AWB_RGBS_THR_B_EN | IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT;
>> - params.acc_param.awb.config.grid = bdsGrid;
>> awbGrid_ = bdsGrid;
>>
>> params.use.acc_bnr = 1;
>
More information about the libcamera-devel
mailing list