[PATCH 1/6] Use LUT based implementation for (co)sine functions

[PATCH 1/6] Use LUT based implementation for (co)sine functions

[Prashant Laddha]

Replaced Taylor series calculation for (co)sine with a
look up table (LUT) for sine values.

Also reworked fixed point implementation to reduce rounding errors.

Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Antti Palosaari <crope@iki.fi>
Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

Signed-off-by: Prashant Laddha <prladdha@cisco.com>
---
 drivers/media/platform/vivid/Makefile        |   2 +-
 drivers/media/platform/vivid/vivid-sdr-cap.c |  87 ++++++-------
 drivers/media/platform/vivid/vivid-sin.c     | 184 +++++++++++++++++++++++++++
 drivers/media/platform/vivid/vivid-sin.h     |  31 +++++
 4 files changed, 252 insertions(+), 52 deletions(-)
 create mode 100644 drivers/media/platform/vivid/vivid-sin.c
 create mode 100644 drivers/media/platform/vivid/vivid-sin.h

diff --git a/drivers/media/platform/vivid/Makefile b/drivers/media/platform/vivid/Makefile
index 756fc12..9d5fe1c 100644
--- a/drivers/media/platform/vivid/Makefile
+++ b/drivers/media/platform/vivid/Makefile
@@ -2,5 +2,5 @@ vivid-objs := vivid-core.o vivid-ctrls.o vivid-vid-common.o vivid-vbi-gen.o \
 		vivid-vid-cap.o vivid-vid-out.o vivid-kthread-cap.o vivid-kthread-out.o \
 		vivid-radio-rx.o vivid-radio-tx.o vivid-radio-common.o \
 		vivid-rds-gen.o vivid-sdr-cap.o vivid-vbi-cap.o vivid-vbi-out.o \
-		vivid-osd.o vivid-tpg.o vivid-tpg-colors.o
+		vivid-osd.o vivid-tpg.o vivid-tpg-colors.o vivid-sin.o
 obj-$(CONFIG_VIDEO_VIVID) += vivid.o
diff --git a/drivers/media/platform/vivid/vivid-sdr-cap.c b/drivers/media/platform/vivid/vivid-sdr-cap.c
index 4af55f1..1f8b328 100644
--- a/drivers/media/platform/vivid/vivid-sdr-cap.c
+++ b/drivers/media/platform/vivid/vivid-sdr-cap.c
@@ -31,6 +31,7 @@
 #include "vivid-core.h"
 #include "vivid-ctrls.h"
 #include "vivid-sdr-cap.h"
