[PATCH v2 0/9] Add DSP topology management for SKL

[PATCH v2 0/9] Add DSP topology management for SKL

[Vinod Koul]

The SKL driver does not code DSP topology in driver. With this series
the ASoC topology framework is used to parse the topology information
to create widgets, controls and route map for the FW topology.

It adds routines for SKL DSP module configuration, compute resources
for modules, initialize and bind the pipe modules. It uses the SKL IPC
library added earlier to send IPC for initialize the module, bind/unbind
modules.
Last patch in this series removes the unused dais.

changes in v2:
- add more explanation in changelog and code
- add explanation of MCPS
- fix whitespace issues
- remove skl widget check on event handler as it is redundant
- add switch for depth
- add else for direction based code
- remove cast for NHLT blob query
- make debug prints to error
- remove dump info
- clean some debug prints

Jeeja KP (9):
  ASoC: Intel: Skylake: Add pipe and modules handlers
  ASoC: Intel: Skylake: Add module configuration helpers
  ASoC: Intel: Skylake: add DSP platform widget event handlers
  ASoC: Intel: Skylake: Add FE and BE hw_params handling
  ASoC: Intel: Skylake: Add topology core init and handlers
  ASoC: Intel: Skylake: Initialize and load DSP controls
  ASoC: Intel: Skylake: Add DSP support and enable it
  ASoC: Intel: Skylake: Initialize NHLT table
  ASoC: Intel: Skylake: Remove unused CPU dai's

 sound/soc/intel/skylake/Makefile             |    3 +-
 sound/soc/intel/skylake/skl-pcm.c            |  171 ++--
 sound/soc/intel/skylake/skl-topology.c       | 1262 ++++++++++++++++++++++++++
 sound/soc/intel/skylake/skl-topology.h       |   21 +
 sound/soc/intel/skylake/skl-tplg-interface.h |   78 ++
 sound/soc/intel/skylake/skl.c                |   29 +-
 sound/soc/intel/skylake/skl.h                |   11 +
 7 files changed, 1507 insertions(+), 68 deletions(-)
 create mode 100644 sound/soc/intel/skylake/skl-topology.c

-- 
1.9.1

[PATCH v2 1/9] ASoC: Intel: Skylake: Add pipe and modules handlers

[Vinod Koul]

From: Jeeja KP <jeeja.kp@intel.com>

SKL driver needs to instantiate pipelines and modules in the DSP.
The topology in the DSP is modelled as DAPM graph with a PGA
representing a module instance and mixer representing a pipeline
for a group of modules along with the mixer itself.

Here we start adding building block for handling these. We add
resource checks (memory/compute) for pipelines, find the modules
in a pipeline, init modules in a pipe and lastly bind/unbind
modules in a pipe These will be used by pipe event handlers in
subsequent patches

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/intel/skylake/Makefile       |   3 +-
 sound/soc/intel/skylake/skl-topology.c | 220 +++++++++++++++++++++++++++++++++
 sound/soc/intel/skylake/skl-topology.h |  10 ++
 sound/soc/intel/skylake/skl.h          |  11 ++
 4 files changed, 243 insertions(+), 1 deletion(-)
 create mode 100644 sound/soc/intel/skylake/skl-topology.c

