All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] fixp-arith/vivid: update sin/cos implementation
@ 2015-02-04  9:07 Hans Verkuil
  2015-02-04  9:07 ` [PATCH 1/3] fixp-arith: replace sin/cos table by a better precision one Hans Verkuil
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Hans Verkuil @ 2015-02-04  9:07 UTC (permalink / raw)
  To: linux-media; +Cc: Hans de Goede, Dmitry Torokhov, linux-input

These patches improve the sine tone generation in the vivid SDR driver (Software
Defined Radio). The main problem in that driver was the poor sin/cos function
implementation.

This started with improvements in vivid sine tone generation. It turned out
that to improve sine tone, we needed to improve the implementation of the sine
and cosine functions. Instead of improving the sin/cos implementation in vivid,
Mauro suggested that we should improve the common implementation in
include/linux/fixp-arith.h. This is a better approach compared to maintaining
a native fixed implementation of sin/cos for every module. Mauro has provided
the patch for improved sin/cos implementation in fixp-arith.h.

Please find patches for:

1. Improved LUT based fixed point implementation for sine/cosine calculation.
2. Use new sin/cos implementation in sine tone generation code in vivid
3. Modify equation used to generate sample for frequency modulated wave

Since patch 1 touches on the ff-memless driver and the gspca ov534 driver
we need Acks from the driver maintainers or (in the case of ff-memless) from
the input subsystem maintainer.

Some background behind changes in vivid sdr:

Vivid generates a frequency modulated (FM) signal (I and Q components) for SDR.
The 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 the 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,
it is found that sine wave is discontinuous and the frequency spectrum is spread
out a lot. Based on my discussion with Hans and Antti we found that the reasons
are:

1. Limited precision and rounding errors in calculations: the current
   implementation in vivid sir uses Taylor series calculation to calculate
   sine/cosine. Using a look up table based implementation improves the
   accuracy.
2. The equation used to compute modulated signal.


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

* [PATCH 1/3] fixp-arith: replace sin/cos table by a better precision one
  2015-02-04  9:07 [PATCH 0/3] fixp-arith/vivid: update sin/cos implementation Hans Verkuil
@ 2015-02-04  9:07 ` Hans Verkuil
  2015-02-04 19:33   ` Dmitry Torokhov
  2015-02-04  9:07 ` [PATCH 2/3] vivid sdr: Use LUT based implementation for sin/cos() Hans Verkuil
  2015-02-04  9:07 ` [PATCH 3/3] vivid sdr: fix broken sine tone generated for sdr FM Hans Verkuil
  2 siblings, 1 reply; 5+ messages in thread
From: Hans Verkuil @ 2015-02-04  9:07 UTC (permalink / raw)
  To: linux-media
  Cc: Hans de Goede, Dmitry Torokhov, linux-input,
	Mauro Carvalho Chehab, Prashant Laddha, Hans Verkuil

From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

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, which 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.

Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org <linux-input@vger.kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Signed-off-by: Prashant Laddha <prladdha@cisco.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/input/ff-memless.c      |  18 ++++-
 drivers/media/usb/gspca/ov534.c |  11 +--
 include/linux/fixp-arith.h      | 145 +++++++++++++++++++++++++++++-----------
 3 files changed, 125 insertions(+), 49 deletions(-)

diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
index 74c0d8c..fcc6c33 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 a9c866d..146071b 100644
--- a/drivers/media/usb/gspca/ov534.c
+++ b/drivers/media/usb/gspca/ov534.c
@@ -816,21 +816,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 3089d73..d4686fe 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;
+	tmp = radians - (degrees * twopi) / 360;
 
-	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 *= 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
-- 
2.1.4


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

* [PATCH 2/3] vivid sdr: Use LUT based implementation for sin/cos()
  2015-02-04  9:07 [PATCH 0/3] fixp-arith/vivid: update sin/cos implementation Hans Verkuil
  2015-02-04  9:07 ` [PATCH 1/3] fixp-arith: replace sin/cos table by a better precision one Hans Verkuil
@ 2015-02-04  9:07 ` Hans Verkuil
  2015-02-04  9:07 ` [PATCH 3/3] vivid sdr: fix broken sine tone generated for sdr FM Hans Verkuil
  2 siblings, 0 replies; 5+ messages in thread