+#include "vivid-sin.h"
 
 static const struct v4l2_frequency_band bands_adc[] = {
 	{
@@ -423,40 +424,17 @@ int vidioc_g_fmt_sdr_cap(struct file *file, void *fh, struct v4l2_format *f)
 	return 0;
 }
 
-#define FIXP_FRAC    (1 << 15)
-#define FIXP_PI      ((int)(FIXP_FRAC * 3.141592653589))
-
-/* cos() from cx88 driver: cx88-dsp.c */
-static s32 fixp_cos(unsigned int x)
-{
-	u32 t2, t4, t6, t8;
-	u16 period = x / FIXP_PI;
-
-	if (period % 2)
-		return -fixp_cos(x - FIXP_PI);
-	x = x % FIXP_PI;
-	if (x > FIXP_PI/2)
-		return -fixp_cos(FIXP_PI/2 - (x % (FIXP_PI/2)));
-	/* Now x is between 0 and FIXP_PI/2.
-	 * To calculate cos(x) we use it's Taylor polinom. */
-	t2 = x*x/FIXP_FRAC/2;
-	t4 = t2*x/FIXP_FRAC*x/FIXP_FRAC/3/4;
-	t6 = t4*x/FIXP_FRAC*x/FIXP_FRAC/5/6;
-	t8 = t6*x/FIXP_FRAC*x/FIXP_FRAC/7/8;
-	return FIXP_FRAC-t2+t4-t6+t8;
-}
-
-static inline s32 fixp_sin(unsigned int x)
-{
-	return -fixp_cos(x + (FIXP_PI / 2));
-}
-
 void vivid_sdr_cap_process(struct vivid_dev *dev, struct vivid_buffer *buf)
 {
 	u8 *vbuf = vb2_plane_vaddr(&buf->vb, 0);
 	unsigned long i;
 	unsigned long plane_size = vb2_plane_size(&buf->vb, 0);
-	int fixp_src_phase_step, fixp_i, fixp_q;
+	int fixp_i, fixp_q;
+
+	u32 adc_freq;
+	u32 sig_freq;
+	s32 src_phase_inc;
+	s32 mod_phase_inc;
 
 	/*
 	 * TODO: Generated beep tone goes very crackly when sample rate is
@@ -466,34 +444,41 @@ void vivid_sdr_cap_process(struct vivid_dev *dev, struct vivid_buffer *buf)
 
 	/* calculate phase step */
 	#define BEEP_FREQ 1000 /* 1kHz beep */
-	fixp_src_phase_step = DIV_ROUND_CLOSEST(2 * FIXP_PI * BEEP_FREQ,
-			dev->sdr_adc_freq);
+
+	adc_freq = dev->sdr_adc_freq;   /* samples per sec*/
+	sig_freq = BEEP_FREQ;           /* cycles per sec */
+	src_phase_inc = phase_per_sample(sig_freq, adc_freq);
 
 	for (i = 0; i < plane_size; i += 2) {
-		dev->sdr_fixp_mod_phase += fixp_cos(dev->sdr_fixp_src_phase);
-		dev->sdr_fixp_src_phase += fixp_src_phase_step;
 
-		/*
-		 * Transfer phases to [0 / 2xPI] in order to avoid variable
-		 * overflow and make it suitable for cosine implementation
-		 * used, which does not support negative angles.
-		 */
-		while (dev->sdr_fixp_mod_phase < (0 * FIXP_PI))
-			dev->sdr_fixp_mod_phase += (2 * FIXP_PI);
-		while (dev->sdr_fixp_mod_phase > (2 * FIXP_PI))
-			dev->sdr_fixp_mod_phase -= (2 * FIXP_PI);
+		mod_phase_inc = calc_cos(dev->sdr_fixp_src_phase);
+		dev->sdr_fixp_src_phase += src_phase_inc;
+
+		while (dev->sdr_fixp_src_phase >= ((44 << FIX_PT_PREC)/7))
+			dev->sdr_fixp_src_phase -= ((44 << FIX_PT_PREC)/7);
+
+		mod_phase_inc <<= FIX_PT_PREC;
+		mod_phase_inc /= 1275;
+
+		dev->sdr_fixp_mod_phase += mod_phase_inc;
+
+		while (dev->sdr_fixp_mod_phase < 0)
+			dev->sdr_fixp_mod_phase += ((44 << FIX_PT_PREC) / 7);
 
-		while (dev->sdr_fixp_src_phase > (2 * FIXP_PI))
-			dev->sdr_fixp_src_phase -= (2 * FIXP_PI);
+		while (dev->sdr_fixp_mod_phase >= ((44 << FIX_PT_PREC) / 7))
+			dev->sdr_fixp_mod_phase -= ((44 << FIX_PT_PREC) / 7);
 
-		fixp_i = fixp_cos(dev->sdr_fixp_mod_phase);
-		fixp_q = fixp_sin(dev->sdr_fixp_mod_phase);
+		fixp_i = calc_sin(dev->sdr_fixp_mod_phase);
+		fixp_q = calc_cos(dev->sdr_fixp_mod_phase);
 
 		/* convert 'fixp float' to u8 */
-		/* u8 = X * 127.5f + 127.5f; where X is float [-1.0 / +1.0] */
-		fixp_i = fixp_i * 1275 + FIXP_FRAC * 1275;
-		fixp_q = fixp_q * 1275 + FIXP_FRAC * 1275;
-		*vbuf++ = DIV_ROUND_CLOSEST(fixp_i, FIXP_FRAC * 10);
-		*vbuf++ = DIV_ROUND_CLOSEST(fixp_q, FIXP_FRAC * 10);
+		/* u8 = X * 127.5f + 127.5f; where X is float [-1.0 / +1.0]
+		The values stored in sin look table are pre-multipied with 1275.
+		So, only do addition */
+
+		fixp_i += 1275;
+		fixp_q += 1275;
+		*vbuf++ = DIV_ROUND_CLOSEST(fixp_i, 10);
+		*vbuf++ = DIV_ROUND_CLOSEST(fixp_q, 10);
 	}
 }
diff --git a/drivers/media/platform/vivid/vivid-sin.c b/drivers/media/platform/vivid/vivid-sin.c
new file mode 100644
index 0000000..e3d6149
--- /dev/null
+++ b/drivers/media/platform/vivid/vivid-sin.c
@@ -0,0 +1,184 @@
+/*
+ * vivid-sin.c - software defined radio support functions.
+ *
+ * Copyright 2014 Cisco Systems, Inc. and/or its affiliates. All rights
+ * reserved.
+ *
+ * This program is free software; you may redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <linux/errno.h>
+#include <linux/kernel.h>
+
+#include "vivid-sin.h"
+
+#define SIN_TAB_SIZE 256
+
+	/*TODO- Reduce the size of the table */
+/* Since sinewave is symmetric, it can be represented using only quarter
+   of the samples compared to the number of samples used below  */
+
+static s32 sin[257] = {
+	     0,    31,    63,    94,   125,   156,   187,   218,
+	   249,   279,   310,   340,   370,   400,   430,   459,
+	   488,   517,   545,   573,   601,   628,   655,   682,
+	   708,   734,   760,   784,   809,   833,   856,   879,
+	   902,   923,   945,   965,   986,  1005,  1024,  1042,
+	  1060,  1077,  1094,  1109,  1124,  1139,  1153,  1166,
+	  1178,  1190,  1200,  1211,  1220,  1229,  1237,  1244,
+	  1251,  1256,  1261,  1265,  1269,  1272,  1273,  1275,
+	  1275,  1275,  1273,  1272,  1269,  1265,  1261,  1256,
+	  1251,  1244,  1237,  1229,  1220,  1211,  1200,  1190,
+	  1178,  1166,  1153,  1139,  1124,  1109,  1094,  1077,
+	  1060,  1042,  1024,  1005,   986,   965,   945,   923,
+	   902,   879,   856,   833,   809,   784,   760,   734,
+	   708,   682,   655,   628,   601,   573,   545,   517,
+	   488,   459,   430,   400,   370,   340,   310,   279,
+	   249,   218,   187,   156,   125,    94,    63,    31,
+	     0,   -31,   -63,   -94,  -125,   156,  -187,  -218,
+	  -249,  -279,  -310,  -340,  -370,  -400,  -430,  -459,
+	  -488,  -517,  -545,  -573,  -601,  -628,  -655,  -682,
+	  -708,  -734,  -760,  -784,  -809,  -833,  -856,  -879,
+	  -902,  -923,  -945,  -965,  -986, -1005, -1024, -1042,
+	 -1060, -1077, -1094, -1109, -1124, -1139, -1153, -1166,
+	 -1178, -1190, -1200, -1211, -1220, -1229, -1237, -1244,
+	 -1251, -1256, -1261, -1265, -1269, -1272, -1273, -1275,
+	 -1275, -1275, -1273, -1272, -1269, -1265, -1261, -1256,
+	 -1251, -1244, -1237, -1229, -1220, -1211, -1200, -1190,
+	 -1178, -1166, -1153, -1139, -1124, -1109, -1094, -1077,
+	 -1060, -1042, -1024, -1005,  -986,  -965,  -945,  -923,
+	  -902,  -879,  -856,  -833,  -809,  -784,  -760,  -734,
+	  -708,  -682,  -655,  -628,  -601,  -573,  -545,  -517,
+	  -488,  -459,  -430,  -400,  -370,  -340,  -310,  -279,
+	  -249,  -218,  -187,  -156,  -125,   -94,   -63,   -31,
+	     0
+	};
+
+/*
+ * Calculation of sine is implemented using a look up table for range of
+ * phase values from 0 to 2*pi. Look table contains finite entries, say N.
+ *
+ * Since sinusoid are periodic with period 2*pi, look table stores entries
+ * for phase values from 0 to 2*pi.
+ *
+ * The interval [0,2*pi] is divided into N equal intervals, each representing
+ * a phase increment of (2*pi/N)
+ *
+ * The index 'n' in look up table stores sine value for phase (2*pi*n/N) as -
+ * sin_tab[N] = {sin(0), sin(1*(2*pi/N)),sin(2*(2*pi/N)), ..., sin(N*(2*pi/N))}
+ *
+   |---------|---------|---------|---- . . . .|---------| . . . . |---------|
+   0    (2*pi/N)  (2*2*pi/N) (3*2*pi/N)   (n*2*pi/N)                       2*pi
+ *
+ * Generation of sine waveform with different frequencies -
+ *
+ * Consider a sine tone with frequency 'f', so,
+ *      num_cycles_per_sec = f
+ * Let sampling frequency be 'Fs' so,
+ *     num_samples_per_sec = Fs
+ * So, num_samples_per_cycle = Fs/f
+ * Let Nc = Fs/f
+ *
+ * That is, number of samples during one interval of 0 tp 2*pi = Nc, each
+ * sample represents a phase increment of (2*pi/Nc = 2*pi*f/Fs).
+ *
+ * So, for "k" th sample the phase, phi =  k*(2*pi/Nc)
+ *
+ * The sine value at phase increments of (2*pi / Nc) is calculated by
+ * interpolating two adjecent samples from sine look table
+ *
+ * As an example, sine value for phase (phi) is calculated below
+ *
+ * 1. Find the interval [(n*2*pi/N), ((n+1)*2*pi/N)] to which phi belongs
+
+ *  0    (2*pi/N)      (n*2*pi/N) ((n+1)*2*pi/N)                     2*pi
+ *  |---------| . . . . . |---------|---------|  . . . . .  |---------|
+ *			        ^
+ *			 d0-- > | < --d1
+ *			        |
+ *			       phi = k*(2*pi/Nc)
+ *
+ * For phase (phi) the index n in sine table is given by (phi)/(2*pi/N)
+ *        n = integer part of ( phi / (2*pi / N) )
+ *
+ * 2. Find distance d0 and d1 using fractional arithmatic.
+ *        d0 =  phi - n*(2*pi / N),
+ *        d1 =  2*pi / N - d0
+ *
+ * 3. The calculations of fractions are done using fixed point implementation
+ *
+ * 4. To improve the precision of fixed point implementations, divisions
+ *    in different calculations are delayed till last operations. Say,
+ *    d0 = phi - n*(2*pi / N)
+ *    d0 = phi - n * (2 * (22 / 7) / N) , substitute pi = 22 / 7
+ *    d0 = phi - (n * 44 * N) / 7
+ */
+s32 calc_sin(u32 phase)
+{
+	u32 index;
+	u32 d0;
+	u32 d1;
+	s32 result;
+	u64 temp0;
+	u64 temp1;
+
+	temp0 = phase * SIN_TAB_SIZE;
+	index = (temp0 * 7) / (44 << FIX_PT_PREC);
+
+	temp0 = (temp0 * 7) / 44;
+	temp1 = index << FIX_PT_PREC;
+
+	d1 =  temp0 - temp1;
+	d0 = (1 << FIX_PT_PREC) - d1;
+
+	result = (d0 * sin[index % 256] + d1 * sin[(index+1)%256]);
+
+	return result >> FIX_PT_PREC;
+}
+
+s32 calc_cos(u32 phase)
+{
+	u32 index;
+	u32 d0;
+	u32 d1;
+	s32 result;
+	u64 temp0;
+	u64 temp1;
+
+	temp0 = phase * SIN_TAB_SIZE;
+	index = (temp0 * 7) / (44 << FIX_PT_PREC);
+
+	temp0 = (temp0 * 7) / 44;
+	temp1 = index << FIX_PT_PREC;
+
+	d1 =  temp0 - temp1;
+	d0 = (1 << FIX_PT_PREC) - d1;
+
+	index += 64;
+	result = (d0 * sin[index % 256] + d1 * sin[(index+1)%256]);
+
+	return result >> FIX_PT_PREC;
+}
+
+u32 phase_per_sample(u32 signal_freq, u32 sampling_freq)
+{
+	/* phase increment or decrement with each sample is given by
+	 *  (2 x Pi x signal frequency)/sampling frequency
+	 * To get a better accuracy with fixed point implementation we use
+	 *  Pi = 22/7
+	 * */
+	u64 temp = 44 * ((u64)signal_freq << FIX_PT_PREC);
+
+	return temp / (7*sampling_freq);
+}
diff --git a/drivers/media/platform/vivid/vivid-sin.h b/drivers/media/platform/vivid/vivid-sin.h
new file mode 100644
index 0000000..6c8bab2
--- /dev/null
+++ b/drivers/media/platform/vivid/vivid-sin.h
@@ -0,0 +1,31 @@
+/*
+ * vivid-sin.h - support functions used to generate (co)sine waveforms for
+ *               software defined radio.
+ *
+ * Copyright 2014 Cisco Systems, Inc. and/or its affiliates. All rights
+ * reserved.
+ *
+ * This program is free software; you may redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef _VIVID_SIN_H_
+#define _VIVID_SIN_H_
+
+#define FIX_PT_PREC 16
+
+s32 calc_sin(u32 phase);
+s32 calc_cos(u32 phase);
+u32 phase_per_sample(u32 signal_freq, u32 sampling_freq);
+
+#endif
-- 
1.9.1

[PATCH 2/6] Vivid sine gen: Optimization for sine LUT size

[Prashant Laddha]

Exploiting the symmetry and repetitive nature of sine waveform
to reduce size of sine LUT. Values up to phase <= pi/4, can be
used to calculate sine for remaining phases.

Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Antti Palosaari <crope@iki.fi>
Signed-off-by: Prashant Laddha <prladdha@cisco.com>
---
 drivers/media/platform/vivid/vivid-sin.c | 74 ++++++++++++++------------------
 1 file changed, 32 insertions(+), 42 deletions(-)

diff --git a/drivers/media/platform/vivid/vivid-sin.c b/drivers/media/platform/vivid/vivid-sin.c
index e3d6149..2ed9f7f 100644
--- a/drivers/media/platform/vivid/vivid-sin.c
+++ b/drivers/media/platform/vivid/vivid-sin.c
@@ -25,46 +25,38 @@
 
 #define SIN_TAB_SIZE 256
 
-	/*TODO- Reduce the size of the table */
-/* Since sinewave is symmetric, it can be represented using only quarter
-   of the samples compared to the number of samples used below  */
-
-static s32 sin[257] = {
-	     0,    31,    63,    94,   125,   156,   187,   218,
-	   249,   279,   310,   340,   370,   400,   430,   459,
-	   488,   517,   545,   573,   601,   628,   655,   682,
-	   708,   734,   760,   784,   809,   833,   856,   879,
-	   902,   923,   945,   965,   986,  1005,  1024,  1042,
-	  1060,  1077,  1094,  1109,  1124,  1139,  1153,  1166,
-	  1178,  1190,  1200,  1211,  1220,  1229,  1237,  1244,
-	  1251,  1256,  1261,  1265,  1269,  1272,  1273,  1275,
-	  1275,  1275,  1273,  1272,  1269,  1265,  1261,  1256,
-	  1251,  1244,  1237,  1229,  1220,  1211,  1200,  1190,
-	  1178,  1166,  1153,  1139,  1124,  1109,  1094,  1077,
-	  1060,  1042,  1024,  1005,   986,   965,   945,   923,
-	   902,   879,   856,   833,   809,   784,   760,   734,
-	   708,   682,   655,   628,   601,   573,   545,   517,
-	   488,   459,   430,   400,   370,   340,   310,   279,
-	   249,   218,   187,   156,   125,    94,    63,    31,
-	     0,   -31,   -63,   -94,  -125,   156,  -187,  -218,
-	  -249,  -279,  -310,  -340,  -370,  -400,  -430,  -459,
-	  -488,  -517,  -545,  -573,  -601,  -628,  -655,  -682,
-	  -708,  -734,  -760,  -784,  -809,  -833,  -856,  -879,
-	  -902,  -923,  -945,  -965,  -986, -1005, -1024, -1042,
-	 -1060, -1077, -1094, -1109, -1124, -1139, -1153, -1166,
-	 -1178, -1190, -1200, -1211, -1220, -1229, -1237, -1244,
-	 -1251, -1256, -1261, -1265, -1269, -1272, -1273, -1275,
-	 -1275, -1275, -1273, -1272, -1269, -1265, -1261, -1256,
-	 -1251, -1244, -1237, -1229, -1220, -1211, -1200, -1190,
-	 -1178, -1166, -1153, -1139, -1124, -1109, -1094, -1077,
-	 -1060, -1042, -1024, -1005,  -986,  -965,  -945,  -923,
-	  -902,  -879,  -856,  -833,  -809,  -784,  -760,  -734,
-	  -708,  -682,  -655,  -628,  -601,  -573,  -545,  -517,
-	  -488,  -459,  -430,  -400,  -370,  -340,  -310,  -279,
-	  -249,  -218,  -187,  -156,  -125,   -94,   -63,   -31,
-	     0
+static s32 sin[65] = {
+	   0,   31,   63,   94,  125,  156,  187,  218,  249,  279,  310,  340,
+	 370,  400,  430,  459,  488,  517,  545,  573,  601,  628,  655,  682,
+	 708,  734,  760,  784,  809,  833,  856,  879,  902,  923,  945,  965,
+	 986, 1005, 1024, 1042, 1060, 1077, 1094, 1109, 1124, 1139, 1153, 1166,
+	1178, 1190, 1200, 1211, 1220, 1229, 1237, 1244, 1251, 1256, 1261, 1265,
+	1269, 1272, 1273, 1275, 1275
 	};
 
+static s32 get_sin_val(u32 index)
+{
+	u32 tab_index;
+	u32 new_index;
+
+	if (index <= 64)
+		return sin[index];
+	else if (index > 64 && index <= 128) {
+		tab_index = 64 - (index - 64);
+		return sin[tab_index];
+	} else if (index > 128 && index <= 192) {
+		tab_index = index - 128;
+		return (-1) * sin[tab_index];
+	} else if (index > 192 && index <= 255) {
+		tab_index = 64 - (index - 192);
+		return (-1) * sin[tab_index];
+	}
+
+	new_index = index % 256;
+	return get_sin_val(new_index);
+
+}
+
 /*
  * Calculation of sine is implemented using a look up table for range of
  * phase values from 0 to 2*pi. Look table contains finite entries, say N.
@@ -142,8 +134,7 @@ s32 calc_sin(u32 phase)
 	d1 =  temp0 - temp1;
 	d0 = (1 << FIX_PT_PREC) - d1;
 
-	result = (d0 * sin[index % 256] + d1 * sin[(index+1)%256]);
-
+	result = d0 * get_sin_val(index) + d1 * get_sin_val(index+1);
 	return result >> FIX_PT_PREC;
 }
 
@@ -166,8 +157,7 @@ s32 calc_cos(u32 phase)
 	d0 = (1 << FIX_PT_PREC) - d1;
 
 	index += 64;
-	result = (d0 * sin[index % 256] + d1 * sin[(index+1)%256]);
-
+	result = d0 * get_sin_val(index) + d1 * get_sin_val(index+1);
 	return result >> FIX_PT_PREC;
 }
 
-- 
1.9.1

[PATCH 3/6] Vivid sine gen: Refactor get_sin_val ()

[Prashant Laddha]

Removed recursion. Also reduced few if() checks.

Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Antti Palosaari <crope@iki.fi>
Signed-off-by: Prashant Laddha <prladdha@cisco.com>
---
 drivers/media/platform/vivid/vivid-sin.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/vivid/vivid-sin.c b/drivers/media/platform/vivid/vivid-sin.c
index 2ed9f7f..c9face9 100644
--- a/drivers/media/platform/vivid/vivid-sin.c
+++ b/drivers/media/platform/vivid/vivid-sin.c
@@ -36,25 +36,23 @@ static s32 sin[65] = {
 
 static s32 get_sin_val(u32 index)
 {
+	s32 sign = 1;
 	u32 tab_index;
 	u32 new_index;
 
-	if (index <= 64)
-		return sin[index];
-	else if (index > 64 && index <= 128) {
-		tab_index = 64 - (index - 64);
-		return sin[tab_index];
-	} else if (index > 128 && index <= 192) {
-		tab_index = index - 128;
-		return (-1) * sin[tab_index];
-	} else if (index > 192 && index <= 255) {
-		tab_index = 64 - (index - 192);
-		return (-1) * sin[tab_index];
-	}
-
-	new_index = index % 256;
-	return get_sin_val(new_index);
+	new_index = index & 0xFF; /* new_index = index % 256*/
 
+	if (new_index > 128)
+		sign = -1;
+
+	new_index = index & 0x7F; /* new_index = index % 256*/
+
+	if (new_index <= 64)
+		tab_index = new_index;
+	else
+		tab_index = 64 - (new_index - 64);
+
+	return sign * sin[tab_index];
 }
 
 /*
-- 
1.9.1

[PATCH 4/6] Vivid sine gen: Renamed SIN_TAB_SIZE to SIN_LUT_SIZE

[Prashant Laddha]

Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Antti Palosaari <crope@iki.fi>
Signed-off-by: Prashant Laddha <prladdha@cisco.com>
---
 drivers/media/platform/vivid/vivid-sin.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/vivid/vivid-sin.c b/drivers/media/platform/vivid/vivid-sin.c
index c9face9..1ba6df9 100644
--- a/drivers/media/platform/vivid/vivid-sin.c
+++ b/drivers/media/platform/vivid/vivid-sin.c
@@ -23,7 +23,7 @@
 
 #include "vivid-sin.h"
 
-#define SIN_TAB_SIZE 256
+#define SIN_LUT_SIZE 256
 
 static s32 sin[65] = {
 	   0,   31,   63,   94,  125,  156,  187,  218,  249,  279,  310,  340,
@@ -123,7 +123,7 @@ s32 calc_sin(u32 phase)
 	u64 temp0;
 	u64 temp1;
 
-	temp0 = phase * SIN_TAB_SIZE;
+	temp0 = phase * SIN_LUT_SIZE;
 	index = (temp0 * 7) / (44 << FIX_PT_PREC);
 
 	temp0 = (temp0 * 7) / 44;
@@ -145,7 +145,7 @@ s32 calc_cos(u32 phase)
 	u64 temp0;
 	u64 temp1;
 
-	temp0 = phase * SIN_TAB_SIZE;
+	temp0 = phase * SIN_LUT_SIZE;
 	index = (temp0 * 7) / (44 << FIX_PT_PREC);
 
 	temp0 = (temp0 * 7) / 44;
-- 
1.9.1

[PATCH 5/6] Vivid: Increased precision for (co)sine computation

[Prashant Laddha]

1.sin LUT is recomputed with precision of 16 bits to represent
fractional part. (lowest fraction that can be represented now
is 1/2^16, that is 0.000015).

2.Instead of using PI = 22/7 in intermediate calculation, use
precomputed value for 2PI

3 To avoid overflows, use 64 bit variables for intermediate
calculations

Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Antti Palosaari <crope@iki.fi>
Signed-off-by: Prashant Laddha <prladdha@cisco.com>
---
 drivers/media/platform/vivid/vivid-core.h    |  4 +-
 drivers/media/platform/vivid/vivid-sdr-cap.c | 33 +++++-----
 drivers/media/platform/vivid/vivid-sin.c     | 94 ++++++++++++++++------------
 drivers/media/platform/vivid/vivid-sin.h     |  9 +--
 4 files changed, 78 insertions(+), 62 deletions(-)

diff --git a/drivers/media/platform/vivid/vivid-core.h b/drivers/media/platform/vivid/vivid-core.h
index 6f4445a..ea5c5c8 100644
--- a/drivers/media/platform/vivid/vivid-core.h
+++ b/drivers/media/platform/vivid/vivid-core.h
@@ -434,8 +434,8 @@ struct vivid_dev {
 	struct list_head		sdr_cap_active;
 	unsigned			sdr_adc_freq;
 	unsigned			sdr_fm_freq;
-	int				sdr_fixp_src_phase;
-	int				sdr_fixp_mod_phase;
+	s64				sdr_fixp_src_phase;
+	s64				sdr_fixp_mod_phase;
 
 	bool				tstamp_src_is_soe;
 	bool				has_crop_cap;
diff --git a/drivers/media/platform/vivid/vivid-sdr-cap.c b/drivers/media/platform/vivid/vivid-sdr-cap.c
index 1f8b328..1e5abd7 100644
--- a/drivers/media/platform/vivid/vivid-sdr-cap.c
+++ b/drivers/media/platform/vivid/vivid-sdr-cap.c
@@ -429,12 +429,16 @@ void vivid_sdr_cap_process(struct vivid_dev *dev, struct vivid_buffer *buf)
 	u8 *vbuf = vb2_plane_vaddr(&buf->vb, 0);
 	unsigned long i;
 	unsigned long plane_size = vb2_plane_size(&buf->vb, 0);
-	int fixp_i, fixp_q;
+	s64 fixp_i;
+	s64 fixp_q;
 
 	u32 adc_freq;
 	u32 sig_freq;
-	s32 src_phase_inc;
-	s32 mod_phase_inc;
+	s64 src_phase_inc;
+	s64 mod_phase_inc;
+	s64 signal_offset = 1275; /* 127.5 would be added modulated signal*/
+
+	signal_offset <<= FIX_PT_PREC;
 
 	/*
 	 * TODO: Generated beep tone goes very crackly when sample rate is
@@ -454,30 +458,27 @@ void vivid_sdr_cap_process(struct vivid_dev *dev, struct vivid_buffer *buf)
 		mod_phase_inc = calc_cos(dev->sdr_fixp_src_phase);
 		dev->sdr_fixp_src_phase += src_phase_inc;
 
-		while (dev->sdr_fixp_src_phase >= ((44 << FIX_PT_PREC)/7))
-			dev->sdr_fixp_src_phase -= ((44 << FIX_PT_PREC)/7);
-
-		mod_phase_inc <<= FIX_PT_PREC;
-		mod_phase_inc /= 1275;
+		while (dev->sdr_fixp_src_phase >= FIX_PT_2PI)
+			dev->sdr_fixp_src_phase -= FIX_PT_2PI;
 
 		dev->sdr_fixp_mod_phase += mod_phase_inc;
 
 		while (dev->sdr_fixp_mod_phase < 0)
-			dev->sdr_fixp_mod_phase += ((44 << FIX_PT_PREC) / 7);
+			dev->sdr_fixp_mod_phase += FIX_PT_2PI;
 
-		while (dev->sdr_fixp_mod_phase >= ((44 << FIX_PT_PREC) / 7))
-			dev->sdr_fixp_mod_phase -= ((44 << FIX_PT_PREC) / 7);
+		while (dev->sdr_fixp_mod_phase >= FIX_PT_2PI)
+			dev->sdr_fixp_mod_phase -= FIX_PT_2PI;
 
 		fixp_i = calc_sin(dev->sdr_fixp_mod_phase);
 		fixp_q = calc_cos(dev->sdr_fixp_mod_phase);
 
 		/* convert 'fixp float' to u8 */
-		/* u8 = X * 127.5f + 127.5f; where X is float [-1.0 / +1.0]
-		The values stored in sin look table are pre-multipied with 1275.
-		So, only do addition */
+		/* u8 = X * 127.5f + 127.5f; where X is float [-1.0 / +1.0] */
 
-		fixp_i += 1275;
-		fixp_q += 1275;
+		fixp_i = fixp_i * 1275 + signal_offset;
+		fixp_q = fixp_q * 1275 + signal_offset;
+		fixp_i >>= FIX_PT_PREC;
+		fixp_q >>= FIX_PT_PREC;
 		*vbuf++ = DIV_ROUND_CLOSEST(fixp_i, 10);
 		*vbuf++ = DIV_ROUND_CLOSEST(fixp_q, 10);
 	}
