[PATCH 1/1] ASoC: soc-pcm: DPCM cares BE channel constraint

[PATCH 1/1] ASoC: soc-pcm: DPCM cares BE channel constraint

[jiada_wang]

From: Jiada Wang <jiada_wang@mentor.com>

Current DPCM is caring only FE channel configuration. Sometimes
it will be trouble if user selects channel which isn't supported
by BE.

This patch adds new .dpcm_merged_chan on struct snd_soc_dai_link.
DPCM will use FE / BE merged channel if struct snd_soc_dai_link
has it.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 include/sound/soc.h |  2 ++
 sound/soc/soc-pcm.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 1378dcd2128a..f7579f82cc00 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -957,6 +957,8 @@ struct snd_soc_dai_link {
 
 	/* DPCM used FE & BE merged format */
 	unsigned int dpcm_merged_format:1;
+	/* DPCM used FE & BE merged channel */
+	unsigned int dpcm_merged_chan:1;
 
 	/* pmdown_time is ignored at stop */
 	unsigned int ignore_pmdown_time:1;
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 45b52f7b9690..19ebfc958b9d 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1715,6 +1715,46 @@ static u64 dpcm_runtime_base_format(struct snd_pcm_substream *substream)
 	return formats;
 }
 
+static void dpcm_runtime_base_chan(struct snd_pcm_substream *substream,
+				   unsigned int *channels_min,
+				   unsigned int *channels_max)
+{
+	struct snd_soc_pcm_runtime *fe = substream->private_data;
+	struct snd_soc_dpcm *dpcm;
+	int stream = substream->stream;
+
+	if (!fe->dai_link->dpcm_merged_chan)
+		return;
+
+	*channels_min = 0;
+	*channels_max = UINT_MAX;
+
+	/*
+	 * It returns merged BE codec channel;
+	 * if FE want to use it (= dpcm_merged_chan)
+	 */
+
+	list_for_each_entry(dpcm, &fe->dpcm[stream].be_clients, list_be) {
+		struct snd_soc_pcm_runtime *be = dpcm->be;
+		struct snd_soc_dai_driver *codec_dai_drv;
+		struct snd_soc_pcm_stream *codec_stream;
+		int i;
+
+		for (i = 0; i < be->num_codecs; i++) {
+			codec_dai_drv = be->codec_dais[i]->driver;
+			if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+				codec_stream = &codec_dai_drv->playback;
+			else
+				codec_stream = &codec_dai_drv->capture;
+
+			*channels_min = max(*channels_min,
+					    codec_stream->channels_min);
+			*channels_max = min(*channels_max,
+					    codec_stream->channels_max);
+		}
+	}
+}
+
 static void dpcm_set_fe_runtime(struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
@@ -1722,11 +1762,17 @@ static void dpcm_set_fe_runtime(struct snd_pcm_substream *substream)
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct snd_soc_dai_driver *cpu_dai_drv = cpu_dai->driver;
 	u64 format = dpcm_runtime_base_format(substream);
+	unsigned int channels_min = 0, channels_max = UINT_MAX;
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		dpcm_init_runtime_hw(runtime, &cpu_dai_drv->playback, format);
 	else
 		dpcm_init_runtime_hw(runtime, &cpu_dai_drv->capture, format);
+
+	dpcm_runtime_base_chan(substream, &channels_min, &channels_max);
+
+	runtime->hw.channels_min = max(channels_min, runtime->hw.channels_min);
+	runtime->hw.channels_max = min(channels_max, runtime->hw.channels_max);
 }
 
 static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd);
-- 
2.17.0

Applied "ASoC: soc-pcm: DPCM cares BE channel constraint" to the asoc tree

[Mark Brown]

The patch

   ASoC: soc-pcm: DPCM cares BE channel constraint

has been applied to the asoc tree at

   https://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 f4c277b817cc9489fffabffb4e15d2f3b686056c Mon Sep 17 00:00:00 2001
From: Jiada Wang <jiada_wang@mentor.com>
Date: Wed, 20 Jun 2018 18:25:20 +0900
Subject: [PATCH] ASoC: soc-pcm: DPCM cares BE channel constraint

Current DPCM is caring only FE channel configuration. Sometimes
it will be trouble if user selects channel which isn't supported
by BE.

This patch adds new .dpcm_merged_chan on struct snd_soc_dai_link.
DPCM will use FE / BE merged channel if struct snd_soc_dai_link
has it.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 include/sound/soc.h |  2 ++
 sound/soc/soc-pcm.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 1378dcd2128a..f7579f82cc00 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -957,6 +957,8 @@ struct snd_soc_dai_link {
 
 	/* DPCM used FE & BE merged format */
 	unsigned int dpcm_merged_format:1;
+	/* DPCM used FE & BE merged channel */
+	unsigned int dpcm_merged_chan:1;
 
 	/* pmdown_time is ignored at stop */
 	unsigned int ignore_pmdown_time:1;
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 45b52f7b9690..19ebfc958b9d 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1715,6 +1715,46 @@ static u64 dpcm_runtime_base_format(struct snd_pcm_substream *substream)
 	return formats;
 }
 
