[libcamera-devel] [PATCH v4 2/6] ipa: ipu3: Inlink fillParams() in fillParamsBuffer()
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Apr 6 00:53:23 CEST 2022
Hi Umang,
Thank you for the patch.
s/Inlink/Inline/ in the subject line (same for 3/6). It looks like I
wrote "inlinking" instead of "inlining" in my review of v3, sorry about
that :-)
On Thu, Mar 31, 2022 at 10:00:54PM +0530, Umang Jain via libcamera-devel wrote:
> Since we have moved away from switch/case on the operation ID,
> there's little reason to split the operation in two functions.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> src/ipa/ipu3/ipu3.cpp | 47 +++++++++++++++++--------------------------
> 1 file changed, 18 insertions(+), 29 deletions(-)
>
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 7779ad58..23a9033e 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -157,7 +157,6 @@ private:
> ControlInfoMap *ipaControls);
> void updateSessionConfiguration(const ControlInfoMap &sensorControls);
>
> - void fillParams(unsigned int frame, ipu3_uapi_params *params);
> void parseStatistics(unsigned int frame,
> int64_t frameTimestamp,
> const ipu3_uapi_stats_3a *stats);
> @@ -514,6 +513,9 @@ void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)
> * \brief Fill and return a buffer with ISP processing parameters for a frame
> * \param[in] frame The frame number
> * \param[in] bufferId ID of the parameter buffer to fill
> + *
> + * Algorithms are expected to fill the IPU3 parameter buffer for the next
> + * frame given their most recent processing of the ImgU statistics.
> */
> void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
> {
> @@ -527,7 +529,21 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
> ipu3_uapi_params *params =
> reinterpret_cast<ipu3_uapi_params *>(mem.data());
>
> - fillParams(frame, params);
> + /*
> + * The incoming params buffer may contain uninitialised data, or the
> + * parameters of previously queued frames. Clearing the entire buffer
> + * may be an expensive operation, and the kernel will only read from
> + * structures which have their associated use-flag set.
> + *
> + * It is the responsibility of the algorithms to set the use flags
> + * accordingly for any data structure they update during prepare().
> + */
> + params->use = {};
> +
> + for (auto const &algo : algorithms_)
> + algo->prepare(context_, params);
> +
> + paramsBufferReady.emit(frame);
> }
>
> /**
> @@ -570,33 +586,6 @@ void IPAIPU3::queueRequest(const uint32_t frame,
> /* \todo Start processing for 'frame' based on 'controls'. */
> }
>
> -/**
> - * \brief Fill the ImgU parameter buffer for the next frame
> - * \param[in] frame The number of the latest frame processed
> - * \param[out] params The parameter buffer to fill
> - *
> - * Algorithms are expected to fill the IPU3 parameter buffer for the next
> - * frame given their most recent processing of the ImgU statistics.
> - */
> -void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
> -{
> - /*
> - * The incoming params buffer may contain uninitialised data, or the
> - * parameters of previously queued frames. Clearing the entire buffer
> - * may be an expensive operation, and the kernel will only read from
> - * structures which have their associated use-flag set.
> - *
> - * It is the responsibility of the algorithms to set the use flags
> - * accordingly for any data structure they update during prepare().
> - */
> - params->use = {};
> -
> - for (auto const &algo : algorithms_)
> - algo->prepare(context_, params);
> -
> - paramsBufferReady.emit(frame);
> -}
> -
> /**
> * \brief Process the statistics generated by the ImgU
> * \param[in] frame The number of the latest frame processed
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list