[PATCH 0/2] ASoC: tlv320dac33: FIFO mode updates

[PATCH 0/2] ASoC: tlv320dac33: FIFO mode updates

[Peter Ujfalusi]

Hello,

The main feature of this series is the automatic FIFO configuration of the FIFO,
when one of the FIFO mode is selected.
The first patch simplifies the handling of the Mode1 in manual mode, and also
removes the hardwired latency definition from the code, so it can be adjusted
based on the underlying platform.

The second patch adds option to select the automatic FIFO configuration.
When this mode is selected the driver will configure the FIFO to match the
stream's period size with the burst length (in terms of samples bursted out from
the host).
In Mode1 this is trivial, since I can configure the nSample to match with the
period size. However the Mode7 is a bit more complicated. The formula
(UTHR_FROM_PERIOD_SIZE) calculates the UTHR configuration for the given number
of samples.

The patches are generated against:
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git:topic/asoc

---
Peter Ujfalusi (2):
  ASoC: tlv320dac33: Revisit the FIFO Mode1 handling
  ASoC: tlv320dac33: Add support for automatic FIFO configuration

 include/sound/tlv320dac33-plat.h |    2 +
 sound/soc/codecs/tlv320dac33.c   |  125 ++++++++++++++++++++++++--------------
 2 files changed, 82 insertions(+), 45 deletions(-)

--
1.7.2

[PATCH 1/2] ASoC: tlv320dac33: Revisit the FIFO Mode1 handling

[Peter Ujfalusi]

Replace the hardwired latency definition with platform data
parameter, and simplify the nSample parameter calculation.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 include/sound/tlv320dac33-plat.h |    1 +
 sound/soc/codecs/tlv320dac33.c   |   71 ++++++++++++++++++--------------------
 2 files changed, 35 insertions(+), 37 deletions(-)

diff --git a/include/sound/tlv320dac33-plat.h b/include/sound/tlv320dac33-plat.h
index 3f428d5..30ff25b 100644
--- a/include/sound/tlv320dac33-plat.h
+++ b/include/sound/tlv320dac33-plat.h
@@ -15,6 +15,7 @@
 
 struct tlv320dac33_platform_data {
 	int power_gpio;
+	int mode1_i2c_latency; /* latency caused by the i2c writes in us */
 	int keep_bclk;	/* Keep the BCLK running in FIFO modes */
 	u8 burst_bclkdiv;
 };
diff --git a/sound/soc/codecs/tlv320dac33.c b/sound/soc/codecs/tlv320dac33.c
index 2fa946c..3b8f80e 100644
--- a/sound/soc/codecs/tlv320dac33.c
+++ b/sound/soc/codecs/tlv320dac33.c
@@ -49,8 +49,6 @@
 
 #define NSAMPLE_MAX		5700
 
-#define LATENCY_TIME_MS		20
-
 #define MODE7_LTHR		10
 #define MODE7_UTHR		(DAC33_BUFFER_SIZE_SAMPLES - 10)
 
