[PATCH 0/7] Add capture support to compress API

[PATCH 0/7] Add capture support to compress API

[Richard Fitzgerald]

This chain of patches does some clearing up and add capture
support to the compressed API.

Missing is support for capture streams that don't use the
copy callback. Also I have included two patches at the end
of the chain that remove the buffer pointers and just use
the cumulative values however as our setup uses the copy
callback these are untested past build so should be taken
with caution.

Thanks,
Charles

Thess patches apply to:
  git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
  branch "for-next"

based on top of commit
  126825e ALSA: snd-usb: add quirks handler for DSD streams

- RichF

Charles Keepax (7):
  ALSA: compress_core: Update calc_avail to use cumulative values
  ALSA: compress_core: Calculate avail correctly for capture streams
  ALSA: compress_core: Deconstify copy callback buffer
  ALSA: compress_core: Add support for capture streams
  ASoC: soc-compress: Deduce stream direction
  ALSA: compress_core: Remove unused hw_pointer
  ALSA: compress_core: Rework writes to use cumulative values

 include/sound/compress_driver.h |    4 +-
 sound/core/compress_offload.c   |  117 +++++++++++++++++++++++++-------------
 sound/soc/soc-compress.c        |   11 +++-
 3 files changed, 87 insertions(+), 45 deletions(-)

-- 
1.7.2.5

[alsa-devel] [PATCH 1/7] ALSA: compress_core: Update calc_avail to use cumulative values

[Charles Keepax]

The app_pointer is managed locally by the compress core for memory
mapped DSPs but for DSPs that are not memory mapped this would have to
be manually updated from within the DSP driver itself, which is hardly
very idiomatic.

This patch switches to using the cumulative values to calculate the
available buffer space because these are already gracefully passed out
of the DSP driver to the compress core and otherwise should be
functionally equivalent.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
---
 sound/core/compress_offload.c |   23 +++++------------------
 1 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index c84abc8..27bd81a 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -160,8 +160,6 @@ static int snd_compr_update_tstamp(struct snd_compr_stream *stream,
 static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
 		struct snd_compr_avail *avail)
 {
-	long avail_calc; /*this needs to be signed variable */
-
 	memset(avail, 0, sizeof(*avail));
 	snd_compr_update_tstamp(stream, &avail->tstamp);
 	/* Still need to return avail even if tstamp can't be filled in */
@@ -184,22 +182,11 @@ static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
 		return stream->runtime->buffer_size;
 	}
 
-	/* FIXME: this routine isn't consistent, in one test we use
-	 * cumulative values and in the other byte offsets. Do we
-	 * really need the byte offsets if the cumulative values have
-	 * been updated? In the PCM interface app_ptr and hw_ptr are
-	 * already cumulative */
-
-	avail_calc = stream->runtime->buffer_size -
-		(stream->runtime->app_pointer - stream->runtime->hw_pointer);
-	pr_debug("calc avail as %ld, app_ptr %lld, hw+ptr %lld\n", avail_calc,
-			stream->runtime->app_pointer,
-			stream->runtime->hw_pointer);
-	if (avail_calc >= stream->runtime->buffer_size)
-		avail_calc -= stream->runtime->buffer_size;
-	pr_debug("ret avail as %ld\n", avail_calc);
-	avail->avail = avail_calc;
-	return avail_calc;
+	avail->avail = stream->runtime->buffer_size -
+		(stream->runtime->total_bytes_available -
+		 stream->runtime->total_bytes_transferred);
+	pr_debug("ret avail as %lld\n", avail->avail);
+	return avail->avail;
 }
 
 static inline size_t snd_compr_get_avail(struct snd_compr_stream *stream)
-- 
1.7.2.5

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 2/7] ALSA: compress_core: Calculate avail correctly for capture streams

[Charles Keepax]

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
---
 sound/core/compress_offload.c |   29 ++++++++++++++++++-----------
 1 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index 27bd81a..1f69863 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -153,7 +153,10 @@ static int snd_compr_update_tstamp(struct snd_compr_stream *stream,
 	pr_debug("dsp consumed till %d total %d bytes\n",
 		tstamp->byte_offset, tstamp->copied_total);
 	stream->runtime->hw_pointer = tstamp->byte_offset;
-	stream->runtime->total_bytes_transferred = tstamp->copied_total;
+	if (stream->direction == SND_COMPRESS_PLAYBACK)
+		stream->runtime->total_bytes_transferred = tstamp->copied_total;
+	else
+		stream->runtime->total_bytes_available = tstamp->copied_total;
 	return 0;
 }
 
@@ -164,12 +167,9 @@ static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
 	snd_compr_update_tstamp(stream, &avail->tstamp);
 	/* Still need to return avail even if tstamp can't be filled in */
 
-	/* FIXME: This needs to be different for capture stream,
-	   available is # of compressed data, for playback it's
-	   remainder of buffer */
-
 	if (stream->runtime->total_bytes_available == 0 &&
-			stream->runtime->state == SNDRV_PCM_STATE_SETUP) {
+			stream->runtime->state == SNDRV_PCM_STATE_SETUP &&
+			stream->direction == SND_COMPRESS_PLAYBACK) {
 		pr_debug("detected init and someone forgot to do a write\n");
 		return stream->runtime->buffer_size;
 	}
