cx88-dsp.c: missing __divdi3 on 32bit kernel

cx88-dsp.c: missing __divdi3 on 32bit kernel

[Miroslav Šustek]

Hello,
Commit 02fde69f31dc (and 7152a23142bc) on http://linuxtv.org/hg/v4l-dvb/
adds new file cx88-dsp.c which uses 64bit divisions, but these are somehow
not supported on 32bit kernels.

message during compile:
WARNING: "__divdi3" [/root/v4l-dvb/v4l/cx88xx.ko] undefined

Maybe we can use only s32, but I don't know if it's precise enough for that magic math.
Or use some ugly hacks to do 64bit division with 32bit variables.

What hardware do I need to test the cx88-dsp code?

Any suggestions?

- Miroslav

[PATCH] Re: cx88-dsp.c: missing __divdi3 on 32bit kernel

[Miroslav Šustek]

Well this patch should solve it.

I don't know how many samples are processed so:
First patch is for situation when N*N fits in s32.
Second one uses two divisions, but doesn't have any abnormal restrictions for N.

Personally I think that two divisions won't hurt. :)



----- FILE: cx88-dsp_64bit_math1.patch -----

cx88-dsp: fixing 64bit math on 32bit kernels

Note the limitation of N.
Personally I know nothing about possible size of samples array.

From: Miroslav Sustek <sustmidown@centrum.cz>
Signed-off-by: Miroslav Sustek <sustmidown@centrum.cz>

diff -r 8aa1d865373c linux/drivers/media/video/cx88/cx88-dsp.c
--- a/linux/drivers/media/video/cx88/cx88-dsp.c	Wed Apr 01 20:25:00 2009 +0000
+++ b/linux/drivers/media/video/cx88/cx88-dsp.c	Tue Apr 07 00:08:48 2009 +0200
@@ -100,13 +100,22 @@
 	s32 s_prev2 = 0;
 	s32 coeff = 2*int_cos(freq);
 	u32 i;
+
+	s64 tmp;
+	u32 remainder;
+
 	for (i = 0; i < N; i++) {
 		s32 s = x[i] + ((s64)coeff*s_prev/32768) - s_prev2;
 		s_prev2 = s_prev;
 		s_prev = s;
 	}
-	return (u32)(((s64)s_prev2*s_prev2 + (s64)s_prev*s_prev -
-		      (s64)coeff*s_prev2*s_prev/32768)/N/N);
+
+	tmp = (s64)s_prev2*s_prev2 + (s64)s_prev*s_prev -
+		      (s64)coeff*s_prev2*s_prev/32768;
+
+	/* XXX: N must be low enough so that N*N fits in s32.
+	 * Else we need two divisions. */
+	return (u32) div_s64_rem(tmp, N*N, &remainder);
 }
 
 static u32 freq_magnitude(s16 x[], u32 N, u32 freq)



----- FILE: cx88-dsp_64bit_math2.patch -----

cx88-dsp: fixing 64bit math on 32bit kernels

From: Miroslav Sustek <sustmidown@centrum.cz>
Signed-off-by: Miroslav Sustek <sustmidown@centrum.cz>

diff -r 8aa1d865373c linux/drivers/media/video/cx88/cx88-dsp.c
--- a/linux/drivers/media/video/cx88/cx88-dsp.c	Wed Apr 01 20:25:00 2009 +0000
+++ b/linux/drivers/media/video/cx88/cx88-dsp.c	Tue Apr 07 00:26:10 2009 +0200
@@ -100,13 +100,22 @@
 	s32 s_prev2 = 0;
 	s32 coeff = 2*int_cos(freq);
 	u32 i;
+
+	s64 tmp;
+	u32 remainder;
+
 	for (i = 0; i < N; i++) {
 		s32 s = x[i] + ((s64)coeff*s_prev/32768) - s_prev2;
 		s_prev2 = s_prev;
 		s_prev = s;
 	}
-	return (u32)(((s64)s_prev2*s_prev2 + (s64)s_prev*s_prev -
-		      (s64)coeff*s_prev2*s_prev/32768)/N/N);
+
+	tmp = (s64)s_prev2*s_prev2 + (s64)s_prev*s_prev -
+		      (s64)coeff*s_prev2*s_prev/32768;
+
+	tmp = div_s64_rem(tmp, N, &remainder);
+
+	return (u32)div_s64_rem(tmp, N, &remainder);
 }
 
 static u32 freq_magnitude(s16 x[], u32 N, u32 freq)

Re: [PATCH] Re: cx88-dsp.c: missing __divdi3 on 32bit kernel

[Marton Balint]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3094 bytes --]

On Mon, 6 Apr 2009, Miroslav Šustek wrote:

> Well this patch should solve it.
> 
> I don't know how many samples are processed so:
> First patch is for situation when N*N fits in s32.
> Second one uses two divisions, but doesn't have any abnormal restrictions for N.

Both patches are fine, beacuse in the current implementation N is not 
bigger than 576. Thanks for fixing this problem.

Regards,
  Marton


