[PATCH 1/4] ASoC: compress: Only assign compr->ops->copy once

[PATCH 1/4] ASoC: compress: Only assign compr->ops->copy once

[Charles Keepax]

There are only one set of ops on the compressed stream so no need to
reassign the copy callback repeatedly, stop after copy is seen to be
necessary.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 sound/soc/soc-compress.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index ba56f87f96d4c..62875c6a93a14 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -973,6 +973,7 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
 			continue;
 
 		compr->ops->copy = soc_compr_copy;
+		break;
 	}
 
 
-- 
2.11.0

[PATCH 2/4] ASoC: compress: Clarify the intent of current compressed ops handling

[Charles Keepax]

It is proposed that the model adopted for compressed component
support currently should be that multiple components are supported
on a compressed DAI but that they must provide a unified set of
capabilities. So for example having multiple components involved in
the decode is fine but the core will not presently attempt to make
smart decisions like offloading MP3 to this component and AAC another.

To implement this it is suggested that callbacks configuring the state
of the components (trigger, set_params, ack and set_metadata) should be
called on all components and required to succeed on all components
before being considered to have succeeded. However for callbacks that
return information from the driver (copy, get_metadata, pointer,
get_codec_caps, get_caps and get_params) it is expected that either
there is only a single provider on the link or that all components
will return identical results.

Essentially this matches the current implementation of the code and
only small clean ups are undertaken here.

For callbacks configuring the state of the components simplify the
code a little and make intention clearer by aborting as soon as an
error is encountered. The operation has already failed and there is
nothing to be gained from processing the additional callbacks.

For callbacks returning information from the driver only look for the
first callback provided, currently the code will call every callback
only returning the information provided by the last. Again this makes
the currently supported feature set a little more clear.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 sound/soc/soc-compress.c | 105 +++++++++++++++++++++--------------------------
 1 file changed, 47 insertions(+), 58 deletions(-)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 62875c6a93a14..b411143f21ccc 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -363,7 +363,7 @@ static int soc_compr_trigger(struct snd_compr_stream *cstream, int cmd)
 	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret = 0, __ret;
+	int ret = 0;
 
 	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
 
@@ -374,12 +374,10 @@ static int soc_compr_trigger(struct snd_compr_stream *cstream, int cmd)
 		    !component->driver->compr_ops->trigger)
 			continue;
 
-		__ret = component->driver->compr_ops->trigger(cstream, cmd);
-		if (__ret < 0)
-			ret = __ret;
+		ret = component->driver->compr_ops->trigger(cstream, cmd);
+		if (ret < 0)
+			goto out;
 	}
-	if (ret < 0)
-		goto out;
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->trigger)
 		cpu_dai->driver->cops->trigger(cstream, cmd, cpu_dai);
@@ -404,7 +402,7 @@ static int soc_compr_trigger_fe(struct snd_compr_stream *cstream, int cmd)
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = fe->cpu_dai;
-	int ret = 0, __ret, stream;
+	int ret, stream;
 
 	if (cmd == SND_COMPR_TRIGGER_PARTIAL_DRAIN ||
 		cmd == SND_COMPR_TRIGGER_DRAIN) {
@@ -416,9 +414,9 @@ static int soc_compr_trigger_fe(struct snd_compr_stream *cstream, int cmd)
 			    !component->driver->compr_ops->trigger)
 				continue;
 
-			__ret = component->driver->compr_ops->trigger(cstream, cmd);
-			if (__ret < 0)
-				ret = __ret;
+			ret = component->driver->compr_ops->trigger(cstream, cmd);
+			if (ret < 0)
+				return ret;
 		}
 		return ret;
 	}
@@ -444,12 +442,10 @@ static int soc_compr_trigger_fe(struct snd_compr_stream *cstream, int cmd)
 		    !component->driver->compr_ops->trigger)
 			continue;
 
-		__ret = component->driver->compr_ops->trigger(cstream, cmd);
-		if (__ret < 0)
-			ret = __ret;
+		ret = component->driver->compr_ops->trigger(cstream, cmd);
+		if (ret < 0)
+			goto out;
 	}
-	if (ret < 0)
-		goto out;
 
 	fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE;
 
@@ -483,7 +479,7 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream,
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret = 0, __ret;
+	int ret;
 
 	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
 
@@ -507,12 +503,10 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream,
 		    !component->driver->compr_ops->set_params)
 			continue;
 
-		__ret = component->driver->compr_ops->set_params(cstream, params);
-		if (__ret < 0)
-			ret = __ret;
+		ret = component->driver->compr_ops->set_params(cstream, params);
+		if (ret < 0)
+			goto err;
 	}