From: Hans Verkuil @ 2015-02-04  9:07 UTC (permalink / raw)
  To: linux-media
  Cc: Hans de Goede, Dmitry Torokhov, linux-input,
	Mauro Carvalho Chehab, Antti Palosaari, Prashant Laddha,
	Hans Verkuil

From: Prashant Laddha <prladdha@cisco.com>

The common implementation for sin/cos in include/linux/fixp-arith.h
has been improved recently to provide higher precision.

Replacing native implementation of sin/cos in vivid sdr with common
implementation. This serves two purposes:

1. Improved accuracy: the native implementation based on the Taylor
   series is more prone to rounding errors.
2. Reuse of common function: this is better compared to maintaining
   native versions for each driver.

Suggested by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Cc: Antti Palosaari <crope@iki.fi>
Signed-off-by: Prashant Laddha <prladdha@cisco.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/platform/vivid/vivid-sdr-cap.c | 66 ++++++++++++----------------
 1 file changed, 27 insertions(+), 39 deletions(-)

diff --git a/drivers/media/platform/vivid/vivid-sdr-cap.c b/drivers/media/platform/vivid/vivid-sdr-cap.c
index 4af55f1..5e089cb 100644
--- a/drivers/media/platform/vivid/vivid-sdr-cap.c
+++ b/drivers/media/platform/vivid/vivid-sdr-cap.c
@@ -27,6 +27,7 @@
 #include <media/v4l2-common.h>
 #include <media/v4l2-event.h>
 #include <media/v4l2-dv-timings.h>
+#include <linux/fixp-arith.h>
 
 #include "vivid-core.h"
 #include "vivid-ctrls.h"
@@ -423,40 +424,19 @@ 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));
-}
+#define FIXP_N    (15)
+#define FIXP_FRAC (1 << FIXP_N)
+#define FIXP_2PI  ((int)(2 * 3.141592653589 * FIXP_FRAC))
 
 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;
+	s32 src_phase_step;
+	s32 mod_phase_step;
+	s32 fixp_i;
+	s32 fixp_q;
 
 	/*
 	 * TODO: Generated beep tone goes very crackly when sample rate is
@@ -466,28 +446,36 @@ 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,
+	src_phase_step = DIV_ROUND_CLOSEST(FIXP_2PI * BEEP_FREQ,
 			dev->sdr_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;
+		mod_phase_step = fixp_cos32_rad(dev->sdr_fixp_src_phase,
+						FIXP_2PI) >> (31 - FIXP_N);
+
+		dev->sdr_fixp_src_phase += src_phase_step;
+		dev->sdr_fixp_mod_phase += mod_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);
+		while (dev->sdr_fixp_mod_phase < FIXP_2PI)
+			dev->sdr_fixp_mod_phase += FIXP_2PI;
+		while (dev->sdr_fixp_mod_phase > FIXP_2PI)
+			dev->sdr_fixp_mod_phase -= FIXP_2PI;
+
+		while (dev->sdr_fixp_src_phase > FIXP_2PI)
+			dev->sdr_fixp_src_phase -= FIXP_2PI;
 
-		while (dev->sdr_fixp_src_phase > (2 * FIXP_PI))
-			dev->sdr_fixp_src_phase -= (2 * FIXP_PI);
+		fixp_i = fixp_cos32_rad(dev->sdr_fixp_mod_phase, FIXP_2PI);
+		fixp_q = fixp_sin32_rad(dev->sdr_fixp_mod_phase, FIXP_2PI);
 
-		fixp_i = fixp_cos(dev->sdr_fixp_mod_phase);
-		fixp_q = fixp_sin(dev->sdr_fixp_mod_phase);
+		/* Normalize fraction values represented with 32 bit precision
+		 * to fixed point representation with FIXP_N bits */
+		fixp_i >>= (31 - FIXP_N);
+		fixp_q >>= (31 - FIXP_N);
 
 		/* convert 'fixp float' to u8 */
 		/* u8 = X * 127.5f + 127.5f; where X is float [-1.0 / +1.0] */
-- 
2.1.4


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

