Hi David, On 9/6/24 9:56 AM, David Laight wrote: > From: Mauro Carvalho Chehab >> Sent: 06 September 2024 07:16 >> >> Em Sat, 27 Jul 2024 14:51:56 +0200 >> Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> escreveu: >> >>> Using clamp_t() instead of min_t(max_t()) is easier to read. >>> >>> It also reduces the size of the preprocessed files by ~ 193 ko. >>> (see [1] for a discussion about it) >>> >>> $ ls -l ia_css_eed1_8.host*.i >>> 4829993 27 juil. 14:36 ia_css_eed1_8.host.old.i >>> 4636649 27 juil. 14:42 ia_css_eed1_8.host.new.i >>> >>> [1]: https://lore.kernel.org/all/23bdb6fc8d884ceebeb6e8b8653b8cfe@xxxxxxxxxxxxxxxx/ >>> >>> Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> >>> --- >>> .../isp/kernels/eed1_8/ia_css_eed1_8.host.c | 24 +++++++++---------- >>> 1 file changed, 12 insertions(+), 12 deletions(-) >>> >>> diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c >> b/drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c >>> index e4fc90f88e24..96c13ebc4331 100644 >>> --- a/drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c >>> +++ b/drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c >>> @@ -172,25 +172,25 @@ ia_css_eed1_8_vmem_encode( >>> base = shuffle_block * i; >>> >>> for (j = 0; j < IA_CSS_NUMBER_OF_DEW_ENHANCE_SEGMENTS; j++) { >>> - to->e_dew_enh_x[0][base + j] = min_t(int, max_t(int, >>> - from->dew_enhance_seg_x[j], 0), >>> - 8191); >>> - to->e_dew_enh_y[0][base + j] = min_t(int, max_t(int, >>> - from->dew_enhance_seg_y[j], -8192), >>> - 8191); >>> + to->e_dew_enh_x[0][base + j] = clamp_t(int, >>> + from->dew_enhance_seg_x[j], >>> + 0, 8191); >>> + to->e_dew_enh_y[0][base + j] = clamp_t(int, >>> + from->dew_enhance_seg_y[j], >>> + -8192, 8191); >> >> Such change introduces two warnings on smatch: >> >> drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c: >> drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c:177 >> ia_css_eed1_8_vmem_encode() warn: assigning (-8192) to unsigned variable 'to->e_dew_enh_y[0][base + >> j]' >> drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c: >> drivers/staging/media/atomisp/pci/isp/kernels/eed1_8/ia_css_eed1_8.host.c:182 >> ia_css_eed1_8_vmem_encode() warn: assigning (-8192) to unsigned variable 'to->e_dew_enh_a[0][base + >> j]' >> >> Should dew_enhance_seg_x and dew_enhance_seg_y be converted to signed? > > Someone clearly needs to read the code and work out what it is doing. Ack, I'm looking into this now. > First stage is to use clamp() (not clamp_t) to get warnings from the > compiler for the RHS. I already changed this to plain clamp() when merging it, that does not cause any changes since the clamp-s are doing clamp(s32, -8192, 8192) Regards, Hans
