All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] ASoC: Handle multiple codecs with split playback / capture
@ 2015-08-19 15:32 Ricard Wanderlof
  2015-08-20 15:45 ` Pierre-Louis Bossart
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Ricard Wanderlof @ 2015-08-19 15:32 UTC (permalink / raw)
  To: alsa-devel@alsa-project.org, Mark Brown, Liam Girdwood


Add the capability to use multiple codecs on the same DAI linke where
one codec is used for playback and another one is used for capture.

Tested on a setup using an SSM2518 for playback and an ICS43432 I2S MEMS
microphone for capture.

Signed-off-by: Ricard Wanderlof <ricardw@axis.com>
---

The patch was created after a discussion on the alsa-devel mailing list
as to how best to implement this functionality.
(http://mailman.alsa-project.org/pipermail/alsa-devel/2015-June/093214.html).

V2: Minor code change, otherwise the differences compared to V1 are solely
related to comments, in particular considerations given to the 
potential consequences of the patch. After consideration, it seems to me 
that the patch as it stands augments the current framework with the 
functionality indicated in the commit message above, without restricting 
other current uses. Further functionality could be added, but IMHO that 
should be done as the need arises, when it can be properly tested in a 
real-world setup.

 sound/soc/soc-pcm.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 35fe58f4..08407ba 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -34,6 +34,24 @@
 
 #define DPCM_MAX_BE_USERS	8
 
+/*
+ * snd_soc_dai_stream_valid() - check if a DAI supports the given stream
+ *
+ * Returns true if the DAI supports the indicated stream type.
+ */
+static bool snd_soc_dai_stream_valid(struct snd_soc_dai *dai, int stream)
+{
+	struct snd_soc_pcm_stream *codec_stream;
+
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+		codec_stream = &dai->driver->playback;
+	else
+		codec_stream = &dai->driver->capture;
+
+	/* If the codec specifies any rate at all, it supports the stream. */
+	return codec_stream->rates;
+}
+
 /**
  * snd_soc_runtime_activate() - Increment active count for PCM runtime components
  * @rtd: ASoC PCM runtime that is activated
@@ -371,6 +389,20 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 
 	/* first calculate min/max only for CODECs in the DAI link */
 	for (i = 0; i < rtd->num_codecs; i++) {
+
+		/*
+		 * Skip CODECs which don't support the current stream type.
+		 * Otherwise, since the rate, channel, and format values will
+		 * zero in that case, we would have no usable settings left,
+		 * causing the resulting setup to fail.
+		 * At least one CODEC should match, otherwise we should have
+		 * bailed out on a higher level, since there would be no
+		 * CODEC to support the transfer direction in that case.
+		 */
+		if (!snd_soc_dai_stream_valid(rtd->codec_dais[i],
+					      substream->stream))
+			continue;
+
 		codec_dai_drv = rtd->codec_dais[i]->driver;
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 			codec_stream = &codec_dai_drv->playback;
@@ -827,6 +859,23 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *codec_dai = rtd->codec_dais[i];
 		struct snd_pcm_hw_params codec_params;
 
+		/*
+		 * Skip CODECs which don't support the current stream type,
+		 * the idea being that if a CODEC is not used for the currently
+		 * set up transfer direction, it should not need to be
+		 * configured, especially since the configuration used might
+		 * not even be supported by that CODEC. There may be cases
+		 * however where a CODEC needs to be set up although it is
+		 * actually not being used for the transfer, e.g. if a
+		 * capture-only CODEC is acting as an LRCLK and/or BCLK master
+		 * for the DAI link including a playback-only CODEC.
+		 * If this becomes necessary, we will have to augment the
+		 * machine driver setup with information on how to act, so
+		 * we can do the right thing here.
+		 */
+		if (!snd_soc_dai_stream_valid(codec_dai, substream->stream))
+			continue;
+
 		/* copy params for each codec */
 		codec_params = *params;
 
-- 
1.7.10.4


-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

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

* Re: [PATCH v2] ASoC: Handle multiple codecs with split playback / capture
  2015-08-19 15:32 [PATCH v2] ASoC: Handle multiple codecs with split playback / capture Ricard Wanderlof
@ 2015-08-20 15:45 ` Pierre-Louis Bossart
  2015-08-20 17:10   ` Mark Brown
  2015-08-21  7:11   ` Ricard Wanderlof
  2015-09-03 15:05 ` Ricard Wanderlof
  2015-09-20  0:13 ` Applied "ASoC: Handle multiple codecs with split playback / capture" to the asoc tree Mark Brown
  2 siblings, 2 replies; 10+ messages in thread