+static void dpcm_runtime_base_chan(struct snd_pcm_substream *substream,
+				   unsigned int *channels_min,
+				   unsigned int *channels_max)
+{
+	struct snd_soc_pcm_runtime *fe = substream->private_data;
+	struct snd_soc_dpcm *dpcm;
+	int stream = substream->stream;
+
+	if (!fe->dai_link->dpcm_merged_chan)
+		return;
+
+	*channels_min = 0;
+	*channels_max = UINT_MAX;
+
+	/*
+	 * It returns merged BE codec channel;
+	 * if FE want to use it (= dpcm_merged_chan)
+	 */
+
+	list_for_each_entry(dpcm, &fe->dpcm[stream].be_clients, list_be) {
+		struct snd_soc_pcm_runtime *be = dpcm->be;
+		struct snd_soc_dai_driver *codec_dai_drv;
+		struct snd_soc_pcm_stream *codec_stream;
+		int i;
+
+		for (i = 0; i < be->num_codecs; i++) {
+			codec_dai_drv = be->codec_dais[i]->driver;
+			if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+				codec_stream = &codec_dai_drv->playback;
+			else
+				codec_stream = &codec_dai_drv->capture;
+
+			*channels_min = max(*channels_min,
+					    codec_stream->channels_min);
+			*channels_max = min(*channels_max,
+					    codec_stream->channels_max);
+		}
+	}
+}
+
 static void dpcm_set_fe_runtime(struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
@@ -1722,11 +1762,17 @@ static void dpcm_set_fe_runtime(struct snd_pcm_substream *substream)
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct snd_soc_dai_driver *cpu_dai_drv = cpu_dai->driver;
 	u64 format = dpcm_runtime_base_format(substream);
+	unsigned int channels_min = 0, channels_max = UINT_MAX;
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		dpcm_init_runtime_hw(runtime, &cpu_dai_drv->playback, format);
 	else
 		dpcm_init_runtime_hw(runtime, &cpu_dai_drv->capture, format);
+
+	dpcm_runtime_base_chan(substream, &channels_min, &channels_max);
+
+	runtime->hw.channels_min = max(channels_min, runtime->hw.channels_min);
+	runtime->hw.channels_max = min(channels_max, runtime->hw.channels_max);
 }
 
 static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd);
-- 
2.18.0.rc2

Re: [PATCH 1/1] ASoC: soc-pcm: DPCM cares BE channel constraint

[Jerome Brunet]

On Wed, 2018-06-20 at 18:25 +0900, jiada_wang@mentor.com wrote:
> From: Jiada Wang <jiada_wang@mentor.com>
> 
> Current DPCM is caring only FE channel configuration. Sometimes
> it will be trouble if user selects channel which isn't supported
> by BE.
> 
> This patch adds new .dpcm_merged_chan on struct snd_soc_dai_link.
> DPCM will use FE / BE merged channel if struct snd_soc_dai_link
> has it.
> 
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> ---
>  include/sound/soc.h |  2 ++
>  sound/soc/soc-pcm.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 1378dcd2128a..f7579f82cc00 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -957,6 +957,8 @@ struct snd_soc_dai_link {
>  
>  	/* DPCM used FE & BE merged format */
>  	unsigned int dpcm_merged_format:1;
> +	/* DPCM used FE & BE merged channel */
> +	unsigned int dpcm_merged_chan:1;

Jiada, Mark,

Do you think we could extend this flag to let the link choose whether the merge
should be performed on the codec dais (as done here) or on the backend cpu dais
?

I have more less the same need as Jiada but since my card uses multicodec links,
merging on the codec dais does not work for me.

Like in soc_pcm_init_runtime_hw(), we can't enforce channels min/max based on
the codec when there is multiple codecs on the link.

Thx
Regards

Jerome

>  
>  	/* pmdown_time is ignored at stop */
>  	unsigned int ignore_pmdown_time:1;
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 45b52f7b9690..19ebfc958b9d 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -1715,6 +1715,46 @@ static u64 dpcm_runtime_base_format(struct snd_pcm_substream *substream)
>  	return formats;
>  }
>  
> +static void dpcm_runtime_base_chan(struct snd_pcm_substream *substream,
> +				   unsigned int *channels_min,
> +				   unsigned int *channels_max)
> +{
> +	struct snd_soc_pcm_runtime *fe = substream->private_data;
> +	struct snd_soc_dpcm *dpcm;
> +	int stream = substream->stream;
> +
> +	if (!fe->dai_link->dpcm_merged_chan)
> +		return;
> +
> +	*channels_min = 0;
> +	*channels_max = UINT_MAX;
> +
> +	/*
> +	 * It returns merged BE codec channel;
> +	 * if FE want to use it (= dpcm_merged_chan)
> +	 */
> +
> +	list_for_each_entry(dpcm, &fe->dpcm[stream].be_clients, list_be) {
> +		struct snd_soc_pcm_runtime *be = dpcm->be;
> +		struct snd_soc_dai_driver *codec_dai_drv;
> +		struct snd_soc_pcm_stream *codec_stream;
> +		int i;
> +
> +		for (i = 0; i < be->num_codecs; i++) {
> +			codec_dai_drv = be->codec_dais[i]->driver;
> +			if (stream == SNDRV_PCM_STREAM_PLAYBACK)
> +				codec_stream = &codec_dai_drv->playback;
> +			else
> +				codec_stream = &codec_dai_drv->capture;
> +
> +			*channels_min = max(*channels_min,
> +					    codec_stream->channels_min);
> +			*channels_max = min(*channels_max,
> +					    codec_stream->channels_max);
> +		}
> +	}
> +}
> +
>  static void dpcm_set_fe_runtime(struct snd_pcm_substream *substream)
>  {
>  	struct snd_pcm_runtime *runtime = substream->runtime;
> @@ -1722,11 +1762,17 @@ static void dpcm_set_fe_runtime(struct snd_pcm_substream *substream)
>  	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
>  	struct snd_soc_dai_driver *cpu_dai_drv = cpu_dai->driver;
>  	u64 format = dpcm_runtime_base_format(substream);
> +	unsigned int channels_min = 0, channels_max = UINT_MAX;
>  
>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>  		dpcm_init_runtime_hw(runtime, &cpu_dai_drv->playback, format);
>  	else
>  		dpcm_init_runtime_hw(runtime, &cpu_dai_drv->capture, format);
> +
> +	dpcm_runtime_base_chan(substream, &channels_min, &channels_max);
> +
> +	runtime->hw.channels_min = max(channels_min, runtime->hw.channels_min);
> +	runtime->hw.channels_max = min(channels_max, runtime->hw.channels_max);
>  }
>  
>  static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd);

