[PATCH 1/3] ASoC: change set_tdm_slot api to allow slot_width override.

[PATCH 1/3] ASoC: change set_tdm_slot api to allow slot_width override.

[Mark Brown]

From: Daniel Ribeiro <drwyrm@gmail.com>

Extend set_tdm_slot to allow the user to arbitrarily set the frame width
and active TX/RX slots.

Updates magician.c and wm9081.c for the new set_tdm_slot(). wm9081.c
still doesn't handle the slot_width override.

While being there, correct an incorrect use of SlotsPerFrm(7) use in
bitmask on pxa-ssp.c (SSCR0_SlotsPerFrm(x) is (((x) - 1) << 24)) ).

(this series is meant for Mark's for-2.6.32 branch)

Signed-off-by: Daniel Ribeiro <drwyrm@gmail.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 include/sound/soc-dai.h   |    5 +++--
 sound/soc/codecs/wm9081.c |    5 +++--
 sound/soc/pxa/magician.c  |    2 +-
 sound/soc/pxa/pxa-ssp.c   |   27 ++++++++++++++++++++-------
 sound/soc/soc-core.c      |    9 ++++++---
 5 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index 25d62ac..2d3fa29 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -106,7 +106,7 @@ int snd_soc_dai_set_pll(struct snd_soc_dai *dai,
 int snd_soc_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt);
 
 int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai,
-	unsigned int mask, int slots);
+	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width);
 
 int snd_soc_dai_set_tristate(struct snd_soc_dai *dai, int tristate);
 