From: Pierre-Louis Bossart @ 2015-08-20 15:45 UTC (permalink / raw)
  To: Ricard Wanderlof, alsa-devel@alsa-project.org, Mark Brown,
	Liam Girdwood

On 8/19/15 10:32 AM, Ricard Wanderlof wrote:
>
> Add the capability to use multiple codecs on the same DAI linke where
> one codec is used for playback and another one is used for capture.
>
> Tested on a setup using an SSM2518 for playback and an ICS43432 I2S MEMS
> microphone for capture.
>
> Signed-off-by: Ricard Wanderlof <ricardw@axis.com>
> ---
>
> The patch was created after a discussion on the alsa-devel mailing list
> as to how best to implement this functionality.
> (http://mailman.alsa-project.org/pipermail/alsa-devel/2015-June/093214.html).
>
> V2: Minor code change, otherwise the differences compared to V1 are solely
> related to comments, in particular considerations given to the
> potential consequences of the patch. After consideration, it seems to me
> that the patch as it stands augments the current framework with the
> functionality indicated in the commit message above, without restricting
> other current uses. Further functionality could be added, but IMHO that
> should be done as the need arises, when it can be properly tested in a
> real-world setup.
>
>   sound/soc/soc-pcm.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 49 insertions(+)
>
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 35fe58f4..08407ba 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -34,6 +34,24 @@
>
>   #define DPCM_MAX_BE_USERS	8
>
> +/*
> + * snd_soc_dai_stream_valid() - check if a DAI supports the given stream
> + *
> + * Returns true if the DAI supports the indicated stream type.
> + */
> +static bool snd_soc_dai_stream_valid(struct snd_soc_dai *dai, int stream)
> +{
> +	struct snd_soc_pcm_stream *codec_stream;
> +
> +	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
> +		codec_stream = &dai->driver->playback;
> +	else
> +		codec_stream = &dai->driver->capture;
> +
> +	/* If the codec specifies any rate at all, it supports the stream. */
> +	return codec_stream->rates;
> +}
> +
>   /**
>    * snd_soc_runtime_activate() - Increment active count for PCM runtime components
>    * @rtd: ASoC PCM runtime that is activated
> @@ -371,6 +389,20 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
>
>   	/* first calculate min/max only for CODECs in the DAI link */
>   	for (i = 0; i < rtd->num_codecs; i++) {
> +
> +		/*
> +		 * Skip CODECs which don't support the current stream type.
> +		 * Otherwise, since the rate, channel, and format values will
> +		 * zero in that case, we would have no usable settings left,
> +		 * causing the resulting setup to fail.
> +		 * At least one CODEC should match, otherwise we should have
> +		 * bailed out on a higher level, since there would be no
> +		 * CODEC to support the transfer direction in that case.
> +		 */
> +		if (!snd_soc_dai_stream_valid(rtd->codec_dais[i],
> +					      substream->stream))

Maybe I misunderstood but shouldn't there be some sort of verification 
that the codecs can use the same number of slots between playback and 
capture if they share the same LRCLK/FS? e.g. it's not uncommon to have 
4 mic capture and 2 ch playback. If the capture and playback is handled 
by different chips you'd still need to maintain some level of consistency.

> +			continue;
> +
>   		codec_dai_drv = rtd->codec_dais[i]->driver;
>   		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>   			codec_stream = &codec_dai_drv->playback;
> @@ -827,6 +859,23 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
>   		struct snd_soc_dai *codec_dai = rtd->codec_dais[i];
>   		struct snd_pcm_hw_params codec_params;
>
> +		/*
> +		 * Skip CODECs which don't support the current stream type,
> +		 * the idea being that if a CODEC is not used for the currently
> +		 * set up transfer direction, it should not need to be
> +		 * configured, especially since the configuration used might
> +		 * not even be supported by that CODEC. There may be cases
> +		 * however where a CODEC needs to be set up although it is
> +		 * actually not being used for the transfer, e.g. if a
> +		 * capture-only CODEC is acting as an LRCLK and/or BCLK master
> +		 * for the DAI link including a playback-only CODEC.
> +		 * If this becomes necessary, we will have to augment the
> +		 * machine driver setup with information on how to act, so
> +		 * we can do the right thing here.
> +		 */
> +		if (!snd_soc_dai_stream_valid(codec_dai, substream->stream))
> +			continue;
> +
>   		/* copy params for each codec */
>   		codec_params = *params;
>
>

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

* Re: [PATCH v2] ASoC: Handle multiple codecs with split playback / capture
  2015-08-20 15:45 ` Pierre-Louis Bossart
@ 2015-08-20 17:10   ` Mark Brown
  2015-08-21  7:11   ` Ricard Wanderlof
  1 sibling, 0 replies; 10+ messages in thread
From: Mark Brown @ 2015-08-20 17:10 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: alsa-devel@alsa-project.org, Ricard Wanderlof, Liam Girdwood


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

On Thu, Aug 20, 2015 at 10:45:21AM -0500, Pierre-Louis Bossart wrote:
> On 8/19/15 10:32 AM, Ricard Wanderlof wrote:

> >+		/*
> >+		 * Skip CODECs which don't support the current stream type.
> >+		 * Otherwise, since the rate, channel, and format values will
> >+		 * zero in that case, we would have no usable settings left,
> >+		 * causing the resulting setup to fail.
> >+		 * At least one CODEC should match, otherwise we should have
> >+		 * bailed out on a higher level, since there would be no
> >+		 * CODEC to support the transfer direction in that case.
> >+		 */
> >+		if (!snd_soc_dai_stream_valid(rtd->codec_dais[i],
> >+					      substream->stream))

> Maybe I misunderstood but shouldn't there be some sort of verification that
> the codecs can use the same number of slots between playback and capture if
> they share the same LRCLK/FS? e.g. it's not uncommon to have 4 mic capture
> and 2 ch playback. If the capture and playback is handled by different chips
> you'd still need to maintain some level of consistency.

Yes, this was my point about it being hard to work out what's going on
with regard to things being shared.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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



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

* Re: [PATCH v2] ASoC: Handle multiple codecs with split playback / capture
  2015-08-20 15:45 ` Pierre-Louis Bossart
  2015-08-20 17:10   ` Mark Brown
@ 2015-08-21  7:11   ` Ricard Wanderlof
       [not found]     ` <55D72E1B.2040109@linux.intel.com>
  1 sibling, 1 reply; 10+ messages in thread
From: Ricard Wanderlof @ 2015-08-21  7:11 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: alsa-devel@alsa-project.org, Mark Brown, Liam Girdwood


On Thu, 20 Aug 2015, Pierre-Louis Bossart wrote:

> >   /**
> >    * snd_soc_runtime_activate() - Increment active count for PCM runtime components
> >    * @rtd: ASoC PCM runtime that is activated
> > @@ -371,6 +389,20 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
> >
> >   	/* first calculate min/max only for CODECs in the DAI link */
> >   	for (i = 0; i < rtd->num_codecs; i++) {
> > +
> > +		/*
> > +		 * Skip CODECs which don't support the current stream type.
> > +		 * Otherwise, since the rate, channel, and format values will
> > +		 * zero in that case, we would have no usable settings left,
> > +		 * causing the resulting setup to fail.
> > +		 * At least one CODEC should match, otherwise we should have
> > +		 * bailed out on a higher level, since there would be no
> > +		 * CODEC to support the transfer direction in that case.
> > +		 */
> > +		if (!snd_soc_dai_stream_valid(rtd->codec_dais[i],
> > +					      substream->stream))
> 
> Maybe I misunderstood but shouldn't there be some sort of verification 
> that the codecs can use the same number of slots between playback and 
> capture if they share the same LRCLK/FS? e.g. it's not uncommon to have 
> 4 mic capture and 2 ch playback. If the capture and playback is handled 
> by different chips you'd still need to maintain some level of consistency.

You're probably right, although it may be that that sort of thing is 
handled at a higher level, e.g. the machine driver in such a setup would 
configure both codecs to use TDM before we even get to this function, I 
don't know.

I think we'd have to take care of that situation if and when it arises. 
The setup I have here doesn't allow me to test it.

/Ricard
-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

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

* Re: [PATCH v2] ASoC: Handle multiple codecs with split playback / capture
       [not found]     ` <55D72E1B.2040109@linux.intel.com>
@ 2015-08-21 14:05       ` Ricard Wanderlof
  0 siblings, 0 replies; 10+ messages in thread
From: Ricard Wanderlof @ 2015-08-21 14:05 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: alsa-devel, Liam Girdwood


On Fri, 21 Aug 2015, Pierre-Louis Bossart wrote:

> On 8/21/15 2:11 AM, Ricard Wanderlof wrote:
> >
> > On Thu, 20 Aug 2015, Pierre-Louis Bossart wrote:
> >
> >>>    /**
> >>>     * snd_soc_runtime_activate() - Increment active count for PCM runtime components
> >>>     * @rtd: ASoC PCM runtime that is activated
> >>> @@ -371,6 +389,20 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
> >>>
> >>>    	/* first calculate min/max only for CODECs in the DAI link */
> >>>    	for (i = 0; i < rtd->num_codecs; i++) {
> >>> +
> >>> +		/*
> >>> +		 * Skip CODECs which don't support the current stream type.
> >>> +		 * Otherwise, since the rate, channel, and format values will
> >>> +		 * zero in that case, we would have no usable settings left,
> >>> +		 * causing the resulting setup to fail.
> >>> +		 * At least one CODEC should match, otherwise we should have
> >>> +		 * bailed out on a higher level, since there would be no
> >>> +		 * CODEC to support the transfer direction in that case.
> >>> +		 */
> >>> +		if (!snd_soc_dai_stream_valid(rtd->codec_dais[i],
> >>> +					      substream->stream))
> >>
> >> Maybe I misunderstood but shouldn't there be some sort of verification
> >> that the codecs can use the same number of slots between playback and
> >> capture if they share the same LRCLK/FS? e.g. it's not uncommon to have
> >> 4 mic capture and 2 ch playback. If the capture and playback is handled
> >> by different chips you'd still need to maintain some level of consistency.
> >
> > You're probably right, although it may be that that sort of thing is
> > handled at a higher level, e.g. the machine driver in such a setup would
> > configure both codecs to use TDM before we even get to this function, I
> > don't know.
> >
> > I think we'd have to take care of that situation if and when it arises.
> > The setup I have here doesn't allow me to test it.
> 
> ok, sounds fine. maybe you could add this to the comments or commit 
> message then so that this assumption is known.

Ok, good point. Will do.

/Ricard
-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

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

* Re: [PATCH v2] ASoC: Handle multiple codecs with split playback / capture
  2015-08-19 15:32 [PATCH v2] ASoC: Handle multiple codecs with split playback / capture Ricard Wanderlof
  2015-08-20 15:45 ` Pierre-Louis Bossart
@ 2015-09-03 15:05 ` Ricard Wanderlof
  2015-09-03 15:18   ` Mark Brown
  2015-09-20  0:13 ` Applied "ASoC: Handle multiple codecs with split playback / capture" to the asoc tree Mark Brown
  2 siblings, 1 reply; 10+ messages in thread
From: Ricard Wanderlof @ 2015-09-03 15:05 UTC (permalink / raw)
  To: alsa-devel@alsa-project.org, Mark Brown, Liam Girdwood


Any comments, ACKs or NAKs on the patch below? It's been about a week 
since I posted it.

/Ricard

On Wed, 19 Aug 2015, Ricard Wanderlof wrote:

> Add the capability to use multiple codecs on the same DAI linke where
> one codec is used for playback and another one is used for capture.
> 
> Tested on a setup using an SSM2518 for playback and an ICS43432 I2S MEMS
> microphone for capture.
> 
> Signed-off-by: Ricard Wanderlof <ricardw@axis.com>
> ---
> 
> The patch was created after a discussion on the alsa-devel mailing list
> as to how best to implement this functionality.
> (http://mailman.alsa-project.org/pipermail/alsa-devel/2015-June/093214.html).
> 
> V2: Minor code change, otherwise the differences compared to V1 are solely
> related to comments, in particular considerations given to the 
> potential consequences of the patch. After consideration, it seems to me 
> that the patch as it stands augments the current framework with the 
> functionality indicated in the commit message above, without restricting 
> other current uses. Further functionality could be added, but IMHO that 
> should be done as the need arises, when it can be properly tested in a 
> real-world setup.
> 
>  sound/soc/soc-pcm.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 35fe58f4..08407ba 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -34,6 +34,24 @@
>  
>  #define DPCM_MAX_BE_USERS	8
>  
> +/*
> + * snd_soc_dai_stream_valid() - check if a DAI supports the given stream
> + *
> + * Returns true if the DAI supports the indicated stream type.
> + */
> +static bool snd_soc_dai_stream_valid(struct snd_soc_dai *dai, int stream)
> +{
> +	struct snd_soc_pcm_stream *codec_stream;
> +
> +	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
> +		codec_stream = &dai->driver->playback;
> +	else
> +		codec_stream = &dai->driver->capture;
> +
> +	/* If the codec specifies any rate at all, it supports the stream. */
> +	return codec_stream->rates;
> +}
> +
>  /**
>   * snd_soc_runtime_activate() - Increment active count for PCM runtime components
>   * @rtd: ASoC PCM runtime that is activated
> @@ -371,6 +389,20 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
>  
>  	/* first calculate min/max only for CODECs in the DAI link */
>  	for (i = 0; i < rtd->num_codecs; i++) {
> +
> +		/*
> +		 * Skip CODECs which don't support the current stream type.
> +		 * Otherwise, since the rate, channel, and format values will
> +		 * zero in that case, we would have no usable settings left,
> +		 * causing the resulting setup to fail.
> +		 * At least one CODEC should match, otherwise we should have
> +		 * bailed out on a higher level, since there would be no
> +		 * CODEC to support the transfer direction in that case.
> +		 */
> +		if (!snd_soc_dai_stream_valid(rtd->codec_dais[i],
> +					      substream->stream))
> +			continue;
> +
>  		codec_dai_drv = rtd->codec_dais[i]->driver;
>  		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>  			codec_stream = &codec_dai_drv->playback;
> @@ -827,6 +859,23 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
>  		struct snd_soc_dai *codec_dai = rtd->codec_dais[i];
>  		struct snd_pcm_hw_params codec_params;
>  
> +		/*
> +		 * Skip CODECs which don't support the current stream type,
> +		 * the idea being that if a CODEC is not used for the currently
> +		 * set up transfer direction, it should not need to be
> +		 * configured, especially since the configuration used might
> +		 * not even be supported by that CODEC. There may be cases
> +		 * however where a CODEC needs to be set up although it is
> +		 * actually not being used for the transfer, e.g. if a
> +		 * capture-only CODEC is acting as an LRCLK and/or BCLK master
> +		 * for the DAI link including a playback-only CODEC.
> +		 * If this becomes necessary, we will have to augment the
> +		 * machine driver setup with information on how to act, so
> +		 * we can do the right thing here.
> +		 */
> +		if (!snd_soc_dai_stream_valid(codec_dai, substream->stream))
> +			continue;
> +
>  		/* copy params for each codec */
>  		codec_params = *params;
>  
> -- 
> 1.7.10.4
> 
> 
> -- 
> Ricard Wolf Wanderlöf                           ricardw(at)axis.com
> Axis Communications AB, Lund, Sweden            www.axis.com
> Phone +46 46 272 2016                           Fax +46 46 13 61 30
> 

-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

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

* Re: [PATCH v2] ASoC: Handle multiple codecs with split playback / capture
  2015-09-03 15:05 ` Ricard Wanderlof
@ 2015-09-03 15:18   ` Mark Brown
  2015-09-03 15:59     ` Ricard Wanderlof
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2015-09-03 15:18 UTC (permalink / raw)
  To: Ricard Wanderlof; +Cc: alsa-devel@alsa-project.org, Liam Girdwood


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

On Thu, Sep 03, 2015 at 05:05:14PM +0200, Ricard Wanderlof wrote:
> 
> Any comments, ACKs or NAKs on the patch below? It's been about a week 
> since I posted it.
> 
> /Ricard
> 
> On Wed, 19 Aug 2015, Ricard Wanderlof wrote:

Please don't top post or send content free pings, they only add to the
mail volume, and allow a reasonable amount of time for review.  A week
is not reasonable, people travel, take holidays or whatever - two weeks
is at the *lower* bound.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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



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

* Re: [PATCH v2] ASoC: Handle multiple codecs with split playback / capture
  2015-09-03 15:18   ` Mark Brown
@ 2015-09-03 15:59     ` Ricard Wanderlof
  2015-09-03 16:24       ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Ricard Wanderlof @ 2015-09-03 15:59 UTC (permalink / raw)
  To: Mark Brown; +Cc: alsa-devel@alsa-project.org


On Thu, 3 Sep 2015, Mark Brown wrote:

> On Thu, Sep 03, 2015 at 05:05:14PM +0200, Ricard Wanderlof wrote:
> > 
> > Any comments, ACKs or NAKs on the patch below? It's been about a week 
> > since I posted it.
> > 
> > /Ricard
> > 
> > On Wed, 19 Aug 2015, Ricard Wanderlof wrote:
> 
> Please don't top post or send content free pings, they only add to the 
> mail volume, and allow a reasonable amount of time for review.  A week is 
> not reasonable, people travel, take holidays or whatever - two weeks is at 
> the *lower* bound.

Sure no problem. It seemed to me that subsequently submitted patches had 
been reviewed and applied, which led me to think that this one had gotten 
lost in the maelstrom, and people also tend to forget or miss things so a 
friendly reminder is not usually out of order. I've gotten the opposite 
response at other times when querying after a month, getting the response, 
'why didn't you ask earlier'.

No rush otherwise, I'll just sit tight.

/Ricard
-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

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

* Re: [PATCH v2] ASoC: Handle multiple codecs with split playback / capture
  2015-09-03 15:59     ` Ricard Wanderlof
@ 2015-09-03 16:24       ` Mark Brown
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2015-09-03 16:24 UTC (permalink / raw)
  To: Ricard Wanderlof; +Cc: alsa-devel@alsa-project.org


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

On Thu, Sep 03, 2015 at 05:59:35PM +0200, Ricard Wanderlof wrote:

> Sure no problem. It seemed to me that subsequently submitted patches had 
> been reviewed and applied, which led me to think that this one had gotten 
> lost in the maelstrom, and people also tend to forget or miss things so a 
> friendly reminder is not usually out of order. I've gotten the opposite 
> response at other times when querying after a month, getting the response, 
> 'why didn't you ask earlier'.

This is a complicated patch submitted right at the end of the
development cycle which means it's harder than most to review, and
obviously your other patch needed quite a few revisions.  And, just to
emphasise, content free pings are just a waste of space - nobody can
review an empty e-mail, if your patch has been lost then your mail is
not an adequate replacement.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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



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

* Applied "ASoC: Handle multiple codecs with split playback / capture" to the asoc tree
  2015-08-19 15:32 [PATCH v2] ASoC: Handle multiple codecs with split playback / capture Ricard Wanderlof
  2015-08-20 15:45 ` Pierre-Louis Bossart
  2015-09-03 15:05 ` Ricard Wanderlof
@ 2015-09-20  0:13 ` Mark Brown
  2 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2015-09-20  0:13 UTC (permalink / raw)
  To: Ricard Wanderlof, Mark Brown; +Cc: alsa-devel

The patch

   ASoC: Handle multiple codecs with split playback / capture

has been applied to the asoc tree at

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

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From cde79035c6cf578dd33dfea3e39666efc27cbcf2 Mon Sep 17 00:00:00 2001
From: Ricard Wanderlof <ricard.wanderlof@axis.com>
Date: Mon, 24 Aug 2015 14:16:51 +0200
Subject: [PATCH] ASoC: Handle multiple codecs with split playback / capture

Add the capability to use multiple codecs on the same DAI linke where
one codec is used for playback and another one is used for capture.

Tested on a setup using an SSM2518 for playback and an ICS43432 I2S MEMS
microphone for capture.

No verification is made that the playback and capture codec setups are
compatible in terms of number of TDM slots (where applicable).

Signed-off-by: Ricard Wanderlof <ricardw@axis.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-pcm.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 70e4b9d..3173958 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -34,6 +34,24 @@
 
 #define DPCM_MAX_BE_USERS	8
 
+/*
+ * snd_soc_dai_stream_valid() - check if a DAI supports the given stream
+ *
+ * Returns true if the DAI supports the indicated stream type.
+ */
+static bool snd_soc_dai_stream_valid(struct snd_soc_dai *dai, int stream)
+{
+	struct snd_soc_pcm_stream *codec_stream;
+
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+		codec_stream = &dai->driver->playback;
+	else
+		codec_stream = &dai->driver->capture;
+
+	/* If the codec specifies any rate at all, it supports the stream. */
+	return codec_stream->rates;
+}
+
 /**
  * snd_soc_runtime_activate() - Increment active count for PCM runtime components
  * @rtd: ASoC PCM runtime that is activated
@@ -371,6 +389,20 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 
 	/* first calculate min/max only for CODECs in the DAI link */
 	for (i = 0; i < rtd->num_codecs; i++) {
+
+		/*
+		 * Skip CODECs which don't support the current stream type.
+		 * Otherwise, since the rate, channel, and format values will
+		 * zero in that case, we would have no usable settings left,
+		 * causing the resulting setup to fail.
+		 * At least one CODEC should match, otherwise we should have
+		 * bailed out on a higher level, since there would be no
+		 * CODEC to support the transfer direction in that case.
+		 */
+		if (!snd_soc_dai_stream_valid(rtd->codec_dais[i],
+					      substream->stream))
+			continue;
+
 		codec_dai_drv = rtd->codec_dais[i]->driver;
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 			codec_stream = &codec_dai_drv->playback;
@@ -827,6 +859,23 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *codec_dai = rtd->codec_dais[i];
 		struct snd_pcm_hw_params codec_params;
 
+		/*
+		 * Skip CODECs which don't support the current stream type,
+		 * the idea being that if a CODEC is not used for the currently
+		 * set up transfer direction, it should not need to be
+		 * configured, especially since the configuration used might
+		 * not even be supported by that CODEC. There may be cases
+		 * however where a CODEC needs to be set up although it is
+		 * actually not being used for the transfer, e.g. if a
+		 * capture-only CODEC is acting as an LRCLK and/or BCLK master
+		 * for the DAI link including a playback-only CODEC.
+		 * If this becomes necessary, we will have to augment the
+		 * machine driver setup with information on how to act, so
+		 * we can do the right thing here.
+		 */
+		if (!snd_soc_dai_stream_valid(codec_dai, substream->stream))
+			continue;
+
 		/* copy params for each codec */
 		codec_params = *params;
 
-- 
2.5.0

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

end of thread, other threads:[~2015-09-20  0:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-19 15:32 [PATCH v2] ASoC: Handle multiple codecs with split playback / capture Ricard Wanderlof
2015-08-20 15:45 ` Pierre-Louis Bossart
2015-08-20 17:10   ` Mark Brown
2015-08-21  7:11   ` Ricard Wanderlof
     [not found]     ` <55D72E1B.2040109@linux.intel.com>
2015-08-21 14:05       ` Ricard Wanderlof
2015-09-03 15:05 ` Ricard Wanderlof
2015-09-03 15:18   ` Mark Brown
2015-09-03 15:59     ` Ricard Wanderlof
2015-09-03 16:24       ` Mark Brown
2015-09-20  0:13 ` Applied "ASoC: Handle multiple codecs with split playback / capture" to the asoc tree Mark Brown

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