-	if (ret < 0)
-		goto err;
 
 	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->set_params) {
 		ret = rtd->dai_link->compr_ops->set_params(cstream);
@@ -533,7 +527,7 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream,
 
 	cancel_delayed_work_sync(&rtd->delayed_work);
 
-	return ret;
+	return 0;
 
 err:
 	mutex_unlock(&rtd->pcm_mutex);
@@ -549,7 +543,7 @@ static int soc_compr_set_params_fe(struct snd_compr_stream *cstream,
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = fe->cpu_dai;
-	int ret = 0, __ret, stream;
+	int ret, stream;
 
 	if (cstream->direction == SND_COMPRESS_PLAYBACK)
 		stream = SNDRV_PCM_STREAM_PLAYBACK;
@@ -571,12 +565,10 @@ static int soc_compr_set_params_fe(struct snd_compr_stream *cstream,
 		    !component->driver->compr_ops->set_params)
 			continue;
 
-		__ret = component->driver->compr_ops->set_params(cstream, params);
-		if (__ret < 0)
-			ret = __ret;
+		ret = component->driver->compr_ops->set_params(cstream, params);
+		if (ret < 0)
+			goto out;
 	}
-	if (ret < 0)
-		goto out;
 
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->set_params) {
 		ret = fe->dai_link->compr_ops->set_params(cstream);
@@ -618,7 +610,7 @@ static int soc_compr_get_params(struct snd_compr_stream *cstream,
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret = 0, __ret;
+	int ret = 0;
 
 	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
 
@@ -635,9 +627,8 @@ static int soc_compr_get_params(struct snd_compr_stream *cstream,
 		    !component->driver->compr_ops->get_params)
 			continue;
 
-		__ret = component->driver->compr_ops->get_params(cstream, params);
-		if (__ret < 0)
-			ret = __ret;
+		ret = component->driver->compr_ops->get_params(cstream, params);
+		break;
 	}
 
 err:
@@ -651,7 +642,7 @@ static int soc_compr_get_caps(struct snd_compr_stream *cstream,
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
-	int ret = 0, __ret;
+	int ret = 0;
 
 	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
 
@@ -662,9 +653,8 @@ static int soc_compr_get_caps(struct snd_compr_stream *cstream,
 		    !component->driver->compr_ops->get_caps)
 			continue;
 
-		__ret = component->driver->compr_ops->get_caps(cstream, caps);
-		if (__ret < 0)
-			ret = __ret;
+		ret = component->driver->compr_ops->get_caps(cstream, caps);
+		break;
 	}
 
 	mutex_unlock(&rtd->pcm_mutex);
@@ -677,7 +667,7 @@ static int soc_compr_get_codec_caps(struct snd_compr_stream *cstream,
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
-	int ret = 0, __ret;
+	int ret = 0;
 
 	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
 
@@ -688,9 +678,9 @@ static int soc_compr_get_codec_caps(struct snd_compr_stream *cstream,
 		    !component->driver->compr_ops->get_codec_caps)
 			continue;
 
-		__ret = component->driver->compr_ops->get_codec_caps(cstream, codec);
-		if (__ret < 0)
-			ret = __ret;
+		ret = component->driver->compr_ops->get_codec_caps(cstream,
+								   codec);
+		break;
 	}
 
 	mutex_unlock(&rtd->pcm_mutex);
@@ -703,7 +693,7 @@ static int soc_compr_ack(struct snd_compr_stream *cstream, size_t bytes)
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret = 0, __ret;
+	int ret = 0;
 
 	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
 
@@ -720,9 +710,9 @@ static int soc_compr_ack(struct snd_compr_stream *cstream, size_t bytes)
 		    !component->driver->compr_ops->ack)
 			continue;
 
-		__ret = component->driver->compr_ops->ack(cstream, bytes);
-		if (__ret < 0)
-			ret = __ret;
+		ret = component->driver->compr_ops->ack(cstream, bytes);
+		if (ret < 0)
+			goto err;
 	}
 
 err:
@@ -736,7 +726,7 @@ static int soc_compr_pointer(struct snd_compr_stream *cstream,
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
-	int ret = 0, __ret;
+	int ret = 0;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 
 	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
@@ -751,9 +741,8 @@ static int soc_compr_pointer(struct snd_compr_stream *cstream,
 		    !component->driver->compr_ops->pointer)
 			continue;
 
-		__ret = component->driver->compr_ops->pointer(cstream, tstamp);
-		if (__ret < 0)
-			ret = __ret;
+		ret = component->driver->compr_ops->pointer(cstream, tstamp);
+		break;
 	}
 
 	mutex_unlock(&rtd->pcm_mutex);
@@ -792,7 +781,7 @@ static int soc_compr_set_metadata(struct snd_compr_stream *cstream,
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret = 0, __ret;
+	int ret;
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->set_metadata) {
 		ret = cpu_dai->driver->cops->set_metadata(cstream, metadata, cpu_dai);
@@ -807,12 +796,13 @@ static int soc_compr_set_metadata(struct snd_compr_stream *cstream,
 		    !component->driver->compr_ops->set_metadata)
 			continue;
 
-		__ret = component->driver->compr_ops->set_metadata(cstream, metadata);
-		if (__ret < 0)
-			ret = __ret;
+		ret = component->driver->compr_ops->set_metadata(cstream,
+								 metadata);
+		if (ret < 0)
+			return ret;
 	}
 
-	return ret;
+	return 0;
 }
 
 static int soc_compr_get_metadata(struct snd_compr_stream *cstream,
@@ -822,7 +812,7 @@ static int soc_compr_get_metadata(struct snd_compr_stream *cstream,
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret = 0, __ret;
+	int ret;
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->get_metadata) {
 		ret = cpu_dai->driver->cops->get_metadata(cstream, metadata, cpu_dai);
@@ -837,12 +827,11 @@ static int soc_compr_get_metadata(struct snd_compr_stream *cstream,
 		    !component->driver->compr_ops->get_metadata)
 			continue;
 
-		__ret = component->driver->compr_ops->get_metadata(cstream, metadata);
-		if (__ret < 0)
-			ret = __ret;
+		return component->driver->compr_ops->get_metadata(cstream,
+								  metadata);
 	}
 
-	return ret;
+	return 0;
 }
 
 /* ASoC Compress operations */
-- 
2.11.0

Re: [PATCH 2/4] ASoC: compress: Clarify the intent of current compressed ops handling

[Vinod Koul]

On Thu, Apr 26, 2018 at 05:30:05PM +0100, Charles Keepax wrote:
> It is proposed that the model adopted for compressed component
> support currently should be that multiple components are supported
> on a compressed DAI but that they must provide a unified set of
> capabilities. So for example having multiple components involved in
> the decode is fine but the core will not presently attempt to make
> smart decisions like offloading MP3 to this component and AAC another.

Well this is supposed to be entirely a userspace call, we just present
devices with capabilities and the usespace decides... Btw capabilities are
supposed to be dynamic.

Looking at the code again now, I realized that we are treating compress like
PCM. It makes little sense to me to have multiple components for a
compressed device, does that model on your systems..?

> To implement this it is suggested that callbacks configuring the state
> of the components (trigger, set_params, ack and set_metadata) should be
> called on all components and required to succeed on all components
> before being considered to have succeeded. However for callbacks that
> return information from the driver (copy, get_metadata, pointer,
> get_codec_caps, get_caps and get_params) it is expected that either
> there is only a single provider on the link or that all components
> will return identical results.
> 
> Essentially this matches the current implementation of the code and
> only small clean ups are undertaken here.

> For callbacks configuring the state of the components simplify the
> code a little and make intention clearer by aborting as soon as an
> error is encountered. The operation has already failed and there is
> nothing to be gained from processing the additional callbacks.
> 
> For callbacks returning information from the driver only look for the
> first callback provided, currently the code will call every callback
> only returning the information provided by the last. Again this makes
> the currently supported feature set a little more clear.

Btw code changes look fine but I think we need broader cleanup here...

-- 
~Vinod

Re: [PATCH 2/4] ASoC: compress: Clarify the intent of current compressed ops handling

[Charles Keepax]

On Thu, May 03, 2018 at 09:57:14PM +0530, Vinod Koul wrote:
> On Thu, Apr 26, 2018 at 05:30:05PM +0100, Charles Keepax wrote:
> > It is proposed that the model adopted for compressed component
> > support currently should be that multiple components are supported
> > on a compressed DAI but that they must provide a unified set of
> > capabilities. So for example having multiple components involved in
> > the decode is fine but the core will not presently attempt to make
> > smart decisions like offloading MP3 to this component and AAC another.
> 
> Well this is supposed to be entirely a userspace call, we just present
> devices with capabilities and the usespace decides... Btw capabilities are
> supposed to be dynamic.

My intention was to not suggest otherwise. The only point I
was really making is that if there are multiple components on
the link the core won't make attempt to amalgamate output from
those components, but we will inform all components of activity
on the DAI.

> Looking at the code again now, I realized that we are treating compress like
> PCM. It makes little sense to me to have multiple components for a
> compressed device, does that model on your systems..?

So that was very much my initial reaction as well [1], and
certainly for our devices it only really makes sense to have a
single component handle the compressed ops. Hence why I initially
started with basically just returning the first component with
compressed ops.

That said however, thinking about it more I do think there are
pretty reasonable systems that have multiple components on a
compressed DAI. For example I could imagine a system where you
have a DSP on both the AP and CODEC ends of the DAI link and
they split the decode doing some work on each. In that case
one would want calls like open and set_params to go to both
components.

As you say separate decode engines probably belong on separate
DAIs and that kinda is what led me to the current series. It
should implement enough to enable basic multi-component use-cases
but makes it more clear that we are not supporting multiplexing
multiple offloads onto a single DAI at the moment.

Thanks,
Charles

[1] https://patchwork.kernel.org/patch/10182603/

Re: [PATCH 2/4] ASoC: compress: Clarify the intent of current compressed ops handling

[Charles Keepax]

On Fri, May 04, 2018 at 12:59:22PM +0100, Charles Keepax wrote:
> On Thu, May 03, 2018 at 09:57:14PM +0530, Vinod Koul wrote:
> > On Thu, Apr 26, 2018 at 05:30:05PM +0100, Charles Keepax wrote:
> > > It is proposed that the model adopted for compressed component
> > > support currently should be that multiple components are supported
> > > on a compressed DAI but that they must provide a unified set of
> > > capabilities. So for example having multiple components involved in
> > > the decode is fine but the core will not presently attempt to make
> > > smart decisions like offloading MP3 to this component and AAC another.
> > 
> > Well this is supposed to be entirely a userspace call, we just present
> > devices with capabilities and the usespace decides... Btw capabilities are
> > supposed to be dynamic.
> 
> My intention was to not suggest otherwise. The only point I
> was really making is that if there are multiple components on
> the link the core won't make attempt to amalgamate output from
> those components, but we will inform all components of activity
> on the DAI.
> 
> > Looking at the code again now, I realized that we are treating compress like
> > PCM. It makes little sense to me to have multiple components for a
> > compressed device, does that model on your systems..?
> 
> So that was very much my initial reaction as well [1], and
> certainly for our devices it only really makes sense to have a
> single component handle the compressed ops. Hence why I initially
> started with basically just returning the first component with
> compressed ops.
> 
> That said however, thinking about it more I do think there are
> pretty reasonable systems that have multiple components on a
> compressed DAI. For example I could imagine a system where you
> have a DSP on both the AP and CODEC ends of the DAI link and
> they split the decode doing some work on each. In that case
> one would want calls like open and set_params to go to both
> components.
> 
> As you say separate decode engines probably belong on separate
> DAIs and that kinda is what led me to the current series. It
> should implement enough to enable basic multi-component use-cases
> but makes it more clear that we are not supporting multiplexing
> multiple offloads onto a single DAI at the moment.
> 
> Thanks,
> Charles
> 
> [1] https://patchwork.kernel.org/patch/10182603/

