Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
* [PATCH v2] media: venus: use div64_u64() instead of do_div()
@ 2024-01-02 13:15 Himanshu Bhavani
  2024-01-04 23:14 ` David Laight
  0 siblings, 1 reply; 2+ messages in thread
From: Himanshu Bhavani @ 2024-01-02 13:15 UTC (permalink / raw)
  To: stanimir.k.varbanov, quic_vgarodia, agross, andersson,
	konrad.dybcio, mchehab, quic_dikshita
  Cc: himanshu.bhavani, Bryan O'Donoghue, linux-media,
	linux-arm-msm, linux-kernel

do_div() does a 64-by-32 division.
When the divisor is u64, do_div() truncates it to 32 bits,
this means it can test non-zero and be truncated to zero for
division.

fix do_div.cocci warning:
do_div() does a 64-by-32 division, please consider using div64_u64
instead.

Signed-off-by: Himanshu Bhavani <himanshu.bhavani@siliconsignals.io>
---
 drivers/media/platform/qcom/venus/venc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 44b13696cf82..ad6c31c272ac 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -409,13 +409,13 @@ static int venc_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
 	out->capability = V4L2_CAP_TIMEPERFRAME;
 
 	us_per_frame = timeperframe->numerator * (u64)USEC_PER_SEC;
-	do_div(us_per_frame, timeperframe->denominator);
+	us_per_frame = div64_u64(us_per_frame, timeperframe->denominator);
 
 	if (!us_per_frame)
 		return -EINVAL;
 
 	fps = (u64)USEC_PER_SEC;
-	do_div(fps, us_per_frame);
+	fps = div64_u64(fps, us_per_frame);
 
 	inst->timeperframe = *timeperframe;
 	inst->fps = fps;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* RE: [PATCH v2] media: venus: use div64_u64() instead of do_div()
  2024-01-02 13:15 [PATCH v2] media: venus: use div64_u64() instead of do_div() Himanshu Bhavani
@ 2024-01-04 23:14 ` David Laight
  0 siblings, 0 replies; 2+ messages in thread
From: David Laight @ 2024-01-04 23:14 UTC (permalink / raw)
  To: 'Himanshu Bhavani', stanimir.k.varbanov@gmail.com,
	quic_vgarodia@quicinc.com, agross@kernel.org,
	andersson@kernel.org, konrad.dybcio@linaro.org,
	mchehab@kernel.org, quic_dikshita@quicinc.com
  Cc: Bryan O'Donoghue, linux-media@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org

From: Himanshu Bhavani
> Sent: 02 January 2024 13:15
> 
> do_div() does a 64-by-32 division.
> When the divisor is u64, do_div() truncates it to 32 bits,
> this means it can test non-zero and be truncated to zero for
> division.
> 
> fix do_div.cocci warning:
> do_div() does a 64-by-32 division, please consider using div64_u64
> instead.

That message is really wrong, it should ask you to check the domains
of the divisor and dividend to ensure the quotient won't exceed 32bits.

I'm not sure about this code, but it looks like the second do_div()
could just be a divide, it is USEC_PER_SEC/n which is well inside 32bits.
The 'n' is the result of the first divide - so that is small as well.

64-by-64 divides are horribly slow on 32bit.
They are even about twice as slow as 64-by-32 on intel x64-64 chips.

	David

> 
> Signed-off-by: Himanshu Bhavani <himanshu.bhavani@siliconsignals.io>
> ---
>  drivers/media/platform/qcom/venus/venc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index 44b13696cf82..ad6c31c272ac 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -409,13 +409,13 @@ static int venc_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
>  	out->capability = V4L2_CAP_TIMEPERFRAME;
> 
>  	us_per_frame = timeperframe->numerator * (u64)USEC_PER_SEC;
> -	do_div(us_per_frame, timeperframe->denominator);
> +	us_per_frame = div64_u64(us_per_frame, timeperframe->denominator);
> 
>  	if (!us_per_frame)
>  		return -EINVAL;
> 
>  	fps = (u64)USEC_PER_SEC;
> -	do_div(fps, us_per_frame);
> +	fps = div64_u64(fps, us_per_frame);
> 
>  	inst->timeperframe = *timeperframe;
>  	inst->fps = fps;
> --
> 2.25.1
> 

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-01-04 23:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-02 13:15 [PATCH v2] media: venus: use div64_u64() instead of do_div() Himanshu Bhavani
2024-01-04 23:14 ` David Laight

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox