[RFC PATCH 0/2] ASoC: add compress stream support

[RFC PATCH 0/2] ASoC: add compress stream support

[Vinod Koul]

Here is the much delayed support for compress audio streams in ASoC.
This patch series is in RFC form, once we have agreed on this I will send
the patches supporting Intel driver as well.

Mainly we indicate if the dai is of compressed nature and based on that
register a new compressed device. Also the platform driver adds the compress
stream operations.
Since the codecs handles PCM only, the assumption is that they will not have
anything to do here and machine needs to set codec appropriately for compressed
streams. In future if codec with such support are available then we can simply
add compressed operations into codec as well

Comments, criticism welcome...

--
Namarta Kohli (1):
  ASoC: add compress stream support

Vinod Koul (1):
  ASoC: add definations for compressed operations

 include/sound/compress_driver.h |    1 +
 include/sound/soc-dai.h         |    2 +
 include/sound/soc.h             |   17 ++-
 sound/soc/Makefile              |    2 +-
 sound/soc/soc-compress.c        |  297 +++++++++++++++++++++++++++++++++++++++
 sound/soc/soc-core.c            |   10 ++-
 6 files changed, 326 insertions(+), 3 deletions(-)
 create mode 100644 sound/soc/soc-compress.c

[RFC PATCH 1/2] ASoC: add definitions for compressed operations

[Vinod Koul]

Here we update the asoc structures to add compress stream definitions
First the struct snd_soc_dai_driver adds a new member to indicate if the dai is
compressed or pcm. Next we add a new structre the struct snd_soc_compr_ops in
the struct snd_soc_dai_link. This is to be used for machine driver to perform
any operations required for setting up compressed audio streams

next is the compressed data operations, they are added using struct
snd_compr_ops in the struct snd_soc_platform_driver.

Signed-off-by: Namarta Kohli <namartax.kohli@intel.com>
Signed-off-by: Ramesh Babu K V <ramesh.babu@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@linux.intel.com>
---
 include/sound/compress_driver.h |    1 +
 include/sound/soc-dai.h         |    2 ++
 include/sound/soc.h             |   17 ++++++++++++++++-
 3 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
index 48f2a1f..f2912ab 100644
--- a/include/sound/compress_driver.h
+++ b/include/sound/compress_driver.h
@@ -61,6 +61,7 @@ struct snd_compr_runtime {
 	u64 total_bytes_available;
 	u64 total_bytes_transferred;
 	wait_queue_head_t sleep;
+	void *private_data;
 };
 
 /**
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index 1f69e0a..a05218d 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -205,6 +205,8 @@ struct snd_soc_dai_driver {
 	int (*remove)(struct snd_soc_dai *dai);
 	int (*suspend)(struct snd_soc_dai *dai);
 	int (*resume)(struct snd_soc_dai *dai);
+	/* compress dai */
+	bool compress_dai;
 
 	/* ops */
 	const struct snd_soc_dai_ops *ops;
diff --git a/include/sound/soc.h b/include/sound/soc.h
index e063380..796b4cd 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -22,6 +22,7 @@
 #include <linux/regmap.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
+#include <sound/compress_driver.h>
 #include <sound/control.h>
 #include <sound/ac97_codec.h>
 
@@ -399,6 +400,7 @@ int snd_soc_platform_read(struct snd_soc_platform *platform,
 int snd_soc_platform_write(struct snd_soc_platform *platform,
 					unsigned int reg, unsigned int val);
 int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num);
+int soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num);
 
 struct snd_pcm_substream *snd_soc_get_dai_substream(struct snd_soc_card *card,
 		const char *dai_link, int stream);
@@ -632,6 +634,13 @@ struct snd_soc_ops {
 	int (*trigger)(struct snd_pcm_substream *, int);
 };
 