Just adding Vinod's kernel.org address.

Thanks,
Charles

Re: [PATCH 2/4] ASoC: compress: Clarify the intent of current compressed ops handling

[Vinod Koul]

On 04-05-18, 12:59, Charles Keepax wrote:
> On Thu, May 03, 2018 at 09:57:14PM +0530, Vinod Koul wrote:
> > On Thu, Apr 26, 2018 at 05:30:05PM +0100, Charles Keepax wrote:
> > > It is proposed that the model adopted for compressed component
> > > support currently should be that multiple components are supported
> > > on a compressed DAI but that they must provide a unified set of
> > > capabilities. So for example having multiple components involved in
> > > the decode is fine but the core will not presently attempt to make
> > > smart decisions like offloading MP3 to this component and AAC another.
> > 
> > Well this is supposed to be entirely a userspace call, we just present
> > devices with capabilities and the usespace decides... Btw capabilities are
> > supposed to be dynamic.
> 
> My intention was to not suggest otherwise. The only point I
> was really making is that if there are multiple components on
> the link the core won't make attempt to amalgamate output from
> those components, but we will inform all components of activity
> on the DAI.
> 
> > Looking at the code again now, I realized that we are treating compress like
> > PCM. It makes little sense to me to have multiple components for a
> > compressed device, does that model on your systems..?
> 
> So that was very much my initial reaction as well [1], and
> certainly for our devices it only really makes sense to have a
> single component handle the compressed ops. Hence why I initially
> started with basically just returning the first component with
> compressed ops.
> 
> That said however, thinking about it more I do think there are
> pretty reasonable systems that have multiple components on a
> compressed DAI. For example I could imagine a system where you
> have a DSP on both the AP and CODEC ends of the DAI link and
> they split the decode doing some work on each. In that case
> one would want calls like open and set_params to go to both
> components.

Am not sure if we can split the decode into multiple DSPs like
this :) Yes one can do processing and one can decode if both have
the capability but I don't forsee that being split, so not sure
if we need it!!!

> As you say separate decode engines probably belong on separate
> DAIs and that kinda is what led me to the current series. It
> should implement enough to enable basic multi-component use-cases
> but makes it more clear that we are not supporting multiplexing
> multiple offloads onto a single DAI at the moment.
> 
> Thanks,
> Charles
> 
> [1] https://patchwork.kernel.org/patch/10182603/

-- 
~Vinod

Re: [PATCH 2/4] ASoC: compress: Clarify the intent of current compressed ops handling

[Charles Keepax]

On Tue, May 08, 2018 at 02:03:19PM +0530, Vinod Koul wrote:
> On 04-05-18, 12:59, Charles Keepax wrote:
> > On Thu, May 03, 2018 at 09:57:14PM +0530, Vinod Koul wrote:
> > > On Thu, Apr 26, 2018 at 05:30:05PM +0100, Charles Keepax wrote:
> > That said however, thinking about it more I do think there are
> > pretty reasonable systems that have multiple components on a
> > compressed DAI. For example I could imagine a system where you
> > have a DSP on both the AP and CODEC ends of the DAI link and
> > they split the decode doing some work on each. In that case
> > one would want calls like open and set_params to go to both
> > components.
> 
> Am not sure if we can split the decode into multiple DSPs like
> this :) Yes one can do processing and one can decode if both have
> the capability but I don't forsee that being split, so not sure
> if we need it!!!
>

I think you definitely could split the decode across multiple
DSPs like that, we have had processing split across multiple
cores for various things on the CODECs. That said "Need it"
is very strong, I would say such a system is plausible but
not something I am aware anyone is actually building right now.

I guess my thinking was that the system is basically already
supporting multiple components so it seemed like a step
backwards to rip it out given there are plausible systems were
it might be useful. And this patch is really just tidying up
the already submitted code a little rather than changing the
functionality in any substancial way.

That said if you feel strongly about it, I certainly don't
need the support in the foreseeable future. I am happy to
go back to something similar to my earlier patch that just
locates the first device with compressed ops and calls the
ops on that?  Although it might still be worth merging this
one as an intermediate tidy up in that case.

Thanks,
Charles

Re: [PATCH 2/4] ASoC: compress: Clarify the intent of current compressed ops handling

[Vinod Koul]

