[v3] ASoC: dapm: Add new widget type snd_soc_dapm_dsp_component
diff mbox

Message ID 1497555964-4862-1-git-send-email-liam.r.girdwood@linux.intel.com
State New
Headers show

Commit Message

Liam Girdwood June 15, 2017, 7:46 p.m. UTC
Add a new widget type to represent internal DSP components and allow DSP
drivers and firmware to define topologies using this widget type.

Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>

---

Takashi, I'll send a subsequent patch for alsa-lib if Mark is happy with this
version.

Changes V2.
   o No need for multiple types since core does not care.
Changes v1.
   o Added some documentation.
   o Split codec widget into encoder and decoder to avoid confusion with HW
     CODECS.

 Documentation/sound/soc/dapm.rst | 4 ++++
 include/sound/soc-dapm.h         | 1 +
 include/uapi/sound/asoc.h        | 3 ++-
 sound/soc/soc-topology.c         | 1 +
 4 files changed, 8 insertions(+), 1 deletion(-)

Comments

Mark Brown June 16, 2017, 11:09 a.m. UTC | #1
On Thu, Jun 15, 2017 at 08:46:04PM +0100, Liam Girdwood wrote:
> Add a new widget type to represent internal DSP components and allow DSP
> drivers and firmware to define topologies using this widget type.

Hrm, this is replacing the previous ones with buffer, effect, and the
SRCs?  I actually thought most of the types could be useful there - I
can see from a sequencing point of view we might want to start buffers
first, and the SRC and encoder/decoder widgets are going to be useful
when we're tracking more than just simple routing with DAPM.  The only
bit I was really concerned about was the overlap between pipeline and
effect.
Liam Girdwood June 16, 2017, 12:26 p.m. UTC | #2
On Fri, 2017-06-16 at 12:09 +0100, Mark Brown wrote:
> On Thu, Jun 15, 2017 at 08:46:04PM +0100, Liam Girdwood wrote:
> > Add a new widget type to represent internal DSP components and allow DSP
> > drivers and firmware to define topologies using this widget type.
> 
> Hrm, this is replacing the previous ones with buffer, effect, and the
> SRCs?  I actually thought most of the types could be useful there - I
> can see from a sequencing point of view we might want to start buffers
> first, and the SRC and encoder/decoder widgets are going to be useful
> when we're tracking more than just simple routing with DAPM. 

I'm not sure if they would be useful for sequencing by the DAPM core
since the widgets will really be internal to the DSP and there would be
an increased latency cost of sequencing outside of the DSP.

The main use that I can see is in defining topologies (and probably
exporting topology) and this is where the current DAPM widget list has
some missing types wrt to DSPs. 

I can either :-

1) Use a specific buffer, src, asrc, etc widget types for each component

or

2) Use a generic widget and append private data with type.

Preference is 1 as it simplifies driver code.

>  The only
> bit I was really concerned about was the overlap between pipeline and
> effect.

I know, but you made me think about whether the core really does care
about DSP specific types. :)

Ok, I can redo and use specific types, but there still is a problem of
how do we configure a pipeline (a collection of widgets, where several
pipelines can make a path from PCM to DAI). This configuration data is
not hw/sw params type data, but more DSP scheduling type data. My
original thought was the pipeline widget, as there is nothing else that
cleanly fits into the ABI and that the pipeline scheduling data is DAPM
widget/graph related anyway.

Another alternative is extending the topology ABI and introduce a
pipeline object that would be ignore by the core and just used by
certain drivers and firmwares ?

Liam
Liam Girdwood June 16, 2017, 2:06 p.m. UTC | #3
On Fri, 2017-06-16 at 13:26 +0100, Liam Girdwood wrote:

> 
> Ok, I can redo and use specific types, but there still is a problem of
> how do we configure a pipeline (a collection of widgets, where several
> pipelines can make a path from PCM to DAI). This configuration data is
> not hw/sw params type data, but more DSP scheduling type data. My
> original thought was the pipeline widget, as there is nothing else that
> cleanly fits into the ABI and that the pipeline scheduling data is DAPM
> widget/graph related anyway.
> 
> Another alternative is extending the topology ABI and introduce a
> pipeline object that would be ignore by the core and just used by
> certain drivers and firmwares ?
> 