@@ -178,13 +178,20 @@ static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
 			stream->runtime->total_bytes_transferred);
 	if (stream->runtime->total_bytes_available ==
 				stream->runtime->total_bytes_transferred) {
-		pr_debug("both pointers are same, returning full avail\n");
-		return stream->runtime->buffer_size;
+		if (stream->direction == SND_COMPRESS_PLAYBACK) {
+			pr_debug("both pointers are same, returning full avail\n");
+			return stream->runtime->buffer_size;
+		} else {
+			pr_debug("both pointers are same, returning no avail\n");
+			return 0;
+		}
 	}
 
-	avail->avail = stream->runtime->buffer_size -
-		(stream->runtime->total_bytes_available -
-		 stream->runtime->total_bytes_transferred);
+	avail->avail = stream->runtime->total_bytes_available -
+			stream->runtime->total_bytes_transferred;
+	if (stream->direction == SND_COMPRESS_PLAYBACK)
+		avail->avail = stream->runtime->buffer_size - avail->avail;
+
 	pr_debug("ret avail as %lld\n", avail->avail);
 	return avail->avail;
 }
-- 
1.7.2.5

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 3/7] ALSA: compress_core: Deconstify copy callback buffer

[Charles Keepax]

The buffer passed to the copy callback should not be const because the
copy callback can be used for capture and playback.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
---
 include/sound/compress_driver.h |    2 +-
 sound/core/compress_offload.c   |    8 +++++---
 sound/soc/soc-compress.c        |    2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
index ff6c741..db8273a 100644
--- a/include/sound/compress_driver.h
+++ b/include/sound/compress_driver.h
@@ -121,7 +121,7 @@ struct snd_compr_ops {
 	int (*trigger)(struct snd_compr_stream *stream, int cmd);
 	int (*pointer)(struct snd_compr_stream *stream,
 			struct snd_compr_tstamp *tstamp);
-	int (*copy)(struct snd_compr_stream *stream, const char __user *buf,
+	int (*copy)(struct snd_compr_stream *stream, char __user *buf,
 		       size_t count);
 	int (*mmap)(struct snd_compr_stream *stream,
 			struct vm_area_struct *vma);
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index 1f69863..52ca4cc 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -272,10 +272,12 @@ static ssize_t snd_compr_write(struct file *f, const char __user *buf,
 	if (avail > count)
 		avail = count;
 
-	if (stream->ops->copy)
-		retval = stream->ops->copy(stream, buf, avail);
-	else
+	if (stream->ops->copy) {
+		char __user* cbuf = (char __user*)buf;
+		retval = stream->ops->copy(stream, cbuf, avail);
+	} else {
 		retval = snd_compr_write_data(stream, buf, avail);
+	}
 	if (retval > 0)
 		stream->runtime->total_bytes_available += retval;
 
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 29093a3..da83e56 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -315,7 +315,7 @@ static int soc_compr_pointer(struct snd_compr_stream *cstream,
 }
 
 static int soc_compr_copy(struct snd_compr_stream *cstream,
-			  const char __user *buf, size_t count)
+			  char __user *buf, size_t count)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_platform *platform = rtd->platform;
-- 
1.7.2.5

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 4/7] ALSA: compress_core: Add support for capture streams

[Charles Keepax]

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
---
 sound/core/compress_offload.c |   43 +++++++++++++++++++++++++++++++++++++++-
 1 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index 52ca4cc..52a2765 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -296,7 +296,41 @@ static ssize_t snd_compr_write(struct file *f, const char __user *buf,
 static ssize_t snd_compr_read(struct file *f, char __user *buf,
 		size_t count, loff_t *offset)
 {
-	return -ENXIO;
+	struct snd_compr_file *data = f->private_data;
+	struct snd_compr_stream *stream;
+	size_t avail;
+	int retval;
+
+	if (snd_BUG_ON(!data))
+		return -EFAULT;
+
+	stream = &data->stream;
+	mutex_lock(&stream->device->lock);
+
+	/* read is allowed when stream is running */
+	if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING) {
+		retval = -EBADFD;
+		goto out;
+	}
+
+	avail = snd_compr_get_avail(stream);
+	pr_debug("avail returned %ld\n", (unsigned long)avail);
+	/* calculate how much we can read from buffer */
+	if (avail > count)
+		avail = count;
+
+	if (stream->ops->copy) {
+		retval = stream->ops->copy(stream, buf, avail);
+	} else {
+		retval = -ENXIO;
+		goto out;
+	}
+	if (retval > 0)
+		stream->runtime->total_bytes_transferred += retval;
+
+out:
+	mutex_unlock(&stream->device->lock);
+	return retval;
 }
 
 static int snd_compr_mmap(struct file *f, struct vm_area_struct *vma)
@@ -481,9 +515,14 @@ snd_compr_set_params(struct snd_compr_stream *stream, unsigned long arg)
 		retval = stream->ops->set_params(stream, params);
 		if (retval)
 			goto out;
-		stream->runtime->state = SNDRV_PCM_STATE_SETUP;
+
 		stream->metadata_set = false;
 		stream->next_track = false;
+
+		if (stream->direction == SND_COMPRESS_PLAYBACK)
+			stream->runtime->state = SNDRV_PCM_STATE_SETUP;
+		else
+			stream->runtime->state = SNDRV_PCM_STATE_PREPARED;
 	} else {
 		return -EPERM;
 	}