* [PATCH 3/3] vivid sdr: fix broken sine tone generated for sdr FM
  2015-02-04  9:07 [PATCH 0/3] fixp-arith/vivid: update sin/cos implementation Hans Verkuil
  2015-02-04  9:07 ` [PATCH 1/3] fixp-arith: replace sin/cos table by a better precision one Hans Verkuil
  2015-02-04  9:07 ` [PATCH 2/3] vivid sdr: Use LUT based implementation for sin/cos() Hans Verkuil
@ 2015-02-04  9:07 ` Hans Verkuil
  2 siblings, 0 replies; 5+ messages in thread
From: Hans Verkuil @ 2015-02-04  9:07 UTC (permalink / raw)
  To: linux-media
  Cc: Hans de Goede, Dmitry Torokhov, linux-input, Antti Palosaari,
	Prashant Laddha, Hans Verkuil

From: Prashant Laddha <prladdha@cisco.com>

FM (frequency modulated) signal for SDR is generated by varying the
phase, where phase variation is proportional to input signal. It is
seen that, the larger phase increments leads to discontinuities in
the signal recovered after demodulation. Reducing the extent of phase
variation with respect to input signal, equivalent to reducing the
modulation index.

Tested using FM receiver flow graph in gnuradio-companion.

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

diff --git a/drivers/media/platform/vivid/vivid-sdr-cap.c b/drivers/media/platform/vivid/vivid-sdr-cap.c
index 5e089cb..caf1316 100644
--- a/drivers/media/platform/vivid/vivid-sdr-cap.c
+++ b/drivers/media/platform/vivid/vivid-sdr-cap.c
@@ -454,7 +454,7 @@ void vivid_sdr_cap_process(struct vivid_dev *dev, struct vivid_buffer *buf)
 						FIXP_2PI) >> (31 - FIXP_N);
 
 		dev->sdr_fixp_src_phase += src_phase_step;
-		dev->sdr_fixp_mod_phase += mod_phase_step;
+		dev->sdr_fixp_mod_phase += mod_phase_step / 4;
 
 		/*
 		 * Transfer phases to [0 / 2xPI] in order to avoid variable
-- 
2.1.4


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

* Re: [PATCH 1/3] fixp-arith: replace sin/cos table by a better precision one
  2015-02-04  9:07 ` [PATCH 1/3] fixp-arith: replace sin/cos table by a better precision one Hans Verkuil
@ 2015-02-04 19:33   ` Dmitry Torokhov
  0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2015-02-04 19:33 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, Hans de Goede, linux-input, Mauro Carvalho Chehab,
	Prashant Laddha, Hans Verkuil

On Wed, Feb 04, 2015 at 10:07:30AM +0100, Hans Verkuil wrote:
> From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> 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, which 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.
> 
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org <linux-input@vger.kernel.org>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> Signed-off-by: Prashant Laddha <prladdha@cisco.com>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

For input bit:

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> ---
>  drivers/input/ff-memless.c      |  18 ++++-
>  drivers/media/usb/gspca/ov534.c |  11 +--
>  include/linux/fixp-arith.h      | 145 +++++++++++++++++++++++++++++-----------
>  3 files changed, 125 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
> index 74c0d8c..fcc6c33 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 a9c866d..146071b 100644
> --- a/drivers/media/usb/gspca/ov534.c
> +++ b/drivers/media/usb/gspca/ov534.c
> @@ -816,21 +816,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 3089d73..d4686fe 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;
> +	tmp = radians - (degrees * twopi) / 360;
>  
> -	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 *= 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
> -- 
> 2.1.4
> 

-- 
Dmitry

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

end of thread, other threads:[~2015-02-04 19:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-04  9:07 [PATCH 0/3] fixp-arith/vivid: update sin/cos implementation Hans Verkuil
2015-02-04  9:07 ` [PATCH 1/3] fixp-arith: replace sin/cos table by a better precision one Hans Verkuil
2015-02-04 19:33   ` Dmitry Torokhov
2015-02-04  9:07 ` [PATCH 2/3] vivid sdr: Use LUT based implementation for sin/cos() Hans Verkuil
2015-02-04  9:07 ` [PATCH 3/3] vivid sdr: fix broken sine tone generated for sdr FM Hans Verkuil

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.