I think I'm tending towards having a scheduler widget now (instead of a
pipeline widget) and that would be a valid part of the graph and also
discoverable via debugfs etc.

Liam

---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
Mark Brown June 16, 2017, 4:52 p.m. UTC | #4
On Fri, Jun 16, 2017 at 03:06:38PM +0100, Liam Girdwood wrote:
> On Fri, 2017-06-16 at 13:26 +0100, Liam Girdwood wrote:

> > Ok, I can redo and use specific types, but there still is a problem of
> > how do we configure a pipeline (a collection of widgets, where several
> > pipelines can make a path from PCM to DAI). This configuration data is
> > not hw/sw params type data, but more DSP scheduling type data. My
> > original thought was the pipeline widget, as there is nothing else that
> > cleanly fits into the ABI and that the pipeline scheduling data is DAPM
> > widget/graph related anyway.

> > Another alternative is extending the topology ABI and introduce a
> > pipeline object that would be ignore by the core and just used by
> > certain drivers and firmwares ?

> I think I'm tending towards having a scheduler widget now (instead of a
> pipeline widget) and that would be a valid part of the graph and also
> discoverable via debugfs etc.

Ah, I see what you're going for with that now I think.  So something
that looks in DAPM terms a bit like a supply widget (in that it's not
really part of the path itself but provides things the path needs in
order to function)?  I think that makes sense to me now too.

Patch
diff mbox

diff --git a/Documentation/sound/soc/dapm.rst b/Documentation/sound/soc/dapm.rst
index a27f42befa4d..540a86012e29 100644
--- a/Documentation/sound/soc/dapm.rst
+++ b/Documentation/sound/soc/dapm.rst
@@ -105,6 +105,10 @@  Pre
 	Special PRE widget (exec before all others)
 Post
 	Special POST widget (exec after all others)
+DSP Component
+	Internal DSP component not used by the ASoC core but may be used
+	by DSP driver and DSP firmware.
+
 
 (Widgets are defined in include/sound/soc-dapm.h)
 
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index a466f4bdc835..87133c974991 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -510,6 +510,7 @@  enum snd_soc_dapm_type {
 	snd_soc_dapm_dai_out,
 	snd_soc_dapm_dai_link,		/* link between two DAI structures */
 	snd_soc_dapm_kcontrol,		/* Auto-disabled kcontrol */
+	snd_soc_dapm_dsp_component,	/* DSP internal component */
 };
 
 enum snd_soc_dapm_subclass {
diff --git a/include/uapi/sound/asoc.h b/include/uapi/sound/asoc.h
index 6702533c8bd8..e805ef651538 100644
--- a/include/uapi/sound/asoc.h
+++ b/include/uapi/sound/asoc.h
@@ -73,7 +73,8 @@ 
 #define SND_SOC_TPLG_DAPM_DAI_IN	13
 #define SND_SOC_TPLG_DAPM_DAI_OUT	14
 #define SND_SOC_TPLG_DAPM_DAI_LINK	15
-#define SND_SOC_TPLG_DAPM_LAST		SND_SOC_TPLG_DAPM_DAI_LINK
+#define SND_SOC_TPLG_DAPM_DSP_COMPONENT	16
+#define SND_SOC_TPLG_DAPM_LAST		SND_SOC_TPLG_DAPM_DSP_COMPONENT
 
 /* Header magic number and string sizes */
 #define SND_SOC_TPLG_MAGIC		0x41536F43 /* ASoC */
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 12e189701924..17751736957f 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -242,6 +242,7 @@  static const struct soc_tplg_map dapm_map[] = {
 	{SND_SOC_TPLG_DAPM_DAI_IN, snd_soc_dapm_dai_in},
 	{SND_SOC_TPLG_DAPM_DAI_OUT, snd_soc_dapm_dai_out},
 	{SND_SOC_TPLG_DAPM_DAI_LINK, snd_soc_dapm_dai_link},
+	{SND_SOC_TPLG_DAPM_DSP_COMPONENT, snd_soc_dapm_dsp_component},
 };
 
 static int tplc_chan_get_reg(struct soc_tplg *tplg,