-- 
1.7.2.5

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 5/7] ASoC: soc-compress: Deduce stream direction

[Charles Keepax]

Previously we just hard coded all streams as playback streams, this
patch checks the DAI to see if it is a capture or playback stream. It is
worth noting that at this time only unidirectional streams are
supported.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
---
 sound/soc/soc-compress.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index da83e56..3853f7e 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -384,7 +384,14 @@ int soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
 	/* check client and interface hw capabilities */
 	snprintf(new_name, sizeof(new_name), "%s %s-%d",
 			rtd->dai_link->stream_name, codec_dai->name, num);
-	direction = SND_COMPRESS_PLAYBACK;
+
+	if (codec_dai->driver->playback.channels_min)
+		direction = SND_COMPRESS_PLAYBACK;
+	else if (codec_dai->driver->capture.channels_min)
+		direction = SND_COMPRESS_CAPTURE;
+	else
+		return -EINVAL;
+
 	compr = kzalloc(sizeof(*compr), GFP_KERNEL);
 	if (compr == NULL) {
 		snd_printk(KERN_ERR "Cannot allocate compr\n");
-- 
1.7.2.5

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 6/7] ALSA: compress_core: Remove unused hw_pointer

[Charles Keepax]

Only tested as far as build.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
---
 include/sound/compress_driver.h |    1 -
 sound/core/compress_offload.c   |    2 --
 2 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
