dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio
@ 2019-02-25 16:42 Sven Van Asbroeck
  2019-02-25 16:42 ` [RFC PATCH 2/2] ASoC: core: standardize snd_soc_dai_set_bclk_ratio() behaviour Sven Van Asbroeck
  2019-02-26  0:35 ` [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio Kuninori Morimoto
  0 siblings, 2 replies; 13+ messages in thread
From: Sven Van Asbroeck @ 2019-02-25 16:42 UTC (permalink / raw)
  To: Russell King - ARM Linux admin, Mark Brown, Peter Ujfalusi, Jyri Sarha
  Cc: alsa-devel, Kuninori Morimoto, David Airlie, Takashi Iwai,
	Liam Girdwood, dri-devel, Daniel Vetter

In some situations, codec configuration will depend on the
bclk_ratio generated by the cpu dai. The tda998x hdmi transmitter
is an example of this.

Allow simple-card to set the bclk_ratio via the 'bclk-slot-ratio'
devicetree property, which describes the bclk to slot rate ratio.

This value is converted to the bclk to sample ratio before being
passed into set_bclk_ratio().

Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
 sound/soc/generic/simple-card.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 3fe34417ec89..61e9ba4e9b58 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -25,6 +25,7 @@ struct simple_card_data {
 		struct asoc_simple_card_data adata;
 		struct snd_soc_codec_conf *codec_conf;
 		unsigned int mclk_fs;
+		unsigned int bclk_slot_ratio;
 	} *dai_props;
 	struct asoc_simple_jack hp_jack;
 	struct asoc_simple_jack mic_jack;
@@ -97,7 +98,7 @@ static int asoc_simple_card_hw_params(struct snd_pcm_substream *substream,
 	struct simple_card_data *priv = snd_soc_card_get_drvdata(rtd->card);
 	struct simple_dai_props *dai_props =
 		simple_priv_to_props(priv, rtd->num);
-	unsigned int mclk, mclk_fs = 0;
+	unsigned int mclk, bclk_ratio, mclk_fs = 0, bclk_slot_ratio = 0;
 	int ret = 0;
 
 	if (dai_props->mclk_fs)
@@ -124,6 +125,23 @@ static int asoc_simple_card_hw_params(struct snd_pcm_substream *substream,
 		if (ret && ret != -ENOTSUPP)
 			goto err;
 	}
+
+	if (dai_props->bclk_slot_ratio)
+		bclk_slot_ratio = dai_props->bclk_slot_ratio;
+
+	if (bclk_slot_ratio) {
+		/* FIXME do we need to care about TDM slots here ? */
+		bclk_ratio = snd_soc_calc_frame_size(bclk_slot_ratio,
+				params_channels(params), 1);
+
+		ret = snd_soc_dai_set_bclk_ratio(codec_dai, bclk_ratio);
+		if (ret && ret != -ENOTSUPP)
+			goto err;
+
+		ret = snd_soc_dai_set_bclk_ratio(cpu_dai, bclk_ratio);
+		if (ret && ret != -ENOTSUPP)
+			goto err;
+	}
 	return 0;
 err:
 	return ret;
@@ -277,6 +295,12 @@ static int asoc_simple_card_dai_link_of_dpcm(struct device_node *top,
 	of_property_read_u32(node, prop, &dai_props->mclk_fs);
 	of_property_read_u32(np,   prop, &dai_props->mclk_fs);
 
+	snprintf(prop, sizeof(prop), "%sbclk-slot-ratio", prefix);
+	of_property_read_u32(top,  PREFIX "bclk-slot-ratio",
+				&dai_props->bclk_slot_ratio);
+	of_property_read_u32(node, prop, &dai_props->bclk_slot_ratio);
+	of_property_read_u32(np,   prop, &dai_props->bclk_slot_ratio);
+
 	ret = asoc_simple_card_parse_daifmt(dev, node, codec,
 					    prefix, &dai_link->dai_fmt);
 	if (ret < 0)
@@ -349,6 +373,13 @@ static int asoc_simple_card_dai_link_of(struct device_node *top,
 	of_property_read_u32(cpu,   prop, &dai_props->mclk_fs);
 	of_property_read_u32(codec, prop, &dai_props->mclk_fs);
 
+	snprintf(prop, sizeof(prop), "%sbclk-slot-ratio", prefix);
+	of_property_read_u32(top,  PREFIX "bclk-slot-ratio",
+					&dai_props->bclk_slot_ratio);
+	of_property_read_u32(node,  prop, &dai_props->bclk_slot_ratio);
+	of_property_read_u32(cpu,   prop, &dai_props->bclk_slot_ratio);
+	of_property_read_u32(codec, prop, &dai_props->bclk_slot_ratio);
+
 	ret = asoc_simple_card_parse_cpu(cpu, dai_link,
 					 DAI, CELL, &single_cpu);
 	if (ret < 0)
-- 
2.17.1

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

* [RFC PATCH 2/2] ASoC: core: standardize snd_soc_dai_set_bclk_ratio() behaviour
  2019-02-25 16:42 [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio Sven Van Asbroeck
@ 2019-02-25 16:42 ` Sven Van Asbroeck
  2019-02-26  0:35 ` [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio Kuninori Morimoto
  1 sibling, 0 replies; 13+ messages in thread
From: Sven Van Asbroeck @ 2019-02-25 16:42 UTC (permalink / raw)
  To: Russell King - ARM Linux admin, Mark Brown, Peter Ujfalusi, Jyri Sarha
  Cc: alsa-devel, Kuninori Morimoto, David Airlie, Takashi Iwai,
	Liam Girdwood, dri-devel, Daniel Vetter

snd_soc_dai_set_bclk_ratio() should behave like other
snd_soc_dai_XXX functions, and return -ENOTSUPP if the
callback in driver->ops is NULL.

Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
 sound/soc/soc-core.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 50617db05c46..5611caf25ea3 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2550,10 +2550,11 @@ EXPORT_SYMBOL_GPL(snd_soc_component_set_pll);
  */
 int snd_soc_dai_set_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio)
 {
-	if (dai->driver->ops->set_bclk_ratio)
-		return dai->driver->ops->set_bclk_ratio(dai, ratio);
-	else
+	if (dai->driver == NULL)
 		return -EINVAL;
+	if (dai->driver->ops->set_bclk_ratio == NULL)
+		return -ENOTSUPP;
+	return dai->driver->ops->set_bclk_ratio(dai, ratio);
 }
 EXPORT_SYMBOL_GPL(snd_soc_dai_set_bclk_ratio);
 
-- 
2.17.1

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

* Re: [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio
  2019-02-25 16:42 [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio Sven Van Asbroeck
  2019-02-25 16:42 ` [RFC PATCH 2/2] ASoC: core: standardize snd_soc_dai_set_bclk_ratio() behaviour Sven Van Asbroeck
@ 2019-02-26  0:35 ` Kuninori Morimoto
  2019-02-26  9:11   ` Russell King - ARM Linux admin
  1 sibling, 1 reply; 13+ messages in thread
From: Kuninori Morimoto @ 2019-02-26  0:35 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: alsa-devel, Liam Girdwood, David Airlie, Takashi Iwai,
	Russell King - ARM Linux admin, Jyri Sarha, Jaroslav Kysela,
	Peter Ujfalusi, Mark Brown, dri-devel


Hi Sven

Thank you for your patch

> In some situations, codec configuration will depend on the
> bclk_ratio generated by the cpu dai. The tda998x hdmi transmitter
> is an example of this.
> 
> Allow simple-card to set the bclk_ratio via the 'bclk-slot-ratio'
> devicetree property, which describes the bclk to slot rate ratio.
> 
> This value is converted to the bclk to sample ratio before being
> passed into set_bclk_ratio().
> 
> Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
> ---

Please update Documentation, too

> @@ -97,7 +98,7 @@ static int asoc_simple_card_hw_params(struct snd_pcm_substream *substream,
>  	struct simple_card_data *priv = snd_soc_card_get_drvdata(rtd->card);
>  	struct simple_dai_props *dai_props =
>  		simple_priv_to_props(priv, rtd->num);
> -	unsigned int mclk, mclk_fs = 0;
> +	unsigned int mclk, bclk_ratio, mclk_fs = 0, bclk_slot_ratio = 0;
>  	int ret = 0;
>  
>  	if (dai_props->mclk_fs)
(snip)
> +	if (bclk_slot_ratio) {
> +		/* FIXME do we need to care about TDM slots here ? */
> +		bclk_ratio = snd_soc_calc_frame_size(bclk_slot_ratio,
> +				params_channels(params), 1);
> +
> +		ret = snd_soc_dai_set_bclk_ratio(codec_dai, bclk_ratio);
> +		if (ret && ret != -ENOTSUPP)
> +			goto err;
> +
> +		ret = snd_soc_dai_set_bclk_ratio(cpu_dai, bclk_ratio);
> +		if (ret && ret != -ENOTSUPP)
> +			goto err;
> +	}

Not a big deal, but "bclk_ratio" is used only here.
We can define it here

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio
  2019-02-26  0:35 ` [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio Kuninori Morimoto
@ 2019-02-26  9:11   ` Russell King - ARM Linux admin
  2019-02-26 14:53     ` Sven Van Asbroeck
  2019-02-26 18:46     ` Sven Van Asbroeck
  0 siblings, 2 replies; 13+ messages in thread
From: Russell King - ARM Linux admin @ 2019-02-26  9:11 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Sven Van Asbroeck, alsa-devel, David Airlie, Takashi Iwai,
	Liam Girdwood, Jyri Sarha, Jaroslav Kysela, Peter Ujfalusi,
	Mark Brown, dri-devel

On Tue, Feb 26, 2019 at 09:35:47AM +0900, Kuninori Morimoto wrote:
> > @@ -97,7 +98,7 @@ static int asoc_simple_card_hw_params(struct snd_pcm_substream *substream,
> >  	struct simple_card_data *priv = snd_soc_card_get_drvdata(rtd->card);
> >  	struct simple_dai_props *dai_props =
> >  		simple_priv_to_props(priv, rtd->num);
> > -	unsigned int mclk, mclk_fs = 0;
> > +	unsigned int mclk, bclk_ratio, mclk_fs = 0, bclk_slot_ratio = 0;
> >  	int ret = 0;
> >  
> >  	if (dai_props->mclk_fs)
> (snip)
> > +	if (bclk_slot_ratio) {
> > +		/* FIXME do we need to care about TDM slots here ? */
> > +		bclk_ratio = snd_soc_calc_frame_size(bclk_slot_ratio,
> > +				params_channels(params), 1);
> > +
> > +		ret = snd_soc_dai_set_bclk_ratio(codec_dai, bclk_ratio);
> > +		if (ret && ret != -ENOTSUPP)
> > +			goto err;
> > +
> > +		ret = snd_soc_dai_set_bclk_ratio(cpu_dai, bclk_ratio);
> > +		if (ret && ret != -ENOTSUPP)
> > +			goto err;
> > +	}
> 
> Not a big deal, but "bclk_ratio" is used only here.
> We can define it here

This code actually requires a lot more thought - while it may solve
Sven's issue, it isn't generic.  So far, I have this table put
together from various sources of information:

			bclk_ratio
sample width	current	mcasp   fslssi  kirkwood
16		32	32	64	64
24		48	48	64	64
32		64	64	64	64

Let's also consider whether it should depend on the number of channels.
I2S uses a WS/LRCLK signal to differentiate each channel and to demark
where the MSB bit is.

If userspace negotiates one channel, what happens - if WS becomes
static, then there is no signal indicating where the MSB bit is in
the stream, so there is no way for a receiver to synchronise.  So,
it is highly unlikely that bclk_ratio = channels * sample_width.

If the signal continues toggling, what does it do for the inactive
channel - is that just one BCLK period long or does it assume there
is still the second channel?  If the former, it means we could end
up with bclk_ratios of 17, 25, 33 which imho is unlikely, and
would mess up TDA998x's CTS measurement.

What about setups where we have multiple I2S data signals (to support
multi-channel audio) - if we have four channels transmitted on two
data lines (as would be required by the TDA998x), that doesn't mean
BCLK becomes sample_width * 4.

So, I don't think the number of channels comes into the bclk_ratio
calculation at all.  Given the above code, it effectively means we'd
always specify channels = 2 to snd_soc_calc_frame_size().

Given that it is normal to talk about I2S being clocked at "64fs",
"32fs" etc, wouldn't it just be much neater to specify _this_ value
in DT, rather than half that value?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio
  2019-02-26  9:11   ` Russell King - ARM Linux admin
@ 2019-02-26 14:53     ` Sven Van Asbroeck
  2019-02-26 15:23       ` Mark Brown
  2019-02-26 15:45       ` Russell King - ARM Linux admin
  2019-02-26 18:46     ` Sven Van Asbroeck
  1 sibling, 2 replies; 13+ messages in thread
From: Sven Van Asbroeck @ 2019-02-26 14:53 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: alsa-devel, Kuninori Morimoto, David Airlie, Takashi Iwai,
	Liam Girdwood, Jyri Sarha, Peter Ujfalusi, Mark Brown, dri-devel,
	Daniel Vetter

On Tue, Feb 26, 2019 at 4:11 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> This code actually requires a lot more thought - while it may solve
> Sven's issue, it isn't generic.

Wholeheartedly agree. My patches are marked RFC in the hope that they will be
picked apart by folks much more in touch with the "big audio picture" than
I am. It's great to learn from this process.

> So far, I have this table put
> together from various sources of information:
>
>                         bclk_ratio
> sample width    current mcasp   fslssi  kirkwood
> 16              32      32      64      64
> 24              48      48      64      64
> 32              64      64      64      64
>
> Let's also consider whether it should depend on the number of channels.
> I2S uses a WS/LRCLK signal to differentiate each channel and to demark
> where the MSB bit is.
>
> If userspace negotiates one channel, what happens - if WS becomes
> static, then there is no signal indicating where the MSB bit is in
> the stream, so there is no way for a receiver to synchronise.  So,
> it is highly unlikely that bclk_ratio = channels * sample_width.
>
> If the signal continues toggling, what does it do for the inactive
> channel - is that just one BCLK period long or does it assume there
> is still the second channel?  If the former, it means we could end
> up with bclk_ratios of 17, 25, 33 which imho is unlikely, and
> would mess up TDA998x's CTS measurement.

Good point. i2s with a single channel makes no sense.

>
> What about setups where we have multiple I2S data signals (to support
> multi-channel audio) - if we have four channels transmitted on two
> data lines (as would be required by the TDA998x), that doesn't mean
> BCLK becomes sample_width * 4.
>
> So, I don't think the number of channels comes into the bclk_ratio
> calculation at all.  Given the above code, it effectively means we'd
> always specify channels = 2 to snd_soc_calc_frame_size().

I notice that hdmi-codec.c supports up to 8 channels in hdmi multi-channel
playback mode. If we had a _theoretical_ hdmi xmitter with 8chan support,
would the bclk_ratio not be 8 x slot_size - or 8 x 32 if using an fsl_ssi
in master mode?

This will of course never happen with the tda998x, because
        .max_i2s_channels = 2,
but we are thinking about a generic solution here.

>
> Given that it is normal to talk about I2S being clocked at "64fs",
> "32fs" etc, wouldn't it just be much neater to specify _this_ value
> in DT, rather than half that value?
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio
  2019-02-26 14:53     ` Sven Van Asbroeck
@ 2019-02-26 15:23       ` Mark Brown
  2019-02-26 15:45       ` Russell King - ARM Linux admin
  1 sibling, 0 replies; 13+ messages in thread
From: Mark Brown @ 2019-02-26 15:23 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: alsa-devel, Kuninori Morimoto, Liam Girdwood, David Airlie,
	Takashi Iwai, Russell King - ARM Linux admin, Jyri Sarha,
	Peter Ujfalusi, dri-devel, Daniel Vetter


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

On Tue, Feb 26, 2019 at 09:53:22AM -0500, Sven Van Asbroeck wrote:
> On Tue, Feb 26, 2019 at 4:11 AM Russell King - ARM Linux admin

> > If the signal continues toggling, what does it do for the inactive
> > channel - is that just one BCLK period long or does it assume there
> > is still the second channel?  If the former, it means we could end
> > up with bclk_ratios of 17, 25, 33 which imho is unlikely, and
> > would mess up TDA998x's CTS measurement.

> Good point. i2s with a single channel makes no sense.

As with every bad idea you can think of that never stopped anyone making
such hardware.  Mono is usually implemented by either leaving the right
channel idle or playing the same signal on both channels.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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



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

* Re: [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio
  2019-02-26 14:53     ` Sven Van Asbroeck
  2019-02-26 15:23       ` Mark Brown
@ 2019-02-26 15:45       ` Russell King - ARM Linux admin
  2019-02-26 16:21         ` Mark Brown
  2019-02-26 16:31         ` Sven Van Asbroeck
  1 sibling, 2 replies; 13+ messages in thread
From: Russell King - ARM Linux admin @ 2019-02-26 15:45 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: alsa-devel, Kuninori Morimoto, David Airlie, Takashi Iwai,
	Liam Girdwood, Jyri Sarha, Jaroslav Kysela, Peter Ujfalusi,
	Mark Brown, dri-devel

On Tue, Feb 26, 2019 at 09:53:22AM -0500, Sven Van Asbroeck wrote:
> I notice that hdmi-codec.c supports up to 8 channels in hdmi multi-channel
> playback mode. If we had a _theoretical_ hdmi xmitter with 8chan support,
> would the bclk_ratio not be 8 x slot_size - or 8 x 32 if using an fsl_ssi
> in master mode?
> 
> This will of course never happen with the tda998x, because
>         .max_i2s_channels = 2,
> but we are thinking about a generic solution here.

The way TDA998x supports multichannel audio with I2S is as follows:

"The TDA9983B supports the NXP I2S-bus format. There are four I2S-bus
stereo input channels (AP1 to AP4), which enable 8 uncompressed audio
channels to be carried."

There is only one WS input and one SCK (bclk) input, which are common
to each of the I2S buses.

The TDA19988 reduces this down to two I2S buses, which means it
supports only up to 4 uncompressed channels.  hdmi-codec doesn't
take account of these restrictions, and just assumes the maximal
number of channels are always available.

So, given this parallel bus architecture, it means that whether we
have 2, 4, 6, or 8 channels is irrelevant to the number of bitclocks
per sample - the number of bitclocks would be the same.

I can't see how you'd extend a single I2S setup to support multi-
channel audio without either adding more I2S data lines or adding
additional WS signals (so making it e.g., a binary number).

Adding more WS signals makes the bus deviate from the I2S standard,
thereby making it impossible to connect a set of standard DACs to
such a source, whereas adding more I2S data lines, you just connect
each DAC to each I2S data line and common up the bit clock and WS
signals across all.

In other words, the TDA998x approach is really the only sane way
forward.

Now, as far as transmitter support, I believe TI Davinci SoCs use
this - my Onkyo TX-NR609 AV receiver uses a DA830 SoC as a DSP to
do the surround decode, which feeds multi-channel audio out to a
set of DACs over a parallel I2S bus.  The "mcasp" audio driver
has multiple serialisers to cope with this - see
Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio
  2019-02-26 15:45       ` Russell King - ARM Linux admin
@ 2019-02-26 16:21         ` Mark Brown
  2019-02-26 16:31         ` Sven Van Asbroeck
  1 sibling, 0 replies; 13+ messages in thread
From: Mark Brown @ 2019-02-26 16:21 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Sven Van Asbroeck, alsa-devel, Kuninori Morimoto, David Airlie,
	Takashi Iwai, Liam Girdwood, Jyri Sarha, Jaroslav Kysela,
	Peter Ujfalusi, dri-devel


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

On Tue, Feb 26, 2019 at 03:45:19PM +0000, Russell King - ARM Linux admin wrote:

> Adding more WS signals makes the bus deviate from the I2S standard,
> thereby making it impossible to connect a set of standard DACs to
> such a source, whereas adding more I2S data lines, you just connect
> each DAC to each I2S data line and common up the bit clock and WS
> signals across all.

> In other words, the TDA998x approach is really the only sane way
> forward.

Right.  You do also see some things doing this by stuffing all the left
samples together in the left clock cycle and all the right samples
together in the right clock cycle but that's usually more programmable
hardware that also supports DSP modes and it's just falling out of the
implementation with little effort rather than someone sitting down and
thinking this is a good idea AFAICT.

> Now, as far as transmitter support, I believe TI Davinci SoCs use
> this - my Onkyo TX-NR609 AV receiver uses a DA830 SoC as a DSP to
> do the surround decode, which feeds multi-channel audio out to a
> set of DACs over a parallel I2S bus.  The "mcasp" audio driver
> has multiple serialisers to cope with this - see
> Documentation/devicetree/bindings/sound/davinci-mcasp-audio.txt.

Samsung LSI have multi-channel I2S with multiple data lines in their v5
and later controllers as well.  I can't think of any other examples
upstream off the top of my head.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio
  2019-02-26 15:45       ` Russell King - ARM Linux admin
  2019-02-26 16:21         ` Mark Brown
@ 2019-02-26 16:31         ` Sven Van Asbroeck
  2019-02-26 16:41           ` Mark Brown
  2019-02-26 17:03           ` Russell King - ARM Linux admin
  1 sibling, 2 replies; 13+ messages in thread
From: Sven Van Asbroeck @ 2019-02-26 16:31 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: alsa-devel, Kuninori Morimoto, David Airlie, Takashi Iwai,
	Liam Girdwood, Jyri Sarha, Peter Ujfalusi, Mark Brown, dri-devel,
	Daniel Vetter

On Tue, Feb 26, 2019 at 10:45 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
>
> I can't see how you'd extend a single I2S setup to support multi-
> channel audio without either adding more I2S data lines or adding
> additional WS signals (so making it e.g., a binary number).
>

That's a very good point too. In light of this, I struggle to understand how
the ssl_ssi can specify this:

static struct snd_soc_dai_driver fsl_ssi_dai_template = {
        .playback = {
                .stream_name = "CPU-Playback",
                .channels_min = 1,
                .channels_max = 32,
        },

There is talk in the manual about "network mode", which could work by changing
the LRCLK only at the first slot - thereby allowing clients to receive all
slots just by counting, as long as they know the slot size?

LRCLK   _____/-----------------\_____/---------
DATA    SLOT1|SLOT2|SLOT3|SLOT4|SLOT1

Well idk, I'm way out of my depth here.

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

* Re: [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio
  2019-02-26 16:31         ` Sven Van Asbroeck
@ 2019-02-26 16:41           ` Mark Brown
  2019-02-26 17:03           ` Russell King - ARM Linux admin
  1 sibling, 0 replies; 13+ messages in thread
From: Mark Brown @ 2019-02-26 16:41 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: alsa-devel, Kuninori Morimoto, Liam Girdwood, David Airlie,
	Takashi Iwai, Russell King - ARM Linux admin, Jyri Sarha,
	Jaroslav Kysela, Peter Ujfalusi, dri-devel


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

On Tue, Feb 26, 2019 at 11:31:15AM -0500, Sven Van Asbroeck wrote:

> That's a very good point too. In light of this, I struggle to understand how
> the ssl_ssi can specify this:

> static struct snd_soc_dai_driver fsl_ssi_dai_template = {
>         .playback = {
>                 .stream_name = "CPU-Playback",
>                 .channels_min = 1,
>                 .channels_max = 32,
>         },

> There is talk in the manual about "network mode", which could work by changing
> the LRCLK only at the first slot - thereby allowing clients to receive all
> slots just by counting, as long as they know the slot size?

That's basically what the FSL controllers want to support - they're
generic programmable serial ports, their most native formats are the DSP
modes which only pay attention to one edge of LRCLK and then just have
however many samples one after another.  Several other devices like the
PXA SSP ports are similar.  They can generate I2S style LRCLKs but many
implementations struggle to interpret incoming I2S properly, they often
have issues with triggering on both rising and falling edges so get
confused if there's any unused clock cycles.

For a DSP mode it's more:

LRCLK   /\______________________/\____
DATA    |SLOT1|SLOT2|SLOT3|SLOT4|SLOT1

I2S style it'd end up as more like:

LRCLK   /-------------\_____________/------
DATA    |SLOTL1|SLOTL2|SLOTR1|SLOTR2|SLOTL1

This sort of stuff is why ASoC has a preference for exact clocking - it
improves interoperability with hardware that's generic serial ports as
it means that you only need to pay attention to one edge of the LRCLK
even if it's doing something more than just a simple pulse.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio
  2019-02-26 16:31         ` Sven Van Asbroeck
  2019-02-26 16:41           ` Mark Brown
@ 2019-02-26 17:03           ` Russell King - ARM Linux admin
  2019-02-27 18:36             ` Mark Brown
  1 sibling, 1 reply; 13+ messages in thread
From: Russell King - ARM Linux admin @ 2019-02-26 17:03 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: alsa-devel, Kuninori Morimoto, David Airlie, Takashi Iwai,
	Liam Girdwood, Jyri Sarha, Jaroslav Kysela, Peter Ujfalusi,
	Mark Brown, dri-devel

On Tue, Feb 26, 2019 at 11:31:15AM -0500, Sven Van Asbroeck wrote:
> On Tue, Feb 26, 2019 at 10:45 AM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> >
> > I can't see how you'd extend a single I2S setup to support multi-
> > channel audio without either adding more I2S data lines or adding
> > additional WS signals (so making it e.g., a binary number).
> >
> 
> That's a very good point too. In light of this, I struggle to understand how
> the ssl_ssi can specify this:
> 
> static struct snd_soc_dai_driver fsl_ssi_dai_template = {
>         .playback = {
>                 .stream_name = "CPU-Playback",
>                 .channels_min = 1,
>                 .channels_max = 32,
>         },
> 
> There is talk in the manual about "network mode", which could work by changing
> the LRCLK only at the first slot - thereby allowing clients to receive all
> slots just by counting, as long as they know the slot size?
> 
> LRCLK   _____/-----------------\_____/---------
> DATA    SLOT1|SLOT2|SLOT3|SLOT4|SLOT1

From what I gather, these are described using SND_SOC_DAIFMT_DSP_A
and SND_SOC_DAIFMT_DSP_B dai formats, and the parameters are
controlled not through snd_soc_dai_set_bclk_ratio() but via
snd_soc_dai_set_tdm_slot().

So, IMHO, the TDM formats should be disregarded from consideration
here.  Mark, ack?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio
  2019-02-26  9:11   ` Russell King - ARM Linux admin
  2019-02-26 14:53     ` Sven Van Asbroeck
@ 2019-02-26 18:46     ` Sven Van Asbroeck
  1 sibling, 0 replies; 13+ messages in thread
From: Sven Van Asbroeck @ 2019-02-26 18:46 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: alsa-devel, Kuninori Morimoto, David Airlie, Takashi Iwai,
	Liam Girdwood, Jyri Sarha, Peter Ujfalusi, Mark Brown, dri-devel,
	Daniel Vetter

On Tue, Feb 26, 2019 at 4:11 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> Given that it is normal to talk about I2S being clocked at "64fs",
> "32fs" etc, wouldn't it just be much neater to specify _this_ value
> in DT, rather than half that value?
>

Would we be able to reach consensus on Russell's suggestion ?

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

* Re: [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio
  2019-02-26 17:03           ` Russell King - ARM Linux admin
@ 2019-02-27 18:36             ` Mark Brown
  0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2019-02-27 18:36 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Sven Van Asbroeck, alsa-devel, Kuninori Morimoto, David Airlie,
	Takashi Iwai, Liam Girdwood, Jyri Sarha, Jaroslav Kysela,
	Peter Ujfalusi, dri-devel


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

On Tue, Feb 26, 2019 at 05:03:49PM +0000, Russell King - ARM Linux admin wrote:
> On Tue, Feb 26, 2019 at 11:31:15AM -0500, Sven Van Asbroeck wrote:

> > There is talk in the manual about "network mode", which could work by changing
> > the LRCLK only at the first slot - thereby allowing clients to receive all
> > slots just by counting, as long as they know the slot size?

> > LRCLK   _____/-----------------\_____/---------
> > DATA    SLOT1|SLOT2|SLOT3|SLOT4|SLOT1

> From what I gather, these are described using SND_SOC_DAIFMT_DSP_A
> and SND_SOC_DAIFMT_DSP_B dai formats, and the parameters are

Roughly, yes - the DSP formats are formats which only care about one
edge of the LRCLK, the difference being if the first data bit is the
same clock cycle as the transition or the next clock cycle.

> controlled not through snd_soc_dai_set_bclk_ratio() but via
> snd_soc_dai_set_tdm_slot().

> So, IMHO, the TDM formats should be disregarded from consideration
> here.  Mark, ack?

I'd expect set_bclk_ratio() to cover all formats - usually the hardware
either has zero flexibility when it comes to the LRCLK format or isn't
super bothered, this stuff is mainly used to adapt the more flexible
hardware to the less flexible hardware.

TDM is definitely something that only really makes sense with DSP
formats though, I2S TDM is just weird.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2019-02-27 18:36 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-25 16:42 [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio Sven Van Asbroeck
2019-02-25 16:42 ` [RFC PATCH 2/2] ASoC: core: standardize snd_soc_dai_set_bclk_ratio() behaviour Sven Van Asbroeck
2019-02-26  0:35 ` [RFC PATCH 1/2] ASoC: simple-card: add support for bclk_ratio Kuninori Morimoto
2019-02-26  9:11   ` Russell King - ARM Linux admin
2019-02-26 14:53     ` Sven Van Asbroeck
2019-02-26 15:23       ` Mark Brown
2019-02-26 15:45       ` Russell King - ARM Linux admin
2019-02-26 16:21         ` Mark Brown
2019-02-26 16:31         ` Sven Van Asbroeck
2019-02-26 16:41           ` Mark Brown
2019-02-26 17:03           ` Russell King - ARM Linux admin
2019-02-27 18:36             ` Mark Brown
2019-02-26 18:46     ` Sven Van Asbroeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).