[PATCH v3 0/8] Add support for voice control on Arizona ADSP

[PATCH v3 0/8] Add support for voice control on Arizona ADSP

[Charles Keepax]

This series add support for voice control on wm8280/5110. This is
done by opening a compressed record channel, upon which data will
be available once the voice control functionality has been
triggered on the DSP.

Changes since v2:
 - Fix specified using in printf for size_t in wm_adsp_compr_read,
   as reported by the kbuild test robot.

Thanks,
Charles

Charles Keepax (8):
  ASoC: wm5110: Provide basic hookup for voice control
  ASoC: wm_adsp: Factor out finding the location of an algorithm region
  ALSA: compress: Add SND_AUDIOCODEC_BESPOKE
  ASoC: wm_adsp: Add support for opening a compressed stream
  ASoC: wm_adsp: Add code to locate and initialise compressed buffer
  ASoC: wm_adsp: Attach buffers and streams together
  ASoC: wm_adsp: Add a handler for the compressed IRQ
  ASoC: wm_adsp: Pull data through compressed read

 include/uapi/sound/compress_params.h |   5 +-
 sound/soc/codecs/Kconfig             |   1 +
 sound/soc/codecs/arizona.h           |   2 +-
 sound/soc/codecs/wm5110.c            |  95 +++-
 sound/soc/codecs/wm_adsp.c           | 907 ++++++++++++++++++++++++++++++++++-
 sound/soc/codecs/wm_adsp.h           |  21 +
 6 files changed, 1010 insertions(+), 21 deletions(-)

-- 
2.1.4

[PATCH v3 1/8] ASoC: wm5110: Provide basic hookup for voice control

[Charles Keepax]

Register a platform driver for the CODEC and add DAIs that will be used
to connect a compressed record path for the voice control functionality.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/Kconfig   |  1 +
 sound/soc/codecs/arizona.h |  2 +-
 sound/soc/codecs/wm5110.c  | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index c971371..57499ea 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -220,6 +220,7 @@ config SND_SOC_WM_HUBS
 
 config SND_SOC_WM_ADSP
 	tristate
+	select SND_SOC_COMPRESS
 	default y if SND_SOC_CS47L24=y
 	default y if SND_SOC_WM5102=y
 	default y if SND_SOC_WM5110=y
diff --git a/sound/soc/codecs/arizona.h b/sound/soc/codecs/arizona.h
index b4f1867..8b6adb5 100644
--- a/sound/soc/codecs/arizona.h
+++ b/sound/soc/codecs/arizona.h
@@ -57,7 +57,7 @@
 #define ARIZONA_CLK_98MHZ  5
 #define ARIZONA_CLK_147MHZ 6
 
-#define ARIZONA_MAX_DAI  6
+#define ARIZONA_MAX_DAI  8
 #define ARIZONA_MAX_ADSP 4
 
 #define ARIZONA_DVFS_SR1_RQ	0x001
diff --git a/sound/soc/codecs/wm5110.c b/sound/soc/codecs/wm5110.c
index e93e542..67d5651 100644
--- a/sound/soc/codecs/wm5110.c
+++ b/sound/soc/codecs/wm5110.c
@@ -1810,6 +1810,9 @@ static const struct snd_soc_dapm_route wm5110_dapm_routes[] = {
 	{ "Slim2 Capture", NULL, "SYSCLK" },
 	{ "Slim3 Capture", NULL, "SYSCLK" },
 
+	{ "Voice Control DSP", NULL, "DSP3" },
+	{ "Voice Control DSP", NULL, "SYSCLK" },
+
 	{ "IN1L PGA", NULL, "IN1L" },
 	{ "IN1R PGA", NULL, "IN1R" },
 
@@ -2132,6 +2135,27 @@ static struct snd_soc_dai_driver wm5110_dai[] = {
 		 },
 		.ops = &arizona_simple_dai_ops,
 	},
+	{
+		.name = "wm5110-cpu-voicectrl",
+		.capture = {
+			.stream_name = "Voice Control CPU",
+			.channels_min = 1,
+			.channels_max = 1,
+			.rates = WM5110_RATES,
+			.formats = WM5110_FORMATS,
+		},
+		.compress_new = snd_soc_new_compress,
+	},
+	{
+		.name = "wm5110-dsp-voicectrl",
+		.capture = {
+			.stream_name = "Voice Control DSP",
+			.channels_min = 1,
+			.channels_max = 1,
+			.rates = WM5110_RATES,
+			.formats = WM5110_FORMATS,
+		},
+	},
 };
 
 static int wm5110_codec_probe(struct snd_soc_codec *codec)
@@ -2224,6 +2248,13 @@ static struct snd_soc_codec_driver soc_codec_dev_wm5110 = {
 	.num_dapm_routes = ARRAY_SIZE(wm5110_dapm_routes),
 };
 
+static struct snd_compr_ops wm5110_compr_ops = {
+};
+
+static struct snd_soc_platform_driver wm5110_compr_platform = {
+	.compr_ops = &wm5110_compr_ops,
+};
+
 static int wm5110_probe(struct platform_device *pdev)
 {
 	struct arizona *arizona = dev_get_drvdata(pdev->dev.parent);
@@ -2284,8 +2315,21 @@ static int wm5110_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_idle(&pdev->dev);
 
-	return snd_soc_register_codec(&pdev->dev, &soc_codec_dev_wm5110,
+	ret = snd_soc_register_platform(&pdev->dev, &wm5110_compr_platform);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to register platform: %d\n", ret);
+		goto error;
+	}
+
+	ret = snd_soc_register_codec(&pdev->dev, &soc_codec_dev_wm5110,
 				      wm5110_dai, ARRAY_SIZE(wm5110_dai));
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to register codec: %d\n", ret);
+		snd_soc_unregister_platform(&pdev->dev);
+	}
+
+error:
+	return ret;
 }
 
 static int wm5110_remove(struct platform_device *pdev)
-- 
2.1.4

[PATCH v3 2/8] ASoC: wm_adsp: Factor out finding the location of an algorithm region

[Charles Keepax]

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index b083642..2b99f46 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -1362,6 +1362,19 @@ static void *wm_adsp_read_algs(struct wm_adsp *dsp, size_t n_algs,
 	return alg;
 }
 
+static struct wm_adsp_alg_region *
+	wm_adsp_find_alg_region(struct wm_adsp *dsp, int type, unsigned int id)
+{
+	struct wm_adsp_alg_region *alg_region;
+
+	list_for_each_entry(alg_region, &dsp->alg_regions, list) {
+		if (id == alg_region->alg && type == alg_region->type)
+			return alg_region;
+	}
+
+	return NULL;
+}
+
 static struct wm_adsp_alg_region *wm_adsp_create_region(struct wm_adsp *dsp,
 							int type, __be32 id,
 							__be32 base)
@@ -1734,22 +1747,16 @@ static int wm_adsp_load_coeff(struct wm_adsp *dsp)
 				break;
 			}
 
-			reg = 0;
-			list_for_each_entry(alg_region,
-					    &dsp->alg_regions, list) {
-				if (le32_to_cpu(blk->id) == alg_region->alg &&
-				    type == alg_region->type) {
-					reg = alg_region->base;
-					reg = wm_adsp_region_to_reg(mem,
-								    reg);
-					reg += offset;
-					break;
-				}
-			}
-
-			if (reg == 0)
+			alg_region = wm_adsp_find_alg_region(dsp, type,
+						le32_to_cpu(blk->id));
+			if (alg_region) {
+				reg = alg_region->base;
+				reg = wm_adsp_region_to_reg(mem, reg);
+				reg += offset;
+			} else {
 				adsp_err(dsp, "No %x for algorithm %x\n",
 					 type, le32_to_cpu(blk->id));
+			}
 			break;
 
 		default:
-- 
2.1.4

[PATCH v3 3/8] ALSA: compress: Add SND_AUDIOCODEC_BESPOKE

[Charles Keepax]

When working with the compressed framework occasionally vendors will
use esoteric internal audio formats. For such formats it doesn't really
make sense to add an new define to the kernel as their use is not
sufficiently general.

This patch adds a new define SND_AUDIOCODEC_BESPOKE that vendors can use
in such situations.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Acked-by: Vinod Koul <vinod.koul@intel.com>
---
 include/uapi/sound/compress_params.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/uapi/sound/compress_params.h b/include/uapi/sound/compress_params.h
index d9bd9ca..9625484 100644
--- a/include/uapi/sound/compress_params.h
+++ b/include/uapi/sound/compress_params.h
@@ -73,7 +73,8 @@
 #define SND_AUDIOCODEC_IEC61937              ((__u32) 0x0000000B)
 #define SND_AUDIOCODEC_G723_1                ((__u32) 0x0000000C)
 #define SND_AUDIOCODEC_G729                  ((__u32) 0x0000000D)
-#define SND_AUDIOCODEC_MAX                   SND_AUDIOCODEC_G729
+#define SND_AUDIOCODEC_BESPOKE               ((__u32) 0x0000000E)
+#define SND_AUDIOCODEC_MAX                   SND_AUDIOCODEC_BESPOKE
 
 /*
  * Profile and modes are listed with bit masks. This allows for a
@@ -312,7 +313,7 @@ struct snd_enc_flac {
 
 struct snd_enc_generic {
 	__u32 bw;	/* encoder bandwidth */
-	__s32 reserved[15];
+	__s32 reserved[15];	/* Can be used for SND_AUDIOCODEC_BESPOKE */
 } __attribute__((packed, aligned(4)));
 
 union snd_codec_options {
-- 
2.1.4

[PATCH v3 4/8] ASoC: wm_adsp: Add support for opening a compressed stream

[Charles Keepax]

Allow user-space to open a compressed stream, although no data will be
passed yet, as part of this adding the ability to define supported
capabilities per firmware and check these match the stream being opened.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm5110.c  |  23 ++++++
 sound/soc/codecs/wm_adsp.c | 194 ++++++++++++++++++++++++++++++++++++++++++++-
 sound/soc/codecs/wm_adsp.h |  13 +++
 3 files changed, 227 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/wm5110.c b/sound/soc/codecs/wm5110.c
index 67d5651..8c0fd91 100644
--- a/sound/soc/codecs/wm5110.c
+++ b/sound/soc/codecs/wm5110.c
@@ -2158,6 +2158,25 @@ static struct snd_soc_dai_driver wm5110_dai[] = {
 	},
 };
 