On 08-05-18, 11:52, Charles Keepax wrote:
> On Tue, May 08, 2018 at 02:03:19PM +0530, Vinod Koul wrote:
> > On 04-05-18, 12:59, Charles Keepax wrote:
> > > On Thu, May 03, 2018 at 09:57:14PM +0530, Vinod Koul wrote:
> > > > On Thu, Apr 26, 2018 at 05:30:05PM +0100, Charles Keepax wrote:
> > > That said however, thinking about it more I do think there are
> > > pretty reasonable systems that have multiple components on a
> > > compressed DAI. For example I could imagine a system where you
> > > have a DSP on both the AP and CODEC ends of the DAI link and
> > > they split the decode doing some work on each. In that case
> > > one would want calls like open and set_params to go to both
> > > components.
> > 
> > Am not sure if we can split the decode into multiple DSPs like
> > this :) Yes one can do processing and one can decode if both have
> > the capability but I don't forsee that being split, so not sure
> > if we need it!!!
> >
> 
> I think you definitely could split the decode across multiple
> DSPs like that, we have had processing split across multiple
> cores for various things on the CODECs. That said "Need it"
> is very strong, I would say such a system is plausible but
> not something I am aware anyone is actually building right now.

well processing is different, we are dealing with PCM data.
Decode is one shot, you get PCM output and then can do whatever
you want with it.

If decode is split what is the output format ;-)

> I guess my thinking was that the system is basically already
> supporting multiple components so it seemed like a step
> backwards to rip it out given there are plausible systems were
> it might be useful. And this patch is really just tidying up
> the already submitted code a little rather than changing the
> functionality in any substancial way.
> 
> That said if you feel strongly about it, I certainly don't
> need the support in the foreseeable future. I am happy to
> go back to something similar to my earlier patch that just
> locates the first device with compressed ops and calls the
> ops on that?  Although it might still be worth merging this
> one as an intermediate tidy up in that case.

Going back would be better, I leave it upto Mark on how he wants
to do this, either way am fine if end goal is met :)

> 
> Thanks,
> Charles

-- 
-- 
~Vinod

Re: [PATCH 2/4] ASoC: compress: Clarify the intent of current compressed ops handling

[Charles Keepax]

On Tue, May 08, 2018 at 05:34:53PM +0530, Vinod Koul wrote:
> On 08-05-18, 11:52, Charles Keepax wrote:
> > On Tue, May 08, 2018 at 02:03:19PM +0530, Vinod Koul wrote:
> > > On 04-05-18, 12:59, Charles Keepax wrote:
> > > > On Thu, May 03, 2018 at 09:57:14PM +0530, Vinod Koul wrote:
> > > > > On Thu, Apr 26, 2018 at 05:30:05PM +0100, Charles Keepax wrote:
> > > > That said however, thinking about it more I do think there are
> > > > pretty reasonable systems that have multiple components on a
> > > > compressed DAI. For example I could imagine a system where you
> > > > have a DSP on both the AP and CODEC ends of the DAI link and
> > > > they split the decode doing some work on each. In that case
> > > > one would want calls like open and set_params to go to both
> > > > components.
> > > 
> > > Am not sure if we can split the decode into multiple DSPs like
> > > this :) Yes one can do processing and one can decode if both have
> > > the capability but I don't forsee that being split, so not sure
> > > if we need it!!!
> > >
> > 
> > I think you definitely could split the decode across multiple
> > DSPs like that, we have had processing split across multiple
> > cores for various things on the CODECs. That said "Need it"
> > is very strong, I would say such a system is plausible but
> > not something I am aware anyone is actually building right now.
> 
> well processing is different, we are dealing with PCM data.
> Decode is one shot, you get PCM output and then can do whatever
> you want with it.
> 
> If decode is split what is the output format ;-)
> 

The end output format is still PCM, but what is passed from the
DSP on the AP to the DSP on the CODEC could be anything. And
decode is hardly one shot, most decodes are made up of many
steps (frequency domain transforms, lookup tables, etc.). It seems
like we are slightly talking at cross purposes here, I wonder is
this because you are thinking mostly of the DPCM compressed case
where it looks like it is mostly hard-coded to use a PCM backend?
Whereas I am thinking more of how our systems is used where it is
a direct compressed DAI so there isn't really a backend?

> > I guess my thinking was that the system is basically already
> > supporting multiple components so it seemed like a step
> > backwards to rip it out given there are plausible systems were
> > it might be useful. And this patch is really just tidying up
> > the already submitted code a little rather than changing the
> > functionality in any substancial way.
> > 
> > That said if you feel strongly about it, I certainly don't
> > need the support in the foreseeable future. I am happy to
> > go back to something similar to my earlier patch that just
> > locates the first device with compressed ops and calls the
> > ops on that?  Although it might still be worth merging this
> > one as an intermediate tidy up in that case.
> 
> Going back would be better, I leave it upto Mark on how he wants
> to do this, either way am fine if end goal is met :)
> 

Alright will wait and see what Mark thinks, but will dig out my
original patch on the assumption that is probably the way we are
going.

Thanks,
Charles

[PATCH 3/4] ASoC: compress: Add helper functions for component trigger/set_params

[Charles Keepax]

The trigger and set_params callbacks are called from 3 and 2 separate
loops respectively, tidy up the code a little by factoring these out
into helper functions.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 sound/soc/soc-compress.c | 116 ++++++++++++++++++++++-------------------------
 1 file changed, 54 insertions(+), 62 deletions(-)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index b411143f21ccc..3d107652b7ca3 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -355,17 +355,13 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 	return 0;
 }
 