diff --git a/sound/soc/intel/skylake/Makefile b/sound/soc/intel/skylake/Makefile
index 27db22178204..914b6dab9bea 100644
--- a/sound/soc/intel/skylake/Makefile
+++ b/sound/soc/intel/skylake/Makefile
@@ -1,4 +1,5 @@
-snd-soc-skl-objs := skl.o skl-pcm.o skl-nhlt.o skl-messages.o
+snd-soc-skl-objs := skl.o skl-pcm.o skl-nhlt.o skl-messages.o \
+skl-topology.o
 
 obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE) += snd-soc-skl.o
 
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
new file mode 100644
index 000000000000..6d0eee6a04ff
--- /dev/null
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -0,0 +1,220 @@
+/*
+ *  skl-topology.c - Implements Platform component ALSA controls/widget
+ *  handlers.
+ *
+ *  Copyright (C) 2014-2015 Intel Corp
+ *  Author: Jeeja KP <jeeja.kp@intel.com>
+ *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/firmware.h>
+#include <sound/soc.h>
+#include <sound/soc-topology.h>
+#include "skl-sst-dsp.h"
+#include "skl-sst-ipc.h"
+#include "skl-topology.h"
+#include "skl.h"
+#include "skl-tplg-interface.h"
+
+/*
+ * SKL DSP driver modelling uses only few DAPM widgets so for rest we will
+ * ignore. This helpers checks if the SKL driver handles this widget type
+ */
+static int is_skl_dsp_widget_type(struct snd_soc_dapm_widget *w)
+{
+	switch (w->id) {
+	case snd_soc_dapm_dai_link:
+	case snd_soc_dapm_dai_in:
+	case snd_soc_dapm_aif_in:
+	case snd_soc_dapm_aif_out:
+	case snd_soc_dapm_dai_out:
+	case snd_soc_dapm_switch:
+		return false;
+	default:
+		return true;
+	}
+}
+
+static int skl_tplg_bind_unbind_pipes(struct skl_module_cfg *src_module,
+	struct skl_module_cfg *sink_module, struct skl_sst *ctx, bool bind)
+{
+	int ret;
+
+	if (!bind) {
+		ret = skl_stop_pipe(ctx, src_module->pipe);
+		if (ret < 0)
+			return ret;
+
+		ret = skl_unbind_modules(ctx, src_module, sink_module);
+	} else {
+		ret = skl_bind_modules(ctx, src_module, sink_module);
+	}
+
+	return ret;
+}
+
+/*
+ * Each pipelines needs memory to be allocated. Check if we have free memory
+ * from available pool. Then only add this to pool
+ * This is freed when pipe is deleted
+ * Note: DSP does actual memory management we only keep track for complete
+ * pool
+ */
+static bool skl_tplg_is_pipe_mem_available(struct skl *skl,
+				struct skl_module_cfg *mconfig)
+{
+	struct skl_sst *ctx = skl->skl_sst;
+
+	dev_dbg(ctx->dev, "%s: module_id =%d instance=%d\n", __func__,
+		 mconfig->id.module_id, mconfig->id.instance_id);
+
+	if (skl->resource.mem + mconfig->pipe->memory_pages > skl->resource.max_mem) {
+		dev_err(ctx->dev, "exceeds ppl memory available=%d > mem=%d\n",
+				skl->resource.max_mem, skl->resource.mem);
+		return false;
+	}
+
+	skl->resource.mem += mconfig->pipe->memory_pages;
+	return true;
+}
+
+/*
+ * Pipeline needs needs DSP CPU resources for computation, this is quantified
+ * in MCPS (Million Clocks Per Second) required for module/pipe
+ *
+ * Each pipelines needs mcps to be allocated. Check if we have mcps for this
+ * pipe. This adds the mcps to driver counter
+ * This is removed on pipeline delete
+ */
+static bool skl_tplg_is_pipe_mcps_available(struct skl *skl,
+				struct skl_module_cfg *mconfig)
+{
+	struct skl_sst *ctx = skl->skl_sst;
+
+	dev_dbg(ctx->dev, "%s: module_id = %d instance=%d\n", __func__,
+			mconfig->id.module_id, mconfig->id.instance_id);
+
+	if (skl->resource.mcps + mconfig->mcps > skl->resource.max_mcps) {
+		dev_err(ctx->dev, "exceeds ppl memory available=%d > mem=%d\n",
+				skl->resource.max_mcps, skl->resource.mcps);
+		return false;
+	}
+
+	skl->resource.mcps += mconfig->mcps;
+	return true;
+}
+
+/*
+ * A pipe can have multiple modules each of the will be a DAPM widget as
+ * well. While managing a pipeline we need to get the list of all the
+ * widgets in a pipelines, so this helper - skl_tplg_get_pipe_widget() helps
+ * to get the SKL type widgets in that pipeline
+ */
+static int skl_tplg_get_pipe_widget(struct device *dev,
+	struct snd_soc_dapm_widget *w, struct skl_pipe *pipe)
+{
+	struct skl_module_cfg *src_module = NULL;
+	struct snd_soc_dapm_path *p = NULL;
+	struct skl_pipe_module *p_module = NULL;
+
+	p_module = devm_kzalloc(dev, sizeof(*p_module), GFP_KERNEL);
+	if (!p_module)
+		return -ENOMEM;
+
+	p_module->w = w;
+	list_add_tail(&p_module->node, &pipe->w_list);
+
+	list_for_each_entry(p, &w->sinks, list_source) {
+		if ((p->sink->priv == NULL)
+			&& (!is_skl_dsp_widget_type(w)))
+			continue;
+
+		if ((p->sink->priv != NULL) && (p->connect)
+			&& (is_skl_dsp_widget_type(p->sink))) {
+			src_module = p->sink->priv;
+			if (pipe->ppl_id == src_module->pipe->ppl_id) {
+				dev_dbg(dev, "found widget=%s\n", p->sink->name);
+				skl_tplg_get_pipe_widget(dev, p->sink, pipe);
+			}
+		}
+	}
+	return 0;
+}
+
+/*
+ * Inside a pipe instance, we can have various modules. These modules need
+ * to instantiated in DSP by invoking INIT_MODULE IPC, which is achieved by
+ * skl_init_module() routine, so invoke that for all modules in a pipeline
+ */
+static int skl_tplg_init_pipe_modules(struct skl *skl, struct skl_pipe *pipe)
+{
+	struct skl_pipe_module *w_module;
+	struct snd_soc_dapm_widget *w;
+	struct skl_module_cfg *mconfig;
+	struct skl_sst *ctx = skl->skl_sst;
+	int ret = 0;
+
+	dev_dbg(ctx->dev, "%s: pipe=%d\n", __func__, pipe->ppl_id);
+	list_for_each_entry(w_module, &pipe->w_list, node) {
+		w = w_module->w;
+		dev_dbg(ctx->dev, "Pipe Module =%s\n", w->name);
+		mconfig = w->priv;
+
+		/* check resource available */
+		if (!skl_tplg_is_pipe_mcps_available(skl, mconfig))
+			return -ENOMEM;
+
+		ret = skl_init_module(ctx, mconfig, NULL);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+/*
+ * Once all the modules in a pipe are instantiated, they need to be
+ * connected.
+ * On removal, before deleting a pipeline the modules need to disconnected.
+ *
+ * This is achieved by binding/unbinding these modules
+ */
+static int skl_tplg_bind_unbind_pipe_modules(struct skl_sst *ctx,
+				struct skl_pipe *pipe, bool bind)
+{
+	struct skl_pipe_module *w_module;
+	struct skl_module_cfg *src_module = NULL;
+	struct skl_module_cfg *dst_module;
+	int ret = 0;
+
+	dev_dbg(ctx->dev, "%s: pipe=%d\n", __func__, pipe->ppl_id);
+	list_for_each_entry(w_module, &pipe->w_list, node) {
+		dst_module = w_module->w->priv;
+
+		if (src_module == NULL) {
+			src_module = dst_module;
+			continue;
+		}
+
+		if (bind)
+			ret = skl_bind_modules(ctx, src_module, dst_module);
+		else
+			ret = skl_unbind_modules(ctx, src_module, dst_module);
+		if (ret < 0)
+			return ret;
+
+		src_module = dst_module;
+	}
+	return 0;
+}
diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h
index 8c7767baa94f..73d7916ee33e 100644
--- a/sound/soc/intel/skylake/skl-topology.h
+++ b/sound/soc/intel/skylake/skl-topology.h
@@ -263,6 +263,16 @@ struct skl_module_cfg {
 	struct skl_specific_cfg formats_config;
 };
 
+struct skl_pipeline {
+	struct skl_pipe *pipe;
+	struct list_head node;
+};
+
+struct skl_dapm_path_list {
+	struct snd_soc_dapm_path *dapm_path;
+	struct list_head node;
+};
+
 int skl_create_pipeline(struct skl_sst *ctx, struct skl_pipe *pipe);
 
 int skl_run_pipe(struct skl_sst *ctx, struct skl_pipe *pipe);
diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
index f7fdbb02947f..e980d7897642 100644
--- a/sound/soc/intel/skylake/skl.h
+++ b/sound/soc/intel/skylake/skl.h
@@ -48,6 +48,13 @@
 #define AZX_REG_VS_SDXEFIFOS_XBASE	0x1094
 #define AZX_REG_VS_SDXEFIFOS_XINTERVAL	0x20
 
+struct skl_dsp_resource {
+	u32 max_mcps;
+	u32 max_mem;
+	u32 mcps;
+	u32 mem;
+};
+
 struct skl {
 	struct hdac_ext_bus ebus;
 	struct pci_dev *pci;
@@ -57,6 +64,10 @@ struct skl {
 
 	void __iomem *nhlt; /* nhlt ptr */
 	struct skl_sst *skl_sst; /* sst skl ctx */
+
+	struct skl_dsp_resource resource;
+	struct list_head ppl_list;
+	struct list_head dapm_path_list;
 };
 
 #define skl_to_ebus(s)	(&(s)->ebus)
-- 
1.9.1

Re: [PATCH v2 1/9] ASoC: Intel: Skylake: Add pipe and modules handlers

[Mark Brown]

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

On Mon, Aug 17, 2015 at 10:56:36PM +0530, Vinod Koul wrote:

> +static int skl_tplg_bind_unbind_pipes(struct skl_module_cfg *src_module,
> +	struct skl_module_cfg *sink_module, struct skl_sst *ctx, bool bind)
> +{
> +	int ret;
> +
> +	if (!bind) {
> +		ret = skl_stop_pipe(ctx, src_module->pipe);
> +		if (ret < 0)
> +			return ret;
> +
> +		ret = skl_unbind_modules(ctx, src_module, sink_module);
> +	} else {
> +		ret = skl_bind_modules(ctx, src_module, sink_module);
> +	}
> +
> +	return ret;
> +}

Can we *please* stop having these functions which optionally do several
different things with nothing at all shared between the different paths?
It just adds a layer of indentation in the function and makes everything
harder to read (including at the call site - how does the reader know if
a given call will bind or unbind?).

I'm sure I've raised this before.

> +static bool skl_tplg_is_pipe_mem_available(struct skl *skl,
> +                               struct skl_module_cfg *mconfig)

I'm not seeing any users of this function (unlike the mcps checker).

> +	if (skl->resource.mem + mconfig->pipe->memory_pages > skl->resource.max_mem) {
> +		dev_err(ctx->dev, "exceeds ppl memory available=%d > mem=%d\n",
> +				skl->resource.max_mem, skl->resource.mem);
> +		return false;
> +	}

I'm not sure how the user is going to be able to tell what exceeded the
maximum memory here.  

> +static bool skl_tplg_is_pipe_mcps_available(struct skl *skl,
> +				struct skl_module_cfg *mconfig)

This looks an awful lot like the memory check.  Can we make a struct or
ideally array for the constraints and then have a single function which
checks them all?

> +	list_for_each_entry(p, &w->sinks, list_source) {
> +		if ((p->sink->priv == NULL)
> +			&& (!is_skl_dsp_widget_type(w)))
> +			continue;
> +
> +		if ((p->sink->priv != NULL) && (p->connect)
> +			&& (is_skl_dsp_widget_type(p->sink))) {

This is harder to read with the extra brackets.

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

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

Re: [PATCH v2 1/9] ASoC: Intel: Skylake: Add pipe and modules handlers

[Vinod Koul]

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

On Sat, Sep 19, 2015 at 09:00:23AM -0700, Mark Brown wrote:
> On Mon, Aug 17, 2015 at 10:56:36PM +0530, Vinod Koul wrote:
> 
> > +static int skl_tplg_bind_unbind_pipes(struct skl_module_cfg *src_module,
> > +	struct skl_module_cfg *sink_module, struct skl_sst *ctx, bool bind)
> > +{
> > +	int ret;
> > +
> > +	if (!bind) {
> > +		ret = skl_stop_pipe(ctx, src_module->pipe);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		ret = skl_unbind_modules(ctx, src_module, sink_module);
> > +	} else {
> > +		ret = skl_bind_modules(ctx, src_module, sink_module);
> > +	}
> > +
> > +	return ret;
> > +}
> 
> Can we *please* stop having these functions which optionally do several
> different things with nothing at all shared between the different paths?
> It just adds a layer of indentation in the function and makes everything
> harder to read (including at the call site - how does the reader know if
> a given call will bind or unbind?).

Well while reading based on the argument bind we would know if we are doing
bind or unbind. While binding we call only bind. On unbind we have to stop
and then unbind.

I will remove this and few other wrappers like this which will help in
readability and reviews

> 
> I'm sure I've raised this before.
Sorry to miss that, will fix this time for sure


> > +static bool skl_tplg_is_pipe_mem_available(struct skl *skl,
> > +                               struct skl_module_cfg *mconfig)
> 
> I'm not seeing any users of this function (unlike the mcps checker).

It is called in skl_tplg_mixer_dapm_pre_pmu_event() which is in Patch 3 of
this series.

> > +	if (skl->resource.mem + mconfig->pipe->memory_pages > skl->resource.max_mem) {
> > +		dev_err(ctx->dev, "exceeds ppl memory available=%d > mem=%d\n",
> > +				skl->resource.max_mem, skl->resource.mem);
> > +		return false;
> > +	}
> 
> I'm not sure how the user is going to be able to tell what exceeded the
> maximum memory here.  

Module Id and Instance Id are printed before this but yes that is debug, so
merging the two to error alone makes sense, will do that here


> > +static bool skl_tplg_is_pipe_mcps_available(struct skl *skl,
> > +				struct skl_module_cfg *mconfig)
> 
> This looks an awful lot like the memory check.  Can we make a struct or
> ideally array for the constraints and then have a single function which
> checks them all?

No it only two checks, one for MCPS and one for memory. We cannot have
single function as they are checked at two places. Due to FW construction.
MCPS is property of each module whereas memory is for complete pipe.
For each module while initialization we check MCPS whereas for pipe creation
we check memory


> > +	list_for_each_entry(p, &w->sinks, list_source) {
> > +		if ((p->sink->priv == NULL)
> > +			&& (!is_skl_dsp_widget_type(w)))
> > +			continue;
> > +
> > +		if ((p->sink->priv != NULL) && (p->connect)
> > +			&& (is_skl_dsp_widget_type(p->sink))) {
> 
> This is harder to read with the extra brackets.
Will fix this

-- 
~Vinod

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

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

Re: [PATCH v2 1/9] ASoC: Intel: Skylake: Add pipe and modules handlers

[Mark Brown]

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

On Mon, Sep 21, 2015 at 09:07:13AM +0530, Vinod Koul wrote:
> On Sat, Sep 19, 2015 at 09:00:23AM -0700, Mark Brown wrote:
> > On Mon, Aug 17, 2015 at 10:56:36PM +0530, Vinod Koul wrote:

> > > +	if (skl->resource.mem + mconfig->pipe->memory_pages > skl->resource.max_mem) {
> > > +		dev_err(ctx->dev, "exceeds ppl memory available=%d > mem=%d\n",
> > > +				skl->resource.max_mem, skl->resource.mem);
> > > +		return false;
> > > +	}

> > I'm not sure how the user is going to be able to tell what exceeded the
> > maximum memory here.  

> Module Id and Instance Id are printed before this but yes that is debug, so
> merging the two to error alone makes sense, will do that here

They're printed at debug level so might not appear but this is an error
message.

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

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

[PATCH v2 2/9] ASoC: Intel: Skylake: Add module configuration helpers

[Vinod Koul]

From: Jeeja KP <jeeja.kp@intel.com>

To configure a module, driver needs to send input and output PCM
params for a module in DSP. The FE PCM params come from hw_params
ie from user, for a BE they also come from hw_params but from
BE-link fixups.
So based on PCM params required driver has to find a converter
module (src/updown/format) and then do the conversion and
calculate PCM params in these pipelines In this patch we add the
helper modules which allow driver to do these calculations.

Signed-off-by: Hardik T Shah <hardik.t.shah@intel.com>
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/intel/skylake/skl-topology.c | 126 +++++++++++++++++++++++++++++++++
 1 file changed, 126 insertions(+)

diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 6d0eee6a04ff..81b0683bbdeb 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -27,6 +27,10 @@
 #include "skl.h"
 #include "skl-tplg-interface.h"
 
+#define SKL_CH_FIXUP_MASK		(1 << 0)
+#define SKL_RATE_FIXUP_MASK		(1 << 1)
+#define SKL_FMT_FIXUP_MASK		(1 << 2)
+
 /*
  * SKL DSP driver modelling uses only few DAPM widgets so for rest we will
  * ignore. This helpers checks if the SKL driver handles this widget type
@@ -115,6 +119,126 @@ static bool skl_tplg_is_pipe_mcps_available(struct skl *skl,
 	return true;
 }
 
+static void skl_dump_mconfig(struct skl_sst *ctx,
+					struct skl_module_cfg *mcfg)
+{
+	dev_dbg(ctx->dev, "Dumping config\n");
+	dev_dbg(ctx->dev, "Input Format:\n");
+	dev_dbg(ctx->dev, "channels = %d\n", mcfg->in_fmt.channels);
+	dev_dbg(ctx->dev, "s_freq = %d\n", mcfg->in_fmt.s_freq);
+	dev_dbg(ctx->dev, "ch_cfg = %d\n", mcfg->in_fmt.ch_cfg);
+	dev_dbg(ctx->dev, "valid bit depth = %d\n", mcfg->in_fmt.valid_bit_depth);
+	dev_dbg(ctx->dev, "Output Format:\n");
+	dev_dbg(ctx->dev, "channels = %d\n", mcfg->out_fmt.channels);
+	dev_dbg(ctx->dev, "s_freq = %d\n", mcfg->out_fmt.s_freq);
+	dev_dbg(ctx->dev, "valid bit depth = %d\n", mcfg->out_fmt.valid_bit_depth);
+	dev_dbg(ctx->dev, "ch_cfg = %d\n", mcfg->out_fmt.ch_cfg);
+}
+
+static void skl_tplg_update_params(struct skl_module_fmt *fmt,
+			struct skl_pipe_params *params, int fixup)
+{
+	if (fixup & SKL_RATE_FIXUP_MASK)
+		fmt->s_freq = params->s_freq;
+	if (fixup & SKL_CH_FIXUP_MASK)
+		fmt->channels = params->ch;
+	if (fixup & SKL_FMT_FIXUP_MASK)
+		fmt->valid_bit_depth = params->s_fmt;
+}
+
+/*
+ * A pipeline may have modules which impact the pcm parameters, like SRC,
+ * channel converter, format converter.
+ * We need to calculate the output params by applying the 'fixup'
+ * Topology will tell driver which type of fixup is to be applied by
+ * supplying the fixup mask, so based on that we calculate the output
+ *
+ * Now In FE the pcm hw_params is source/target format. Same is applicable
+ * for BE with its hw_params invoked.
+ * here based on FE, BE pipeline and direction we calculate the input and
+ * outfix and then apply that for a module
+ */
+static void skl_tplg_update_params_fixup(struct skl_module_cfg *m_cfg,
+		struct skl_pipe_params *params, bool is_fe)
+{
+	int in_fixup, out_fixup;
+	struct skl_module_fmt *in_fmt, *out_fmt;
+
+	in_fmt = &m_cfg->in_fmt;
+	out_fmt = &m_cfg->out_fmt;
+
+	if (params->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		if (is_fe) {
+			in_fixup = m_cfg->params_fixup;
+			out_fixup = (~m_cfg->converter) & m_cfg->params_fixup;
+		} else {
+			out_fixup = m_cfg->params_fixup;
+			in_fixup = (~m_cfg->converter) & m_cfg->params_fixup;
+		}
+	} else {
+		if (is_fe) {
+			out_fixup = m_cfg->params_fixup;
+			in_fixup = (~m_cfg->converter) & m_cfg->params_fixup;
+		} else {
+			in_fixup = m_cfg->params_fixup;
+			out_fixup = (~m_cfg->converter) & m_cfg->params_fixup;
+		}
+	}
+
+	skl_tplg_update_params(in_fmt, params, in_fixup);
+	skl_tplg_update_params(out_fmt, params, out_fixup);
+}
+
+/*
+ * A module needs input and output buffers, which are dependent upon pcm
+ * params, so once we have calculate params, we need buffer calculation as
+ * well.
+ */
+static void skl_tplg_update_buffer_size(struct skl_sst *ctx,
+				struct skl_module_cfg *mcfg)
+{
+	int multiplier = 1;
+
+	if (mcfg->m_type == SKL_MODULE_TYPE_SRCINT)
+		multiplier = 5;
+
+	mcfg->ibs = (mcfg->in_fmt.s_freq / 1000) *
+				(mcfg->in_fmt.channels) *
+				(mcfg->in_fmt.bit_depth >> 3) *
+				multiplier;
+
+	mcfg->obs = (mcfg->out_fmt.s_freq / 1000) *
+				(mcfg->out_fmt.channels) *
+				(mcfg->out_fmt.bit_depth >> 3) *
+				multiplier;
+}
+
+static void skl_tplg_update_module_params(struct snd_soc_dapm_widget *w,
+							struct skl_sst *ctx)
+{
+	struct skl_module_cfg *m_cfg = w->priv;
+	struct skl_pipe_params *params = m_cfg->pipe->p_params;
+	int p_conn_type = m_cfg->pipe->conn_type;
+	bool is_fe;
+
+	if (!m_cfg->params_fixup)
+		return;
+
+	dev_dbg(ctx->dev, "Mconfig for widget=%s BEFORE updation\n", w->name);
+	skl_dump_mconfig(ctx, m_cfg);
+
+	if (p_conn_type == SKL_PIPE_CONN_TYPE_FE)
+		is_fe = true;
+	else
+		is_fe = false;
+
+	skl_tplg_update_params_fixup(m_cfg, params, is_fe);
+	skl_tplg_update_buffer_size(ctx, m_cfg);
+
+	dev_dbg(ctx->dev, "Mconfig for widget=%s AFTER updation\n", w->name);
+	skl_dump_mconfig(ctx, m_cfg);
+}
+
 /*
  * A pipe can have multiple modules each of the will be a DAPM widget as
  * well. While managing a pipeline we need to get the list of all the
@@ -175,6 +299,8 @@ static int skl_tplg_init_pipe_modules(struct skl *skl, struct skl_pipe *pipe)
 		if (!skl_tplg_is_pipe_mcps_available(skl, mconfig))
 			return -ENOMEM;
 
+		/* apply fix/conversion to module params based on FE/BE params*/
+		skl_tplg_update_module_params(w, ctx);
 		ret = skl_init_module(ctx, mconfig, NULL);
 		if (ret < 0)
 			return ret;
-- 
1.9.1

[PATCH v2 3/9] ASoC: Intel: Skylake: add DSP platform widget event handlers

[Vinod Koul]

From: Jeeja KP <jeeja.kp@intel.com>

The Skylake driver topology model tries to model the firmware
rule for pipeline and module creation.
The creation rule is:
 - Create Pipe
 - Add modules to Pipe
 - Connect the modules (bind)
 - Start the pipes

Similarly destroy rule is:
 - Stop the pipe
 - Disconnect it (unbind)
 - Delete the pipe

In driver we use Mixer, as there will always be ONE mixer in a
pipeline to model a pipe. The modules in pipe are modelled as PGA
widgets.  The DAPM sequencing rules (mixer and then PGA) are used
to create the sequence DSP expects as depicted above, and then
widget handlers for PMU and PMD events help in that.

This patch adds widget event handlers for PRE/POST PMU and
PRE/POST PMD event for mixer and pga modules.  These event
handlers invoke pipeline creation, destroy, module creation,
module bind, unbind and pipeline bind unbind

Event handler sequencing is implement to target the DSP FW
sequence expectations to enable path from source to sink pipe for
Playback/Capture.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Hardik T Shah <hardik.t.shah@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/intel/skylake/skl-topology.c | 416 +++++++++++++++++++++++++++++++++
 1 file changed, 416 insertions(+)

diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 81b0683bbdeb..184697188a96 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -344,3 +344,419 @@ static int skl_tplg_bind_unbind_pipe_modules(struct skl_sst *ctx,
 	}
 	return 0;
 }
+
+/*
+ * Mixer module represents a pipeline. So in the Pre-PMU event of mixer we
+ * need create the pipeline. So we do following:
+ *   - check the resources
+ *   - Create the pipeline
+ *   - Initialize the modules in pipeline
+ *   - finally bind all modules together
+ */
+
+static int skl_tplg_mixer_dapm_pre_pmu_event(struct snd_soc_dapm_widget *w,
+								struct skl *skl)
+{
+	int ret;
+	struct skl_module_cfg *mconfig = w->priv;
+	struct skl_pipe *s_pipe = mconfig->pipe;
+	struct skl_sst *ctx = skl->skl_sst;
+
+	dev_dbg(ctx->dev, "%s: widget =%s\n", __func__, w->name);
+
+	/* check resource available */
+	if (!skl_tplg_is_pipe_mcps_available(skl, mconfig))
+		return -EBUSY;
+
+	if (!skl_tplg_is_pipe_mem_available(skl, mconfig))
+		return -ENOMEM;
+
+	/*
+	 * Create list of modules for pipe
+	 * list contains modules from source to sink
+	 */
+	ret = skl_create_pipeline(ctx, mconfig->pipe);
+	if (ret < 0)
+		return ret;
+
+	if (list_empty(&s_pipe->w_list)) {
+		ret = skl_tplg_get_pipe_widget(ctx->dev, w, s_pipe);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* Init all pipe modules from source to sink */
+	ret = skl_tplg_init_pipe_modules(skl, s_pipe);
+	if (ret < 0)
+		return ret;
+
+	/* Bind modules from source to sink */
+	return skl_tplg_bind_unbind_pipe_modules(ctx, s_pipe, true);
+}
+/*
+ * A PGA represents a module in a pipeline. So in the Pre-PMU event of PGA
+ * we need to do following:
+ *   - Bind to sink pipeline
+ *   	Since the sink pipes can be running and we don't get mixer event on
+ *   	connect for already running mixer, we need to find the sink pipes
+ *   	here and bind to them. This way dynamic connect works.
+ *   - Start sink pipeline, if not running
+ *   - Then run current pipe
+ */
+static int skl_tplg_pga_dapm_pre_pmu_event(struct snd_soc_dapm_widget *w,
+								struct skl *skl)
+{
+	struct snd_soc_dapm_path *p;
+	struct skl_dapm_path_list *path_list;
+	struct snd_soc_dapm_widget *source, *sink;
+	struct skl_module_cfg *src_mconfig, *sink_mconfig;
+	struct skl_sst *ctx = skl->skl_sst;
+	int ret = 0;
+
+	dev_dbg(ctx->dev, "%s: widget =%s\n", __func__, w->name);
+
+	source = w;
+	src_mconfig = source->priv;
+
+	/*
+	 * find which sink it is connected to, bind with the sink,
+	 * if sink is not started, start sink pipe first, then start
+	 * this pipe
+	 */
+	list_for_each_entry(p, &w->sinks, list_source) {
+		if (!p->connect)
+			continue;
+
+		dev_dbg(ctx->dev, "%s: src widget=%s\n", __func__, w->name);
+		dev_dbg(ctx->dev, "%s: sink widget=%s\n", __func__, p->sink->name);
+
+		/*
+		 * here we will check widgets in sink pipelines, so that can
+		 * be any widgets type and we are only interested if they are
+		 * ones used for SKL so check that first
+		 */
+		if ((p->sink->priv != NULL) && is_skl_dsp_widget_type(p->sink)) {
+
+			sink = p->sink;
+			src_mconfig = source->priv;
+			sink_mconfig = sink->priv;
+
+			/* Bind source to sink, mixin is always source */
+			ret = skl_tplg_bind_unbind_pipes(src_mconfig,
+						sink_mconfig, ctx, true);
+			if (ret)
+				return ret;
+
+			/* Start sinks pipe first */
+			if (sink_mconfig->pipe->state != SKL_PIPE_STARTED) {
+				ret = skl_run_pipe(ctx, sink_mconfig->pipe);
+				if (ret)
+					return ret;
+			}
+
+			path_list = kzalloc(sizeof(struct skl_dapm_path_list),
+								GFP_KERNEL);
+			if (NULL == path_list)
+				return -ENOMEM;
+
+			/* Add connected path to one global list */
+			path_list->dapm_path = p;
+			list_add_tail(&path_list->node, &skl->dapm_path_list);
+			break;
+		}
+	}
+
+	/* Start source pipe last after starting all sinks */
+	ret = skl_run_pipe(ctx, src_mconfig->pipe);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/*
+ * in the Post-PMU event of mixer we need to do following:
+ *   - Check if this pipe is running
+ *   - if not, then
+ *	- bind this pipeline to its source pipeline
+ *	  if source pipe is already running, this means it is a dynamic
+ *	  connection and we need to bind only to that pipe
+ *	- start this pipeline
+ */
+static int skl_tplg_mixer_dapm_post_pmu_event(struct snd_soc_dapm_widget *w,
+								struct skl *skl)
+{
+	int ret = 0;
+	struct snd_soc_dapm_path *p;
+	struct snd_soc_dapm_widget *source, *sink;
+	struct skl_module_cfg *src_mconfig, *sink_mconfig;
+	struct skl_sst *ctx = skl->skl_sst;
+	int src_pipe_started = 0;
+
+	dev_dbg(ctx->dev, "%s: widget = %s\n", __func__, w->name);
+
+	sink = w;
+	sink_mconfig = sink->priv;
+
+	/*
+	 * If source pipe is already started, that means source is driving
+	 * one more sink before this sink got connected, Since source is
+	 * started, bind this sink to source and start this pipe.
+	 */
+	list_for_each_entry(p, &w->sources, list_sink) {
+		if (!p->connect)
+			continue;
+		dev_dbg(ctx->dev, "sink widget=%s\n", w->name);
+		dev_dbg(ctx->dev, "src widget=%s\n", p->source->name);
+
+		/*
+		 * here we will check widgets in sink pipelines, so that can
+		 * be any widgets type and we are only interested if they are
+		 * ones used for SKL so check that first
+		 */
+		if ((p->source->priv != NULL) && is_skl_dsp_widget_type(p->source)) {
+			source = p->source;
+			src_mconfig = source->priv;
+			sink_mconfig = sink->priv;
+			src_pipe_started = 1;
+
+			/*
+			 * check pipe state, then no need to bind or start the
+			 * pipe
+			 */
+			if (src_mconfig->pipe->state != SKL_PIPE_STARTED)
+				src_pipe_started = 0;
+		}
+	}
+
+	if (src_pipe_started) {
+		ret = skl_tplg_bind_unbind_pipes(src_mconfig, sink_mconfig,
+							 ctx, true);
+		if (ret)
+			return ret;
+
+		return skl_run_pipe(ctx, sink_mconfig->pipe);
+	}
+
+	return ret;
+}
+
+/*
+ * in the Pre-PMD event of mixer we need to do following:
+ *   - Stop the pipe
+ *   - find the source connections and remove that from dapm_path_list
+ *   - unbind with source pipelines if still connected
+ */
+static int skl_tplg_mixer_dapm_pre_pmd_event(struct snd_soc_dapm_widget *w,
+								struct skl *skl)
+{
+	struct snd_soc_dapm_widget *source, *sink;
+	struct skl_module_cfg *src_mconfig, *sink_mconfig;
+	int ret = 0, path_found = 0;
+	struct skl_dapm_path_list *path_list, *tmp_list;
+	struct skl_sst *ctx = skl->skl_sst;
+
+	dev_dbg(ctx->dev, "%s: widget = %s\n", __func__, w->name);
+
+	sink = w;
+	sink_mconfig = sink->priv;
+
+	/* Stop the pipe */
+	ret = skl_stop_pipe(ctx, sink_mconfig->pipe);
+	if (ret)
+		return ret;
+
+	list_for_each_entry_safe(path_list, tmp_list, &skl->dapm_path_list, node) {
+		if (path_list->dapm_path->sink == sink) {
+			dev_dbg(ctx->dev, "Path found = %s\n", path_list->dapm_path->name);
+			source = path_list->dapm_path->source;
+			src_mconfig = source->priv;
+			path_found = 1;
+
+			list_del(&path_list->node);
+			kfree(path_list);
+			break;
+		}
+	}
+
+	/*
+	 * If path_found == 1, that means pmd for source pipe has
+	 * not occurred, source is connected to some other sink.
+	 * so its responsibility of sink to unbind itself from source.
+	 */
+	if (path_found)
+		ret = skl_tplg_bind_unbind_pipes(src_mconfig, sink_mconfig,
+							 ctx, false);
+
+	return ret;
+}
+
+/*
+ * in the Post-PMD event of mixer we need to do following:
+ *   - Free the mcps used
+ *   - Free the mem used
+ *   - Unbind the modules within the pipeline
+ *   - Delete the pipeline (modules are not required to be explicitly
+ *     deleted, pipeline delete is enough here
+ */
+static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
+								struct skl *skl)
+{
+	struct skl_module_cfg *mconfig = w->priv;
+	int ret = 0;
+	struct skl_sst *ctx = skl->skl_sst;
+	struct skl_pipe *s_pipe = mconfig->pipe;
+
+	dev_dbg(ctx->dev, "%s: widget = %s\n", __func__, w->name);
+
+	skl->resource.mcps -= mconfig->mcps;
+
+	ret = skl_tplg_bind_unbind_pipe_modules(ctx, s_pipe, false);
+	if (ret < 0)
+		return ret;
+
+	ret = skl_delete_pipe(ctx, mconfig->pipe);
+	skl->resource.mem -= mconfig->pipe->memory_pages;
+
+	return ret;
+}
+
+/*
+ * in the Post-PMD event of PGA we need to do following:
+ *   - Free the mcps used
+ *   - Stop the pipeline
+ *   - In source pipe is connected, unbind with source pipelines
+ */
+static int skl_tplg_pga_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
+								struct skl *skl)
+{
+	struct snd_soc_dapm_widget *source, *sink;
+	struct skl_module_cfg *src_mconfig, *sink_mconfig;
+	int ret = 0, path_found = 0;
+	struct skl_dapm_path_list *path_list, *tmp_path_list;
+	struct skl_sst *ctx = skl->skl_sst;
+
+	dev_dbg(ctx->dev, "%s: widget = %s\n", __func__, w->name);
+
+	source = w;
+	src_mconfig = source->priv;
+
+	skl->resource.mcps -= src_mconfig->mcps;
+	/* Stop the pipe since this is a mixin module */
+	ret = skl_stop_pipe(ctx, src_mconfig->pipe);
+	if (ret)
+		return ret;
+
+	list_for_each_entry_safe(path_list, tmp_path_list, &skl->dapm_path_list, node) {
+		if (path_list->dapm_path->source == source) {
+			dev_dbg(ctx->dev, "Path found = %s\n", path_list->dapm_path->name);
+			sink = path_list->dapm_path->sink;
+			sink_mconfig = sink->priv;
+			path_found = 1;
+
+			list_del(&path_list->node);
+			kfree(path_list);
+			break;
+		}
+	}
+
+	/*
+	 * This is a connector and if path is found that means
+	 * unbind between source and sink has not happened yet
+	 */
+	if (path_found)
+		ret = skl_tplg_bind_unbind_pipes(src_mconfig, sink_mconfig,
+							 ctx, false);
+
+	return ret;
+}
+
+static struct skl *get_skl_ctx(struct device *dev)
+{
+	struct hdac_ext_bus *ebus = dev_get_drvdata(dev);
+
+	return ebus_to_skl(ebus);
+}
+
+/*
+ * In modelling, we assume there will be ONLY one mixer in a pipeline.  If
+ * mixer is not required then it is treated as static mixer aka vmixer with
+ * a hard path to source module
+ * So we don't need to check if source is started or not as hard path puts
+ * dependency on each other
+ */
+static int skl_tplg_vmixer_event(struct snd_soc_dapm_widget *w,
+				struct snd_kcontrol *k, int event)
+{
+	struct snd_soc_dapm_context *dapm = w->dapm;
+	struct skl *skl = get_skl_ctx(dapm->dev);
+
+	dev_dbg(dapm->dev, "%s:widget=%s event=%d\n", __func__, w->name, event);
+
+	switch (event) {
+	case SND_SOC_DAPM_PRE_PMU:
+		return skl_tplg_mixer_dapm_pre_pmu_event(w, skl);
+
+	case SND_SOC_DAPM_POST_PMD:
+		return skl_tplg_mixer_dapm_post_pmd_event(w, skl);
+	}
+
+	return 0;
+}
+
+/*
+ * In modelling, we assume there will be ONLY one mixer in a pipeline. If a
+ * second one is required that is created as another pipe entity.
+ * The mixer is responsible for pipe management and represent a pipeline
+ * instance
+ */
+static int skl_tplg_mixer_event(struct snd_soc_dapm_widget *w,
+				struct snd_kcontrol *k, int event)
+{
+	struct snd_soc_dapm_context *dapm = w->dapm;
+	struct skl *skl = get_skl_ctx(dapm->dev);
+
+	dev_dbg(dapm->dev, "%s:widget=%s event=%d\n", __func__, w->name, event);
+
+	switch (event) {
+	case SND_SOC_DAPM_PRE_PMU:
+		return skl_tplg_mixer_dapm_pre_pmu_event(w, skl);
+
+	case SND_SOC_DAPM_POST_PMU:
+		return skl_tplg_mixer_dapm_post_pmu_event(w, skl);
+
+	case SND_SOC_DAPM_PRE_PMD:
+		return skl_tplg_mixer_dapm_pre_pmd_event(w, skl);
+
+	case SND_SOC_DAPM_POST_PMD:
+		return skl_tplg_mixer_dapm_post_pmd_event(w, skl);
+	}
+
+	return 0;
+}
+
+/*
+ * In modelling, we assumed rest of the modules in pipeline are PGA. But we
+ * are interested in last PGA (leaf PGA) in a pipeline to disconnect with
+ * the sink when it is running (two FE to one BE or one FE to two BE)
+ * scenarios
+ */
+static int skl_tplg_pga_event(struct snd_soc_dapm_widget *w,
+			struct snd_kcontrol *k, int event)
+
+{
+	struct snd_soc_dapm_context *dapm = w->dapm;
+	struct skl *skl = get_skl_ctx(dapm->dev);
+
+	dev_dbg(dapm->dev, "%s:widget=%s event=%d\n", __func__, w->name, event);
+
+	switch (event) {
+	case SND_SOC_DAPM_PRE_PMU:
+		return skl_tplg_pga_dapm_pre_pmu_event(w, skl);
+
+	case SND_SOC_DAPM_POST_PMD:
+		return skl_tplg_pga_dapm_post_pmd_event(w, skl);
+	}
+
+	return 0;
+}
-- 
1.9.1

Re: [PATCH v2 3/9] ASoC: Intel: Skylake: add DSP platform widget event handlers

[Liam Girdwood]

On Mon, 2015-08-17 at 22:56 +0530, Vinod Koul wrote:
> From: Jeeja KP <jeeja.kp@intel.com>
> 
> The Skylake driver topology model tries to model the firmware
> rule for pipeline and module creation.
> The creation rule is:
>  - Create Pipe
>  - Add modules to Pipe
>  - Connect the modules (bind)
>  - Start the pipes
> 
> Similarly destroy rule is:
>  - Stop the pipe
>  - Disconnect it (unbind)
>  - Delete the pipe
> 
> In driver we use Mixer, as there will always be ONE mixer in a
> pipeline to model a pipe. The modules in pipe are modelled as PGA
> widgets.  The DAPM sequencing rules (mixer and then PGA) are used
> to create the sequence DSP expects as depicted above, and then
> widget handlers for PMU and PMD events help in that.
> 
> This patch adds widget event handlers for PRE/POST PMU and
> PRE/POST PMD event for mixer and pga modules.  These event
> handlers invoke pipeline creation, destroy, module creation,
> module bind, unbind and pipeline bind unbind
> 
> Event handler sequencing is implement to target the DSP FW
> sequence expectations to enable path from source to sink pipe for
> Playback/Capture.
> 
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> Signed-off-by: Hardik T Shah <hardik.t.shah@intel.com>
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  sound/soc/intel/skylake/skl-topology.c | 416 +++++++++++++++++++++++++++++++++
>  1 file changed, 416 insertions(+)
> 

snip...

> +
> +/*
> + * in the Pre-PMD event of mixer we need to do following:
> + *   - Stop the pipe
> + *   - find the source connections and remove that from dapm_path_list
> + *   - unbind with source pipelines if still connected
> + */
> +static int skl_tplg_mixer_dapm_pre_pmd_event(struct snd_soc_dapm_widget *w,
> +								struct skl *skl)
> +{
> +	struct snd_soc_dapm_widget *source, *sink;
> +	struct skl_module_cfg *src_mconfig, *sink_mconfig;
> +	int ret = 0, path_found = 0;
> +	struct skl_dapm_path_list *path_list, *tmp_list;
> +	struct skl_sst *ctx = skl->skl_sst;
> +
> +	dev_dbg(ctx->dev, "%s: widget = %s\n", __func__, w->name);
> +
> +	sink = w;
> +	sink_mconfig = sink->priv;
> +
> +	/* Stop the pipe */
> +	ret = skl_stop_pipe(ctx, sink_mconfig->pipe);
> +	if (ret)
> +		return ret;
> +
> +	list_for_each_entry_safe(path_list, tmp_list, &skl->dapm_path_list, node) {
> +		if (path_list->dapm_path->sink == sink) {
> +			dev_dbg(ctx->dev, "Path found = %s\n", path_list->dapm_path->name);
> +			source = path_list->dapm_path->source;
> +			src_mconfig = source->priv;
> +			path_found = 1;
> +
> +			list_del(&path_list->node);
> +			kfree(path_list);
> +			break;
> +		}
> +	}
> +

There is a lot of list walking and manipulation in this series and it's
not clear where any locks are being held to prevent list corruption.
I'm assuming list items are being added and removed as part of
loading/unloading the topology data but it looks like we are also
manipulating component lists during DAPM events ?

Liam

Re: [PATCH v2 3/9] ASoC: Intel: Skylake: add DSP platform widget event handlers

[Vinod Koul]

On Thu, Sep 17, 2015 at 10:47:58AM +0100, Liam Girdwood wrote:

Hi Liam,

> On Mon, 2015-08-17 at 22:56 +0530, Vinod Koul wrote:
> > From: Jeeja KP <jeeja.kp@intel.com>
> > 
> > The Skylake driver topology model tries to model the firmware
> > rule for pipeline and module creation.
> > The creation rule is:
> >  - Create Pipe
> >  - Add modules to Pipe
> >  - Connect the modules (bind)
> >  - Start the pipes
> > 
> > Similarly destroy rule is:
> >  - Stop the pipe
> >  - Disconnect it (unbind)
> >  - Delete the pipe
> > 
> > In driver we use Mixer, as there will always be ONE mixer in a
> > pipeline to model a pipe. The modules in pipe are modelled as PGA
> > widgets.  The DAPM sequencing rules (mixer and then PGA) are used
> > to create the sequence DSP expects as depicted above, and then
> > widget handlers for PMU and PMD events help in that.
> > 
> > This patch adds widget event handlers for PRE/POST PMU and
> > PRE/POST PMD event for mixer and pga modules.  These event
> > handlers invoke pipeline creation, destroy, module creation,
> > module bind, unbind and pipeline bind unbind
> > 
> > Event handler sequencing is implement to target the DSP FW
> > sequence expectations to enable path from source to sink pipe for
> > Playback/Capture.
> > 
> > Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> > Signed-off-by: Hardik T Shah <hardik.t.shah@intel.com>
> > Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> > ---
> >  sound/soc/intel/skylake/skl-topology.c | 416 +++++++++++++++++++++++++++++++++
> >  1 file changed, 416 insertions(+)
> > 
> 
> snip...
> 
> > +
> > +/*
> > + * in the Pre-PMD event of mixer we need to do following:
> > + *   - Stop the pipe
> > + *   - find the source connections and remove that from dapm_path_list
> > + *   - unbind with source pipelines if still connected
> > + */
> > +static int skl_tplg_mixer_dapm_pre_pmd_event(struct snd_soc_dapm_widget *w,
> > +								struct skl *skl)
> > +{
> > +	struct snd_soc_dapm_widget *source, *sink;
> > +	struct skl_module_cfg *src_mconfig, *sink_mconfig;
> > +	int ret = 0, path_found = 0;
> > +	struct skl_dapm_path_list *path_list, *tmp_list;
> > +	struct skl_sst *ctx = skl->skl_sst;
> > +
> > +	dev_dbg(ctx->dev, "%s: widget = %s\n", __func__, w->name);
> > +
> > +	sink = w;
> > +	sink_mconfig = sink->priv;
> > +
> > +	/* Stop the pipe */
> > +	ret = skl_stop_pipe(ctx, sink_mconfig->pipe);
> > +	if (ret)
> > +		return ret;
> > +
> > +	list_for_each_entry_safe(path_list, tmp_list, &skl->dapm_path_list, node) {
> > +		if (path_list->dapm_path->sink == sink) {
> > +			dev_dbg(ctx->dev, "Path found = %s\n", path_list->dapm_path->name);
> > +			source = path_list->dapm_path->source;
> > +			src_mconfig = source->priv;
> > +			path_found = 1;
> > +
> > +			list_del(&path_list->node);
> > +			kfree(path_list);
> > +			break;
> > +		}
> > +	}
> > +
> 
> There is a lot of list walking and manipulation in this series and it's
> not clear where any locks are being held to prevent list corruption.
> I'm assuming list items are being added and removed as part of
> loading/unloading the topology data but it looks like we are also
> manipulating component lists during DAPM events ?

We have a driver list dapm_path_list where we store the widgets powered up.
This gives us a very quick reference of the paths which are powered up in
the graph and helps fast traversal to check if we should power up a path as
path is connected to something else which is powered up already (mixng two
paths) and similarly while disconnecting.

Please note all these are handled only in event handlers for widgets, so
they will be invoked by dapm with mutex, dapm_mutex held, so we didn't think
we would need another lock here

-- 
~Vinod

Re: [PATCH v2 3/9] ASoC: Intel: Skylake: add DSP platform widget event handlers

[Liam Girdwood]

On Thu, 2015-09-17 at 17:08 +0530, Vinod Koul wrote:
> On Thu, Sep 17, 2015 at 10:47:58AM +0100, Liam Girdwood wrote:
> 
> Hi Liam,
> 
> > On Mon, 2015-08-17 at 22:56 +0530, Vinod Koul wrote:
> > > From: Jeeja KP <jeeja.kp@intel.com>
> > > 
> > > The Skylake driver topology model tries to model the firmware
> > > rule for pipeline and module creation.
> > > The creation rule is:
> > >  - Create Pipe
> > >  - Add modules to Pipe
> > >  - Connect the modules (bind)
> > >  - Start the pipes
> > > 
> > > Similarly destroy rule is:
> > >  - Stop the pipe
> > >  - Disconnect it (unbind)
> > >  - Delete the pipe
> > > 
> > > In driver we use Mixer, as there will always be ONE mixer in a
> > > pipeline to model a pipe. The modules in pipe are modelled as PGA
> > > widgets.  The DAPM sequencing rules (mixer and then PGA) are used
> > > to create the sequence DSP expects as depicted above, and then
> > > widget handlers for PMU and PMD events help in that.
> > > 
> > > This patch adds widget event handlers for PRE/POST PMU and
> > > PRE/POST PMD event for mixer and pga modules.  These event
> > > handlers invoke pipeline creation, destroy, module creation,
> > > module bind, unbind and pipeline bind unbind
> > > 
> > > Event handler sequencing is implement to target the DSP FW
> > > sequence expectations to enable path from source to sink pipe for
> > > Playback/Capture.
> > > 
> > > Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> > > Signed-off-by: Hardik T Shah <hardik.t.shah@intel.com>
> > > Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> > > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> > > ---
> > >  sound/soc/intel/skylake/skl-topology.c | 416 +++++++++++++++++++++++++++++++++
> > >  1 file changed, 416 insertions(+)
> > > 
> > 
> > snip...
> > 
> > > +
> > > +/*
> > > + * in the Pre-PMD event of mixer we need to do following:
> > > + *   - Stop the pipe
> > > + *   - find the source connections and remove that from dapm_path_list
> > > + *   - unbind with source pipelines if still connected
> > > + */
> > > +static int skl_tplg_mixer_dapm_pre_pmd_event(struct snd_soc_dapm_widget *w,
> > > +								struct skl *skl)
> > > +{
> > > +	struct snd_soc_dapm_widget *source, *sink;
> > > +	struct skl_module_cfg *src_mconfig, *sink_mconfig;
> > > +	int ret = 0, path_found = 0;
> > > +	struct skl_dapm_path_list *path_list, *tmp_list;
> > > +	struct skl_sst *ctx = skl->skl_sst;
> > > +
> > > +	dev_dbg(ctx->dev, "%s: widget = %s\n", __func__, w->name);
> > > +
> > > +	sink = w;
> > > +	sink_mconfig = sink->priv;
> > > +
> > > +	/* Stop the pipe */
> > > +	ret = skl_stop_pipe(ctx, sink_mconfig->pipe);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	list_for_each_entry_safe(path_list, tmp_list, &skl->dapm_path_list, node) {
> > > +		if (path_list->dapm_path->sink == sink) {
> > > +			dev_dbg(ctx->dev, "Path found = %s\n", path_list->dapm_path->name);
> > > +			source = path_list->dapm_path->source;
> > > +			src_mconfig = source->priv;
> > > +			path_found = 1;
> > > +
> > > +			list_del(&path_list->node);
> > > +			kfree(path_list);
> > > +			break;
> > > +		}
> > > +	}
> > > +
> > 
> > There is a lot of list walking and manipulation in this series and it's
> > not clear where any locks are being held to prevent list corruption.
> > I'm assuming list items are being added and removed as part of
> > loading/unloading the topology data but it looks like we are also
> > manipulating component lists during DAPM events ?
> 
> We have a driver list dapm_path_list where we store the widgets powered up.
> This gives us a very quick reference of the paths which are powered up in
> the graph and helps fast traversal to check if we should power up a path as
> path is connected to something else which is powered up already (mixng two
> paths) and similarly while disconnecting.
> 
> Please note all these are handled only in event handlers for widgets, so
> they will be invoked by dapm with mutex, dapm_mutex held, so we didn't think
> we would need another lock here
> 

Ok, I was thinking that may be the case. It may be worth while stating
this in comments where this applies.

Liam

Re: [PATCH v2 3/9] ASoC: Intel: Skylake: add DSP platform widget event handlers

[Vinod Koul]

On Thu, Sep 17, 2015 at 01:25:07PM +0100, Liam Girdwood wrote:
 
> > > There is a lot of list walking and manipulation in this series and it's
> > > not clear where any locks are being held to prevent list corruption.
> > > I'm assuming list items are being added and removed as part of
> > > loading/unloading the topology data but it looks like we are also
> > > manipulating component lists during DAPM events ?
> > 
> > We have a driver list dapm_path_list where we store the widgets powered up.
> > This gives us a very quick reference of the paths which are powered up in
> > the graph and helps fast traversal to check if we should power up a path as
> > path is connected to something else which is powered up already (mixng two
> > paths) and similarly while disconnecting.
> > 
> > Please note all these are handled only in event handlers for widgets, so
> > they will be invoked by dapm with mutex, dapm_mutex held, so we didn't think
> > we would need another lock here
> > 
> 
> Ok, I was thinking that may be the case. It may be worth while stating
> this in comments where this applies.

It's actually specfied that we add all connected paths to drivers list.
I am okay to add more, will do so here if there any anymore comments from
Mark, otherwise will add as an update in patches after this series

Thanks
-- 
~Vinod

Re: [PATCH v2 3/9] ASoC: Intel: Skylake: add DSP platform widget event handlers

[Mark Brown]

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

On Mon, Aug 17, 2015 at 10:56:38PM +0530, Vinod Koul wrote:

> +	if (list_empty(&s_pipe->w_list)) {
> +		ret = skl_tplg_get_pipe_widget(ctx->dev, w, s_pipe);
> +		if (ret < 0)
> +			return ret;
> +	}

I'm not entirely clear what this is supposed to do or if it does it
correctly - I suspect it's trying to always create the pipe widget which
might be a bit clearer if it just created the pipe widget.  The fact
that it's checking if a list is empty is a bit worrying, what if some
but not all of the entries needed on that list are there?  Is just
getting a widget really sufficient initialisation?

> +	return skl_tplg_bind_unbind_pipe_modules(ctx, s_pipe, true);
> +}
> +/*
> + * A PGA represents a module in a pipeline. So in the Pre-PMU event of PGA

Blank line between functions and the next thing.  I am getting fed up of
seeing this.

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

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

Re: [PATCH v2 3/9] ASoC: Intel: Skylake: add DSP platform widget event handlers

[Vinod Koul]

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

On Sat, Sep 19, 2015 at 09:11:22AM -0700, Mark Brown wrote:
> On Mon, Aug 17, 2015 at 10:56:38PM +0530, Vinod Koul wrote:
> 
> > +	if (list_empty(&s_pipe->w_list)) {
> > +		ret = skl_tplg_get_pipe_widget(ctx->dev, w, s_pipe);
> > +		if (ret < 0)
> > +			return ret;
> > +	}
> 
> I'm not entirely clear what this is supposed to do or if it does it
> correctly - I suspect it's trying to always create the pipe widget which
> might be a bit clearer if it just created the pipe widget.  The fact
> that it's checking if a list is empty is a bit worrying, what if some
> but not all of the entries needed on that list are there?  Is just
> getting a widget really sufficient initialisation?

For quick reference, on first run of a pipe, we create a w_list of all
widgets in that pipe. This list is not freed on PMD event as widgets within
a pipe are static. This saves us cycles to get widgets in pipe every time.

So if we already initialized all the widgets of a pipeline we don't need
that anymore. I will add comment for this as well

> 
> > +	return skl_tplg_bind_unbind_pipe_modules(ctx, s_pipe, true);
> > +}
> > +/*
> > + * A PGA represents a module in a pipeline. So in the Pre-PMU event of PGA
> 
> Blank line between functions and the next thing.  I am getting fed up of
> seeing this.
Will fix these

Thanks
-- 
~Vinod

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

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

[PATCH v2 4/9] ASoC: Intel: Skylake: Add FE and BE hw_params handling

[Vinod Koul]

From: Jeeja KP <jeeja.kp@intel.com>

For FE and BE, the PCM parameters come from FE and BE hw_params
values passed. For a FE we convert the FE params to DSP expected
module format and pass to DSP. For a BE we need to find the
gateway settings (i2s/PDM) to be applied. These are queried from
NHLT table and applied.

Further for BE based on direction the settings are applied as
either source or destination parameters.

These helpers here allow the format to be calculated and queried
as per firmware format.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/intel/skylake/skl-topology.c | 283 +++++++++++++++++++++++++++++++++
 sound/soc/intel/skylake/skl-topology.h |  11 ++
 2 files changed, 294 insertions(+)

diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 184697188a96..287935eab68b 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -760,3 +760,286 @@ static int skl_tplg_pga_event(struct snd_soc_dapm_widget *w,
 
 	return 0;
 }
+
+/*
+ * The FE params are passed by hw_params of the DAI.
+ * On hw_params, the params are stored in Gateway module of the FE and we
+ * need to calculate the format in DSP module configuration, that conversion
+ * is done here
+ */
+static int skl_tplg_update_pipe_params(struct device *dev,
+		struct skl_module_cfg *mconfig, struct skl_pipe_params *params)
+{
+	struct skl_pipe *pipe = mconfig->pipe;
+	struct skl_module_fmt *format = NULL;
+
+	memcpy(pipe->p_params, params, sizeof(*params));
+
+	if (params->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		format = &mconfig->in_fmt;
+	else
+		format = &mconfig->out_fmt;
+
+	/* set the hw_params */
+	format->s_freq = params->s_freq;
+	format->channels = params->ch;
+	format->valid_bit_depth = skl_get_bit_depth(params->s_fmt);
+
+	/*
+	 * 16 bit is 16 bit container whereas 24 bit is in 32 bit container so
+	 * update bit depth accordingly
+	 */
+	switch (format->valid_bit_depth) {
+	case SKL_DEPTH_16BIT:
+		format->bit_depth = format->valid_bit_depth;
+		break;
+
+	case SKL_DEPTH_24BIT:
+		format->bit_depth = SKL_DEPTH_32BIT;
+		break;
+
+	default:
+		dev_err(dev, "Invalid bit depth %x for pipe\n",
+				format->valid_bit_depth);
+		return -EINVAL;
+	}
+
+	if (params->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		mconfig->ibs = (format->s_freq / 1000) *
+				(format->channels) *
+				(format->bit_depth >> 3);
+	} else {
+		mconfig->obs = (format->s_freq / 1000) *
+				(format->channels) *
+				(format->bit_depth >> 3);
+	}
+
+	return 0;
+}
+
+/*
+ * Query the module config for the FE DAI
+ * This is used to find the hw_params set for that DAI and apply to FE
+ * pipeline
+ */
+static struct skl_module_cfg *
+skl_tplg_fe_get_cpr_module(struct snd_soc_dai *dai, int stream)
+{
+	struct snd_soc_dapm_widget *w;
+	struct snd_soc_dapm_path *p = NULL;
+
+	dev_dbg(dai->dev, "%s: enter, dai-name=%s\n", __func__, dai->name);
+
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		w = dai->playback_widget;
+		dev_dbg(dai->dev, "Stream name=%s\n", w->name);
+		list_for_each_entry(p, &w->sinks, list_source) {
+			if (p->connect && p->sink->power &&
+					is_skl_dsp_widget_type(p->sink))
+				continue;
+
+			if (p->sink->priv) {
+				dev_dbg(dai->dev, "set params for %s\n", p->sink->name);
+				return p->sink->priv;
+			}
+		}
+	} else {
+		w = dai->capture_widget;
+		dev_dbg(dai->dev, "Stream name=%s\n", w->name);
+		list_for_each_entry(p, &w->sources, list_sink) {
+			if (p->connect && p->source->power &&
+					is_skl_dsp_widget_type(p->source))
+				continue;
+
+			if (p->source->priv) {
+				dev_dbg(dai->dev, "set params for %s\n", p->source->name);
+				return p->source->priv;
+			}
+		}
+	}
+	return NULL;
+}
+
+int skl_tplg_fe_update_params(struct snd_soc_dai *dai,
+			struct skl_pipe_params *params)
+{
+	struct skl_module_cfg *m_cfg;
+
+	m_cfg = skl_tplg_fe_get_cpr_module(dai, params->stream);
+
+	if (m_cfg)
+		skl_tplg_update_pipe_params(dai->dev, m_cfg, params);
+
+	return 0;
+}
+
+static u8 skl_tplg_be_link_type(int dev_type)
+{
+	int ret;
+
+	switch (dev_type) {
+	case SKL_DEVICE_BT:
+		ret = NHLT_LINK_SSP;
+		break;
+
+	case SKL_DEVICE_DMIC:
+		ret = NHLT_LINK_DMIC;
+		break;
+
+	case SKL_DEVICE_I2S:
+		ret = NHLT_LINK_SSP;
+		break;
+
+	case SKL_DEVICE_HDALINK:
+		ret = NHLT_LINK_HDA;
+		break;
+
+	default:
+		ret = NHLT_LINK_INVALID;
+		break;
+	}
+
+	return ret;
+}
+
+/*
+ * Fill the BE gateway parameters
+ * The BE gateway expects a blob of parameters which are kept in the ACPI
+ * NHLT blob, so query the blob for interface type (i2s/pdm) and instance.
+ * The port can have multiple settings so pick based on the PCM
+ * parameters
+ */
+static int skl_tplg_be_fill_pipe_params(struct snd_soc_dai *dai,
+		struct skl_module_cfg *mconfig, struct skl_pipe_params *params)
+{
+	struct skl_pipe *pipe = mconfig->pipe;
+	struct nhlt_specific_cfg *cfg;
+	struct skl *skl = get_skl_ctx(dai->dev);
+	int link_type = skl_tplg_be_link_type(mconfig->dev_type);
+
+	memcpy(pipe->p_params, params, sizeof(*params));
+
+	/* update the blob based on virtual bus_id*/
+	cfg = skl_get_ep_blob(skl, mconfig->vbus_id, link_type, params->s_fmt,
+				params->ch, params->s_freq, params->stream);
+	if (cfg) {
+		mconfig->formats_config.caps_size = cfg->size;
+		memcpy(mconfig->formats_config.caps, &cfg->caps, cfg->size);
+	} else {
+		dev_err(dai->dev, "Blob is NULL for id %x type %d dirn %d\n",
+				mconfig->vbus_id, link_type, params->stream);
+		dev_err(dai->dev, "PCM: ch %d, freq %d, fmt %d\n",
+				 params->ch, params->s_freq, params->s_fmt);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int skl_tplg_be_set_params(struct snd_soc_dai *dai,
+		struct snd_soc_dapm_widget *w, struct skl_pipe_params *params)
+{
+	if (!w->power) {
+		dev_dbg(dai->dev, "set params for widget=%s\n", w->name);
+		return skl_tplg_be_fill_pipe_params(dai, w->priv, params);
+	}
+
+	return -EBUSY;
+}
+
+static int skl_tplg_be_set_src_pipe_params(struct snd_soc_dai *dai,
+		struct snd_soc_dapm_widget *w, struct skl_pipe_params *params)
+{
+	struct snd_soc_dapm_path *p;
+	int ret = 0;
+
+	dev_dbg(dai->dev, "%s: widget name=%s\n", __func__, w->name);
+
+	list_for_each_entry(p, &w->sources, list_sink) {
+		if (p->connect && is_skl_dsp_widget_type(p->source) &&
+						 p->source->priv) {
+			ret = skl_tplg_be_set_params(dai, p->source, params);
+
+			if (ret < 0)
+				return ret;
+		} else {
+			ret = skl_tplg_be_set_src_pipe_params(dai, p->source,
+									 params);
+		}
+	}
+
+	return ret;
+}
+
+static int skl_tplg_be_set_sink_pipe_params(struct snd_soc_dai *dai,
+	struct snd_soc_dapm_widget *w, struct skl_pipe_params *params)
+{
+	struct snd_soc_dapm_path *p = NULL;
+	int ret = 0;
+
+	dev_dbg(dai->dev, "widget name=%s\n", w->name);
+
+	list_for_each_entry(p, &w->sinks, list_source) {
+		if (p->connect && is_skl_dsp_widget_type(p->sink) &&
+						p->sink->priv) {
+			ret = skl_tplg_be_set_params(dai, p->sink, params);
+			if (ret < 0)
+				return ret;
+
+		} else {
+			ret = skl_tplg_be_set_sink_pipe_params(
+						dai, p->sink, params);
+		}
+	}
+
+	return ret;
+}
+
+/*
+ * BE hw_params can be a source parameters (capture) or sink parameters
+ * (playback). Based on sink and source we need to either find the source
+ * list or the sink list and set the pipeline parameters
+ */
+int skl_tplg_be_update_params(struct snd_soc_dai *dai,
+				struct skl_pipe_params *params)
+{
+	struct snd_soc_dapm_widget *w;
+
+	dev_dbg(dai->dev, "%s: dai-name=%s\n", __func__, dai->name);
+
+	if (params->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		w = dai->playback_widget;
+
+		dev_dbg(dai->dev, "Stream name %s\n", w->name);
+		return skl_tplg_be_set_src_pipe_params(dai, w, params);
+
+	} else {
+		w = dai->capture_widget;
+
+		dev_dbg(dai->dev, "Stream name %s\n", w->name);
+		return skl_tplg_be_set_sink_pipe_params(dai, w, params);
+	}
+
+	return 0;
+}
+
+/* Run/Stop the FE pipeline */
+int skl_tplg_set_fe_pipeline_state(struct snd_soc_dai *dai, bool start,
+								int stream)
+{
+	struct skl *skl = get_skl_ctx(dai->dev);
+	struct skl_sst *ctx = skl->skl_sst;
+	struct skl_module_cfg *mconfig = NULL;
+
+	dev_dbg(dai->dev, "%s: enter, dai-name=%s dir=%d\n", __func__, dai->name, stream);
+
+	mconfig = skl_tplg_fe_get_cpr_module(dai, stream);
+	if (mconfig != NULL) {
+		if (start)
+			return skl_run_pipe(ctx, mconfig->pipe);
+		else
+			return skl_stop_pipe(ctx, mconfig->pipe);
+	}
+
+	return 0;
+}
diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h
index 73d7916ee33e..94c721e4d095 100644
--- a/sound/soc/intel/skylake/skl-topology.h
+++ b/sound/soc/intel/skylake/skl-topology.h
@@ -273,6 +273,17 @@ struct skl_dapm_path_list {
 	struct list_head node;
 };
 
+int skl_tplg_fe_update_params(struct snd_soc_dai *dai,
+	struct skl_pipe_params *params);
+int skl_tplg_be_update_params(struct snd_soc_dai *dai,
+	struct skl_pipe_params *params);
+void skl_tplg_set_be_dmic_config(struct snd_soc_dai *dai,
+	struct skl_pipe_params *params, int stream);
+int skl_tplg_init(struct snd_soc_platform *platform,
+				struct hdac_ext_bus *ebus);
+int skl_tplg_set_fe_pipeline_state(struct snd_soc_dai *dai,
+			bool start, int stream);
+
 int skl_create_pipeline(struct skl_sst *ctx, struct skl_pipe *pipe);
 
 int skl_run_pipe(struct skl_sst *ctx, struct skl_pipe *pipe);
-- 
1.9.1

Re: [PATCH v2 4/9] ASoC: Intel: Skylake: Add FE and BE hw_params handling

[Mark Brown]

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

On Mon, Aug 17, 2015 at 10:56:39PM +0530, Vinod Koul wrote:

> +/* Run/Stop the FE pipeline */
> +int skl_tplg_set_fe_pipeline_state(struct snd_soc_dai *dai, bool start,
> +								int stream)
> +{
> +	struct skl *skl = get_skl_ctx(dai->dev);
> +	struct skl_sst *ctx = skl->skl_sst;
> +	struct skl_module_cfg *mconfig = NULL;
> +
> +	dev_dbg(dai->dev, "%s: enter, dai-name=%s dir=%d\n", __func__, dai->name, stream);
> +
> +	mconfig = skl_tplg_fe_get_cpr_module(dai, stream);
> +	if (mconfig != NULL) {
> +		if (start)
> +			return skl_run_pipe(ctx, mconfig->pipe);
> +		else
> +			return skl_stop_pipe(ctx, mconfig->pipe);
> +	}
> +
> +	return 0;
> +}

Another one of these wrappers with basically no shared code :(  

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

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

Re: [PATCH v2 4/9] ASoC: Intel: Skylake: Add FE and BE hw_params handling

[Vinod Koul]

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

On Sat, Sep 19, 2015 at 09:22:10AM -0700, Mark Brown wrote:
> On Mon, Aug 17, 2015 at 10:56:39PM +0530, Vinod Koul wrote:
> 
> > +/* Run/Stop the FE pipeline */
> > +int skl_tplg_set_fe_pipeline_state(struct snd_soc_dai *dai, bool start,
> > +								int stream)
> > +{
> > +	struct skl *skl = get_skl_ctx(dai->dev);
> > +	struct skl_sst *ctx = skl->skl_sst;
> > +	struct skl_module_cfg *mconfig = NULL;
> > +
> > +	dev_dbg(dai->dev, "%s: enter, dai-name=%s dir=%d\n", __func__, dai->name, stream);
> > +
> > +	mconfig = skl_tplg_fe_get_cpr_module(dai, stream);
> > +	if (mconfig != NULL) {
> > +		if (start)
> > +			return skl_run_pipe(ctx, mconfig->pipe);
> > +		else
> > +			return skl_stop_pipe(ctx, mconfig->pipe);
> > +	}
> > +
> > +	return 0;
> > +}
> 
> Another one of these wrappers with basically no shared code :(  
Yes this is wrapper over pipeline start and stop. This function is invoked
from the PCM trigger code so we though providing a wrapper makes it look
better. I will remove this and invoke skl_run_pipe and skl_stop_pipe from
PCM trigger now

Will check if we can remove other wrappers as well

Thanks
-- 
~Vinod

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

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

[PATCH v2 5/9] ASoC: Intel: Skylake: Add topology core init and handlers

[Vinod Koul]

From: Jeeja KP <jeeja.kp@intel.com>

The SKL driver does not code DSP topology in driver. It uses the
newly added ASoC topology core to parse the topology information
(controls, widgets and map) from topology binary.
Each topology element passed private data which contains
information that driver used to identify the module instance
within firmware and send IPCs for that module to DSP firmware
along with parameters.
This patch adds init routine to invoke topology load and callback
for topology creation.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/intel/skylake/skl-topology.c       | 217 +++++++++++++++++++++++++++
 sound/soc/intel/skylake/skl-tplg-interface.h |  78 ++++++++++
 2 files changed, 295 insertions(+)

diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 287935eab68b..d18ce8c461a4 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -1043,3 +1043,220 @@ int skl_tplg_set_fe_pipeline_state(struct snd_soc_dai *dai, bool start,
 
 	return 0;
 }
+
+const static struct snd_soc_tplg_widget_events skl_tplg_widget_ops[] = {
+	{SKL_MIXER_EVENT, skl_tplg_mixer_event},
+	{SKL_VMIXER_EVENT, skl_tplg_vmixer_event},
+	{SKL_PGA_EVENT, skl_tplg_pga_event},
+};
+
+/*
+ * The topology binary passes the pin info for a module so initialize the pin
+ * info passed into module instance
+ */
+static void skl_fill_module_pin_info(struct device *dev,
+			struct skl_module_pin *m_pin,
+			int max_pin)
+{
+	int i;
+
+	for (i = 0; i < max_pin; i++) {
+		m_pin[i].id.module_id = 0;
+		m_pin[i].id.instance_id = 0;
+		m_pin[i].in_use = false;
+		m_pin[i].is_dynamic = true;
+		m_pin[i].pin_index = i;
+	}
+}
+
+/*
+ * Add pipeline from topology binary into driver pipeline list
+ *
+ * If already added we return that instance
+ * Otherwise we create a new instance and add into driver list
+ */
+static struct skl_pipe *skl_tplg_add_pipe(struct device *dev,
+			struct skl *skl, struct skl_dfw_pipe *dfw_pipe)
+{
+	struct skl_pipeline *ppl;
+	struct skl_pipe *pipe;
+	struct skl_pipe_params *params;
+
+	list_for_each_entry(ppl, &skl->ppl_list, node) {
+		if (ppl->pipe->ppl_id == dfw_pipe->pipe_id)
+			return ppl->pipe;
+	}
+
+	ppl = devm_kzalloc(dev, sizeof(*ppl), GFP_KERNEL);
+	if (!ppl)
+		return NULL;
+
+	pipe = devm_kzalloc(dev, sizeof(*pipe), GFP_KERNEL);
+	if (!pipe)
+		return NULL;
+
+	params = devm_kzalloc(dev, sizeof(*params), GFP_KERNEL);
+	if (!params)
+		return NULL;
+
+	pipe->ppl_id = dfw_pipe->pipe_id;
+	pipe->memory_pages = dfw_pipe->memory_pages;
+	pipe->pipe_priority = dfw_pipe->pipe_priority;
+	pipe->conn_type = dfw_pipe->conn_type;
+	pipe->state = SKL_PIPE_INVALID;
+	pipe->p_params = params;
+	INIT_LIST_HEAD(&pipe->w_list);
+
+	ppl->pipe = pipe;
+	list_add(&ppl->node, &skl->ppl_list);
+
+	return ppl->pipe;
+}
+
+/*
+ * Topology core widget load callback
+ *
+ * This is used to save the private data for each widget which gives
+ * information to the driver about module and pipeline parameters which DSP
+ * FW expects like ids, resource values, formats etc
+ */
+static int skl_tplg_widget_load(struct snd_soc_component *cmpnt,
+					struct snd_soc_dapm_widget *w,
+					struct snd_soc_tplg_dapm_widget *tplg_w)
+{
+	int ret;
+	struct hdac_ext_bus *ebus = snd_soc_component_get_drvdata(cmpnt);
+	struct skl *skl = ebus_to_skl(ebus);
+	struct hdac_bus *bus = ebus_to_hbus(ebus);
+	struct skl_module_cfg *mconfig;
+	struct skl_pipe *pipe;
+	struct skl_dfw_module *dfw_config = (struct skl_dfw_module *)tplg_w->priv.data;
+
+	if (!tplg_w->priv.size)
+		goto bind_event;
+
+	mconfig = devm_kzalloc(bus->dev, sizeof(*mconfig), GFP_KERNEL);
+
+	if (!mconfig)
+		return -ENOMEM;
+
+	w->priv = mconfig;
+	mconfig->id.module_id = dfw_config->module_id;
+	mconfig->id.instance_id = dfw_config->instance_id;
+	mconfig->mcps = dfw_config->max_mcps;
+	mconfig->ibs = dfw_config->ibs;
+	mconfig->obs = dfw_config->obs;
+	mconfig->core_id = dfw_config->core_id;
+	mconfig->max_in_queue = dfw_config->max_in_queue;
+	mconfig->max_out_queue = dfw_config->max_out_queue;
+	mconfig->is_loadable = dfw_config->is_loadable;
+	mconfig->in_fmt.channels = dfw_config->in_fmt.channels;
+	mconfig->in_fmt.s_freq = dfw_config->in_fmt.freq;
+	mconfig->in_fmt.bit_depth = dfw_config->in_fmt.bit_depth;
+	mconfig->in_fmt.valid_bit_depth = dfw_config->in_fmt.valid_bit_depth;
+	mconfig->in_fmt.ch_cfg = dfw_config->in_fmt.ch_cfg;
+	mconfig->out_fmt.channels = dfw_config->out_fmt.channels;
+	mconfig->out_fmt.s_freq = dfw_config->out_fmt.freq;
+	mconfig->out_fmt.bit_depth = dfw_config->out_fmt.bit_depth;
+	mconfig->out_fmt.valid_bit_depth = dfw_config->out_fmt.valid_bit_depth;
+	mconfig->out_fmt.ch_cfg = dfw_config->out_fmt.ch_cfg;
+	mconfig->params_fixup = dfw_config->params_fixup;
+	mconfig->converter = dfw_config->converter;
+	mconfig->m_type = dfw_config->module_type;
+	mconfig->vbus_id = dfw_config->vbus_id;
+
+	pipe = skl_tplg_add_pipe(bus->dev, skl, &dfw_config->pipe);
+	if (pipe)
+		mconfig->pipe = pipe;
+
+	mconfig->dev_type = dfw_config->dev_type;
+	mconfig->hw_conn_type = dfw_config->hw_conn_type;
+	mconfig->time_slot = dfw_config->time_slot;
+	mconfig->formats_config.caps_size = dfw_config->caps.caps_size;
+
+	mconfig->m_in_pin = devm_kzalloc(bus->dev, (mconfig->max_in_queue) *
+						sizeof(*mconfig->m_in_pin),
+						GFP_KERNEL);
+	if (!mconfig->m_in_pin)
+		return -ENOMEM;
+
+	mconfig->m_out_pin = devm_kzalloc(bus->dev, (mconfig->max_in_queue) *
+						sizeof(*mconfig->m_out_pin),
+						GFP_KERNEL);
+	if (!mconfig->m_out_pin)
+		return -ENOMEM;
+
+	skl_fill_module_pin_info(bus->dev, mconfig->m_in_pin,
+				mconfig->max_in_queue);
+	skl_fill_module_pin_info(bus->dev, mconfig->m_out_pin,
+				mconfig->max_out_queue);
+
+	if (mconfig->formats_config.caps_size == 0)
+		goto bind_event;
+
+	mconfig->formats_config.caps = (u32 *)devm_kzalloc(bus->dev,
+				mconfig->formats_config.caps_size, GFP_KERNEL);
+
+	if (mconfig->formats_config.caps == NULL)
+		return -ENOMEM;
+
+	memcpy(mconfig->formats_config.caps, dfw_config->caps.caps,
+						 dfw_config->caps.caps_size);
+
+bind_event:
+	if (tplg_w->event_type == 0) {
+		dev_info(bus->dev, "ASoC: No event handler required\n");
+		return 0;
+	}
+
+	ret = snd_soc_tplg_widget_bind_event(w, skl_tplg_widget_ops,
+			ARRAY_SIZE(skl_tplg_widget_ops), tplg_w->event_type);
+
+	if (ret) {
+		dev_err(bus->dev, "%s: No matching event handlers found for %d\n",
+					__func__, tplg_w->event_type);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static struct snd_soc_tplg_ops skl_tplg_ops  = {
+	.widget_load = skl_tplg_widget_load,
+};
+
+/* This will be read from topology manifest, currently defined here */
+#define SKL_MAX_MCPS 30000000
+#define SKL_FW_MAX_MEM 1000000
+
+/*
+ * SKL topology init routine
+ */
+int skl_tplg_init(struct snd_soc_platform *platform, struct hdac_ext_bus *ebus)
+{
+	int ret;
+	const struct firmware *fw;
+	struct hdac_bus *bus = ebus_to_hbus(ebus);
+	struct skl *skl = ebus_to_skl(ebus);
+
+	ret = request_firmware(&fw, "dfw_sst.bin", bus->dev);
+	if (ret < 0) {
+		dev_err(bus->dev, "config firmware request failed with %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * The complete tplg for SKL is loaded as index 0, we don't use
+	 * any other index
+	 */
+	ret = snd_soc_tplg_component_load(&platform->component, &skl_tplg_ops, fw, 0);
+	if (ret < 0) {
+		dev_err(bus->dev, "tplg component load failed%d\n", ret);
+		return -EINVAL;
+	}
+
+	skl->resource.max_mcps = SKL_MAX_MCPS;
+	skl->resource.max_mem = SKL_FW_MAX_MEM;
+
+	return 0;
+}
diff --git a/sound/soc/intel/skylake/skl-tplg-interface.h b/sound/soc/intel/skylake/skl-tplg-interface.h
index a50689825bca..b258c90d89f1 100644
--- a/sound/soc/intel/skylake/skl-tplg-interface.h
+++ b/sound/soc/intel/skylake/skl-tplg-interface.h
@@ -19,6 +19,27 @@
 #ifndef __HDA_TPLG_INTERFACE_H__
 #define __HDA_TPLG_INTERFACE_H__
 
+/* Default types range from 0~12. type can range from 0 to 0xff
+ * SST types start at higher to avoid any overlapping in future */
+#define SOC_CONTROL_TYPE_HDA_SST_ALGO_PARAMS	200
+#define SOC_CONTROL_TYPE_HDA_SST_MUX		201
+#define SOC_CONTROL_TYPE_HDA_SST_MIX		201
+#define SOC_CONTROL_TYPE_HDA_SST_BYTE		203
+
+#define HDA_SST_CFG_MAX	900 /* size of copier cfg*/
+#define MAX_IN_QUEUE 8
+#define MAX_OUT_QUEUE 8
+
+/* Event types goes here */
+/* Reserve event type 0 for no event handlers */
+enum skl_event_types {
+	SKL_EVENT_NONE = 0,
+	SKL_MIXER_EVENT,
+	SKL_MUX_EVENT,
+	SKL_VMIXER_EVENT,
+	SKL_PGA_EVENT
+};
+
 /**
  * enum skl_ch_cfg - channel configuration
  *
@@ -85,4 +106,61 @@ enum skl_dev_type {
 	SKL_DEVICE_HDALINK = 0x4,
 	SKL_DEVICE_NONE
 };
+
+struct skl_dfw_module_pin {
+	u16 module_id;
+	u16 instance_id;
+	u8 pin_id;
+	bool is_dynamic;
+} __packed;
+
+struct skl_dfw_module_fmt {
+	u32 channels;
+	u32 freq;
+	u32 bit_depth;
+	u32 valid_bit_depth;
+	u32 ch_cfg;
+} __packed;
+
+struct skl_dfw_module_caps {
+	u32 caps_size;
+	u32 caps[HDA_SST_CFG_MAX];
+};
+
+struct skl_dfw_pipe {
+	u8 pipe_id;
+	u8 pipe_priority;
+	u16 conn_type;
+	u32 memory_pages;
+} __packed;
+
+struct skl_dfw_module {
+	u16 module_id;
+	u16 instance_id;
+	u32 max_mcps;
+	u8 core_id;
+	u8 max_in_queue;
+	u8 max_out_queue;
+	u8 is_loadable;
+	u8 conn_type;
+	u8 dev_type;
+	u8 hw_conn_type;
+	u8 time_slot;
+	u32 obs;
+	u32 ibs;
+	u32 params_fixup;
+	u32 converter;
+	u32 module_type;
+	u32 vbus_id;
+	struct skl_dfw_pipe pipe;
+	struct skl_dfw_module_fmt in_fmt;
+	struct skl_dfw_module_fmt out_fmt;
+	struct skl_dfw_module_caps caps;
+} __packed;
+
+struct skl_dfw_algo_data {
+	u32 max;
+	char *params;
+} __packed;
+
 #endif
-- 
1.9.1

Re: [PATCH v2 5/9] ASoC: Intel: Skylake: Add topology core init and handlers

[Liam Girdwood]

On Mon, 2015-08-17 at 22:56 +0530, Vinod Koul wrote:
> From: Jeeja KP <jeeja.kp@intel.com>
> 
> The SKL driver does not code DSP topology in driver. It uses the
> newly added ASoC topology core to parse the topology information
> (controls, widgets and map) from topology binary.
> Each topology element passed private data which contains
> information that driver used to identify the module instance
> within firmware and send IPCs for that module to DSP firmware
> along with parameters.
> This patch adds init routine to invoke topology load and callback
> for topology creation.
> 
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---

> +
> +/*
> + * SKL topology init routine
> + */
> +int skl_tplg_init(struct snd_soc_platform *platform, struct hdac_ext_bus *ebus)
> +{
> +	int ret;
> +	const struct firmware *fw;
> +	struct hdac_bus *bus = ebus_to_hbus(ebus);
> +	struct skl *skl = ebus_to_skl(ebus);
> +
> +	ret = request_firmware(&fw, "dfw_sst.bin", bus->dev);
> +	if (ret < 0) {
> +		dev_err(bus->dev, "config firmware request failed with %d\n", ret);

It would be good to say what file name we are failing with here. 

> +		return ret;
> +	}
> +
> +	/*
> +	 * The complete tplg for SKL is loaded as index 0, we don't use
> +	 * any other index
> +	 */
> +	ret = snd_soc_tplg_component_load(&platform->component, &skl_tplg_ops, fw, 0);
> +	if (ret < 0) {
> +		dev_err(bus->dev, "tplg component load failed%d\n", ret);
> +		return -EINVAL;
> +	}
> +
> +	skl->resource.max_mcps = SKL_MAX_MCPS;
> +	skl->resource.max_mem = SKL_FW_MAX_MEM;
> +
> +	return 0;
> +}
> diff --git a/sound/soc/intel/skylake/skl-tplg-interface.h b/sound/soc/intel/skylake/skl-tplg-interface.h
> index a50689825bca..b258c90d89f1 100644
> --- a/sound/soc/intel/skylake/skl-tplg-interface.h
> +++ b/sound/soc/intel/skylake/skl-tplg-interface.h
> @@ -19,6 +19,27 @@
>  #ifndef __HDA_TPLG_INTERFACE_H__
>  #define __HDA_TPLG_INTERFACE_H__
>  
> +/* Default types range from 0~12. type can range from 0 to 0xff
> + * SST types start at higher to avoid any overlapping in future */
> +#define SOC_CONTROL_TYPE_HDA_SST_ALGO_PARAMS	200
> +#define SOC_CONTROL_TYPE_HDA_SST_MUX		201
> +#define SOC_CONTROL_TYPE_HDA_SST_MIX		201
> +#define SOC_CONTROL_TYPE_HDA_SST_BYTE		203

These are lower than 0xff

Liam

Re: [PATCH v2 5/9] ASoC: Intel: Skylake: Add topology core init and handlers

[Vinod Koul]

On Fri, Sep 18, 2015 at 10:55:57AM +0100, Liam Girdwood wrote:

> > +/* Default types range from 0~12. type can range from 0 to 0xff
> > + * SST types start at higher to avoid any overlapping in future */
> > +#define SOC_CONTROL_TYPE_HDA_SST_ALGO_PARAMS	200
> > +#define SOC_CONTROL_TYPE_HDA_SST_MUX		201
> > +#define SOC_CONTROL_TYPE_HDA_SST_MIX		201
> > +#define SOC_CONTROL_TYPE_HDA_SST_BYTE		203
> 
> These are lower than 0xff

Thanks for pointing, I will update these...

-- 
~Vinod

[PATCH v2 6/9] ASoC: Intel: Skylake: Initialize and load DSP controls

[Vinod Koul]

From: Jeeja KP <jeeja.kp@intel.com>

Initialize and creates DSP controls if processing pipe capability
is supported by HW. Updates the dma_id, hw_params to module param
to be used when DSP module has to be configured.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/intel/skylake/skl-pcm.c | 138 ++++++++++++++++++++++++++++++--------
 1 file changed, 111 insertions(+), 27 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 7d617bf493bc..3e491149bf0c 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -24,6 +24,7 @@
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include "skl.h"
+#include "skl-topology.h"
 
 #define HDA_MONO 1
 #define HDA_STEREO 2
@@ -214,6 +215,7 @@ static int skl_pcm_hw_params(struct snd_pcm_substream *substream,
 	struct hdac_ext_bus *ebus = dev_get_drvdata(dai->dev);
 	struct hdac_ext_stream *stream = get_hdac_ext_stream(substream);
 	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct skl_pipe_params p_params = {0};
 	int ret, dma_id;
 
 	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
@@ -228,6 +230,13 @@ static int skl_pcm_hw_params(struct snd_pcm_substream *substream,
 	dma_id = hdac_stream(stream)->stream_tag - 1;
 	dev_dbg(dai->dev, "dma_id=%d\n", dma_id);
 
+	p_params.s_fmt = snd_pcm_format_width(params_format(params));
+	p_params.ch = params_channels(params);
+	p_params.s_freq = params_rate(params);
+	p_params.host_dma_id = dma_id;
+	p_params.stream = substream->stream;
+	skl_tplg_fe_update_params(dai, &p_params);
+
 	return 0;
 }
 
@@ -268,6 +277,46 @@ static int skl_pcm_hw_free(struct snd_pcm_substream *substream,
 	return skl_substream_free_pages(ebus_to_hbus(ebus), substream);
 }
 
+static int skl_be_hw_params(struct snd_pcm_substream *substream,
+				struct snd_pcm_hw_params *params,
+				struct snd_soc_dai *dai)
+{
+	struct skl_pipe_params p_params = {0};
+
+	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
+
+	p_params.s_fmt = snd_pcm_format_width(params_format(params));
+	p_params.ch = params_channels(params);
+	p_params.s_freq = params_rate(params);
+	p_params.stream = substream->stream;
+	skl_tplg_be_update_params(dai, &p_params);
+
+	return 0;
+}
+
+static int skl_pcm_trigger(struct snd_pcm_substream *substream, int cmd,
+		struct snd_soc_dai *dai)
+{
+	dev_dbg(dai->dev, "In %s cmd=%d\n", __func__, cmd);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+	case SNDRV_PCM_TRIGGER_RESUME:
+		return skl_tplg_set_fe_pipeline_state(dai, true,
+							substream->stream);
+
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+		return skl_tplg_set_fe_pipeline_state(dai, false,
+							substream->stream);
+
+	default:
+		return 0;
+	}
+
+	return 0;
+}
+
 static int skl_link_hw_params(struct snd_pcm_substream *substream,
 				struct snd_pcm_hw_params *params,
 				struct snd_soc_dai *dai)
@@ -277,9 +326,10 @@ static int skl_link_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_pcm_runtime *rtd = snd_pcm_substream_chip(substream);
 	struct skl_dma_params *dma_params;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
-	int dma_id;
+	struct skl_pipe_params p_params = {0};
+
+	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
 
-	pr_debug("%s\n", __func__);
 	link_dev = snd_hdac_ext_stream_assign(ebus, substream,
 					HDAC_EXT_STREAM_TYPE_LINK);
 	if (!link_dev)
@@ -293,7 +343,14 @@ static int skl_link_hw_params(struct snd_pcm_substream *substream,
 	if (dma_params)
 		dma_params->stream_tag =  hdac_stream(link_dev)->stream_tag;
 	snd_soc_dai_set_dma_data(codec_dai, substream, (void *)dma_params);
-	dma_id = hdac_stream(link_dev)->stream_tag - 1;
+
+	p_params.s_fmt = snd_pcm_format_width(params_format(params));
+	p_params.ch = params_channels(params);
+	p_params.s_freq = params_rate(params);
+	p_params.stream = substream->stream;
+	p_params.link_dma_id = hdac_stream(link_dev)->stream_tag - 1;
+
+	skl_tplg_be_update_params(dai, &p_params);
 
 	return 0;
 }
@@ -308,8 +365,6 @@ static int skl_link_pcm_prepare(struct snd_pcm_substream *substream,
 	unsigned int format_val = 0;
 	struct skl_dma_params *dma_params;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
-	struct snd_pcm_hw_params *params;
-	struct snd_interval *channels, *rate;
 	struct hdac_ext_link *link;
 
 	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
@@ -317,18 +372,6 @@ static int skl_link_pcm_prepare(struct snd_pcm_substream *substream,
 		dev_dbg(dai->dev, "already stream is prepared - returning\n");
 		return 0;
 	}
-	params  = devm_kzalloc(dai->dev, sizeof(*params), GFP_KERNEL);
-	if (params == NULL)
-		return -ENOMEM;
-
-	channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS);
-	channels->min = channels->max = substream->runtime->channels;
-	rate = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
-	rate->min = rate->max = substream->runtime->rate;
-	snd_mask_set(&params->masks[SNDRV_PCM_HW_PARAM_FORMAT -
-					SNDRV_PCM_HW_PARAM_FIRST_MASK],
-					substream->runtime->format);
-
 
 	dma_params  = (struct skl_dma_params *)
 			snd_soc_dai_get_dma_data(codec_dai, substream);
@@ -399,13 +442,13 @@ static int skl_link_hw_free(struct snd_pcm_substream *substream,
 	return 0;
 }
 
-static int skl_hda_be_startup(struct snd_pcm_substream *substream,
+static int skl_be_startup(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *dai)
 {
 	return pm_runtime_get_sync(dai->dev);
 }
 
-static void skl_hda_be_shutdown(struct snd_pcm_substream *substream,
+static void skl_be_shutdown(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *dai)
 {
 	pm_runtime_mark_last_busy(dai->dev);
@@ -418,20 +461,28 @@ static struct snd_soc_dai_ops skl_pcm_dai_ops = {
 	.prepare = skl_pcm_prepare,
 	.hw_params = skl_pcm_hw_params,
 	.hw_free = skl_pcm_hw_free,
+	.trigger = skl_pcm_trigger,
 };
 
 static struct snd_soc_dai_ops skl_dmic_dai_ops = {
-	.startup = skl_hda_be_startup,
-	.shutdown = skl_hda_be_shutdown,
+	.startup = skl_be_startup,
+	.hw_params = skl_be_hw_params,
+	.shutdown = skl_be_shutdown,
+};
+
+static struct snd_soc_dai_ops skl_be_ssp_dai_ops = {
+	.startup = skl_be_startup,
+	.hw_params = skl_be_hw_params,
+	.shutdown = skl_be_shutdown,
 };
 
 static struct snd_soc_dai_ops skl_link_dai_ops = {
-	.startup = skl_hda_be_startup,
+	.startup = skl_be_startup,
 	.prepare = skl_link_pcm_prepare,
 	.hw_params = skl_link_hw_params,
 	.hw_free = skl_link_hw_free,
 	.trigger = skl_link_pcm_trigger,
-	.shutdown = skl_hda_be_shutdown,
+	.shutdown = skl_be_shutdown,
 };
 
 static struct snd_soc_dai_driver skl_platform_dai[] = {
@@ -488,6 +539,24 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
 },
 /* BE CPU  Dais */
 {
+	.name = "SSP0 Pin",
+	.ops = &skl_be_ssp_dai_ops,
+	.playback = {
+		.stream_name = "ssp0 Tx",
+		.channels_min = HDA_STEREO,
+		.channels_max = HDA_STEREO,
+		.rates = SNDRV_PCM_RATE_48000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+	},
+	.capture = {
+		.stream_name = "ssp0 Rx",
+		.channels_min = HDA_STEREO,
+		.channels_max = HDA_STEREO,
+		.rates = SNDRV_PCM_RATE_48000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+	},
+},
+{
 	.name = "iDisp Pin",
 	.ops = &skl_link_dai_ops,
 	.playback = {
@@ -577,7 +646,7 @@ static int skl_platform_open(struct snd_pcm_substream *substream)
 	return 0;
 }
 
-static int skl_pcm_trigger(struct snd_pcm_substream *substream,
+static int skl_coupled_trigger(struct snd_pcm_substream *substream,
 					int cmd)
 {
 	struct hdac_ext_bus *ebus = get_bus_ctx(substream);
@@ -651,7 +720,7 @@ static int skl_pcm_trigger(struct snd_pcm_substream *substream,
 	return 0;
 }
 
-static int skl_dsp_trigger(struct snd_pcm_substream *substream,
+static int skl_decoupled_trigger(struct snd_pcm_substream *substream,
 		int cmd)
 {
 	struct hdac_ext_bus *ebus = get_bus_ctx(substream);
@@ -708,9 +777,9 @@ static int skl_platform_pcm_trigger(struct snd_pcm_substream *substream,
 	struct hdac_ext_bus *ebus = get_bus_ctx(substream);
 
 	if (ebus->ppcap)
-		return skl_dsp_trigger(substream, cmd);
+		return skl_decoupled_trigger(substream, cmd);
 	else
-		return skl_pcm_trigger(substream, cmd);
+		return skl_coupled_trigger(substream, cmd);
 }
 
 /* calculate runtime delay from LPIB */
@@ -877,7 +946,17 @@ static int skl_pcm_new(struct snd_soc_pcm_runtime *rtd)
 	return retval;
 }
 
+static int skl_platform_soc_probe(struct snd_soc_platform *platform)
+{
+	struct hdac_ext_bus *ebus = dev_get_drvdata(platform->dev);
+
+	if (ebus->ppcap)
+		return skl_tplg_init(platform, ebus);
+
+	return 0;
+}
 static struct snd_soc_platform_driver skl_platform_drv  = {
+	.probe		= skl_platform_soc_probe,
 	.ops		= &skl_platform_ops,
 	.pcm_new	= skl_pcm_new,
 	.pcm_free	= skl_pcm_free,
@@ -890,6 +969,11 @@ static const struct snd_soc_component_driver skl_component = {
 int skl_platform_register(struct device *dev)
 {
 	int ret;
+	struct hdac_ext_bus *ebus = dev_get_drvdata(dev);
+	struct skl *skl = ebus_to_skl(ebus);
+
+	INIT_LIST_HEAD(&skl->ppl_list);
+	INIT_LIST_HEAD(&skl->dapm_path_list);
 
 	ret = snd_soc_register_platform(dev, &skl_platform_drv);
 	if (ret) {
-- 
1.9.1

Re: [PATCH v2 6/9] ASoC: Intel: Skylake: Initialize and load DSP controls

[Liam Girdwood]

On Mon, 2015-08-17 at 22:56 +0530, Vinod Koul wrote:
> From: Jeeja KP <jeeja.kp@intel.com>
> 
> Initialize and creates DSP controls if processing pipe capability
> is supported by HW. Updates the dma_id, hw_params to module param
> to be used when DSP module has to be configured.
> 
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  sound/soc/intel/skylake/skl-pcm.c | 138 ++++++++++++++++++++++++++++++--------
>  1 file changed, 111 insertions(+), 27 deletions(-)
> 
> diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
> index 7d617bf493bc..3e491149bf0c 100644
> --- a/sound/soc/intel/skylake/skl-pcm.c
> +++ b/sound/soc/intel/skylake/skl-pcm.c
> @@ -24,6 +24,7 @@
>  #include <sound/pcm_params.h>
>  #include <sound/soc.h>
>  #include "skl.h"
> +#include "skl-topology.h"
>  
>  #define HDA_MONO 1
>  #define HDA_STEREO 2
> @@ -214,6 +215,7 @@ static int skl_pcm_hw_params(struct snd_pcm_substream *substream,
>  	struct hdac_ext_bus *ebus = dev_get_drvdata(dai->dev);
>  	struct hdac_ext_stream *stream = get_hdac_ext_stream(substream);
>  	struct snd_pcm_runtime *runtime = substream->runtime;
> +	struct skl_pipe_params p_params = {0};
>  	int ret, dma_id;
>  
>  	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
> @@ -228,6 +230,13 @@ static int skl_pcm_hw_params(struct snd_pcm_substream *substream,
>  	dma_id = hdac_stream(stream)->stream_tag - 1;
>  	dev_dbg(dai->dev, "dma_id=%d\n", dma_id);
>  
> +	p_params.s_fmt = snd_pcm_format_width(params_format(params));
> +	p_params.ch = params_channels(params);
> +	p_params.s_freq = params_rate(params);
> +	p_params.host_dma_id = dma_id;
> +	p_params.stream = substream->stream;
> +	skl_tplg_fe_update_params(dai, &p_params);
> +
>  	return 0;
>  }
>  
> @@ -268,6 +277,46 @@ static int skl_pcm_hw_free(struct snd_pcm_substream *substream,
>  	return skl_substream_free_pages(ebus_to_hbus(ebus), substream);
>  }
>  
> +static int skl_be_hw_params(struct snd_pcm_substream *substream,
> +				struct snd_pcm_hw_params *params,
> +				struct snd_soc_dai *dai)
> +{
> +	struct skl_pipe_params p_params = {0};
> +
> +	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
> +

Seeing a lot of dev_dbg(__func__) in this series that really are just
tracing. Probably best to trace the calls properly or remove if it's
just development debugging.

Liam

Re: [PATCH v2 6/9] ASoC: Intel: Skylake: Initialize and load DSP controls

[Vinod Koul]

On Fri, Sep 18, 2015 at 10:58:54AM +0100, Liam Girdwood wrote:

> > +static int skl_be_hw_params(struct snd_pcm_substream *substream,
> > +				struct snd_pcm_hw_params *params,
> > +				struct snd_soc_dai *dai)
> > +{
> > +	struct skl_pipe_params p_params = {0};
> > +
> > +	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
> > +
> 
> Seeing a lot of dev_dbg(__func__) in this series that really are just
> tracing. Probably best to trace the calls properly or remove if it's
> just development debugging.

Yes tracing will be added later, I will remove bunch of these but keep few
to help debug as we are still adding features :)

-- 
~Vinod

Re: [PATCH v2 6/9] ASoC: Intel: Skylake: Initialize and load DSP controls

[Mark Brown]

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

On Fri, Sep 18, 2015 at 10:58:54AM +0100, Liam Girdwood wrote:
> On Mon, 2015-08-17 at 22:56 +0530, Vinod Koul wrote:

> > +				struct snd_soc_dai *dai)
> > +{
> > +	struct skl_pipe_params p_params = {0};
> > +
> > +	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);

> Seeing a lot of dev_dbg(__func__) in this series that really are just
> tracing. Probably best to trace the calls properly or remove if it's
> just development debugging.

It's all through the whole Intel DSP codebase.  :(

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

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

Re: [PATCH v2 6/9] ASoC: Intel: Skylake: Initialize and load DSP controls

[Vinod Koul]

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

On Sat, Sep 19, 2015 at 09:26:20AM -0700, Mark Brown wrote:
> On Fri, Sep 18, 2015 at 10:58:54AM +0100, Liam Girdwood wrote:
> > On Mon, 2015-08-17 at 22:56 +0530, Vinod Koul wrote:
> 
> > > +				struct snd_soc_dai *dai)
> > > +{
> > > +	struct skl_pipe_params p_params = {0};
> > > +
> > > +	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
> 
> > Seeing a lot of dev_dbg(__func__) in this series that really are just
> > tracing. Probably best to trace the calls properly or remove if it's
> > just development debugging.
> 
> It's all through the whole Intel DSP codebase.  :(

Yes the code is getting features added so we thought of keeping this for a
while, but I agree this doesn't help much, we are adding tracing anyway so
lets add that. Will remove most of these...

Thanks
-- 
~Vinod

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

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

[PATCH v2 7/9] ASoC: Intel: Skylake: Add DSP support and enable it

[Vinod Koul]

From: Jeeja KP <jeeja.kp@intel.com>

If processing pipe capability is supported, add DSP support.
Adds initialization/free/suspend/resume DSP functionality.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/intel/skylake/skl.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 348d094e81d6..2f1890e703c6 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -166,11 +166,16 @@ static int skl_runtime_suspend(struct device *dev)
 	struct pci_dev *pci = to_pci_dev(dev);
 	struct hdac_ext_bus *ebus = pci_get_drvdata(pci);
 	struct hdac_bus *bus = ebus_to_hbus(ebus);
+	struct skl *skl = ebus_to_skl(ebus);
+	int ret;
 
 	dev_dbg(bus->dev, "in %s\n", __func__);
 
 	/* enable controller wake up event */
 	snd_hdac_chip_updatew(bus, WAKEEN, 0, STATESTS_INT_MASK);
+	ret = skl_suspend_dsp(skl);
+	if (ret < 0)
+		return ret;
 
 	snd_hdac_bus_stop_chip(bus);
 	snd_hdac_bus_enter_link_reset(bus);
@@ -183,7 +188,7 @@ static int skl_runtime_resume(struct device *dev)
 	struct pci_dev *pci = to_pci_dev(dev);
 	struct hdac_ext_bus *ebus = pci_get_drvdata(pci);
 	struct hdac_bus *bus = ebus_to_hbus(ebus);
-	struct skl *hda = ebus_to_skl(ebus);
+	struct skl *skl = ebus_to_skl(ebus);
 	int status;
 
 	dev_dbg(bus->dev, "in %s\n", __func__);
@@ -191,12 +196,12 @@ static int skl_runtime_resume(struct device *dev)
 	/* Read STATESTS before controller reset */
 	status = snd_hdac_chip_readw(bus, STATESTS);
 
-	skl_init_pci(hda);
+	skl_init_pci(skl);
 	snd_hdac_bus_init_chip(bus, true);
 	/* disable controller Wake Up event */
 	snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, 0);
 
-	return 0;
+	return skl_resume_dsp(skl);
 }
 #endif /* CONFIG_PM */
 
@@ -457,17 +462,19 @@ static int skl_probe(struct pci_dev *pci,
 
 	/* check if dsp is there */
 	if (ebus->ppcap) {
-		/* TODO register with dsp IPC */
-		dev_dbg(bus->dev, "Register dsp\n");
+		err = skl_init_dsp(skl);
+		if (err < 0) {
+			dev_dbg(bus->dev, "error failed to register dsp\n");
+			goto out_free;
+		}
 	}
-
 	if (ebus->mlcap)
 		snd_hdac_ext_bus_get_ml_capabilities(ebus);
 
 	/* create device for soc dmic */
 	err = skl_dmic_device_register(skl);
 	if (err < 0)
-		goto out_free;
+		goto out_dsp_free;
 
 	/* register platform dai and controls */
 	err = skl_platform_register(bus->dev);
@@ -491,6 +498,8 @@ out_unregister:
 	skl_platform_unregister(bus->dev);
 out_dmic_free:
 	skl_dmic_device_unregister(skl);
+out_dsp_free:
+	skl_free_dsp(skl);
 out_free:
 	skl->init_failed = 1;
 	skl_free(ebus);
@@ -507,6 +516,7 @@ static void skl_remove(struct pci_dev *pci)
 		pm_runtime_get_noresume(&pci->dev);
 	pci_dev_put(pci);
 	skl_platform_unregister(&pci->dev);
+	skl_free_dsp(skl);
 	skl_dmic_device_unregister(skl);
 	skl_free(ebus);
 	dev_set_drvdata(&pci->dev, NULL);
-- 
1.9.1

[PATCH v2 8/9] ASoC: Intel: Skylake: Initialize NHLT table

[Vinod Koul]

From: Jeeja KP <jeeja.kp@intel.com>

Load and Initialize Non HDA Link Table in Skylake driver
to get platform configuration.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
---
 sound/soc/intel/skylake/skl.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 2f1890e703c6..ca135b8ab5c0 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -458,6 +458,11 @@ static int skl_probe(struct pci_dev *pci,
 	if (err < 0)
 		goto out_free;
 
+	skl->nhlt = skl_nhlt_init(bus->dev);
+
+	if (skl->nhlt == NULL)
+		goto out_free;
+
 	pci_set_drvdata(skl->pci, ebus);
 
 	/* check if dsp is there */
-- 
1.9.1

Re: [PATCH v2 8/9] ASoC: Intel: Skylake: Initialize NHLT table

[Mark Brown]

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

On Mon, Aug 17, 2015 at 10:56:43PM +0530, Vinod Koul wrote:
> From: Jeeja KP <jeeja.kp@intel.com>
> 
> Load and Initialize Non HDA Link Table in Skylake driver
> to get platform configuration.

Didn't we need this before we started trying to use the NHLT?

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

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

Re: [PATCH v2 8/9] ASoC: Intel: Skylake: Initialize NHLT table

[Vinod Koul]

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

On Sat, Sep 19, 2015 at 09:27:20AM -0700, Mark Brown wrote:
> On Mon, Aug 17, 2015 at 10:56:43PM +0530, Vinod Koul wrote:
> > From: Jeeja KP <jeeja.kp@intel.com>
> > 
> > Load and Initialize Non HDA Link Table in Skylake driver
> > to get platform configuration.
> 
> Didn't we need this before we started trying to use the NHLT?

We have only added NHLT code, not calls to it. Since the whole driver
series is not merged we thought best to add enabling bits in later patches,
so added here at last.

Thanks
-- 
~Vinod

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

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

[PATCH v2 9/9] ASoC: Intel: Skylake: Remove unused CPU dai's

[Vinod Koul]

From: Jeeja KP <jeeja.kp@intel.com>

We need to create CPU DAI for each endpoint instance. For this we
should have one DMIC DAI, one HDA DAI and SSP DAI.  Thus, DMIC23,
HDA-SPK/AMIC was not required so this patch removes them

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/intel/skylake/skl-pcm.c | 33 ---------------------------------
 1 file changed, 33 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 3e491149bf0c..c96411dc89f5 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -579,17 +579,6 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
 	},
 },
 {
-	.name = "DMIC23 Pin",
-	.ops = &skl_dmic_dai_ops,
-	.capture = {
-		.stream_name = "DMIC23 Rx",
-		.channels_min = HDA_STEREO,
-		.channels_max = HDA_STEREO,
-		.rates = SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_16000,
-		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
-	},
-},
-{
 	.name = "HD-Codec Pin",
 	.ops = &skl_link_dai_ops,
 	.playback = {
@@ -607,28 +596,6 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
 		.formats = SNDRV_PCM_FMTBIT_S16_LE,
 	},
 },
-{
-	.name = "HD-Codec-SPK Pin",
-	.ops = &skl_link_dai_ops,
-	.playback = {
-		.stream_name = "HD-Codec-SPK Tx",
-		.channels_min = HDA_STEREO,
-		.channels_max = HDA_STEREO,
-		.rates = SNDRV_PCM_RATE_48000,
-		.formats = SNDRV_PCM_FMTBIT_S16_LE,
-	},
-},
-{
-	.name = "HD-Codec-AMIC Pin",
-	.ops = &skl_link_dai_ops,
-	.capture = {
-		.stream_name = "HD-Codec-AMIC Rx",
-		.channels_min = HDA_STEREO,
-		.channels_max = HDA_STEREO,
-		.rates = SNDRV_PCM_RATE_48000,
-		.formats = SNDRV_PCM_FMTBIT_S16_LE,
-	},
-},
 };
 
 static int skl_platform_open(struct snd_pcm_substream *substream)
-- 
1.9.1

Re: [PATCH v2 0/9] Add DSP topology management for SKL

[Vinod Koul]

On Mon, Aug 17, 2015 at 10:56:35PM +0530, Vinod Koul wrote:
> The SKL driver does not code DSP topology in driver. With this series
> the ASoC topology framework is used to parse the topology information
> to create widgets, controls and route map for the FW topology.
> 
> It adds routines for SKL DSP module configuration, compute resources
> for modules, initialize and bind the pipe modules. It uses the SKL IPC
> library added earlier to send IPC for initialize the module, bind/unbind
> modules.
> Last patch in this series removes the unused dais.

Hi Mark,

I was wondering if you have any feedback on this series ?

Thanks

-- 
~Vinod

> 
> changes in v2:
> - add more explanation in changelog and code
> - add explanation of MCPS
> - fix whitespace issues
> - remove skl widget check on event handler as it is redundant
> - add switch for depth
> - add else for direction based code
> - remove cast for NHLT blob query
> - make debug prints to error
> - remove dump info
> - clean some debug prints
> 
> Jeeja KP (9):
>   ASoC: Intel: Skylake: Add pipe and modules handlers
>   ASoC: Intel: Skylake: Add module configuration helpers
>   ASoC: Intel: Skylake: add DSP platform widget event handlers
>   ASoC: Intel: Skylake: Add FE and BE hw_params handling
>   ASoC: Intel: Skylake: Add topology core init and handlers
>   ASoC: Intel: Skylake: Initialize and load DSP controls
>   ASoC: Intel: Skylake: Add DSP support and enable it
>   ASoC: Intel: Skylake: Initialize NHLT table
>   ASoC: Intel: Skylake: Remove unused CPU dai's
> 
>  sound/soc/intel/skylake/Makefile             |    3 +-
>  sound/soc/intel/skylake/skl-pcm.c            |  171 ++--
>  sound/soc/intel/skylake/skl-topology.c       | 1262 ++++++++++++++++++++++++++
>  sound/soc/intel/skylake/skl-topology.h       |   21 +
>  sound/soc/intel/skylake/skl-tplg-interface.h |   78 ++
>  sound/soc/intel/skylake/skl.c                |   29 +-
>  sound/soc/intel/skylake/skl.h                |   11 +
>  7 files changed, 1507 insertions(+), 68 deletions(-)
>  create mode 100644 sound/soc/intel/skylake/skl-topology.c
> 
> -- 
> 1.9.1
> 

Re: [PATCH v2 0/9] Add DSP topology management for SKL

[Mark Brown]

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

On Thu, Sep 03, 2015 at 01:44:54PM +0530, Vinod Koul wrote:

> I was wondering if you have any feedback on this series ?

Please don't send content free pings (as I'm sure I've mentioned
before).  I've not looked at this yet because it's another large,
invasive patch series which you sent at the very end of the development
cycle.

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

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

Re: [PATCH v2 0/9] Add DSP topology management for SKL

[Mark Brown]

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

On Fri, Sep 11, 2015 at 12:45:00PM +0100, Mark Brown wrote:
> On Thu, Sep 03, 2015 at 01:44:54PM +0530, Vinod Koul wrote:

> > I was wondering if you have any feedback on this series ?

> Please don't send content free pings (as I'm sure I've mentioned
> before).  I've not looked at this yet because it's another large,
> invasive patch series which you sent at the very end of the development
> cycle.

So, I've now looked at these, though not fully.  To expand on the above
a bit: these are big bits of very device specific code that's really
hard to read and understand and when I do read it I'm seeing similar
issues again and again both in terms of big picture stuff and the
trivial details (including the inconsistencies in things like commenting
style that I'm not specifically pulling up).  I'm really worried that
I'm often seeing fragments of things that may or may not work well
together.

These Intel devices not only have a lot more code than most devices do,
they also get a lot more exposure than most devices do to end users so I
actually have to try to make some effort to understand what's going on
and can't skimp on review even though I feel like I am.  I'm really
concerned about where we're heading here and how robustly the results
are going to work.

This all really slows down review, both directly in requiring detailed
checks of hard to follow the code and as a result of making almost all
other code review seem much more productive.

As well as the low level code quality issues it would really help if
rather than continually sending large blocks of functionality we were
starting from something that was a minimally viable but functional
system and then extending that.  That should be a lot easier to follow.

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

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

Re: [PATCH v2 0/9] Add DSP topology management for SKL

[Vinod Koul]

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

On Sat, Sep 19, 2015 at 09:56:12AM -0700, Mark Brown wrote:
> On Fri, Sep 11, 2015 at 12:45:00PM +0100, Mark Brown wrote:
> > On Thu, Sep 03, 2015 at 01:44:54PM +0530, Vinod Koul wrote:
> 
> > > I was wondering if you have any feedback on this series ?
> 
> > Please don't send content free pings (as I'm sure I've mentioned
> > before).  I've not looked at this yet because it's another large,
> > invasive patch series which you sent at the very end of the development
> > cycle.
> 
> So, I've now looked at these, though not fully.  To expand on the above
> a bit: these are big bits of very device specific code that's really
> hard to read and understand and when I do read it I'm seeing similar
> issues again and again both in terms of big picture stuff and the
> trivial details (including the inconsistencies in things like commenting
> style that I'm not specifically pulling up).  I'm really worried that
> I'm often seeing fragments of things that may or may not work well
> together.
> 
> These Intel devices not only have a lot more code than most devices do,
> they also get a lot more exposure than most devices do to end users so I
> actually have to try to make some effort to understand what's going on
> and can't skimp on review even though I feel like I am.  I'm really
> concerned about where we're heading here and how robustly the results
> are going to work.
> 
> This all really slows down review, both directly in requiring detailed
> checks of hard to follow the code and as a result of making almost all
> other code review seem much more productive.
> 
> As well as the low level code quality issues it would really help if
> rather than continually sending large blocks of functionality we were
> starting from something that was a minimally viable but functional
> system and then extending that.  That should be a lot easier to follow.

Thanks Mark for the review,

I wouldn't have pinged but had been more than few weeks so was wondering if
this got missed. I did realise you were travelling and merge window came
up, but still thought Mark's typical turnaound is 1-2 weeks so was expecting
something. I do understand this take time but no harm in checking if it is
beyond usual window one expects.

Now on the code due to inherent nature of HW (we have HDA + DSP + I2S!!!)
and SW scalability, we do need lots of code for basic audio.
Btw  _this_ patch series is the last one to get basic minimal audio out of SKL.
After this we will add machine and then features...

We did decide to use DPCM, DAPM and topology here to solve manging the DSP
using these frameworks. yes hard coding a simple mixer and two pipes could
have made code look easy to follow but then we would be redoing those bits
as we scale these to defferent designs.

And to provide what we are trying here I have attached the SKL Topology conf
file to give a picture of topology we are trying to build as a reference.

On code issues, I will fix the style and wrappers issues you raised and
other instances where this might be required.

Also where you feel something is missing please do ask. Sometimes things are
perceived intuitive for us as we are doing this for quire some time but
may not be so for fresh eyes :)

Thanks
-- 
~Vinod

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

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

Re: [PATCH v2 0/9] Add DSP topology management for SKL

[Mark Brown]

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

On Mon, Sep 21, 2015 at 09:27:39AM +0530, Vinod Koul wrote:

> I wouldn't have pinged but had been more than few weeks so was wondering if
> this got missed. I did realise you were travelling and merge window came
> up, but still thought Mark's typical turnaound is 1-2 weeks so was expecting
> something. I do understand this take time but no harm in checking if it is
> beyond usual window one expects.

I'm also not going to merge large patches which have wide impact close
to the merge window and I don't tend to review things without a view to
merging them, especially not things that are more work to review.

> We did decide to use DPCM, DAPM and topology here to solve manging the DSP
> using these frameworks. yes hard coding a simple mixer and two pipes could
> have made code look easy to follow but then we would be redoing those bits
> as we scale these to defferent designs.

Sure, and my point here is that that this process of redoing things
would most likely be a lot faster since it'd be a lot easier to follow
which would make the review a lot easier.

> And to provide what we are trying here I have attached the SKL Topology conf
> file to give a picture of topology we are trying to build as a reference.

I think you missed the attachment here...

> Also where you feel something is missing please do ask. Sometimes things are
> perceived intuitive for us as we are doing this for quire some time but
> may not be so for fresh eyes :)

I have been doing this mostly but it's easy to miss things without the
picture of where we're heading.

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

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

Re: [PATCH v2 0/9] Add DSP topology management for SKL

[Vinod Koul]

[-- Attachment #1.1.1: Type: text/plain, Size: 870 bytes --]

On Mon, Sep 21, 2015 at 09:33:53AM -0700, Mark Brown wrote:
> On Mon, Sep 21, 2015 at 09:27:39AM +0530, Vinod Koul wrote:
> > We did decide to use DPCM, DAPM and topology here to solve manging the DSP
> > using these frameworks. yes hard coding a simple mixer and two pipes could
> > have made code look easy to follow but then we would be redoing those bits
> > as we scale these to defferent designs.
> 
> Sure, and my point here is that that this process of redoing things
> would most likely be a lot faster since it'd be a lot easier to follow
> which would make the review a lot easier.
> 
> > And to provide what we are trying here I have attached the SKL Topology conf
> > file to give a picture of topology we are trying to build as a reference.
> 
> I think you missed the attachment here...

Oops sorry about that, here you go

-- 
~Vinod

[-- Attachment #1.1.2: skl_i2s.conf --]
[-- Type: text/plain, Size: 13318 bytes --]

SectionControlMixer."media0_mixin" {
	index "1"
	
	max "1"
	invert "false"
	no_pm "true"

	channel."fl" {
		reg "-1"
		shift "0"
	}

	channel."fr" {
		reg "-1"
		shift "0"
	}

	ops."ctl" {
		info "64"
		get "64"
		put "64"
	}
}

SectionControlMixer."media1_mixin" {
	index "1"
	
	max "1"
	invert "false"
	no_pm "true"

	channel."fl" {
		reg "-1"
		shift "0"
	}

	channel."fr" {
		reg "-1"
		shift "0"
	}

	ops."ctl" {
		info "64"
		get "64"
		put "64"
	}
}

SectionControlMixer."media2_mixin" {
	index "1"
	
	max "1"
	invert "false"
	no_pm "true"

	channel."fl" {
		reg "-1"
		shift "0"
	}

	channel."fr" {
		reg "-1"
		shift "0"
	}

	ops."ctl" {
		info "64"
		get "64"
		put "64"
	}
}

SectionControlMixer."media3_mixin" {
	index "1"
	
	max "1"
	invert "false"
	no_pm "true"

	channel."fl" {
		reg "-1"
		shift "0"
	}

	channel."fr" {
		reg "-1"
		shift "0"
	}

	ops."ctl" {
		info "64"
		get "64"
		put "64"
	}
}

SectionControlMixer."speech_mixin" {
	index "1"
	
	max "1"
	invert "false"
	no_pm "true"

	channel."fl" {
		reg "-1"
		shift "0"
	}

	channel."fr" {
		reg "-1"
		shift "0"
	}

	ops."ctl" {
		info "64"
		get "64"
		put "64"
	}
}

SectionControlMixer."tone_mixin" {
	index "1"
	
	max "1"
	invert "false"
	no_pm "true"

	channel."fl" {
		reg "-1"
		shift "0"
	}

	channel."fr" {
		reg "-1"
		shift "0"
	}

	ops."ctl" {
		info "64"
		get "64"
		put "64"
	}
}

SectionControlMixer."dmic01_hifi_mixin" {
	index "1"
	
	max "1"
	invert "false"
	no_pm "true"

	channel."fl" {
		reg "-1"
		shift "0"
	}

	channel."fr" {
		reg "-1"
		shift "0"
	}

	ops."ctl" {
		info "64"
		get "64"
		put "64"
	}
}

SectionControlMixer."dmic23_hifi_mixin" {
	index "1"
	
	max "1"
	invert "false"
	no_pm "true"

	channel."fl" {
		reg "-1"
		shift "0"
	}

	channel."fr" {
		reg "-1"
		shift "0"
	}

	ops."ctl" {
		info "64"
		get "64"
		put "64"
	}
}

SectionControlMixer."dmic01_voice_mixin" {
	index "1"
	
	max "1"
	invert "false"
	no_pm "true"

	channel."fl" {
		reg "-1"
		shift "0"
	}

	channel."fr" {
		reg "-1"
		shift "0"
	}

	ops."ctl" {
		info "64"
		get "64"
		put "64"
	}
}

SectionControlMixer."dmic23_voice_mixin" {
	index "1"
	
	max "1"
	invert "false"
	no_pm "true"

	channel."fl" {
		reg "-1"
		shift "0"
	}

	channel."fr" {
		reg "-1"
		shift "0"
	}

	ops."ctl" {
		info "64"
		get "64"
		put "64"
	}
}

SectionControlMixer."codec0_in_mixin" {
	index "1"
	
	max "1"
	invert "false"
	no_pm "true"

	channel."fl" {
		reg "-1"
		shift "0"
	}

	channel."fr" {
		reg "-1"
		shift "0"
	}

	ops."ctl" {
		info "64"
		get "64"
		put "64"
	}
}

SectionControlMixer."codec1_in_mixin" {
	index "1"
	
	max "1"
	invert "false"
	no_pm "true"

	channel."fl" {
		reg "-1"
		shift "0"
	}

	channel."fr" {
		reg "-1"
		shift "0"
	}

	ops."ctl" {
		info "64"
		get "64"
		put "64"
	}
}

SectionWidget."codec0_out" {

	index "1"
	type "aif_out"
	no_pm "true"
}

SectionWidget."codec1_out" {

	index "1"
	type "aif_out"
	no_pm "true"
}

SectionWidget."iDisp_out" {

	index "1"
	type "aif_out"
	no_pm "true"
}

SectionWidget."dmic01_hifi" {

	index "1"
	type "aif_in"
	no_pm "true"
}

SectionWidget."dmic23_hifi" {

	index "1"
	type "aif_in"
	no_pm "true"
}

SectionWidget."codec0_in" {

	index "1"
	type "aif_in"
	no_pm "true"
}

SectionWidget."codec1_in" {

	index "1"
	type "aif_in"
	no_pm "true"
}

SectionWidget."media2_in cpr vmix 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "3"
	event_flags "15"
	subseq "0"

	data "media2_in cpr vmix 0"
}

SectionWidget."media3_in cpr vmix 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "3"
	event_flags "15"
	subseq "0"

	data "media3_in cpr vmix 0"
}

SectionWidget."media01_out cpr vmix 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "3"
	event_flags "15"
	subseq "0"

	data "media01_out cpr vmix 0"
}

SectionWidget."codec0_in cpr vmix 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "3"
	event_flags "15"
	subseq "0"

	data "codec0_in cpr vmix 0"
}

SectionWidget."codec1_in cpr vmix 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "3"
	event_flags "15"
	subseq "0"

	data "codec1_in cpr vmix 0"
}

SectionWidget."dmic01_hifi cpr vmix 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "3"
	event_flags "15"
	subseq "0"

	data "dmic01_hifi cpr vmix 0"
}

SectionWidget."dmic23_hifi cpr vmix 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "3"
	event_flags "15"
	subseq "0"

	data "dmic23_hifi cpr vmix 0"
}

SectionWidget."codec1_out cpr 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "4"
	subseq "0"

	data "codec1_out cpr 0"
}

SectionWidget."codec0_out cpr 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "4"
	subseq "0"

	data "codec0_out cpr 0"
}

SectionWidget."iDisp_out cpr 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "4"
	subseq "0"

	data "iDisp_out cpr 0"
}

SectionWidget."media0_out cpr 0" {
	index "1"
	type "mixer"
	no_pm "true"
	subseq "0"

	data "media0_out cpr 0"
}

SectionWidget."media2_out cpr 0" {
	index "1"
	type "mixer"
	no_pm "true"
	subseq "0"

	data "media2_out cpr 0"
}

SectionWidget."codec1_out_mixout mix 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "1"
	event_flags "15"
	subseq "10"
	
	data "codec1_out_mixout mix 0"

	mixer [
		"media0_mixin"
		"media1_mixin"
		"media2_mixin"
		"media3_mixin"
		"speech_mixin"
		"tone_mixin"
		"dmic01_hifi_mixin"
		"dmic23_hifi_mixin"
		"dmic01_voice_mixin"
		"dmic23_voice_mixin"
		"codec0_in_mixin"
		"codec1_in_mixin"
	]
}

SectionWidget."media0_mixout mix 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "1"
	event_flags "15"
	subseq "10"
	
	data "media0_mixout mix 0"

	mixer [
		"media0_mixin"
		"media1_mixin"
		"media2_mixin"
		"media3_mixin"
		"speech_mixin"
		"tone_mixin"
		"dmic01_hifi_mixin"
		"dmic23_hifi_mixin"
		"dmic01_voice_mixin"
		"dmic23_voice_mixin"
		"codec0_in_mixin"
		"codec1_in_mixin"
	]
}

SectionWidget."media2_mixout mix 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "1"
	event_flags "15"
	subseq "10"
	
	data "media2_mixout mix 0"

	mixer [
		"media0_mixin"
		"media1_mixin"
		"media2_mixin"
		"media3_mixin"
		"speech_mixin"
		"tone_mixin"
		"dmic01_hifi_mixin"
		"dmic23_hifi_mixin"
		"dmic01_voice_mixin"
		"dmic23_voice_mixin"
		"codec0_in_mixin"
		"codec1_in_mixin"
	]
}

SectionWidget."codec0_out_mixout mix 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "1"
	event_flags "15"
	subseq "10"
	
	data "codec0_out_mixout mix 0"

	mixer [
		"media0_mixin"
		"media1_mixin"
		"media2_mixin"
		"media3_mixin"
		"speech_mixin"
		"tone_mixin"
		"dmic01_hifi_mixin"
		"dmic23_hifi_mixin"
		"dmic01_voice_mixin"
		"dmic23_voice_mixin"
		"codec0_in_mixin"
		"codec1_in_mixin"
	]
}

SectionWidget."iDisp_out_mixout mix 0" {
	index "1"
	type "mixer"
	no_pm "true"
	event_type "1"
	event_flags "15"
	subseq "10"
	
	data "iDisp_out_mixout mix 0"

	mixer [
		"media0_mixin"
		"media1_mixin"
		"media2_mixin"
		"media3_mixin"
		"speech_mixin"
		"tone_mixin"
		"dmic01_hifi_mixin"
		"dmic23_hifi_mixin"
		"dmic01_voice_mixin"
		"dmic23_voice_mixin"
		"codec0_in_mixin"
		"codec1_in_mixin"
	]
}

SectionWidget."media2_mixin" {
	index "1"
	type "pga"
	no_pm "true"
	event_type "4"
	event_flags "15"
	subseq "10"

	data "media2_mixin"
}

SectionWidget."dmic01_hifi_mixin" {
	index "1"
	type "pga"
	no_pm "true"
	event_type "4"
	event_flags "15"
	subseq "10"

	data "dmic01_hifi_mixin"
}

SectionWidget."dmic23_hifi_mixin" {
	index "1"
	type "pga"
	no_pm "true"
	event_type "4"
	event_flags "15"
	subseq "10"

	data "dmic23_hifi_mixin"
}

SectionWidget."media3_mixin" {
	index "1"
	type "pga"
	no_pm "true"
	event_type "4"
	event_flags "15"
	subseq "10"

	data "media3_mixin"
}

SectionWidget."codec0_in_mixin" {
	index "1"
	type "pga"
	no_pm "true"
	event_type "4"
	event_flags "15"
	subseq "10"

	data "codec0_in_mixin"
}

SectionWidget."codec1_in_mixin" {
	index "1"
	type "pga"
	no_pm "true"
	event_type "4"
	event_flags "15"
	subseq "10"

	data "codec1_in_mixin"
}

SectionPCMCapabilities."System Playback" {

	formats "S24_LE,S16_LE"
	rate_min "48000"
	rate_max "48000"
	channels_min "2"
	channels_max "2"
}

SectionGraph."Pipeline 1 Graph" {

	index "1"

	lines [
		"media2_in cpr vmix 0, , System Playback"
		"media2_mixin, , media2_in cpr vmix 0"
		"media3_in cpr vmix 0, , Deepbuffer Playback"
		"media3_mixin, , media3_in cpr vmix 0"
		"codec1_out_mixout mix 0, media0_mixin, media0_mixin"
		"codec1_out_mixout mix 0, media1_mixin, media1_mixin"
		"codec1_out_mixout mix 0, media2_mixin, media2_mixin"
		"codec1_out_mixout mix 0, media3_mixin, media3_mixin"
		"codec1_out_mixout mix 0, speech_mixin, speech_mixin"
		"codec1_out_mixout mix 0, tone_mixin, tone_mixin"
		"codec1_out_mixout mix 0, dmic01_hifi_mixin, dmic01_hifi_mixin"
		"codec1_out_mixout mix 0, dmic23_hifi_mixin, dmic23_hifi_mixin"
		"codec1_out_mixout mix 0, dmic01_voice_mixin, dmic01_voice_mixin"
		"codec1_out_mixout mix 0, dmic23_voice_mixin, dmic23_voice_mixin"
		"codec1_out_mixout mix 0, codec0_in_mixin, codec0_in_mixin"
		"codec1_out_mixout mix 0, codec1_in_mixin, codec1_in_mixin"
		"codec1_out cpr 0, , codec1_out_mixout mix 0"
		"codec1_out, , codec1_out cpr 0"
		"codec0_out_mixout mix 0, media0_mixin, media0_mixin"
		"codec0_out_mixout mix 0, media1_mixin, media1_mixin"
		"codec0_out_mixout mix 0, media2_mixin, media2_mixin"
		"codec0_out_mixout mix 0, media3_mixin, media3_mixin"
		"codec0_out_mixout mix 0, speech_mixin, speech_mixin"
		"codec0_out_mixout mix 0, tone_mixin, tone_mixin"
		"codec0_out_mixout mix 0, dmic01_hifi_mixin, dmic01_hifi_mixin"
		"codec0_out_mixout mix 0, dmic23_hifi_mixin, dmic23_hifi_mixin"
		"codec0_out_mixout mix 0, dmic01_voice_mixin, dmic01_voice_mixin"
		"codec0_out_mixout mix 0, dmic23_voice_mixin, dmic23_voice_mixin"
		"codec0_out_mixout mix 0, codec0_in_mixin, codec0_in_mixin"
		"codec0_out_mixout mix 0, codec1_in_mixin, codec1_in_mixin"
		"codec0_out cpr 0, , codec0_out_mixout mix 0"
		"codec0_out, , codec0_out cpr 0"
		"iDisp_out_mixout mix 0, media0_mixin, media0_mixin"
		"iDisp_out_mixout mix 0, media1_mixin, media1_mixin"
		"iDisp_out_mixout mix 0, media2_mixin, media2_mixin"
		"iDisp_out_mixout mix 0, media3_mixin, media3_mixin"
		"iDisp_out_mixout mix 0, speech_mixin, speech_mixin"
		"iDisp_out_mixout mix 0, tone_mixin, tone_mixin"
		"iDisp_out_mixout mix 0, dmic01_hifi_mixin, dmic01_hifi_mixin"
		"iDisp_out_mixout mix 0, dmic23_hifi_mixin, dmic23_hifi_mixin"
		"iDisp_out_mixout mix 0, dmic01_voice_mixin, dmic01_voice_mixin"
		"iDisp_out_mixout mix 0, dmic23_voice_mixin, dmic23_voice_mixin"
		"iDisp_out_mixout mix 0, codec0_in_mixin, codec0_in_mixin"
		"iDisp_out_mixout mix 0, codec1_in_mixin, codec1_in_mixin"
		"iDisp_out cpr 0, , iDisp_out_mixout mix 0"
		"iDisp_out, , iDisp_out cpr 0"
		"System Capture, , media0_out cpr 0"
		"media0_out cpr 0, , media0_mixout mix 0"
		"media0_mixout mix 0, media0_mixin, media0_mixin"
		"media0_mixout mix 0, media1_mixin, media1_mixin"
		"media0_mixout mix 0, media2_mixin, media2_mixin"
		"media0_mixout mix 0, media3_mixin, media3_mixin"
		"media0_mixout mix 0, speech_mixin, speech_mixin"
		"media0_mixout mix 0, tone_mixin, tone_mixin"
		"media0_mixout mix 0, dmic01_hifi_mixin, dmic01_hifi_mixin"
		"media0_mixout mix 0, dmic23_hifi_mixin, dmic23_hifi_mixin"
		"media0_mixout mix 0, dmic01_voice_mixin, dmic01_voice_mixin"
		"media0_mixout mix 0, dmic23_voice_mixin, dmic23_voice_mixin"
		"media0_mixout mix 0, codec0_in_mixin, codec0_in_mixin"
		"media0_mixout mix 0, codec1_in_mixin, codec1_in_mixin"
		"Reference Capture, , media2_out cpr 0"
		"media2_out cpr 0, , media2_mixout mix 0"
		"media2_mixout mix 0, media0_mixin, media0_mixin"
		"media2_mixout mix 0, media1_mixin, media1_mixin"
		"media2_mixout mix 0, media2_mixin, media2_mixin"
		"media2_mixout mix 0, media3_mixin, media3_mixin"
		"media2_mixout mix 0, speech_mixin, speech_mixin"
		"media2_mixout mix 0, tone_mixin, tone_mixin"
		"media2_mixout mix 0, dmic01_hifi_mixin, dmic01_hifi_mixin"
		"media2_mixout mix 0, dmic23_hifi_mixin, dmic23_hifi_mixin"
		"media2_mixout mix 0, dmic01_voice_mixin, dmic01_voice_mixin"
		"media2_mixout mix 0, dmic23_voice_mixin, dmic23_voice_mixin"
		"media2_mixout mix 0, codec0_in_mixin, codec0_in_mixin"
		"media2_mixout mix 0, codec1_in_mixin, codec1_in_mixin"
		"codec0_in_mixin, , codec0_in cpr vmix 0"
		"codec0_in cpr vmix 0, , codec0_in"
		"codec1_in_mixin, , codec1_in cpr vmix 0"
		"codec1_in cpr vmix 0, , codec1_in"
		"dmic01_hifi_mixin, , dmic01_hifi cpr vmix 0"
		"dmic01_hifi cpr vmix 0, , dmic01_hifi"
		"dmic23_hifi_mixin, , dmic23_hifi cpr vmix 0"
		"dmic23_hifi cpr vmix 0, , dmic23_hifi"
	]
}

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

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