index db8273a..2d7de96 100644
--- a/include/sound/compress_driver.h
+++ b/include/sound/compress_driver.h
@@ -56,7 +56,6 @@ struct snd_compr_runtime {
 	u64 buffer_size;
 	u32 fragment_size;
 	u32 fragments;
-	u64 hw_pointer;
 	u64 app_pointer;
 	u64 total_bytes_available;
 	u64 total_bytes_transferred;
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index 52a2765..36d7688 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -152,7 +152,6 @@ static int snd_compr_update_tstamp(struct snd_compr_stream *stream,
 	stream->ops->pointer(stream, tstamp);
 	pr_debug("dsp consumed till %d total %d bytes\n",
 		tstamp->byte_offset, tstamp->copied_total);
-	stream->runtime->hw_pointer = tstamp->byte_offset;
 	if (stream->direction == SND_COMPRESS_PLAYBACK)
 		stream->runtime->total_bytes_transferred = tstamp->copied_total;
 	else
@@ -657,7 +656,6 @@ static int snd_compr_stop(struct snd_compr_stream *stream)
 	if (!retval) {
 		stream->runtime->state = SNDRV_PCM_STATE_SETUP;
 		wake_up(&stream->runtime->sleep);
-		stream->runtime->hw_pointer = 0;
 		stream->runtime->app_pointer = 0;
 		stream->runtime->total_bytes_available = 0;
 		stream->runtime->total_bytes_transferred = 0;
-- 
1.7.2.5

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 7/7] ALSA: compress_core: Rework writes to use cumulative values

[Charles Keepax]

This patch reworks the writes to use cumulative values thus making the
app_pointer unecessary and removing it.

Only tested as far as build.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
---
 include/sound/compress_driver.h |    1 -
 sound/core/compress_offload.c   |   18 +++++++++++-------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
index 2d7de96..9031a26 100644
--- a/include/sound/compress_driver.h
+++ b/include/sound/compress_driver.h
@@ -56,7 +56,6 @@ struct snd_compr_runtime {
 	u64 buffer_size;
 	u32 fragment_size;
 	u32 fragments;
-	u64 app_pointer;
 	u64 total_bytes_available;
 	u64 total_bytes_transferred;
 	wait_queue_head_t sleep;
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index 36d7688..7941ace 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -28,11 +28,13 @@
 #include <linux/file.h>
 #include <linux/fs.h>
 #include <linux/list.h>
+#include <linux/math64.h>
 #include <linux/mm.h>
 #include <linux/mutex.h>
 #include <linux/poll.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
+#include <linux/types.h>
 #include <linux/uio.h>
 #include <linux/uaccess.h>
 #include <linux/module.h>
@@ -223,21 +225,24 @@ static int snd_compr_write_data(struct snd_compr_stream *stream,
 	void *dstn;
 	size_t copy;
 	struct snd_compr_runtime *runtime = stream->runtime;
+	/* 64-bit Modulus */
+	u64 app_pointer = div64_u64(runtime->total_bytes_available,
+				    runtime->buffer_size);
+	app_pointer = runtime->total_bytes_available -
+		      (app_pointer * runtime->buffer_size);
 
-	dstn = runtime->buffer + runtime->app_pointer;
+	dstn = runtime->buffer + app_pointer;
 	pr_debug("copying %ld at %lld\n",
-			(unsigned long)count, runtime->app_pointer);
-	if (count < runtime->buffer_size - runtime->app_pointer) {
+			(unsigned long)count, app_pointer);
+	if (count < runtime->buffer_size - app_pointer) {
 		if (copy_from_user(dstn, buf, count))
 			return -EFAULT;
-		runtime->app_pointer += count;
 	} else {
-		copy = runtime->buffer_size - runtime->app_pointer;
+		copy = runtime->buffer_size - app_pointer;
 		if (copy_from_user(dstn, buf, copy))
 			return -EFAULT;
 		if (copy_from_user(runtime->buffer, buf + copy, count - copy))
 			return -EFAULT;
-		runtime->app_pointer = count - copy;
 	}
 	/* if DSP cares, let it know data has been written */
 	if (stream->ops->ack)
@@ -656,7 +661,6 @@ static int snd_compr_stop(struct snd_compr_stream *stream)
 	if (!retval) {
 		stream->runtime->state = SNDRV_PCM_STATE_SETUP;
 		wake_up(&stream->runtime->sleep);
-		stream->runtime->app_pointer = 0;
 		stream->runtime->total_bytes_available = 0;
 		stream->runtime->total_bytes_transferred = 0;
 	}
-- 
1.7.2.5

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 0/7] Add capture support to compress API

[Leanne Girdwood]

REMOVE THIS ADDRESS FROM THIS MAILING LIST PLEASE


On 18 April 2013 19:58, Richard Fitzgerald
<rf@opensource.wolfsonmicro.com>wrote:

> This chain of patches does some clearing up and add capture
> support to the compressed API.
>
> Missing is support for capture streams that don't use the
> copy callback. Also I have included two patches at the end
> of the chain that remove the buffer pointers and just use
> the cumulative values however as our setup uses the copy
> callback these are untested past build so should be taken
> with caution.
>
> Thanks,
> Charles
>
> Thess patches apply to:
>   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
>   branch "for-next"
>
> based on top of commit
>   126825e ALSA: snd-usb: add quirks handler for DSD streams
>
> - RichF
>
> Charles Keepax (7):
>   ALSA: compress_core: Update calc_avail to use cumulative values
>   ALSA: compress_core: Calculate avail correctly for capture streams
>   ALSA: compress_core: Deconstify copy callback buffer
>   ALSA: compress_core: Add support for capture streams
>   ASoC: soc-compress: Deduce stream direction
>   ALSA: compress_core: Remove unused hw_pointer
>   ALSA: compress_core: Rework writes to use cumulative values
>
>  include/sound/compress_driver.h |    4 +-
>  sound/core/compress_offload.c   |  117
> +++++++++++++++++++++++++-------------
>  sound/soc/soc-compress.c        |   11 +++-
>  3 files changed, 87 insertions(+), 45 deletions(-)
>
> --
> 1.7.2.5
>
>

Re: [PATCH 0/7] Add capture support to compress API

[Takashi Iwai]

At Thu, 18 Apr 2013 20:20:43 +1000,
Leanne Girdwood wrote:
> 
> REMOVE THIS ADDRESS FROM THIS MAILING LIST PLEASE

Added the right Girdwood to Cc.
Richard, you owe Liam for a wine he needs to bring tonight :)


Takashi

> On 18 April 2013 19:58, Richard Fitzgerald
> <rf@opensource.wolfsonmicro.com>wrote:
> 
> > This chain of patches does some clearing up and add capture
> > support to the compressed API.
> >
> > Missing is support for capture streams that don't use the
> > copy callback. Also I have included two patches at the end
> > of the chain that remove the buffer pointers and just use
> > the cumulative values however as our setup uses the copy
> > callback these are untested past build so should be taken
> > with caution.
> >
> > Thanks,
> > Charles
> >
> > Thess patches apply to:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> >   branch "for-next"
> >
> > based on top of commit
> >   126825e ALSA: snd-usb: add quirks handler for DSD streams
> >
> > - RichF
> >
> > Charles Keepax (7):
> >   ALSA: compress_core: Update calc_avail to use cumulative values
> >   ALSA: compress_core: Calculate avail correctly for capture streams
> >   ALSA: compress_core: Deconstify copy callback buffer
> >   ALSA: compress_core: Add support for capture streams
> >   ASoC: soc-compress: Deduce stream direction
> >   ALSA: compress_core: Remove unused hw_pointer
> >   ALSA: compress_core: Rework writes to use cumulative values
> >
> >  include/sound/compress_driver.h |    4 +-
> >  sound/core/compress_offload.c   |  117
> > +++++++++++++++++++++++++-------------
> >  sound/soc/soc-compress.c        |   11 +++-
> >  3 files changed, 87 insertions(+), 45 deletions(-)
> >
> > --
> > 1.7.2.5
> >
> >

Re: [PATCH 4/7] ALSA: compress_core: Add support for capture streams

[Vinod Koul]

On Thu, Apr 18, 2013 at 11:02:08AM +0100, Charles Keepax wrote:
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> ---
>  sound/core/compress_offload.c |   43 +++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> index 52ca4cc..52a2765 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -296,7 +296,41 @@ static ssize_t snd_compr_write(struct file *f, const char __user *buf,
>  static ssize_t snd_compr_read(struct file *f, char __user *buf,
>  		size_t count, loff_t *offset)
>  {
> -	return -ENXIO;
> +	struct snd_compr_file *data = f->private_data;
> +	struct snd_compr_stream *stream;
> +	size_t avail;
> +	int retval;
> +
> +	if (snd_BUG_ON(!data))
> +		return -EFAULT;
> +
> +	stream = &data->stream;
> +	mutex_lock(&stream->device->lock);
> +
> +	/* read is allowed when stream is running */
> +	if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING) {
> +		retval = -EBADFD;
> +		goto out;
minor one but shouldnt we allow read even when we have stoped or paused the
stream. Stop is important as you might want to get all buffers after you have
stopped.
> +	}
> +
> +	avail = snd_compr_get_avail(stream);
> +	pr_debug("avail returned %ld\n", (unsigned long)avail);
> +	/* calculate how much we can read from buffer */
> +	if (avail > count)
> +		avail = count;
> +
> +	if (stream->ops->copy) {
> +		retval = stream->ops->copy(stream, buf, avail);
> +	} else {
> +		retval = -ENXIO;
> +		goto out;
> +	}
> +	if (retval > 0)
> +		stream->runtime->total_bytes_transferred += retval;
> +
> +out:
> +	mutex_unlock(&stream->device->lock);
> +	return retval;
>  }
>  
>  static int snd_compr_mmap(struct file *f, struct vm_area_struct *vma)
> @@ -481,9 +515,14 @@ snd_compr_set_params(struct snd_compr_stream *stream, unsigned long arg)
>  		retval = stream->ops->set_params(stream, params);
>  		if (retval)
>  			goto out;
> -		stream->runtime->state = SNDRV_PCM_STATE_SETUP;
> +
>  		stream->metadata_set = false;
>  		stream->next_track = false;
> +
> +		if (stream->direction == SND_COMPRESS_PLAYBACK)
> +			stream->runtime->state = SNDRV_PCM_STATE_SETUP;
> +		else
> +			stream->runtime->state = SNDRV_PCM_STATE_PREPARED;
>  	} else {
>  		return -EPERM;
>  	}
> -- 
> 1.7.2.5
> 

Re: [PATCH 4/7] ALSA: compress_core: Add support for capture streams

[Richard Fitzgerald]

> >  static ssize_t snd_compr_read(struct file *f, char __user *buf,
> >  		size_t count, loff_t *offset)
> >  {
> > -	return -ENXIO;
> > +	struct snd_compr_file *data = f->private_data;
> > +	struct snd_compr_stream *stream;
> > +	size_t avail;
> > +	int retval;
> > +
> > +	if (snd_BUG_ON(!data))
> > +		return -EFAULT;
> > +
> > +	stream = &data->stream;
> > +	mutex_lock(&stream->device->lock);
> > +
> > +	/* read is allowed when stream is running */
> > +	if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING) {
> > +		retval = -EBADFD;
> > +		goto out;
> minor one but shouldnt we allow read even when we have stoped or paused the
> stream. Stop is important as you might want to get all buffers after you have
> stopped.

Yes, looks like a bug to me. Not sure that it's a blocker - new feature so
nobody is going to be using this immediately.
As you can't capture sounds from the future(!) the DSP could only hold data up
to the point you stopped/paused. But the DSP could have some audio packets queued
that it hasn't sent to host yet and you still need to read them.
You'd probably not have much queued anyway as it would be disturbing for user
if they monitor recording through their device headphones and it has significant
latency from the real world sounds?

Re: [PATCH 4/7] ALSA: compress_core: Add support for capture streams

[Leanne Girdwood]

KINDLY REMOVE THIS ADDRESS FROM YOUR MAILING LIST

On Friday, 19 April 2013, Vinod Koul wrote:

> On Thu, Apr 18, 2013 at 11:02:08AM +0100, Charles Keepax wrote:
> > Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com<javascript:;>
> >
> > Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com<javascript:;>
> >
> > ---
> >  sound/core/compress_offload.c |   43
> +++++++++++++++++++++++++++++++++++++++-
> >  1 files changed, 41 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/core/compress_offload.c
> b/sound/core/compress_offload.c
> > index 52ca4cc..52a2765 100644
> > --- a/sound/core/compress_offload.c
> > +++ b/sound/core/compress_offload.c
> > @@ -296,7 +296,41 @@ static ssize_t snd_compr_write(struct file *f,
> const char __user *buf,
> >  static ssize_t snd_compr_read(struct file *f, char __user *buf,
> >               size_t count, loff_t *offset)
> >  {
> > -     return -ENXIO;
> > +     struct snd_compr_file *data = f->private_data;
> > +     struct snd_compr_stream *stream;
> > +     size_t avail;
> > +     int retval;
> > +
> > +     if (snd_BUG_ON(!data))
> > +             return -EFAULT;
> > +
> > +     stream = &data->stream;
> > +     mutex_lock(&stream->device->lock);
> > +
> > +     /* read is allowed when stream is running */
> > +     if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING) {
> > +             retval = -EBADFD;
> > +             goto out;
> minor one but shouldnt we allow read even when we have stoped or paused the
> stream. Stop is important as you might want to get all buffers after you
> have
> stopped.
> > +     }
> > +
> > +     avail = snd_compr_get_avail(stream);
> > +     pr_debug("avail returned %ld\n", (unsigned long)avail);
> > +     /* calculate how much we can read from buffer */
> > +     if (avail > count)
> > +             avail = count;
> > +
> > +     if (stream->ops->copy) {
> > +             retval = stream->ops->copy(stream, buf, avail);
> > +     } else {
> > +             retval = -ENXIO;
> > +             goto out;
> > +     }
> > +     if (retval > 0)
> > +             stream->runtime->total_bytes_transferred += retval;
> > +
> > +out:
> > +     mutex_unlock(&stream->device->lock);
> > +     return retval;
> >  }
> >
> >  static int snd_compr_mmap(struct file *f, struct vm_area_struct *vma)
> > @@ -481,9 +515,14 @@ snd_compr_set_params(struct snd_compr_stream
> *stream, unsigned long arg)
> >               retval = stream->ops->set_params(stream, params);
> >               if (retval)
> >                       goto out;
> > -             stream->runtime->state = SNDRV_PCM_STATE_SETUP;
> > +
> >               stream->metadata_set = false;
> >               stream->next_track = false;
> > +
> > +             if (stream->direction == SND_COMPRESS_PLAYBACK)
> > +                     stream->runtime->state = SNDRV_PCM_STATE_SETUP;
> > +             else
> > +                     stream->runtime->state = SNDRV_PCM_STATE_PREPARED;
> >       } else {
> >               return -EPERM;
> >       }
> > --
> > 1.7.2.5
> >
>

Re: [PATCH 5/7] ASoC: soc-compress: Deduce stream direction

[Vinod Koul]

On Thu, Apr 18, 2013 at 11:02:38AM +0100, Charles Keepax wrote:
> Previously we just hard coded all streams as playback streams, this
> patch checks the DAI to see if it is a capture or playback stream. It is
> worth noting that at this time only unidirectional streams are
> supported.
well the intention was the compress device would be either playback or record.
you should have two devices if you support both.

> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> ---
>  sound/soc/soc-compress.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
> index da83e56..3853f7e 100644
> --- a/sound/soc/soc-compress.c
> +++ b/sound/soc/soc-compress.c
> @@ -384,7 +384,14 @@ int soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
>  	/* check client and interface hw capabilities */
>  	snprintf(new_name, sizeof(new_name), "%s %s-%d",
>  			rtd->dai_link->stream_name, codec_dai->name, num);
> -	direction = SND_COMPRESS_PLAYBACK;
> +
> +	if (codec_dai->driver->playback.channels_min)
> +		direction = SND_COMPRESS_PLAYBACK;
> +	else if (codec_dai->driver->capture.channels_min)
> +		direction = SND_COMPRESS_CAPTURE;
> +	else
> +		return -EINVAL;
> +
>  	compr = kzalloc(sizeof(*compr), GFP_KERNEL);
>  	if (compr == NULL) {
>  		snd_printk(KERN_ERR "Cannot allocate compr\n");
> -- 
> 1.7.2.5
> 

Re: [PATCH 0/7] Add capture support to compress API

[Takashi Iwai]

At Thu, 18 Apr 2013 10:58:41 +0100,
Richard Fitzgerald wrote:
> 
> This chain of patches does some clearing up and add capture
> support to the compressed API.
> 
> Missing is support for capture streams that don't use the
> copy callback. Also I have included two patches at the end
> of the chain that remove the buffer pointers and just use
> the cumulative values however as our setup uses the copy
> callback these are untested past build so should be taken
> with caution.
> 
> Thanks,
> Charles
> 
> Thess patches apply to:
>   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
>   branch "for-next"
> 
> based on top of commit
>   126825e ALSA: snd-usb: add quirks handler for DSD streams

Vinod, Pierre, could you guys check these patches?

If these are OK, I can still merge for 3.10.  Although it's already
late for 3.10, there are little users of compress stuff, so the impact
must be fairly small.  If you guys aren't still happy, let's postpone
for 3.11.


If we really merge it, don't you mind me merging soc-compress patch in
my tree (directly to for-next branch), Mark?


thanks,

Takashi

> 
> - RichF
> 
> Charles Keepax (7):
>   ALSA: compress_core: Update calc_avail to use cumulative values
>   ALSA: compress_core: Calculate avail correctly for capture streams
>   ALSA: compress_core: Deconstify copy callback buffer
>   ALSA: compress_core: Add support for capture streams
>   ASoC: soc-compress: Deduce stream direction
>   ALSA: compress_core: Remove unused hw_pointer
>   ALSA: compress_core: Rework writes to use cumulative values
> 
>  include/sound/compress_driver.h |    4 +-
>  sound/core/compress_offload.c   |  117 +++++++++++++++++++++++++-------------
>  sound/soc/soc-compress.c        |   11 +++-
>  3 files changed, 87 insertions(+), 45 deletions(-)
> 
> -- 
> 1.7.2.5
> 

Re: [PATCH 0/7] Add capture support to compress API

[Vinod Koul]

On Fri, Apr 19, 2013 at 03:41:11PM +0200, Takashi Iwai wrote:
> At Thu, 18 Apr 2013 10:58:41 +0100,
> Richard Fitzgerald wrote:
> > 
> > This chain of patches does some clearing up and add capture
> > support to the compressed API.
> > 
> > Missing is support for capture streams that don't use the
> > copy callback. Also I have included two patches at the end
> > of the chain that remove the buffer pointers and just use
> > the cumulative values however as our setup uses the copy
> > callback these are untested past build so should be taken
> > with caution.
> > 
> > Thanks,
> > Charles
> > 
> > Thess patches apply to:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> >   branch "for-next"
> > 
> > based on top of commit
> >   126825e ALSA: snd-usb: add quirks handler for DSD streams
> 
> Vinod, Pierre, could you guys check these patches?
> 
> If these are OK, I can still merge for 3.10.  Although it's already
> late for 3.10, there are little users of compress stuff, so the impact
> must be fairly small.  If you guys aren't still happy, let's postpone
> for 3.11.
Sorry for delay and thrash on patches, my sound clone was bad...

I think I am okay with series approach. We need small fix for state check for
read but that need not block this series

Acked-by: Vinod Koul <vinod.koul@intel.com>
> 
> 
> If we really merge it, don't you mind me merging soc-compress patch in
> my tree (directly to for-next branch), Mark?
i think that should be sensible :)

--
~Vinod
> 
> 
> thanks,
> 
> Takashi
> 
> > 
> > - RichF
> > 
> > Charles Keepax (7):
> >   ALSA: compress_core: Update calc_avail to use cumulative values
> >   ALSA: compress_core: Calculate avail correctly for capture streams
> >   ALSA: compress_core: Deconstify copy callback buffer
> >   ALSA: compress_core: Add support for capture streams
> >   ASoC: soc-compress: Deduce stream direction
> >   ALSA: compress_core: Remove unused hw_pointer
> >   ALSA: compress_core: Rework writes to use cumulative values
> > 
> >  include/sound/compress_driver.h |    4 +-
> >  sound/core/compress_offload.c   |  117 +++++++++++++++++++++++++-------------
> >  sound/soc/soc-compress.c        |   11 +++-
> >  3 files changed, 87 insertions(+), 45 deletions(-)
> > 
> > -- 
> > 1.7.2.5
> > 

Re: [PATCH 0/7] Add capture support to compress API

[Vinod Koul]

On Fri, Apr 19, 2013 at 07:00:58PM +0530, Vinod Koul wrote:
> On Fri, Apr 19, 2013 at 03:41:11PM +0200, Takashi Iwai wrote:
> > At Thu, 18 Apr 2013 10:58:41 +0100,
> > Richard Fitzgerald wrote:
> > > 
> > > This chain of patches does some clearing up and add capture
> > > support to the compressed API.
Charles,

I am expecting capture support patches for tinycompress :)