@@ -107,6 +105,8 @@ struct tlv320dac33_priv {
 					 * this */
 	enum dac33_fifo_modes fifo_mode;/* FIFO mode selection */
 	unsigned int nsample;		/* burst read amount from host */
+	int mode1_i2c_latency;		/* latency caused by the i2c writes in
+					 * us */
 	u8 burst_bclkdiv;		/* BCLK divider value in burst mode */
 	unsigned int burst_rate;	/* Interface speed in Burst modes */
 
@@ -649,7 +649,7 @@ static inline void dac33_prefill_handler(struct tlv320dac33_priv *dac33)
 	switch (dac33->fifo_mode) {
 	case DAC33_FIFO_MODE1:
 		dac33_write16(codec, DAC33_NSAMPLE_MSB,
-			DAC33_THRREG(dac33->nsample + dac33->alarm_threshold));
+			DAC33_THRREG(dac33->nsample));
 
 		/* Take the timestamps */
 		spin_lock_irq(&dac33->lock);
@@ -798,6 +798,10 @@ static void dac33_shutdown(struct snd_pcm_substream *substream,
 	struct tlv320dac33_priv *dac33 = snd_soc_codec_get_drvdata(codec);
 
 	dac33->substream = NULL;
+
+	/* Reset the nSample restrictions */
+	dac33->nsample_min = 0;
+	dac33->nsample_max = NSAMPLE_MAX;
 }
 
 static int dac33_hw_params(struct snd_pcm_substream *substream,
@@ -1040,48 +1044,38 @@ static void dac33_calculate_times(struct snd_pcm_substream *substream)
 	struct snd_soc_device *socdev = rtd->socdev;
 	struct snd_soc_codec *codec = socdev->card->codec;
 	struct tlv320dac33_priv *dac33 = snd_soc_codec_get_drvdata(codec);
+	unsigned int period_size = substream->runtime->period_size;
+	unsigned int rate = substream->runtime->rate;
 	unsigned int nsample_limit;
 
 	/* In bypass mode we don't need to calculate */
 	if (!dac33->fifo_mode)
 		return;
 
-	/* Number of samples (16bit, stereo) in one period */
-	dac33->nsample_min = snd_pcm_lib_period_bytes(substream) / 4;
-
-	/* Number of samples (16bit, stereo) in ALSA buffer */
-	dac33->nsample_max = snd_pcm_lib_buffer_bytes(substream) / 4;
-	/* Subtract one period from the total */
-	dac33->nsample_max -= dac33->nsample_min;
-
-	/* Number of samples for LATENCY_TIME_MS / 2 */
-	dac33->alarm_threshold = substream->runtime->rate /
-				 (1000 / (LATENCY_TIME_MS / 2));
-
-	/* Find and fix up the lowest nsmaple limit */
-	nsample_limit = substream->runtime->rate / (1000 / LATENCY_TIME_MS);
-
-	if (dac33->nsample_min < nsample_limit)
-		dac33->nsample_min = nsample_limit;
-
-	if (dac33->nsample < dac33->nsample_min)
-		dac33->nsample = dac33->nsample_min;
-
-	/*
-	 * Find and fix up the highest nsmaple limit
-	 * In order to not overflow the DAC33 buffer substract the
-	 * alarm_threshold value from the size of the DAC33 buffer
-	 */
-	nsample_limit = DAC33_BUFFER_SIZE_SAMPLES - dac33->alarm_threshold;
-
-	if (dac33->nsample_max > nsample_limit)
-		dac33->nsample_max = nsample_limit;
-
-	if (dac33->nsample > dac33->nsample_max)
-		dac33->nsample = dac33->nsample_max;
-
 	switch (dac33->fifo_mode) {
 	case DAC33_FIFO_MODE1:
+		/* Number of samples under i2c latency */
+		dac33->alarm_threshold = US_TO_SAMPLES(rate,
+						dac33->mode1_i2c_latency);
+		/* nSample time shall not be shorter than i2c latency */
+		dac33->nsample_min = dac33->alarm_threshold;
+		/*
+		 * nSample should not be bigger than alsa buffer minus
+		 * size of one period to avoid overruns
+		 */
+		dac33->nsample_max = substream->runtime->buffer_size -
+					period_size;
+		nsample_limit = DAC33_BUFFER_SIZE_SAMPLES -
+				dac33->alarm_threshold;
+		if (dac33->nsample_max > nsample_limit)
+			dac33->nsample_max = nsample_limit;
+
+		/* Correct the nSample if it is outside of the ranges */
+		if (dac33->nsample < dac33->nsample_min)
+			dac33->nsample = dac33->nsample_min;
+		if (dac33->nsample > dac33->nsample_max)
+			dac33->nsample = dac33->nsample_max;
+
 		dac33->mode1_us_burst = SAMPLES_TO_US(dac33->burst_rate,
 						      dac33->nsample);
 		dac33->t_stamp1 = 0;
@@ -1519,6 +1513,9 @@ static int __devinit dac33_i2c_probe(struct i2c_client *client,
 	/* Pre calculate the burst rate */
 	dac33->burst_rate = BURST_BASEFREQ_HZ / dac33->burst_bclkdiv / 32;
 	dac33->keep_bclk = pdata->keep_bclk;
+	dac33->mode1_i2c_latency = pdata->mode1_i2c_latency;
+	if (!dac33->mode1_i2c_latency)
+		dac33->mode1_i2c_latency = 10000; /* 10ms */
 	dac33->irq = client->irq;
 	dac33->nsample = NSAMPLE_MAX;
 	dac33->nsample_max = NSAMPLE_MAX;
-- 
1.7.2

Re: [PATCH 1/2] ASoC: tlv320dac33: Revisit the FIFO Mode1 handling

[Mark Brown]

On Tue, Jul 27, 2010 at 09:46:16AM +0300, Peter Ujfalusi wrote:
> Replace the hardwired latency definition with platform data
> parameter, and simplify the nSample parameter calculation.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

I did notice that the default changes from 20ms:

> -#define LATENCY_TIME_MS		20

> +	if (!dac33->mode1_i2c_latency)
> +		dac33->mode1_i2c_latency = 10000; /* 10ms */

to 10ms with the new code but that shouldn't be a problem.

Re: [PATCH 1/2] ASoC: tlv320dac33: Revisit the FIFO Mode1 handling

[Peter Ujfalusi]

On Tuesday 27 July 2010 19:32:44 ext Mark Brown wrote:
> I did notice that the default changes from 20ms:
> > -#define LATENCY_TIME_MS		20
> > 
> > +	if (!dac33->mode1_i2c_latency)
> > +		dac33->mode1_i2c_latency = 10000; /* 10ms */
> 
> to 10ms with the new code but that shouldn't be a problem.

Correct.
The LATENCY_TIME_MS was used for two things:
 alarm threshold = LATENCY_TIME_MS / 2
 nSample lower limit = LATENCY_TIME_MS

The new default (10ms) comes from the old alarm threshold time.

There is no real need to limit the nSample for higher value, it is safe to use 
the same time constraint for alarm threshold and nSample lower limit.
The important thing is actually the alarm threshold value here: we need to have 
enough time to handle the interrupt, schedule the work, and write to nSample 
registers (via i2c) within certain amount of time.
Probably naming the mode1_i2c_latency differently, like mode1_latency would be 
better.

Probably it would be better if I resend the series with this change?

Thanks,
Péter

[PATCH 2/2] ASoC: tlv320dac33: Add support for automatic FIFO configuration

[Peter Ujfalusi]

Platform parameter to enable automatic FIFO configuration when
the codec is in Mode1 or Mode7 FIFO mode.
When this mode is selected, the controls for changing
nSample (in Mode1), and UTHR (in Mode7) are not added.
The driver configures the FIFO configuration based on
the stream's period size in a way, that every burst will
read period size of data from the host.
In Mode7 we need to use a formula, which gives close enough
aproximation for the burst length from the host point
of view.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 include/sound/tlv320dac33-plat.h |    1 +
 sound/soc/codecs/tlv320dac33.c   |   90 +++++++++++++++++++++++++++-----------
 2 files changed, 65 insertions(+), 26 deletions(-)

diff --git a/include/sound/tlv320dac33-plat.h b/include/sound/tlv320dac33-plat.h
index 30ff25b..5cd887b 100644
--- a/include/sound/tlv320dac33-plat.h
+++ b/include/sound/tlv320dac33-plat.h
@@ -16,6 +16,7 @@
 struct tlv320dac33_platform_data {
 	int power_gpio;
 	int mode1_i2c_latency; /* latency caused by the i2c writes in us */
+	int auto_fifo_config; /* FIFO config based on the period size */
 	int keep_bclk;	/* Keep the BCLK running in FIFO modes */
 	u8 burst_bclkdiv;
 };
diff --git a/sound/soc/codecs/tlv320dac33.c b/sound/soc/codecs/tlv320dac33.c
index 3b8f80e..05734d9 100644
--- a/sound/soc/codecs/tlv320dac33.c
+++ b/sound/soc/codecs/tlv320dac33.c
@@ -60,6 +60,9 @@
 #define US_TO_SAMPLES(rate, us) \
 	(rate / (1000000 / us))
 
+#define UTHR_FROM_PERIOD_SIZE(samples, playrate, burstrate) \
+	((samples * 5000) / ((burstrate * 5000) / (burstrate - playrate)))
+
 static void dac33_calculate_times(struct snd_pcm_substream *substream);
 static int dac33_prepare_chip(struct snd_pcm_substream *substream);
 
@@ -107,6 +110,8 @@ struct tlv320dac33_priv {
 	unsigned int nsample;		/* burst read amount from host */
 	int mode1_i2c_latency;		/* latency caused by the i2c writes in
 					 * us */
+	int auto_fifo_config; 		/* Configure the FIFO based on the
+					 * period size */
 	u8 burst_bclkdiv;		/* BCLK divider value in burst mode */
 	unsigned int burst_rate;	/* Interface speed in Burst modes */
 
@@ -538,13 +543,16 @@ static const struct snd_kcontrol_new dac33_snd_controls[] = {
 		 DAC33_LINEL_TO_LLO_VOL, DAC33_LINER_TO_RLO_VOL, 0, 127, 1),
 };
 
-static const struct snd_kcontrol_new dac33_nsample_snd_controls[] = {
+static const struct snd_kcontrol_new dac33_mode_snd_controls[] = {
+	SOC_ENUM_EXT("FIFO Mode", dac33_fifo_mode_enum,
+		 dac33_get_fifo_mode, dac33_set_fifo_mode),
+};
+
+static const struct snd_kcontrol_new dac33_fifo_snd_controls[] = {
 	SOC_SINGLE_EXT("nSample", 0, 0, 5900, 0,
-		 dac33_get_nsample, dac33_set_nsample),
+		dac33_get_nsample, dac33_set_nsample),
 	SOC_SINGLE_EXT("UTHR", 0, 0, MODE7_UTHR, 0,
 		 dac33_get_uthr, dac33_set_uthr),
-	SOC_ENUM_EXT("FIFO Mode", dac33_fifo_mode_enum,
-		 dac33_get_fifo_mode, dac33_set_fifo_mode),
 };
 
 /* Analog bypass */
@@ -1057,24 +1065,38 @@ static void dac33_calculate_times(struct snd_pcm_substream *substream)
 		/* Number of samples under i2c latency */
 		dac33->alarm_threshold = US_TO_SAMPLES(rate,
 						dac33->mode1_i2c_latency);
-		/* nSample time shall not be shorter than i2c latency */
-		dac33->nsample_min = dac33->alarm_threshold;
-		/*
-		 * nSample should not be bigger than alsa buffer minus
-		 * size of one period to avoid overruns
-		 */
-		dac33->nsample_max = substream->runtime->buffer_size -
-					period_size;
-		nsample_limit = DAC33_BUFFER_SIZE_SAMPLES -
-				dac33->alarm_threshold;
-		if (dac33->nsample_max > nsample_limit)
-			dac33->nsample_max = nsample_limit;
-
-		/* Correct the nSample if it is outside of the ranges */
-		if (dac33->nsample < dac33->nsample_min)
-			dac33->nsample = dac33->nsample_min;
-		if (dac33->nsample > dac33->nsample_max)
-			dac33->nsample = dac33->nsample_max;
+		if (dac33->auto_fifo_config) {
+			if (period_size <= dac33->alarm_threshold)
+				/*
+				 * Configure nSamaple to number of periods,
+				 * which covers the latency requironment.
+				 */
+				dac33->nsample = period_size *
+				       ((dac33->alarm_threshold / period_size) +
+				       (dac33->alarm_threshold % period_size ?
+				       1 : 0));
+			else
+				dac33->nsample = period_size;
+		} else {
+			/* nSample time shall not be shorter than i2c latency */
+			dac33->nsample_min = dac33->alarm_threshold;
+			/*
+			 * nSample should not be bigger than alsa buffer minus
+			 * size of one period to avoid overruns
+			 */
+			dac33->nsample_max = substream->runtime->buffer_size -
+						period_size;
+			nsample_limit = DAC33_BUFFER_SIZE_SAMPLES -
+					dac33->alarm_threshold;
+			if (dac33->nsample_max > nsample_limit)
+				dac33->nsample_max = nsample_limit;
+
+			/* Correct the nSample if it is outside of the ranges */
+			if (dac33->nsample < dac33->nsample_min)
+				dac33->nsample = dac33->nsample_min;
+			if (dac33->nsample > dac33->nsample_max)
+				dac33->nsample = dac33->nsample_max;
+		}
 
 		dac33->mode1_us_burst = SAMPLES_TO_US(dac33->burst_rate,
 						      dac33->nsample);
@@ -1082,6 +1104,16 @@ static void dac33_calculate_times(struct snd_pcm_substream *substream)
 		dac33->t_stamp2 = 0;
 		break;
 	case DAC33_FIFO_MODE7:
+		if (dac33->auto_fifo_config) {
+			dac33->uthr = UTHR_FROM_PERIOD_SIZE(
+					period_size,
+					rate,
+					dac33->burst_rate) + 9;
+			if (dac33->uthr > MODE7_UTHR)
+				dac33->uthr = MODE7_UTHR;
+			if (dac33->uthr < (MODE7_LTHR + 10))
+				dac33->uthr = (MODE7_LTHR + 10);
+		}
 		dac33->mode7_us_to_lthr =
 				SAMPLES_TO_US(substream->runtime->rate,
 					dac33->uthr - MODE7_LTHR + 1);
@@ -1379,10 +1411,15 @@ static int dac33_soc_probe(struct platform_device *pdev)
 
 	snd_soc_add_controls(codec, dac33_snd_controls,
 			     ARRAY_SIZE(dac33_snd_controls));
-	/* Only add the nSample controls, if we have valid IRQ number */
-	if (dac33->irq >= 0)
-		snd_soc_add_controls(codec, dac33_nsample_snd_controls,
-				     ARRAY_SIZE(dac33_nsample_snd_controls));
+	/* Only add the FIFO controls, if we have valid IRQ number */
+	if (dac33->irq >= 0) {
+		snd_soc_add_controls(codec, dac33_mode_snd_controls,
+				     ARRAY_SIZE(dac33_mode_snd_controls));
+		/* FIFO usage controls only, if autoio config is not selected */
+		if (!dac33->auto_fifo_config)
+			snd_soc_add_controls(codec, dac33_fifo_snd_controls,
+					ARRAY_SIZE(dac33_fifo_snd_controls));
+	}
 
 	dac33_add_widgets(codec);
 
@@ -1513,6 +1550,7 @@ static int __devinit dac33_i2c_probe(struct i2c_client *client,
 	/* Pre calculate the burst rate */
 	dac33->burst_rate = BURST_BASEFREQ_HZ / dac33->burst_bclkdiv / 32;
 	dac33->keep_bclk = pdata->keep_bclk;
+	dac33->auto_fifo_config = pdata->auto_fifo_config;
 	dac33->mode1_i2c_latency = pdata->mode1_i2c_latency;
 	if (!dac33->mode1_i2c_latency)
 		dac33->mode1_i2c_latency = 10000; /* 10ms */
-- 
1.7.2

Re: [PATCH 2/2] ASoC: tlv320dac33: Add support for automatic FIFO configuration

[Mark Brown]

On Tue, Jul 27, 2010 at 09:46:17AM +0300, Peter Ujfalusi wrote:
> Platform parameter to enable automatic FIFO configuration when
> the codec is in Mode1 or Mode7 FIFO mode.
> When this mode is selected, the controls for changing
> nSample (in Mode1), and UTHR (in Mode7) are not added.
> The driver configures the FIFO configuration based on
> the stream's period size in a way, that every burst will
> read period size of data from the host.
> In Mode7 we need to use a formula, which gives close enough
> aproximation for the burst length from the host point
> of view.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>