diff --git a/drivers/media/platform/vivid/vivid-sin.c b/drivers/media/platform/vivid/vivid-sin.c
index 1ba6df9..24680ea 100644
--- a/drivers/media/platform/vivid/vivid-sin.c
+++ b/drivers/media/platform/vivid/vivid-sin.c
@@ -26,19 +26,23 @@
 #define SIN_LUT_SIZE 256
 
 static s32 sin[65] = {
-	   0,   31,   63,   94,  125,  156,  187,  218,  249,  279,  310,  340,
-	 370,  400,  430,  459,  488,  517,  545,  573,  601,  628,  655,  682,
-	 708,  734,  760,  784,  809,  833,  856,  879,  902,  923,  945,  965,
-	 986, 1005, 1024, 1042, 1060, 1077, 1094, 1109, 1124, 1139, 1153, 1166,
-	1178, 1190, 1200, 1211, 1220, 1229, 1237, 1244, 1251, 1256, 1261, 1265,
-	1269, 1272, 1273, 1275, 1275
+	    0,   1608,  3216,  4821,  6424,  8022,  9616, 11204,
+	12785,  14359, 15924, 17479, 19024, 20557, 22078, 23586,
+	25080,  26558, 28020, 29466, 30893, 32303, 33692, 35062,
+	36410,  37736, 39040, 40320, 41576, 42806, 44011, 45190,
+	46341,	47464, 48559, 49624, 50660, 51665, 52639, 53581,
+	54491,	55368, 56212, 57022, 57798, 58538, 59244, 59914,
+	60547,	61145, 61705, 62228, 62714, 63162, 63572, 63944,
+	64277,	64571, 64827, 65043, 65220, 65358, 65457, 65516,
+	65536
 	};
 
-static s32 get_sin_val(u32 index)
+static s64 get_sin_val(u32 index)
 {
 	s32 sign = 1;
 	u32 tab_index;
 	u32 new_index;
+	s64 result;
 
 	new_index = index & 0xFF; /* new_index = index % 256*/
 
@@ -52,7 +56,12 @@ static s32 get_sin_val(u32 index)
 	else
 		tab_index = 64 - (new_index - 64);
 
-	return sign * sin[tab_index];
+	/* If fixed point precision is more than the precision used to compute
+	 * sin table (16 bit currently), then multiply sine values */
+
+	result = sin[tab_index] << (FIX_PT_PREC - 16);
+
+	return sign * result;
 }
 
 /*
@@ -111,62 +120,67 @@ static s32 get_sin_val(u32 index)
  * 4. To improve the precision of fixed point implementations, divisions
  *    in different calculations are delayed till last operations. Say,
  *    d0 = phi - n*(2*pi / N)
- *    d0 = phi - n * (2 * (22 / 7) / N) , substitute pi = 22 / 7
- *    d0 = phi - (n * 44 * N) / 7
  */
-s32 calc_sin(u32 phase)
+s64 calc_sin(u64 phase)
 {
-	u32 index;
-	u32 d0;
-	u32 d1;
-	s32 result;
+	u64 index;
+	u64 d0;
+	u64 d1;
+	s64 result;
 	u64 temp0;
 	u64 temp1;
 
 	temp0 = phase * SIN_LUT_SIZE;
-	index = (temp0 * 7) / (44 << FIX_PT_PREC);
+	index = temp0 / (u64)(FIX_PT_2PI);
 
-	temp0 = (temp0 * 7) / 44;
-	temp1 = index << FIX_PT_PREC;
+	temp1 = index * (u64)(FIX_PT_2PI);
+	temp1 /= SIN_LUT_SIZE;
 
-	d1 =  temp0 - temp1;
-	d0 = (1 << FIX_PT_PREC) - d1;
+	d1 =  phase - temp1;
+	d0 = (1ULL << FIX_PT_PREC) - d1;
 
 	result = d0 * get_sin_val(index) + d1 * get_sin_val(index+1);
 	return result >> FIX_PT_PREC;
 }
 
-s32 calc_cos(u32 phase)
+s64 calc_cos(u64 phase)
 {
-	u32 index;
-	u32 d0;
-	u32 d1;
-	s32 result;
+	u64 index;
+	u64 d0;
+	u64 d1;
+	s64 result;
 	u64 temp0;
 	u64 temp1;
 
 	temp0 = phase * SIN_LUT_SIZE;
-	index = (temp0 * 7) / (44 << FIX_PT_PREC);
+	index = temp0 / (u64)(FIX_PT_2PI);
 
-	temp0 = (temp0 * 7) / 44;
-	temp1 = index << FIX_PT_PREC;
+	temp1 = index * (u64)(FIX_PT_2PI);
+	temp1 /= SIN_LUT_SIZE;
 
-	d1 =  temp0 - temp1;
-	d0 = (1 << FIX_PT_PREC) - d1;
+	d1 =  phase - temp1;
+	d0 = (1ULL << FIX_PT_PREC) - d1;
+
+	index += (SIN_LUT_SIZE / 4); /* offset for cosine values */
 
-	index += 64;
 	result = d0 * get_sin_val(index) + d1 * get_sin_val(index+1);
+
 	return result >> FIX_PT_PREC;
 }
 
-u32 phase_per_sample(u32 signal_freq, u32 sampling_freq)
+/* phase increment or decrement per sample is calculated as
+ *
+ *		phase covered in 1 cycle = 2.pi
+ *		phase_per_sample = 2.pi / num_samples_per_cycle
+ *
+ *		num_samples_per_cycle = samples_per_sec
+ *		phase_per_sample = (2.pi x cycles_per_sec) / samples_per_sec
+ * */
+
+u64 phase_per_sample(u32 signal_freq, u32 sampling_freq)
 {
-	/* phase increment or decrement with each sample is given by
-	 *  (2 x Pi x signal frequency)/sampling frequency
-	 * To get a better accuracy with fixed point implementation we use
-	 *  Pi = 22/7
-	 * */
-	u64 temp = 44 * ((u64)signal_freq << FIX_PT_PREC);
-
-	return temp / (7*sampling_freq);
+	u64 cycles_per_sec = signal_freq;
+	u64 samples_per_sec = sampling_freq;
+
+	return (FIX_PT_2PI * cycles_per_sec) / samples_per_sec;
 }
diff --git a/drivers/media/platform/vivid/vivid-sin.h b/drivers/media/platform/vivid/vivid-sin.h
index 6c8bab2..b27a94e 100644
--- a/drivers/media/platform/vivid/vivid-sin.h
+++ b/drivers/media/platform/vivid/vivid-sin.h
@@ -22,10 +22,11 @@
 #ifndef _VIVID_SIN_H_
 #define _VIVID_SIN_H_
 
-#define FIX_PT_PREC 16
+#define FIX_PT_PREC (24)
+#define FIX_PT_2PI (0x6487ED5)
 
-s32 calc_sin(u32 phase);
-s32 calc_cos(u32 phase);
-u32 phase_per_sample(u32 signal_freq, u32 sampling_freq);
+s64 calc_sin(u64 phase);
+s64 calc_cos(u64 phase);
+u64 phase_per_sample(u32 signal_freq, u32 sampling_freq);
 
 #endif
-- 
1.9.1

[PATCH 6/6] Vivid SDR: Modified equation used for generating FM

[Prashant Laddha]

FM (frequency modulated) signal for SDR is generated by varying the
phase. The phase variation is proportional to input signal. It was
observed that, the larger phase increments leads to discontinuties
in the signal recovered after demodulation. This was one of the
reason for broken (cracky) sine tone after SDR receiver.

This patch modifies the phase calculation on two counts -
1. Phase of modulated signal, is varied around a center value.
2. The extent of phase variation with respect to input
signal is reduced. This is equivalent to reducing modulation index.

FM signal generated using modifed equation, do not lead to broken
sine tone.

Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Antti Palosaari <crope@iki.fi>
Signed-off-by: Prashant Laddha <prladdha@cisco.com>
---
 drivers/media/platform/vivid/vivid-sdr-cap.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/vivid/vivid-sdr-cap.c b/drivers/media/platform/vivid/vivid-sdr-cap.c