@@ -140,7 +140,8 @@ struct snd_soc_dai_ops {
 	 */
 	int (*set_fmt)(struct snd_soc_dai *dai, unsigned int fmt);
 	int (*set_tdm_slot)(struct snd_soc_dai *dai,
-		unsigned int mask, int slots);
+		unsigned int tx_mask, unsigned int rx_mask,
+		int slots, int slot_width);
 	int (*set_tristate)(struct snd_soc_dai *dai, int tristate);
 
 	/*
diff --git a/sound/soc/codecs/wm9081.c b/sound/soc/codecs/wm9081.c
index 156f2a4..c965323 100644
--- a/sound/soc/codecs/wm9081.c
+++ b/sound/soc/codecs/wm9081.c
@@ -1214,8 +1214,9 @@ static int wm9081_set_sysclk(struct snd_soc_dai *codec_dai,
 	return 0;
 }
 
+/* FIXME: Needs to handle slot_width */
 static int wm9081_set_tdm_slot(struct snd_soc_dai *dai,
-			       unsigned int mask, int slots)
+	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
 {
 	struct snd_soc_codec *codec = dai->codec;
 	unsigned int aif1 = wm9081_read(codec, WM9081_AUDIO_INTERFACE_1);
@@ -1227,7 +1228,7 @@ static int wm9081_set_tdm_slot(struct snd_soc_dai *dai,
 
 	aif1 |= (slots - 1) << WM9081_AIFDAC_TDM_MODE_SHIFT;
 
-	switch (mask) {
+	switch (rx_mask) {
 	case 1:
 		break;
 	case 2:
diff --git a/sound/soc/pxa/magician.c b/sound/soc/pxa/magician.c
index 8889cd3..9f7c61e 100644
--- a/sound/soc/pxa/magician.c
+++ b/sound/soc/pxa/magician.c
@@ -190,7 +190,7 @@ static int magician_playback_hw_params(struct snd_pcm_substream *substream,
 	if (ret < 0)
 		return ret;
 
-	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 1, 1);
+	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 1, 0, 1, width);
 	if (ret < 0)
 		return ret;
 
diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c
index e22c5ce..5b9ed64 100644
--- a/sound/soc/pxa/pxa-ssp.c
+++ b/sound/soc/pxa/pxa-ssp.c
@@ -375,21 +375,34 @@ static int pxa_ssp_set_dai_pll(struct snd_soc_dai *cpu_dai,
  * Set the active slots in TDM/Network mode
  */
 static int pxa_ssp_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai,
-	unsigned int mask, int slots)
+	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
 {
 	struct ssp_priv *priv = cpu_dai->private_data;
 	struct ssp_device *ssp = priv->dev.ssp;
 	u32 sscr0;
 
-	sscr0 = ssp_read_reg(ssp, SSCR0) & ~SSCR0_SlotsPerFrm(7);
+	sscr0 = ssp_read_reg(ssp, SSCR0);
+	sscr0 &= ~(SSCR0_MOD | SSCR0_SlotsPerFrm(8) | SSCR0_EDSS | SSCR0_DSS);
+
+	/* set slot width */
+	if (slot_width > 16)
+		sscr0 |= SSCR0_EDSS | SSCR0_DataSize(slot_width - 16);
+	else
+		sscr0 |= SSCR0_DataSize(slot_width);
 
-	/* set number of active slots */
-	sscr0 |= SSCR0_SlotsPerFrm(slots);
+	if (slots > 1) {
+		/* enable network mode */
+		sscr0 |= SSCR0_MOD;
+
+		/* set number of active slots */
+		sscr0 |= SSCR0_SlotsPerFrm(slots);
+
+		/* set active slot mask */
+		ssp_write_reg(ssp, SSTSA, tx_mask);
+		ssp_write_reg(ssp, SSRSA, rx_mask);
+	}
 	ssp_write_reg(ssp, SSCR0, sscr0);
 
-	/* set active slot mask */
-	ssp_write_reg(ssp, SSTSA, mask);
-	ssp_write_reg(ssp, SSRSA, mask);
 	return 0;
 }
 
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index fb8d7a7..e984a17 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2223,17 +2223,20 @@ EXPORT_SYMBOL_GPL(snd_soc_dai_set_fmt);
 /**
  * snd_soc_dai_set_tdm_slot - configure DAI TDM.
  * @dai: DAI
- * @mask: DAI specific mask representing used slots.
+ * @tx_mask: bitmask representing active TX slots.
+ * @rx_mask: bitmask representing active RX slots.
  * @slots: Number of slots in use.
+ * @slot_width: Width in bits for each slot.
  *
  * Configures a DAI for TDM operation. Both mask and slots are codec and DAI
  * specific.
  */
 int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai,
-	unsigned int mask, int slots)
+	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
 {
 	if (dai->ops && dai->ops->set_tdm_slot)
-		return dai->ops->set_tdm_slot(dai, mask, slots);
+		return dai->ops->set_tdm_slot(dai, tx_mask, rx_mask,
+				slots, slot_width);
 	else
 		return -EINVAL;
 }
-- 
1.6.3.3

[PATCH 2/3] ASoC: pxa-ssp.c, Automatically set TDM when needed

[Mark Brown]

From: Daniel Ribeiro <drwyrm@gmail.com>

* Automatically sets TDM mode for frame_width larger than 32 bits, if
  the user doesn't setup the TDM slots with set_tdm_slot().
* Reset SSCR0_EDSS and SSCR0_DSS on pxa_ssp_set_dai_fmt.
* Makes SSCR0_MOD optional.
* Clears SSCR1_RWOT case SSCR0_MOD is set.

Signed-off-by: Daniel Ribeiro <drwyrm@gmail.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 sound/soc/pxa/pxa-ssp.c |   92 +++++++++++++++++++++++++++--------------------
 1 files changed, 53 insertions(+), 39 deletions(-)

diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c
index 5b9ed64..d60492e 100644
--- a/sound/soc/pxa/pxa-ssp.c
+++ b/sound/soc/pxa/pxa-ssp.c
@@ -452,8 +452,8 @@ static int pxa_ssp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	}
 
 	/* reset port settings */
-	sscr0 = ssp_read_reg(ssp, SSCR0) &
-		(SSCR0_ECS |  SSCR0_NCS | SSCR0_MOD | SSCR0_ACS);
+	sscr0 = ssp_read_reg(ssp, SSCR0) & (SSCR0_ECS | SSCR0_NCS | SSCR0_MOD |
+			SSCR0_ACS | SSCR0_DSS | SSCR0_EDSS);
 	sscr1 = SSCR1_RxTresh(8) | SSCR1_TxTresh(7);
 	sspsp = 0;
 
@@ -496,7 +496,7 @@ static int pxa_ssp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	case SND_SOC_DAIFMT_DSP_A:
 		sspsp |= SSPSP_FSRT;
 	case SND_SOC_DAIFMT_DSP_B:
-		sscr0 |= SSCR0_MOD | SSCR0_PSP;
+		sscr0 |= SSCR0_PSP;
 		sscr1 |= SSCR1_TRAIL | SSCR1_RWOT;
 		break;
 
@@ -532,48 +532,70 @@ static int pxa_ssp_hw_params(struct snd_pcm_substream *substream,
 	struct ssp_priv *priv = cpu_dai->private_data;
 	struct ssp_device *ssp = priv->dev.ssp;
 	int chn = params_channels(params);
-	u32 sscr0;
-	u32 sspsp;
+	u32 sscr0, sscr1, sspsp;
 	int width = snd_pcm_format_physical_width(params_format(params));
-	int ttsa = ssp_read_reg(ssp, SSTSA) & 0xf;
+	int slot_width, frame_width = 0;
+
+	/* check if the user explicitly set a slot_width */
+	sscr0 = ssp_read_reg(ssp, SSCR0);
+
+	if (sscr0 & (SSCR0_EDSS | SSCR0_DSS))
+		slot_width = (sscr0 & SSCR0_DSS) +
+			(sscr0 & SSCR0_EDSS ? 17 : 1);
+	else
+		frame_width = slot_width = width * chn;
 
 	/* generate correct DMA params */
 	if (cpu_dai->dma_data)
 		kfree(cpu_dai->dma_data);
 
-	/* Network mode with one active slot (ttsa == 1) can be used
-	 * to force 16-bit frame width on the wire (for S16_LE), even
-	 * with two channels. Use 16-bit DMA transfers for this case.
-	 */
-	cpu_dai->dma_data = ssp_get_dma_params(ssp,
-			((chn == 2) && (ttsa != 1)) || (width == 32),
+	cpu_dai->dma_data = ssp_get_dma_params(ssp, slot_width > 16,
 			substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
 
 	/* we can only change the settings if the port is not in use */
-	if (ssp_read_reg(ssp, SSCR0) & SSCR0_SSE)
+	if (sscr0 & SSCR0_SSE)
 		return 0;
 
-	/* clear selected SSP bits */
-	sscr0 = ssp_read_reg(ssp, SSCR0) & ~(SSCR0_DSS | SSCR0_EDSS);
-	ssp_write_reg(ssp, SSCR0, sscr0);
-
-	/* bit size */
-	sscr0 = ssp_read_reg(ssp, SSCR0);
-	switch (params_format(params)) {
-	case SNDRV_PCM_FORMAT_S16_LE:
 #ifdef CONFIG_PXA3xx
-		if (cpu_is_pxa3xx())
-			sscr0 |= SSCR0_FPCKE;
+	if (slot_width == 16 && cpu_is_pxa3xx())
+		sscr0 |= SSCR0_FPCKE;
 #endif
-		sscr0 |= SSCR0_DataSize(16);
-		break;
-	case SNDRV_PCM_FORMAT_S24_LE:
-		sscr0 |= (SSCR0_EDSS | SSCR0_DataSize(8));
-		break;
-	case SNDRV_PCM_FORMAT_S32_LE:
-		sscr0 |= (SSCR0_EDSS | SSCR0_DataSize(16));
-		break;
+
+	if (frame_width > 0) {
+		/* Not using network mode */
+		if (frame_width > 16)
+			sscr0 |= SSCR0_EDSS | SSCR0_DataSize(frame_width - 16);
+		else
+			sscr0 |= SSCR0_DataSize(frame_width);
+
+		if (frame_width > 32) {
+			/*
+			 * Network mode is needed to support this frame width.
+			 * We assume the wire is not networked and setup a
+			 * fake network mode here. Use as many slots as needed
+			 * each with 32 bits.
+			 */
+			int slots = frame_width / 32;
+
+			sscr0 |= SSCR0_MOD;
+			sscr0 |= SSCR0_SlotsPerFrm(slots);
+
+			/*
+			 * Set active slots. Only set an active TX slot
+			 * if we are going to use it.
+			 */
+			ssp_write_reg(ssp, SSRSA, slots - 1);
+			if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+				ssp_write_reg(ssp, SSTSA, slots - 1);
+		}
 	}
+
+	/* If SSCR0_MOD is set we can't use SSCR1_RWOT */
+	if (sscr0 & SSCR0_MOD) {
+		sscr1 = ssp_read_reg(ssp, SSCR1);
+		ssp_write_reg(ssp, SSCR1, sscr1 & ~SSCR1_RWOT);
+	}
+
 	ssp_write_reg(ssp, SSCR0, sscr0);
 
 	switch (priv->dai_fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
@@ -620,14 +642,6 @@ static int pxa_ssp_hw_params(struct snd_pcm_substream *substream,
 		break;
 	}
 
-	/* When we use a network mode, we always require TDM slots
-	 * - complain loudly and fail if they've not been set up yet.
-	 */
-	if ((sscr0 & SSCR0_MOD) && !ttsa) {
-		dev_err(&ssp->pdev->dev, "No TDM timeslot configured\n");
-		return -EINVAL;
-	}
-
 	dump_registers(ssp);
 
 	return 0;
-- 
1.6.3.3

Re: [PATCH 2/3] ASoC: pxa-ssp.c, Automatically set TDM when needed

[Daniel Mack]

Ok, I spent some time in debugging this again, and here are my
findings.

Again, to summarize what this is all about - what I need the code to
produce is the following output:

	- syncronous LRCLK
	- 64fs on I2SCLK
	- I2S data on the first 16bits of each LRCLK edge
	-> http://caiaq.de/download/tmp/2.png

So I applied all three patches and added the following call to my board
support code:

	snd_soc_dai_set_tdm_slot(cpu_dai, 3, 3, 2, 16);

Which translates into "give me 2 slots with 16 bits each, and distribute
them over the timeline as a 0-1-0-1 pattern". Which is exactly what I
need, right?

But unfortunately, the code didn't make the SSP port do that. I ended up
having a asynchronous clock and the data somewhere where I wouldn't have
expected them: http://caiaq.de/download/tmp/1.png

I did some minor modifications to the patches 2/3 and 3/3 which made it
work for me eventually - find them inline.

Not sure whether I got the theory about the API right, and of course I
didn't test on any other hardware than the one I have.


On Thu, Aug 06, 2009 at 03:55:18PM +0100, Mark Brown wrote:
> * Automatically sets TDM mode for frame_width larger than 32 bits, if
>   the user doesn't setup the TDM slots with set_tdm_slot().
> * Reset SSCR0_EDSS and SSCR0_DSS on pxa_ssp_set_dai_fmt.
> * Makes SSCR0_MOD optional.
> * Clears SSCR1_RWOT case SSCR0_MOD is set.
> 
> Signed-off-by: Daniel Ribeiro <drwyrm@gmail.com>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  sound/soc/pxa/pxa-ssp.c |   92 +++++++++++++++++++++++++++--------------------
>  1 files changed, 53 insertions(+), 39 deletions(-)
> 
> diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c
> index 5b9ed64..d60492e 100644
> --- a/sound/soc/pxa/pxa-ssp.c
> +++ b/sound/soc/pxa/pxa-ssp.c

[...]

> @@ -532,48 +532,70 @@ static int pxa_ssp_hw_params(struct snd_pcm_substream *substream,
>  	struct ssp_priv *priv = cpu_dai->private_data;
>  	struct ssp_device *ssp = priv->dev.ssp;
>  	int chn = params_channels(params);
> -	u32 sscr0;
> -	u32 sspsp;
> +	u32 sscr0, sscr1, sspsp;
>  	int width = snd_pcm_format_physical_width(params_format(params));
> -	int ttsa = ssp_read_reg(ssp, SSTSA) & 0xf;
> +	int slot_width, frame_width = 0;
> +
> +	/* check if the user explicitly set a slot_width */
> +	sscr0 = ssp_read_reg(ssp, SSCR0);
> +
> +	if (sscr0 & (SSCR0_EDSS | SSCR0_DSS))
> +		slot_width = (sscr0 & SSCR0_DSS) +
> +			(sscr0 & SSCR0_EDSS ? 17 : 1);
> +	else
> +		frame_width = slot_width = width * chn;
>  
>  	/* generate correct DMA params */
>  	if (cpu_dai->dma_data)
>  		kfree(cpu_dai->dma_data);
>  
> -	/* Network mode with one active slot (ttsa == 1) can be used
> -	 * to force 16-bit frame width on the wire (for S16_LE), even
> -	 * with two channels. Use 16-bit DMA transfers for this case.
> -	 */
> -	cpu_dai->dma_data = ssp_get_dma_params(ssp,
> -			((chn == 2) && (ttsa != 1)) || (width == 32),
> +	cpu_dai->dma_data = ssp_get_dma_params(ssp, slot_width > 16,
>  			substream->stream == SNDRV_PCM_STREAM_PLAYBACK);

The condition for the width4 parameter needs to be
(slot_width * slots) > 16 for me. With that parameter set to 0, audio
plays at half speed on my system.

And in patch 3/3,


> diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c
> index d60492e..aac85fe 100644
> --- a/sound/soc/pxa/pxa-ssp.c
> +++ b/sound/soc/pxa/pxa-ssp.c

[...]

> @@ -561,6 +543,59 @@ static int pxa_ssp_hw_params(struct snd_pcm_substream *substream,
>  		sscr0 |= SSCR0_FPCKE;
>  #endif
>  
> +	sspsp = ssp_read_reg(ssp, SSPSP);
> +	switch (priv->dai_fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> +	case SND_SOC_DAIFMT_I2S:
> +		/*
> +		 * I2S and LEFT_J are stereo only, we have to send data for
> +		 * both channels.
> +		 */
> +		if (chn == 1)
> +			frame_width *= 2;
> +
> +		/*
> +		 * If the user did not use network mode, we assume the codec
> +		 * is I2S compliant.
> +		 */
> +		if (frame_width > 0) {
> +			sspsp |= SSPSP_SFRMWDTH(frame_width / 2);
> +			sspsp |= SSPSP_FSRT;
> +		} else {
> +			/*
> +			 * Otherwise we assume that it is a single TDM slot, and
> +			 * the user is abusing set_tdm_slot to support an
> +			 * out of spec codec.
> +			 */
> +			int slots = ((sscr0 & SSCR0_SlotsPerFrm(8)) >> 24) + 1;
> +
> +			/* PXA2XX doesn't support DMYSTOP > 3 */
> +			if (slot_width != (width * 2) && !cpu_is_pxa3xx())
> +				return -EINVAL;
> +
> +			sspsp |= SSPSP_DMYSTRT(1);
> +			sspsp |= SSPSP_DMYSTOP(
> +					(slot_width * slots) / 2 - width - 1);
> +			sspsp |= SSPSP_SFRMWDTH((slot_width * slots) / 2);

These two calculations are wrong my case. What works here is

	sspsp |= SSPSP_DMYSTOP(
			(slot_width * slots * chn) / 2 - width - 1);
	sspsp |= SSPSP_SFRMWDTH((slot_width * slots * chn) / 2);

... which is another multiplication by factor 2. But I'm not sure if I
got the wrong variable with value 2 :)

I then end up having the following register contents:

	SSCR0 0xa10003ff
	SSCR1 0x00601d08
	SSPSP 0x31a00084
	SSACD 0x00000060
	SSTSA 0x00000003
	SSRSA 0x00000003

Daniel

Re: [PATCH 2/3] ASoC: pxa-ssp.c, Automatically set TDM when needed

[Daniel Ribeiro]

[-- Attachment #1.1: Type: text/plain, Size: 5168 bytes --]

Hi, sorry for the delay on replying to this thread, busy with other
stuff. :)

Em Qua, 2009-08-12 às 20:17 +0200, Daniel Mack escreveu:
> Again, to summarize what this is all about - what I need the code to
> produce is the following output:
> 
> 	- syncronous LRCLK
> 	- 64fs on I2SCLK
> 	- I2S data on the first 16bits of each LRCLK edge
> 	-> http://caiaq.de/download/tmp/2.png
> 
> So I applied all three patches and added the following call to my board
> support code:
> 
> 	snd_soc_dai_set_tdm_slot(cpu_dai, 3, 3, 2, 16);
> 
> Which translates into "give me 2 slots with 16 bits each, and distribute
> them over the timeline as a 0-1-0-1 pattern". Which is exactly what I
> need, right?

No. in an ideal world, set_tdm_slot(3, 3, 2, 16) shoud give you 2x16
bits slots, on a 1-1 timeline, and shouldn't be a valid network mode
configuration, in this case, one should use a frame width of 32bits and
_not_ use network mode.

0-1-0-1 should be 4 slots, with slots 2 and 4 active, but I think that
what you want is 1-0-1-0 = set_tdm_slot(5, 5, 4, 16). But... See below.

> But unfortunately, the code didn't make the SSP port do that. I ended up
> having a asynchronous clock and the data somewhere where I wouldn't have
> expected them: http://caiaq.de/download/tmp/1.png

I think that the source of the problem is that we can't assert SFRM past
the end of the first slot, and DMYSTOP changes the slot_width on the
wire.

Consider that PXA is _slave_ of SFRM: (SFRMDIR=1)

* SFRMWDTH is ignored.
* Only one edge (set by SFRMP) is considered.


And when PXA is _master_ of SFRM: (SFRMDIR=0)

* SFRMWDTH is needed for I2S.
* SFRMWDTH can't be asserted past the end of DMYSTOP.
* DMYSTOP changes the slot width, as it generates extra cycles after the
last bit of data.
* DMYSTOP only changes the _active_ slots width. (this is what causes
async SFRM).
* DMYSTOP is restricted to 0-3 on PXA2XX
* DMYSTOP must be cleared in network mode, or when FSRT is set.


We want to write 2x16bit samples with 1-0-1-0 pattern for I2S, so we
can:

* set_tdm_slot(5, 5, 4, 16) if pxa is slave of SFRM.
* use SFRMWDTH(16) | DMYSTOP(16) if pxa is master of SFRM. (pxa3 only)
* use DMYSTOP(16) if pxa is slave of SFRM. (pxa3 only)


> I did some minor modifications to the patches 2/3 and 3/3 which made it
> work for me eventually - find them inline.
> 
> Not sure whether I got the theory about the API right, and of course I
> didn't test on any other hardware than the one I have.
> 
> 

> > -	cpu_dai->dma_data = ssp_get_dma_params(ssp,
> > -			((chn == 2) && (ttsa != 1)) || (width == 32),
> > +	cpu_dai->dma_data = ssp_get_dma_params(ssp, slot_width > 16,
> >  			substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
> 
> The condition for the width4 parameter needs to be
> (slot_width * slots) > 16 for me. With that parameter set to 0, audio
> plays at half speed on my system.

Consider that you have real network mode, and you are using 2 slots of
16 bit mono audio. You want to ignore 16bit for each 16bit of data. ie,
only one channel active. You really don't want 32bit DMA in this case.

I agree that this should be (slot_width * number_of_active_slots) > 16
tough.

> And in patch 3/3,
> 

> > +			sspsp |= SSPSP_DMYSTOP(
> > +					(slot_width * slots) / 2 - width - 1);
> > +			sspsp |= SSPSP_SFRMWDTH((slot_width * slots) / 2);
> 
> These two calculations are wrong my case. What works here is
> 
> 	sspsp |= SSPSP_DMYSTOP(
> 			(slot_width * slots * chn) / 2 - width - 1);
> 	sspsp |= SSPSP_SFRMWDTH((slot_width * slots * chn) / 2);
> 
> ... which is another multiplication by factor 2. But I'm not sure if I
> got the wrong variable with value 2 :)

DMYSTOP itself should be accounted for slot_width, but this will only
work if all slots are active otherwise you will get asynchronous SFRM.
Something like:

/*
 * If the user did not use network mode, we assume the codec
 * is I2S compliant.
 */
if (frame_width > 0) {
	sspsp |= SSPSP_SFRMWDTH(frame_width / 2);
	sspsp |= SSPSP_FSRT;
} else {
	if (sscr0 & SSCR0_SFRMDIR) {
		/* pxa slave, FSRT is enough */
		sspsp |= SSPSP_FSRT;
	} else {
		/* pxa master */
		int slots = ((sscr0 & SSCR0_SlotsPerFrm(8)) >> 24) + 1;

		/* PXA2XX doesn't support DMYSTOP > 3 */
		if (!cpu_is_pxa3xx())
			return -EINVAL;
		/* 
		 * We need all slots active, otherwise DMYSTOP causes
		 * asynchronous SFRM as it only appends bit cycles after
		 * active slots.
		 */
		if (ssta != (1 << slots) - 1)
			return -EINVAL;

		/* slot width on the wire is doubled by DMYSTOP */
		sspsp |= SSPSP_DMYSTRT(1);
		sspsp |= SSPSP_DMYSTOP(slot_width * slots - width - 1);
		sspsp |= SSPSP_SFRMWDTH(slot_width * slots);


This should be able to deal with I2S when pxa is slave, on both pxa2xx
and pxa3xx, with set_tdm_slot(5, 5, 4, 16).

Or when pxa is master, on pxa3xx only, with set_tdm_slot(3, 3, 2, 16).
This still violates the "DMYSTOP must be clear on network mode" rule,
but as all slots are active its not really network mode. And it seems to
work for you, so... ;)
		

-- 
Daniel Ribeiro

[-- Attachment #1.2: Esta é uma parte de mensagem assinada digitalmente --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 2/3] ASoC: pxa-ssp.c, Automatically set TDM when needed

[Mark Brown]

On Fri, Aug 14, 2009 at 01:17:41PM -0300, Daniel Ribeiro wrote:

> This should be able to deal with I2S when pxa is slave, on both pxa2xx
> and pxa3xx, with set_tdm_slot(5, 5, 4, 16).

> Or when pxa is master, on pxa3xx only, with set_tdm_slot(3, 3, 2, 16).
> This still violates the "DMYSTOP must be clear on network mode" rule,
> but as all slots are active its not really network mode. And it seems to
> work for you, so... ;)

This seems really confusing from a usability point of view - the master
and slave configurations should look the same.  I'd expec the slave mode
configuration you quote above to be looking for twice as many clocks as
the master mode one.  The driver should ideally encapsulate this.

Re: [PATCH 2/3] ASoC: pxa-ssp.c, Automatically set TDM when needed

[Daniel Ribeiro]

[-- Attachment #1.1: Type: text/plain, Size: 1273 bytes --]

Em Sáb, 2009-08-15 às 10:51 +0100, Mark Brown escreveu:
> On Fri, Aug 14, 2009 at 01:17:41PM -0300, Daniel Ribeiro wrote:
> 
> > This should be able to deal with I2S when pxa is slave, on both pxa2xx
> > and pxa3xx, with set_tdm_slot(5, 5, 4, 16).
> 
> > Or when pxa is master, on pxa3xx only, with set_tdm_slot(3, 3, 2, 16).
> > This still violates the "DMYSTOP must be clear on network mode" rule,
> > but as all slots are active its not really network mode. And it seems to
> > work for you, so... ;)
> 
> This seems really confusing from a usability point of view - the master
> and slave configurations should look the same.  I'd expec the slave mode
> configuration you quote above to be looking for twice as many clocks as
> the master mode one.  The driver should ideally encapsulate this.

Yes, this is really confusing. The hardware is weird and behaves
different when it is driving SFRM.

On both (slave/master) modes it will use the same clk cycles, its just
that on master half the clocks are skipped by DMYSTOP.

Maybe Daniel Mack can test a setup with PXA slave of SFRM, and if it
works for him, we can drop the pxa3xx-only I2S/master mode for now. It
is a mess and I don't see an easy way to solve this. :(

-- 
Daniel Ribeiro

[-- Attachment #1.2: Esta é uma parte de mensagem assinada digitalmente --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 2/3] ASoC: pxa-ssp.c, Automatically set TDM when needed

[Daniel Mack]

On Fri, Aug 14, 2009 at 01:17:41PM -0300, Daniel Ribeiro wrote:
> Em Qua, 2009-08-12 às 20:17 +0200, Daniel Mack escreveu:
> > 
> > 	snd_soc_dai_set_tdm_slot(cpu_dai, 3, 3, 2, 16);
> > 
> > Which translates into "give me 2 slots with 16 bits each, and distribute
> > them over the timeline as a 0-1-0-1 pattern". Which is exactly what I
> > need, right?
> 
> No. in an ideal world, set_tdm_slot(3, 3, 2, 16) shoud give you 2x16
> bits slots, on a 1-1 timeline, and shouldn't be a valid network mode
> configuration, in this case, one should use a frame width of 32bits and
> _not_ use network mode.
> 
> 0-1-0-1 should be 4 slots, with slots 2 and 4 active, but I think that
> what you want is 1-0-1-0 = set_tdm_slot(5, 5, 4, 16). But... See below.

Maybe we should first decide which TDM mode _should_ be correct for the
mode I'm working on, and then amend the actual implementation? And
provide some documentation once the API is decided.

> > But unfortunately, the code didn't make the SSP port do that. I ended up
> > having a asynchronous clock and the data somewhere where I wouldn't have
> > expected them: http://caiaq.de/download/tmp/1.png
> 
> I think that the source of the problem is that we can't assert SFRM past
> the end of the first slot, and DMYSTOP changes the slot_width on the
> wire.

Yes, I've seen this, too.

(Just a sidenote: one way to debug that engine is to interrupt the
execution using Marvell's XDB tool and then manually modify the register
set while the PXA keeps on playing the same DMA buffer over and over
again. You'll get ~1/2 sec of audio in a loop - enough to judge whether
it's playing at the correct speed.)

> Consider that PXA is _slave_ of SFRM: (SFRMDIR=1)
> 
> * SFRMWDTH is ignored.
> * Only one edge (set by SFRMP) is considered.
> 
> 
> And when PXA is _master_ of SFRM: (SFRMDIR=0)

As Mark pointed out already, this implementation details should be
hidden behind the API. Users really shouldn't need to care.

> * SFRMWDTH is needed for I2S.
> * SFRMWDTH can't be asserted past the end of DMYSTOP.
> * DMYSTOP changes the slot width, as it generates extra cycles after the
> last bit of data.
> * DMYSTOP only changes the _active_ slots width. (this is what causes
> async SFRM).
> * DMYSTOP is restricted to 0-3 on PXA2XX
> * DMYSTOP must be cleared in network mode, or when FSRT is set.

Well, for this last point: that's not actually true. The configuration
I'm currently working with _does_ have the MOD bit set which indicates
a 'network' mode (whatever the PXA's implementation does with that
particular information) and DMYSTOP _does_ matter. Which is a
contradiction to the specs, yes, but that's what it's like.

> We want to write 2x16bit samples with 1-0-1-0 pattern for I2S, so we
> can:
> 
> * set_tdm_slot(5, 5, 4, 16) if pxa is slave of SFRM.

Wouldn't 1-0-1-0 rather be 0xa?

> * use SFRMWDTH(16) | DMYSTOP(16) if pxa is master of SFRM. (pxa3 only)
> * use DMYSTOP(16) if pxa is slave of SFRM. (pxa3 only)

> 
> > I did some minor modifications to the patches 2/3 and 3/3 which made it
> > work for me eventually - find them inline.
> > 
> > Not sure whether I got the theory about the API right, and of course I
> > didn't test on any other hardware than the one I have.
> > 
> > 
> 
> > > -	cpu_dai->dma_data = ssp_get_dma_params(ssp,
> > > -			((chn == 2) && (ttsa != 1)) || (width == 32),
> > > +	cpu_dai->dma_data = ssp_get_dma_params(ssp, slot_width > 16,
> > >  			substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
> > 
> > The condition for the width4 parameter needs to be
> > (slot_width * slots) > 16 for me. With that parameter set to 0, audio
> > plays at half speed on my system.
> 
> Consider that you have real network mode, and you are using 2 slots of
> 16 bit mono audio. You want to ignore 16bit for each 16bit of data. ie,
> only one channel active. You really don't want 32bit DMA in this case.

I can just say that in case the DMA material is _not_ 32bit, audio will
play at half speed. So the mode I'm using seems to need the double amout
of PCM data. For whatever reason.

> I agree that this should be (slot_width * number_of_active_slots) > 16
> tough.

Ok. And that won't break your use case?

> > > +			sspsp |= SSPSP_DMYSTOP(
> > > +					(slot_width * slots) / 2 - width - 1);
> > > +			sspsp |= SSPSP_SFRMWDTH((slot_width * slots) / 2);
> > 
> > These two calculations are wrong my case. What works here is
> > 
> > 	sspsp |= SSPSP_DMYSTOP(
> > 			(slot_width * slots * chn) / 2 - width - 1);
> > 	sspsp |= SSPSP_SFRMWDTH((slot_width * slots * chn) / 2);
> > 
> > ... which is another multiplication by factor 2. But I'm not sure if I
> > got the wrong variable with value 2 :)
> 
> DMYSTOP itself should be accounted for slot_width, but this will only
> work if all slots are active otherwise you will get asynchronous SFRM.
> Something like:

[...]

Care to send modified patches? I'd be lucky to test them.

> This should be able to deal with I2S when pxa is slave, on both pxa2xx
> and pxa3xx, with set_tdm_slot(5, 5, 4, 16).
> 
> Or when pxa is master, on pxa3xx only, with set_tdm_slot(3, 3, 2, 16).

Again - shouldn't we care for a consistent calling convention here and
handle the special cases in the implementation rather than feeding
nebulous integer arguments to it, which vary depending on the clocking
directions?

> This still violates the "DMYSTOP must be clear on network mode" rule,
> but as all slots are active its not really network mode. And it seems to
> work for you, so... ;)

Yeah, that makes no sense at all. I think we should ignore that comment.
It seems to be simply wrong, or we're not in network mode afterall, even
though we selected it. I don't know.

Thanks,
Daniel

Re: [PATCH 2/3] ASoC: pxa-ssp.c, Automatically set TDM when needed

[Mark Brown]

On Mon, Aug 17, 2009 at 05:35:48PM +0200, Daniel Mack wrote:
> On Fri, Aug 14, 2009 at 01:17:41PM -0300, Daniel Ribeiro wrote:

> > Or when pxa is master, on pxa3xx only, with set_tdm_slot(3, 3, 2, 16).

> Again - shouldn't we care for a consistent calling convention here and
> handle the special cases in the implementation rather than feeding
> nebulous integer arguments to it, which vary depending on the clocking
> directions?

Yes, the arguments to set_tdm_slot() should have a consistent meaning
over all devices and configurations of those devices.  There will be an
inversion of the input vs. output between two connected devices but that
would be the only difference.

The key issue here is if we're looking at stereo slots or mono slots.
Stereo is probably a bit easier to work with when you're using that
(especially for I2S derived modes where the interface is fundamentally
stereo) but obviously there's the potential to run into cases where that
causes issues so mono covers our bets more.

> > This still violates the "DMYSTOP must be clear on network mode" rule,
> > but as all slots are active its not really network mode. And it seems to
> > work for you, so... ;)

> Yeah, that makes no sense at all. I think we should ignore that comment.
> It seems to be simply wrong, or we're not in network mode afterall, even
> though we selected it. I don't know.

Or the network mode is perfectly happy to support a range slot numbers
including zero.

Re: [PATCH 2/3] ASoC: pxa-ssp.c, Automatically set TDM when needed

[Daniel Ribeiro]

[-- Attachment #1.1: Type: text/plain, Size: 3750 bytes --]

Em Seg, 2009-08-17 às 17:35 +0200, Daniel Mack escreveu: 
> > 0-1-0-1 should be 4 slots, with slots 2 and 4 active, but I think that
> > what you want is 1-0-1-0 = set_tdm_slot(5, 5, 4, 16). But... See below.
> 
> Maybe we should first decide which TDM mode _should_ be correct for the
> mode I'm working on, and then amend the actual implementation? And
> provide some documentation once the API is decided.

The correct should be set_tdm_slots(5, 5, 4, 16), no doubt on this.

The problem is that "16" becomes "32" only on the active slots because
of DMYSTOP. :(

> > * set_tdm_slot(5, 5, 4, 16) if pxa is slave of SFRM.
> 
> Wouldn't 1-0-1-0 rather be 0xa?

Well, I'm reading 1-0-1-0 as a time-line(1-2-3-4) from left to right,
not as the binary value of the TTSA register. Activating slots 1 and 3
means 0x5(0101b) on TTSA value (right to left).

Confusing... ;)

> > Consider that you have real network mode, and you are using 2 slots of
> > 16 bit mono audio. You want to ignore 16bit for each 16bit of data. ie,
> > only one channel active. You really don't want 32bit DMA in this case.
> 
> I can just say that in case the DMA material is _not_ 32bit, audio will
> play at half speed. So the mode I'm using seems to need the double amout
> of PCM data. For whatever reason.

Its stereo, 2x16. So 32b DMA is correct. I use 32b DMA for 16bit stereo
too, but with 1 slot of 32bits. :)

> > I agree that this should be (slot_width * number_of_active_slots) > 16
> > tough.
> 
> Ok. And that won't break your use case?

No, in my case number_of_active_slots is always 1. (Both with and
without network mode on the wire)

> > > > +			sspsp |= SSPSP_DMYSTOP(
> > > > +					(slot_width * slots) / 2 - width - 1);
> > > > +			sspsp |= SSPSP_SFRMWDTH((slot_width * slots) / 2);
> > > 
> > > These two calculations are wrong my case. What works here is
> > > 
> > > 	sspsp |= SSPSP_DMYSTOP(
> > > 			(slot_width * slots * chn) / 2 - width - 1);
> > > 	sspsp |= SSPSP_SFRMWDTH((slot_width * slots * chn) / 2);
> > > 
> > > ... which is another multiplication by factor 2. But I'm not sure if I
> > > got the wrong variable with value 2 :)
> > 
> > DMYSTOP itself should be accounted for slot_width, but this will only
> > work if all slots are active otherwise you will get asynchronous SFRM.
> > Something like:
> 
> [...]
> 
> Care to send modified patches? I'd be lucky to test them.

Unfortunately I would not be able to test them at the moment, not even
compile test.. :/

I will send new patches when I'm back to my workstation.

> > This should be able to deal with I2S when pxa is slave, on both pxa2xx
> > and pxa3xx, with set_tdm_slot(5, 5, 4, 16).
> > 
> > Or when pxa is master, on pxa3xx only, with set_tdm_slot(3, 3, 2, 16).
> 
> Again - shouldn't we care for a consistent calling convention here and
> handle the special cases in the implementation rather than feeding
> nebulous integer arguments to it, which vary depending on the clocking
> directions?

Yes, I think we can change the slots configuration in the case we set
DMYSTOP.

> > This still violates the "DMYSTOP must be clear on network mode" rule,
> > but as all slots are active its not really network mode. And it seems to
> > work for you, so... ;)
> 
> Yeah, that makes no sense at all. I think we should ignore that comment.
> It seems to be simply wrong, or we're not in network mode afterall, even
> though we selected it. I don't know.

The comment is right for real network mode, when you want to ignore some
timeslots. In your case, as all slots are active its not really network
mode, its just a way to increase the frame width above the 32b limit.

-- 
Daniel Ribeiro

[-- Attachment #1.2: Esta é uma parte de mensagem assinada digitalmente --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 2/3] ASoC: pxa-ssp.c, Automatically set TDM when needed

[Daniel Mack]

On Thu, Aug 06, 2009 at 03:55:18PM +0100, Mark Brown wrote:
> * Automatically sets TDM mode for frame_width larger than 32 bits, if
>   the user doesn't setup the TDM slots with set_tdm_slot().
> * Reset SSCR0_EDSS and SSCR0_DSS on pxa_ssp_set_dai_fmt.
> * Makes SSCR0_MOD optional.
> * Clears SSCR1_RWOT case SSCR0_MOD is set.
> 
> Signed-off-by: Daniel Ribeiro <drwyrm@gmail.com>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  sound/soc/pxa/pxa-ssp.c |   92 +++++++++++++++++++++++++++--------------------
>  1 files changed, 53 insertions(+), 39 deletions(-)
> 
> diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c
> index 5b9ed64..d60492e 100644
> --- a/sound/soc/pxa/pxa-ssp.c
> +++ b/sound/soc/pxa/pxa-ssp.c

[...]

> @@ -532,48 +532,70 @@ static int pxa_ssp_hw_params(struct snd_pcm_substream *substream,
>  	struct ssp_priv *priv = cpu_dai->private_data;
>  	struct ssp_device *ssp = priv->dev.ssp;
>  	int chn = params_channels(params);
> -	u32 sscr0;
> -	u32 sspsp;
> +	u32 sscr0, sscr1, sspsp;
>  	int width = snd_pcm_format_physical_width(params_format(params));
> -	int ttsa = ssp_read_reg(ssp, SSTSA) & 0xf;
> +	int slot_width, frame_width = 0;
> +
> +	/* check if the user explicitly set a slot_width */
> +	sscr0 = ssp_read_reg(ssp, SSCR0);
> +
> +	if (sscr0 & (SSCR0_EDSS | SSCR0_DSS))
> +		slot_width = (sscr0 & SSCR0_DSS) +
> +			(sscr0 & SSCR0_EDSS ? 17 : 1);
> +	else
> +		frame_width = slot_width = width * chn;
>  
>  	/* generate correct DMA params */
>  	if (cpu_dai->dma_data)
>  		kfree(cpu_dai->dma_data);
>  
> -	/* Network mode with one active slot (ttsa == 1) can be used
> -	 * to force 16-bit frame width on the wire (for S16_LE), even
> -	 * with two channels. Use 16-bit DMA transfers for this case.
> -	 */
> -	cpu_dai->dma_data = ssp_get_dma_params(ssp,
> -			((chn == 2) && (ttsa != 1)) || (width == 32),
> +	cpu_dai->dma_data = ssp_get_dma_params(ssp, slot_width > 16,
>  			substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
>  
>  	/* we can only change the settings if the port is not in use */
> -	if (ssp_read_reg(ssp, SSCR0) & SSCR0_SSE)
> +	if (sscr0 & SSCR0_SSE)
>  		return 0;
>  
> -	/* clear selected SSP bits */
> -	sscr0 = ssp_read_reg(ssp, SSCR0) & ~(SSCR0_DSS | SSCR0_EDSS);
> -	ssp_write_reg(ssp, SSCR0, sscr0);
> -
> -	/* bit size */
> -	sscr0 = ssp_read_reg(ssp, SSCR0);
> -	switch (params_format(params)) {
> -	case SNDRV_PCM_FORMAT_S16_LE:
>  #ifdef CONFIG_PXA3xx
> -		if (cpu_is_pxa3xx())
> -			sscr0 |= SSCR0_FPCKE;
> +	if (slot_width == 16 && cpu_is_pxa3xx())

And I forgot to mention that this should read ">= 16".

Daniel

[PATCH 3/3] ASoC: pxa-ssp.c handle the I2S and LEFT_J cases.

[Mark Brown]

From: Daniel Ribeiro <drwyrm@gmail.com>

Handles I2S and LEFT_J cases, if the user does _not_ call set_tdm_slot
we assume that the codec is I2S compliant, and that we can use
frame_width = sample_width * channels.

If the user calls set_tdm_slot then we assume that the wire is _not_
networked, and that the user wants to set a non-standard frame format
instead, in this case we set the frame duration (the LRCLK falling edge)
to be half of the wire frame width.

eg.
* standard 2*[16|24|32|..] bits I2S/LEFT_J doesn't need to call
set_tdm_slot
* to use 2*16bits I2S/LEFTJ on 64bits frames, with the LRCLK falling
edge at the 32nd bitclk (+1 for I2S) needs to set_tdm_slot(5, 5, 4, 16)
(4 slots of 16 bits, first and third RX/TX slots active). This should
work even if pxa is slave of SFRM.

Signed-off-by: Daniel Ribeiro <drwyrm@gmail.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 arch/arm/mach-pxa/include/mach/regs-ssp.h |   14 ++--
 sound/soc/pxa/pxa-ssp.c                   |  121 +++++++++++++---------------
 2 files changed, 63 insertions(+), 72 deletions(-)

diff --git a/arch/arm/mach-pxa/include/mach/regs-ssp.h b/arch/arm/mach-pxa/include/mach/regs-ssp.h
index 6a2ed35..060e23b 100644
--- a/arch/arm/mach-pxa/include/mach/regs-ssp.h
+++ b/arch/arm/mach-pxa/include/mach/regs-ssp.h
@@ -108,21 +108,21 @@
 #define SSSR_TINT		(1 << 19)	/* Receiver Time-out Interrupt */
 #define SSSR_PINT		(1 << 18)	/* Peripheral Trailing Byte Interrupt */
 
-#if defined(CONFIG_PXA3xx)
-#define SSPSP_EDMYSTOP(x)	((x) << 28)     /* Extended Dummy Stop */
-#define SSPSP_EDMYSTRT(x)	((x) << 26)     /* Extended Dummy Start */
-#endif
-
 #define SSPSP_FSRT		(1 << 25)	/* Frame Sync Relative Timing */
-#define SSPSP_DMYSTOP(x)	((x) << 23)	/* Dummy Stop */
 #define SSPSP_SFRMWDTH(x)	((x) << 16)	/* Serial Frame Width */
 #define SSPSP_SFRMDLY(x)	((x) << 9)	/* Serial Frame Delay */
-#define SSPSP_DMYSTRT(x)	((x) << 7)	/* Dummy Start */
 #define SSPSP_STRTDLY(x)	((x) << 4)	/* Start Delay */
 #define SSPSP_ETDS		(1 << 3)	/* End of Transfer data State */
 #define SSPSP_SFRMP		(1 << 2)	/* Serial Frame Polarity */
 #define SSPSP_SCMODE(x)		((x) << 0)	/* Serial Bit Rate Clock Mode */
 
+/* NOTE: PXA3xx extends the bit number of dummy start and stop, the macros
+ * below are compatible with PXA25x/27x as long as the parameter is within
+ * the correct limits, driver code has to take care of this.
+ */
+#define SSPSP_DMYSTRT(x)       ((((x) & 3) << 7)  | ((((x) >> 2) & 3) << 26))
+#define SSPSP_DMYSTOP(x)       ((((x) & 3) << 23) | ((((x) >> 2) & 7) << 28))
+
 #define SSACD_SCDB		(1 << 3)	/* SSPSYSCLK Divider Bypass */
 #define SSACD_ACPS(x)		((x) << 4)	/* Audio clock PLL select */
 #define SSACD_ACDS(x)		((x) << 0)	/* Audio clock divider select */
diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c
index d60492e..aac85fe 100644
--- a/sound/soc/pxa/pxa-ssp.c
+++ b/sound/soc/pxa/pxa-ssp.c
@@ -179,21 +179,6 @@ static void ssp_set_scr(struct ssp_device *ssp, u32 div)
 	ssp_write_reg(ssp, SSCR0, sscr0);
 }
 
-/**
- * ssp_get_clkdiv - get SSP clock divider
- */
-static u32 ssp_get_scr(struct ssp_device *ssp)
-{
-	u32 sscr0 = ssp_read_reg(ssp, SSCR0);
-	u32 div;
-
-	if (cpu_is_pxa25x() && ssp->type == PXA25x_SSP)
-		div = ((sscr0 >> 8) & 0xff) * 2 + 2;
-	else
-		div = ((sscr0 >> 8) & 0xfff) + 1;
-	return div;
-}
-
 /*
  * Set the SSP ports SYSCLK.
  */
@@ -487,17 +472,14 @@ static int pxa_ssp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	}
 
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
-	case SND_SOC_DAIFMT_I2S:
-		sscr0 |= SSCR0_PSP;
-		sscr1 |= SSCR1_RWOT | SSCR1_TRAIL;
-		/* See hw_params() */
-		break;
-
 	case SND_SOC_DAIFMT_DSP_A:
 		sspsp |= SSPSP_FSRT;
 	case SND_SOC_DAIFMT_DSP_B:
+	case SND_SOC_DAIFMT_LEFT_J:
+	case SND_SOC_DAIFMT_I2S:
 		sscr0 |= SSCR0_PSP;
 		sscr1 |= SSCR1_TRAIL | SSCR1_RWOT;
+		/* See hw_params() for I2S and LEFT_J */
 		break;
 
 	default:
@@ -561,6 +543,59 @@ static int pxa_ssp_hw_params(struct snd_pcm_substream *substream,
 		sscr0 |= SSCR0_FPCKE;
 #endif
 
+	sspsp = ssp_read_reg(ssp, SSPSP);
+	switch (priv->dai_fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		/*
+		 * I2S and LEFT_J are stereo only, we have to send data for
+		 * both channels.
+		 */
+		if (chn == 1)
+			frame_width *= 2;
+
+		/*
+		 * If the user did not use network mode, we assume the codec
+		 * is I2S compliant.
+		 */
+		if (frame_width > 0) {
+			sspsp |= SSPSP_SFRMWDTH(frame_width / 2);
+			sspsp |= SSPSP_FSRT;
+		} else {
+			/*
+			 * Otherwise we assume that it is a single TDM slot, and
+			 * the user is abusing set_tdm_slot to support an
+			 * out of spec codec.
+			 */
+			int slots = ((sscr0 & SSCR0_SlotsPerFrm(8)) >> 24) + 1;
+
+			/* PXA2XX doesn't support DMYSTOP > 3 */
+			if (slot_width != (width * 2) && !cpu_is_pxa3xx())
+				return -EINVAL;
+
+			sspsp |= SSPSP_DMYSTRT(1);
+			sspsp |= SSPSP_DMYSTOP(
+					(slot_width * slots) / 2 - width - 1);
+			sspsp |= SSPSP_SFRMWDTH((slot_width * slots) / 2);
+		}
+		break;
+
+	case SND_SOC_DAIFMT_LEFT_J:
+		if (chn == 1)
+			frame_width *= 2;
+
+		if (frame_width > 0) {
+			sspsp |= SSPSP_SFRMWDTH(frame_width / 2);
+		} else {
+			int slots = ((sscr0 & SSCR0_SlotsPerFrm(8)) >> 24) + 1;
+
+			sspsp |= SSPSP_SFRMWDTH((slot_width * slots) / 2);
+		}
+		break;
+	default:
+		break;
+	}
+	ssp_write_reg(ssp, SSPSP, sspsp);
+
 	if (frame_width > 0) {
 		/* Not using network mode */
 		if (frame_width > 16)
@@ -598,50 +633,6 @@ static int pxa_ssp_hw_params(struct snd_pcm_substream *substream,
 
 	ssp_write_reg(ssp, SSCR0, sscr0);
 
-	switch (priv->dai_fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
-	case SND_SOC_DAIFMT_I2S:
-	       sspsp = ssp_read_reg(ssp, SSPSP);
-
-		if ((ssp_get_scr(ssp) == 4) && (width == 16)) {
-			/* This is a special case where the bitclk is 64fs
-			* and we're not dealing with 2*32 bits of audio
-			* samples.
-			*
-			* The SSP values used for that are all found out by
-			* trying and failing a lot; some of the registers
-			* needed for that mode are only available on PXA3xx.
-			*/
-
-#ifdef CONFIG_PXA3xx
-			if (!cpu_is_pxa3xx())
-				return -EINVAL;
-
-			sspsp |= SSPSP_SFRMWDTH(width * 2);
-			sspsp |= SSPSP_SFRMDLY(width * 4);
-			sspsp |= SSPSP_EDMYSTOP(3);
-			sspsp |= SSPSP_DMYSTOP(3);
-			sspsp |= SSPSP_DMYSTRT(1);
-#else
-			return -EINVAL;
-#endif
-		} else {
-			/* The frame width is the width the LRCLK is
-			 * asserted for; the delay is expressed in
-			 * half cycle units.  We need the extra cycle
-			 * because the data starts clocking out one BCLK
-			 * after LRCLK changes polarity.
-			 */
-			sspsp |= SSPSP_SFRMWDTH(width + 1);
-			sspsp |= SSPSP_SFRMDLY((width + 1) * 2);
-			sspsp |= SSPSP_DMYSTRT(1);
-		}
-
-		ssp_write_reg(ssp, SSPSP, sspsp);
-		break;
-	default:
-		break;
-	}
-
 	dump_registers(ssp);
 
 	return 0;
-- 
1.6.3.3

Re: [PATCH 1/3] ASoC: change set_tdm_slot api to allow slot_width override.

[Daniel Mack]

On Thu, Aug 06, 2009 at 03:55:17PM +0100, Mark Brown wrote:
> Extend set_tdm_slot to allow the user to arbitrarily set the frame width
> and active TX/RX slots.
> 
> Updates magician.c and wm9081.c for the new set_tdm_slot(). wm9081.c
> still doesn't handle the slot_width override.
> 
> While being there, correct an incorrect use of SlotsPerFrm(7) use in
> bitmask on pxa-ssp.c (SSCR0_SlotsPerFrm(x) is (((x) - 1) << 24)) ).
> 
> (this series is meant for Mark's for-2.6.32 branch)
> 
> Signed-off-by: Daniel Ribeiro <drwyrm@gmail.com>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Ok, thanks. Give me some days to build up my test environment here with
all the protocol analyzers and so - I'll get back on this issue next
week.

Daniel

Re: [PATCH 1/3] ASoC: change set_tdm_slot api to allow slot_width override.

[Mark Brown]

On Thu, Aug 06, 2009 at 03:55:17PM +0100, Mark Brown wrote:
> From: Daniel Ribeiro <drwyrm@gmail.com>
> 
> Extend set_tdm_slot to allow the user to arbitrarily set the frame width
> and active TX/RX slots.
> 
> Updates magician.c and wm9081.c for the new set_tdm_slot(). wm9081.c
> still doesn't handle the slot_width override.
> 
> While being there, correct an incorrect use of SlotsPerFrm(7) use in
> bitmask on pxa-ssp.c (SSCR0_SlotsPerFrm(x) is (((x) - 1) << 24)) ).

In order to pick up the set_tdm_slot() API change I'm going to apply
this first patch just now.  Please let me know if you run into any
problems - I'm still unable to run any PXA3xx tests due to issues with
my development platform.

[PATCH 1/3] ASoC: change set_tdm_slot api to allow slot_width override.

[Daniel Ribeiro]

[-- Attachment #1.1: Type: text/plain, Size: 5441 bytes --]

Extend set_tdm_slot to allow the user to arbitrarily set the frame width
and active TX/RX slots.

Updates magician.c and wm9081.c for the new set_tdm_slot(). wm9081.c
still doesn't handle the slot_width override.

While being there, correct an incorrect use of SlotsPerFrm(7) use in
bitmask on pxa-ssp.c (SSCR0_SlotsPerFrm(x) is (((x) - 1) << 24)) ).

(this series is meant for Mark's for-2.6.32 branch)

Signed-off-by: Daniel Ribeiro <drwyrm@gmail.com>

---
 include/sound/soc-dai.h   |    5 +++--
 sound/soc/codecs/wm9081.c |    5 +++--
 sound/soc/pxa/magician.c  |    2 +-
 sound/soc/pxa/pxa-ssp.c   |   27 ++++++++++++++++++++-------
 sound/soc/soc-core.c      |    9 ++++++---
 5 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index 352d7ee..6692410 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -106,7 +106,7 @@ int snd_soc_dai_set_pll(struct snd_soc_dai *dai,
 int snd_soc_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt);
 
 int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai,
-	unsigned int mask, int slots);
+	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width);
 
 int snd_soc_dai_set_tristate(struct snd_soc_dai *dai, int tristate);
 
@@ -140,7 +140,8 @@ struct snd_soc_dai_ops {
 	 */
 	int (*set_fmt)(struct snd_soc_dai *dai, unsigned int fmt);
 	int (*set_tdm_slot)(struct snd_soc_dai *dai,
-		unsigned int mask, int slots);
+		unsigned int tx_mask, unsigned int rx_mask,
+		int slots, int slot_width);
 	int (*set_tristate)(struct snd_soc_dai *dai, int tristate);
 
 	/*
diff --git a/sound/soc/codecs/wm9081.c b/sound/soc/codecs/wm9081.c
index 86fc57e..c47d573 100644
--- a/sound/soc/codecs/wm9081.c
+++ b/sound/soc/codecs/wm9081.c
@@ -1206,8 +1206,9 @@ static int wm9081_set_sysclk(struct snd_soc_dai *codec_dai,
 	return 0;
 }
 
+/* FIXME: Needs to handle slot_width */
 static int wm9081_set_tdm_slot(struct snd_soc_dai *dai,
-			       unsigned int mask, int slots)
+	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
 {
 	struct snd_soc_codec *codec = dai->codec;
 	unsigned int aif1 = wm9081_read(codec, WM9081_AUDIO_INTERFACE_1);
@@ -1219,7 +1220,7 @@ static int wm9081_set_tdm_slot(struct snd_soc_dai *dai,
 
 	aif1 |= (slots - 1) << WM9081_AIFDAC_TDM_MODE_SHIFT;
 
-	switch (mask) {
+	switch (rx_mask) {
 	case 1:
 		break;
 	case 2:
diff --git a/sound/soc/pxa/magician.c b/sound/soc/pxa/magician.c
index c89a3cd..34437d8 100644
--- a/sound/soc/pxa/magician.c
+++ b/sound/soc/pxa/magician.c
@@ -188,7 +188,7 @@ static int magician_playback_hw_params(struct snd_pcm_substream *substream,
 	if (ret < 0)
 		return ret;
 
-	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 1, 1);
+	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 1, 0, 1, width);
 	if (ret < 0)
 		return ret;
 
diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c
index e22c5ce..5b9ed64 100644
--- a/sound/soc/pxa/pxa-ssp.c
+++ b/sound/soc/pxa/pxa-ssp.c
@@ -375,21 +375,34 @@ static int pxa_ssp_set_dai_pll(struct snd_soc_dai *cpu_dai,
  * Set the active slots in TDM/Network mode
  */
 static int pxa_ssp_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai,
-	unsigned int mask, int slots)
+	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
 {
 	struct ssp_priv *priv = cpu_dai->private_data;
 	struct ssp_device *ssp = priv->dev.ssp;
 	u32 sscr0;
 
-	sscr0 = ssp_read_reg(ssp, SSCR0) & ~SSCR0_SlotsPerFrm(7);
+	sscr0 = ssp_read_reg(ssp, SSCR0);
+	sscr0 &= ~(SSCR0_MOD | SSCR0_SlotsPerFrm(8) | SSCR0_EDSS | SSCR0_DSS);
+
+	/* set slot width */
+	if (slot_width > 16)
+		sscr0 |= SSCR0_EDSS | SSCR0_DataSize(slot_width - 16);
+	else
+		sscr0 |= SSCR0_DataSize(slot_width);
 
-	/* set number of active slots */
-	sscr0 |= SSCR0_SlotsPerFrm(slots);
+	if (slots > 1) {
+		/* enable network mode */
+		sscr0 |= SSCR0_MOD;
+
+		/* set number of active slots */
+		sscr0 |= SSCR0_SlotsPerFrm(slots);
+
+		/* set active slot mask */
+		ssp_write_reg(ssp, SSTSA, tx_mask);
+		ssp_write_reg(ssp, SSRSA, rx_mask);
+	}
 	ssp_write_reg(ssp, SSCR0, sscr0);
 
-	/* set active slot mask */
-	ssp_write_reg(ssp, SSTSA, mask);
-	ssp_write_reg(ssp, SSRSA, mask);
 	return 0;
 }
 
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index e1a920c..c72a8e8 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2133,17 +2133,20 @@ EXPORT_SYMBOL_GPL(snd_soc_dai_set_fmt);
 /**
  * snd_soc_dai_set_tdm_slot - configure DAI TDM.
  * @dai: DAI
- * @mask: DAI specific mask representing used slots.
+ * @tx_mask: bitmask representing active TX slots.
+ * @rx_mask: bitmask representing active RX slots.
  * @slots: Number of slots in use.
+ * @slot_width: Width in bits for each slot.
  *
  * Configures a DAI for TDM operation. Both mask and slots are codec and DAI
  * specific.
  */
 int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai,
-	unsigned int mask, int slots)
+	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
 {
 	if (dai->ops && dai->ops->set_tdm_slot)
-		return dai->ops->set_tdm_slot(dai, mask, slots);
+		return dai->ops->set_tdm_slot(dai, tx_mask, rx_mask,
+				slots, slot_width);
 	else
 		return -EINVAL;
 }
-- 
tg: (c787980..) asoc/set_tdm_slot (depends on: asoc/remove-code-duplication)

-- 
Daniel Ribeiro

[-- Attachment #1.2: Esta é uma parte de mensagem assinada digitalmente --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/3] ASoC: change set_tdm_slot api to allow slot_width override.

[pHilipp Zabel]

2009/6/16 Daniel Ribeiro <drwyrm@gmail.com>:
> Extend set_tdm_slot to allow the user to arbitrarily set the frame width
> and active TX/RX slots.
>
> Updates magician.c and wm9081.c for the new set_tdm_slot(). wm9081.c
> still doesn't handle the slot_width override.
>
> While being there, correct an incorrect use of SlotsPerFrm(7) use in
> bitmask on pxa-ssp.c (SSCR0_SlotsPerFrm(x) is (((x) - 1) << 24)) ).
>
> (this series is meant for Mark's for-2.6.32 branch)
>
> Signed-off-by: Daniel Ribeiro <drwyrm@gmail.com>
>
> ---
>  include/sound/soc-dai.h   |    5 +++--
>  sound/soc/codecs/wm9081.c |    5 +++--
>  sound/soc/pxa/magician.c  |    2 +-
>  sound/soc/pxa/pxa-ssp.c   |   27 ++++++++++++++++++++-------

FWIW, the patchset doesn't break magician.
Tested-by: Philipp Zabel <philipp.zabel@gmail.com>
in my dev tree (for-2.6.31 and for-2.6.32 trees merged).

regards
Philipp

>  sound/soc/soc-core.c      |    9 ++++++---
>  5 files changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
> index 352d7ee..6692410 100644
> --- a/include/sound/soc-dai.h
> +++ b/include/sound/soc-dai.h
> @@ -106,7 +106,7 @@ int snd_soc_dai_set_pll(struct snd_soc_dai *dai,
>  int snd_soc_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt);
>
>  int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai,
> -       unsigned int mask, int slots);
> +       unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width);
>
>  int snd_soc_dai_set_tristate(struct snd_soc_dai *dai, int tristate);
>
> @@ -140,7 +140,8 @@ struct snd_soc_dai_ops {
>         */
>        int (*set_fmt)(struct snd_soc_dai *dai, unsigned int fmt);
>        int (*set_tdm_slot)(struct snd_soc_dai *dai,
> -               unsigned int mask, int slots);
> +               unsigned int tx_mask, unsigned int rx_mask,
> +               int slots, int slot_width);
>        int (*set_tristate)(struct snd_soc_dai *dai, int tristate);
>
>        /*
> diff --git a/sound/soc/codecs/wm9081.c b/sound/soc/codecs/wm9081.c
> index 86fc57e..c47d573 100644
> --- a/sound/soc/codecs/wm9081.c
> +++ b/sound/soc/codecs/wm9081.c
> @@ -1206,8 +1206,9 @@ static int wm9081_set_sysclk(struct snd_soc_dai *codec_dai,
>        return 0;
>  }
>
> +/* FIXME: Needs to handle slot_width */
>  static int wm9081_set_tdm_slot(struct snd_soc_dai *dai,
> -                              unsigned int mask, int slots)
> +       unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
>  {
>        struct snd_soc_codec *codec = dai->codec;
>        unsigned int aif1 = wm9081_read(codec, WM9081_AUDIO_INTERFACE_1);
> @@ -1219,7 +1220,7 @@ static int wm9081_set_tdm_slot(struct snd_soc_dai *dai,
>
>        aif1 |= (slots - 1) << WM9081_AIFDAC_TDM_MODE_SHIFT;
>
> -       switch (mask) {
> +       switch (rx_mask) {
>        case 1:
>                break;
>        case 2:
> diff --git a/sound/soc/pxa/magician.c b/sound/soc/pxa/magician.c
> index c89a3cd..34437d8 100644
> --- a/sound/soc/pxa/magician.c
> +++ b/sound/soc/pxa/magician.c
> @@ -188,7 +188,7 @@ static int magician_playback_hw_params(struct snd_pcm_substream *substream,
>        if (ret < 0)
>                return ret;
>
> -       ret = snd_soc_dai_set_tdm_slot(cpu_dai, 1, 1);
> +       ret = snd_soc_dai_set_tdm_slot(cpu_dai, 1, 0, 1, width);
>        if (ret < 0)
>                return ret;
>
> diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c
> index e22c5ce..5b9ed64 100644
> --- a/sound/soc/pxa/pxa-ssp.c
> +++ b/sound/soc/pxa/pxa-ssp.c
> @@ -375,21 +375,34 @@ static int pxa_ssp_set_dai_pll(struct snd_soc_dai *cpu_dai,
>  * Set the active slots in TDM/Network mode
>  */
>  static int pxa_ssp_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai,
> -       unsigned int mask, int slots)
> +       unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
>  {
>        struct ssp_priv *priv = cpu_dai->private_data;
>        struct ssp_device *ssp = priv->dev.ssp;
>        u32 sscr0;
>
> -       sscr0 = ssp_read_reg(ssp, SSCR0) & ~SSCR0_SlotsPerFrm(7);
> +       sscr0 = ssp_read_reg(ssp, SSCR0);
> +       sscr0 &= ~(SSCR0_MOD | SSCR0_SlotsPerFrm(8) | SSCR0_EDSS | SSCR0_DSS);
> +
> +       /* set slot width */
> +       if (slot_width > 16)
> +               sscr0 |= SSCR0_EDSS | SSCR0_DataSize(slot_width - 16);
> +       else
> +               sscr0 |= SSCR0_DataSize(slot_width);
>
> -       /* set number of active slots */
> -       sscr0 |= SSCR0_SlotsPerFrm(slots);
> +       if (slots > 1) {
> +               /* enable network mode */
> +               sscr0 |= SSCR0_MOD;
> +
> +               /* set number of active slots */
> +               sscr0 |= SSCR0_SlotsPerFrm(slots);
> +
> +               /* set active slot mask */
> +               ssp_write_reg(ssp, SSTSA, tx_mask);
> +               ssp_write_reg(ssp, SSRSA, rx_mask);
> +       }
>        ssp_write_reg(ssp, SSCR0, sscr0);
>
> -       /* set active slot mask */
> -       ssp_write_reg(ssp, SSTSA, mask);
> -       ssp_write_reg(ssp, SSRSA, mask);
>        return 0;
>  }
>
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index e1a920c..c72a8e8 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -2133,17 +2133,20 @@ EXPORT_SYMBOL_GPL(snd_soc_dai_set_fmt);
>  /**
>  * snd_soc_dai_set_tdm_slot - configure DAI TDM.
>  * @dai: DAI
> - * @mask: DAI specific mask representing used slots.
> + * @tx_mask: bitmask representing active TX slots.
> + * @rx_mask: bitmask representing active RX slots.
>  * @slots: Number of slots in use.
> + * @slot_width: Width in bits for each slot.
>  *
>  * Configures a DAI for TDM operation. Both mask and slots are codec and DAI
>  * specific.
>  */
>  int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai,
> -       unsigned int mask, int slots)
> +       unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
>  {
>        if (dai->ops && dai->ops->set_tdm_slot)
> -               return dai->ops->set_tdm_slot(dai, mask, slots);
> +               return dai->ops->set_tdm_slot(dai, tx_mask, rx_mask,
> +                               slots, slot_width);
>        else
>                return -EINVAL;
>  }
> --
> tg: (c787980..) asoc/set_tdm_slot (depends on: asoc/remove-code-duplication)
>
> --
> Daniel Ribeiro
>

Re: [PATCH 1/3] ASoC: change set_tdm_slot api to allow slot_width override.

[Mark Brown]

On Mon, Jun 15, 2009 at 09:44:31PM -0300, Daniel Ribeiro wrote:
> Extend set_tdm_slot to allow the user to arbitrarily set the frame width
> and active TX/RX slots.

> Updates magician.c and wm9081.c for the new set_tdm_slot(). wm9081.c
> still doesn't handle the slot_width override.

> While being there, correct an incorrect use of SlotsPerFrm(7) use in
> bitmask on pxa-ssp.c (SSCR0_SlotsPerFrm(x) is (((x) - 1) << 24)) ).

> (this series is meant for Mark's for-2.6.32 branch)

> Signed-off-by: Daniel Ribeiro <drwyrm@gmail.com>

So.  These have been sitting for quite some time now.  Unfortunately I
went on holiday immediately after it was last submitted my Zylonite
system has been bricked for pretty much all of the time since then so
I've been remiss in following up.  Sorry about that, my bad.

My inclination here is to apply these changes - they look like a clear
win for pretty much every machine except Daniel Mack's out of tree
system which needs the 64 bit TDM slots.  It's a shame to break that but
there seems to be such a clear advantage from the changes, especially
the new set_tdm_slot() API, that it seems like a win overall.  I've got
a feeling that the fix for that system may have to be allowing the
machine driver to rewrite the hw_params for the CPU driver to lie about
the data format.

I'll try to do that this week.

Re: [PATCH 1/3] ASoC: change set_tdm_slot api to allow slot_width override.

[Daniel Mack]

On Tue, Aug 04, 2009 at 09:55:45PM +0100, Mark Brown wrote:
> On Mon, Jun 15, 2009 at 09:44:31PM -0300, Daniel Ribeiro wrote:
> > Extend set_tdm_slot to allow the user to arbitrarily set the frame width
> > and active TX/RX slots.
> 
> > Updates magician.c and wm9081.c for the new set_tdm_slot(). wm9081.c
> > still doesn't handle the slot_width override.
> 
> > While being there, correct an incorrect use of SlotsPerFrm(7) use in
> > bitmask on pxa-ssp.c (SSCR0_SlotsPerFrm(x) is (((x) - 1) << 24)) ).
> 
> > (this series is meant for Mark's for-2.6.32 branch)
> 
> > Signed-off-by: Daniel Ribeiro <drwyrm@gmail.com>
> 
> So.  These have been sitting for quite some time now.  Unfortunately I
> went on holiday immediately after it was last submitted my Zylonite
> system has been bricked for pretty much all of the time since then so
> I've been remiss in following up.  Sorry about that, my bad.
> 
> My inclination here is to apply these changes - they look like a clear
> win for pretty much every machine except Daniel Mack's out of tree
> system which needs the 64 bit TDM slots.  It's a shame to break that but
> there seems to be such a clear advantage from the changes, especially
> the new set_tdm_slot() API, that it seems like a win overall.  I've got
> a feeling that the fix for that system may have to be allowing the
> machine driver to rewrite the hw_params for the CPU driver to lie about
> the data format.

Hmm. I like to stress that I appreciate the efford taken to clean up all
the API around this, and I would like to give my 'tested-by' on the
patches.

However, if it is about cleaning up things properly, we should make sure
the system does what it's been told to do, and with Daniel Ribeiro's
patches, it unfortunately doesn't yet. I'm only blaming the utterly
broken PXA register set for that, but that should be fixed before the
whole series goes in. And I'm quite sure I'm not the only user of 64 bit
TDM slots, even though I might be the only one having discussed that.

Daniel, could you resend the latest version of your patches? I hope to
find some time to again test them, find the differences in the register
bits and amend them in a way that they actually do what is needed for my
system.

And btw - the code for the system I'm working on will be release
eventually, of course, and hopefully even be merged mainline. 

Thanks,
Daniel

Re: [PATCH 1/3] ASoC: change set_tdm_slot api to allow slot_width override.

[Eric Miao]

Mark Brown wrote:
> On Mon, Jun 15, 2009 at 09:44:31PM -0300, Daniel Ribeiro wrote:
>> Extend set_tdm_slot to allow the user to arbitrarily set the frame width
>> and active TX/RX slots.
> 
>> Updates magician.c and wm9081.c for the new set_tdm_slot(). wm9081.c
>> still doesn't handle the slot_width override.
> 
>> While being there, correct an incorrect use of SlotsPerFrm(7) use in
>> bitmask on pxa-ssp.c (SSCR0_SlotsPerFrm(x) is (((x) - 1) << 24)) ).
> 
>> (this series is meant for Mark's for-2.6.32 branch)
> 
>> Signed-off-by: Daniel Ribeiro <drwyrm@gmail.com>
> 
> So.  These have been sitting for quite some time now.  Unfortunately I
> went on holiday immediately after it was last submitted my Zylonite
> system has been bricked for pretty much all of the time since then so
> I've been remiss in following up.  Sorry about that, my bad.
> 
> My inclination here is to apply these changes - they look like a clear
> win for pretty much every machine except Daniel Mack's out of tree
> system which needs the 64 bit TDM slots.  It's a shame to break that but
> there seems to be such a clear advantage from the changes, especially
> the new set_tdm_slot() API, that it seems like a win overall.  I've got
> a feeling that the fix for that system may have to be allowing the
> machine driver to rewrite the hw_params for the CPU driver to lie about
> the data format.
> 
> I'll try to do that this week.
> 

I seem to remember this failed on Littleton - playback just doesn't work
any more, and DMA looks to be inactive. Didn't follow this thread for a
long time, but Daniel, could you repost the whole series if you have any
update

Re: [PATCH 1/3] ASoC: change set_tdm_slot api to allow slot_width override.

[Mark Brown]

On Thu, Aug 06, 2009 at 10:36:22PM +0800, Eric Miao wrote:

> I seem to remember this failed on Littleton - playback just doesn't work
> any more, and DMA looks to be inactive. Didn't follow this thread for a
> long time, but Daniel, could you repost the whole series if you have any
> update

I don't think anyone actually reported that on the list at the time.
Actually, speaking of Littleton it'd be good to get at least the CODEC
driver merged - ISTR the issues with that should all be fairly simple to
resolve and there's no need to wait for the machine support before
merging the CODEC driver.

I've pushed the patches to a branch on my ASoC git:

  git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git

and reposted them a minute ago.