+struct snd_soc_compr_ops {
+	int (*startup)(struct snd_compr_stream *);
+	void (*shutdown)(struct snd_compr_stream *);
+	int (*set_params)(struct snd_compr_stream *);
+	int (*trigger)(struct snd_compr_stream *);
+};
+
 /* SoC cache ops */
 struct snd_soc_cache_ops {
 	const char *name;
@@ -787,9 +796,12 @@ struct snd_soc_platform_driver {
 	snd_pcm_sframes_t (*delay)(struct snd_pcm_substream *,
 		struct snd_soc_dai *);
 
-	/* platform stream ops */
+	/* platform stream pcm ops */
 	struct snd_pcm_ops *ops;
 
+	/* platform stream compress ops */
+	struct snd_compr_ops *compr_ops;
+
 	/* platform stream completion event */
 	int (*stream_event)(struct snd_soc_dapm_context *dapm, int event);
 
@@ -891,6 +903,7 @@ struct snd_soc_dai_link {
 
 	/* machine stream operations */
 	struct snd_soc_ops *ops;
+	struct snd_soc_compr_ops *compr_ops;
 };
 
 struct snd_soc_codec_conf {
@@ -1027,6 +1040,7 @@ struct snd_soc_pcm_runtime {
 
 	/* runtime devices */
 	struct snd_pcm *pcm;
+	struct snd_compr *compr;
 	struct snd_soc_codec *codec;
 	struct snd_soc_platform *platform;
 	struct snd_soc_dai *codec_dai;
@@ -1039,6 +1053,7 @@ struct snd_soc_pcm_runtime {
 #endif
 };
 
+
 /* mixer control */
 struct soc_mixer_control {
 	int min, max, platform_max;
-- 
1.7.0.4

[RFC PATCH 2/2] ASoC: add compress stream support

[Vinod Koul]

From: Namarta Kohli <namartax.kohli@intel.com>

This patch adds the support to parse the compress dai's and then also adds the
soc-compress.c file while handles the compress stream operations, mostly analogous 
to what is done in the soc-pcm.c and additional handling of the compress
operations

Signed-off-by: Namarta Kohli <namartax.kohli@intel.com>
Signed-off-by: Ramesh Babu K V <ramesh.babu@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@linux.intel.com>
---
 sound/soc/Makefile       |    2 +-
 sound/soc/soc-compress.c |  297 ++++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/soc-core.c     |   10 ++-
 3 files changed, 307 insertions(+), 2 deletions(-)
 create mode 100644 sound/soc/soc-compress.c

diff --git a/sound/soc/Makefile b/sound/soc/Makefile
index 00a555a..c126400 100644
--- a/sound/soc/Makefile
+++ b/sound/soc/Makefile
@@ -1,5 +1,5 @@
 snd-soc-core-objs := soc-core.o soc-dapm.o soc-jack.o soc-cache.o soc-utils.o
-snd-soc-core-objs += soc-pcm.o soc-io.o
+snd-soc-core-objs += soc-pcm.o soc-compress.o soc-io.o
 
 snd-soc-dmaengine-pcm-objs := soc-dmaengine-pcm.o
 obj-$(CONFIG_SND_SOC_DMAENGINE_PCM) += snd-soc-dmaengine-pcm.o
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
new file mode 100644
index 0000000..573f9bc
--- /dev/null
+++ b/sound/soc/soc-compress.c
@@ -0,0 +1,297 @@
+/*
+ * soc-compress.c  --  ALSA SoC Compress
+ *
+ * Copyright (C) 2012 Intel Corp.
+ *
+ * Authors: Namarta Kohli <namartax.kohli@intel.com>
+ *          Ramesh Babu K V <ramesh.babu@linux.intel.com>
+ *          Vinod Koul <vinod.koul@linux.intel.com>
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+#include <sound/core.h>
+#include <sound/compress_params.h>
+#include <sound/compress_driver.h>
+#include <sound/soc.h>
+#include <sound/initval.h>
+
+static int soc_compr_open(struct snd_compr_stream *cstream)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_platform *platform = rtd->platform;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	int ret = 0;
+
+	if (platform->driver->compr_ops && platform->driver->compr_ops->open) {
+		ret = platform->driver->compr_ops->open(cstream);
+		if (ret < 0) {
+			printk(KERN_ERR "asoc: can't open platform %s\n", platform->name);
+			goto out;
+		}
+	}
+
+	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->startup) {
+		ret = rtd->dai_link->compr_ops->startup(cstream);
+		if (ret < 0) {
+			printk(KERN_ERR "asoc: %s startup failed\n", rtd->dai_link->name);
+			goto machine_err;
+		}
+	}
+
+	if (cstream->direction == SND_COMPRESS_PLAYBACK) {
+		cpu_dai->playback_active++;
+		codec_dai->playback_active++;
+	} else {
+		cpu_dai->capture_active++;
+		codec_dai->capture_active++;
+	}
+	cpu_dai->active++;
+	codec_dai->active++;
+	rtd->codec->active++;
+
+	return 0;
+
+machine_err:
+	if (platform->driver->compr_ops && platform->driver->compr_ops->free)
+		platform->driver->compr_ops->free(cstream);
+out:
+	return ret;
+}
+
+static int soc_compr_free(struct snd_compr_stream *cstream)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_platform *platform = rtd->platform;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	struct snd_soc_codec *codec = rtd->codec;
+
+	if (cstream->direction == SND_COMPRESS_PLAYBACK) {
+		cpu_dai->playback_active--;
+		codec_dai->playback_active--;
+	} else {
+		cpu_dai->capture_active--;
+		codec_dai->capture_active--;
+	}
+
+	cpu_dai->active--;
+	codec_dai->active--;
+	codec->active--;
+
+	if (!cpu_dai->active)
+		cpu_dai->rate = 0;
+
+	if (!codec_dai->active)
+		codec_dai->rate = 0;
+
+	/* Muting the DAC suppresses artifacts caused during digital
+	 * shutdown, for example from stopping clocks.
+	 */
+	if (cstream->direction == SND_COMPRESS_PLAYBACK)
+		snd_soc_dai_digital_mute(codec_dai, 1);
+
+	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->shutdown)
+		rtd->dai_link->compr_ops->shutdown(cstream);
+
+	if (platform->driver->compr_ops && platform->driver->compr_ops->free)
+		platform->driver->compr_ops->free(cstream);
+		cpu_dai->runtime = NULL;
+
+	if (cstream->direction == SND_COMPRESS_PLAYBACK) {
+		if (!rtd->pmdown_time || codec->ignore_pmdown_time ||
+		    rtd->dai_link->ignore_pmdown_time) {
+			snd_soc_dapm_stream_event(rtd,
+					SNDRV_PCM_STREAM_PLAYBACK,
+					SND_SOC_DAPM_STREAM_STOP);
+		} else
+			codec_dai->pop_wait = 1;
+			schedule_delayed_work(&rtd->delayed_work,
+				msecs_to_jiffies(rtd->pmdown_time));
+	} else {
+		/* capture streams can be powered down now */
+		snd_soc_dapm_stream_event(rtd,
+			SNDRV_PCM_STREAM_CAPTURE,
+			SND_SOC_DAPM_STREAM_STOP);
+	}
+
+	mutex_unlock(&rtd->pcm_mutex);
+	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_platform *platform = rtd->platform;
+	int ret = 0;
+
+	if (platform->driver->compr_ops && platform->driver->compr_ops->trigger) {
+		ret = platform->driver->compr_ops->trigger(cstream, cmd);
+		if (ret < 0)
+			return ret;
+	}
+	return ret;
+}
+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_platform *platform = rtd->platform;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	int ret = 0;
+
+	/* first we call set_params for the platform driver
+	 * this should configure the soc side
+	 * if the machine has compressed ops then we call that as well
+	 * expectation is that platform and machine will configure everything
+	 * for this compress path, like configuring pcm port for codec
+	 */
+	if (platform->driver->compr_ops && platform->driver->compr_ops->set_params) {
+		ret = platform->driver->compr_ops->set_params(cstream, params);
+		if (ret < 0)
+			return ret;
+	}
+	if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->set_params) {
+		ret = rtd->dai_link->compr_ops->set_params(cstream);
+		if (ret < 0)
+			return ret;
+	}
+
+	snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_PLAYBACK,
+				SND_SOC_DAPM_STREAM_START);
+
+	snd_soc_dai_digital_mute(codec_dai, 0);
+	return ret;
+}
+
+int soc_compr_get_params(struct snd_compr_stream *cstream,
+					struct snd_codec *params)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_platform *platform = rtd->platform;
+	int ret = 0;
+
+	if (platform->driver->compr_ops && platform->driver->compr_ops->get_params) {
+		ret = platform->driver->compr_ops->get_params(cstream, params);
+		if (ret < 0)
+			return ret;
+	}
+	return ret;
+}
+
+int soc_compr_get_caps(struct snd_compr_stream *cstream,
+				struct snd_compr_caps *caps)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_platform *platform = rtd->platform;
+	int ret = 0;
+
+	if (platform->driver->compr_ops && platform->driver->compr_ops->get_caps) {
+		ret = platform->driver->compr_ops->get_caps(cstream, caps);
+		if (ret < 0)
+			return ret;
+	}
+	return ret;
+}
+
+int soc_compr_get_codec_caps(struct snd_compr_stream *cstream,
+				struct snd_compr_codec_caps *codec)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_platform *platform = rtd->platform;
+	int ret = 0;
+
+	if (platform->driver->compr_ops && platform->driver->compr_ops->get_codec_caps)
+		ret = platform->driver->compr_ops->get_codec_caps(cstream, codec);
+		if (ret < 0)
+			return ret;
+	return ret;
+}
+
+int soc_compr_ack(struct snd_compr_stream *cstream, size_t bytes)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_platform *platform = rtd->platform;
+	int ret = 0;
+
+	if (platform->driver->compr_ops && platform->driver->compr_ops->ack)
+		ret = platform->driver->compr_ops->ack(cstream, bytes);
+		if (ret < 0)
+			return ret;
+
+	return ret;
+}
+
+int soc_compr_pointer(struct snd_compr_stream *cstream,
+			struct snd_compr_tstamp *tstamp)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_platform *platform = rtd->platform;
+
+	if (platform->driver->compr_ops && platform->driver->compr_ops->pointer)
+		platform->driver->compr_ops->pointer(cstream, tstamp);
+
+	return 0;
+}
+
+/* ASoC Compress operations */
+static struct snd_compr_ops soc_compr_ops = {
+	.open		= soc_compr_open,
+	.free		= soc_compr_free,
+	.set_params	= soc_compr_set_params,
+	.get_params	= soc_compr_get_params,
+	.trigger	= soc_compr_trigger,
+	.pointer	= soc_compr_pointer,
+	.ack		= soc_compr_ack,
+	.get_caps	= soc_compr_get_caps,
+	.get_codec_caps = soc_compr_get_codec_caps
+};
+
+/* create a new compress */
+int soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
+{
+	struct snd_soc_codec *codec = rtd->codec;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	struct snd_compr *compr;
+	char new_name[64];
+	int ret = 0, direction = 0;
+
+	/* check client and interface hw capabilities */
+	snprintf(new_name, sizeof(new_name), "%s %s-%d",
+			rtd->dai_link->stream_name, codec_dai->name, num);
+	/* TODO add support for cature */
+	direction = SND_COMPRESS_PLAYBACK;
+	compr = kzalloc(sizeof(*compr), GFP_KERNEL);
+	if (compr == NULL) {
+		snd_printk(KERN_ERR "Cannot allocate compr\n");
+		return -ENOMEM;
+	}
+
+	compr->ops = &soc_compr_ops;
+	mutex_init(&compr->lock);
+	ret = snd_compress_new(rtd->card->snd_card, num, direction, compr);
+	if (ret < 0) {
+		printk(KERN_ERR "asoc: can't create pcm for codec %s\n", codec->name);
+		kfree(compr);
+		return ret;
+	}
+
+	rtd->compr = compr;
+	compr->private_data = rtd;
+
+	printk(KERN_INFO "asoc: %s <-> %s mapping ok\n", codec_dai->name,
+		cpu_dai->name);
+	return ret;
+}
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index f219b2f..492275b 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1388,7 +1388,15 @@ static int soc_probe_link_dais(struct snd_soc_card *card, int num, int order)
 	if (ret < 0)
 		pr_warn("asoc: failed to add pmdown_time sysfs:%d\n", ret);
 