> > > 
> > > Missing is support for capture streams that don't use the
> > > copy callback. Also I have included two patches at the end
> > > of the chain that remove the buffer pointers and just use
> > > the cumulative values however as our setup uses the copy
> > > callback these are untested past build so should be taken
> > > with caution.
> > > 
> > > Thanks,
> > > Charles
> > > 
> > > Thess patches apply to:
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> > >   branch "for-next"
> > > 
> > > based on top of commit
> > >   126825e ALSA: snd-usb: add quirks handler for DSD streams
> > 
> > Vinod, Pierre, could you guys check these patches?
> > 
> > If these are OK, I can still merge for 3.10.  Although it's already
> > late for 3.10, there are little users of compress stuff, so the impact
> > must be fairly small.  If you guys aren't still happy, let's postpone
> > for 3.11.
> Sorry for delay and thrash on patches, my sound clone was bad...
> 
> I think I am okay with series approach. We need small fix for state check for
> read but that need not block this series
> 
> Acked-by: Vinod Koul <vinod.koul@intel.com>
> > 
> > 
> > If we really merge it, don't you mind me merging soc-compress patch in
> > my tree (directly to for-next branch), Mark?
> i think that should be sensible :)
> 
> --
> ~Vinod
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> > > 
> > > - RichF
> > > 
> > > Charles Keepax (7):
> > >   ALSA: compress_core: Update calc_avail to use cumulative values
> > >   ALSA: compress_core: Calculate avail correctly for capture streams
> > >   ALSA: compress_core: Deconstify copy callback buffer
> > >   ALSA: compress_core: Add support for capture streams
> > >   ASoC: soc-compress: Deduce stream direction
> > >   ALSA: compress_core: Remove unused hw_pointer
> > >   ALSA: compress_core: Rework writes to use cumulative values
> > > 
> > >  include/sound/compress_driver.h |    4 +-
> > >  sound/core/compress_offload.c   |  117 +++++++++++++++++++++++++-------------
> > >  sound/soc/soc-compress.c        |   11 +++-
> > >  3 files changed, 87 insertions(+), 45 deletions(-)
> > > 
> > > -- 
> > > 1.7.2.5
> > > 