Re: [PATCH 1/1] ASoC: soc-pcm: DPCM cares BE channel constraint

[Mark Brown]

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

On Tue, Jun 26, 2018 at 12:18:38PM +0200, Jerome Brunet wrote:
> On Wed, 2018-06-20 at 18:25 +0900, jiada_wang@mentor.com wrote:

> > +	/* DPCM used FE & BE merged channel */
> > +	unsigned int dpcm_merged_chan:1;

> Jiada, Mark,

> Do you think we could extend this flag to let the link choose whether the merge
> should be performed on the codec dais (as done here) or on the backend cpu dais
> ?

> I have more less the same need as Jiada but since my card uses multicodec links,
> merging on the codec dais does not work for me.

> Like in soc_pcm_init_runtime_hw(), we can't enforce channels min/max based on
> the codec when there is multiple codecs on the link.

Ugh, probably that'd work.  The ideal thing would be to remove DPCM but
we're stuck with it for the time being :(

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

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

Re: [PATCH 1/1] ASoC: soc-pcm: DPCM cares BE channel constraint

[Jerome Brunet]

On Tue, 2018-06-26 at 12:55 +0100, Mark Brown wrote:
> On Tue, Jun 26, 2018 at 12:18:38PM +0200, Jerome Brunet wrote:
> > On Wed, 2018-06-20 at 18:25 +0900, jiada_wang@mentor.com wrote:
> > > +	/* DPCM used FE & BE merged channel */
> > > +	unsigned int dpcm_merged_chan:1;
> > Jiada, Mark,
> > Do you think we could extend this flag to let the link choose whether the merge
> > should be performed on the codec dais (as done here) or on the backend cpu dais
> > ?
> > I have more less the same need as Jiada but since my card uses multicodec links,
> > merging on the codec dais does not work for me.
> > Like in soc_pcm_init_runtime_hw(), we can't enforce channels min/max based on
> > the codec when there is multiple codecs on the link.
> 
> Ugh, probably that'd work.  The ideal thing would be to remove DPCM but
> we're stuck with it for the time being :(

>From comment here and in other mails, I get that you are not big fan of DPCM :)
It is indeed a complex beast ...

It there anything else we can use to represent audio routing within the SoC ?
The SoC I'm working on make heavy use of this, unfortunately. 

Re: [PATCH 1/1] ASoC: soc-pcm: DPCM cares BE channel constraint

[Mark Brown]

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

On Tue, Jun 26, 2018 at 02:08:40PM +0200, Jerome Brunet wrote:

> From comment here and in other mails, I get that you are not big fan of DPCM :)
> It is indeed a complex beast ...

> It there anything else we can use to represent audio routing within the SoC ?
> The SoC I'm working on make heavy use of this, unfortunately. 

The goal is to be able to propagate digital configuration along audio
paths in much the same way as we currently do for power with DAPM.  Lars
did a presentation with a good writeup of a proposal for that at ELC-E a
few years ago.  Unfortunately the only person that is really able to
find time to work on this at the minute is Morimoto-san, this is what
his componentization changes are driving towards - part of the design is
that everything in the system will be a component and we can therefore
treat every device identically.

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

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