-	if (!dai_link->params) {
+	if (cpu_dai->driver->compress_dai) {
+		/*create compress device*/
+		ret = soc_new_compress(rtd, num);
+		if (ret < 0 ) {
+			printk(KERN_ERR "asoc: can't create offload %s\n",
+					 dai_link->stream_name);
+			return ret;
+		}
+	} else if (!dai_link->params) {
 		/* create the pcm */
 		ret = soc_new_pcm(rtd, num);
 		if (ret < 0) {
-- 
1.7.0.4

Re: [RFC PATCH 2/2] ASoC: add compress stream support

[Mark Brown]

On Tue, Jul 31, 2012 at 12:11:40PM +0530, Vinod Koul wrote:
> From: Namarta Kohli <namartax.kohli@intel.com>
> 
> This patch adds the support to parse the compress dai's and then also adds the
> soc-compress.c file while handles the compress stream operations, mostly analogous 
> to what is done in the soc-pcm.c and additional handling of the compress
> operations

This is all mostly fine, though it's very sad that there's so much code
duplication between this and the regular PCM streams.  Some smallish
comments below.

> +	if (cstream->direction == SND_COMPRESS_PLAYBACK) {
> +		cpu_dai->playback_active++;
> +		codec_dai->playback_active++;
> +	} else {
> +		cpu_dai->capture_active++;
> +		codec_dai->capture_active++;
> +	}
> +	cpu_dai->active++;
> +	codec_dai->active++;

In quite a few places in the code there's missing blanks between blocks
of code.

> +int soc_compr_set_params(struct snd_compr_stream *cstream,
> +					struct snd_compr_params *params)
> +{

> +	snd_soc_dai_digital_mute(codec_dai, 0);

This looks wrong - the unmute should be associated with the start of the
data transfer rather than the configuration.  But we don't have any
other ops...  can we have one, or a non-atomic trigger?

> +	/* check client and interface hw capabilities */
> +	snprintf(new_name, sizeof(new_name), "%s %s-%d",
> +			rtd->dai_link->stream_name, codec_dai->name, num);
> +	/* TODO add support for cature */

Meow :)

> +	printk(KERN_INFO "asoc: %s <-> %s mapping ok\n", codec_dai->name,
> +		cpu_dai->name);

Say it's a compressed mapping?

> +	if (cpu_dai->driver->compress_dai) {
> +		/*create compress device*/
> +		ret = soc_new_compress(rtd, num);
> +		if (ret < 0 ) {
> +			printk(KERN_ERR "asoc: can't create offload %s\n",
> +					 dai_link->stream_name);
> +			return ret;
> +		}

Should we be complaining here if there's PCM operations too?
Alternatively, should we support mixed PCM and compressed DAIs (possibly
be creating two pcms from a single dai_link)?

Re: [RFC PATCH 2/2] ASoC: add compress stream support

[Takashi Iwai]

At Thu, 2 Aug 2012 19:09:27 +0100,
Mark Brown wrote:
> 
> On Tue, Jul 31, 2012 at 12:11:40PM +0530, Vinod Koul wrote:
> > From: Namarta Kohli <namartax.kohli@intel.com>
> > 
> > This patch adds the support to parse the compress dai's and then also adds the
> > soc-compress.c file while handles the compress stream operations, mostly analogous 
> > to what is done in the soc-pcm.c and additional handling of the compress
> > operations
> 
> This is all mostly fine, though it's very sad that there's so much code
> duplication between this and the regular PCM streams.

Yeah, it's a generic issue in the compress API, too.  The design is a
kind of fork from PCM.  We can consider a bit more consolidation in
the core part as well in future.


Takashi

Re: [RFC PATCH 2/2] ASoC: add compress stream support

[Vinod Koul]

On Thu, 2012-08-02 at 19:09 +0100, Mark Brown wrote:
> > +int soc_compr_set_params(struct snd_compr_stream *cstream,
> > +					struct snd_compr_params *params)
> > +{
> 
> > +	snd_soc_dai_digital_mute(codec_dai, 0);
> 
> This looks wrong - the unmute should be associated with the start of the
> data transfer rather than the configuration.  But we don't have any
> other ops...  can we have one, or a non-atomic trigger?
Sure we discussed this, but wasnt too sure. Will change to trigger, it
seem the correct way.

> 
> > +	/* check client and interface hw capabilities */
> > +	snprintf(new_name, sizeof(new_name), "%s %s-%d",
> > +			rtd->dai_link->stream_name, codec_dai->name, num);
> > +	/* TODO add support for cature */
> 
> Meow :)
:D, some trvial ocde was still misisng, rest of the code should work for
both although havent been able to test that part.
> 
> > +	printk(KERN_INFO "asoc: %s <-> %s mapping ok\n", codec_dai->name,
> > +		cpu_dai->name);
> 
> Say it's a compressed mapping?
> 
> > +	if (cpu_dai->driver->compress_dai) {
> > +		/*create compress device*/
> > +		ret = soc_new_compress(rtd, num);
> > +		if (ret < 0 ) {
> > +			printk(KERN_ERR "asoc: can't create offload %s\n",
> > +					 dai_link->stream_name);
> > +			return ret;
> > +		}
> 
> Should we be complaining here if there's PCM operations too?
I think so. A device should be either PCM or compress.

> Alternatively, should we support mixed PCM and compressed DAIs (possibly
> be creating two pcms from a single dai_link)?
I can't perceive why that should be done :)

-- 
~Vinod

Re: [RFC PATCH 2/2] ASoC: add compress stream support

[Takashi Iwai]

At Thu, 2 Aug 2012 19:09:27 +0100,
Mark Brown wrote:
> 
> > +int soc_compr_set_params(struct snd_compr_stream *cstream,
> > +					struct snd_compr_params *params)
> > +{
> 
> > +	snd_soc_dai_digital_mute(codec_dai, 0);
> 
> This looks wrong - the unmute should be associated with the start of the
> data transfer rather than the configuration.  But we don't have any
> other ops...  can we have one, or a non-atomic trigger?

I've been thinking of the possibility for a long time.
The problem is how to handle the trigger(STOP) gracefully.
Maybe creating a dedicated work struct would be required.
Also it'd require a new schedulable lock for the substream on the top
of snd_pcm_stream_lock*().  But before adding it, we should revisit
the current lock mess in the code...


Takashi

Re: [RFC PATCH 2/2] ASoC: add compress stream support

[Mark Brown]

On Mon, Aug 06, 2012 at 10:01:32AM +0200, Takashi Iwai wrote:

> of snd_pcm_stream_lock*().  But before adding it, we should revisit
> the current lock mess in the code...

Yes, simplifying the locking scheme would be really helpful all round.

Re: [RFC PATCH 2/2] ASoC: add compress stream support

[Vinod Koul]

On Tue, 2012-07-31 at 12:11 +0530, Vinod Koul wrote:
> diff --git a/sound/soc/Makefile b/sound/soc/Makefile
> index 00a555a..c126400 100644
> --- a/sound/soc/Makefile
> +++ b/sound/soc/Makefile
> @@ -1,5 +1,5 @@
>  snd-soc-core-objs := soc-core.o soc-dapm.o soc-jack.o soc-cache.o
> soc-utils.o
> -snd-soc-core-objs += soc-pcm.o soc-io.o
> +snd-soc-core-objs += soc-pcm.o soc-compress.o soc-io.o 
Mark,

I am planning to add a symbol, SND_SOC_COMPRESS_OFFLOAD and compile this
file and other changes under this flag.
This will select SND_COMPRESS_OFFLOAD and users should select this
symbol when using compressed offload...

Let me know if its fine and will send patch for this.

Thanks
-- 
~Vinod

Re: [RFC PATCH 2/2] ASoC: add compress stream support

[Mark Brown]

On Mon, Aug 06, 2012 at 11:02:01AM +0530, Vinod Koul wrote:
> On Tue, 2012-07-31 at 12:11 +0530, Vinod Koul wrote:

> > -snd-soc-core-objs += soc-pcm.o soc-io.o
> > +snd-soc-core-objs += soc-pcm.o soc-compress.o soc-io.o 

> I am planning to add a symbol, SND_SOC_COMPRESS_OFFLOAD and compile this
> file and other changes under this flag.
> This will select SND_COMPRESS_OFFLOAD and users should select this
> symbol when using compressed offload...

> Let me know if its fine and will send patch for this.

To be honest given the code for compressed stuff is so small in
comparison with the size of ASoC that I'm not sure it's worth bothering,
we should probably just enable it unconditionally with ASoC. 

Re: [RFC PATCH 2/2] ASoC: add compress stream support

[Vinod Koul]

On Mon, 2012-08-06 at 13:59 +0100, Mark Brown wrote:
> On Mon, Aug 06, 2012 at 11:02:01AM +0530, Vinod Koul wrote:
> > On Tue, 2012-07-31 at 12:11 +0530, Vinod Koul wrote:
> 
> > > -snd-soc-core-objs += soc-pcm.o soc-io.o
> > > +snd-soc-core-objs += soc-pcm.o soc-compress.o soc-io.o 
> 
> > I am planning to add a symbol, SND_SOC_COMPRESS_OFFLOAD and compile this
> > file and other changes under this flag.
> > This will select SND_COMPRESS_OFFLOAD and users should select this
> > symbol when using compressed offload...
> 
> > Let me know if its fine and will send patch for this.
> 
> To be honest given the code for compressed stuff is so small in
> comparison with the size of ASoC that I'm not sure it's worth bothering,
> we should probably just enable it unconditionally with ASoC. 
Okay, so select SND_COMPRESS_OFFLOAD it is in SND_SOC :)

-- 
~Vinod

Re: [RFC PATCH 0/2] ASoC: add compress stream support

[Mark Brown]

On Tue, Jul 31, 2012 at 12:11:38PM +0530, Vinod Koul wrote:
> Here is the much delayed support for compress audio streams in ASoC.
> This patch series is in RFC form, once we have agreed on this I will send
> the patches supporting Intel driver as well.

These should probably be a single patch.