Re: [PATCH 0/7] Add capture support to compress API

[Richard Fitzgerald]

> I am expecting capture support patches for tinycompress :)

Your expectations are correct. Charles has done them and I
didn't notice that they hadn't been posted.
I think I need to rebase them first. Will do them soon as
I get the time.

Re: [PATCH 0/7] Add capture support to compress API

[Takashi Iwai]

At Fri, 19 Apr 2013 19:00:58 +0530,
Vinod Koul wrote:
> 
> On Fri, Apr 19, 2013 at 03:41:11PM +0200, Takashi Iwai wrote:
> > At Thu, 18 Apr 2013 10:58:41 +0100,
> > Richard Fitzgerald wrote:
> > > 
> > > This chain of patches does some clearing up and add capture
> > > support to the compressed API.
> > > 
> > > Missing is support for capture streams that don't use the
> > > copy callback. Also I have included two patches at the end
> > > of the chain that remove the buffer pointers and just use
> > > the cumulative values however as our setup uses the copy
> > > callback these are untested past build so should be taken
> > > with caution.
> > > 
> > > Thanks,
> > > Charles
> > > 
> > > Thess patches apply to:
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> > >   branch "for-next"
> > > 
> > > based on top of commit
> > >   126825e ALSA: snd-usb: add quirks handler for DSD streams
> > 
> > Vinod, Pierre, could you guys check these patches?
> > 
> > If these are OK, I can still merge for 3.10.  Although it's already
> > late for 3.10, there are little users of compress stuff, so the impact
> > must be fairly small.  If you guys aren't still happy, let's postpone
> > for 3.11.
> Sorry for delay and thrash on patches, my sound clone was bad...
> 
> I think I am okay with series approach. We need small fix for state check for
> read but that need not block this series
> 
> Acked-by: Vinod Koul <vinod.koul@intel.com>
> > 
> > 
> > If we really merge it, don't you mind me merging soc-compress patch in
> > my tree (directly to for-next branch), Mark?
> i think that should be sensible :)