-static int soc_compr_trigger(struct snd_compr_stream *cstream, int cmd)
+static int soc_compr_components_trigger(struct snd_compr_stream *cstream,
+					int cmd)
 {
-
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
-	struct snd_soc_dai *codec_dai = rtd->codec_dai;
-	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret = 0;
-
-	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
+	int ret;
 
 	for_each_rtdcom(rtd, rtdcom) {
 		component = rtdcom->component;
@@ -376,9 +372,25 @@ static int soc_compr_trigger(struct snd_compr_stream *cstream, int cmd)
 
 		ret = component->driver->compr_ops->trigger(cstream, cmd);
 		if (ret < 0)
-			goto out;
+			return ret;
 	}
 
+	return 0;
+}
+
+static int soc_compr_trigger(struct snd_compr_stream *cstream, int cmd)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	int ret;
+
+	mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
+
+	ret = soc_compr_components_trigger(cstream, cmd);
+	if (ret < 0)
+		goto out;
+
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->trigger)
 		cpu_dai->driver->cops->trigger(cstream, cmd, cpu_dai);
 
@@ -399,27 +411,12 @@ static int soc_compr_trigger(struct snd_compr_stream *cstream, int cmd)
 static int soc_compr_trigger_fe(struct snd_compr_stream *cstream, int cmd)
 {
 	struct snd_soc_pcm_runtime *fe = cstream->private_data;
-	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = fe->cpu_dai;
 	int ret, stream;
 
 	if (cmd == SND_COMPR_TRIGGER_PARTIAL_DRAIN ||
-		cmd == SND_COMPR_TRIGGER_DRAIN) {
-
-		for_each_rtdcom(fe, rtdcom) {
-			component = rtdcom->component;
-
-			if (!component->driver->compr_ops ||
-			    !component->driver->compr_ops->trigger)
-				continue;
-
-			ret = component->driver->compr_ops->trigger(cstream, cmd);
-			if (ret < 0)
-				return ret;
-		}
-		return ret;
-	}
+	    cmd == SND_COMPR_TRIGGER_DRAIN)
+		return soc_compr_components_trigger(cstream, cmd);
 
 	if (cstream->direction == SND_COMPRESS_PLAYBACK)
 		stream = SNDRV_PCM_STREAM_PLAYBACK;
@@ -435,17 +432,9 @@ static int soc_compr_trigger_fe(struct snd_compr_stream *cstream, int cmd)
 			goto out;
 	}
 
-	for_each_rtdcom(fe, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->trigger)
-			continue;
-
-		ret = component->driver->compr_ops->trigger(cstream, cmd);
-		if (ret < 0)
-			goto out;
-	}
+	ret = soc_compr_components_trigger(cstream, cmd);
+	if (ret < 0)
+		goto out;
 
 	fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE;
 
@@ -472,12 +461,33 @@ static int soc_compr_trigger_fe(struct snd_compr_stream *cstream, int cmd)
 	return ret;
 }
 
-static int soc_compr_set_params(struct snd_compr_stream *cstream,
-					struct snd_compr_params *params)
+static int soc_compr_components_set_params(struct snd_compr_stream *cstream,
+					   struct snd_compr_params *params)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
 	struct snd_soc_rtdcom_list *rtdcom;
+	int ret;
+
+	for_each_rtdcom(rtd, rtdcom) {
+		component = rtdcom->component;
+
+		if (!component->driver->compr_ops ||
+		    !component->driver->compr_ops->set_params)
+			continue;
+
+		ret = component->driver->compr_ops->set_params(cstream, params);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int soc_compr_set_params(struct snd_compr_stream *cstream,
+				struct snd_compr_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	int ret;
 
@@ -496,17 +506,9 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream,
 			goto err;
 	}
 
-	for_each_rtdcom(rtd, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->set_params)
-			continue;
-
-		ret = component->driver->compr_ops->set_params(cstream, params);
-		if (ret < 0)
-			goto err;
-	}
+	ret = soc_compr_components_set_params(cstream, params);
+	if (ret < 0)
+		goto err;
 
 	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->set_params) {
 		ret = rtd->dai_link->compr_ops->set_params(cstream);
@@ -540,8 +542,6 @@ static int soc_compr_set_params_fe(struct snd_compr_stream *cstream,
 	struct snd_soc_pcm_runtime *fe = cstream->private_data;
 	struct snd_pcm_substream *fe_substream =
 		 fe->pcm->streams[cstream->direction].substream;
-	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = fe->cpu_dai;
 	int ret, stream;
 
@@ -558,17 +558,9 @@ static int soc_compr_set_params_fe(struct snd_compr_stream *cstream,
 			goto out;
 	}
 
-	for_each_rtdcom(fe, rtdcom) {
-		component = rtdcom->component;
-
-		if (!component->driver->compr_ops ||
-		    !component->driver->compr_ops->set_params)
-			continue;
-
-		ret = component->driver->compr_ops->set_params(cstream, params);
-		if (ret < 0)
-			goto out;
-	}
+	ret = soc_compr_components_set_params(cstream, params);
+	if (ret < 0)
+		goto out;
 
 	if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->set_params) {
 		ret = fe->dai_link->compr_ops->set_params(cstream);
-- 
2.11.0

[PATCH 4/4] ASoC: compress: Fix up some trivial formatting issues

[Charles Keepax]

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 sound/soc/soc-compress.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 3d107652b7ca3..e661182716608 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -287,8 +287,8 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 	if (cstream->direction == SND_COMPRESS_PLAYBACK) {
 		if (snd_soc_runtime_ignore_pmdown_time(rtd)) {
 			snd_soc_dapm_stream_event(rtd,
-					SNDRV_PCM_STREAM_PLAYBACK,
-					SND_SOC_DAPM_STREAM_STOP);
+						  SNDRV_PCM_STREAM_PLAYBACK,
+						  SND_SOC_DAPM_STREAM_STOP);
 		} else {
 			rtd->pop_wait = 1;
 			queue_delayed_work(system_power_efficient_wq,
@@ -298,8 +298,8 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 	} else {
 		/* capture streams can be powered down now */
 		snd_soc_dapm_stream_event(rtd,
-			SNDRV_PCM_STREAM_CAPTURE,
-			SND_SOC_DAPM_STREAM_STOP);
+					  SNDRV_PCM_STREAM_CAPTURE,
+					  SND_SOC_DAPM_STREAM_STOP);
 	}
 
 	mutex_unlock(&rtd->pcm_mutex);
@@ -423,7 +423,6 @@ static int soc_compr_trigger_fe(struct snd_compr_stream *cstream, int cmd)
 	else
 		stream = SNDRV_PCM_STREAM_CAPTURE;
 
-
 	mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->trigger) {
@@ -518,10 +517,10 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream,
 
 	if (cstream->direction == SND_COMPRESS_PLAYBACK)
 		snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_PLAYBACK,
-					SND_SOC_DAPM_STREAM_START);
+					  SND_SOC_DAPM_STREAM_START);
 	else
 		snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_CAPTURE,
