| Message ID | 20220331121032.2150025-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hello Kieran, Thank you for the patch, On 3/31/22 17:40, Kieran Bingham via libcamera-devel wrote: > Provide enforcement of the selection of the block_{width,height}_log2 > parameters to the capabilities of the hardware. > > While this selection is currently hardcoded to the minimum, providing > the restriction now allows for further dynamic sizing in the future and > documents the restrictions directly in code, making use of the already > existing constants. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > --- > > Note, this is kept separate from my other AF improvement series, as it > can be applied independantly - but due to a compilation failure if > kAfMinGridBlockWidth, kAfMaxGridBlockWidth kAfMinGridBlockHeight, and > kAfMaxGridBlockHeight are not used, this patch must be applied before > that series can be integrated. > > src/ipa/ipu3/algorithms/af.cpp | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > index 6c25d02ec7f0..abfaad6164f1 100644 > --- a/src/ipa/ipu3/algorithms/af.cpp > +++ b/src/ipa/ipu3/algorithms/af.cpp > @@ -139,6 +139,22 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > grid.height = kAfMinGridHeight; > grid.block_width_log2 = kAfMinGridBlockWidth; > grid.block_height_log2 = kAfMinGridBlockHeight; > + > + /* > + * \todo - while this clamping code is effectively a no-op, it satisfies > + * the compiler that the definitions regarding the hardware restrictions > + * are used, and paves the way to support dynamic grid sizing in the > + * future. While the block_{width,height}_log2 remain assigned to the > + * minimum, this code should be optimized out by the compiler. > + */ > + grid.block_width_log2 = std::clamp(grid.block_width_log2, > + kAfMinGridBlockWidth, > + kAfMaxGridBlockWidth); > + > + grid.block_height_log2 = std::clamp(grid.block_height_log2, > + kAfMinGridBlockHeight, > + kAfMaxGridBlockHeight); > + > grid.height_per_slice = kAfDefaultHeightPerSlice; > > /* Position the AF grid in the center of the BDS output. */
Hi Kieran, Thank you for the patch. On Thu, Mar 31, 2022 at 01:10:32PM +0100, Kieran Bingham via libcamera-devel wrote: > Provide enforcement of the selection of the block_{width,height}_log2 > parameters to the capabilities of the hardware. > > While this selection is currently hardcoded to the minimum, providing > the restriction now allows for further dynamic sizing in the future and > documents the restrictions directly in code, making use of the already > existing constants. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > > Note, this is kept separate from my other AF improvement series, as it > can be applied independantly - but due to a compilation failure if > kAfMinGridBlockWidth, kAfMaxGridBlockWidth kAfMinGridBlockHeight, and > kAfMaxGridBlockHeight are not used, this patch must be applied before > that series can be integrated. > > src/ipa/ipu3/algorithms/af.cpp | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > index 6c25d02ec7f0..abfaad6164f1 100644 > --- a/src/ipa/ipu3/algorithms/af.cpp > +++ b/src/ipa/ipu3/algorithms/af.cpp > @@ -139,6 +139,22 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > grid.height = kAfMinGridHeight; > grid.block_width_log2 = kAfMinGridBlockWidth; > grid.block_height_log2 = kAfMinGridBlockHeight; > + > + /* > + * \todo - while this clamping code is effectively a no-op, it satisfies > + * the compiler that the definitions regarding the hardware restrictions "... that the definitions of the hardware limits constants are used" ? "satisfy ... that" sounds a bit weird to me, but maybe it's just me. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + * are used, and paves the way to support dynamic grid sizing in the > + * future. While the block_{width,height}_log2 remain assigned to the > + * minimum, this code should be optimized out by the compiler. > + */ > + grid.block_width_log2 = std::clamp(grid.block_width_log2, > + kAfMinGridBlockWidth, > + kAfMaxGridBlockWidth); > + > + grid.block_height_log2 = std::clamp(grid.block_height_log2, > + kAfMinGridBlockHeight, > + kAfMaxGridBlockHeight); > + > grid.height_per_slice = kAfDefaultHeightPerSlice; > > /* Position the AF grid in the center of the BDS output. */
diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp index 6c25d02ec7f0..abfaad6164f1 100644 --- a/src/ipa/ipu3/algorithms/af.cpp +++ b/src/ipa/ipu3/algorithms/af.cpp @@ -139,6 +139,22 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) grid.height = kAfMinGridHeight; grid.block_width_log2 = kAfMinGridBlockWidth; grid.block_height_log2 = kAfMinGridBlockHeight; + + /* + * \todo - while this clamping code is effectively a no-op, it satisfies + * the compiler that the definitions regarding the hardware restrictions + * are used, and paves the way to support dynamic grid sizing in the + * future. While the block_{width,height}_log2 remain assigned to the + * minimum, this code should be optimized out by the compiler. + */ + grid.block_width_log2 = std::clamp(grid.block_width_log2, + kAfMinGridBlockWidth, + kAfMaxGridBlockWidth); + + grid.block_height_log2 = std::clamp(grid.block_height_log2, + kAfMinGridBlockHeight, + kAfMaxGridBlockHeight); + grid.height_per_slice = kAfDefaultHeightPerSlice; /* Position the AF grid in the center of the BDS output. */
Provide enforcement of the selection of the block_{width,height}_log2 parameters to the capabilities of the hardware. While this selection is currently hardcoded to the minimum, providing the restriction now allows for further dynamic sizing in the future and documents the restrictions directly in code, making use of the already existing constants. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- Note, this is kept separate from my other AF improvement series, as it can be applied independantly - but due to a compilation failure if kAfMinGridBlockWidth, kAfMaxGridBlockWidth kAfMinGridBlockHeight, and kAfMaxGridBlockHeight are not used, this patch must be applied before that series can be integrated. src/ipa/ipu3/algorithms/af.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)