OK, I merged the original patches now to for-next branch.


thanks,

Takashi

Re: [PATCH 0/7] Add capture support to compress API

[Mark Brown]

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

On Fri, Apr 19, 2013 at 03:41:11PM +0200, Takashi Iwai wrote:

> If we really merge it, don't you mind me merging soc-compress patch in
> my tree (directly to for-next branch), Mark?

Yes, that's fine for me - for v3.11 I suspect it'd make sense to have it
in my tree though as there'll probably be other stuff that depends on
it.

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

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

[PATCH 0/7] Add capture support to compress API

[Charles Keepax]

This chain of patches does some clearing up and add capture
support to the compressed API.

Missing is support for capture streams that don't use the
copy callback. Also I have included two patches at the end
of the chain that remove the buffer pointers and just use
the cumulative values however as our setup uses the copy
callback these are untested past build so should be taken
with caution.

Thanks,
Charles

Charles Keepax (7):
  ALSA: compress_core: Update calc_avail to use cumulative values
  ALSA: compress_core: Calculate avail correctly for capture streams
  ALSA: compress_core: Deconstify copy callback buffer
  ALSA: compress_core: Add support for capture streams
  ASoC: soc-compress: Deduce stream direction
  ALSA: compress_core: Remove unused hw_pointer
  ALSA: compress_core: Rework writes to use cumulative values

 include/sound/compress_driver.h |    4 +-
 sound/core/compress_offload.c   |  117 +++++++++++++++++++++++++-------------
 sound/soc/soc-compress.c        |   11 +++-
 3 files changed, 87 insertions(+), 45 deletions(-)