-					SND_SOC_DAPM_STREAM_START);
+					  SND_SOC_DAPM_STREAM_START);
 
 	/* cancel any delayed stream shutdown that is pending */
 	rtd->pop_wait = 0;
@@ -537,7 +536,7 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream,
 }
 
 static int soc_compr_set_params_fe(struct snd_compr_stream *cstream,
-					struct snd_compr_params *params)
+				   struct snd_compr_params *params)
 {
 	struct snd_soc_pcm_runtime *fe = cstream->private_data;
 	struct snd_pcm_substream *fe_substream =
@@ -596,7 +595,7 @@ static int soc_compr_set_params_fe(struct snd_compr_stream *cstream,
 }
 
 static int soc_compr_get_params(struct snd_compr_stream *cstream,
-					struct snd_codec *params)
+				struct snd_codec *params)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
@@ -629,7 +628,7 @@ static int soc_compr_get_params(struct snd_compr_stream *cstream,
 }
 
 static int soc_compr_get_caps(struct snd_compr_stream *cstream,
-				struct snd_compr_caps *caps)
+			      struct snd_compr_caps *caps)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
@@ -654,7 +653,7 @@ static int soc_compr_get_caps(struct snd_compr_stream *cstream,
 }
 
 static int soc_compr_get_codec_caps(struct snd_compr_stream *cstream,
-				struct snd_compr_codec_caps *codec)
+				    struct snd_compr_codec_caps *codec)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
@@ -713,7 +712,7 @@ static int soc_compr_ack(struct snd_compr_stream *cstream, size_t bytes)
 }
 
 static int soc_compr_pointer(struct snd_compr_stream *cstream,
-			struct snd_compr_tstamp *tstamp)
+			     struct snd_compr_tstamp *tstamp)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
@@ -767,7 +766,7 @@ static int soc_compr_copy(struct snd_compr_stream *cstream,
 }
 
 static int soc_compr_set_metadata(struct snd_compr_stream *cstream,
-				struct snd_compr_metadata *metadata)
+				  struct snd_compr_metadata *metadata)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
@@ -798,7 +797,7 @@ static int soc_compr_set_metadata(struct snd_compr_stream *cstream,
 }
 
 static int soc_compr_get_metadata(struct snd_compr_stream *cstream,
-				struct snd_compr_metadata *metadata)
+				  struct snd_compr_metadata *metadata)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
@@ -957,7 +956,6 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
 		break;
 	}
 
-
 	mutex_init(&compr->lock);
 	ret = snd_compress_new(rtd->card->snd_card, num, direction,
 				new_name, compr);
-- 
2.11.0

Applied "ASoC: compress: Fix up some trivial formatting issues" to the asoc tree

[Mark Brown]