index 1e5abd7..1e77771 100644
--- a/drivers/media/platform/vivid/vivid-sdr-cap.c
+++ b/drivers/media/platform/vivid/vivid-sdr-cap.c
@@ -455,7 +455,9 @@ void vivid_sdr_cap_process(struct vivid_dev *dev, struct vivid_buffer *buf)
 
 	for (i = 0; i < plane_size; i += 2) {
 
-		mod_phase_inc = calc_cos(dev->sdr_fixp_src_phase);
+		mod_phase_inc = calc_cos(dev->sdr_fixp_src_phase) / 4 +
+						src_phase_inc;
+
 		dev->sdr_fixp_src_phase += src_phase_inc;
 
 		while (dev->sdr_fixp_src_phase >= FIX_PT_2PI)
-- 
1.9.1

Improvements in (co)sine generation in vivid sdr

[Prashant Laddha (prladdha)]

Hi,

Sharing the patches towards some improvements in sine generation. This was
one of the item in to-do list for vivid.

Vivid generates a frequency modulated (FM) signal (I and Q components) for
SDR. FM signal is generated by varying the phase in proportion to input
signal. Basically it is implementing FM by doing a phase modulation. It
was reported that on SDR receiver, when the sine signal is recovered after
demodulation, the quality of tone is cracky.

Looking at time and frequency plot of the signal recovered after
demodulation, I found that sine wave discontinuous and frequency spectrum
was spread out a lot. Based on my discussion with Hans and Antti, there
were two suspected reasons 1. Rounding errors in calculations 2. The
equation used to compute modulated signal.

Please find the patches for -

1. Use look up table (LUT) for computing (co)sine values (patches 1 to 4)
This will replace Taylor series based computation. LUT based
implementation for (co)sine calculation have following advantages -
a. Rounding errors would be less.
b. One could easily control and improve the precision of calculation
c. Computationally more efficient

2. Increase precision of calculation (patch 5)
Use higher precision for sine LUT as well as all other calculations (phase
values etc). 

3. Modify equation used to generate sample for frequency modulated wave
(patch 6)

Please review these patches and share your comments. I will share the
time, frequency plots for signal seen before and after the proposed
improvements.

Thanks to Hans and Antti for helping me to understand the problem and
later reviewing the solution for it.

Regards,
Prashant

Re: [PATCH 1/6] Use LUT based implementation for (co)sine functions

[Mauro Carvalho Chehab]

Em Mon, 15 Dec 2014 14:49:17 +0530
Prashant Laddha <prladdha@cisco.com> escreveu:

> Replaced Taylor series calculation for (co)sine with a
> look up table (LUT) for sine values.

Kernel has already a LUT for sin/cos at:
	include/linux/fixp-arith.h

The best would be to either use it or improve its precision, if the one there
is not good enough.

Regards,
Mauro

> 
> Also reworked fixed point implementation to reduce rounding errors.
> 
> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Antti Palosaari <crope@iki.fi>
> Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> Signed-off-by: Prashant Laddha <prladdha@cisco.com>
> ---
>  drivers/media/platform/vivid/Makefile        |   2 +-
>  drivers/media/platform/vivid/vivid-sdr-cap.c |  87 ++++++-------
>  drivers/media/platform/vivid/vivid-sin.c     | 184 +++++++++++++++++++++++++++
>  drivers/media/platform/vivid/vivid-sin.h     |  31 +++++
>  4 files changed, 252 insertions(+), 52 deletions(-)
>  create mode 100644 drivers/media/platform/vivid/vivid-sin.c
>  create mode 100644 drivers/media/platform/vivid/vivid-sin.h
> 
> diff --git a/drivers/media/platform/vivid/Makefile b/drivers/media/platform/vivid/Makefile
> index 756fc12..9d5fe1c 100644
> --- a/drivers/media/platform/vivid/Makefile
> +++ b/drivers/media/platform/vivid/Makefile
> @@ -2,5 +2,5 @@ vivid-objs := vivid-core.o vivid-ctrls.o vivid-vid-common.o vivid-vbi-gen.o \
>  		vivid-vid-cap.o vivid-vid-out.o vivid-kthread-cap.o vivid-kthread-out.o \
>  		vivid-radio-rx.o vivid-radio-tx.o vivid-radio-common.o \
>  		vivid-rds-gen.o vivid-sdr-cap.o vivid-vbi-cap.o vivid-vbi-out.o \
> -		vivid-osd.o vivid-tpg.o vivid-tpg-colors.o
> +		vivid-osd.o vivid-tpg.o vivid-tpg-colors.o vivid-sin.o
>  obj-$(CONFIG_VIDEO_VIVID) += vivid.o
> diff --git a/drivers/media/platform/vivid/vivid-sdr-cap.c b/drivers/media/platform/vivid/vivid-sdr-cap.c
> index 4af55f1..1f8b328 100644
> --- a/drivers/media/platform/vivid/vivid-sdr-cap.c
> +++ b/drivers/media/platform/vivid/vivid-sdr-cap.c
> @@ -31,6 +31,7 @@
>  #include "vivid-core.h"
>  #include "vivid-ctrls.h"
>  #include "vivid-sdr-cap.h"
> +#include "vivid-sin.h"
>  
>  static const struct v4l2_frequency_band bands_adc[] = {
>  	{
> @@ -423,40 +424,17 @@ int vidioc_g_fmt_sdr_cap(struct file *file, void *fh, struct v4l2_format *f)
>  	return 0;
>  }
>  
> -#define FIXP_FRAC    (1 << 15)
> -#define FIXP_PI      ((int)(FIXP_FRAC * 3.141592653589))
> -
> -/* cos() from cx88 driver: cx88-dsp.c */
> -static s32 fixp_cos(unsigned int x)
> -{
> -	u32 t2, t4, t6, t8;
> -	u16 period = x / FIXP_PI;
> -
> -	if (period % 2)
> -		return -fixp_cos(x - FIXP_PI);
> -	x = x % FIXP_PI;
> -	if (x > FIXP_PI/2)
> -		return -fixp_cos(FIXP_PI/2 - (x % (FIXP_PI/2)));
> -	/* Now x is between 0 and FIXP_PI/2.
> -	 * To calculate cos(x) we use it's Taylor polinom. */
> -	t2 = x*x/FIXP_FRAC/2;
> -	t4 = t2*x/FIXP_FRAC*x/FIXP_FRAC/3/4;
> -	t6 = t4*x/FIXP_FRAC*x/FIXP_FRAC/5/6;
> -	t8 = t6*x/FIXP_FRAC*x/FIXP_FRAC/7/8;
> -	return FIXP_FRAC-t2+t4-t6+t8;
> -}
> -
> -static inline s32 fixp_sin(unsigned int x)
> -{
> -	return -fixp_cos(x + (FIXP_PI / 2));
> -}
> -
>  void vivid_sdr_cap_process(struct vivid_dev *dev, struct vivid_buffer *buf)
>  {
>  	u8 *vbuf = vb2_plane_vaddr(&buf->vb, 0);
>  	unsigned long i;
>  	unsigned long plane_size = vb2_plane_size(&buf->vb, 0);
> -	int fixp_src_phase_step, fixp_i, fixp_q;
> +	int fixp_i, fixp_q;
> +
> +	u32 adc_freq;
> +	u32 sig_freq;
> +	s32 src_phase_inc;
> +	s32 mod_phase_inc;
>  
>  	/*
>  	 * TODO: Generated beep tone goes very crackly when sample rate is
> @@ -466,34 +444,41 @@ void vivid_sdr_cap_process(struct vivid_dev *dev, struct vivid_buffer *buf)
>  
>  	/* calculate phase step */
>  	#define BEEP_FREQ 1000 /* 1kHz beep */
> -	fixp_src_phase_step = DIV_ROUND_CLOSEST(2 * FIXP_PI * BEEP_FREQ,
> -			dev->sdr_adc_freq);
> +
> +	adc_freq = dev->sdr_adc_freq;   /* samples per sec*/
> +	sig_freq = BEEP_FREQ;           /* cycles per sec */
> +	src_phase_inc = phase_per_sample(sig_freq, adc_freq);
>  
>  	for (i = 0; i < plane_size; i += 2) {
> -		dev->sdr_fixp_mod_phase += fixp_cos(dev->sdr_fixp_src_phase);
> -		dev->sdr_fixp_src_phase += fixp_src_phase_step;
>  
> -		/*
> -		 * Transfer phases to [0 / 2xPI] in order to avoid variable
> -		 * overflow and make it suitable for cosine implementation
> -		 * used, which does not support negative angles.
> -		 */
> -		while (dev->sdr_fixp_mod_phase < (0 * FIXP_PI))
> -			dev->sdr_fixp_mod_phase += (2 * FIXP_PI);
> -		while (dev->sdr_fixp_mod_phase > (2 * FIXP_PI))
> -			dev->sdr_fixp_mod_phase -= (2 * FIXP_PI);
> +		mod_phase_inc = calc_cos(dev->sdr_fixp_src_phase);
> +		dev->sdr_fixp_src_phase += src_phase_inc;
> +
> +		while (dev->sdr_fixp_src_phase >= ((44 << FIX_PT_PREC)/7))
> +			dev->sdr_fixp_src_phase -= ((44 << FIX_PT_PREC)/7);
> +
> +		mod_phase_inc <<= FIX_PT_PREC;
> +		mod_phase_inc /= 1275;
> +
> +		dev->sdr_fixp_mod_phase += mod_phase_inc;
> +
> +		while (dev->sdr_fixp_mod_phase < 0)
> +			dev->sdr_fixp_mod_phase += ((44 << FIX_PT_PREC) / 7);
>  
> -		while (dev->sdr_fixp_src_phase > (2 * FIXP_PI))
> -			dev->sdr_fixp_src_phase -= (2 * FIXP_PI);
> +		while (dev->sdr_fixp_mod_phase >= ((44 << FIX_PT_PREC) / 7))
> +			dev->sdr_fixp_mod_phase -= ((44 << FIX_PT_PREC) / 7);
>  
> -		fixp_i = fixp_cos(dev->sdr_fixp_mod_phase);
> -		fixp_q = fixp_sin(dev->sdr_fixp_mod_phase);
> +		fixp_i = calc_sin(dev->sdr_fixp_mod_phase);
> +		fixp_q = calc_cos(dev->sdr_fixp_mod_phase);
>  
>  		/* convert 'fixp float' to u8 */
> -		/* u8 = X * 127.5f + 127.5f; where X is float [-1.0 / +1.0] */
> -		fixp_i = fixp_i * 1275 + FIXP_FRAC * 1275;
> -		fixp_q = fixp_q * 1275 + FIXP_FRAC * 1275;
> -		*vbuf++ = DIV_ROUND_CLOSEST(fixp_i, FIXP_FRAC * 10);
> -		*vbuf++ = DIV_ROUND_CLOSEST(fixp_q, FIXP_FRAC * 10);
> +		/* u8 = X * 127.5f + 127.5f; where X is float [-1.0 / +1.0]
> +		The values stored in sin look table are pre-multipied with 1275.
> +		So, only do addition */
> +
> +		fixp_i += 1275;
> +		fixp_q += 1275;
> +		*vbuf++ = DIV_ROUND_CLOSEST(fixp_i, 10);
> +		*vbuf++ = DIV_ROUND_CLOSEST(fixp_q, 10);
>  	}
>  }
> diff --git a/drivers/media/platform/vivid/vivid-sin.c b/drivers/media/platform/vivid/vivid-sin.c
> new file mode 100644
> index 0000000..e3d6149
> --- /dev/null
> +++ b/drivers/media/platform/vivid/vivid-sin.c
> @@ -0,0 +1,184 @@
> +/*
> + * vivid-sin.c - software defined radio support functions.
> + *
> + * Copyright 2014 Cisco Systems, Inc. and/or its affiliates. All rights
> + * reserved.
> + *
> + * This program is free software; you may redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +
> +#include "vivid-sin.h"
> +
> +#define SIN_TAB_SIZE 256
> +
> +	/*TODO- Reduce the size of the table */
> +/* Since sinewave is symmetric, it can be represented using only quarter
> +   of the samples compared to the number of samples used below  */
> +
> +static s32 sin[257] = {
> +	     0,    31,    63,    94,   125,   156,   187,   218,
> +	   249,   279,   310,   340,   370,   400,   430,   459,
> +	   488,   517,   545,   573,   601,   628,   655,   682,
> +	   708,   734,   760,   784,   809,   833,   856,   879,
> +	   902,   923,   945,   965,   986,  1005,  1024,  1042,
> +	  1060,  1077,  1094,  1109,  1124,  1139,  1153,  1166,
> +	  1178,  1190,  1200,  1211,  1220,  1229,  1237,  1244,
> +	  1251,  1256,  1261,  1265,  1269,  1272,  1273,  1275,
> +	  1275,  1275,  1273,  1272,  1269,  1265,  1261,  1256,
> +	  1251,  1244,  1237,  1229,  1220,  1211,  1200,  1190,
> +	  1178,  1166,  1153,  1139,  1124,  1109,  1094,  1077,
> +	  1060,  1042,  1024,  1005,   986,   965,   945,   923,
> +	   902,   879,   856,   833,   809,   784,   760,   734,
> +	   708,   682,   655,   628,   601,   573,   545,   517,
> +	   488,   459,   430,   400,   370,   340,   310,   279,
> +	   249,   218,   187,   156,   125,    94,    63,    31,
> +	     0,   -31,   -63,   -94,  -125,   156,  -187,  -218,
> +	  -249,  -279,  -310,  -340,  -370,  -400,  -430,  -459,
> +	  -488,  -517,  -545,  -573,  -601,  -628,  -655,  -682,
> +	  -708,  -734,  -760,  -784,  -809,  -833,  -856,  -879,
> +	  -902,  -923,  -945,  -965,  -986, -1005, -1024, -1042,
> +	 -1060, -1077, -1094, -1109, -1124, -1139, -1153, -1166,
> +	 -1178, -1190, -1200, -1211, -1220, -1229, -1237, -1244,
> +	 -1251, -1256, -1261, -1265, -1269, -1272, -1273, -1275,
> +	 -1275, -1275, -1273, -1272, -1269, -1265, -1261, -1256,
> +	 -1251, -1244, -1237, -1229, -1220, -1211, -1200, -1190,
> +	 -1178, -1166, -1153, -1139, -1124, -1109, -1094, -1077,
> +	 -1060, -1042, -1024, -1005,  -986,  -965,  -945,  -923,
> +	  -902,  -879,  -856,  -833,  -809,  -784,  -760,  -734,
> +	  -708,  -682,  -655,  -628,  -601,  -573,  -545,  -517,
> +	  -488,  -459,  -430,  -400,  -370,  -340,  -310,  -279,
> +	  -249,  -218,  -187,  -156,  -125,   -94,   -63,   -31,
> +	     0
> +	};
> +
> +/*
> + * Calculation of sine is implemented using a look up table for range of
> + * phase values from 0 to 2*pi. Look table contains finite entries, say N.
> + *
> + * Since sinusoid are periodic with period 2*pi, look table stores entries
> + * for phase values from 0 to 2*pi.
> + *
> + * The interval [0,2*pi] is divided into N equal intervals, each representing
> + * a phase increment of (2*pi/N)
> + *
> + * The index 'n' in look up table stores sine value for phase (2*pi*n/N) as -
> + * sin_tab[N] = {sin(0), sin(1*(2*pi/N)),sin(2*(2*pi/N)), ..., sin(N*(2*pi/N))}
> + *
> +   |---------|---------|---------|---- . . . .|---------| . . . . |---------|
> +   0    (2*pi/N)  (2*2*pi/N) (3*2*pi/N)   (n*2*pi/N)                       2*pi
> + *
> + * Generation of sine waveform with different frequencies -
> + *
> + * Consider a sine tone with frequency 'f', so,
> + *      num_cycles_per_sec = f
> + * Let sampling frequency be 'Fs' so,
> + *     num_samples_per_sec = Fs
> + * So, num_samples_per_cycle = Fs/f
> + * Let Nc = Fs/f
> + *
> + * That is, number of samples during one interval of 0 tp 2*pi = Nc, each
> + * sample represents a phase increment of (2*pi/Nc = 2*pi*f/Fs).
> + *
> + * So, for "k" th sample the phase, phi =  k*(2*pi/Nc)
> + *
> + * The sine value at phase increments of (2*pi / Nc) is calculated by
> + * interpolating two adjecent samples from sine look table
> + *
> + * As an example, sine value for phase (phi) is calculated below
> + *
> + * 1. Find the interval [(n*2*pi/N), ((n+1)*2*pi/N)] to which phi belongs
> +
> + *  0    (2*pi/N)      (n*2*pi/N) ((n+1)*2*pi/N)                     2*pi
> + *  |---------| . . . . . |---------|---------|  . . . . .  |---------|
> + *			        ^
> + *			 d0-- > | < --d1
> + *			        |
> + *			       phi = k*(2*pi/Nc)
> + *
> + * For phase (phi) the index n in sine table is given by (phi)/(2*pi/N)
> + *        n = integer part of ( phi / (2*pi / N) )
> + *
> + * 2. Find distance d0 and d1 using fractional arithmatic.
> + *        d0 =  phi - n*(2*pi / N),
> + *        d1 =  2*pi / N - d0
> + *
> + * 3. The calculations of fractions are done using fixed point implementation
> + *
> + * 4. To improve the precision of fixed point implementations, divisions
> + *    in different calculations are delayed till last operations. Say,
> + *    d0 = phi - n*(2*pi / N)
> + *    d0 = phi - n * (2 * (22 / 7) / N) , substitute pi = 22 / 7
> + *    d0 = phi - (n * 44 * N) / 7
> + */
> +s32 calc_sin(u32 phase)
> +{
> +	u32 index;
> +	u32 d0;
> +	u32 d1;
> +	s32 result;
> +	u64 temp0;
> +	u64 temp1;
> +
> +	temp0 = phase * SIN_TAB_SIZE;
> +	index = (temp0 * 7) / (44 << FIX_PT_PREC);
> +
> +	temp0 = (temp0 * 7) / 44;
> +	temp1 = index << FIX_PT_PREC;
> +
> +	d1 =  temp0 - temp1;
> +	d0 = (1 << FIX_PT_PREC) - d1;
> +
> +	result = (d0 * sin[index % 256] + d1 * sin[(index+1)%256]);
> +
> +	return result >> FIX_PT_PREC;
> +}
> +
> +s32 calc_cos(u32 phase)
> +{
> +	u32 index;
> +	u32 d0;
> +	u32 d1;
> +	s32 result;
> +	u64 temp0;
> +	u64 temp1;
> +
> +	temp0 = phase * SIN_TAB_SIZE;
> +	index = (temp0 * 7) / (44 << FIX_PT_PREC);
> +
> +	temp0 = (temp0 * 7) / 44;
> +	temp1 = index << FIX_PT_PREC;
> +
> +	d1 =  temp0 - temp1;
> +	d0 = (1 << FIX_PT_PREC) - d1;
> +
> +	index += 64;
> +	result = (d0 * sin[index % 256] + d1 * sin[(index+1)%256]);
> +
> +	return result >> FIX_PT_PREC;
> +}
> +
> +u32 phase_per_sample(u32 signal_freq, u32 sampling_freq)
> +{
> +	/* phase increment or decrement with each sample is given by
> +	 *  (2 x Pi x signal frequency)/sampling frequency
> +	 * To get a better accuracy with fixed point implementation we use
> +	 *  Pi = 22/7
> +	 * */
> +	u64 temp = 44 * ((u64)signal_freq << FIX_PT_PREC);
> +
> +	return temp / (7*sampling_freq);
> +}
> diff --git a/drivers/media/platform/vivid/vivid-sin.h b/drivers/media/platform/vivid/vivid-sin.h
> new file mode 100644
> index 0000000..6c8bab2
> --- /dev/null
> +++ b/drivers/media/platform/vivid/vivid-sin.h
> @@ -0,0 +1,31 @@
> +/*
> + * vivid-sin.h - support functions used to generate (co)sine waveforms for
> + *               software defined radio.
> + *
> + * Copyright 2014 Cisco Systems, Inc. and/or its affiliates. All rights
> + * reserved.
> + *
> + * This program is free software; you may redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef _VIVID_SIN_H_
> +#define _VIVID_SIN_H_
> +
> +#define FIX_PT_PREC 16
> +
> +s32 calc_sin(u32 phase);
> +s32 calc_cos(u32 phase);
> +u32 phase_per_sample(u32 signal_freq, u32 sampling_freq);
> +
> +#endif

Re: [PATCH 1/6] Use LUT based implementation for (co)sine functions

[Antti Palosaari]

On 12/15/2014 03:13 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 15 Dec 2014 14:49:17 +0530
> Prashant Laddha <prladdha@cisco.com> escreveu:
>
>> Replaced Taylor series calculation for (co)sine with a
>> look up table (LUT) for sine values.
>
> Kernel has already a LUT for sin/cos at:
> 	include/linux/fixp-arith.h
>
> The best would be to either use it or improve its precision, if the one there
> is not good enough.

I looked that one when made generator. It has poor precision and it uses 
degrees not radians. But surely it is correct practice improve existing 
than introduce new.

regards
Antti


-- 
http://palosaari.fi/

Re: [PATCH 1/6] Use LUT based implementation for (co)sine functions

[Prashant Laddha (prladdha)]

Antti, Mauro,

Thanks for your comments.

On 15/12/14 7:00 pm, "Antti Palosaari" <crope@iki.fi> wrote:


>On 12/15/2014 03:13 PM, Mauro Carvalho Chehab wrote:
>> Em Mon, 15 Dec 2014 14:49:17 +0530
>> Prashant Laddha <prladdha@cisco.com> escreveu:
>>
>>> Replaced Taylor series calculation for (co)sine with a
>>> look up table (LUT) for sine values.
>>
>> Kernel has already a LUT for sin/cos at:
>> 	include/linux/fixp-arith.h
>>
>> The best would be to either use it or improve its precision, if the one
>>there
>> is not good enough.

Thanks. I had not looked at this file earlier. But now when I looked at
this file I agree with Antti¹s comments below.


>
>I looked that one when made generator. It has poor precision and it uses
>degrees not radians.

> 
Also, it does not support calculation for phase values falling in middle
of two entries of LUT.


>But surely it is correct practice improve existing
>than introduce new.

I agree. Probably we can start looking into how to improve existing. I
looked at dependancies. As of now functions in fixp-arith is used by two
other files. Replacing current implementation in fixp-arith.h with high
precision will not work as it is, because caller functions are using
lesser precision. We probably need to discuss more on how to improve
existing implementation.

Some thoughts -
1. Going by the name fixp-arith.h, I feel, it should have larger scope
than just (co)sine implementation. It can include divide as well. One
could also consider option to keep all trignometric functions in another
file
2. One could support APIs to provide output with different precisions, say
16, 32, 64 bits etc. Not sure how final implementation would be but one
option would be to do internal computation with highest
precision possible and then truncate the result to have desired precision
based on the API called.


Regards,
Prashant

Re: [PATCH 1/6] Use LUT based implementation for (co)sine functions

[Mauro Carvalho Chehab]

Em Tue, 16 Dec 2014 06:41:18 +0000
"Prashant Laddha (prladdha)" <prladdha@cisco.com> escreveu:

> Antti, Mauro,
> 
> Thanks for your comments.
> 
> On 15/12/14 7:00 pm, "Antti Palosaari" <crope@iki.fi> wrote:
> 
> 
> >On 12/15/2014 03:13 PM, Mauro Carvalho Chehab wrote:
> >> Em Mon, 15 Dec 2014 14:49:17 +0530
> >> Prashant Laddha <prladdha@cisco.com> escreveu:
> >>
> >>> Replaced Taylor series calculation for (co)sine with a
> >>> look up table (LUT) for sine values.
> >>
> >> Kernel has already a LUT for sin/cos at:
> >> 	include/linux/fixp-arith.h
> >>
> >> The best would be to either use it or improve its precision, if the one
> >>there
> >> is not good enough.
> 
> Thanks. I had not looked at this file earlier. But now when I looked at
> this file I agree with Antti¹s comments below.
> 
> 
> >
> >I looked that one when made generator. It has poor precision and it uses
> >degrees not radians.
> 
> > 
> Also, it does not support calculation for phase values falling in middle
> of two entries of LUT.
> 
> 
> >But surely it is correct practice improve existing
> >than introduce new.
> 
> I agree. Probably we can start looking into how to improve existing. I
> looked at dependancies. As of now functions in fixp-arith is used by two
> other files. Replacing current implementation in fixp-arith.h with high
> precision will not work as it is, because caller functions are using
> lesser precision. We probably need to discuss more on how to improve
> existing implementation.

if you do a:
	$ git grep -e 'sin(' --or -e 'cos('

You'll notice that there are other drivers that have also their own
sin()/cos() needs, with their own implementation.

I'm not saying we should touch them, but it is worth to check what are
their needs, in order to be sure that whatever we do would latter fit
on their usages.

> Some thoughts -
> 1. Going by the name fixp-arith.h, I feel, it should have larger scope
> than just (co)sine implementation. It can include divide as well. One
> could also consider option to keep all trignometric functions in another
> file

We could do that, but let's start with sin/cos functions. Btw, there are
log implementation functions already at dvb-core at
	drivers/media/dvb-core/dvb_math.[ch]

Those are also candidates to be moved to a more generic place.

> 2. One could support APIs to provide output with different precisions, say
> 16, 32, 64 bits etc. Not sure how final implementation would be but one
> option would be to do internal computation with highest
> precision possible and then truncate the result to have desired precision
> based on the API called.

Internally, I would stick with a 32 bits implementation, where the 1.0 would
mean S32_MAX. 64 bits is likely an overkill, and would be slower on 32 bits
machines.

Using degrees also seem to be a good thing, as the table will be symmetric,
and the 90th value will be equal to 1.0 (for a sin table). 

That means that we need an array with just 90 elements instead of 256.
To be generic enough, the logic should use modulus math, to be sure that
the given value will be converted into an angle between 0 and 360, and
then down-converted to 0-90, in order to use a table with just a quarter
of the values, e. g. something like:

static inline const s32 sin_table[] = {
	0x00000000, 0x023be165, 0x04779632, 0x06b2f1d2, 0x08edc7b6, 0x0b27eb5c, 
	0x0d61304d, 0x0f996a26, 0x11d06c96, 0x14060b67, 0x163a1a7d, 0x186c6ddd, 
	0x1a9cd9ac, 0x1ccb3236, 0x1ef74bf2, 0x2120fb82, 0x234815ba, 0x256c6f9e, 
	0x278dde6e, 0x29ac379f, 0x2bc750e8, 0x2ddf003f, 0x2ff31bdd, 0x32037a44, 
	0x340ff241, 0x36185aee, 0x381c8bb5, 0x3a1c5c56, 0x3c17a4e7, 0x3e0e3ddb, 
	0x3fffffff, 0x41ecc483, 0x43d464fa, 0x45b6bb5d, 0x4793a20f, 0x496af3e1, 
	0x4b3c8c11, 0x4d084650, 0x4ecdfec6, 0x508d9210, 0x5246dd48, 0x53f9be04, 
	0x55a6125a, 0x574bb8e5, 0x58ea90c2, 0x5a827999, 0x5c135399, 0x5d9cff82, 
	0x5f1f5ea0, 0x609a52d1, 0x620dbe8a, 0x637984d3, 0x64dd894f, 0x6639b039, 
	0x678dde6d, 0x68d9f963, 0x6a1de735, 0x6b598ea1, 0x6c8cd70a, 0x6db7a879, 
	0x6ed9eba0, 0x6ff389de, 0x71046d3c, 0x720c8074, 0x730baeec, 0x7401e4bf, 
	0x74ef0ebb, 0x75d31a5f, 0x76adf5e5, 0x777f903b, 0x7847d908, 0x7906c0af, 
	0x79bc384c, 0x7a6831b8, 0x7b0a9f8c, 0x7ba3751c, 0x7c32a67c, 0x7cb82884, 
	0x7d33f0c8, 0x7da5f5a3, 0x7e0e2e31, 0x7e6c924f, 0x7ec11aa3, 0x7f0bc095, 
	0x7f4c7e52, 0x7f834ecf, 0x7fb02dc4, 0x7fd317b3, 0x7fec09e1, 0x7ffb025e, 
	0x7fffffff
};

s32 fixp_sin32(int value)
{
	s32 ret;
	bool negative = false;

	value = (value % 360 + 360) % 360;
	if (value > 180) {
		negative = true;
		value -= 180;
	}
	if (value > 90)
		value = 180 - value;

	ret = sin_table[value];

	return negative ? -ret : ret;
}

And then define the 16 bits and cos() variants for it with:

#define fixp_cos32(v) fixp_sin32((v) + 90)
#define fixp_sin16(v) (fixp_sin32(v) >> 16)
#define fixp_cos16(v) (fixp_cos32(v) >> 16)

If we end latter to need 64 bits, it shouldn't be hard to change
the logic there to add a different table.

Regards,
Mauro

Re: [PATCH 1/6] Use LUT based implementation for (co)sine functions

[Mauro Carvalho Chehab]

Em Tue, 16 Dec 2014 08:45:53 -0200
Mauro Carvalho Chehab <mchehab@osg.samsung.com> escreveu:

> Em Tue, 16 Dec 2014 06:41:18 +0000
> "Prashant Laddha (prladdha)" <prladdha@cisco.com> escreveu:
> 
> > Antti, Mauro,
> > 
> > Thanks for your comments.
> > 
> > On 15/12/14 7:00 pm, "Antti Palosaari" <crope@iki.fi> wrote:
> > 
> > 
> > >On 12/15/2014 03:13 PM, Mauro Carvalho Chehab wrote:
> > >> Em Mon, 15 Dec 2014 14:49:17 +0530
> > >> Prashant Laddha <prladdha@cisco.com> escreveu:
> > >>
> > >>> Replaced Taylor series calculation for (co)sine with a
> > >>> look up table (LUT) for sine values.
> > >>
> > >> Kernel has already a LUT for sin/cos at:
> > >> 	include/linux/fixp-arith.h
> > >>
> > >> The best would be to either use it or improve its precision, if the one
> > >>there
> > >> is not good enough.
> > 
> > Thanks. I had not looked at this file earlier. But now when I looked at
> > this file I agree with Antti¹s comments below.
> > 
> > 
> > >
> > >I looked that one when made generator. It has poor precision and it uses
> > >degrees not radians.
> > 
> > > 
> > Also, it does not support calculation for phase values falling in middle
> > of two entries of LUT.
> > 
> > 
> > >But surely it is correct practice improve existing
> > >than introduce new.
> > 
> > I agree. Probably we can start looking into how to improve existing. I
> > looked at dependancies. As of now functions in fixp-arith is used by two
> > other files. Replacing current implementation in fixp-arith.h with high
> > precision will not work as it is, because caller functions are using
> > lesser precision. We probably need to discuss more on how to improve
> > existing implementation.
> 
> if you do a:
> 	$ git grep -e 'sin(' --or -e 'cos('
> 
> You'll notice that there are other drivers that have also their own
> sin()/cos() needs, with their own implementation.
> 
> I'm not saying we should touch them, but it is worth to check what are
> their needs, in order to be sure that whatever we do would latter fit
> on their usages.
> 
> > Some thoughts -
> > 1. Going by the name fixp-arith.h, I feel, it should have larger scope
> > than just (co)sine implementation. It can include divide as well. One
> > could also consider option to keep all trignometric functions in another
> > file
> 
> We could do that, but let's start with sin/cos functions. Btw, there are
> log implementation functions already at dvb-core at
> 	drivers/media/dvb-core/dvb_math.[ch]
> 
> Those are also candidates to be moved to a more generic place.
> 
> > 2. One could support APIs to provide output with different precisions, say
> > 16, 32, 64 bits etc. Not sure how final implementation would be but one
> > option would be to do internal computation with highest
> > precision possible and then truncate the result to have desired precision
> > based on the API called.
> 
> Internally, I would stick with a 32 bits implementation, where the 1.0 would
> mean S32_MAX. 64 bits is likely an overkill, and would be slower on 32 bits
> machines.
> 
> Using degrees also seem to be a good thing, as the table will be symmetric,
> and the 90th value will be equal to 1.0 (for a sin table). 
> 
> That means that we need an array with just 90 elements instead of 256.
> To be generic enough, the logic should use modulus math, to be sure that
> the given value will be converted into an angle between 0 and 360, and
> then down-converted to 0-90, in order to use a table with just a quarter
> of the values, e. g. something like:
> 
> static inline const s32 sin_table[] = {
> 	0x00000000, 0x023be165, 0x04779632, 0x06b2f1d2, 0x08edc7b6, 0x0b27eb5c, 
> 	0x0d61304d, 0x0f996a26, 0x11d06c96, 0x14060b67, 0x163a1a7d, 0x186c6ddd, 
> 	0x1a9cd9ac, 0x1ccb3236, 0x1ef74bf2, 0x2120fb82, 0x234815ba, 0x256c6f9e, 
> 	0x278dde6e, 0x29ac379f, 0x2bc750e8, 0x2ddf003f, 0x2ff31bdd, 0x32037a44, 
> 	0x340ff241, 0x36185aee, 0x381c8bb5, 0x3a1c5c56, 0x3c17a4e7, 0x3e0e3ddb, 
> 	0x3fffffff, 0x41ecc483, 0x43d464fa, 0x45b6bb5d, 0x4793a20f, 0x496af3e1, 
> 	0x4b3c8c11, 0x4d084650, 0x4ecdfec6, 0x508d9210, 0x5246dd48, 0x53f9be04, 
> 	0x55a6125a, 0x574bb8e5, 0x58ea90c2, 0x5a827999, 0x5c135399, 0x5d9cff82, 
> 	0x5f1f5ea0, 0x609a52d1, 0x620dbe8a, 0x637984d3, 0x64dd894f, 0x6639b039, 
> 	0x678dde6d, 0x68d9f963, 0x6a1de735, 0x6b598ea1, 0x6c8cd70a, 0x6db7a879, 
> 	0x6ed9eba0, 0x6ff389de, 0x71046d3c, 0x720c8074, 0x730baeec, 0x7401e4bf, 
> 	0x74ef0ebb, 0x75d31a5f, 0x76adf5e5, 0x777f903b, 0x7847d908, 0x7906c0af, 
> 	0x79bc384c, 0x7a6831b8, 0x7b0a9f8c, 0x7ba3751c, 0x7c32a67c, 0x7cb82884, 
> 	0x7d33f0c8, 0x7da5f5a3, 0x7e0e2e31, 0x7e6c924f, 0x7ec11aa3, 0x7f0bc095, 
> 	0x7f4c7e52, 0x7f834ecf, 0x7fb02dc4, 0x7fd317b3, 0x7fec09e1, 0x7ffb025e, 
> 	0x7fffffff
> };
> 
> s32 fixp_sin32(int value)
> {
> 	s32 ret;
> 	bool negative = false;
> 
> 	value = (value % 360 + 360) % 360;
> 	if (value > 180) {
> 		negative = true;
> 		value -= 180;
> 	}
> 	if (value > 90)
> 		value = 180 - value;
> 
> 	ret = sin_table[value];
> 
> 	return negative ? -ret : ret;
> }
> 
> And then define the 16 bits and cos() variants for it with:
> 
> #define fixp_cos32(v) fixp_sin32((v) + 90)
> #define fixp_sin16(v) (fixp_sin32(v) >> 16)
> #define fixp_cos16(v) (fixp_cos32(v) >> 16)
> 
> If we end latter to need 64 bits, it shouldn't be hard to change
> the logic there to add a different table.

Forgot to mention, but it could make sense to have a version
that would allow more than 360 values for the angle.
We can do that via linear interpolation. 

Something like (untested):

#define INT_2PI			((s32)(2 * 3.141592653589 * 32768.0))

static inline s32 fixp_sin32_rad(u32 radians)
{
	int degree;
	s32 v1, v2, dx, dy;
	u64 tmp;

	degree = (radians * 360) / INT_2PI;

	v1 = fixp_sin32(degree);
	v2 = fixp_sin32(degree + 1);

	dx = INT_2PI / 360;
	dy = v2 - v1;

	tmp = radians - (degree * INT_2PI) / 360;
	tmp *= dy;
	do_div(tmp, dx);

	ret (s32)(v1 + tmp64);
}


Regards,
Mauro

[RFC PATCH] fixp-arith: replace sin/cos table by a better precision one

[Mauro Carvalho Chehab]

The cos table used at fixp-arith.h has only 8 bits of precision.
That causes problems if it is reused on other drivers.

As some media drivers require a higher precision sin/cos
implementation, replace the current implementation by one that
will provide 32 bits precision.

The values generated by the new implementation matches the
32 bit precision of glibc's sin for an angle measured in
integer degrees.

It also provides support for fractional angles via linear
interpolation. On experimental calculus, when used a table
with a 0.001 degree angle, the maximum error for sin is
0.000038, with is likely good enough for practical purposes.

There are some logic there that seems to be specific to the
usage inside ff-memless.c. Move those logic to there, as they're
not needed elsewhere.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

---

Instead of adding yet-another implementation of integer sin()/cos(),
let's fix the one that already exists on a worldwide header for one
that would provide the needed precision.

While I tested the implementation on userspace, I didn't actually
check if ff-memless and ov534 drivers are working properly with
this change. Please test.

diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
index 74c0d8c6002a..fcc6c3368182 100644
--- a/drivers/input/ff-memless.c
+++ b/drivers/input/ff-memless.c
@@ -237,6 +237,18 @@ static u16 ml_calculate_direction(u16 direction, u16 force,
 		(force + new_force)) << 1;
 }
 
+#define FRAC_N 8
+static inline s16 fixp_new16(s16 a)
+{
+	return ((s32)a) >> (16 - FRAC_N);
+}
+
+static inline s16 fixp_mult(s16 a, s16 b)
+{
+	a = ((s32)a * 0x100) / 0x7fff;
+	return ((s32)(a * b)) >> FRAC_N;
+}
+
 /*
  * Combine two effects and apply gain.
  */
@@ -247,7 +259,7 @@ static void ml_combine_effects(struct ff_effect *effect,
 	struct ff_effect *new = state->effect;
 	unsigned int strong, weak, i;
 	int x, y;
-	fixp_t level;
+	s16 level;
 
 	switch (new->type) {
 	case FF_CONSTANT:
@@ -255,8 +267,8 @@ static void ml_combine_effects(struct ff_effect *effect,
 		level = fixp_new16(apply_envelope(state,
 					new->u.constant.level,
 					&new->u.constant.envelope));
-		x = fixp_mult(fixp_sin(i), level) * gain / 0xffff;
-		y = fixp_mult(-fixp_cos(i), level) * gain / 0xffff;
+		x = fixp_mult(fixp_sin16(i), level) * gain / 0xffff;
+		y = fixp_mult(-fixp_cos16(i), level) * gain / 0xffff;
 		/*
 		 * here we abuse ff_ramp to hold x and y of constant force
 		 * If in future any driver wants something else than x and y
diff --git a/drivers/media/usb/gspca/ov534.c b/drivers/media/usb/gspca/ov534.c
index 90f0d637cd9d..50b72f6dfdc6 100644
--- a/drivers/media/usb/gspca/ov534.c
+++ b/drivers/media/usb/gspca/ov534.c
@@ -810,21 +810,16 @@ static void sethue(struct gspca_dev *gspca_dev, s32 val)
 		s16 huesin;
 		s16 huecos;
 
-		/* fixp_sin and fixp_cos accept only positive values, while
-		 * our val is between -90 and 90
-		 */
-		val += 360;
-
 		/* According to the datasheet the registers expect HUESIN and
 		 * HUECOS to be the result of the trigonometric functions,
 		 * scaled by 0x80.
 		 *
-		 * The 0x100 here represents the maximun absolute value
+		 * The 0x7fff here represents the maximum absolute value
 		 * returned byt fixp_sin and fixp_cos, so the scaling will
 		 * consider the result like in the interval [-1.0, 1.0].
 		 */
-		huesin = fixp_sin(val) * 0x80 / 0x100;
-		huecos = fixp_cos(val) * 0x80 / 0x100;
+		huesin = fixp_sin16(val) * 0x80 / 0x7fff;
+		huecos = fixp_cos16(val) * 0x80 / 0x7fff;
 
 		if (huesin < 0) {
 			sccb_reg_write(gspca_dev, 0xab,
diff --git a/include/linux/fixp-arith.h b/include/linux/fixp-arith.h
index 3089d7382325..857e7f782140 100644
--- a/include/linux/fixp-arith.h
+++ b/include/linux/fixp-arith.h
@@ -29,59 +29,104 @@
 
 #include <linux/types.h>
 
-/* The type representing fixed-point values */
-typedef s16 fixp_t;
-
-#define FRAC_N 8
-#define FRAC_MASK ((1<<FRAC_N)-1)
-
-/* Not to be used directly. Use fixp_{cos,sin} */
-static const fixp_t cos_table[46] = {
-	0x0100,	0x00FF,	0x00FF,	0x00FE,	0x00FD,	0x00FC,	0x00FA,	0x00F8,
-	0x00F6,	0x00F3,	0x00F0,	0x00ED,	0x00E9,	0x00E6,	0x00E2,	0x00DD,
-	0x00D9,	0x00D4,	0x00CF,	0x00C9,	0x00C4,	0x00BE,	0x00B8,	0x00B1,
-	0x00AB,	0x00A4,	0x009D,	0x0096,	0x008F,	0x0087,	0x0080,	0x0078,
-	0x0070,	0x0068,	0x005F,	0x0057,	0x004F,	0x0046,	0x003D,	0x0035,
-	0x002C,	0x0023,	0x001A,	0x0011,	0x0008, 0x0000
+static const s32 sin_table[] = {
+	0x00000000, 0x023be165, 0x04779632, 0x06b2f1d2, 0x08edc7b6, 0x0b27eb5c,
+	0x0d61304d, 0x0f996a26, 0x11d06c96, 0x14060b67, 0x163a1a7d, 0x186c6ddd,
+	0x1a9cd9ac, 0x1ccb3236, 0x1ef74bf2, 0x2120fb82, 0x234815ba, 0x256c6f9e,
+	0x278dde6e, 0x29ac379f, 0x2bc750e8, 0x2ddf003f, 0x2ff31bdd, 0x32037a44,
+	0x340ff241, 0x36185aee, 0x381c8bb5, 0x3a1c5c56, 0x3c17a4e7, 0x3e0e3ddb,
+	0x3fffffff, 0x41ecc483, 0x43d464fa, 0x45b6bb5d, 0x4793a20f, 0x496af3e1,
+	0x4b3c8c11, 0x4d084650, 0x4ecdfec6, 0x508d9210, 0x5246dd48, 0x53f9be04,
+	0x55a6125a, 0x574bb8e5, 0x58ea90c2, 0x5a827999, 0x5c135399, 0x5d9cff82,
+	0x5f1f5ea0, 0x609a52d1, 0x620dbe8a, 0x637984d3, 0x64dd894f, 0x6639b039,
+	0x678dde6d, 0x68d9f963, 0x6a1de735, 0x6b598ea1, 0x6c8cd70a, 0x6db7a879,
+	0x6ed9eba0, 0x6ff389de, 0x71046d3c, 0x720c8074, 0x730baeec, 0x7401e4bf,
+	0x74ef0ebb, 0x75d31a5f, 0x76adf5e5, 0x777f903b, 0x7847d908, 0x7906c0af,
+	0x79bc384c, 0x7a6831b8, 0x7b0a9f8c, 0x7ba3751c, 0x7c32a67c, 0x7cb82884,
+	0x7d33f0c8, 0x7da5f5a3, 0x7e0e2e31, 0x7e6c924f, 0x7ec11aa3, 0x7f0bc095,
+	0x7f4c7e52, 0x7f834ecf, 0x7fb02dc4, 0x7fd317b3, 0x7fec09e1, 0x7ffb025e,
+	0x7fffffff
 };
 
+/**
+ * fixp_sin32() returns the sin of an angle in degrees
+ *
+ * @degrees: angle, in degrees. It can be positive or negative
+ *
+ * The returned value ranges from -0x7fffffff to +0x7fffffff.
+ */
 
-/* a: 123 -> 123.0 */
-static inline fixp_t fixp_new(s16 a)
+static inline s32 fixp_sin32(int degrees)
 {
-	return a<<FRAC_N;
-}
+	s32 ret;
+	bool negative = false;
 
-/* a: 0xFFFF -> -1.0
-      0x8000 -> 1.0
-      0x0000 -> 0.0
-*/
-static inline fixp_t fixp_new16(s16 a)
-{
-	return ((s32)a)>>(16-FRAC_N);
+	degrees = (degrees % 360 + 360) % 360;
+	if (degrees > 180) {
+		negative = true;
+		degrees -= 180;
+	}
+	if (degrees > 90)
+		degrees = 180 - degrees;
+
+	ret = sin_table[degrees];
+
+	return negative ? -ret : ret;
 }
 
-static inline fixp_t fixp_cos(unsigned int degrees)
+/* cos(x) = sin(x + 90 degrees) */
+#define fixp_cos32(v) fixp_sin32((v) + 90)
+
+/*
+ * 16 bits variants
+ *
+ * The returned value ranges from -0x7fff to 0x7fff
+ */
+
+#define fixp_sin16(v) (fixp_sin32(v) >> 16)
+#define fixp_cos16(v) (fixp_cos32(v) >> 16)
+
+/**
+ * fixp_sin32_rad() - calculates the sin of an angle in radians
+ *
+ * @radians: angle, in radians
+ * @twopi: value to be used for 2*pi
+ *
+ * Provides a variant for the cases where just 360
+ * values is not enough. This function uses linear
+ * interpolation to a wider range of values given by
+ * twopi var.
+ *
+ * Experimental tests gave a maximum difference of
+ * 0.000038 between the value calculated by sin() and
+ * the one produced by this function, when twopi is
+ * equal to 360000. That seems to be enough precision
+ * for practical purposes.
+ */
+static inline s32 fixp_sin32_rad(u32 radians, u32 twopi)
 {
-	int quadrant = (degrees / 90) & 3;
-	unsigned int i = degrees % 90;
+	int degrees;
+	s32 v1, v2, dx, dy;
+	s64 tmp;
 
-	if (quadrant == 1 || quadrant == 3)
-		i = 90 - i;
+	degrees = (radians * 360) / twopi;
 
-	i >>= 1;
+	v1 = fixp_sin32(degrees);
+	v2 = fixp_sin32(degrees + 1);
 
-	return (quadrant == 1 || quadrant == 2)? -cos_table[i] : cos_table[i];
-}
+	dx = twopi / 360;
+	dy = v2 - v1;
 
-static inline fixp_t fixp_sin(unsigned int degrees)
-{
-	return -fixp_cos(degrees + 90);
-}
+	tmp = radians - (degrees * twopi) / 360;
+	tmp *= dy;
+	do_div(tmp, dx);
 
-static inline fixp_t fixp_mult(fixp_t a, fixp_t b)
-{
-	return ((s32)(a*b))>>FRAC_N;
+	return v1 + tmp;
 }
 
+/* cos(x) = sin(x + pi radians) */
+
+#define fixp_cos32_rad(rad, twopi)	\
+	fixp_sin32_rad(rad + twopi/2, twopi)
+
 #endif
-- 
2.1.0

Re: [RFC PATCH] fixp-arith: replace sin/cos table by a better precision one

[Prashant Laddha (prladdha)]

Thanks for the patch, Mauro.  Just a correction below.

> 
>+/* cos(x) = sin(x + pi radians) */
>+
This should pi / 2. Correcting for the same below.
>+#define fixp_cos32_rad(rad, twopi)	\
>+	fixp_sin32_rad(rad + twopi/2, twopi)
          fixp_sin32_rad(rad + twopi/4, twopi)


>+

I think this patch will serve the need. I will test it for vivid sir tone
generation. I will rework my patches to use sin/cos functions from
fixp-arith.h.
>

[RFC PATCHv2] fixp-arith: replace sin/cos table by a better precision one

[Mauro Carvalho Chehab]

The cos table used at fixp-arith.h has only 8 bits of precision.
That causes problems if it is reused on other drivers.

As some media drivers require a higher precision sin/cos
implementation, replace the current implementation by one that
will provide 32 bits precision.

The values generated by the new implementation matches the
32 bit precision of glibc's sin for an angle measured in
integer degrees.

It also provides support for fractional angles via linear
interpolation. On experimental calculus, when used a table
with a 0.001 degree angle, the maximum error for sin is
0.000038, with is likely good enough for practical purposes.

There are some logic there that seems to be specific to the
usage inside ff-memless.c. Move those logic to there, as they're
not needed elsewhere.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

---

Instead of adding yet-another implementation of integer sin()/cos(),
let's fix the one that already exists on a worldwide header for one
that would provide the needed precision.

While I tested the implementation on userspace, I didn't actually
check if ff-memless and ov534 drivers are working properly with
this change. Please test.

v2: Fixed fixp_cos32_rad() as per Prashant Laddha review

diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
index 74c0d8c6002a..fcc6c3368182 100644
--- a/drivers/input/ff-memless.c
+++ b/drivers/input/ff-memless.c
@@ -237,6 +237,18 @@ static u16 ml_calculate_direction(u16 direction, u16 force,
 		(force + new_force)) << 1;
 }
 
+#define FRAC_N 8
+static inline s16 fixp_new16(s16 a)
+{
+	return ((s32)a) >> (16 - FRAC_N);
+}
+
+static inline s16 fixp_mult(s16 a, s16 b)
+{
+	a = ((s32)a * 0x100) / 0x7fff;
+	return ((s32)(a * b)) >> FRAC_N;
+}
+
 /*
  * Combine two effects and apply gain.
  */
@@ -247,7 +259,7 @@ static void ml_combine_effects(struct ff_effect *effect,
 	struct ff_effect *new = state->effect;
 	unsigned int strong, weak, i;
 	int x, y;
-	fixp_t level;
+	s16 level;
 
 	switch (new->type) {
 	case FF_CONSTANT:
@@ -255,8 +267,8 @@ static void ml_combine_effects(struct ff_effect *effect,
 		level = fixp_new16(apply_envelope(state,
 					new->u.constant.level,
 					&new->u.constant.envelope));
-		x = fixp_mult(fixp_sin(i), level) * gain / 0xffff;
-		y = fixp_mult(-fixp_cos(i), level) * gain / 0xffff;
+		x = fixp_mult(fixp_sin16(i), level) * gain / 0xffff;
+		y = fixp_mult(-fixp_cos16(i), level) * gain / 0xffff;
 		/*
 		 * here we abuse ff_ramp to hold x and y of constant force
 		 * If in future any driver wants something else than x and y
diff --git a/drivers/media/usb/gspca/ov534.c b/drivers/media/usb/gspca/ov534.c
index 90f0d637cd9d..50b72f6dfdc6 100644
--- a/drivers/media/usb/gspca/ov534.c
+++ b/drivers/media/usb/gspca/ov534.c
@@ -810,21 +810,16 @@ static void sethue(struct gspca_dev *gspca_dev, s32 val)
 		s16 huesin;
 		s16 huecos;
 
-		/* fixp_sin and fixp_cos accept only positive values, while
-		 * our val is between -90 and 90
-		 */
-		val += 360;
-
 		/* According to the datasheet the registers expect HUESIN and
 		 * HUECOS to be the result of the trigonometric functions,
 		 * scaled by 0x80.
 		 *
-		 * The 0x100 here represents the maximun absolute value
+		 * The 0x7fff here represents the maximum absolute value
 		 * returned byt fixp_sin and fixp_cos, so the scaling will
 		 * consider the result like in the interval [-1.0, 1.0].
 		 */
-		huesin = fixp_sin(val) * 0x80 / 0x100;
-		huecos = fixp_cos(val) * 0x80 / 0x100;
+		huesin = fixp_sin16(val) * 0x80 / 0x7fff;
+		huecos = fixp_cos16(val) * 0x80 / 0x7fff;
 
 		if (huesin < 0) {
 			sccb_reg_write(gspca_dev, 0xab,
diff --git a/include/linux/fixp-arith.h b/include/linux/fixp-arith.h
index 3089d7382325..47b3091a8236 100644
--- a/include/linux/fixp-arith.h
+++ b/include/linux/fixp-arith.h
@@ -29,59 +29,104 @@
 
 #include <linux/types.h>
 
-/* The type representing fixed-point values */
-typedef s16 fixp_t;
-
-#define FRAC_N 8
-#define FRAC_MASK ((1<<FRAC_N)-1)
-
-/* Not to be used directly. Use fixp_{cos,sin} */
-static const fixp_t cos_table[46] = {
-	0x0100,	0x00FF,	0x00FF,	0x00FE,	0x00FD,	0x00FC,	0x00FA,	0x00F8,
-	0x00F6,	0x00F3,	0x00F0,	0x00ED,	0x00E9,	0x00E6,	0x00E2,	0x00DD,
-	0x00D9,	0x00D4,	0x00CF,	0x00C9,	0x00C4,	0x00BE,	0x00B8,	0x00B1,
-	0x00AB,	0x00A4,	0x009D,	0x0096,	0x008F,	0x0087,	0x0080,	0x0078,
-	0x0070,	0x0068,	0x005F,	0x0057,	0x004F,	0x0046,	0x003D,	0x0035,
-	0x002C,	0x0023,	0x001A,	0x0011,	0x0008, 0x0000
+static const s32 sin_table[] = {
+	0x00000000, 0x023be165, 0x04779632, 0x06b2f1d2, 0x08edc7b6, 0x0b27eb5c,
+	0x0d61304d, 0x0f996a26, 0x11d06c96, 0x14060b67, 0x163a1a7d, 0x186c6ddd,
+	0x1a9cd9ac, 0x1ccb3236, 0x1ef74bf2, 0x2120fb82, 0x234815ba, 0x256c6f9e,
+	0x278dde6e, 0x29ac379f, 0x2bc750e8, 0x2ddf003f, 0x2ff31bdd, 0x32037a44,
+	0x340ff241, 0x36185aee, 0x381c8bb5, 0x3a1c5c56, 0x3c17a4e7, 0x3e0e3ddb,
+	0x3fffffff, 0x41ecc483, 0x43d464fa, 0x45b6bb5d, 0x4793a20f, 0x496af3e1,
+	0x4b3c8c11, 0x4d084650, 0x4ecdfec6, 0x508d9210, 0x5246dd48, 0x53f9be04,
+	0x55a6125a, 0x574bb8e5, 0x58ea90c2, 0x5a827999, 0x5c135399, 0x5d9cff82,
+	0x5f1f5ea0, 0x609a52d1, 0x620dbe8a, 0x637984d3, 0x64dd894f, 0x6639b039,
+	0x678dde6d, 0x68d9f963, 0x6a1de735, 0x6b598ea1, 0x6c8cd70a, 0x6db7a879,
+	0x6ed9eba0, 0x6ff389de, 0x71046d3c, 0x720c8074, 0x730baeec, 0x7401e4bf,
+	0x74ef0ebb, 0x75d31a5f, 0x76adf5e5, 0x777f903b, 0x7847d908, 0x7906c0af,
+	0x79bc384c, 0x7a6831b8, 0x7b0a9f8c, 0x7ba3751c, 0x7c32a67c, 0x7cb82884,
+	0x7d33f0c8, 0x7da5f5a3, 0x7e0e2e31, 0x7e6c924f, 0x7ec11aa3, 0x7f0bc095,
+	0x7f4c7e52, 0x7f834ecf, 0x7fb02dc4, 0x7fd317b3, 0x7fec09e1, 0x7ffb025e,
+	0x7fffffff
 };
 
+/**
+ * fixp_sin32() returns the sin of an angle in degrees
+ *
+ * @degrees: angle, in degrees. It can be positive or negative
+ *
+ * The returned value ranges from -0x7fffffff to +0x7fffffff.
+ */
 
-/* a: 123 -> 123.0 */
-static inline fixp_t fixp_new(s16 a)
+static inline s32 fixp_sin32(int degrees)
 {
-	return a<<FRAC_N;
-}
+	s32 ret;
+	bool negative = false;
 
-/* a: 0xFFFF -> -1.0
-      0x8000 -> 1.0
-      0x0000 -> 0.0
-*/
-static inline fixp_t fixp_new16(s16 a)
-{
-	return ((s32)a)>>(16-FRAC_N);
+	degrees = (degrees % 360 + 360) % 360;
+	if (degrees > 180) {
+		negative = true;
+		degrees -= 180;
+	}
+	if (degrees > 90)
+		degrees = 180 - degrees;
+
+	ret = sin_table[degrees];
+
+	return negative ? -ret : ret;
 }
 
-static inline fixp_t fixp_cos(unsigned int degrees)
+/* cos(x) = sin(x + 90 degrees) */
+#define fixp_cos32(v) fixp_sin32((v) + 90)
+
+/*
+ * 16 bits variants
+ *
+ * The returned value ranges from -0x7fff to 0x7fff
+ */
+
+#define fixp_sin16(v) (fixp_sin32(v) >> 16)
+#define fixp_cos16(v) (fixp_cos32(v) >> 16)
+
+/**
+ * fixp_sin32_rad() - calculates the sin of an angle in radians
+ *
+ * @radians: angle, in radians
+ * @twopi: value to be used for 2*pi
+ *
+ * Provides a variant for the cases where just 360
+ * values is not enough. This function uses linear
+ * interpolation to a wider range of values given by
+ * twopi var.
+ *
+ * Experimental tests gave a maximum difference of
+ * 0.000038 between the value calculated by sin() and
+ * the one produced by this function, when twopi is
+ * equal to 360000. That seems to be enough precision
+ * for practical purposes.
+ */
+static inline s32 fixp_sin32_rad(u32 radians, u32 twopi)
 {
-	int quadrant = (degrees / 90) & 3;
-	unsigned int i = degrees % 90;
+	int degrees;
+	s32 v1, v2, dx, dy;
+	s64 tmp;
 
-	if (quadrant == 1 || quadrant == 3)
-		i = 90 - i;
+	degrees = (radians * 360) / twopi;
 
-	i >>= 1;
+	v1 = fixp_sin32(degrees);
+	v2 = fixp_sin32(degrees + 1);
 
-	return (quadrant == 1 || quadrant == 2)? -cos_table[i] : cos_table[i];
-}
+	dx = twopi / 360;
+	dy = v2 - v1;
 
-static inline fixp_t fixp_sin(unsigned int degrees)
-{
-	return -fixp_cos(degrees + 90);
-}
+	tmp = radians - (degrees * twopi) / 360;
+	tmp *= dy;
+	do_div(tmp, dx);
 
-static inline fixp_t fixp_mult(fixp_t a, fixp_t b)
-{
-	return ((s32)(a*b))>>FRAC_N;
+	return v1 + tmp;
 }
 
+/* cos(x) = sin(x + pi/2 radians) */
+
+#define fixp_cos32_rad(rad, twopi)	\
+	fixp_sin32_rad(rad + twopi / 4, twopi)
+
 #endif
-- 
2.1.0

Re: [RFC PATCHv2] fixp-arith: replace sin/cos table by a better precision one

[Prashant Laddha (prladdha)]

[-- Attachment #1: Type: text/plain, Size: 1750 bytes --]

Hi Mauro,

I was able to test your patch with vivid sdr tone generation. It calls
sin/cos functions with radians as argument. I find that the sine wave
generated using fixp_sin32_rad() show discontinuities, especially around
90, 180 degrees. After debugging it further, these discontinuities seems
to be originating due to division of negative number. Please find it below

On 17/12/14 7:12 pm, "Mauro Carvalho Chehab" <mchehab@osg.samsung.com>
wrote:

>
>+ */
>+static inline s32 fixp_sin32_rad(u32 radians, u32 twopi)
> {
>+	int degrees;
>+	s32 v1, v2, dx, dy;
>+	s64 tmp;
>+	degrees = (radians * 360) / twopi;

Not sure if we should use higher precision here. But just a question - in
case, caller function uses higher precision for representing radians,
(radians * 360) can probably overflow, right ? So, could we possibly
specify on max precision for representing radian fraction cannot be more
than 18 bits. 

>+	v1 = fixp_sin32(degrees);
>+	v2 = fixp_sin32(degrees + 1);
>+	dx = twopi / 360;
>+	dy = v2 - v1;
>+	tmp = radians - (degrees * twopi) / 360;

Same as above.

>+	tmp *= dy;
>+	do_div(tmp, dx);

tmp can go negative. do_div() cannot handle negative number. We could
probably avoid do_div and do tmp / dx here. If we want to use do_div(), we
could still do it by modifying radian to degree calculation.

tmp goes negative when the slope sine waveform is negative, that is 2nd
and 3rd quadrant. We could avoid it by deciding the quadrant based on
radians and then calling fixp_sin32(). I modified the function on lines of
fixp_sin32() and tested. It works fine. Attaching a diff for fixp-arith.h
for with the this change to avoid negative values of tmp in
fixp_sin32_rad().

>2.1.0
>


[-- Attachment #2: fixp-arith.diff --]
[-- Type: application/octet-stream, Size: 5055 bytes --]

diff --git a/include/linux/fixp-arith.h b/include/linux/fixp-arith.h
index 3089d73..5973429 100644
--- a/include/linux/fixp-arith.h
+++ b/include/linux/fixp-arith.h
@@ -29,59 +29,116 @@
 
 #include <linux/types.h>
 
-/* The type representing fixed-point values */
-typedef s16 fixp_t;
-
-#define FRAC_N 8
-#define FRAC_MASK ((1<<FRAC_N)-1)
-
-/* Not to be used directly. Use fixp_{cos,sin} */
-static const fixp_t cos_table[46] = {
-	0x0100,	0x00FF,	0x00FF,	0x00FE,	0x00FD,	0x00FC,	0x00FA,	0x00F8,
-	0x00F6,	0x00F3,	0x00F0,	0x00ED,	0x00E9,	0x00E6,	0x00E2,	0x00DD,
-	0x00D9,	0x00D4,	0x00CF,	0x00C9,	0x00C4,	0x00BE,	0x00B8,	0x00B1,
-	0x00AB,	0x00A4,	0x009D,	0x0096,	0x008F,	0x0087,	0x0080,	0x0078,
-	0x0070,	0x0068,	0x005F,	0x0057,	0x004F,	0x0046,	0x003D,	0x0035,
-	0x002C,	0x0023,	0x001A,	0x0011,	0x0008, 0x0000
+static const s32 sin_table[] = {
+       0x00000000, 0x023be165, 0x04779632, 0x06b2f1d2, 0x08edc7b6, 0x0b27eb5c,
+       0x0d61304d, 0x0f996a26, 0x11d06c96, 0x14060b67, 0x163a1a7d, 0x186c6ddd,
+       0x1a9cd9ac, 0x1ccb3236, 0x1ef74bf2, 0x2120fb82, 0x234815ba, 0x256c6f9e,
+       0x278dde6e, 0x29ac379f, 0x2bc750e8, 0x2ddf003f, 0x2ff31bdd, 0x32037a44,
+       0x340ff241, 0x36185aee, 0x381c8bb5, 0x3a1c5c56, 0x3c17a4e7, 0x3e0e3ddb,
+       0x3fffffff, 0x41ecc483, 0x43d464fa, 0x45b6bb5d, 0x4793a20f, 0x496af3e1,
+       0x4b3c8c11, 0x4d084650, 0x4ecdfec6, 0x508d9210, 0x5246dd48, 0x53f9be04,
+       0x55a6125a, 0x574bb8e5, 0x58ea90c2, 0x5a827999, 0x5c135399, 0x5d9cff82,
+       0x5f1f5ea0, 0x609a52d1, 0x620dbe8a, 0x637984d3, 0x64dd894f, 0x6639b039,
+       0x678dde6d, 0x68d9f963, 0x6a1de735, 0x6b598ea1, 0x6c8cd70a, 0x6db7a879,
+       0x6ed9eba0, 0x6ff389de, 0x71046d3c, 0x720c8074, 0x730baeec, 0x7401e4bf,
+       0x74ef0ebb, 0x75d31a5f, 0x76adf5e5, 0x777f903b, 0x7847d908, 0x7906c0af,
+       0x79bc384c, 0x7a6831b8, 0x7b0a9f8c, 0x7ba3751c, 0x7c32a67c, 0x7cb82884,
+       0x7d33f0c8, 0x7da5f5a3, 0x7e0e2e31, 0x7e6c924f, 0x7ec11aa3, 0x7f0bc095,
+       0x7f4c7e52, 0x7f834ecf, 0x7fb02dc4, 0x7fd317b3, 0x7fec09e1, 0x7ffb025e,
+       0x7fffffff
 };
 
+/**
+ * fixp_sin32() returns the sin of an angle in degrees
+ *
+ * @degrees: angle, in degrees. It can be positive or negative
+ *
+ * The returned value ranges from -0x7fffffff to +0x7fffffff.
+ */
 
-/* a: 123 -> 123.0 */
-static inline fixp_t fixp_new(s16 a)
+static inline s32 fixp_sin32(int degrees)
 {
-	return a<<FRAC_N;
-}
+       s32 ret;
+       bool negative = false;
 
-/* a: 0xFFFF -> -1.0
-      0x8000 -> 1.0
-      0x0000 -> 0.0
-*/
-static inline fixp_t fixp_new16(s16 a)
-{
-	return ((s32)a)>>(16-FRAC_N);
-}
+       degrees = (degrees % 360 + 360) % 360;
+       if (degrees > 180) {
+               negative = true;
+               degrees -= 180;
+	   }
+       if (degrees > 90)
+               degrees = 180 - degrees;
 
-static inline fixp_t fixp_cos(unsigned int degrees)
-{
-	int quadrant = (degrees / 90) & 3;
-	unsigned int i = degrees % 90;
-
-	if (quadrant == 1 || quadrant == 3)
-		i = 90 - i;
+       ret = sin_table[degrees];
 
-	i >>= 1;
+       return negative ? -ret : ret;
 
-	return (quadrant == 1 || quadrant == 2)? -cos_table[i] : cos_table[i];
 }
 
-static inline fixp_t fixp_sin(unsigned int degrees)
-{
-	return -fixp_cos(degrees + 90);
-}
+/* cos(x) = sin(x + 90 degrees) */
+#define fixp_cos32(v) fixp_sin32((v) + 90)
 
-static inline fixp_t fixp_mult(fixp_t a, fixp_t b)
+/*
+ * 16 bits variants
+ *
+ * The returned value ranges from -0x7fff to 0x7fff
+ */
+
+#define fixp_sin16(v) (fixp_sin32(v) >> 16)
+#define fixp_cos16(v) (fixp_cos32(v) >> 16)
+
+/**
+ * fixp_sin32_rad() - calculates the sin of an angle in radians
+ *
+ * @radians: angle, in radians
+ * @twopi: value to be used for 2*pi
+ *
+ * Provides a variant for the cases where just 360
+ * values is not enough. This function uses linear
+ * interpolation to a wider range of values given by
+ * twopi var.
+ *
+ * Experimental tests gave a maximum difference of
+ * 0.000038 between the value calculated by sin() and
+ * the one produced by this function, when twopi is
+ * equal to 360000. That seems to be enough precision
+ * for practical purposes.
+ */
+static inline s32 fixp_sin32_rad(u32 radians, u32 twopi)
 {
-	return ((s32)(a*b))>>FRAC_N;
+       int degrees;
+       s32 v1, v2, dx, dy;
+       s64 tmp;
+       u32 pi = twopi / 2;
+       s32 ret;
+       bool negative = false;
+
+       radians = (radians % twopi + twopi) % twopi;
+       if (radians > pi) {
+               negative = true;
+               radians -= pi;
+	   }
+       if (radians > pi /2)
+               radians = pi - radians;
+
+       degrees = (radians * 360) / twopi;
+
+       v1 = fixp_sin32(degrees);
+       v2 = fixp_sin32(degrees + 1);
+
+       dx = twopi / 360;
+       dy = v2 - v1;
+
+       tmp = radians - (degrees * twopi) / 360;
+       do_div(tmp, dx);
+       ret = v1 + tmp;
+
+       return negative ? -ret : ret;
 }
 
+/* cos(x) = sin(x + pi / 2 radians) */
+
+#define fixp_cos32_rad(rad, twopi)     \
+       fixp_sin32_rad(rad + twopi / 4, twopi)
+
 #endif

Re: [RFC PATCHv2] fixp-arith: replace sin/cos table by a better precision one

[Mauro Carvalho Chehab]

Em Wed, 17 Dec 2014 19:11:33 +0000
"Prashant Laddha (prladdha)" <prladdha@cisco.com> escreveu:

> Hi Mauro,
> 
> I was able to test your patch with vivid sdr tone generation. It calls
> sin/cos functions with radians as argument. I find that the sine wave
> generated using fixp_sin32_rad() show discontinuities, especially around
> 90, 180 degrees. After debugging it further, these discontinuities seems
> to be originating due to division of negative number. Please find it below
> 
> On 17/12/14 7:12 pm, "Mauro Carvalho Chehab" <mchehab@osg.samsung.com>
> wrote:
> 
> >
> >+ */
> >+static inline s32 fixp_sin32_rad(u32 radians, u32 twopi)
> > {
> >+	int degrees;
> >+	s32 v1, v2, dx, dy;
> >+	s64 tmp;
> >+	degrees = (radians * 360) / twopi;
> 
> Not sure if we should use higher precision here. But just a question - in
> case, caller function uses higher precision for representing radians,
> (radians * 360) can probably overflow, right ? So, could we possibly
> specify on max precision for representing radian fraction cannot be more
> than 18 bits. 

True. We could call BUG_ON() if this routine is called with a
number of bits higher than a given amount.

> 
> >+	v1 = fixp_sin32(degrees);
> >+	v2 = fixp_sin32(degrees + 1);
> >+	dx = twopi / 360;
> >+	dy = v2 - v1;
> >+	tmp = radians - (degrees * twopi) / 360;
> 
> Same as above.
> 
> >+	tmp *= dy;
> >+	do_div(tmp, dx);
> 
> tmp can go negative. do_div() cannot handle negative number. We could
> probably avoid do_div and do tmp / dx here. If we want to use do_div(), we
> could still do it by modifying radian to degree calculation.

Well, we should then use div_s64(), as defined at include/linux/math64.h.
This is likely better than adding some magic.

> tmp goes negative when the slope sine waveform is negative, that is 2nd
> and 3rd quadrant. We could avoid it by deciding the quadrant based on
> radians and then calling fixp_sin32(). I modified the function on lines of
> fixp_sin32() and tested. It works fine. Attaching a diff for fixp-arith.h
> for with the this change to avoid negative values of tmp in
> fixp_sin32_rad().

Please check if the enclosed patch is better.

Thanks!
Mauro

fixp-arith: replace sin/cos table by a better precision one

The cos table used at fixp-arith.h has only 8 bits of precision.
That causes problems if it is reused on other drivers.

As some media drivers require a higher precision sin/cos
implementation, replace the current implementation by one that
will provide 32 bits precision.

The values generated by the new implementation matches the
32 bit precision of glibc's sin for an angle measured in
integer degrees.

It also provides support for fractional angles via linear
interpolation. On experimental calculus, when used a table
with a 0.001 degree angle, the maximum error for sin is
0.000038, with is likely good enough for practical purposes.

There are some logic there that seems to be specific to the
usage inside ff-memless.c. Move those logic to there, as they're
not needed elsewhere.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
index 74c0d8c6002a..fcc6c3368182 100644
--- a/drivers/input/ff-memless.c
+++ b/drivers/input/ff-memless.c
@@ -237,6 +237,18 @@ static u16 ml_calculate_direction(u16 direction, u16 force,
 		(force + new_force)) << 1;
 }
 
+#define FRAC_N 8
+static inline s16 fixp_new16(s16 a)
+{
+	return ((s32)a) >> (16 - FRAC_N);
+}
+
+static inline s16 fixp_mult(s16 a, s16 b)
+{
+	a = ((s32)a * 0x100) / 0x7fff;
+	return ((s32)(a * b)) >> FRAC_N;
+}
+
 /*
  * Combine two effects and apply gain.
  */
@@ -247,7 +259,7 @@ static void ml_combine_effects(struct ff_effect *effect,
 	struct ff_effect *new = state->effect;
 	unsigned int strong, weak, i;
 	int x, y;
-	fixp_t level;
+	s16 level;
 
 	switch (new->type) {
 	case FF_CONSTANT:
@@ -255,8 +267,8 @@ static void ml_combine_effects(struct ff_effect *effect,
 		level = fixp_new16(apply_envelope(state,
 					new->u.constant.level,
 					&new->u.constant.envelope));
-		x = fixp_mult(fixp_sin(i), level) * gain / 0xffff;
-		y = fixp_mult(-fixp_cos(i), level) * gain / 0xffff;
+		x = fixp_mult(fixp_sin16(i), level) * gain / 0xffff;
+		y = fixp_mult(-fixp_cos16(i), level) * gain / 0xffff;
 		/*
 		 * here we abuse ff_ramp to hold x and y of constant force
 		 * If in future any driver wants something else than x and y
diff --git a/drivers/media/usb/gspca/ov534.c b/drivers/media/usb/gspca/ov534.c
index 90f0d637cd9d..50b72f6dfdc6 100644
--- a/drivers/media/usb/gspca/ov534.c
+++ b/drivers/media/usb/gspca/ov534.c
@@ -810,21 +810,16 @@ static void sethue(struct gspca_dev *gspca_dev, s32 val)
 		s16 huesin;
 		s16 huecos;
 
-		/* fixp_sin and fixp_cos accept only positive values, while
-		 * our val is between -90 and 90
-		 */
-		val += 360;
-
 		/* According to the datasheet the registers expect HUESIN and
 		 * HUECOS to be the result of the trigonometric functions,
 		 * scaled by 0x80.
 		 *
-		 * The 0x100 here represents the maximun absolute value
+		 * The 0x7fff here represents the maximum absolute value
 		 * returned byt fixp_sin and fixp_cos, so the scaling will
 		 * consider the result like in the interval [-1.0, 1.0].
 		 */
-		huesin = fixp_sin(val) * 0x80 / 0x100;
-		huecos = fixp_cos(val) * 0x80 / 0x100;
+		huesin = fixp_sin16(val) * 0x80 / 0x7fff;
+		huecos = fixp_cos16(val) * 0x80 / 0x7fff;
 
 		if (huesin < 0) {
 			sccb_reg_write(gspca_dev, 0xab,
diff --git a/include/linux/fixp-arith.h b/include/linux/fixp-arith.h
index 3089d7382325..f3b3d2cbfcbe 100644
--- a/include/linux/fixp-arith.h
+++ b/include/linux/fixp-arith.h
@@ -1,6 +1,8 @@
 #ifndef _FIXP_ARITH_H
 #define _FIXP_ARITH_H
 
+#include <linux/math64.h>
+
 /*
  * Simplistic fixed-point arithmetics.
  * Hmm, I'm probably duplicating some code :(
@@ -29,59 +31,126 @@
 
 #include <linux/types.h>
 
-/* The type representing fixed-point values */
-typedef s16 fixp_t;
+static const s32 sin_table[] = {
+	0x00000000, 0x023be165, 0x04779632, 0x06b2f1d2, 0x08edc7b6, 0x0b27eb5c,
+	0x0d61304d, 0x0f996a26, 0x11d06c96, 0x14060b67, 0x163a1a7d, 0x186c6ddd,
+	0x1a9cd9ac, 0x1ccb3236, 0x1ef74bf2, 0x2120fb82, 0x234815ba, 0x256c6f9e,
+	0x278dde6e, 0x29ac379f, 0x2bc750e8, 0x2ddf003f, 0x2ff31bdd, 0x32037a44,
+	0x340ff241, 0x36185aee, 0x381c8bb5, 0x3a1c5c56, 0x3c17a4e7, 0x3e0e3ddb,
+	0x3fffffff, 0x41ecc483, 0x43d464fa, 0x45b6bb5d, 0x4793a20f, 0x496af3e1,
+	0x4b3c8c11, 0x4d084650, 0x4ecdfec6, 0x508d9210, 0x5246dd48, 0x53f9be04,
+	0x55a6125a, 0x574bb8e5, 0x58ea90c2, 0x5a827999, 0x5c135399, 0x5d9cff82,
+	0x5f1f5ea0, 0x609a52d1, 0x620dbe8a, 0x637984d3, 0x64dd894f, 0x6639b039,
+	0x678dde6d, 0x68d9f963, 0x6a1de735, 0x6b598ea1, 0x6c8cd70a, 0x6db7a879,
+	0x6ed9eba0, 0x6ff389de, 0x71046d3c, 0x720c8074, 0x730baeec, 0x7401e4bf,
+	0x74ef0ebb, 0x75d31a5f, 0x76adf5e5, 0x777f903b, 0x7847d908, 0x7906c0af,
+	0x79bc384c, 0x7a6831b8, 0x7b0a9f8c, 0x7ba3751c, 0x7c32a67c, 0x7cb82884,
+	0x7d33f0c8, 0x7da5f5a3, 0x7e0e2e31, 0x7e6c924f, 0x7ec11aa3, 0x7f0bc095,
+	0x7f4c7e52, 0x7f834ecf, 0x7fb02dc4, 0x7fd317b3, 0x7fec09e1, 0x7ffb025e,
+	0x7fffffff
+};
 
-#define FRAC_N 8
-#define FRAC_MASK ((1<<FRAC_N)-1)
+/**
+ * __fixp_sin32() returns the sin of an angle in degrees
+ *
+ * @degrees: angle, in degrees, from 0 to 360.
+ *
+ * The returned value ranges from -0x7fffffff to +0x7fffffff.
+ */
+static inline s32 __fixp_sin32(int degrees)
+{
+	s32 ret;
+	bool negative = false;
 
-/* Not to be used directly. Use fixp_{cos,sin} */
-static const fixp_t cos_table[46] = {
-	0x0100,	0x00FF,	0x00FF,	0x00FE,	0x00FD,	0x00FC,	0x00FA,	0x00F8,
-	0x00F6,	0x00F3,	0x00F0,	0x00ED,	0x00E9,	0x00E6,	0x00E2,	0x00DD,
-	0x00D9,	0x00D4,	0x00CF,	0x00C9,	0x00C4,	0x00BE,	0x00B8,	0x00B1,
-	0x00AB,	0x00A4,	0x009D,	0x0096,	0x008F,	0x0087,	0x0080,	0x0078,
-	0x0070,	0x0068,	0x005F,	0x0057,	0x004F,	0x0046,	0x003D,	0x0035,
-	0x002C,	0x0023,	0x001A,	0x0011,	0x0008, 0x0000
-};
+	if (degrees > 180) {
+		negative = true;
+		degrees -= 180;
+	}
+	if (degrees > 90)
+		degrees = 180 - degrees;
 
+	ret = sin_table[degrees];
 
-/* a: 123 -> 123.0 */
-static inline fixp_t fixp_new(s16 a)
-{
-	return a<<FRAC_N;
+	return negative ? -ret : ret;
 }
 
-/* a: 0xFFFF -> -1.0
-      0x8000 -> 1.0
-      0x0000 -> 0.0
-*/
-static inline fixp_t fixp_new16(s16 a)
+/**
+ * fixp_sin32() returns the sin of an angle in degrees
+ *
+ * @degrees: angle, in degrees. The angle can be positive or negative
+ *
+ * The returned value ranges from -0x7fffffff to +0x7fffffff.
+ */
+static inline s32 fixp_sin32(int degrees)
 {
-	return ((s32)a)>>(16-FRAC_N);
+	degrees = (degrees % 360 + 360) % 360;
+
+	return __fixp_sin32(degrees);
 }
 
-static inline fixp_t fixp_cos(unsigned int degrees)
+/* cos(x) = sin(x + 90 degrees) */
+#define fixp_cos32(v) fixp_sin32((v) + 90)
+
+/*
+ * 16 bits variants
+ *
+ * The returned value ranges from -0x7fff to 0x7fff
+ */
+
+#define fixp_sin16(v) (fixp_sin32(v) >> 16)
+#define fixp_cos16(v) (fixp_cos32(v) >> 16)
+
+/**
+ * fixp_sin32_rad() - calculates the sin of an angle in radians
+ *
+ * @radians: angle, in radians
+ * @twopi: value to be used for 2*pi
+ *
+ * Provides a variant for the cases where just 360
+ * values is not enough. This function uses linear
+ * interpolation to a wider range of values given by
+ * twopi var.
+ *
+ * Experimental tests gave a maximum difference of
+ * 0.000038 between the value calculated by sin() and
+ * the one produced by this function, when twopi is
+ * equal to 360000. That seems to be enough precision
+ * for practical purposes.
+ *
+ * Please notice that two high numbers for twopi could cause
+ * overflows, so the routine will not allow values of twopi
+ * bigger than 1^18.
+ */
+static inline s32 fixp_sin32_rad(u32 radians, u32 twopi)
 {
-	int quadrant = (degrees / 90) & 3;
-	unsigned int i = degrees % 90;
+	int degrees;
+	s32 v1, v2, dx, dy;
+	s64 tmp;
 
-	if (quadrant == 1 || quadrant == 3)
-		i = 90 - i;
+	/*
+	 * Avoid too large values for twopi, as we don't want overflows.
+	 */
+	BUG_ON(twopi > 1 << 18);
 
-	i >>= 1;
+	degrees = (radians * 360) / twopi;
 
-	return (quadrant == 1 || quadrant == 2)? -cos_table[i] : cos_table[i];
-}
+	degrees = (degrees % 360 + 360) % 360;
+	v1 = __fixp_sin32(degrees);
 
-static inline fixp_t fixp_sin(unsigned int degrees)
-{
-	return -fixp_cos(degrees + 90);
-}
+	v2 = fixp_sin32(degrees + 1);
 
-static inline fixp_t fixp_mult(fixp_t a, fixp_t b)
-{
-	return ((s32)(a*b))>>FRAC_N;
+	dx = twopi / 360;
+	dy = v2 - v1;
+
+	tmp = radians - (degrees * twopi) / 360;
+	tmp *= dy;
+
+	return v1 +  div_s64(tmp, dx);
 }
 
+/* cos(x) = sin(x + pi/2 radians) */
+
+#define fixp_cos32_rad(rad, twopi)	\
+	fixp_sin32_rad(rad + twopi / 4, twopi)
+
 #endif