+static int wm5110_open(struct snd_compr_stream *stream)
+{
+	struct snd_soc_pcm_runtime *rtd = stream->private_data;
+	struct wm5110_priv *priv = snd_soc_codec_get_drvdata(rtd->codec);
+	struct arizona *arizona = priv->core.arizona;
+	int n_adsp;
+
+	if (strcmp(rtd->codec_dai->name, "wm5110-dsp-voicectrl") == 0) {
+		n_adsp = 2;
+	} else {
+		dev_err(arizona->dev,
+			"No suitable compressed stream for DAI '%s'\n",
+			rtd->codec_dai->name);
+		return -EINVAL;
+	}
+
+	return wm_adsp_compr_open(&priv->core.adsp[n_adsp], stream);
+}
+
 static int wm5110_codec_probe(struct snd_soc_codec *codec)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
@@ -2249,6 +2268,10 @@ static struct snd_soc_codec_driver soc_codec_dev_wm5110 = {
 };
 
 static struct snd_compr_ops wm5110_compr_ops = {
+	.open = wm5110_open,
+	.free = wm_adsp_compr_free,
+	.set_params = wm_adsp_compr_set_params,
+	.get_caps = wm_adsp_compr_get_caps,
 };
 
 static struct snd_soc_platform_driver wm5110_compr_platform = {
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 2b99f46..22b57bc 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -229,8 +229,42 @@ static const char *wm_adsp_fw_text[WM_ADSP_NUM_FW] = {
 	[WM_ADSP_FW_MISC] =     "Misc",
 };
 
-static struct {
+struct wm_adsp_compr {
+	struct wm_adsp *dsp;
+
+	struct snd_compr_stream *stream;
+	struct snd_compressed_buffer size;
+};
+
+#define WM_ADSP_DATA_WORD_SIZE         3
+
+#define WM_ADSP_MIN_FRAGMENTS          1
+#define WM_ADSP_MAX_FRAGMENTS          256
+#define WM_ADSP_MIN_FRAGMENT_SIZE      (64 * WM_ADSP_DATA_WORD_SIZE)
+#define WM_ADSP_MAX_FRAGMENT_SIZE      (4096 * WM_ADSP_DATA_WORD_SIZE)
+
+struct wm_adsp_fw_caps {
+	u32 id;
+	struct snd_codec_desc desc;
+};
+
+static const struct wm_adsp_fw_caps ez2control_caps[] = {
+	{
+		.id = SND_AUDIOCODEC_BESPOKE,
+		.desc = {
+			.max_ch = 1,
+			.sample_rates = { 16000 },
+			.num_sample_rates = 1,
+			.formats = SNDRV_PCM_FMTBIT_S16_LE,
+		},
+	},
+};
+
+static const struct {
 	const char *file;
+	int compr_direction;
+	int num_caps;
+	const struct wm_adsp_fw_caps *caps;
 } wm_adsp_fw[WM_ADSP_NUM_FW] = {
 	[WM_ADSP_FW_MBC_VSS] =  { .file = "mbc-vss" },
 	[WM_ADSP_FW_HIFI] =     { .file = "hifi" },
@@ -238,7 +272,12 @@ static struct {
 	[WM_ADSP_FW_TX_SPK] =   { .file = "tx-spk" },
 	[WM_ADSP_FW_RX] =       { .file = "rx" },
 	[WM_ADSP_FW_RX_ANC] =   { .file = "rx-anc" },
-	[WM_ADSP_FW_CTRL] =     { .file = "ctrl" },
+	[WM_ADSP_FW_CTRL] =     {
+		.file = "ctrl",
+		.compr_direction = SND_COMPRESS_CAPTURE,
+		.num_caps = ARRAY_SIZE(ez2control_caps),
+		.caps = ez2control_caps,
+	},
 	[WM_ADSP_FW_ASR] =      { .file = "asr" },
 	[WM_ADSP_FW_TRACE] =    { .file = "trace" },
 	[WM_ADSP_FW_SPK_PROT] = { .file = "spk-prot" },
@@ -461,7 +500,7 @@ static int wm_adsp_fw_put(struct snd_kcontrol *kcontrol,
 
 	mutex_lock(&dsp[e->shift_l].pwr_lock);
 
-	if (dsp[e->shift_l].running)
+	if (dsp[e->shift_l].running || dsp[e->shift_l].compr)
 		ret = -EBUSY;
 	else
 		dsp[e->shift_l].fw = ucontrol->value.integer.value[0];
@@ -2175,4 +2214,153 @@ int wm_adsp2_init(struct wm_adsp *dsp)
 }
 EXPORT_SYMBOL_GPL(wm_adsp2_init);
 
+int wm_adsp_compr_open(struct wm_adsp *dsp, struct snd_compr_stream *stream)
+{
+	struct wm_adsp_compr *compr;
+	int ret = 0;
+
+	mutex_lock(&dsp->pwr_lock);
+
+	if (wm_adsp_fw[dsp->fw].num_caps == 0) {
+		adsp_err(dsp, "Firmware does not support compressed API\n");
+		ret = -ENXIO;
+		goto out;
+	}
+
+	if (wm_adsp_fw[dsp->fw].compr_direction != stream->direction) {
+		adsp_err(dsp, "Firmware does not support stream direction\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	compr = kzalloc(sizeof(*compr), GFP_KERNEL);
+	if (!compr) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	compr->dsp = dsp;
+	compr->stream = stream;
+
+	dsp->compr = compr;
+
+	stream->runtime->private_data = compr;
+
+out:
+	mutex_unlock(&dsp->pwr_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(wm_adsp_compr_open);
+
+int wm_adsp_compr_free(struct snd_compr_stream *stream)
+{
+	struct wm_adsp_compr *compr = stream->runtime->private_data;
+	struct wm_adsp *dsp = compr->dsp;
+
+	mutex_lock(&dsp->pwr_lock);
+
+	dsp->compr = NULL;
+
+	kfree(compr);
+
+	mutex_unlock(&dsp->pwr_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(wm_adsp_compr_free);
+
+static int wm_adsp_compr_check_params(struct snd_compr_stream *stream,
+				      struct snd_compr_params *params)
+{
+	struct wm_adsp_compr *compr = stream->runtime->private_data;
+	struct wm_adsp *dsp = compr->dsp;
+	const struct wm_adsp_fw_caps *caps;
+	const struct snd_codec_desc *desc;
+	int i, j;
+
+	if (params->buffer.fragment_size < WM_ADSP_MIN_FRAGMENT_SIZE ||
+	    params->buffer.fragment_size > WM_ADSP_MAX_FRAGMENT_SIZE ||
+	    params->buffer.fragments < WM_ADSP_MIN_FRAGMENTS ||
+	    params->buffer.fragments > WM_ADSP_MAX_FRAGMENTS ||
+	    params->buffer.fragment_size % WM_ADSP_DATA_WORD_SIZE) {
+		adsp_err(dsp, "Invalid buffer fragsize=%d fragments=%d\n",
+			 params->buffer.fragment_size,
+			 params->buffer.fragments);
+
+		return -EINVAL;
+	}
+
+	for (i = 0; i < wm_adsp_fw[dsp->fw].num_caps; i++) {
+		caps = &wm_adsp_fw[dsp->fw].caps[i];
+		desc = &caps->desc;
+
+		if (caps->id != params->codec.id)
+			continue;
+
+		if (stream->direction == SND_COMPRESS_PLAYBACK) {
+			if (desc->max_ch < params->codec.ch_out)
+				continue;
+		} else {
+			if (desc->max_ch < params->codec.ch_in)
+				continue;
+		}
+
+		if (!(desc->formats & (1 << params->codec.format)))
+			continue;
+
+		for (j = 0; j < desc->num_sample_rates; ++j)
+			if (desc->sample_rates[j] == params->codec.sample_rate)
+				return 0;
+	}
+
+	adsp_err(dsp, "Invalid params id=%u ch=%u,%u rate=%u fmt=%u\n",
+		 params->codec.id, params->codec.ch_in, params->codec.ch_out,
+		 params->codec.sample_rate, params->codec.format);
+	return -EINVAL;
+}
+
+int wm_adsp_compr_set_params(struct snd_compr_stream *stream,
+			     struct snd_compr_params *params)
+{
+	struct wm_adsp_compr *compr = stream->runtime->private_data;
+	int ret;
+
+	ret = wm_adsp_compr_check_params(stream, params);
+	if (ret)
+		return ret;
+
+	compr->size = params->buffer;
+
+	adsp_dbg(compr->dsp, "fragment_size=%d fragments=%d\n",
+		 compr->size.fragment_size, compr->size.fragments);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(wm_adsp_compr_set_params);
+
+int wm_adsp_compr_get_caps(struct snd_compr_stream *stream,
+			   struct snd_compr_caps *caps)
+{
+	struct wm_adsp_compr *compr = stream->runtime->private_data;
+	int fw = compr->dsp->fw;
+	int i;
+
+	if (wm_adsp_fw[fw].caps) {
+		for (i = 0; i < wm_adsp_fw[fw].num_caps; i++)
+			caps->codecs[i] = wm_adsp_fw[fw].caps[i].id;
+
+		caps->num_codecs = i;
+		caps->direction = wm_adsp_fw[fw].compr_direction;
+
+		caps->min_fragment_size = WM_ADSP_MIN_FRAGMENT_SIZE;
+		caps->max_fragment_size = WM_ADSP_MAX_FRAGMENT_SIZE;
+		caps->min_fragments = WM_ADSP_MIN_FRAGMENTS;
+		caps->max_fragments = WM_ADSP_MAX_FRAGMENTS;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(wm_adsp_compr_get_caps);
+
 MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index d2a8c78..33c9b52 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -15,6 +15,7 @@
 
 #include <sound/soc.h>
 #include <sound/soc-dapm.h>
+#include <sound/compress_driver.h>
 
 #include "wmfw.h"
 
@@ -30,6 +31,8 @@ struct wm_adsp_alg_region {
 	unsigned int base;
 };
 
+struct wm_adsp_compr;
+
 struct wm_adsp {
 	const char *part;
 	int num;
@@ -59,6 +62,8 @@ struct wm_adsp {
 
 	struct work_struct boot_work;
 
+	struct wm_adsp_compr *compr;
+
 	struct mutex pwr_lock;
 
 #ifdef CONFIG_DEBUG_FS
@@ -97,4 +102,12 @@ int wm_adsp2_early_event(struct snd_soc_dapm_widget *w,
 int wm_adsp2_event(struct snd_soc_dapm_widget *w,
 		   struct snd_kcontrol *kcontrol, int event);
 
+extern int wm_adsp_compr_open(struct wm_adsp *dsp,
+			      struct snd_compr_stream *stream);
+extern int wm_adsp_compr_free(struct snd_compr_stream *stream);
+extern int wm_adsp_compr_set_params(struct snd_compr_stream *stream,
+				    struct snd_compr_params *params);
+extern int wm_adsp_compr_get_caps(struct snd_compr_stream *stream,
+				  struct snd_compr_caps *caps);
+
 #endif
-- 
2.1.4

Re: [PATCH v3 4/8] ASoC: wm_adsp: Add support for opening a compressed stream

[Mark Brown]

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

On Tue, Dec 15, 2015 at 11:29:45AM +0000, Charles Keepax wrote:

> +int wm_adsp_compr_open(struct wm_adsp *dsp, struct snd_compr_stream *stream)
> +{
> +	struct wm_adsp_compr *compr;
> +	int ret = 0;
> +
> +	mutex_lock(&dsp->pwr_lock);
> +
> +	if (wm_adsp_fw[dsp->fw].num_caps == 0) {
> +		adsp_err(dsp, "Firmware does not support compressed API\n");
> +		ret = -ENXIO;
> +		goto out;
> +	}
> +
> +	if (wm_adsp_fw[dsp->fw].compr_direction != stream->direction) {
> +		adsp_err(dsp, "Firmware does not support stream direction\n");
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	compr = kzalloc(sizeof(*compr), GFP_KERNEL);

You're doing this under lock but not checking for an attempt to allocate
on a DSP already in use.

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

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

Re: [PATCH v3 4/8] ASoC: wm_adsp: Add support for opening a compressed stream

[Charles Keepax]

On Wed, Dec 23, 2015 at 12:14:56AM +0000, Mark Brown wrote:
> On Tue, Dec 15, 2015 at 11:29:45AM +0000, Charles Keepax wrote:
> 
> > +int wm_adsp_compr_open(struct wm_adsp *dsp, struct snd_compr_stream *stream)
> > +{
> > +	struct wm_adsp_compr *compr;
> > +	int ret = 0;
> > +
> > +	mutex_lock(&dsp->pwr_lock);
> > +
> > +	if (wm_adsp_fw[dsp->fw].num_caps == 0) {
> > +		adsp_err(dsp, "Firmware does not support compressed API\n");
> > +		ret = -ENXIO;
> > +		goto out;
> > +	}
> > +
> > +	if (wm_adsp_fw[dsp->fw].compr_direction != stream->direction) {
> > +		adsp_err(dsp, "Firmware does not support stream direction\n");
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> > +
> > +	compr = kzalloc(sizeof(*compr), GFP_KERNEL);
> 
> You're doing this under lock but not checking for an attempt to allocate
> on a DSP already in use.

A check does actually get added later in:

ASoC: wm_adsp: Attach buffers and streams together

I think that is really just a bit of a rebasing messup. I will
pull that forward into this patch for the next spin.

Thanks,
Charles

Re: [PATCH v3 4/8] ASoC: wm_adsp: Add support for opening a compressed stream

[Charles Keepax]

On Wed, Dec 23, 2015 at 10:00:44AM +0000, Charles Keepax wrote:
> On Wed, Dec 23, 2015 at 12:14:56AM +0000, Mark Brown wrote:
> > On Tue, Dec 15, 2015 at 11:29:45AM +0000, Charles Keepax wrote:
> > 
> > > +int wm_adsp_compr_open(struct wm_adsp *dsp, struct snd_compr_stream *stream)
> > > +{
> > > +	struct wm_adsp_compr *compr;
> > > +	int ret = 0;
> > > +
> > > +	mutex_lock(&dsp->pwr_lock);
> > > +
> > > +	if (wm_adsp_fw[dsp->fw].num_caps == 0) {
> > > +		adsp_err(dsp, "Firmware does not support compressed API\n");
> > > +		ret = -ENXIO;
> > > +		goto out;
> > > +	}
> > > +
> > > +	if (wm_adsp_fw[dsp->fw].compr_direction != stream->direction) {
> > > +		adsp_err(dsp, "Firmware does not support stream direction\n");
> > > +		ret = -EINVAL;
> > > +		goto out;
> > > +	}
> > > +
> > > +	compr = kzalloc(sizeof(*compr), GFP_KERNEL);
> > 
> > You're doing this under lock but not checking for an attempt to allocate
> > on a DSP already in use.
> 
> A check does actually get added later in:
> 
> ASoC: wm_adsp: Attach buffers and streams together
> 
> I think that is really just a bit of a rebasing messup. I will
> pull that forward into this patch for the next spin.

Oops.. missed you had applied it anyway. Thanks, will fix up the
comments on the last patch and resend today.

Thanks,
Charles

[PATCH v3 5/8] ASoC: wm_adsp: Add code to locate and initialise compressed buffer

[Charles Keepax]

Add code that locates and initialises the buffer of compressed data on
the DSP if the firmware supported compressed data capture. The buffer
struct (wm_adsp_compr_buf) is kept separate from the stream struct
(wm_adsp_compr) this will allow much easier support of multiple
streams of data from the one DSP in the future, although support for
this will not be added in this patch chain.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c | 291 +++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/wm_adsp.h |   2 +
 2 files changed, 293 insertions(+)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 22b57bc..131b662 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -229,6 +229,58 @@ static const char *wm_adsp_fw_text[WM_ADSP_NUM_FW] = {
 	[WM_ADSP_FW_MISC] =     "Misc",
 };
 
+struct wm_adsp_system_config_xm_hdr {
+	__be32 sys_enable;
+	__be32 fw_id;
+	__be32 fw_rev;
+	__be32 boot_status;
+	__be32 watchdog;
+	__be32 dma_buffer_size;
+	__be32 rdma[6];
+	__be32 wdma[8];
+	__be32 build_job_name[3];
+	__be32 build_job_number;
+};
+
+struct wm_adsp_alg_xm_struct {
+	__be32 magic;
+	__be32 smoothing;
+	__be32 threshold;
+	__be32 host_buf_ptr;
+	__be32 start_seq;
+	__be32 high_water_mark;
+	__be32 low_water_mark;
+	__be64 smoothed_power;
+};
+
+struct wm_adsp_buffer {
+	__be32 X_buf_base;		/* XM base addr of first X area */
+	__be32 X_buf_size;		/* Size of 1st X area in words */
+	__be32 X_buf_base2;		/* XM base addr of 2nd X area */
+	__be32 X_buf_brk;		/* Total X size in words */
+	__be32 Y_buf_base;		/* YM base addr of Y area */
+	__be32 wrap;			/* Total size X and Y in words */
+	__be32 high_water_mark;		/* Point at which IRQ is asserted */
+	__be32 irq_count;		/* bits 1-31 count IRQ assertions */
+	__be32 irq_ack;			/* acked IRQ count, bit 0 enables IRQ */
+	__be32 next_write_index;	/* word index of next write */
+	__be32 next_read_index;		/* word index of next read */
+	__be32 error;			/* error if any */
+	__be32 oldest_block_index;	/* word index of oldest surviving */
+	__be32 requested_rewind;	/* how many blocks rewind was done */
+	__be32 reserved_space;		/* internal */
+	__be32 min_free;		/* min free space since stream start */
+	__be32 blocks_written[2];	/* total blocks written (64 bit) */
+	__be32 words_written[2];	/* total words written (64 bit) */
+};
+
+struct wm_adsp_compr_buf {
+	struct wm_adsp *dsp;
+
+	struct wm_adsp_buffer_region *regions;
+	u32 host_buf_ptr;
+};
+
 struct wm_adsp_compr {
 	struct wm_adsp *dsp;
 
@@ -243,9 +295,53 @@ struct wm_adsp_compr {
 #define WM_ADSP_MIN_FRAGMENT_SIZE      (64 * WM_ADSP_DATA_WORD_SIZE)
 #define WM_ADSP_MAX_FRAGMENT_SIZE      (4096 * WM_ADSP_DATA_WORD_SIZE)
 
+#define WM_ADSP_ALG_XM_STRUCT_MAGIC    0x49aec7
+
+#define HOST_BUFFER_FIELD(field) \
+	(offsetof(struct wm_adsp_buffer, field) / sizeof(__be32))
+
+#define ALG_XM_FIELD(field) \
+	(offsetof(struct wm_adsp_alg_xm_struct, field) / sizeof(__be32))
+
+static int wm_adsp_buffer_init(struct wm_adsp *dsp);
+static int wm_adsp_buffer_free(struct wm_adsp *dsp);
+
+struct wm_adsp_buffer_region {
+	unsigned int offset;
+	unsigned int cumulative_size;
+	unsigned int mem_type;
+	unsigned int base_addr;
+};
+
+struct wm_adsp_buffer_region_def {
+	unsigned int mem_type;
+	unsigned int base_offset;
+	unsigned int size_offset;
+};
+
+static struct wm_adsp_buffer_region_def ez2control_regions[] = {
+	{
+		.mem_type = WMFW_ADSP2_XM,
+		.base_offset = HOST_BUFFER_FIELD(X_buf_base),
+		.size_offset = HOST_BUFFER_FIELD(X_buf_size),
+	},
+	{
+		.mem_type = WMFW_ADSP2_XM,
+		.base_offset = HOST_BUFFER_FIELD(X_buf_base2),
+		.size_offset = HOST_BUFFER_FIELD(X_buf_brk),
+	},
+	{
+		.mem_type = WMFW_ADSP2_YM,
+		.base_offset = HOST_BUFFER_FIELD(Y_buf_base),
+		.size_offset = HOST_BUFFER_FIELD(wrap),
+	},
+};
+
 struct wm_adsp_fw_caps {
 	u32 id;
 	struct snd_codec_desc desc;
+	int num_regions;
+	struct wm_adsp_buffer_region_def *region_defs;
 };
 
 static const struct wm_adsp_fw_caps ez2control_caps[] = {
@@ -257,6 +353,8 @@ static const struct wm_adsp_fw_caps ez2control_caps[] = {
 			.num_sample_rates = 1,
 			.formats = SNDRV_PCM_FMTBIT_S16_LE,
 		},
+		.num_regions = ARRAY_SIZE(ez2control_regions),
+		.region_defs = ez2control_regions,
 	},
 };
 
@@ -2120,6 +2218,10 @@ int wm_adsp2_event(struct snd_soc_dapm_widget *w,
 					 ADSP2_CORE_ENA | ADSP2_START);
 		if (ret != 0)
 			goto err;
+
+		if (wm_adsp_fw[dsp->fw].num_caps != 0)
+			ret = wm_adsp_buffer_init(dsp);
+
 		break;
 
 	case SND_SOC_DAPM_PRE_PMD:
@@ -2154,6 +2256,9 @@ int wm_adsp2_event(struct snd_soc_dapm_widget *w,
 			kfree(alg_region);
 		}
 
+		if (wm_adsp_fw[dsp->fw].num_caps != 0)
+			wm_adsp_buffer_free(dsp);
+
 		mutex_unlock(&dsp->pwr_lock);
 
 		adsp_dbg(dsp, "Shutdown complete\n");
@@ -2363,4 +2468,190 @@ int wm_adsp_compr_get_caps(struct snd_compr_stream *stream,
 }
 EXPORT_SYMBOL_GPL(wm_adsp_compr_get_caps);
 
+static int wm_adsp_read_data_block(struct wm_adsp *dsp, int mem_type,
+				   unsigned int mem_addr,
+				   unsigned int num_words, u32 *data)
+{
+	struct wm_adsp_region const *mem = wm_adsp_find_region(dsp, mem_type);
+	unsigned int i, reg;
+	int ret;
+
+	if (!mem)
+		return -EINVAL;
+
+	reg = wm_adsp_region_to_reg(mem, mem_addr);
+
+	ret = regmap_raw_read(dsp->regmap, reg, data,
+			      sizeof(*data) * num_words);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < num_words; ++i)
+		data[i] = be32_to_cpu(data[i]) & 0x00ffffffu;
+
+	return 0;
+}
+
+static inline int wm_adsp_read_data_word(struct wm_adsp *dsp, int mem_type,
+					 unsigned int mem_addr, u32 *data)
+{
+	return wm_adsp_read_data_block(dsp, mem_type, mem_addr, 1, data);
+}
+
+static int wm_adsp_write_data_word(struct wm_adsp *dsp, int mem_type,
+				   unsigned int mem_addr, u32 data)
+{
+	struct wm_adsp_region const *mem = wm_adsp_find_region(dsp, mem_type);
+	unsigned int reg;
+
+	if (!mem)
+		return -EINVAL;
+
+	reg = wm_adsp_region_to_reg(mem, mem_addr);
+
+	data = cpu_to_be32(data & 0x00ffffffu);
+
+	return regmap_raw_write(dsp->regmap, reg, &data, sizeof(data));
+}
+
+static inline int wm_adsp_buffer_read(struct wm_adsp_compr_buf *buf,
+				      unsigned int field_offset, u32 *data)
+{
+	return wm_adsp_read_data_word(buf->dsp, WMFW_ADSP2_XM,
+				      buf->host_buf_ptr + field_offset, data);
+}
+
+static inline int wm_adsp_buffer_write(struct wm_adsp_compr_buf *buf,
+				       unsigned int field_offset, u32 data)
+{
+	return wm_adsp_write_data_word(buf->dsp, WMFW_ADSP2_XM,
+				       buf->host_buf_ptr + field_offset, data);
+}
+
+static int wm_adsp_buffer_locate(struct wm_adsp_compr_buf *buf)
+{
+	struct wm_adsp_alg_region *alg_region;
+	struct wm_adsp *dsp = buf->dsp;
+	u32 xmalg, addr, magic;
+	int i, ret;
+
+	alg_region = wm_adsp_find_alg_region(dsp, WMFW_ADSP2_XM, dsp->fw_id);
+	xmalg = sizeof(struct wm_adsp_system_config_xm_hdr) / sizeof(__be32);
+
+	addr = alg_region->base + xmalg + ALG_XM_FIELD(magic);
+	ret = wm_adsp_read_data_word(dsp, WMFW_ADSP2_XM, addr, &magic);
+	if (ret < 0)
+		return ret;
+
+	if (magic != WM_ADSP_ALG_XM_STRUCT_MAGIC)
+		return -EINVAL;
+
+	addr = alg_region->base + xmalg + ALG_XM_FIELD(host_buf_ptr);
+	for (i = 0; i < 5; ++i) {
+		ret = wm_adsp_read_data_word(dsp, WMFW_ADSP2_XM, addr,
+					     &buf->host_buf_ptr);
+		if (ret < 0)
+			return ret;
+
+		if (buf->host_buf_ptr)
+			break;
+
+		usleep_range(1000, 2000);
+	}
+
+	if (!buf->host_buf_ptr)
+		return -EIO;
+
+	adsp_dbg(dsp, "host_buf_ptr=%x\n", buf->host_buf_ptr);
+
+	return 0;
+}
+
+static int wm_adsp_buffer_populate(struct wm_adsp_compr_buf *buf)
+{
+	const struct wm_adsp_fw_caps *caps = wm_adsp_fw[buf->dsp->fw].caps;
+	struct wm_adsp_buffer_region *region;
+	u32 offset = 0;
+	int i, ret;
+
+	for (i = 0; i < caps->num_regions; ++i) {
+		region = &buf->regions[i];
+
+		region->offset = offset;
+		region->mem_type = caps->region_defs[i].mem_type;
+
+		ret = wm_adsp_buffer_read(buf, caps->region_defs[i].base_offset,
+					  &region->base_addr);
+		if (ret < 0)
+			return ret;
+
+		ret = wm_adsp_buffer_read(buf, caps->region_defs[i].size_offset,
+					  &offset);
+		if (ret < 0)
+			return ret;
+
+		region->cumulative_size = offset;
+
+		adsp_dbg(buf->dsp,
+			 "region=%d type=%d base=%04x off=%04x size=%04x\n",
+			 i, region->mem_type, region->base_addr,
+			 region->offset, region->cumulative_size);
+	}
+
+	return 0;
+}
+
+static int wm_adsp_buffer_init(struct wm_adsp *dsp)
+{
+	struct wm_adsp_compr_buf *buf;
+	int ret;
+
+	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	buf->dsp = dsp;
+
+	ret = wm_adsp_buffer_locate(buf);
+	if (ret < 0) {
+		adsp_err(dsp, "Failed to acquire host buffer: %d\n", ret);
+		goto err_buffer;
+	}
+
+	buf->regions = kcalloc(wm_adsp_fw[dsp->fw].caps->num_regions,
+			       sizeof(*buf->regions), GFP_KERNEL);
+	if (!buf->regions) {
+		ret = -ENOMEM;
+		goto err_buffer;
+	}
+
+	ret = wm_adsp_buffer_populate(buf);
+	if (ret < 0) {
+		adsp_err(dsp, "Failed to populate host buffer: %d\n", ret);
+		goto err_regions;
+	}
+
+	dsp->buffer = buf;
+
+	return 0;
+
+err_regions:
+	kfree(buf->regions);
+err_buffer:
+	kfree(buf);
+	return ret;
+}
+
+static int wm_adsp_buffer_free(struct wm_adsp *dsp)
+{
+	if (dsp->buffer) {
+		kfree(dsp->buffer->regions);
+		kfree(dsp->buffer);
+
+		dsp->buffer = NULL;
+	}
+
+	return 0;
+}
+
 MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index 33c9b52..0b2205a 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -32,6 +32,7 @@ struct wm_adsp_alg_region {
 };
 
 struct wm_adsp_compr;
+struct wm_adsp_compr_buf;
 
 struct wm_adsp {
 	const char *part;
@@ -63,6 +64,7 @@ struct wm_adsp {
 	struct work_struct boot_work;
 
 	struct wm_adsp_compr *compr;
+	struct wm_adsp_compr_buf *buffer;
 
 	struct mutex pwr_lock;
 
-- 
2.1.4

[PATCH v3 6/8] ASoC: wm_adsp: Attach buffers and streams together

[Charles Keepax]

The stream is created whilst the compressed stream is opened and a
buffer is created when the DSP powers up. It is necessary at a point
once both the DSP has powered up and the the stream has been opened to
connect a stream to a buffer on the DSP. This is done in the trigger
callback as this is after the DSP has been powered and obviously the
stream must be open. Note that whilst the connect is currently trivial
it is expected that this will get more complex when support for multiple
buffers/streams per DSP is added.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm5110.c  |  1 +
 sound/soc/codecs/wm_adsp.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/wm_adsp.h |  1 +
 3 files changed, 64 insertions(+)

diff --git a/sound/soc/codecs/wm5110.c b/sound/soc/codecs/wm5110.c
index 8c0fd91..c364096 100644
--- a/sound/soc/codecs/wm5110.c
+++ b/sound/soc/codecs/wm5110.c
@@ -2272,6 +2272,7 @@ static struct snd_compr_ops wm5110_compr_ops = {
 	.free = wm_adsp_compr_free,
 	.set_params = wm_adsp_compr_set_params,
 	.get_caps = wm_adsp_compr_get_caps,
+	.trigger = wm_adsp_compr_trigger,
 };
 
 static struct snd_soc_platform_driver wm5110_compr_platform = {
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 131b662..415dbbb 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -283,6 +283,7 @@ struct wm_adsp_compr_buf {
 
 struct wm_adsp_compr {
 	struct wm_adsp *dsp;
+	struct wm_adsp_compr_buf *buf;
 
 	struct snd_compr_stream *stream;
 	struct snd_compressed_buffer size;
@@ -2338,6 +2339,13 @@ int wm_adsp_compr_open(struct wm_adsp *dsp, struct snd_compr_stream *stream)
 		goto out;
 	}
 
+	if (dsp->compr) {
+		/* It is expect this limitation will be removed in future */
+		adsp_err(dsp, "Only a single stream supported per DSP\n");
+		ret = -EBUSY;
+		goto out;
+	}
+
 	compr = kzalloc(sizeof(*compr), GFP_KERNEL);
 	if (!compr) {
 		ret = -ENOMEM;
@@ -2654,4 +2662,58 @@ static int wm_adsp_buffer_free(struct wm_adsp *dsp)
 	return 0;
 }
 
+static inline int wm_adsp_compr_attached(struct wm_adsp_compr *compr)
+{
+	return compr->buf != NULL;
+}
+
+static int wm_adsp_compr_attach(struct wm_adsp_compr *compr)
+{
+	/*
+	 * Note this will be more complex once each DSP can support multiple
+	 * streams
+	 */
+	if (!compr->dsp->buffer)
+		return -EINVAL;
+
+	compr->buf = compr->dsp->buffer;
+
+	return 0;
+}
+
+int wm_adsp_compr_trigger(struct snd_compr_stream *stream, int cmd)
+{
+	struct wm_adsp_compr *compr = stream->runtime->private_data;
+	struct wm_adsp *dsp = compr->dsp;
+	int ret = 0;
+
+	adsp_dbg(dsp, "Trigger: %d\n", cmd);
+
+	mutex_lock(&dsp->pwr_lock);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		if (wm_adsp_compr_attached(compr))
+			break;
+
+		ret = wm_adsp_compr_attach(compr);
+		if (ret < 0) {
+			adsp_err(dsp, "Failed to link buffer and stream: %d\n",
+				 ret);
+			break;
+		}
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	mutex_unlock(&dsp->pwr_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(wm_adsp_compr_trigger);
+
 MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index 0b2205a..43af093 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -111,5 +111,6 @@ extern int wm_adsp_compr_set_params(struct snd_compr_stream *stream,
 				    struct snd_compr_params *params);
 extern int wm_adsp_compr_get_caps(struct snd_compr_stream *stream,
 				  struct snd_compr_caps *caps);
+extern int wm_adsp_compr_trigger(struct snd_compr_stream *stream, int cmd);
 
 #endif
-- 
2.1.4

[PATCH v3 7/8] ASoC: wm_adsp: Add a handler for the compressed IRQ

[Charles Keepax]

Here support is added for responding to DSP IRQs that are used to
indicate data being available on the DSP. The idea is that we check the
amount of data available upon receipt of an IRQ and on subsequent calls
to the pointer callback we recheck once less than one fragment is
available (to avoid excessive SPI traffic), if there is truely less than
one fragment available we ack the last IRQ and wait for a new one.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm5110.c  |  24 ++++++
 sound/soc/codecs/wm_adsp.c | 187 +++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/wm_adsp.h |   3 +
 3 files changed, 214 insertions(+)

diff --git a/sound/soc/codecs/wm5110.c b/sound/soc/codecs/wm5110.c
index c364096..c6c176e 100644
--- a/sound/soc/codecs/wm5110.c
+++ b/sound/soc/codecs/wm5110.c
@@ -2177,10 +2177,20 @@ static int wm5110_open(struct snd_compr_stream *stream)
 	return wm_adsp_compr_open(&priv->core.adsp[n_adsp], stream);
 }
 
+static irqreturn_t wm5110_adsp2_irq(int irq, void *data)
+{
+	struct wm5110_priv *florida = data;
+
+	wm_adsp_compr_handle_irq(&florida->core.adsp[2]);
+
+	return IRQ_HANDLED;
+}
+
 static int wm5110_codec_probe(struct snd_soc_codec *codec)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
 	struct wm5110_priv *priv = snd_soc_codec_get_drvdata(codec);
+	struct arizona *arizona = priv->core.arizona;
 	int i, ret;
 
 	priv->core.arizona->dapm = dapm;
@@ -2189,6 +2199,14 @@ static int wm5110_codec_probe(struct snd_soc_codec *codec)
 	arizona_init_gpio(codec);
 	arizona_init_mono(codec);
 
+	ret = arizona_request_irq(arizona, ARIZONA_IRQ_DSP_IRQ1,
+				  "ADSP2 Compressed IRQ", wm5110_adsp2_irq,
+				  priv);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to request DSP IRQ: %d\n", ret);
+		return ret;
+	}
+
 	for (i = 0; i < WM5110_NUM_ADSP; ++i) {
 		ret = wm_adsp2_codec_probe(&priv->core.adsp[i], codec);
 		if (ret)
@@ -2209,12 +2227,15 @@ err_adsp2_codec_probe:
 	for (--i; i >= 0; --i)
 		wm_adsp2_codec_remove(&priv->core.adsp[i], codec);
 
+	arizona_free_irq(arizona, ARIZONA_IRQ_DSP_IRQ1, priv);
+
 	return ret;
 }
 
 static int wm5110_codec_remove(struct snd_soc_codec *codec)
 {
 	struct wm5110_priv *priv = snd_soc_codec_get_drvdata(codec);
+	struct arizona *arizona = priv->core.arizona;
 	int i;
 
 	for (i = 0; i < WM5110_NUM_ADSP; ++i)
@@ -2222,6 +2243,8 @@ static int wm5110_codec_remove(struct snd_soc_codec *codec)
 
 	priv->core.arizona->dapm = NULL;
 
+	arizona_free_irq(arizona, ARIZONA_IRQ_DSP_IRQ1, priv);
+
 	return 0;
 }
 
@@ -2273,6 +2296,7 @@ static struct snd_compr_ops wm5110_compr_ops = {
 	.set_params = wm_adsp_compr_set_params,
 	.get_caps = wm_adsp_compr_get_caps,
 	.trigger = wm_adsp_compr_trigger,
+	.pointer = wm_adsp_compr_pointer,
 };
 
 static struct snd_soc_platform_driver wm5110_compr_platform = {
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 415dbbb..9d68ba5 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -279,6 +279,11 @@ struct wm_adsp_compr_buf {
 
 	struct wm_adsp_buffer_region *regions;
 	u32 host_buf_ptr;
+
+	u32 error;
+	u32 irq_ack;
+	int read_index;
+	int avail;
 };
 
 struct wm_adsp_compr {
@@ -287,6 +292,8 @@ struct wm_adsp_compr {
 
 	struct snd_compr_stream *stream;
 	struct snd_compressed_buffer size;
+
+	unsigned int copied_total;
 };
 
 #define WM_ADSP_DATA_WORD_SIZE         3
@@ -2433,6 +2440,11 @@ static int wm_adsp_compr_check_params(struct snd_compr_stream *stream,
 	return -EINVAL;
 }
 
+static inline unsigned int wm_adsp_compr_frag_words(struct wm_adsp_compr *compr)
+{
+	return compr->size.fragment_size / WM_ADSP_DATA_WORD_SIZE;
+}
+
 int wm_adsp_compr_set_params(struct snd_compr_stream *stream,
 			     struct snd_compr_params *params)
 {
@@ -2619,6 +2631,8 @@ static int wm_adsp_buffer_init(struct wm_adsp *dsp)
 		return -ENOMEM;
 
 	buf->dsp = dsp;
+	buf->read_index = -1;
+	buf->irq_ack = 0xFFFFFFFF;
 
 	ret = wm_adsp_buffer_locate(buf);
 	if (ret < 0) {
@@ -2702,6 +2716,16 @@ int wm_adsp_compr_trigger(struct snd_compr_stream *stream, int cmd)
 				 ret);
 			break;
 		}
+
+		/* Trigger the IRQ at one fragment of data */
+		ret = wm_adsp_buffer_write(compr->buf,
+					   HOST_BUFFER_FIELD(high_water_mark),
+					   wm_adsp_compr_frag_words(compr));
+		if (ret < 0) {
+			adsp_err(dsp, "Failed to set high water mark: %d\n",
+				 ret);
+			break;
+		}
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 		break;
@@ -2716,4 +2740,167 @@ int wm_adsp_compr_trigger(struct snd_compr_stream *stream, int cmd)
 }
 EXPORT_SYMBOL_GPL(wm_adsp_compr_trigger);
 
+static inline int wm_adsp_buffer_size(struct wm_adsp_compr_buf *buf)
+{
+	int last_region = wm_adsp_fw[buf->dsp->fw].caps->num_regions - 1;
+
+	return buf->regions[last_region].cumulative_size;
+}
+
+static int wm_adsp_buffer_update_avail(struct wm_adsp_compr_buf *buf)
+{
+	u32 next_read_index, next_write_index;
+	int write_index, read_index, avail;
+	int ret;
+
+	/* Only sync read index if we haven't already read a valid index */
+	if (buf->read_index < 0) {
+		ret = wm_adsp_buffer_read(buf,
+				HOST_BUFFER_FIELD(next_read_index),
+				&next_read_index);
+		if (ret < 0)
+			return ret;
+
+		read_index = sign_extend32(next_read_index, 23);
+
+		if (read_index < 0) {
+			adsp_dbg(buf->dsp, "Avail check on unstarted stream\n");
+			return 0;
+		}
+
+		buf->read_index = read_index;
+	}
+
+	ret = wm_adsp_buffer_read(buf, HOST_BUFFER_FIELD(next_write_index),
+			&next_write_index);
+	if (ret < 0)
+		return ret;
+
+	write_index = sign_extend32(next_write_index, 23);
+
+	avail = write_index - buf->read_index;
+	if (avail < 0)
+		avail += wm_adsp_buffer_size(buf);
+
+	adsp_dbg(buf->dsp, "readindex=0x%x, writeindex=0x%x, avail=%d\n",
+		 buf->read_index, write_index, avail);
+
+	buf->avail = avail;
+
+	return 0;
+}
+
+int wm_adsp_compr_handle_irq(struct wm_adsp *dsp)
+{
+	struct wm_adsp_compr_buf *buf = dsp->buffer;
+	int ret = 0;
+
+	mutex_lock(&dsp->pwr_lock);
+
+	if (!buf) {
+		adsp_err(dsp, "Spurious buffer IRQ\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	adsp_dbg(dsp, "Handling buffer IRQ\n");
+
+	ret = wm_adsp_buffer_read(buf, HOST_BUFFER_FIELD(error), &buf->error);
+	if (ret < 0) {
+		adsp_err(dsp, "Failed to check buffer error: %d\n", ret);
+		goto out;
+	}
+	if (buf->error != 0) {
+		adsp_err(dsp, "Buffer error occurred: %d\n", buf->error);
+		ret = -EIO;
+		goto out;
+	}
+
+	ret = wm_adsp_buffer_read(buf, HOST_BUFFER_FIELD(irq_count),
+				  &buf->irq_ack);
+	if (ret < 0) {
+		adsp_err(dsp, "Failed to get irq_count: %d\n", ret);
+		goto out;
+	}
+
+	ret = wm_adsp_buffer_update_avail(buf);
+	if (ret < 0) {
+		adsp_err(dsp, "Error reading avail: %d\n", ret);
+		goto out;
+	}
+
+out:
+	mutex_unlock(&dsp->pwr_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(wm_adsp_compr_handle_irq);
+
+static int wm_adsp_buffer_ack_irq(struct wm_adsp_compr_buf *buf)
+{
+	if (buf->irq_ack & 0x01)
+		return 0;
+
+	adsp_dbg(buf->dsp, "Acking buffer IRQ(0x%x)\n", buf->irq_ack);
+
+	buf->irq_ack |= 0x01;
+
+	return wm_adsp_buffer_write(buf, HOST_BUFFER_FIELD(irq_ack),
+				    buf->irq_ack);
+}
+
+int wm_adsp_compr_pointer(struct snd_compr_stream *stream,
+			  struct snd_compr_tstamp *tstamp)
+{
+	struct wm_adsp_compr *compr = stream->runtime->private_data;
+	struct wm_adsp_compr_buf *buf = compr->buf;
+	struct wm_adsp *dsp = compr->dsp;
+	int ret = 0;
+
+	adsp_dbg(dsp, "Pointer request\n");
+
+	mutex_lock(&dsp->pwr_lock);
+
+	if (!compr->buf) {
+		ret = -ENXIO;
+		goto out;
+	}
+
+	if (compr->buf->error) {
+		ret = -EIO;
+		goto out;
+	}
+
+	if (buf->avail < wm_adsp_compr_frag_words(compr)) {
+		ret = wm_adsp_buffer_update_avail(buf);
+		if (ret < 0) {
+			adsp_err(dsp, "Error reading avail: %d\n", ret);
+			goto out;
+		}
+
+		/*
+		 * If we really have less than 1 fragment available ack the
+		 * last DSP IRQ and rely on the IRQ to inform us once a whole
+		 * fragment is available.
+		 */
+		if (buf->avail < wm_adsp_compr_frag_words(compr)) {
+			ret = wm_adsp_buffer_ack_irq(buf);
+			if (ret < 0) {
+				adsp_err(dsp, "Failed to ack buffer IRQ: %d\n",
+					 ret);
+				goto out;
+			}
+		}
+	}
+
+	tstamp->copied_total = compr->copied_total;
+	tstamp->copied_total += buf->avail * WM_ADSP_DATA_WORD_SIZE;
+
+out:
+	mutex_unlock(&dsp->pwr_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(wm_adsp_compr_pointer);
+
 MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index 43af093..522fa1a 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -112,5 +112,8 @@ extern int wm_adsp_compr_set_params(struct snd_compr_stream *stream,
 extern int wm_adsp_compr_get_caps(struct snd_compr_stream *stream,
 				  struct snd_compr_caps *caps);
 extern int wm_adsp_compr_trigger(struct snd_compr_stream *stream, int cmd);
+extern int wm_adsp_compr_handle_irq(struct wm_adsp *dsp);
+extern int wm_adsp_compr_pointer(struct snd_compr_stream *stream,
+				 struct snd_compr_tstamp *tstamp);
 
 #endif
-- 
2.1.4

Re: [PATCH v3 7/8] ASoC: wm_adsp: Add a handler for the compressed IRQ

[Mark Brown]

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

On Tue, Dec 15, 2015 at 11:29:48AM +0000, Charles Keepax wrote:

> +static irqreturn_t wm5110_adsp2_irq(int irq, void *data)
> +{
> +	struct wm5110_priv *florida = data;
> +
> +	wm_adsp_compr_handle_irq(&florida->core.adsp[2]);
> +
> +	return IRQ_HANDLED;
> +}

We unconditionally handle the IRQ...

> +int wm_adsp_compr_handle_irq(struct wm_adsp *dsp)
> +{
> +	struct wm_adsp_compr_buf *buf = dsp->buffer;
> +	int ret = 0;
> +
> +	mutex_lock(&dsp->pwr_lock);
> +
> +	if (!buf) {
> +		adsp_err(dsp, "Spurious buffer IRQ\n");
> +		ret = -EINVAL;
> +		goto out;
> +	}

...though we even have code to handle spurious IRQs.  I'd expect
IRQ_NONE if the interrupt wasn't handled, allowing genirq's error
handling and diagnostics to take effect.

> +static int wm_adsp_buffer_ack_irq(struct wm_adsp_compr_buf *buf)
> +{
> +	if (buf->irq_ack & 0x01)
> +		return 0;
> +
> +	adsp_dbg(buf->dsp, "Acking buffer IRQ(0x%x)\n", buf->irq_ack);
> +
> +	buf->irq_ack |= 0x01;
> +
> +	return wm_adsp_buffer_write(buf, HOST_BUFFER_FIELD(irq_ack),
> +				    buf->irq_ack);
> +}

This is confusing, this isn't actually in the interrupt path...

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

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

Re: [PATCH v3 7/8] ASoC: wm_adsp: Add a handler for the compressed IRQ

[Charles Keepax]

On Wed, Dec 23, 2015 at 12:19:07AM +0000, Mark Brown wrote:
> On Tue, Dec 15, 2015 at 11:29:48AM +0000, Charles Keepax wrote:
> 
> > +static irqreturn_t wm5110_adsp2_irq(int irq, void *data)
> > +{
> > +	struct wm5110_priv *florida = data;
> > +
> > +	wm_adsp_compr_handle_irq(&florida->core.adsp[2]);
> > +
> > +	return IRQ_HANDLED;
> > +}
> 
> We unconditionally handle the IRQ...
> 
> > +int wm_adsp_compr_handle_irq(struct wm_adsp *dsp)
> > +{
> > +	struct wm_adsp_compr_buf *buf = dsp->buffer;
> > +	int ret = 0;
> > +
> > +	mutex_lock(&dsp->pwr_lock);
> > +
> > +	if (!buf) {
> > +		adsp_err(dsp, "Spurious buffer IRQ\n");
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> 
> ...though we even have code to handle spurious IRQs.  I'd expect
> IRQ_NONE if the interrupt wasn't handled, allowing genirq's error
> handling and diagnostics to take effect.

Yeah that seems sensible I will fix that up.

> 
> > +static int wm_adsp_buffer_ack_irq(struct wm_adsp_compr_buf *buf)
> > +{
> > +	if (buf->irq_ack & 0x01)
> > +		return 0;
> > +
> > +	adsp_dbg(buf->dsp, "Acking buffer IRQ(0x%x)\n", buf->irq_ack);
> > +
> > +	buf->irq_ack |= 0x01;
> > +
> > +	return wm_adsp_buffer_write(buf, HOST_BUFFER_FIELD(irq_ack),
> > +				    buf->irq_ack);
> > +}
> 
> This is confusing, this isn't actually in the interrupt path...

Acking the last IRQ basically tells the firmware that it is free
to send another. There is no point in doing so until we have
to wait for data. As we are just actively streaming available
data we can progress along fine without enabling the IRQ.

I am somewhat torn between a comment and renaming the function. I
will try to add some sort of reasonable comment.

Thanks,
Charles

Re: [PATCH v3 7/8] ASoC: wm_adsp: Add a handler for the compressed IRQ

[Mark Brown]

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

On Wed, Dec 23, 2015 at 09:58:06AM +0000, Charles Keepax wrote:
> On Wed, Dec 23, 2015 at 12:19:07AM +0000, Mark Brown wrote:
> > On Tue, Dec 15, 2015 at 11:29:48AM +0000, Charles Keepax wrote:

> > > +static int wm_adsp_buffer_ack_irq(struct wm_adsp_compr_buf *buf)
> > > +{
> > > +	if (buf->irq_ack & 0x01)

> > This is confusing, this isn't actually in the interrupt path...

> Acking the last IRQ basically tells the firmware that it is free
> to send another. There is no point in doing so until we have
> to wait for data. As we are just actively streaming available
> data we can progress along fine without enabling the IRQ.

> I am somewhat torn between a comment and renaming the function. I
> will try to add some sort of reasonable comment.

This doesn't sound like it's really acknowledging an IRQ - you have
level triggered interrupts here so if the interrupt isn't acknowledged
the interrupt handler will constantly be called.

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

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

Re: [PATCH v3 7/8] ASoC: wm_adsp: Add a handler for the compressed IRQ

[Charles Keepax]

On Thu, Dec 24, 2015 at 07:31:37PM +0000, Mark Brown wrote:
> On Wed, Dec 23, 2015 at 09:58:06AM +0000, Charles Keepax wrote:
> > On Wed, Dec 23, 2015 at 12:19:07AM +0000, Mark Brown wrote:
> > > On Tue, Dec 15, 2015 at 11:29:48AM +0000, Charles Keepax wrote:
> 
> > > > +static int wm_adsp_buffer_ack_irq(struct wm_adsp_compr_buf *buf)
> > > > +{
> > > > +	if (buf->irq_ack & 0x01)
> 
> > > This is confusing, this isn't actually in the interrupt path...
> 
> > Acking the last IRQ basically tells the firmware that it is free
> > to send another. There is no point in doing so until we have
> > to wait for data. As we are just actively streaming available
> > data we can progress along fine without enabling the IRQ.
> 
> > I am somewhat torn between a comment and renaming the function. I
> > will try to add some sort of reasonable comment.
> 
> This doesn't sound like it's really acknowledging an IRQ - you have
> level triggered interrupts here so if the interrupt isn't acknowledged
> the interrupt handler will constantly be called.

It kinda is acking the IRQ just at the firmware level, not the
hardware level. The physical IRQ all gets acked through regmap
so that is all handled. This code here lets the firmware know,
which it will then use to decide whether it should send a new IRQ
or not.

I could perhaps rename the function to
wm_adsp_buffer_request_irq? and buf->irq_ack to buf->irq_count?
That might make the usage a little more clear.

Thanks,
Charles

Re: [PATCH v3 7/8] ASoC: wm_adsp: Add a handler for the compressed IRQ

[Mark Brown]

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

On Tue, Dec 29, 2015 at 03:43:13PM +0000, Charles Keepax wrote:
> On Thu, Dec 24, 2015 at 07:31:37PM +0000, Mark Brown wrote:

> > > I am somewhat torn between a comment and renaming the function. I
> > > will try to add some sort of reasonable comment.

> > This doesn't sound like it's really acknowledging an IRQ - you have
> > level triggered interrupts here so if the interrupt isn't acknowledged
> > the interrupt handler will constantly be called.

> It kinda is acking the IRQ just at the firmware level, not the
> hardware level. The physical IRQ all gets acked through regmap
> so that is all handled. This code here lets the firmware know,
> which it will then use to decide whether it should send a new IRQ
> or not.

That's not an interrupt acknowlegement, it's a request for more data.

> I could perhaps rename the function to
> wm_adsp_buffer_request_irq? and buf->irq_ack to buf->irq_count?
> That might make the usage a little more clear.

That might be a bit clearer, yes - it looks like this is a mailbox on
the DSP that you're kicking?

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

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

Re: [PATCH v3 7/8] ASoC: wm_adsp: Add a handler for the compressed IRQ

[Charles Keepax]

On Tue, Jan 05, 2016 at 02:20:25PM +0000, Mark Brown wrote:
> On Tue, Dec 29, 2015 at 03:43:13PM +0000, Charles Keepax wrote:
> > On Thu, Dec 24, 2015 at 07:31:37PM +0000, Mark Brown wrote:
> 
> > > > I am somewhat torn between a comment and renaming the function. I
> > > > will try to add some sort of reasonable comment.
> 
> > > This doesn't sound like it's really acknowledging an IRQ - you have
> > > level triggered interrupts here so if the interrupt isn't acknowledged
> > > the interrupt handler will constantly be called.
> 
> > It kinda is acking the IRQ just at the firmware level, not the
> > hardware level. The physical IRQ all gets acked through regmap
> > so that is all handled. This code here lets the firmware know,
> > which it will then use to decide whether it should send a new IRQ
> > or not.
> 
> That's not an interrupt acknowlegement, it's a request for more data.

Well a request to let us know about there being more data. We will
keep consuming data as it is generated until we reach a point where
we have less than one fragment, then we set this and wait for an
IRQ to say we have more than a fragment again.

> 
> > I could perhaps rename the function to
> > wm_adsp_buffer_request_irq? and buf->irq_ack to buf->irq_count?
> > That might make the usage a little more clear.
> 
> That might be a bit clearer, yes - it looks like this is a mailbox on
> the DSP that you're kicking?

Effectively you could think of it as a mailbox, I haven't looked
much at the framework but I suspect it is a little overkill for
what we want to do here.

Thanks,
Charles

Re: [PATCH v3 7/8] ASoC: wm_adsp: Add a handler for the compressed IRQ

[Mark Brown]

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

On Tue, Jan 05, 2016 at 02:36:36PM +0000, Charles Keepax wrote:
> On Tue, Jan 05, 2016 at 02:20:25PM +0000, Mark Brown wrote:

> > That's not an interrupt acknowlegement, it's a request for more data.

> Well a request to let us know about there being more data. We will
> keep consuming data as it is generated until we reach a point where
> we have less than one fragment, then we set this and wait for an
> IRQ to say we have more than a fragment again.

Whatever it is it's not an interrupt being acknowledged, if anything
it's more one being unmasked but it seems like it's probably just a
general software channel.

> > > I could perhaps rename the function to
> > > wm_adsp_buffer_request_irq? and buf->irq_ack to buf->irq_count?
> > > That might make the usage a little more clear.

> > That might be a bit clearer, yes - it looks like this is a mailbox on
> > the DSP that you're kicking?

> Effectively you could think of it as a mailbox, I haven't looked
> much at the framework but I suspect it is a little overkill for
> what we want to do here.

I'm not suggesting using the framework, I'm saying don't describe it as
an interrupt when it's clearly not one and does things that would be
bugs if it were actually an interrupt.

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

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

[PATCH v3 8/8] ASoC: wm_adsp: Pull data through compressed read

[Charles Keepax]

Data is read in blocks of up to one fragment is size from the circular
buffer on the DSP and is re-packed to remove the padding byte that
exists in the DSP memory map.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm5110.c  |   1 +
 sound/soc/codecs/wm_adsp.c | 138 +++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/wm_adsp.h |   2 +
 3 files changed, 141 insertions(+)

diff --git a/sound/soc/codecs/wm5110.c b/sound/soc/codecs/wm5110.c
index c6c176e..9ed864b 100644
--- a/sound/soc/codecs/wm5110.c
+++ b/sound/soc/codecs/wm5110.c
@@ -2297,6 +2297,7 @@ static struct snd_compr_ops wm5110_compr_ops = {
 	.get_caps = wm_adsp_compr_get_caps,
 	.trigger = wm_adsp_compr_trigger,
 	.pointer = wm_adsp_compr_pointer,
+	.copy = wm_adsp_compr_copy,
 };
 
 static struct snd_soc_platform_driver wm5110_compr_platform = {
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 9d68ba5..fbcd4ce 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -293,6 +293,7 @@ struct wm_adsp_compr {
 	struct snd_compr_stream *stream;
 	struct snd_compressed_buffer size;
 
+	u32 *raw_buf;
 	unsigned int copied_total;
 };
 
@@ -2382,6 +2383,7 @@ int wm_adsp_compr_free(struct snd_compr_stream *stream)
 
 	dsp->compr = NULL;
 
+	kfree(compr->raw_buf);
 	kfree(compr);
 
 	mutex_unlock(&dsp->pwr_lock);
@@ -2449,6 +2451,7 @@ int wm_adsp_compr_set_params(struct snd_compr_stream *stream,
 			     struct snd_compr_params *params)
 {
 	struct wm_adsp_compr *compr = stream->runtime->private_data;
+	unsigned int size;
 	int ret;
 
 	ret = wm_adsp_compr_check_params(stream, params);
@@ -2460,6 +2463,11 @@ int wm_adsp_compr_set_params(struct snd_compr_stream *stream,
 	adsp_dbg(compr->dsp, "fragment_size=%d fragments=%d\n",
 		 compr->size.fragment_size, compr->size.fragments);
 
+	size = wm_adsp_compr_frag_words(compr) * sizeof(*compr->raw_buf);
+	compr->raw_buf = kmalloc(size, GFP_DMA | GFP_KERNEL);
+	if (!compr->raw_buf)
+		return -ENOMEM;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(wm_adsp_compr_set_params);
@@ -2793,6 +2801,7 @@ static int wm_adsp_buffer_update_avail(struct wm_adsp_compr_buf *buf)
 int wm_adsp_compr_handle_irq(struct wm_adsp *dsp)
 {
 	struct wm_adsp_compr_buf *buf = dsp->buffer;
+	struct wm_adsp_compr *compr = dsp->compr;
 	int ret = 0;
 
 	mutex_lock(&dsp->pwr_lock);
@@ -2829,6 +2838,9 @@ int wm_adsp_compr_handle_irq(struct wm_adsp *dsp)
 		goto out;
 	}
 
+	if (compr->stream)
+		snd_compr_fragment_elapsed(compr->stream);
+
 out:
 	mutex_unlock(&dsp->pwr_lock);
 
@@ -2903,4 +2915,130 @@ out:
 }
 EXPORT_SYMBOL_GPL(wm_adsp_compr_pointer);
 
+static int wm_adsp_buffer_capture_block(struct wm_adsp_compr *compr, int target)
+{
+	struct wm_adsp_compr_buf *buf = compr->buf;
+	u8 *pack_in = (u8 *)compr->raw_buf;
+	u8 *pack_out = (u8 *)compr->raw_buf;
+	unsigned int adsp_addr;
+	int mem_type, nwords, max_read;
+	int i, j, ret;
+
+	/* Calculate read parameters */
+	for (i = 0; i < wm_adsp_fw[buf->dsp->fw].caps->num_regions; ++i)
+		if (buf->read_index < buf->regions[i].cumulative_size)
+			break;
+
+	if (i == wm_adsp_fw[buf->dsp->fw].caps->num_regions)
+		return -EINVAL;
+
+	mem_type = buf->regions[i].mem_type;
+	adsp_addr = buf->regions[i].base_addr +
+		    (buf->read_index - buf->regions[i].offset);
+
+	max_read = wm_adsp_compr_frag_words(compr);
+	nwords = buf->regions[i].cumulative_size - buf->read_index;
+
+	if (nwords > target)
+		nwords = target;
+	if (nwords > buf->avail)
+		nwords = buf->avail;
+	if (nwords > max_read)
+		nwords = max_read;
+	if (!nwords)
+		return 0;
+
+	/* Read data from DSP */
+	ret = wm_adsp_read_data_block(buf->dsp, mem_type, adsp_addr,
+				      nwords, compr->raw_buf);
+	if (ret < 0)
+		return ret;
+
+	/* Remove the padding bytes from the data read from the DSP */
+	for (i = 0; i < nwords; i++) {
+		for (j = 0; j < WM_ADSP_DATA_WORD_SIZE; j++)
+			*pack_out++ = *pack_in++;
+
+		pack_in += sizeof(*(compr->raw_buf)) - WM_ADSP_DATA_WORD_SIZE;
+	}
+
+	/* update read index to account for words read */
+	buf->read_index += nwords;
+	if (buf->read_index == wm_adsp_buffer_size(buf))
+		buf->read_index = 0;
+
+	ret = wm_adsp_buffer_write(buf, HOST_BUFFER_FIELD(next_read_index),
+				   buf->read_index);
+	if (ret < 0)
+		return ret;
+
+	/* update avail to account for words read */
+	buf->avail -= nwords;
+
+	return nwords;
+}
+
+static int wm_adsp_compr_read(struct wm_adsp_compr *compr,
+			      char __user *buf, size_t count)
+{
+	struct wm_adsp *dsp = compr->dsp;
+	int ntotal = 0;
+	int nwords, nbytes;
+
+	adsp_dbg(dsp, "Requested read of %zu bytes\n", count);
+
+	if (!compr->buf)
+		return -ENXIO;
+
+	if (compr->buf->error)
+		return -EIO;
+
+	count /= WM_ADSP_DATA_WORD_SIZE;
+
+	do {
+		nwords = wm_adsp_buffer_capture_block(compr, count);
+		if (nwords < 0) {
+			adsp_err(dsp, "Failed to capture block: %d\n", nwords);
+			return nwords;
+		}
+
+		nbytes = nwords * WM_ADSP_DATA_WORD_SIZE;
+
+		adsp_dbg(dsp, "Read %d bytes\n", nbytes);
+
+		if (copy_to_user(buf + ntotal, compr->raw_buf, nbytes)) {
+			adsp_err(dsp, "Failed to copy data to user: %d, %d\n",
+				 ntotal, nbytes);
+			return -EFAULT;
+		}
+
+		count -= nwords;
+		ntotal += nbytes;
+	} while (nwords > 0 && count > 0);
+
+	compr->copied_total += ntotal;
+
+	return ntotal;
+}
+
+int wm_adsp_compr_copy(struct snd_compr_stream *stream, char __user *buf,
+		       size_t count)
+{
+	struct wm_adsp_compr *compr = stream->runtime->private_data;
+	struct wm_adsp *dsp = compr->dsp;
+	int ret;
+
+	mutex_lock(&dsp->pwr_lock);
+
+	if (stream->direction == SND_COMPRESS_CAPTURE)
+		ret = wm_adsp_compr_read(compr, buf, count);
+	else
+		ret = -ENOTSUPP;
+
+	mutex_unlock(&dsp->pwr_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(wm_adsp_compr_copy);
+
 MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index 522fa1a..1a928ec 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -115,5 +115,7 @@ extern int wm_adsp_compr_trigger(struct snd_compr_stream *stream, int cmd);
 extern int wm_adsp_compr_handle_irq(struct wm_adsp *dsp);
 extern int wm_adsp_compr_pointer(struct snd_compr_stream *stream,
 				 struct snd_compr_tstamp *tstamp);
+extern int wm_adsp_compr_copy(struct snd_compr_stream *stream,
+			      char __user *buf, size_t count);
 
 #endif
-- 
2.1.4

Applied "ASoC: wm_adsp: Pull data through compressed read" to the asoc tree

[Mark Brown]

The patch

   ASoC: wm_adsp: Pull data through compressed read

has been applied to the asoc tree at

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

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

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

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

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

Thanks,
Mark

>From 83a40ce993cda0757b102389e38446e79a2cc172 Mon Sep 17 00:00:00 2001
From: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Date: Wed, 6 Jan 2016 12:33:19 +0000
Subject: [PATCH] ASoC: wm_adsp: Pull data through compressed read

Data is read in blocks of up to one fragment is size from the circular
buffer on the DSP and is re-packed to remove the padding byte that
exists in the DSP memory map.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/wm5110.c  |   1 +
 sound/soc/codecs/wm_adsp.c | 138 +++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/wm_adsp.h |   2 +
 3 files changed, 141 insertions(+)

diff --git a/sound/soc/codecs/wm5110.c b/sound/soc/codecs/wm5110.c
index dde94c4a0caa..61fa7cc91d15 100644
--- a/sound/soc/codecs/wm5110.c
+++ b/sound/soc/codecs/wm5110.c
@@ -2300,6 +2300,7 @@ static struct snd_compr_ops wm5110_compr_ops = {
 	.get_caps = wm_adsp_compr_get_caps,
 	.trigger = wm_adsp_compr_trigger,
 	.pointer = wm_adsp_compr_pointer,
+	.copy = wm_adsp_compr_copy,
 };
 
 static struct snd_soc_platform_driver wm5110_compr_platform = {
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 49ef0bbe9892..33806d487b8a 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -293,6 +293,7 @@ struct wm_adsp_compr {
 	struct snd_compr_stream *stream;
 	struct snd_compressed_buffer size;
 
+	u32 *raw_buf;
 	unsigned int copied_total;
 };
 
@@ -2385,6 +2386,7 @@ int wm_adsp_compr_free(struct snd_compr_stream *stream)
 
 	dsp->compr = NULL;
 
+	kfree(compr->raw_buf);
 	kfree(compr);
 
 	mutex_unlock(&dsp->pwr_lock);
@@ -2452,6 +2454,7 @@ int wm_adsp_compr_set_params(struct snd_compr_stream *stream,
 			     struct snd_compr_params *params)
 {
 	struct wm_adsp_compr *compr = stream->runtime->private_data;
+	unsigned int size;
 	int ret;
 
 	ret = wm_adsp_compr_check_params(stream, params);
@@ -2463,6 +2466,11 @@ int wm_adsp_compr_set_params(struct snd_compr_stream *stream,
 	adsp_dbg(compr->dsp, "fragment_size=%d fragments=%d\n",
 		 compr->size.fragment_size, compr->size.fragments);
 
+	size = wm_adsp_compr_frag_words(compr) * sizeof(*compr->raw_buf);
+	compr->raw_buf = kmalloc(size, GFP_DMA | GFP_KERNEL);
+	if (!compr->raw_buf)
+		return -ENOMEM;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(wm_adsp_compr_set_params);
@@ -2796,6 +2804,7 @@ static int wm_adsp_buffer_update_avail(struct wm_adsp_compr_buf *buf)
 int wm_adsp_compr_handle_irq(struct wm_adsp *dsp)
 {
 	struct wm_adsp_compr_buf *buf = dsp->buffer;
+	struct wm_adsp_compr *compr = dsp->compr;
 	int ret = 0;
 
 	mutex_lock(&dsp->pwr_lock);
@@ -2832,6 +2841,9 @@ int wm_adsp_compr_handle_irq(struct wm_adsp *dsp)
 		goto out;
 	}
 
+	if (compr->stream)
+		snd_compr_fragment_elapsed(compr->stream);
+
 out:
 	mutex_unlock(&dsp->pwr_lock);
 
@@ -2907,4 +2919,130 @@ out:
 }
 EXPORT_SYMBOL_GPL(wm_adsp_compr_pointer);
 
+static int wm_adsp_buffer_capture_block(struct wm_adsp_compr *compr, int target)
+{
+	struct wm_adsp_compr_buf *buf = compr->buf;
+	u8 *pack_in = (u8 *)compr->raw_buf;
+	u8 *pack_out = (u8 *)compr->raw_buf;
+	unsigned int adsp_addr;
+	int mem_type, nwords, max_read;
+	int i, j, ret;
+
+	/* Calculate read parameters */
+	for (i = 0; i < wm_adsp_fw[buf->dsp->fw].caps->num_regions; ++i)
+		if (buf->read_index < buf->regions[i].cumulative_size)
+			break;
+
+	if (i == wm_adsp_fw[buf->dsp->fw].caps->num_regions)
+		return -EINVAL;
+
+	mem_type = buf->regions[i].mem_type;
+	adsp_addr = buf->regions[i].base_addr +
+		    (buf->read_index - buf->regions[i].offset);
+
+	max_read = wm_adsp_compr_frag_words(compr);
+	nwords = buf->regions[i].cumulative_size - buf->read_index;
+
+	if (nwords > target)
+		nwords = target;
+	if (nwords > buf->avail)
+		nwords = buf->avail;
+	if (nwords > max_read)
+		nwords = max_read;
+	if (!nwords)
+		return 0;
+
+	/* Read data from DSP */
+	ret = wm_adsp_read_data_block(buf->dsp, mem_type, adsp_addr,
+				      nwords, compr->raw_buf);
+	if (ret < 0)
+		return ret;
+
+	/* Remove the padding bytes from the data read from the DSP */
+	for (i = 0; i < nwords; i++) {
+		for (j = 0; j < WM_ADSP_DATA_WORD_SIZE; j++)
+			*pack_out++ = *pack_in++;
+
+		pack_in += sizeof(*(compr->raw_buf)) - WM_ADSP_DATA_WORD_SIZE;
+	}
+
+	/* update read index to account for words read */
+	buf->read_index += nwords;
+	if (buf->read_index == wm_adsp_buffer_size(buf))
+		buf->read_index = 0;
+
+	ret = wm_adsp_buffer_write(buf, HOST_BUFFER_FIELD(next_read_index),
+				   buf->read_index);
+	if (ret < 0)
+		return ret;
+
+	/* update avail to account for words read */
+	buf->avail -= nwords;
+
+	return nwords;
+}
+
+static int wm_adsp_compr_read(struct wm_adsp_compr *compr,
+			      char __user *buf, size_t count)
+{
+	struct wm_adsp *dsp = compr->dsp;
+	int ntotal = 0;
+	int nwords, nbytes;
+
+	adsp_dbg(dsp, "Requested read of %zu bytes\n", count);
+
+	if (!compr->buf)
+		return -ENXIO;
+
+	if (compr->buf->error)
+		return -EIO;
+
+	count /= WM_ADSP_DATA_WORD_SIZE;
+
+	do {
+		nwords = wm_adsp_buffer_capture_block(compr, count);
+		if (nwords < 0) {
+			adsp_err(dsp, "Failed to capture block: %d\n", nwords);
+			return nwords;
+		}
+
+		nbytes = nwords * WM_ADSP_DATA_WORD_SIZE;
+
+		adsp_dbg(dsp, "Read %d bytes\n", nbytes);
+
+		if (copy_to_user(buf + ntotal, compr->raw_buf, nbytes)) {
+			adsp_err(dsp, "Failed to copy data to user: %d, %d\n",
+				 ntotal, nbytes);
+			return -EFAULT;
+		}
+
+		count -= nwords;
+		ntotal += nbytes;
+	} while (nwords > 0 && count > 0);
+
+	compr->copied_total += ntotal;
+
+	return ntotal;
+}
+
+int wm_adsp_compr_copy(struct snd_compr_stream *stream, char __user *buf,
+		       size_t count)
+{
+	struct wm_adsp_compr *compr = stream->runtime->private_data;
+	struct wm_adsp *dsp = compr->dsp;
+	int ret;
+
+	mutex_lock(&dsp->pwr_lock);
+
+	if (stream->direction == SND_COMPRESS_CAPTURE)
+		ret = wm_adsp_compr_read(compr, buf, count);
+	else
+		ret = -ENOTSUPP;
+
+	mutex_unlock(&dsp->pwr_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(wm_adsp_compr_copy);
+
 MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index 522fa1ada4e6..1a928ec54741 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -115,5 +115,7 @@ extern int wm_adsp_compr_trigger(struct snd_compr_stream *stream, int cmd);
 extern int wm_adsp_compr_handle_irq(struct wm_adsp *dsp);
 extern int wm_adsp_compr_pointer(struct snd_compr_stream *stream,
 				 struct snd_compr_tstamp *tstamp);
+extern int wm_adsp_compr_copy(struct snd_compr_stream *stream,
+			      char __user *buf, size_t count);
 
 #endif
-- 
2.7.0.rc3