[PATCH] [media] ti-vpe: get rid of some smatch warnings

[PATCH] [media] ti-vpe: get rid of some smatch warnings

[Mauro Carvalho Chehab]

When compiled on i386, it produces several warnings:

	./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
	./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
	./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
	./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
	./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
	./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue

I suspect that some gcc optimization could be causing the asm code to be
incorrectly generated. Splitting it into two macro calls fix the issues
and gets us rid of 6 smatch warnings, with is a good thing. As it should
not cause any troubles, as we're basically doing the same thing, let's
apply such change to vpe.c.

Cc: Benoit Parrot <bparrot@ti.com>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 drivers/media/platform/ti-vpe/vpe.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
index 0626593a8b22..f0156b7759e9 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -1615,20 +1615,32 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f,
 	 */
 	depth_bytes = depth >> 3;
 
-	if (depth_bytes == 3)
+	if (depth_bytes == 3) {
 		/*
 		 * if bpp is 3(as in some RGB formats), the pixel width doesn't
 		 * really help in ensuring line stride is 16 byte aligned
 		 */
 		w_align = 4;
-	else
+	} else {
 		/*
 		 * for the remainder bpp(4, 2 and 1), the pixel width alignment
 		 * can ensure a line stride alignment of 16 bytes. For example,
 		 * if bpp is 2, then the line stride can be 16 byte aligned if
 		 * the width is 8 byte aligned
 		 */
-		w_align = order_base_2(VPDMA_DESC_ALIGN / depth_bytes);
+
+		/*
+		 * HACK: using order_base_2() here causes lots of asm output
+		 * errors with smatch, on i386:
+		 * ./arch/x86/include/asm/bitops.h:457:22:
+		 *		 warning: asm output is not an lvalue
+		 * Perhaps some gcc optimization is doing the wrong thing
+		 * there.
+		 * Let's get rid of them by doing the calculus on two steps
+		 */
+		w_align = roundup_pow_of_two(VPDMA_DESC_ALIGN / depth_bytes);
+		w_align = ilog2(w_align);
+	}
 
 	v4l_bound_align_image(&pix->width, MIN_W, MAX_W, w_align,
 			      &pix->height, MIN_H, MAX_H, H_ALIGN,
-- 
2.9.3

Re: [PATCH] [media] ti-vpe: get rid of some smatch warnings

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 1462 bytes --]

Hi,

On 22/11/16 13:09, Mauro Carvalho Chehab wrote:
> When compiled on i386, it produces several warnings:
> 
> 	./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
> 	./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
> 	./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
> 	./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
> 	./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
> 	./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an lvalue
> 
> I suspect that some gcc optimization could be causing the asm code to be
> incorrectly generated. Splitting it into two macro calls fix the issues
> and gets us rid of 6 smatch warnings, with is a good thing. As it should
> not cause any troubles, as we're basically doing the same thing, let's
> apply such change to vpe.c.
> 
> Cc: Benoit Parrot <bparrot@ti.com>
> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
>  drivers/media/platform/ti-vpe/vpe.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)

I think the point of COMPILE_TEST is to improve the quality of the code.
This patch doesn't improve the quality, on the contrary.

If those warnings on a (buggy?) i386 gcc are a problem, I suggest
removing COMPILE_TEST for vpe.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]