> 
> Personally I think that two divisions won't hurt. :)
> 
> 
> 
> ----- FILE: cx88-dsp_64bit_math1.patch -----
> 
> cx88-dsp: fixing 64bit math on 32bit kernels
> 
> Note the limitation of N.
> Personally I know nothing about possible size of samples array.
> 
> From: Miroslav Sustek <sustmidown@centrum.cz>
> Signed-off-by: Miroslav Sustek <sustmidown@centrum.cz>
> 
> diff -r 8aa1d865373c linux/drivers/media/video/cx88/cx88-dsp.c
> --- a/linux/drivers/media/video/cx88/cx88-dsp.c	Wed Apr 01 20:25:00 2009 +0000
> +++ b/linux/drivers/media/video/cx88/cx88-dsp.c	Tue Apr 07 00:08:48 2009 +0200
> @@ -100,13 +100,22 @@
>  	s32 s_prev2 = 0;
>  	s32 coeff = 2*int_cos(freq);
>  	u32 i;
> +
> +	s64 tmp;
> +	u32 remainder;
> +
>  	for (i = 0; i < N; i++) {
>  		s32 s = x[i] + ((s64)coeff*s_prev/32768) - s_prev2;
>  		s_prev2 = s_prev;
>  		s_prev = s;
>  	}
> -	return (u32)(((s64)s_prev2*s_prev2 + (s64)s_prev*s_prev -
> -		      (s64)coeff*s_prev2*s_prev/32768)/N/N);
> +
> +	tmp = (s64)s_prev2*s_prev2 + (s64)s_prev*s_prev -
> +		      (s64)coeff*s_prev2*s_prev/32768;
> +
> +	/* XXX: N must be low enough so that N*N fits in s32.
> +	 * Else we need two divisions. */
> +	return (u32) div_s64_rem(tmp, N*N, &remainder);
>  }
>  
>  static u32 freq_magnitude(s16 x[], u32 N, u32 freq)
> 
> 
> 
> ----- FILE: cx88-dsp_64bit_math2.patch -----
> 
> cx88-dsp: fixing 64bit math on 32bit kernels
> 
> From: Miroslav Sustek <sustmidown@centrum.cz>
> Signed-off-by: Miroslav Sustek <sustmidown@centrum.cz>
> 
> diff -r 8aa1d865373c linux/drivers/media/video/cx88/cx88-dsp.c
> --- a/linux/drivers/media/video/cx88/cx88-dsp.c	Wed Apr 01 20:25:00 2009 +0000
> +++ b/linux/drivers/media/video/cx88/cx88-dsp.c	Tue Apr 07 00:26:10 2009 +0200
> @@ -100,13 +100,22 @@
>  	s32 s_prev2 = 0;
>  	s32 coeff = 2*int_cos(freq);
>  	u32 i;
> +
> +	s64 tmp;
> +	u32 remainder;
> +
>  	for (i = 0; i < N; i++) {
>  		s32 s = x[i] + ((s64)coeff*s_prev/32768) - s_prev2;
>  		s_prev2 = s_prev;
>  		s_prev = s;
>  	}
> -	return (u32)(((s64)s_prev2*s_prev2 + (s64)s_prev*s_prev -
> -		      (s64)coeff*s_prev2*s_prev/32768)/N/N);
> +
> +	tmp = (s64)s_prev2*s_prev2 + (s64)s_prev*s_prev -
> +		      (s64)coeff*s_prev2*s_prev/32768;
> +
> +	tmp = div_s64_rem(tmp, N, &remainder);
> +
> +	return (u32)div_s64_rem(tmp, N, &remainder);
>  }
>  
>  static u32 freq_magnitude(s16 x[], u32 N, u32 freq)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] Re: cx88-dsp.c: missing __divdi3 on 32bit kernel

[Marton Balint]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2089 bytes --]

On Tue, 7 Apr 2009, Marton Balint wrote:
> On Mon, 6 Apr 2009, Miroslav Šustek wrote:
> 
> > Well this patch should solve it.
> > 
> > I don't know how many samples are processed so:
> > First patch is for situation when N*N fits in s32.
> > Second one uses two divisions, but doesn't have any abnormal restrictions for N.
> 
> Both patches are fine, beacuse in the current implementation N is not 
> bigger than 576. Thanks for fixing this problem.

It seems that an #include is missing to math64.h. Below is a patch that 
adds the missing include.

Regards,
   Marton

# HG changeset patch
# User Marton Balint <cus@fazekas.hu>
# Date 1239222228 -7200
# Node ID c07293f1b5e44a8614f8f84c6b4fc586a02e69eb
# Parent  202a1c7ec37968f35678980b5f3d9812e5961ef0
cx88: dsp: add missing include to math64.h

From: Marton Balint <cus@fazekas.hu>

This patch adds a missing include to math64.h and replaces div_s64_rem with
div_s64 since the remainder is not used anyway.

Priority: normal

Signed-off-by: Marton Balint <cus@fazekas.hu>

diff -r 202a1c7ec379 -r c07293f1b5e4 linux/drivers/media/video/cx88/cx88-dsp.c
--- a/linux/drivers/media/video/cx88/cx88-dsp.c	Mon Apr 06 23:07:04 2009 +0000
+++ b/linux/drivers/media/video/cx88/cx88-dsp.c	Wed Apr 08 22:23:48 2009 +0200
@@ -22,6 +22,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/jiffies.h>
+#include <linux/math64.h>
 
 #include "cx88.h"
 #include "cx88-reg.h"
@@ -100,9 +101,7 @@
 	s32 s_prev2 = 0;
 	s32 coeff = 2*int_cos(freq);
 	u32 i;
-
 	s64 tmp;
-	u32 remainder;
 
 	for (i = 0; i < N; i++) {
 		s32 s = x[i] + ((s64)coeff*s_prev/32768) - s_prev2;
@@ -113,9 +112,8 @@
 	tmp = (s64)s_prev2*s_prev2 + (s64)s_prev*s_prev -
 		      (s64)coeff*s_prev2*s_prev/32768;
 
-	/* XXX: N must be low enough so that N*N fits in s32.
-	 * Else we need two divisions. */
-	return (u32) div_s64_rem(tmp, N*N, &remainder);
+	/* N is low enough so N*N fits in s32. */
+	return (u32) div_s64(tmp, N*N);
 }
 
 static u32 freq_magnitude(s16 x[], u32 N, u32 freq)