-- 
1.7.2.5

[PATCH 4/7] ALSA: compress_core: Add support for capture streams

[Charles Keepax]

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/core/compress_offload.c |   43 +++++++++++++++++++++++++++++++++++++++-
 1 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index 52ca4cc..d0fc9a1 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -296,7 +296,41 @@ static ssize_t snd_compr_write(struct file *f, const char __user *buf,
 static ssize_t snd_compr_read(struct file *f, char __user *buf,
 		size_t count, loff_t *offset)
 {
-	return -ENXIO;
+	struct snd_compr_file *data = f->private_data;
+	struct snd_compr_stream *stream;
+	size_t avail;
+	int retval;
+
+	if (snd_BUG_ON(!data))
+		return -EFAULT;
+
+	stream = &data->stream;
+	mutex_lock(&stream->device->lock);
+
+	/* read is allowed when stream is running */
+	if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING) {
+		retval = -EBADFD;
+		goto out;
+	}
+
+	avail = snd_compr_get_avail(stream);
+	pr_debug("avail returned %ld\n", (unsigned long)avail);
+	/* calculate how much we can read from buffer */
+	if (avail > count)
+		avail = count;
+
+	if (stream->ops->copy) {
+		retval = stream->ops->copy(stream, buf, avail);
+	} else {
+		retval = -ENXIO;
+		goto out;
+	}
+	if (retval > 0)
+		stream->runtime->total_bytes_transferred += retval;
+
+out:
+	mutex_unlock(&stream->device->lock);
+	return retval;
 }
 
 static int snd_compr_mmap(struct file *f, struct vm_area_struct *vma)
@@ -481,7 +515,12 @@ snd_compr_set_params(struct snd_compr_stream *stream, unsigned long arg)
 		retval = stream->ops->set_params(stream, params);
 		if (retval)
 			goto out;
-		stream->runtime->state = SNDRV_PCM_STATE_SETUP;
+
+		if (stream->direction == SND_COMPRESS_PLAYBACK)
+			stream->runtime->state = SNDRV_PCM_STATE_SETUP;
+		else
+			stream->runtime->state = SNDRV_PCM_STATE_PREPARED;
+
 		stream->metadata_set = false;
 		stream->next_track = false;
 	} else {
-- 
1.7.2.5