The patch

   ASoC: compress: Fix up some trivial formatting issues

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 89027d9ebb5948392dd5a4ccc72d5a4eaa865537 Mon Sep 17 00:00:00 2001
From: Charles Keepax <ckeepax@opensource.cirrus.com>
Date: Thu, 26 Apr 2018 17:30:07 +0100
Subject: [PATCH] ASoC: compress: Fix up some trivial formatting issues

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-compress.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 62875c6a93a1..e095115fa9f9 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -287,8 +287,8 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 	if (cstream->direction == SND_COMPRESS_PLAYBACK) {
 		if (snd_soc_runtime_ignore_pmdown_time(rtd)) {
 			snd_soc_dapm_stream_event(rtd,
-					SNDRV_PCM_STREAM_PLAYBACK,
-					SND_SOC_DAPM_STREAM_STOP);
+						  SNDRV_PCM_STREAM_PLAYBACK,
+						  SND_SOC_DAPM_STREAM_STOP);
 		} else {
 			rtd->pop_wait = 1;
 			queue_delayed_work(system_power_efficient_wq,
@@ -298,8 +298,8 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 	} else {
 		/* capture streams can be powered down now */
 		snd_soc_dapm_stream_event(rtd,
-			SNDRV_PCM_STREAM_CAPTURE,
-			SND_SOC_DAPM_STREAM_STOP);
+					  SNDRV_PCM_STREAM_CAPTURE,
+					  SND_SOC_DAPM_STREAM_STOP);
 	}
 
 	mutex_unlock(&rtd->pcm_mutex);
@@ -428,7 +428,6 @@ static int soc_compr_trigger_fe(struct snd_compr_stream *cstream, int cmd)
 	else
 		stream = SNDRV_PCM_STREAM_CAPTURE;
 
-
 	mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME);
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->trigger) {
@@ -522,10 +521,10 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream,
 
 	if (cstream->direction == SND_COMPRESS_PLAYBACK)
 		snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_PLAYBACK,
-					SND_SOC_DAPM_STREAM_START);
+					  SND_SOC_DAPM_STREAM_START);
 	else
 		snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_CAPTURE,
-					SND_SOC_DAPM_STREAM_START);
+					  SND_SOC_DAPM_STREAM_START);
 
 	/* cancel any delayed stream shutdown that is pending */
 	rtd->pop_wait = 0;
@@ -541,7 +540,7 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream,
 }
 
 static int soc_compr_set_params_fe(struct snd_compr_stream *cstream,
-					struct snd_compr_params *params)
+				   struct snd_compr_params *params)
 {
 	struct snd_soc_pcm_runtime *fe = cstream->private_data;
 	struct snd_pcm_substream *fe_substream =
@@ -612,7 +611,7 @@ static int soc_compr_set_params_fe(struct snd_compr_stream *cstream,
 }
 
 static int soc_compr_get_params(struct snd_compr_stream *cstream,
-					struct snd_codec *params)
+				struct snd_codec *params)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
@@ -646,7 +645,7 @@ static int soc_compr_get_params(struct snd_compr_stream *cstream,
 }
 
 static int soc_compr_get_caps(struct snd_compr_stream *cstream,
-				struct snd_compr_caps *caps)
+			      struct snd_compr_caps *caps)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
@@ -672,7 +671,7 @@ static int soc_compr_get_caps(struct snd_compr_stream *cstream,
 }
 
 static int soc_compr_get_codec_caps(struct snd_compr_stream *cstream,
-				struct snd_compr_codec_caps *codec)
+				    struct snd_compr_codec_caps *codec)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
@@ -731,7 +730,7 @@ static int soc_compr_ack(struct snd_compr_stream *cstream, size_t bytes)
 }
 
 static int soc_compr_pointer(struct snd_compr_stream *cstream,
-			struct snd_compr_tstamp *tstamp)
+			     struct snd_compr_tstamp *tstamp)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
@@ -786,7 +785,7 @@ static int soc_compr_copy(struct snd_compr_stream *cstream,
 }
 
 static int soc_compr_set_metadata(struct snd_compr_stream *cstream,
-				struct snd_compr_metadata *metadata)
+				  struct snd_compr_metadata *metadata)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
@@ -816,7 +815,7 @@ static int soc_compr_set_metadata(struct snd_compr_stream *cstream,
 }
 
 static int soc_compr_get_metadata(struct snd_compr_stream *cstream,
-				struct snd_compr_metadata *metadata)
+				  struct snd_compr_metadata *metadata)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
@@ -976,7 +975,6 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
 		break;
 	}
 
-
 	mutex_init(&compr->lock);
 	ret = snd_compress_new(rtd->card->snd_card, num, direction,
 				new_name, compr);
-- 
2.17.0

Re: [PATCH 1/4] ASoC: compress: Only assign compr->ops->copy once

[Vinod Koul]

On Thu, Apr 26, 2018 at 05:30:04PM +0100, Charles Keepax wrote:
> There are only one set of ops on the compressed stream so no need to
> reassign the copy callback repeatedly, stop after copy is seen to be
> necessary.

Except 2:

Acked-by: Vinod Koul <vkoul@kernel.org>

-- 
~Vinod

Applied "ASoC: compress: Only assign compr->ops->copy once" to the asoc tree

[Mark Brown]

The patch

   ASoC: compress: Only assign compr->ops->copy once

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 ca76db6c7e9c4ab6a98fbe0f92f18bf1375e325d Mon Sep 17 00:00:00 2001
From: Charles Keepax <ckeepax@opensource.cirrus.com>
Date: Thu, 26 Apr 2018 17:30:04 +0100
Subject: [PATCH] ASoC: compress: Only assign compr->ops->copy once

There are only one set of ops on the compressed stream so no need to
reassign the copy callback repeatedly, stop after copy is seen to be
necessary.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-compress.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index ba56f87f96d4..62875c6a93a1 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -973,6 +973,7 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
 			continue;
 
 		compr->ops->copy = soc_compr_copy;
+		break;
 	}
 
 
-- 
2.17.0