[PATCH v4 0/7] ASoC: intel - add skylake PCM driver

[PATCH v4 0/7] ASoC: intel - add skylake PCM driver

[Vinod Koul]

SKL has HDA controller based audio subsystem with DSP and support for I2S,
HDA, PDM links. The hda core code has been moved to sound/hda/ by Takashi
which current HDA drivers use and will also be used by ASoC SKL driver.

The SKL platform driver will load and create the soc_hdac_bus which embeds
the hdac_bus, same for hdac_device (hda codecs) and hdac_stream (pcms) This
is on top of hdac code in Takashi's topic/hda

This patch provides the match function for asoc type hda codecs and let's
them get enumerated by hdac. The second patch in this series adds the
controller specific soc code. Common parts are in hdac core with changes
introduced as part of SKL controller in soc part. Then we add the rest of
controller PCM driver code (still HDA) and last patch breaks the HDA streams
to host and link which will allow insertion of DSP in between these links.

The subsequent series will add IPC driver for SKL (using common IPC
routines), then DSP topology handlers, DSP code with I2S support and then
lastly when DFW is accepted then its handlers.

This patch series adds the hda codec match functions followed by asoc hda
controller routines, then SKL PCM driver and last decouples the controller
for splitting the links

Fixes in v4:
  Updates changelog in patch1 and few other patches
  Address Takashi's comment
  Address Marks comments

Jeeja KP (7):
  ASoC: hda - add ASoC HDA codec match function
  ALSA: hda - add new HDA registers
  ASoC: hda - add asoc hda core bus, controller and stream helpers
  ASoC: intel - add Skylake HDA platform driver
  ASoC: intel - add Skylake HDA audio driver
  ASoC: intel - add makefile support for SKL driver
  ASoC: intel - adds support for decoupled mode in skl driver

 include/sound/hda_register.h          |  88 ++++
 include/sound/soc-hda-codec.h         |  49 ++
 include/sound/soc-hdaudio.h           | 360 +++++++++++++
 sound/soc/Kconfig                     |   1 +
 sound/soc/Makefile                    |   1 +
 sound/soc/hda/Kconfig                 |   3 +
 sound/soc/hda/Makefile                |   4 +
 sound/soc/hda/soc-hda-codec.c         |  89 ++++
 sound/soc/hda/soc-hdac-bus.c          | 115 +++++
 sound/soc/hda/soc-hdac-controller.c   | 296 +++++++++++
 sound/soc/hda/soc-hdac-stream.c       | 409 +++++++++++++++
 sound/soc/intel/Kconfig               |  17 +
 sound/soc/intel/Makefile              |   1 +
 sound/soc/intel/skylake/Makefile      |   3 +
 sound/soc/intel/skylake/hda-skl-pcm.c | 937 ++++++++++++++++++++++++++++++++++
 sound/soc/intel/skylake/hda-skl.c     | 670 ++++++++++++++++++++++++
 sound/soc/intel/skylake/hda-skl.h     |  74 +++
 17 files changed, 3117 insertions(+)
 create mode 100644 include/sound/soc-hda-codec.h
 create mode 100644 include/sound/soc-hdaudio.h
 create mode 100644 sound/soc/hda/Kconfig
 create mode 100644 sound/soc/hda/Makefile
 create mode 100644 sound/soc/hda/soc-hda-codec.c
 create mode 100644 sound/soc/hda/soc-hdac-bus.c
 create mode 100644 sound/soc/hda/soc-hdac-controller.c
 create mode 100644 sound/soc/hda/soc-hdac-stream.c
 create mode 100644 sound/soc/intel/skylake/Makefile
 create mode 100644 sound/soc/intel/skylake/hda-skl-pcm.c
 create mode 100644 sound/soc/intel/skylake/hda-skl.c
 create mode 100644 sound/soc/intel/skylake/hda-skl.h

-- 
1.9.1

[PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Vinod Koul]

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

ASoC hda codec driver will use id_table for registration.
id_table has info of codec vendor_id, revision_id and name.
For ASoC device/driver matching, device vendor_id, rev_id
will be matched to id_table to identify the device/driver.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 include/sound/soc-hda-codec.h | 49 ++++++++++++++++++++++++
 sound/soc/Kconfig             |  1 +
 sound/soc/Makefile            |  1 +
 sound/soc/hda/Kconfig         |  3 ++
 sound/soc/hda/Makefile        |  3 ++
 sound/soc/hda/soc-hda-codec.c | 89 +++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 146 insertions(+)
 create mode 100644 include/sound/soc-hda-codec.h
 create mode 100644 sound/soc/hda/Kconfig
 create mode 100644 sound/soc/hda/Makefile
 create mode 100644 sound/soc/hda/soc-hda-codec.c

diff --git a/include/sound/soc-hda-codec.h b/include/sound/soc-hda-codec.h
new file mode 100644
index 000000000000..850430a3855b
--- /dev/null
+++ b/include/sound/soc-hda-codec.h
@@ -0,0 +1,49 @@
+/*
+ * ASoC High Definition Audio Codec interface Definitions
+ *
+ * Copyright (c) 2014-2015 Intel Corporation
+ *
+ * Author(s): 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 published by the Free
+ *  Software Foundation; either version 2 of the License, or (at your option)
+ *  any later version.
+ *
+ *  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.
+ *
+ */
+
+#ifndef __SOUND_SOC_HDA_CODEC_H
+#define __SOUND_SOC_HDA_CODEC_H
+
+#include <sound/hdaudio.h>
+
+/* HDA device table */
+#define HDA_NAME_SIZE      20
+struct soc_hda_device_id {
+	__u32 id;
+	__u32 rev_id;
+	char name[HDA_NAME_SIZE];
+	unsigned long driver_data;
+};
+
+struct soc_hda_codec_driver {
+	struct hdac_driver core;
+	const struct soc_hda_device_id *id_table;
+};
+
+#define to_hdac_driver(drv) (container_of((drv), \
+				 struct hdac_driver, driver))
+#define to_soc_hda_codec_driver(hdrv) (container_of((hdrv), \
+				 struct soc_hda_codec_driver, core))
+
+int snd_soc_hda_codec_driver_register(struct soc_hda_codec_driver *drv);
+void snd_soc_hda_codec_driver_unregister(struct soc_hda_codec_driver *drv);
+const struct soc_hda_device_id *snd_soc_hda_get_device_id(
+	struct hdac_device *hdev, struct soc_hda_codec_driver *drv);
+
+#endif
diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 3ba52da18bc6..d3903580cb10 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -40,6 +40,7 @@ source "sound/soc/cirrus/Kconfig"
 source "sound/soc/davinci/Kconfig"
 source "sound/soc/dwc/Kconfig"
 source "sound/soc/fsl/Kconfig"
+source "sound/soc/hda/Kconfig"
 source "sound/soc/jz4740/Kconfig"
 source "sound/soc/nuc900/Kconfig"
 source "sound/soc/omap/Kconfig"
diff --git a/sound/soc/Makefile b/sound/soc/Makefile
index 974ba708b482..8741d6a38bf6 100644
--- a/sound/soc/Makefile
+++ b/sound/soc/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_SND_SOC)	+= cirrus/
 obj-$(CONFIG_SND_SOC)	+= davinci/
 obj-$(CONFIG_SND_SOC)	+= dwc/
 obj-$(CONFIG_SND_SOC)	+= fsl/
+obj-$(CONFIG_SND_SOC)	+= hda/
 obj-$(CONFIG_SND_SOC)	+= jz4740/
 obj-$(CONFIG_SND_SOC)	+= intel/
 obj-$(CONFIG_SND_SOC)	+= mxs/
diff --git a/sound/soc/hda/Kconfig b/sound/soc/hda/Kconfig
new file mode 100644
index 000000000000..815943360bc5
--- /dev/null
+++ b/sound/soc/hda/Kconfig
@@ -0,0 +1,3 @@
+config SND_SOC_HDA_CORE
+	tristate
+	select SND_HDA_CORE
diff --git a/sound/soc/hda/Makefile b/sound/soc/hda/Makefile
new file mode 100644
index 000000000000..9585ab180a55
--- /dev/null
+++ b/sound/soc/hda/Makefile
@@ -0,0 +1,3 @@
+snd-soc-hda-core-objs := soc-hda-codec.o
+
+obj-$(CONFIG_SND_SOC_HDA_CORE) += snd-soc-hda-core.o
diff --git a/sound/soc/hda/soc-hda-codec.c b/sound/soc/hda/soc-hda-codec.c
new file mode 100644
index 000000000000..90c8fa53e2cc
--- /dev/null
+++ b/sound/soc/hda/soc-hda-codec.c
@@ -0,0 +1,89 @@
+/*
+ * soc-hda-codec.c ASoC High Definition Audio Codec interface Definitions
+ *
+ * Copyright (c) 2014-2015 Intel Corporation
+ *
+ * Author(s): Jeeja KP <jeeja.kp@intel.com>
+ *
+ *
+ *  This driver is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This driver 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/module.h>
+#include <sound/hdaudio.h>
+#include <sound/soc-hda-codec.h>
+
+/**
+ * snd_soc_hda_get_device_id - gets the hdac device id entry
+ * @hdev: HD-audio core device
+ * @drv: HD-audio soc codec driver
+ *
+ * Compares the hdac device vendor_id and revision_id to the hdac_soc_codec
+ * driver id_table and returns the matching device id entry.
+ */
+const struct soc_hda_device_id *
+snd_soc_hda_get_device_id(struct hdac_device *hdev,
+				struct soc_hda_codec_driver *drv)
+{
+	if (drv->id_table) {
+		const struct soc_hda_device_id *id  = drv->id_table;
+
+		while (id->name[0]) {
+			if (hdev->vendor_id == id->id &&
+				(!id->rev_id || id->rev_id == hdev->revision_id))
+				return id;
+			id++;
+		}
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(snd_soc_hda_get_device_id);
+
+/**
+ * soc_hda_codec_match - ASoC HDA codec match function
+ * @dev: HD-audio core device
+ * @@drv: HD-audio soc codec driver
+ *
+ * This tries to match the hdac device with hdac driver and return 1 on
+ * match, 0 otherwise
+ */
+static int soc_hda_codec_match(struct hdac_device *dev, struct hdac_driver *drv)
+{
+	struct soc_hda_codec_driver *driver = to_soc_hda_codec_driver(drv);
+
+	if (snd_soc_hda_get_device_id(dev, driver))
+		return 1;
+	else
+		return 0;
+}
+
+/**
+ * snd_soc_hda_codec_driver_register - register hda codec driver
+ * @drv: HD Audio soc codec driver
+ */
+int snd_soc_hda_codec_driver_register(struct soc_hda_codec_driver *drv)
+{
+	drv->core.driver.bus = &snd_hda_bus_type;
+	drv->core.type = HDA_DEV_ASOC;
+	drv->core.match = soc_hda_codec_match;
+	return driver_register(&drv->core.driver);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hda_codec_driver_register);
+
+/**
+ * snd_soc_hda_codec_driver_unregister - unregister hda codec driver
+ * @drv: HD Audio soc codec driver
+ */
+void snd_soc_hda_codec_driver_unregister(struct soc_hda_codec_driver *drv)
+{
+	driver_unregister(&drv->core.driver);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hda_codec_driver_unregister);
-- 
1.9.1

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Mark Brown]

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

On Mon, May 11, 2015 at 04:23:59PM +0530, Vinod Koul wrote:
> From: Jeeja KP <jeeja.kp@intel.com>
> 
> ASoC hda codec driver will use id_table for registration.
> id_table has info of codec vendor_id, revision_id and name.
> For ASoC device/driver matching, device vendor_id, rev_id
> will be matched to id_table to identify the device/driver.

I'm sorry but I still don't entirely understand what this is supposed to
do.  It *looks* like it's trying to create a bus for HDA with this:

> +/* HDA device table */
> +#define HDA_NAME_SIZE      20
> +struct soc_hda_device_id {
> +	__u32 id;
> +	__u32 rev_id;
> +	char name[HDA_NAME_SIZE];
> +	unsigned long driver_data;
> +};
> +
> +struct soc_hda_codec_driver {
> +	struct hdac_driver core;
> +	const struct soc_hda_device_id *id_table;
> +};

but it's not actually defining a driver model bus type (though it does
use driver_register()) and I'd expect the bus type to be provided by the
generic HDA code.  Looking (fairly quickly admittedly) at the HDA code
it's not entirely obvious how it all fits together and the changelog for
that just talks about moving code around.

We really need to make sure we don't end up with the same shambles we've
got for AC'97 here (though we're unlikely to get similar touchscreen
type things so it should be less bad).

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

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

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Takashi Iwai]

At Fri, 22 May 2015 13:56:34 +0100,
Mark Brown wrote:
> 
> On Mon, May 11, 2015 at 04:23:59PM +0530, Vinod Koul wrote:
> > From: Jeeja KP <jeeja.kp@intel.com>
> > 
> > ASoC hda codec driver will use id_table for registration.
> > id_table has info of codec vendor_id, revision_id and name.
> > For ASoC device/driver matching, device vendor_id, rev_id
> > will be matched to id_table to identify the device/driver.
> 
> I'm sorry but I still don't entirely understand what this is supposed to
> do.  It *looks* like it's trying to create a bus for HDA with this:
> 
> > +/* HDA device table */
> > +#define HDA_NAME_SIZE      20
> > +struct soc_hda_device_id {
> > +	__u32 id;
> > +	__u32 rev_id;
> > +	char name[HDA_NAME_SIZE];
> > +	unsigned long driver_data;
> > +};
> > +
> > +struct soc_hda_codec_driver {
> > +	struct hdac_driver core;
> > +	const struct soc_hda_device_id *id_table;
> > +};
> 
> but it's not actually defining a driver model bus type (though it does
> use driver_register()) and I'd expect the bus type to be provided by the
> generic HDA code.  Looking (fairly quickly admittedly) at the HDA code
> it's not entirely obvious how it all fits together and the changelog for
> that just talks about moving code around.

The bus is already defined in sound/hda.  There, the actual binding is
done in each hda driver specifics, i.e. in sound/soc/hda and
sound/pci/hda.  It's the way to allow binding completely different
drivers for the very same PCI ID.


Takashi

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Mark Brown]

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

On Fri, May 22, 2015 at 03:35:03PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:

> > I'm sorry but I still don't entirely understand what this is supposed to
> > do.  It *looks* like it's trying to create a bus for HDA with this:

> > but it's not actually defining a driver model bus type (though it does
> > use driver_register()) and I'd expect the bus type to be provided by the
> > generic HDA code.  Looking (fairly quickly admittedly) at the HDA code
> > it's not entirely obvious how it all fits together and the changelog for
> > that just talks about moving code around.

> The bus is already defined in sound/hda.  There, the actual binding is

I see there's a bus_type defined there but this is calling raw
device_register() and providing a match function so it seems like
there's something missing or an abstraction problem.  I'd expect a bus
to be providing things like match functions and bus specific
registration functions.

This is all setting off alarm bells, both the code and the very
aggressive pushing.

> done in each hda driver specifics, i.e. in sound/soc/hda and
> sound/pci/hda.  It's the way to allow binding completely different
> drivers for the very same PCI ID.

I don't really understand this, sorry.  What impact would HDA bus
implementation have on PCI device drivers (by the time you're
registering HDA devices presumably the PCI device will already be
enumerated)?  

Having two drivers for the same PCI function doesn't sound like an
*obviously* good idea (as we discussed recently) but that's a bit of a
separate thing.

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

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

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Takashi Iwai]

At Fri, 22 May 2015 18:41:06 +0100,
Mark Brown wrote:
> 
> On Fri, May 22, 2015 at 03:35:03PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > I'm sorry but I still don't entirely understand what this is supposed to
> > > do.  It *looks* like it's trying to create a bus for HDA with this:
> 
> > > but it's not actually defining a driver model bus type (though it does
> > > use driver_register()) and I'd expect the bus type to be provided by the
> > > generic HDA code.  Looking (fairly quickly admittedly) at the HDA code
> > > it's not entirely obvious how it all fits together and the changelog for
> > > that just talks about moving code around.
> 
> > The bus is already defined in sound/hda.  There, the actual binding is
> 
> I see there's a bus_type defined there but this is calling raw
> device_register() and providing a match function so it seems like
> there's something missing or an abstraction problem.  I'd expect a bus
> to be providing things like match functions and bus specific
> registration functions.

This was intentionally left in that way, so that the upper core layer
(HDA ASoC or legacy core parts) can implement the matching method more
specifically.  It can be done in the hdac_bus itself, but currently
still leaves more room until we see the exact matching condition for
ASoC.

Once when the matching method can be unified, we can move the it to
hdac_bus.  But it's not necessarily done now.

> This is all setting off alarm bells, both the code and the very
> aggressive pushing.
> 
> > done in each hda driver specifics, i.e. in sound/soc/hda and
> > sound/pci/hda.  It's the way to allow binding completely different
> > drivers for the very same PCI ID.
> 
> I don't really understand this, sorry.  What impact would HDA bus
> implementation have on PCI device drivers (by the time you're
> registering HDA devices presumably the PCI device will already be
> enumerated)?  
>
> Having two drivers for the same PCI function doesn't sound like an
> *obviously* good idea (as we discussed recently) but that's a bit of a
> separate thing.

Suggesting two PCI IDs might have been confusing, sorry.

The point is that a HD-audio object can be inherited to two different
level of objects, legacy and ASoC.  Both are bound on the same bus,
but to the corresponding drivers.  Both objects use the very same bus
ops, thus they share the same hdac_bus.


Takashi

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Vinod Koul]

On Fri, May 22, 2015 at 08:13:45PM +0200, Takashi Iwai wrote:
> At Fri, 22 May 2015 18:41:06 +0100,
> Mark Brown wrote:
> > 
> > On Fri, May 22, 2015 at 03:35:03PM +0200, Takashi Iwai wrote:
> > > Mark Brown wrote:
> > 
> > > > I'm sorry but I still don't entirely understand what this is supposed to
> > > > do.  It *looks* like it's trying to create a bus for HDA with this:
> > 
> > > > but it's not actually defining a driver model bus type (though it does
> > > > use driver_register()) and I'd expect the bus type to be provided by the
> > > > generic HDA code.  Looking (fairly quickly admittedly) at the HDA code
> > > > it's not entirely obvious how it all fits together and the changelog for
> > > > that just talks about moving code around.
> > 
> > > The bus is already defined in sound/hda.  There, the actual binding is
> > 
> > I see there's a bus_type defined there but this is calling raw
> > device_register() and providing a match function so it seems like
> > there's something missing or an abstraction problem.  I'd expect a bus
> > to be providing things like match functions and bus specific
> > registration functions.
> 
> This was intentionally left in that way, so that the upper core layer
> (HDA ASoC or legacy core parts) can implement the matching method more
> specifically.  It can be done in the hdac_bus itself, but currently
> still leaves more room until we see the exact matching condition for
> ASoC.
> 
> Once when the matching method can be unified, we can move the it to
> hdac_bus.  But it's not necessarily done now.
> 
> > This is all setting off alarm bells, both the code and the very
> > aggressive pushing.
> > 
> > > done in each hda driver specifics, i.e. in sound/soc/hda and
> > > sound/pci/hda.  It's the way to allow binding completely different
> > > drivers for the very same PCI ID.
> > 
> > I don't really understand this, sorry.  What impact would HDA bus
> > implementation have on PCI device drivers (by the time you're
> > registering HDA devices presumably the PCI device will already be
> > enumerated)?  
> >
> > Having two drivers for the same PCI function doesn't sound like an
> > *obviously* good idea (as we discussed recently) but that's a bit of a
> > separate thing.
> 
> Suggesting two PCI IDs might have been confusing, sorry.
> 
> The point is that a HD-audio object can be inherited to two different
> level of objects, legacy and ASoC.  Both are bound on the same bus,
> but to the corresponding drivers.  Both objects use the very same bus
> ops, thus they share the same hdac_bus.
Thanks a bunch Takashi for clarifying :)

Mark,

I guess one clarification may still help, The SKL Audio controller is PCI
device, the PCM driver in this series will load against the PCI device.
Now the driver will go ahead and initialize the HDA bus. Also we provide our
own matching function here.

The matching function is for matching the devices on HDA link. The probing
of the bus will find the HDA codecs present in the bus and we will match
them based on Vendor ID and Device ID in match function above.

Let us know if you have more questions

-- 
~Vinod

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Mark Brown]

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

On Sat, May 23, 2015 at 11:21:56AM +0530, Vinod Koul wrote:
> On Fri, May 22, 2015 at 08:13:45PM +0200, Takashi Iwai wrote:

> > The point is that a HD-audio object can be inherited to two different
> > level of objects, legacy and ASoC.  Both are bound on the same bus,
> > but to the corresponding drivers.  Both objects use the very same bus
> > ops, thus they share the same hdac_bus.

> Thanks a bunch Takashi for clarifying :)

> I guess one clarification may still help, The SKL Audio controller is PCI
> device, the PCM driver in this series will load against the PCI device.
> Now the driver will go ahead and initialize the HDA bus. Also we provide our
> own matching function here.

> The matching function is for matching the devices on HDA link. The probing
> of the bus will find the HDA codecs present in the bus and we will match
> them based on Vendor ID and Device ID in match function above.

> Let us know if you have more questions

To be honest the above isn't really clarifying things for me.  I know
what a matching function is for, the thing that is really worrying about
this is that we've got different matching functions depending on how the
HDA bus is instantiated.  Given that HDA is an enumerable bus why would
we want or need that - why and how does the matching differ depending on
the driver we're using for the bus?  I would not expect that matching
using the HDA identification registers would be something that varies
depending on how we register the bus.

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

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

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Vinod Koul]

On Mon, May 25, 2015 at 11:48:28AM +0100, Mark Brown wrote:
> On Sat, May 23, 2015 at 11:21:56AM +0530, Vinod Koul wrote:
> > On Fri, May 22, 2015 at 08:13:45PM +0200, Takashi Iwai wrote:
> 
> > > The point is that a HD-audio object can be inherited to two different
> > > level of objects, legacy and ASoC.  Both are bound on the same bus,
> > > but to the corresponding drivers.  Both objects use the very same bus
> > > ops, thus they share the same hdac_bus.
> 
> > Thanks a bunch Takashi for clarifying :)
> 
> > I guess one clarification may still help, The SKL Audio controller is PCI
> > device, the PCM driver in this series will load against the PCI device.
> > Now the driver will go ahead and initialize the HDA bus. Also we provide our
> > own matching function here.
> 
> > The matching function is for matching the devices on HDA link. The probing
> > of the bus will find the HDA codecs present in the bus and we will match
> > them based on Vendor ID and Device ID in match function above.
> 
> > Let us know if you have more questions
> 
> To be honest the above isn't really clarifying things for me.  I know
> what a matching function is for,
I was trying to clarify that we are matching hda codec device here and not
PCI HDA controller

> the thing that is really worrying about
> this is that we've got different matching functions depending on how the
> HDA bus is instantiated.  Given that HDA is an enumerable bus why would
> we want or need that - why and how does the matching differ depending on
> the driver we're using for the bus?  I would not expect that matching
> using the HDA identification registers would be something that varies
> depending on how we register the bus.

The hdac core doesn't actually do matching. If you see the match
function provided by core (hda_bus_match), it is a wrapper and
actual matching for legacy devices is being done in legacy code, see
hda_codec_match. This match function expects the hdac_device to be
wrapped in legacy hda_codec, which we don't need here.

So for ASoC we are embedding hdac_device in soc_hda_codec and using
the vendor_id and revision_id to match, so hda to write a new one.

I do not mind if we commonize this and have common match function in
hdac, if legacy is happy with it. Or perhaps the move to core later on
as ASoC HDA matures, either way whatever you n Takashi agree with
would be okay by me.

HTH

Thanks
-- 
~Vinod

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Takashi Iwai]

At Mon, 25 May 2015 16:51:50 +0530,
Vinod Koul wrote:
> 
> On Mon, May 25, 2015 at 11:48:28AM +0100, Mark Brown wrote:
> > On Sat, May 23, 2015 at 11:21:56AM +0530, Vinod Koul wrote:
> > > On Fri, May 22, 2015 at 08:13:45PM +0200, Takashi Iwai wrote:
> > 
> > > > The point is that a HD-audio object can be inherited to two different
> > > > level of objects, legacy and ASoC.  Both are bound on the same bus,
> > > > but to the corresponding drivers.  Both objects use the very same bus
> > > > ops, thus they share the same hdac_bus.
> > 
> > > Thanks a bunch Takashi for clarifying :)
> > 
> > > I guess one clarification may still help, The SKL Audio controller is PCI
> > > device, the PCM driver in this series will load against the PCI device.
> > > Now the driver will go ahead and initialize the HDA bus. Also we provide our
> > > own matching function here.
> > 
> > > The matching function is for matching the devices on HDA link. The probing
> > > of the bus will find the HDA codecs present in the bus and we will match
> > > them based on Vendor ID and Device ID in match function above.
> > 
> > > Let us know if you have more questions
> > 
> > To be honest the above isn't really clarifying things for me.  I know
> > what a matching function is for,
> I was trying to clarify that we are matching hda codec device here and not
> PCI HDA controller
> 
> > the thing that is really worrying about
> > this is that we've got different matching functions depending on how the
> > HDA bus is instantiated.  Given that HDA is an enumerable bus why would
> > we want or need that - why and how does the matching differ depending on
> > the driver we're using for the bus?  I would not expect that matching
> > using the HDA identification registers would be something that varies
> > depending on how we register the bus.
> 
> The hdac core doesn't actually do matching. If you see the match
> function provided by core (hda_bus_match), it is a wrapper and
> actual matching for legacy devices is being done in legacy code, see
> hda_codec_match. This match function expects the hdac_device to be
> wrapped in legacy hda_codec, which we don't need here.
> 
> So for ASoC we are embedding hdac_device in soc_hda_codec and using
> the vendor_id and revision_id to match, so hda to write a new one.
> 
> I do not mind if we commonize this and have common match function in
> hdac, if legacy is happy with it. Or perhaps the move to core later on
> as ASoC HDA matures, either way whatever you n Takashi agree with
> would be okay by me.

This is the next step.  It would need a fairly big amount of rewrite
in each legacy HDA codec driver, and I don't want it for 4.2.
Once when we get the common criteria for matching (both for asoc and
legacy), we can move to the unified match method.

That said, the reason for individual match mechanism is not to break
the legacy hda code.  If anyone can provide a patch to achieve that
within 100 LOCs and without regression, I'd happily take it :)


Takashi

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Mark Brown]

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

On Mon, May 25, 2015 at 01:55:59PM +0200, Takashi Iwai wrote:
> Vinod Koul wrote:

> > The hdac core doesn't actually do matching. If you see the match
> > function provided by core (hda_bus_match), it is a wrapper and
> > actual matching for legacy devices is being done in legacy code, see
> > hda_codec_match. This match function expects the hdac_device to be
> > wrapped in legacy hda_codec, which we don't need here.

> > So for ASoC we are embedding hdac_device in soc_hda_codec and using
> > the vendor_id and revision_id to match, so hda to write a new one.

You keep on describing what the code does; I can mostly see what the
code does (and could probably figure out extra bits if I wasn't getting
scared off by what I do see) - that's basically what I'm flagging as an
issue.  What I'm having trouble seeing is why this makes sense.  The
question isn't what, it's why.

> > I do not mind if we commonize this and have common match function in
> > hdac, if legacy is happy with it. Or perhaps the move to core later on
> > as ASoC HDA matures, either way whatever you n Takashi agree with
> > would be okay by me.

> This is the next step.  It would need a fairly big amount of rewrite
> in each legacy HDA codec driver, and I don't want it for 4.2.

So how does this actually work then?  If we need to rework all the HDA
CODEC drivers before they work with this new HDA bus (or at least this
new HDA bus when used with controllers that implement a custom match
function) then does it make sense to start trying to use it now?

I would have expected this to be the /first/ step - refactor the
existing HDA code, then add new users.

> Once when we get the common criteria for matching (both for asoc and
> legacy), we can move to the unified match method.

I really don't understand this idea that we need to work out what the
common criteria for matching are, HDA is an enumerable bus isn't it?

> That said, the reason for individual match mechanism is not to break
> the legacy hda code.  If anyone can provide a patch to achieve that
> within 100 LOCs and without regression, I'd happily take it :)

Another way of looking at this is to ask why the CODEC driver matching
can't continue to work the way it currently does - we're transitioning
to a bus (which makes sense as a cleanup) but I've not seen any reason
articulated for tying that up the merge of the Sky Lake code and it
seems like we're not very near being in a position to be able to do
that.  Or could we just keep the bigger hda_codec struct around in the
bus code for now even if it winds up being bloated?

Like I say it's all the why questions that aren't making sense to me
beyond the "we're too scared to touch the existing code" ones.

I'm also wondering how this interacts with the Tegra HDA device by the
way...

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

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

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Takashi Iwai]

At Mon, 25 May 2015 14:58:54 +0100,
Mark Brown wrote:
> 
> On Mon, May 25, 2015 at 01:55:59PM +0200, Takashi Iwai wrote:
> > Vinod Koul wrote:
> 
> > > The hdac core doesn't actually do matching. If you see the match
> > > function provided by core (hda_bus_match), it is a wrapper and
> > > actual matching for legacy devices is being done in legacy code, see
> > > hda_codec_match. This match function expects the hdac_device to be
> > > wrapped in legacy hda_codec, which we don't need here.
> 
> > > So for ASoC we are embedding hdac_device in soc_hda_codec and using
> > > the vendor_id and revision_id to match, so hda to write a new one.
> 
> You keep on describing what the code does; I can mostly see what the
> code does (and could probably figure out extra bits if I wasn't getting
> scared off by what I do see) - that's basically what I'm flagging as an
> issue.  What I'm having trouble seeing is why this makes sense.  The
> question isn't what, it's why.

OK, let's see below.

> > > I do not mind if we commonize this and have common match function in
> > > hdac, if legacy is happy with it. Or perhaps the move to core later on
> > > as ASoC HDA matures, either way whatever you n Takashi agree with
> > > would be okay by me.
> 
> > This is the next step.  It would need a fairly big amount of rewrite
> > in each legacy HDA codec driver, and I don't want it for 4.2.
> 
> So how does this actually work then?  If we need to rework all the HDA
> CODEC drivers before they work with this new HDA bus (or at least this
> new HDA bus when used with controllers that implement a custom match
> function) then does it make sense to start trying to use it now?
> 
> I would have expected this to be the /first/ step - refactor the
> existing HDA code, then add new users.

I don't agree with it.

> > Once when we get the common criteria for matching (both for asoc and
> > legacy), we can move to the unified match method.
> 
> I really don't understand this idea that we need to work out what the
> common criteria for matching are, HDA is an enumerable bus isn't it?

The common match method *does* exist.  It's just shifted to two parts,
one for ASoC and one for legacy, because of two different objects.

> > That said, the reason for individual match mechanism is not to break
> > the legacy hda code.  If anyone can provide a patch to achieve that
> > within 100 LOCs and without regression, I'd happily take it :)
> 
> Another way of looking at this is to ask why the CODEC driver matching
> can't continue to work the way it currently does - we're transitioning
> to a bus (which makes sense as a cleanup) but I've not seen any reason
> articulated for tying that up the merge of the Sky Lake code and it
> seems like we're not very near being in a position to be able to do
> that.  Or could we just keep the bigger hda_codec struct around in the
> bus code for now even if it winds up being bloated?

The most of needed changes are not about hda_bus stuff.  This has been
already done for 4.2, in for-next branch.  The rest works for HDA
legacy codes are rather self-contained, further refactoring of the
legacy code itself.  This refactoring has little merit from
functionality POV; the move to hda_bus was already finished, and user
would see no difference at all.  The resultant code wouldn't be much
better from readability POV, rather the size would bloat.  (Remember
that standardization means nothing but limited customization.)

One of few merits would a possible common match method in hda/bus, but
that's all.  So it's prioritized low for now.

> Like I say it's all the why questions that aren't making sense to me
> beyond the "we're too scared to touch the existing code" ones.

Avoiding a regression is the very first priority, indeed, for a driver
like HDA, which is over 10 years old, fat, complex for literally
thousands of various devices, and still with millions of users and

> I'm also wondering how this interacts with the Tegra HDA device by the
> way...

It's a part of HDA legacy.  No difference from any other HDA devices.
What's the problem with Tegra?


thanks,

Takashi

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Mark Brown]

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

On Tue, May 26, 2015 at 07:24:03AM +0200, Takashi Iwai wrote:
> Mark Brown wrote:
> > On Mon, May 25, 2015 at 01:55:59PM +0200, Takashi Iwai wrote:

> > > Once when we get the common criteria for matching (both for asoc and
> > > legacy), we can move to the unified match method.

> > I really don't understand this idea that we need to work out what the
> > common criteria for matching are, HDA is an enumerable bus isn't it?

> The common match method *does* exist.  It's just shifted to two parts,
> one for ASoC and one for legacy, because of two different objects.

What are these objects supposed to be then and why does the bus for HDA
CODECs know about the differences between them?  They're both called HDA
CODECs which might lead one to think that they are supposed to represent
the same thing.

> > I'm also wondering how this interacts with the Tegra HDA device by the
> > way...

> It's a part of HDA legacy.  No difference from any other HDA devices.
> What's the problem with Tegra?

People keep talking about PCI as being a differentator here, though
apparently the Intel systems are all PCI so perhaps not.

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

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

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Takashi Iwai]

At Tue, 26 May 2015 14:32:56 +0100,
Mark Brown wrote:
> 
> On Tue, May 26, 2015 at 07:24:03AM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> > > On Mon, May 25, 2015 at 01:55:59PM +0200, Takashi Iwai wrote:
> 
> > > > Once when we get the common criteria for matching (both for asoc and
> > > > legacy), we can move to the unified match method.
> 
> > > I really don't understand this idea that we need to work out what the
> > > common criteria for matching are, HDA is an enumerable bus isn't it?
> 
> > The common match method *does* exist.  It's just shifted to two parts,
> > one for ASoC and one for legacy, because of two different objects.
> 
> What are these objects supposed to be then and why does the bus for HDA
> CODECs know about the differences between them?  They're both called HDA
> CODECs which might lead one to think that they are supposed to represent
> the same thing.

Both legacy and ASoC drivers contain own codec drivers, but their
classes inherit the same HD-audio core object class that is managed by
HD-audio core bus.  Meanwhile, since the id table is (still) not
embedded into HD-audio core object, the match method is still placed
in each (asoc and legacy) driver core code.

> > > I'm also wondering how this interacts with the Tegra HDA device by the
> > > way...
> 
> > It's a part of HDA legacy.  No difference from any other HDA devices.
> > What's the problem with Tegra?
> 
> People keep talking about PCI as being a differentator here, though
> apparently the Intel systems are all PCI so perhaps not.

Yeah, Tegra also hits this whole movement.  Or better to say, the
whole HD-audio picture has been rewritten, and the transition about
Tegra was already finished.  For the legacy HDA, the rest are rather
about the codec side.


Takashi

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Mark Brown]

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

On Tue, May 26, 2015 at 03:41:32PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:

> > What are these objects supposed to be then and why does the bus for HDA
> > CODECs know about the differences between them?  They're both called HDA
> > CODECs which might lead one to think that they are supposed to represent
> > the same thing.

> Both legacy and ASoC drivers contain own codec drivers, but their
> classes inherit the same HD-audio core object class that is managed by
> HD-audio core bus.  Meanwhile, since the id table is (still) not
> embedded into HD-audio core object, the match method is still placed
> in each (asoc and legacy) driver core code.

This is still as clear as mud, sorry - why is the ID table not something
that the bus handles?  It seems like a core part of what a bus
abstracts.  Having two completely separate sets of controller and device
drivers that can't interact with each other (which seems like what we're
ending up with here) seems really worrying and I don't think I've seen
any plan for unification either.  It's not just the match function stuff
by the way, some of the later patches also made me wonder about the
mutiple implementations of the bus thing.

I think I'm really not understanding what this bus is supposed to be
doing.  It's possible that there is something I'm missing here (I
imagine there's been some off-list discussions of this) but right now
what I'm seeing is that either the bus is not yet at a point where it
abstracts enough to really be a bus (it's not clear to me how something
would use the bus) or the existing HDA code has been converted to use
the new bus before we're ready to do that.  

What you all seem to be saying is that the existing code for HDA is too
fragile to touch much so we don't want to move much of its functionality
to the new bus yet.  I can believe that but I think I'd be a lot happier
if we were handling that by having the existing HDA code override bus
functionality rather than by implementing key bus functionality in
random places.  It feels like trying to use the bus now is adding
technical debt.

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

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

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Takashi Iwai]

At Tue, 26 May 2015 20:43:16 +0100,
Mark Brown wrote:
> 
> On Tue, May 26, 2015 at 03:41:32PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > What are these objects supposed to be then and why does the bus for HDA
> > > CODECs know about the differences between them?  They're both called HDA
> > > CODECs which might lead one to think that they are supposed to represent
> > > the same thing.
> 
> > Both legacy and ASoC drivers contain own codec drivers, but their
> > classes inherit the same HD-audio core object class that is managed by
> > HD-audio core bus.  Meanwhile, since the id table is (still) not
> > embedded into HD-audio core object, the match method is still placed
> > in each (asoc and legacy) driver core code.
> 
> This is still as clear as mud, sorry - why is the ID table not something
> that the bus handles?  It seems like a core part of what a bus
> abstracts.  Having two completely separate sets of controller and device
> drivers that can't interact with each other (which seems like what we're
> ending up with here) seems really worrying and I don't think I've seen
> any plan for unification either.  It's not just the match function stuff
> by the way, some of the later patches also made me wonder about the
> mutiple implementations of the bus thing.

HDA bus is the place for communication between a device and a
controller.  This is common, no matter whether asoc or legacy.  That
is, an ASoC HDA controller may communicate even with an HDA legacy
codec device, if really wanted.  There is no distinction there.

The fact you might be missing from the beginning of the story is that
we need to implement two different driver sets for ASoC, both for
controller and codec drivers.  They can't be unified with legacy HDA
unless we merge ASoC core codes into ALSA core (e.g. DPCM or DAPM into
ALSA core).  Also ASoC has the different ways of registration, too.

For managing these two sets of stuff better, HDA bus provides the
place for controller/codec communication.  All verbs are executed via
snd_hdac_bus_exec_verb(), basically.  Also, it provides dozens of
helper codes to implement PCM and codec/controller/stream management
codes.  Then both HDA legacy and asoc drivers are built up on them.
This is the current standing point.

The lack of id table in hdac_bus object is just because of the current
form of the legacy HDA *codec* driver.  They aren't translated to a
normal struct device_driver yet.  It's not done because such a
translation is a large amount even though it's done systematically,
and there is always a risk of breakage by that.  But the plan is to
achieve it in 4.3 kernel.  So you can see the unified match method
sooner or later.  No reason to stick with it.


> I think I'm really not understanding what this bus is supposed to be
> doing.  It's possible that there is something I'm missing here (I
> imagine there's been some off-list discussions of this) but right now
> what I'm seeing is that either the bus is not yet at a point where it
> abstracts enough to really be a bus (it's not clear to me how something
> would use the bus) or the existing HDA code has been converted to use
> the new bus before we're ready to do that.  
> 
> What you all seem to be saying is that the existing code for HDA is too
> fragile to touch much so we don't want to move much of its functionality
> to the new bus yet.  I can believe that but I think I'd be a lot happier
> if we were handling that by having the existing HDA code override bus
> functionality rather than by implementing key bus functionality in
> random places.  It feels like trying to use the bus now is adding
> technical debt.

Well, the patchset looks more complicated because lots of codes you're
seeing in this patchset are specific to SKL hardware extension, and it
doesn't (maybe won't) exist in HDA legacy.  For example, patch 3
contains many such codes, which is likely the point where you stopped
reading the rest patches.

If you look through the patches, most of codes are wrappers, just
calling snd_hdac_*().  PCM and PCI driver stuff need more codes than
that because of ASoC-specific data and ops (dai_ops, dai_driver, etc)
and to be an individual PCI driver.  Some codes might be able to be
unified in future, but not everything, as long as they are implemented
as individual drivers.


Takashi

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Mark Brown]

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

On Wed, May 27, 2015 at 08:05:47AM +0200, Takashi Iwai wrote:
> Mark Brown wrote:

> > This is still as clear as mud, sorry - why is the ID table not something
> > that the bus handles?  It seems like a core part of what a bus
> > abstracts.  Having two completely separate sets of controller and device
> > drivers that can't interact with each other (which seems like what we're
> > ending up with here) seems really worrying and I don't think I've seen
> > any plan for unification either.  It's not just the match function stuff
> > by the way, some of the later patches also made me wonder about the
> > mutiple implementations of the bus thing.

> HDA bus is the place for communication between a device and a
> controller.  This is common, no matter whether asoc or legacy.  That
> is, an ASoC HDA controller may communicate even with an HDA legacy
> codec device, if really wanted.  There is no distinction there.

I'm unclear how that would (practically speaking) be accomplished if
they're in two separate worlds for matching.  Come to think of it how
does this work if we have more than one matching system in use - how
does the driver model code cope with that, what happens if one match
function ends up looking at data for another match type?

> The fact you might be missing from the beginning of the story is that
> we need to implement two different driver sets for ASoC, both for
> controller and codec drivers.  They can't be unified with legacy HDA
> unless we merge ASoC core codes into ALSA core (e.g. DPCM or DAPM into
> ALSA core).  Also ASoC has the different ways of registration, too.

Wow, that's not what I'd have expected to be happening at all and really
should have been mentioned at some point.  I need to think about this
further.

What I had expected to see for ASoC/HDA integration was something where
ASoC devices were providing a standard HDA controller and then use the
same CODEC drivers as everything else.  Not doing that means that we
inveitably have some duplication for at least things like CODEC quirks
and device specific support which doesn't seem like the best idea.
There's also userspace interfaces for things like the HDA device graph
that'd need looking at.

Having HDA use ASoC doesn't seem like a terrible idea, there are aspects
of HDA that map quite well onto ASoC, but doesn't seem to be the
intention here and I'm ambivalent about it being worth the effort.  I
don't think it would need us to move ASoC into the core any more than
any other driver, it'd just make it much more widely used.

> > What you all seem to be saying is that the existing code for HDA is too
> > fragile to touch much so we don't want to move much of its functionality
> > to the new bus yet.  I can believe that but I think I'd be a lot happier
> > if we were handling that by having the existing HDA code override bus
> > functionality rather than by implementing key bus functionality in
> > random places.  It feels like trying to use the bus now is adding
> > technical debt.

> Well, the patchset looks more complicated because lots of codes you're
> seeing in this patchset are specific to SKL hardware extension, and it
> doesn't (maybe won't) exist in HDA legacy.  For example, patch 3
> contains many such codes, which is likely the point where you stopped
> reading the rest patches.

I'm too alarmed by what I'm seeing in the apparently generic code to
look at the platform specific code.

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

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

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Takashi Iwai]

At Wed, 27 May 2015 19:34:06 +0100,
Mark Brown wrote:
> 
> On Wed, May 27, 2015 at 08:05:47AM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > This is still as clear as mud, sorry - why is the ID table not something
> > > that the bus handles?  It seems like a core part of what a bus
> > > abstracts.  Having two completely separate sets of controller and device
> > > drivers that can't interact with each other (which seems like what we're
> > > ending up with here) seems really worrying and I don't think I've seen
> > > any plan for unification either.  It's not just the match function stuff
> > > by the way, some of the later patches also made me wonder about the
> > > mutiple implementations of the bus thing.
> 
> > HDA bus is the place for communication between a device and a
> > controller.  This is common, no matter whether asoc or legacy.  That
> > is, an ASoC HDA controller may communicate even with an HDA legacy
> > codec device, if really wanted.  There is no distinction there.
> 
> I'm unclear how that would (practically speaking) be accomplished if
> they're in two separate worlds for matching.  Come to think of it how
> does this work if we have more than one matching system in use - how
> does the driver model code cope with that, what happens if one match
> function ends up looking at data for another match type?

The matching function does the same thing, but it just looks at the
different embedded id tables.  The bus match function already checks
each device/driver type beforehand.

> > The fact you might be missing from the beginning of the story is that
> > we need to implement two different driver sets for ASoC, both for
> > controller and codec drivers.  They can't be unified with legacy HDA
> > unless we merge ASoC core codes into ALSA core (e.g. DPCM or DAPM into
> > ALSA core).  Also ASoC has the different ways of registration, too.
> 
> Wow, that's not what I'd have expected to be happening at all and really
> should have been mentioned at some point.  I need to think about this
> further.
> 
> What I had expected to see for ASoC/HDA integration was something where
> ASoC devices were providing a standard HDA controller and then use the
> same CODEC drivers as everything else.  Not doing that means that we
> inveitably have some duplication for at least things like CODEC quirks
> and device specific support which doesn't seem like the best idea.
> There's also userspace interfaces for things like the HDA device graph
> that'd need looking at.

Actually that's the exact plan -- the codec graph will be parsed in
user-space (or pre-parsed) and mapped via DFW.

The quirks are still open questions.  Currently, ASoC HDA is targeted
only to the new systems with SKL.  The rest will keep supported by the
existing driver.  So, we expect that the amount of quirks can be
reduced much (starting from zero).

> Having HDA use ASoC doesn't seem like a terrible idea, there are aspects
> of HDA that map quite well onto ASoC, but doesn't seem to be the
> intention here and I'm ambivalent about it being worth the effort.  I
> don't think it would need us to move ASoC into the core any more than
> any other driver, it'd just make it much more widely used.

Heh, that's the very reason why we started implementing a different
driver set, after all.  The new driver code is written in minimalistic
manner to fit better with ASoC.  The heavy part has been already
shifted into HDA core side, that is basically a stripped version from
the old HDA code.

ASoC implementation was requested rather due to DPCM, compress
support, etc, which is currently ASoC-specific features.  The current
patchset we've seen is really a tip of iceberg...

> > > What you all seem to be saying is that the existing code for HDA is too
> > > fragile to touch much so we don't want to move much of its functionality
> > > to the new bus yet.  I can believe that but I think I'd be a lot happier
> > > if we were handling that by having the existing HDA code override bus
> > > functionality rather than by implementing key bus functionality in
> > > random places.  It feels like trying to use the bus now is adding
> > > technical debt.
> 
> > Well, the patchset looks more complicated because lots of codes you're
> > seeing in this patchset are specific to SKL hardware extension, and it
> > doesn't (maybe won't) exist in HDA legacy.  For example, patch 3
> > contains many such codes, which is likely the point where you stopped
> > reading the rest patches.
> 
> I'm too alarmed by what I'm seeing in the apparently generic code to
> look at the platform specific code.

Yeah, it's a bit unfortunate.  Maybe it'd have been easier if they
were split better.


Takashi

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Mark Brown]

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

On Wed, May 27, 2015 at 09:17:27PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:

> > I'm unclear how that would (practically speaking) be accomplished if
> > they're in two separate worlds for matching.  Come to think of it how
> > does this work if we have more than one matching system in use - how
> > does the driver model code cope with that, what happens if one match
> > function ends up looking at data for another match type?

> The matching function does the same thing, but it just looks at the
> different embedded id tables.  The bus match function already checks
> each device/driver type beforehand.

How do we safely do the looking at different tables bit though?  I
should probably go back and look again at the original patch...

> > > The fact you might be missing from the beginning of the story is that
> > > we need to implement two different driver sets for ASoC, both for
> > > controller and codec drivers.  They can't be unified with legacy HDA
> > > unless we merge ASoC core codes into ALSA core (e.g. DPCM or DAPM into
> > > ALSA core).  Also ASoC has the different ways of registration, too.
> > 
> > Wow, that's not what I'd have expected to be happening at all and really
> > should have been mentioned at some point.  I need to think about this
> > further.

> > What I had expected to see for ASoC/HDA integration was something where
> > ASoC devices were providing a standard HDA controller and then use the
> > same CODEC drivers as everything else.  Not doing that means that we
> > inveitably have some duplication for at least things like CODEC quirks
> > and device specific support which doesn't seem like the best idea.
> > There's also userspace interfaces for things like the HDA device graph
> > that'd need looking at.

> Actually that's the exact plan -- the codec graph will be parsed in
> user-space (or pre-parsed) and mapped via DFW.

OK, more surprises!

Looking at the Broadwell based I2S systems that don't work with any
released set of upstream software (they need alsa-lib from git) I'm
wondering how close these systems are to getting into the hands of users
and what the transition from existing HDA to userspace parsing HDA is
going to look like.

I guess older distros will just bind the existing HDA controller drivers
to them and use the hardware without any of the shiny new features which
ought to be fine and I'm sure that once everyone has got the new stuff
it will all be glorious, it's the space where people are upgrading
(possibly upgrading the kernel or userspace separately) that worries me.
Especially the bit where people ask me to make their sound work when
they see their system uses ASoC!

> The quirks are still open questions.  Currently, ASoC HDA is targeted
> only to the new systems with SKL.  The rest will keep supported by the
> existing driver.  So, we expect that the amount of quirks can be
> reduced much (starting from zero).

I'm sure that our hardware engineering colleagues wouldn't want us to be
bored and will give us something to do there :)

> > Having HDA use ASoC doesn't seem like a terrible idea, there are aspects
> > of HDA that map quite well onto ASoC, but doesn't seem to be the
> > intention here and I'm ambivalent about it being worth the effort.  I
> > don't think it would need us to move ASoC into the core any more than
> > any other driver, it'd just make it much more widely used.

> Heh, that's the very reason why we started implementing a different
> driver set, after all.  The new driver code is written in minimalistic
> manner to fit better with ASoC.  The heavy part has been already
> shifted into HDA core side, that is basically a stripped version from
> the old HDA code.

> ASoC implementation was requested rather due to DPCM, compress
> support, etc, which is currently ASoC-specific features.  The current
> patchset we've seen is really a tip of iceberg...

Right, and I'm aware of the reuse potential with sharing features with
the embedded side (like we're already seeing with the I2S Broadwells) so
if we don't integrate the two somehow we end up with duplicated drivers
which would be sad.  It's just that I'd anticipated that this would be
being handled by black boxing the HDA part of the system from the ASoC
point of view.

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

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

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Takashi Iwai]

At Thu, 28 May 2015 20:53:59 +0100,
Mark Brown wrote:
> 
> On Wed, May 27, 2015 at 09:17:27PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > I'm unclear how that would (practically speaking) be accomplished if
> > > they're in two separate worlds for matching.  Come to think of it how
> > > does this work if we have more than one matching system in use - how
> > > does the driver model code cope with that, what happens if one match
> > > function ends up looking at data for another match type?
> 
> > The matching function does the same thing, but it just looks at the
> > different embedded id tables.  The bus match function already checks
> > each device/driver type beforehand.
> 
> How do we safely do the looking at different tables bit though?  I
> should probably go back and look again at the original patch...

There is the device type check in hda_bus_match() before calling
hda_driver->match(), so it must be safe.  But as already mentioned,
the match isn't the thing to worry too much, as it'll be unified
sooner or later.  The rest (still unseen) stuff is more worrisome.

> > > > The fact you might be missing from the beginning of the story is that
> > > > we need to implement two different driver sets for ASoC, both for
> > > > controller and codec drivers.  They can't be unified with legacy HDA
> > > > unless we merge ASoC core codes into ALSA core (e.g. DPCM or DAPM into
> > > > ALSA core).  Also ASoC has the different ways of registration, too.
> > > 
> > > Wow, that's not what I'd have expected to be happening at all and really
> > > should have been mentioned at some point.  I need to think about this
> > > further.
> 
> > > What I had expected to see for ASoC/HDA integration was something where
> > > ASoC devices were providing a standard HDA controller and then use the
> > > same CODEC drivers as everything else.  Not doing that means that we
> > > inveitably have some duplication for at least things like CODEC quirks
> > > and device specific support which doesn't seem like the best idea.
> > > There's also userspace interfaces for things like the HDA device graph
> > > that'd need looking at.
> 
> > Actually that's the exact plan -- the codec graph will be parsed in
> > user-space (or pre-parsed) and mapped via DFW.
> 
> OK, more surprises!
> 
> Looking at the Broadwell based I2S systems that don't work with any
> released set of upstream software (they need alsa-lib from git) I'm
> wondering how close these systems are to getting into the hands of users
> and what the transition from existing HDA to userspace parsing HDA is
> going to look like.
> 
> I guess older distros will just bind the existing HDA controller drivers
> to them and use the hardware without any of the shiny new features which
> ought to be fine and I'm sure that once everyone has got the new stuff
> it will all be glorious, it's the space where people are upgrading
> (possibly upgrading the kernel or userspace separately) that worries me.
> Especially the bit where people ask me to make their sound work when
> they see their system uses ASoC!

Right, the time is still a question.  Vinod and Liam can answer at
best.

And it's the reason I suggested for pending merge until the codec side
is ready.  We need the topology stuff merged at first, then HDA-codec
driver together with user-space side.

> > The quirks are still open questions.  Currently, ASoC HDA is targeted
> > only to the new systems with SKL.  The rest will keep supported by the
> > existing driver.  So, we expect that the amount of quirks can be
> > reduced much (starting from zero).
> 
> I'm sure that our hardware engineering colleagues wouldn't want us to be
> bored and will give us something to do there :)

I have no doubt about it, too :)

> > > Having HDA use ASoC doesn't seem like a terrible idea, there are aspects
> > > of HDA that map quite well onto ASoC, but doesn't seem to be the
> > > intention here and I'm ambivalent about it being worth the effort.  I
> > > don't think it would need us to move ASoC into the core any more than
> > > any other driver, it'd just make it much more widely used.
> 
> > Heh, that's the very reason why we started implementing a different
> > driver set, after all.  The new driver code is written in minimalistic
> > manner to fit better with ASoC.  The heavy part has been already
> > shifted into HDA core side, that is basically a stripped version from
> > the old HDA code.
> 
> > ASoC implementation was requested rather due to DPCM, compress
> > support, etc, which is currently ASoC-specific features.  The current
> > patchset we've seen is really a tip of iceberg...
> 
> Right, and I'm aware of the reuse potential with sharing features with
> the embedded side (like we're already seeing with the I2S Broadwells) so
> if we don't integrate the two somehow we end up with duplicated drivers
> which would be sad.  It's just that I'd anticipated that this would be
> being handled by black boxing the HDA part of the system from the ASoC
> point of view.

Yes, blackboxing the existing HDA was the first plan.  But it has
obvious drawbacks when joined to ASoC (inheriting a huge amount of
quirks, different designs; HDA legacy is very self-contained, does
DAPM-like path controls and jack-retasking by itself, etc).

Potentially it's still possible to implement in that way, as a plan B,
if a cleaner implementation from scratch fails.  Let's see...


Takashi

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Vinod Koul]

On Fri, May 29, 2015 at 06:58:24AM +0200, Takashi Iwai wrote:
> At Thu, 28 May 2015 20:53:59 +0100,
> Mark Brown wrote:
> > 
> > On Wed, May 27, 2015 at 09:17:27PM +0200, Takashi Iwai wrote:
> > > Mark Brown wrote:
> > 
> > > > I'm unclear how that would (practically speaking) be accomplished if
> > > > they're in two separate worlds for matching.  Come to think of it how
> > > > does this work if we have more than one matching system in use - how
> > > > does the driver model code cope with that, what happens if one match
> > > > function ends up looking at data for another match type?
> > 
> > > The matching function does the same thing, but it just looks at the
> > > different embedded id tables.  The bus match function already checks
> > > each device/driver type beforehand.
> > 
> > How do we safely do the looking at different tables bit though?  I
> > should probably go back and look again at the original patch...
> 
> There is the device type check in hda_bus_match() before calling
> hda_driver->match(), so it must be safe.  But as already mentioned,
> the match isn't the thing to worry too much, as it'll be unified
> sooner or later.  The rest (still unseen) stuff is more worrisome.
> 
> > > > > The fact you might be missing from the beginning of the story is that
> > > > > we need to implement two different driver sets for ASoC, both for
> > > > > controller and codec drivers.  They can't be unified with legacy HDA
> > > > > unless we merge ASoC core codes into ALSA core (e.g. DPCM or DAPM into
> > > > > ALSA core).  Also ASoC has the different ways of registration, too.
> > > > 
> > > > Wow, that's not what I'd have expected to be happening at all and really
> > > > should have been mentioned at some point.  I need to think about this
> > > > further.
> > 
> > > > What I had expected to see for ASoC/HDA integration was something where
> > > > ASoC devices were providing a standard HDA controller and then use the
> > > > same CODEC drivers as everything else.  Not doing that means that we
> > > > inveitably have some duplication for at least things like CODEC quirks
> > > > and device specific support which doesn't seem like the best idea.
> > > > There's also userspace interfaces for things like the HDA device graph
> > > > that'd need looking at.
> > 
> > > Actually that's the exact plan -- the codec graph will be parsed in
> > > user-space (or pre-parsed) and mapped via DFW.
> > 
> > OK, more surprises!
> > 
> > Looking at the Broadwell based I2S systems that don't work with any
> > released set of upstream software (they need alsa-lib from git) I'm
> > wondering how close these systems are to getting into the hands of users
> > and what the transition from existing HDA to userspace parsing HDA is
> > going to look like.
well userspace parsing is only for the pin mapping and changes which
users want to do on their systems. For basic things the default graph
will be read by existing HDA parser and codec driver will create
widgets based on that. The generic HDA codec library should do most and
the HDA ASoC codecs should do device specfic work (like existing
patch_xxx)

Yes I agree it will take a while before all things fall into place and "Just
work".

> > I guess older distros will just bind the existing HDA controller drivers
> > to them and use the hardware without any of the shiny new features which
> > ought to be fine and I'm sure that once everyone has got the new stuff
> > it will all be glorious, it's the space where people are upgrading
> > (possibly upgrading the kernel or userspace separately) that worries me.
> > Especially the bit where people ask me to make their sound work when
> > they see their system uses ASoC!
For HDA based systems, unless someone really wants ASoC features we
should not enable them yet. I2S systems will be using ASoC. I expect
the next generation to be mature enough for us to make HDA systems
also use ASoC :) by default

> 
> Right, the time is still a question.  Vinod and Liam can answer at
> best.
For BDW, Liam is the best guy. I will be supporting platforms from SKL
onwards, so feel free to redirect them to me.

> 
> And it's the reason I suggested for pending merge until the codec side
> is ready.  We need the topology stuff merged at first, then HDA-codec
> driver together with user-space side.
I dont mind if code doesnt get merged into next merge window and put
in one after that. But in needs to get accepted is some topic branch
so that subsequent series can be  psted and reviewed, merged. Btw
users will not see unless we have machine driver. I will add the
disable feature after this series, so no user conflicts occur

> 
> > > The quirks are still open questions.  Currently, ASoC HDA is targeted
> > > only to the new systems with SKL.  The rest will keep supported by the
> > > existing driver.  So, we expect that the amount of quirks can be
> > > reduced much (starting from zero).
> > 
> > I'm sure that our hardware engineering colleagues wouldn't want us to be
> > bored and will give us something to do there :)
> 
> I have no doubt about it, too :)
> 
> > > > Having HDA use ASoC doesn't seem like a terrible idea, there are aspects
> > > > of HDA that map quite well onto ASoC, but doesn't seem to be the
> > > > intention here and I'm ambivalent about it being worth the effort.  I
> > > > don't think it would need us to move ASoC into the core any more than
> > > > any other driver, it'd just make it much more widely used.
> > 
> > > Heh, that's the very reason why we started implementing a different
> > > driver set, after all.  The new driver code is written in minimalistic
> > > manner to fit better with ASoC.  The heavy part has been already
> > > shifted into HDA core side, that is basically a stripped version from
> > > the old HDA code.
> > 
> > > ASoC implementation was requested rather due to DPCM, compress
> > > support, etc, which is currently ASoC-specific features.  The current
> > > patchset we've seen is really a tip of iceberg...
> > 
> > Right, and I'm aware of the reuse potential with sharing features with
> > the embedded side (like we're already seeing with the I2S Broadwells) so
> > if we don't integrate the two somehow we end up with duplicated drivers
> > which would be sad.  It's just that I'd anticipated that this would be
> > being handled by black boxing the HDA part of the system from the ASoC
> > point of view.
For us, the main issue was managing DSP along with HDA. The SKL DSP
requires us to instantiate DSP toplogy and manage them, without DAPM n
DPCM this was unthinkable. So we leaned on bringing HDA into ASoC. To
use HDA controller the core HDA is in place now

Codecs are different beast here, esp HDA issue of pin mapping and jack
retasking. So idea is to have codec generic parser which creates ASOC
widgets based on parsing and actual codec driver use this or load
their own quirks. For pin mapping, machines can override configuration
using toplogy and change a system to 7.1 audio or three stereo outputs
or 2 stereo audio and 1 mic,. Using toplogy they can write and load
their own configuration, thus moving these configuration out of kernel
and getting us rid of platform hacks


> 
> Yes, blackboxing the existing HDA was the first plan.  But it has
> obvious drawbacks when joined to ASoC (inheriting a huge amount of
> quirks, different designs; HDA legacy is very self-contained, does
> DAPM-like path controls and jack-retasking by itself, etc).
> 
> Potentially it's still possible to implement in that way, as a plan B,
> if a cleaner implementation from scratch fails.  Let's see...
Yes I would still think our plan A is a good one. I dont expect us to
solve all probelms right away but by the time we meet again for u-Conf
we shoudl have dealt with most and can discuss remaining ones.
Pls do add this as topic :)

So Mark, Takashi,

Now that we have debated this and I guess all have the big picture
here, how do we go about this. Do you guys need changes in this series
or this is good for now?

-- 
~Vinod

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Mark Brown]

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

On Fri, May 29, 2015 at 01:45:44PM +0530, Vinod Koul wrote:

> well userspace parsing is only for the pin mapping and changes which
> users want to do on their systems. For basic things the default graph
> will be read by existing HDA parser and codec driver will create
> widgets based on that. The generic HDA codec library should do most and
> the HDA ASoC codecs should do device specfic work (like existing
> patch_xxx)

So we *are* planning to continue to use the existing CODEC support but
with some different interfacing on top of it?

> > And it's the reason I suggested for pending merge until the codec side
> > is ready.  We need the topology stuff merged at first, then HDA-codec
> > driver together with user-space side.

> I dont mind if code doesnt get merged into next merge window and put
> in one after that. But in needs to get accepted is some topic branch
> so that subsequent series can be  psted and reviewed, merged. Btw
> users will not see unless we have machine driver. I will add the
> disable feature after this series, so no user conflicts occur

I need to have another look through.  TBH I suspect it would be easier
with something that was more of a minimum viable prototype than with a
part of the system - with something like this it's as much the overall
shape of the solution that's being reviewed as anything else.

> Now that we have debated this and I guess all have the big picture
> here, how do we go about this. Do you guys need changes in this series
> or this is good for now?

Like I say I really need to review it again, and I'm still not entirely
convinced I understand what's going on well enough.

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

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

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Vinod Koul]

On Fri, May 29, 2015 at 06:35:03PM +0100, Mark Brown wrote:
> On Fri, May 29, 2015 at 01:45:44PM +0530, Vinod Koul wrote:
> 
> > well userspace parsing is only for the pin mapping and changes which
> > users want to do on their systems. For basic things the default graph
> > will be read by existing HDA parser and codec driver will create
> > widgets based on that. The generic HDA codec library should do most and
> > the HDA ASoC codecs should do device specfic work (like existing
> > patch_xxx)
> 
> So we *are* planning to continue to use the existing CODEC support but
> with some different interfacing on top of it?
Nope. Existing codecs wont be reused here.

We are doing new ASoC HDA codec drivers which use common hdac parsing logic.
Since some codecs need own methods we can't write a single HDA generic
codec, and had to do with generic codec parser and then specfic driver
(HDMI, ALC to start with) on top.

> 
> > > And it's the reason I suggested for pending merge until the codec side
> > > is ready.  We need the topology stuff merged at first, then HDA-codec
> > > driver together with user-space side.
> 
> > I dont mind if code doesnt get merged into next merge window and put
> > in one after that. But in needs to get accepted is some topic branch
> > so that subsequent series can be  psted and reviewed, merged. Btw
> > users will not see unless we have machine driver. I will add the
> > disable feature after this series, so no user conflicts occur
> 
> I need to have another look through.  TBH I suspect it would be easier
> with something that was more of a minimum viable prototype than with a
> part of the system - with something like this it's as much the overall
> shape of the solution that's being reviewed as anything else.
Sure, please do let us know if you have more questions on this and overall
approach.

-- 
~Vinod

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Mark Brown]

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

On Mon, Jun 01, 2015 at 10:35:04AM +0530, Vinod Koul wrote:
> On Fri, May 29, 2015 at 06:35:03PM +0100, Mark Brown wrote:
> > On Fri, May 29, 2015 at 01:45:44PM +0530, Vinod Koul wrote:

> > > well userspace parsing is only for the pin mapping and changes which
> > > users want to do on their systems. For basic things the default graph
> > > will be read by existing HDA parser and codec driver will create
> > > widgets based on that. The generic HDA codec library should do most and
> > > the HDA ASoC codecs should do device specfic work (like existing
> > > patch_xxx)

> > So we *are* planning to continue to use the existing CODEC support but
> > with some different interfacing on top of it?

> Nope. Existing codecs wont be reused here.

> We are doing new ASoC HDA codec drivers which use common hdac parsing logic.
> Since some codecs need own methods we can't write a single HDA generic
> codec, and had to do with generic codec parser and then specfic driver
> (HDMI, ALC to start with) on top.

But I thought we already had a generic CODEC parser?  Are you saying
you're going to write a new one?

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

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

Re: [PATCH v4 1/7] ASoC: hda - add ASoC HDA codec match function

[Vinod Koul]

On Tue, Jun 02, 2015 at 11:38:43AM +0100, Mark Brown wrote:
> On Mon, Jun 01, 2015 at 10:35:04AM +0530, Vinod Koul wrote:
> > On Fri, May 29, 2015 at 06:35:03PM +0100, Mark Brown wrote:
> > > On Fri, May 29, 2015 at 01:45:44PM +0530, Vinod Koul wrote:
> 
> > > > well userspace parsing is only for the pin mapping and changes which
> > > > users want to do on their systems. For basic things the default graph
> > > > will be read by existing HDA parser and codec driver will create
> > > > widgets based on that. The generic HDA codec library should do most and
> > > > the HDA ASoC codecs should do device specfic work (like existing
> > > > patch_xxx)
> 
> > > So we *are* planning to continue to use the existing CODEC support but
> > > with some different interfacing on top of it?
> 
> > Nope. Existing codecs wont be reused here.
> 
> > We are doing new ASoC HDA codec drivers which use common hdac parsing logic.
> > Since some codecs need own methods we can't write a single HDA generic
> > codec, and had to do with generic codec parser and then specfic driver
> > (HDMI, ALC to start with) on top.
> 
> But I thought we already had a generic CODEC parser?  Are you saying
> you're going to write a new one?
No I meant we are going to use shiny new HDAC codec parser and build ASoC
HDA codecs which use this new parser.

-- 
~Vinod

[PATCH v4 2/7] ALSA: hda - add new HDA registers

[Vinod Koul]

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

This patch adds new registers as per HD audio Spec like capability registers
for processing pipe, software position based FIFO, Multiple Links and Global
Time Synchronization.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Acked-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 include/sound/hda_register.h | 88 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h
index 4f6d3fce6ee6..b213d3155be1 100644
--- a/include/sound/hda_register.h
+++ b/include/sound/hda_register.h
@@ -28,6 +28,10 @@
 #define AZX_REG_STATESTS		0x0e
 #define AZX_REG_GSTS			0x10
 #define   AZX_GSTS_FSTS		(1 << 1)   /* flush status */
+#define AZX_REG_GCAP2			0x12
+#define AZX_REG_LLCH			0x14
+#define AZX_REG_OUTSTRMPAY		0x18
+#define AZX_REG_INSTRMPAY		0x1A
 #define AZX_REG_INTCTL			0x20
 #define AZX_REG_INTSTS			0x24
 #define AZX_REG_WALLCLK			0x30	/* 24Mhz source */
@@ -81,6 +85,7 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
 #define AZX_REG_SD_FIFOW		0x0e
 #define AZX_REG_SD_FIFOSIZE		0x10
 #define AZX_REG_SD_FORMAT		0x12
+#define AZX_REG_SD_FIFOL		0x14
 #define AZX_REG_SD_BDLPL		0x18
 #define AZX_REG_SD_BDLPU		0x1c
 
@@ -134,6 +139,89 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
 #define AZX_MAX_CORB_ENTRIES	256
 #define AZX_MAX_RIRB_ENTRIES	256
 
+/* Capability header  Structure */
+#define AZX_REG_CAP_HDR			0x0
+#define AZX_CAP_HDR_VER_OFF		28
+#define AZX_CAP_HDR_VER_MASK		(0xF << AZX_CAP_HDR_VER_OFF)
+#define AZX_CAP_HDR_ID_OFF		16
+#define AZX_CAP_HDR_ID_MASK		(0xFFF << AZX_CAP_HDR_ID_OFF)
+#define AZX_CAP_HDR_NXT_PTR_MASK	0xFFFF
+
+/* registers of Software Position Based FIFO Capability Structure */
+#define AZX_SPB_CAP_ID			0x4
+#define AZX_REG_SPB_BASE_ADDR		0x700
+#define AZX_REG_SPB_SPBFCH		0x00
+#define AZX_REG_SPB_SPBFCCTL		0x04
+/* Base used to calculate the iterating register offset */
+#define AZX_SPB_BASE			0x08
+/* Interval used to calculate the iterating register offset */
+#define AZX_SPB_INTERVAL		0x08
+
+/* registers of Global Time Synchronization Capability Structure */
+#define AZX_GTS_CAP_ID			0x1
+#define AZX_REG_GTS_GTSCH		0x00
+#define AZX_REG_GTS_GTSCD		0x04
+#define AZX_REG_GTS_GTSCTLAC		0x0C
+#define AZX_GTS_BASE			0x20
+#define AZX_GTS_INTERVAL		0x20
+
+/* registers for Processing Pipe Capability Structure */
+#define AZX_PP_CAP_ID			0x3
+#define AZX_REG_PP_PPCH			0x10
+#define AZX_REG_PP_PPCTL		0x04
+#define AZX_PPCTL_PIE			(1<<31)
+#define AZX_PPCTL_GPROCEN		(1<<30)
+/* _X_ = dma engine # and cannot * exceed 29 (per spec max 30 dma engines) */
+#define AZX_PPCTL_PROCEN(_X_)		(1<<(_X_))
+
+#define AZX_REG_PP_PPSTS		0x08
+
+#define AZX_PPHC_BASE			0x10
+#define AZX_PPHC_INTERVAL		0x10
+
+#define AZX_REG_PPHCLLPL		0x0
+#define AZX_REG_PPHCLLPU		0x4
+#define AZX_REG_PPHCLDPL		0x8
+#define AZX_REG_PPHCLDPU		0xC
+
+#define AZX_PPLC_BASE			0x10
+#define AZX_PPLC_MULTI			0x10
+#define AZX_PPLC_INTERVAL		0x10
+
+#define AZX_REG_PPLCCTL			0x0
+#define AZX_PPLCCTL_STRM_BITS		4
+#define AZX_PPLCCTL_STRM_SHIFT		20
+#define AZX_REG_MASK(bit_num, offset) \
+	(((1 << (bit_num)) - 1) << (offset))
+#define AZX_PPLCCTL_STRM_MASK \
+	AZX_REG_MASK(AZX_PPLCCTL_STRM_BITS, AZX_PPLCCTL_STRM_SHIFT)
+#define AZX_PPLCCTL_RUN			(1<<1)
+#define AZX_PPLCCTL_STRST		(1<<0)
+
+#define AZX_REG_PPLCFMT			0x4
+#define AZX_REG_PPLCLLPL		0x8
+#define AZX_REG_PPLCLLPU		0xC
+
+/* registers for Multiple Links Capability Structure */
+#define AZX_ML_CAP_ID			0x2
+#define AZX_REG_ML_MLCH			0x00
+#define AZX_REG_ML_MLCD			0x04
+#define AZX_ML_BASE			0x40
+#define AZX_ML_INTERVAL			0x40
+
+#define AZX_REG_ML_LCAP			0x00
+#define AZX_REG_ML_LCTL			0x04
+#define AZX_REG_ML_LOSIDV		0x08
+#define AZX_REG_ML_LSDIID		0x0C
+#define AZX_REG_ML_LPSOO		0x10
+#define AZX_REG_ML_LPSIO		0x12
+#define AZX_REG_ML_LWALFC		0x18
+#define AZX_REG_ML_LOUTPAY		0x20
+#define AZX_REG_ML_LINPAY		0x30
+
+#define AZX_MLCTL_SPA			(1<<16)
+#define AZX_MLCTL_CPA			23
+
 /*
  * helpers to read the stream position
  */
-- 
1.9.1

Re: [PATCH v4 2/7] ALSA: hda - add new HDA registers

[Mark Brown]

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

On Mon, May 11, 2015 at 04:24:00PM +0530, Vinod Koul wrote:
> From: Jeeja KP <jeeja.kp@intel.com>
> 
> This patch adds new registers as per HD audio Spec like capability registers
> for processing pipe, software position based FIFO, Multiple Links and Global
> Time Synchronization.

This appears to have some dependency on Takashi's tree - Takashi, is
there a tag I can pull?

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

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

Re: [PATCH v4 2/7] ALSA: hda - add new HDA registers

[Takashi Iwai]

At Fri, 22 May 2015 13:58:17 +0100,
Mark Brown wrote:
> 
> On Mon, May 11, 2015 at 04:24:00PM +0530, Vinod Koul wrote:
> > From: Jeeja KP <jeeja.kp@intel.com>
> > 
> > This patch adds new registers as per HD audio Spec like capability registers
> > for processing pipe, software position based FIFO, Multiple Links and Global
> > Time Synchronization.
> 
> This appears to have some dependency on Takashi's tree - Takashi, is
> there a tag I can pull?

You can just take the current for-next branch of my git tree.
The topic/hda branch would work, too, at least for building.


Takashi

[PATCH v4 3/7] ASoC: hda - add asoc hda core bus, controller and stream helpers

[Vinod Koul]

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

This patch adds the asoc hdac bus with it initialization and cleanup routines
then we add hdac controller helpers, the new capability parsing and
initialization routines, and link configuration routines
Lastly, we add the hdac stream helpers, the new controller initialization
routines, stream management helpers.

These helpers are added on top of hdac_xxx helpers and the structures
introduced here; soc_hda_bus embeds hdac-bus, soc_hdac_stream embeds
hdac_stream with additional code for additional HDA capabilities.

These will be sued by ASoC HDA controllers like SKL which exhibit these
capabilities

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 include/sound/soc-hdaudio.h         | 360 +++++++++++++++++++++++++++++++
 sound/soc/hda/Makefile              |   3 +-
 sound/soc/hda/soc-hdac-bus.c        | 115 ++++++++++
 sound/soc/hda/soc-hdac-controller.c | 296 ++++++++++++++++++++++++++
 sound/soc/hda/soc-hdac-stream.c     | 409 ++++++++++++++++++++++++++++++++++++
 5 files changed, 1182 insertions(+), 1 deletion(-)
 create mode 100644 include/sound/soc-hdaudio.h
 create mode 100644 sound/soc/hda/soc-hdac-bus.c
 create mode 100644 sound/soc/hda/soc-hdac-controller.c
 create mode 100644 sound/soc/hda/soc-hdac-stream.c

diff --git a/include/sound/soc-hdaudio.h b/include/sound/soc-hdaudio.h
new file mode 100644
index 000000000000..477cd09121a4
--- /dev/null
+++ b/include/sound/soc-hdaudio.h
@@ -0,0 +1,360 @@
+#ifndef __SOUND_SOC_HDAUDIO_H
+#define __SOUND_SOC_HDAUDIO_H
+
+#include <sound/core.h>
+#include <sound/initval.h>
+#include <sound/hda_register.h>
+#include <sound/hdaudio.h>
+
+struct soc_hdac_bus {
+	struct hdac_bus bus;
+	int num_streams;
+
+	bool ppcap:1;
+	bool spbcap:1;
+	bool mlcap:1;
+	bool gtscap:1;
+
+	void __iomem *ppcap_addr;
+	void __iomem *spbcap_addr;
+	void __iomem *mlcap_addr;
+	void __iomem *gtscap_addr;
+
+	/* linked list of hda links */
+	struct list_head hlink_list;
+};
+
+int snd_soc_hdac_bus_init(struct soc_hdac_bus *sbus, struct device *dev,
+		      const struct hdac_bus_ops *ops,
+		      const struct hdac_io_ops *io_ops);
+
+void snd_soc_hdac_bus_exit(struct soc_hdac_bus *sbus);
+int snd_soc_hdac_bus_device_init(struct soc_hdac_bus *sbus, int addr);
+void snd_soc_hdac_bus_device_exit(struct hdac_device *hdev);
+
+#define hdac_bus(sbus)	(&(sbus)->bus)
+#define bus_to_soc_hdac_bus(_bus) \
+	container_of(_bus, struct soc_hdac_bus, bus)
+
+int snd_soc_hdac_bus_parse_capabilities(struct soc_hdac_bus *sbus);
+void snd_soc_hdac_bus_ppcap_enable(struct soc_hdac_bus *chip, bool enable);
+void snd_soc_hdac_bus_ppcap_int_enable(struct soc_hdac_bus *chip, bool enable);
+
+/*
+ * macros for ppcap register read/write
+ */
+#define _soc_hdac_bus_ppcap_write(type, dev, reg, value)		\
+	((dev)->bus.io_ops->reg_write ## type(value, (dev)->ppcap_addr + (reg)))
+#define _soc_hdac_bus_ppcap_read(type, dev, reg)			\
+	((dev)->bus.io_ops->reg_read ## type((dev)->ppcap_addr + (reg)))
+
+/* read/write a register, pass without AZX_REG_ prefix */
+#define soc_hdac_bus_ppcap_writel(dev, reg, value) \
+	_soc_hdac_bus_ppcap_write(l, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_bus_ppcap_writew(dev, reg, value) \
+	_soc_hdac_bus_ppcap_write(w, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_bus_ppcap_writeb(dev, reg, value) \
+	_soc_hdac_bus_ppcap_write(b, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_bus_ppcap_readl(dev, reg) \
+	_soc_hdac_bus_ppcap_read(l, dev, AZX_REG_ ## reg)
+#define soc_hdac_bus_ppcap_readw(dev, reg) \
+	_soc_hdac_bus_ppcap_read(w, dev, AZX_REG_ ## reg)
+#define soc_hdac_bus_ppcap_readb(dev, reg) \
+	_soc_hdac_bus_ppcap_read(b, dev, AZX_REG_ ## reg)
+
+/* update a register, pass without AZX_REG_ prefix */
+#define soc_hdac_bus_ppcap_updatel(dev, reg, mask, val) \
+	soc_hdac_bus_ppcap_writel(dev, reg, \
+			       (soc_hdac_bus_ppcap_readl(dev, reg) & \
+				~(mask)) | (val))
+#define soc_hdac_bus_ppcap_updatew(dev, reg, mask, val) \
+	soc_hdac_bus_ppcap_writew(dev, reg, \
+			       (soc_hdac_bus_ppcap_readw(dev, reg) & \
+				~(mask)) | (val))
+#define soc_hdac_bus_ppcap_updateb(dev, reg, mask, val) \
+	soc_hdac_bus_ppcap_writeb(dev, reg, \
+			       (soc_hdac_bus_ppcap_readb(dev, reg) & \
+				~(mask)) | (val))
+
+void snd_soc_hdac_stream_spbcap_enable(struct soc_hdac_bus *chip,
+				 bool enable, int index);
+/*
+ * macros for spcap register read/write
+ */
+#define _soc_hdac_bus_spbcap_write(type, dev, reg, value)	\
+	((dev)->bus.io_ops->reg_write ## type(value,		\
+	(dev)->spbcap_addr + (reg)))
+#define _soc_hdac_bus_spbcap_read(type, dev, reg)		\
+	((dev)->bus.io_ops->reg_read ## type((dev)->spbcap_addr + (reg)))
+
+/* read/write a register, pass without AZX_REG_ prefix */
+#define soc_hdac_bus_spbcap_writel(dev, reg, value) \
+	_soc_hdac_bus_spbcap_write(l, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_bus_spbcap_writew(dev, reg, value) \
+	_soc_hdac_bus_spbcap_write(w, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_bus_spbcap_writeb(dev, reg, value) \
+	_soc_hdac_bus_spbcap_write(b, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_bus_spbcap_readl(dev, reg) \
+	_soc_hdac_bus_spbcap_read(l, dev, AZX_REG_ ## reg)
+#define soc_hdac_bus_spbcap_readw(dev, reg) \
+	_soc_hdac_bus_spbcap_read(w, dev, AZX_REG_ ## reg)
+#define soc_hdac_bus_spbcap_readb(dev, reg) \
+	_soc_hdac_bus_spbcap_read(b, dev, AZX_REG_ ## reg)
+
+/* update a register, pass without AZX_REG_ prefix */
+#define soc_hdac_bus_spbcap_updatel(dev, reg, mask, val) \
+	soc_hdac_bus_spbcap_writel(dev, reg, \
+			       (soc_hdac_bus_spbcap_readl(dev, reg) & \
+				~(mask)) | (val))
+#define soc_hdac_bus_spbcap_updatew(dev, reg, mask, val) \
+	soc_hdac_bus_spbcap_writew(dev, reg, \
+			       (soc_hdac_bus_spbcap_readw(dev, reg) & \
+				~(mask)) | (val))
+#define soc_hdac_bus_spbcap_updateb(dev, reg, mask, val) \
+	soc_hdac_bus_spbcap_writeb(dev, reg, \
+			       (soc_hdac_bus_spbcap_readb(dev, reg) & \
+				~(mask)) | (val))
+
+int snd_soc_hdac_bus_get_ml_capabilities(struct soc_hdac_bus *bus);
+int snd_soc_hdac_bus_map_codec_to_link(struct soc_hdac_bus *bus, int addr);
+struct soc_hdac_link *snd_soc_hdac_bus_get_link(struct soc_hdac_bus *bus,
+						const char *codec_name);
+
+/*
+ * macros for mlcap register read/write
+ */
+#define _soc_hdac_bus_mlcap_write(type, dev, reg, value)		\
+	((dev)->bus.io_ops->reg_write ## type(value, (dev)->mlcap_addr + (reg)))
+#define _soc_hdac_bus_mlcap_read(type, dev, reg)			\
+	((dev)->bus.io_ops->reg_read ## type((dev)->mlcap_addr + (reg)))
+
+/* read/write a register, pass without AZX_REG_ prefix */
+#define soc_hdac_bus_mlcap_writel(dev, reg, value) \
+	_soc_hdac_bus_mlcap_write(l, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_bus_mlcap_writew(dev, reg, value) \
+	_soc_hdac_bus_mlcap_write(w, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_bus_mlcap_writeb(dev, reg, value) \
+	_soc_hdac_bus_mlcap_write(b, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_bus_mlcap_readl(dev, reg) \
+	_soc_hdac_bus_mlcap_read(l, dev, AZX_REG_ ## reg)
+#define soc_hdac_bus_mlcap_readw(dev, reg) \
+	_soc_hdac_bus_mlcap_read(w, dev, AZX_REG_ ## reg)
+#define soc_hdac_bus_mlcap_readb(dev, reg) \
+	_soc_hdac_bus_mlcap_read(b, dev, AZX_REG_ ## reg)
+
+/* update a register, pass without AZX_REG_ prefix */
+#define soc_hdac_bus_mlcap_updatel(dev, reg, mask, val) \
+	soc_hdac_bus_mlcap_writel(dev, reg, \
+			       (soc_hdac_bus_mlcap_readl(dev, reg) & \
+				~(mask)) | (val))
+#define soc_hdac_bus_mlcap_updatew(dev, reg, mask, val) \
+	soc_hdac_bus_mlcap_writew(dev, reg, \
+			       (soc_hdac_bus_mlcap_readw(dev, reg) & \
+				~(mask)) | (val))
+#define soc_hdac_bus_mlcap_updateb(dev, reg, mask, val) \
+	soc_hdac_bus_mlcap_writeb(dev, reg, \
+			       (soc_hdac_bus_mlcap_readb(dev, reg) & \
+				~(mask)) | (val))
+
+/*
+ * macros for gtscap register read/write
+ */
+#define _soc_hdac_bus_gtscap_write(type, dev, reg, value)	\
+	((dev)->bus.io_ops->reg_write ## type(value,		\
+	(dev)->gtscap_addr + (reg)))
+#define _soc_hdac_bus_gtscap_read(type, dev, reg)		\
+	((dev)->bus.io_ops->reg_read ## type((dev)->gtscap_addr + (reg)))
+
+/* read/write a register, pass without AZX_REG_ prefix */
+#define soc_hdac_bus_gtscap_writel(dev, reg, value) \
+	_soc_hdac_bus_gtscap_write(l, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_bus_gtscap_writew(dev, reg, value) \
+	_soc_hdac_bus_gtscap_write(w, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_bus_gtscap_writeb(dev, reg, value) \
+	_soc_hdac_bus_gtscap_write(b, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_bus_gtscap_readl(dev, reg) \
+	_soc_hdac_bus_gtscap_read(l, dev, AZX_REG_ ## reg)
+#define soc_hdac_bus_gtscap_readw(dev, reg) \
+	_soc_hdac_bus_gtscap_read(w, dev, AZX_REG_ ## reg)
+#define soc_hdac_bus_gtscap_readb(dev, reg) \
+	_soc_hdac_bus_gtscap_read(b, dev, AZX_REG_ ## reg)
+
+/* update a register, pass without AZX_REG_ prefix */
+#define soc_hdac_bus_gtscap_updatel(dev, reg, mask, val) \
+	soc_hdac_bus_gtscap_writel(dev, reg, \
+			       (soc_hdac_bus_gtscap_readl(dev, reg) & \
+				~(mask)) | (val))
+#define soc_hdac_bus_gtscap_updatew(dev, reg, mask, val) \
+	soc_hdac_bus_gtscap_writew(dev, reg, \
+			       (soc_hdac_bus_gtscap_readw(dev, reg) & \
+				~(mask)) | (val))
+#define soc_hdac_bus_gtscap_updateb(dev, reg, mask, val) \
+	soc_hdac_bus_gtscap_writeb(dev, reg, \
+			       (soc_hdac_bus_gtscap_readb(dev, reg) & \
+				~(mask)) | (val))
+enum hdac_stream_type {
+	HDAC_STREAM_TYPE_COUPLED = 0,
+	HDAC_STREAM_TYPE_HOST,
+	HDAC_STREAM_TYPE_LINK
+};
+
+struct soc_hdac_stream {
+	struct hdac_stream hstream;
+
+	void __iomem *pphc_addr; /* processing pipe host stream reg pointer */
+	void __iomem *pplc_addr; /* processing pipe link stream reg pointer */
+
+	bool decoupled:1;
+	bool link_locked:1;
+	bool link_prepared;
+
+	struct snd_pcm_substream *link_substream;
+};
+
+#define hdac_stream(s)		(&(s)->hstream)
+#define stream_to_soc_hdac_stream(s) \
+	container_of(s, struct soc_hdac_stream, hstream)
+
+void snd_soc_hdac_stream_init(struct soc_hdac_bus *bus,
+				struct soc_hdac_stream *stream, int idx,
+				int direction, int tag);
+struct soc_hdac_stream *snd_soc_hdac_stream_assign(struct soc_hdac_bus *bus,
+					   struct snd_pcm_substream *substream,
+					   int type);
+void snd_soc_hdac_stream_release(struct soc_hdac_stream *azx_dev, int type);
+void snd_soc_hdac_stream_decouple(struct soc_hdac_bus *bus,
+				struct soc_hdac_stream *azx_dev, bool decouple);
+void snd_soc_hdac_stop_streams(struct soc_hdac_bus *sbus);
+
+/*
+ * macros for host stream register read/write
+ */
+#define _soc_hdac_host_stream_write(type, dev, reg, value)	\
+	((dev)->hstream.bus->io_ops->reg_write ## type(value,	\
+	(dev)->pphc_addr + (reg)))
+#define _soc_hdac_host_stream_read(type, dev, reg)		\
+	((dev)->hstream.bus->io_ops->reg_read ## type(		\
+	(dev)->pphc_addr + (reg)))
+
+/* read/write a register, pass without AZX_REG_ prefix */
+#define soc_hdac_host_stream_writel(dev, reg, value) \
+	_soc_hdac_host_stream_write(l, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_host_stream_writew(dev, reg, value) \
+	_soc_hdac_host_stream_write(w, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_host_stream_writeb(dev, reg, value) \
+	_soc_hdac_host_stream_write(b, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_host_stream_readl(dev, reg) \
+	_soc_hdac_host_stream_read(l, dev, AZX_REG_ ## reg)
+#define soc_hdac_host_stream_readw(dev, reg) \
+	_soc_hdac_host_stream_read(w, dev, AZX_REG_ ## reg)
+#define soc_hdac_host_stream_readb(dev, reg) \
+	_soc_hdac_host_stream_read(b, dev, AZX_REG_ ## reg)
+
+/* update a register, pass without AZX_REG_ prefix */
+#define soc_hdac_host_stream_updatel(dev, reg, mask, val) \
+	soc_hdac_host_stream_writel(dev, reg, \
+			       (soc_hdac_host_stream_readl(dev, reg) & \
+				~(mask)) | (val))
+#define soc_hdac_host_stream_updatew(dev, reg, mask, val) \
+	soc_hdac_host_stream_writew(dev, reg, \
+			       (soc_hdac_host_stream_readw(dev, reg) & \
+				~(mask)) | (val))
+#define soc_hdac_host_stream_updateb(dev, reg, mask, val) \
+	soc_hdac_host_stream_writeb(dev, reg, \
+			       (soc_hdac_host_stream_readb(dev, reg) & \
+				~(mask)) | (val))
+
+void snd_soc_hdac_link_stream_start(struct soc_hdac_stream *hstream);
+void snd_soc_hdac_link_stream_clear(struct soc_hdac_stream *hstream);
+void snd_soc_hdac_link_stream_reset(struct soc_hdac_stream *hstream);
+int snd_soc_hdac_link_stream_setup(struct soc_hdac_stream *stream, int fmt);
+
+/*
+ * macros for link stream register read/write
+ */
+#define _soc_hdac_link_stream_write(type, dev, reg, value)	\
+	((dev)->hstream.bus->io_ops->reg_write ## type(value,	\
+	(dev)->pplc_addr + (reg)))
+#define _soc_hdac_link_stream_read(type, dev, reg)		\
+	((dev)->hstream.bus->io_ops->reg_read ## type((dev)->pplc_addr + (reg)))
+
+/* read/write a register, pass without AZX_REG_ prefix */
+#define soc_hdac_link_stream_writel(dev, reg, value) \
+	_soc_hdac_link_stream_write(l, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_link_stream_writew(dev, reg, value) \
+	_soc_hdac_link_stream_write(w, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_link_stream_writeb(dev, reg, value) \
+	_soc_hdac_link_stream_write(b, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_link_stream_readl(dev, reg) \
+	_soc_hdac_link_stream_read(l, dev, AZX_REG_ ## reg)
+#define soc_hdac_link_stream_readw(dev, reg) \
+	_soc_hdac_link_stream_read(w, dev, AZX_REG_ ## reg)
+#define soc_hdac_link_stream_readb(dev, reg) \
+	_soc_hdac_link_stream_read(b, dev, AZX_REG_ ## reg)
+
+/* update a register, pass without AZX_REG_ prefix */
+#define soc_hdac_link_stream_updatel(dev, reg, mask, val) \
+	soc_hdac_link_stream_writel(dev, reg, \
+			       (soc_hdac_link_stream_readl(dev, reg) & \
+				~(mask)) | (val))
+#define soc_hdac_link_stream_updatew(dev, reg, mask, val) \
+	soc_hdac_link_stream_writew(dev, reg, \
+			       (soc_hdac_link_stream_readw(dev, reg) & \
+				~(mask)) | (val))
+#define soc_hdac_link_stream_updateb(dev, reg, mask, val) \
+	soc_hdac_link_stream_writeb(dev, reg, \
+			       (soc_hdac_link_stream_readb(dev, reg) & \
+				~(mask)) | (val))
+struct soc_hdac_link {
+	struct hdac_bus *bus;
+	int index;
+	void __iomem *ml_addr; /* link output stream reg pointer */
+	u32 lcaps;   /* link capablities */
+	u16 lsdiid;  /* link sdi identifier */
+	char codec[HDA_MAX_CODECS][32]; /* codecs connectes to the link */
+	struct list_head list;
+};
+
+int snd_soc_hdac_bus_link_power_up(struct soc_hdac_link *link);
+int snd_soc_hdac_bus_link_power_down(struct soc_hdac_link *link);
+void snd_soc_hdac_link_set_stream_id(struct soc_hdac_link *link,
+				 int stream);
+void snd_soc_hdac_link_clear_stream_id(struct soc_hdac_link *link,
+				 int stream);
+
+/*
+ * macros for mutilink register read/ write
+ */
+#define _soc_hdac_link_write(type, dev, reg, value)			\
+	((dev)->bus->io_ops->reg_write ## type(value, (dev)->ml_addr + (reg)))
+#define _soc_hdac_link_read(type, dev, reg)				\
+	((dev)->bus->io_ops->reg_read ## type((dev)->ml_addr + (reg)))
+
+/* read/write a register, pass without AZX_REG_ prefix */
+#define soc_hdac_link_writel(dev, reg, value) \
+	_soc_hdac_link_write(l, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_link_writew(dev, reg, value) \
+	_soc_hdac_link_write(w, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_link_writeb(dev, reg, value) \
+	_soc_hdac_link_write(b, dev, AZX_REG_ ## reg, value)
+#define soc_hdac_link_readl(dev, reg) \
+	_soc_hdac_link_read(l, dev, AZX_REG_ ## reg)
+#define soc_hdac_link_readw(dev, reg) \
+	_soc_hdac_link_read(w, dev, AZX_REG_ ## reg)
+#define soc_hdac_link_readb(dev, reg) \
+	_soc_hdac_link_read(b, dev, AZX_REG_ ## reg)
+
+/* update a register, pass without AZX_REG_ prefix */
+#define soc_hdac_link_updatel(dev, reg, mask, val) \
+	soc_hdac_link_writel(dev, reg, \
+			       (soc_hdac_link_readl(dev, reg) & \
+				~(mask)) | (val))
+#define soc_hdac_link_updatew(dev, reg, mask, val) \
+	soc_hdac_link_writew(dev, reg, \
+			       (soc_hdac_link_readw(dev, reg) & \
+				~(mask)) | (val))
+#define soc_hdac_link_updateb(dev, reg, mask, val) \
+	soc_hdac_link_writeb(dev, reg, \
+			       (soc_hdac_link_readb(dev, reg) & \
+				~(mask)) | (val))
+#endif /* __SOUND_SOC_HDAUDIO_H */
diff --git a/sound/soc/hda/Makefile b/sound/soc/hda/Makefile
index 9585ab180a55..b6d5b1840dc2 100644
--- a/sound/soc/hda/Makefile
+++ b/sound/soc/hda/Makefile
@@ -1,3 +1,4 @@
-snd-soc-hda-core-objs := soc-hda-codec.o
+snd-soc-hda-core-objs := soc-hdac-controller.o soc-hdac-stream.o \
+soc-hdac-bus.o soc-hda-codec.o
 
 obj-$(CONFIG_SND_SOC_HDA_CORE) += snd-soc-hda-core.o
diff --git a/sound/soc/hda/soc-hdac-bus.c b/sound/soc/hda/soc-hdac-bus.c
new file mode 100644
index 000000000000..f541668577c6
--- /dev/null
+++ b/sound/soc/hda/soc-hdac-bus.c
@@ -0,0 +1,115 @@
+/*
+ *  soc-hdac-bus.c - SoC HD-audio core bus functions.
+ *
+ *  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 published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  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/kernel.h>
+#include <linux/delay.h>
+#include <linux/export.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <sound/core.h>
+#include <sound/soc-hdaudio.h>
+
+/**
+ * snd_soc_hdac_bus_init - initialize a HD-audio soc bus
+ * @sbus: the pointer to soc bus object
+ * @dev: device pointer
+ * @ops: bus verb operators
+ * @io_ops: lowlevel I/O operators
+ *
+ * Returns 0 if successful, or a negative error code.
+ */
+int snd_soc_hdac_bus_init(struct soc_hdac_bus *sbus, struct device *dev,
+			const struct hdac_bus_ops *ops,
+			const struct hdac_io_ops *io_ops)
+{
+	int ret;
+
+	ret = snd_hdac_bus_init(&sbus->bus, dev, ops, io_ops);
+	if (ret < 0)
+		return ret;
+
+	INIT_LIST_HEAD(&sbus->hlink_list);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_bus_init);
+
+/**
+ * snd_soc_hdac_bus_exit - clean up a HD-audio soc bus
+ * @sbus: the pointer to soc bus object
+ */
+void snd_soc_hdac_bus_exit(struct soc_hdac_bus *sbus)
+{
+	snd_hdac_bus_exit(&sbus->bus);
+	WARN_ON(!list_empty(&sbus->hlink_list));
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_bus_exit);
+
+static void default_release(struct device *dev)
+{
+	snd_soc_hdac_bus_device_exit(container_of(dev, struct hdac_device, dev));
+}
+
+/**
+ * snd_soc_hdac_device_init - initialize the HD-audio soc codec base device
+ * @sbus: hdac asoc bus to attach to
+ * @addr: codec address
+ *
+ * Returns zero for success or a negative error code.
+ */
+int snd_soc_hdac_bus_device_init(struct soc_hdac_bus *sbus, int addr)
+{
+	struct hdac_device *hdev = NULL;
+	struct hdac_bus *bus = hdac_bus(sbus);
+	char name[15];
+	int ret;
+
+	hdev = kzalloc(sizeof(*hdev), GFP_KERNEL);
+	if (!hdev)
+		return -ENOMEM;
+
+	snprintf(name, sizeof(name), "soc-hda-codec#%03x", addr);
+
+	ret  = snd_hdac_device_init(hdev, bus, name, addr);
+	if (ret < 0) {
+		dev_err(bus->dev, "device init failed for hdac device\n");
+		return ret;
+	}
+	hdev->type = HDA_DEV_ASOC;
+	hdev->dev.release = default_release;
+
+	ret = snd_hdac_device_register(hdev);
+	if (ret) {
+		dev_err(bus->dev, "failed to register hdac device\n");
+		snd_soc_hdac_bus_device_exit(hdev);
+		return ret;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_bus_device_init);
+
+/**
+ * snd_soc_hdac_bus_device_exit - clean up a HD-audio soc codec base device
+ * @hdev: hdac device to clean up
+ */
+void snd_soc_hdac_bus_device_exit(struct hdac_device *hdev)
+{
+	snd_hdac_device_exit(hdev);
+	kfree(hdev);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_bus_device_exit);
diff --git a/sound/soc/hda/soc-hdac-controller.c b/sound/soc/hda/soc-hdac-controller.c
new file mode 100644
index 000000000000..53454703cece
--- /dev/null
+++ b/sound/soc/hda/soc-hdac-controller.c
@@ -0,0 +1,296 @@
+/*
+ *  soc-hdac-controller.c - SoC HD-audio controller functions.
+ *
+ *  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 published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  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/kernel.h>
+#include <linux/delay.h>
+#include <linux/export.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <sound/core.h>
+#include <sound/hda_register.h>
+#include <sound/soc-hdaudio.h>
+
+/**
+ * snd_soc_hdac_bus_parse_capabilities - parse capablity structure
+ * @sbus: the pointer to soc bus object
+ *
+ * Returns 0 if successful, or a negative error code.
+ */
+int snd_soc_hdac_bus_parse_capabilities(struct soc_hdac_bus *sbus)
+{
+	unsigned int cur_cap;
+	unsigned int offset;
+	struct hdac_bus *bus = &sbus->bus;
+
+	offset = snd_hdac_chip_readl(bus, LLCH);
+
+	if (offset < 0)
+		return -EIO;
+
+	sbus->ppcap = false;
+	sbus->mlcap = false;
+	sbus->spbcap = false;
+	sbus->gtscap = false;
+
+	/* Lets walk the linked capabilities list */
+	do {
+		cur_cap = _snd_hdac_chip_read(l, bus, offset);
+
+		if (cur_cap < 0)
+			return -EIO;
+
+		dev_dbg(bus->dev, "Capability version: 0x%x\n",
+				((cur_cap & AZX_CAP_HDR_VER_MASK) >> AZX_CAP_HDR_VER_OFF));
+
+		dev_dbg(bus->dev, "HDA capability ID: 0x%x\n",
+				(cur_cap & AZX_CAP_HDR_ID_MASK) >> AZX_CAP_HDR_ID_OFF);
+
+		switch ((cur_cap & AZX_CAP_HDR_ID_MASK) >> AZX_CAP_HDR_ID_OFF) {
+		case AZX_ML_CAP_ID:
+			dev_dbg(bus->dev, "Found ML capability\n");
+			sbus->mlcap = true;
+			sbus->mlcap_addr = bus->remap_addr + offset;
+			break;
+
+		case AZX_GTS_CAP_ID:
+			dev_dbg(bus->dev, "Found GTS capability offset=%x\n", offset);
+			sbus->gtscap = true;
+			sbus->gtscap_addr = bus->remap_addr + offset;
+			break;
+
+		case AZX_PP_CAP_ID:
+			/* PP capability found, the Audio DSP is present */
+			dev_dbg(bus->dev, "Found PP capability offset=%x\n", offset);
+			sbus->ppcap = true;
+			sbus->ppcap_addr = bus->remap_addr + offset;
+			break;
+
+		case AZX_SPB_CAP_ID:
+			/* SPIB capability found, handler function */
+			dev_dbg(bus->dev, "Found SPB capability\n");
+			sbus->spbcap = true;
+			sbus->spbcap_addr = bus->remap_addr + offset;
+			break;
+
+		default:
+			dev_dbg(bus->dev, "Unknown capability %d\n", cur_cap);
+			break;
+		}
+
+		/* read the offset of next capabiity */
+		offset = cur_cap & AZX_CAP_HDR_NXT_PTR_MASK;
+	} while (offset);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_bus_parse_capabilities);
+
+/*
+ * processing pipe helpers - these helpers are useful for dealing with HDA
+ * new capability of processing pipelines
+ */
+
+/**
+ * snd_soc_hdac_bus_ppcap_enable - enable/disable processing pipe capability
+ * @sbus: HD-audio soc core bus
+ * @enable: flag to turn on/off the capability
+ */
+void snd_soc_hdac_bus_ppcap_enable(struct soc_hdac_bus *sbus, bool enable)
+{
+	struct hdac_bus *bus = &sbus->bus;
+
+	if (!sbus->ppcap) {
+		dev_err(bus->dev, "Address of PP capability is NULL");
+		return;
+	}
+
+
+	if (enable)
+		soc_hdac_bus_ppcap_updatel(sbus, PP_PPCTL, 0, AZX_PPCTL_GPROCEN);
+	else
+		soc_hdac_bus_ppcap_updatel(sbus, PP_PPCTL, AZX_PPCTL_GPROCEN, 0);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_bus_ppcap_enable);
+
+/**
+ * snd_soc_hdac_bus_ppcap_int_enable - ppcap interrupt enable/disable
+ * @sbus: HD-audio soc core bus
+ * @enable: flag to enable/disable interrupt
+ */
+void snd_soc_hdac_bus_ppcap_int_enable(struct soc_hdac_bus *sbus, bool enable)
+{
+	struct hdac_bus *bus = &sbus->bus;
+
+	if (!sbus->ppcap) {
+		dev_err(bus->dev, "Address of PP capability is NULL\n");
+		return;
+	}
+
+	if (enable)
+		soc_hdac_bus_ppcap_updatel(sbus, PP_PPCTL, 0, AZX_PPCTL_PIE);
+	else
+		soc_hdac_bus_ppcap_updatel(sbus, PP_PPCTL, AZX_PPCTL_PIE, 0);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_bus_ppcap_int_enable);
+
+/*
+ * Multilink helpers - these helpers are useful for dealing with HDA
+ * new multilink capability
+ */
+
+/**
+ * snd_soc_hdac_bus_get_ml_capabilities - get multilink capability
+ * @sbus: HD-audio soc core bus
+ *
+ * This will parse all links and read the mlink capabilities and add them
+ * in hlink_list of asoc hdac bus
+ * Note: this will be freed on bus exit by driver
+ */
+int snd_soc_hdac_bus_get_ml_capabilities(struct soc_hdac_bus *sbus)
+{
+	int idx;
+	u32 link_count;
+	struct soc_hdac_link *hlink;
+	struct hdac_bus *bus = &sbus->bus;
+
+	link_count = soc_hdac_bus_mlcap_readl(sbus, ML_MLCD) + 1;
+
+	dev_dbg(bus->dev, "In %s Link count: %d\n", __func__, link_count);
+
+	for (idx = 0; idx < link_count; idx++) {
+		hlink  = kzalloc(sizeof(*hlink), GFP_KERNEL);
+		if (!hlink)
+			return -ENOMEM;
+		hlink->index = idx;
+		hlink->bus = bus;
+		hlink->ml_addr = sbus->mlcap_addr +
+					AZX_ML_BASE +
+					(AZX_ML_INTERVAL *
+					idx);
+		hlink->lcaps  = soc_hdac_link_readl(hlink, ML_LCAP);
+		hlink->lsdiid = soc_hdac_link_readw(hlink, ML_LSDIID);
+
+		list_add_tail(&hlink->list, &sbus->hlink_list);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_bus_get_ml_capabilities);
+
+/**
+ * snd_soc_hdac_bus_map_codec_to_link - maps codec to link
+ * @sbus: HD-audio soc core bus
+ * @addr: codec address
+ */
+int snd_soc_hdac_bus_map_codec_to_link(struct soc_hdac_bus *sbus, int addr)
+{
+	struct soc_hdac_link *hlink;
+	struct hdac_bus *bus = &sbus->bus;
+
+	list_for_each_entry(hlink, &sbus->hlink_list, list) {
+		/* check if SDI bit number is same as Codec address */
+		dev_dbg(bus->dev, "lsdid for %d link %x\n", hlink->index, hlink->lsdiid);
+		if (!hlink->lsdiid)
+			continue;
+
+		if (hlink->lsdiid && (0x1 << addr)) {
+			snprintf(hlink->codec[addr],
+					sizeof(hlink->codec[addr]),
+					"codec#%d", addr);
+			break;
+		}
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_bus_map_codec_to_link);
+
+/**
+ * snd_soc_hdac_bus_get_link_index - get link based on codec name
+ * @sbus: HD-audio soc core bus
+ * @codec_name: codec name
+ */
+struct soc_hdac_link *snd_soc_hdac_bus_get_link(struct soc_hdac_bus *sbus,
+						 const char *codec_name)
+{
+	int i;
+	struct soc_hdac_link *hlink = NULL;
+
+	list_for_each_entry(hlink, &sbus->hlink_list, list) {
+		for (i = 0; i < HDA_MAX_CODECS; i++) {
+			if (!hlink->codec[i][0])
+				break;
+			if (!strncmp(hlink->codec[i], codec_name,
+					strlen(codec_name)))
+				return hlink;
+		}
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_bus_get_link);
+
+int check_hdac_link_power_active(struct soc_hdac_link *link, bool enable)
+{
+	int timeout;
+	u32 val;
+	int mask = (1 << AZX_MLCTL_CPA);
+
+	udelay(3);
+	timeout = 50;
+
+	do {
+		val = soc_hdac_link_readl(link, ML_LCTL);
+		if (enable) {
+			if (((val & mask) >> AZX_MLCTL_CPA))
+				return 0;
+		} else {
+			if (!((val & mask) >> AZX_MLCTL_CPA))
+				return 0;
+		}
+		udelay(3);
+	} while (--timeout);
+
+	return -EIO;
+}
+
+/**
+ * snd_soc_hdac_bus_link_power_up -power up hda link
+ * @link: HD-audio soc link
+ */
+int snd_soc_hdac_bus_link_power_up(struct soc_hdac_link *link)
+{
+	soc_hdac_link_updatel(link, ML_LCTL, 0, AZX_MLCTL_SPA);
+
+	return check_hdac_link_power_active(link, true);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_bus_link_power_up);
+
+/**
+ * snd_soc_hdac_bus_link_power_down -power down hda link
+ * @link: HD-audio soc link
+ */
+int snd_soc_hdac_bus_link_power_down(struct soc_hdac_link *link)
+{
+	soc_hdac_link_updatel(link, ML_LCTL, AZX_MLCTL_SPA, 0);
+
+	return check_hdac_link_power_active(link, false);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_bus_link_power_down);
+
+MODULE_DESCRIPTION("HDA SoC core");
+MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/hda/soc-hdac-stream.c b/sound/soc/hda/soc-hdac-stream.c
new file mode 100644
index 000000000000..81d9eed4ece2
--- /dev/null
+++ b/sound/soc/hda/soc-hdac-stream.c
@@ -0,0 +1,409 @@
+/*
+ *  soc-hdac-stream.c - SoC HD-audio stream  operations.
+ *
+ *  Copyright (C) 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 published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  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/kernel.h>
+#include <linux/delay.h>
+#include <linux/export.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/hdaudio.h>
+#include <sound/hda_register.h>
+#include <sound/soc-hdaudio.h>
+
+/**
+ * snd_soc_hdac_stream_init - initialize each stream (aka device)
+ * @sbus: HD-audio soc core bus
+ * @stream: HD-audio soc core stream object to initialize
+ * @idx: stream index number
+ * @direction: stream direction (SNDRV_PCM_STREAM_PLAYBACK or SNDRV_PCM_STREAM_CAPTURE)
+ * @tag: the tag id to assign
+ *
+ * Assign the starting bdl address to each stream (device) and initialize.
+ */
+void snd_soc_hdac_stream_init(struct soc_hdac_bus *sbus,
+				struct soc_hdac_stream *stream,
+				int idx, int direction, int tag)
+{
+	struct hdac_bus *bus = &sbus->bus;
+
+	if (sbus->ppcap) {
+		stream->pphc_addr = sbus->ppcap_addr +
+					AZX_PPHC_BASE +
+					AZX_PPHC_INTERVAL *
+					idx;
+
+		stream->pplc_addr = sbus->ppcap_addr +
+					AZX_PPLC_BASE +
+					AZX_PPLC_MULTI *
+					sbus->num_streams +
+					AZX_PPLC_INTERVAL *
+					idx;
+	}
+	stream->decoupled = false;
+	snd_hdac_stream_init(bus, &stream->hstream, idx, direction, tag);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_stream_init);
+
+/**
+ * snd_soc_hdac_stream_decouple - decouple the hdac stream
+ * @sbus: HD-audio soc core bus
+ * @stream: HD-audio soc core stream object to initialize
+ * @decouple: flag to decouple
+ */
+void snd_soc_hdac_stream_decouple(struct soc_hdac_bus *sbus,
+				struct soc_hdac_stream *stream, bool decouple)
+{
+	struct hdac_stream *hstream = &stream->hstream;
+	struct hdac_bus *bus = &sbus->bus;
+
+	spin_lock_irq(&bus->reg_lock);
+	if (decouple)
+		soc_hdac_bus_ppcap_updatel(sbus, PP_PPCTL, 0,
+						AZX_PPCTL_PROCEN(hstream->index));
+	else
+		soc_hdac_bus_ppcap_updatel(sbus, PP_PPCTL,
+					AZX_PPCTL_PROCEN(hstream->index), 0);
+	stream->decoupled = decouple;
+	spin_unlock_irq(&bus->reg_lock);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_stream_decouple);
+
+/**
+ * snd_soc_hdac_linkstream_start - start a stream
+ * @stream: HD-audio soc core stream to start
+ */
+void snd_soc_hdac_link_stream_start(struct soc_hdac_stream *stream)
+{
+	soc_hdac_link_stream_updatel(stream, PPLCCTL,
+				0, AZX_PPLCCTL_RUN);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_link_stream_start);
+
+/**
+ * snd_soc_hdac_link_stream_clear - stop a stream DMA
+ * @stream: HD-audio soc core stream to stop
+ */
+void snd_soc_hdac_link_stream_clear(struct soc_hdac_stream *stream)
+{
+	soc_hdac_link_stream_updatel(stream, PPLCCTL,
+					AZX_PPLCCTL_RUN, 0);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_link_stream_clear);
+
+/**
+ * snd_soc_hdac_link_stream_reset - reset a stream
+ * @stream: HD-audio soc core stream to reset
+ */
+void snd_soc_hdac_link_stream_reset(struct soc_hdac_stream *stream)
+{
+	unsigned char val;
+	int timeout;
+
+	snd_soc_hdac_link_stream_clear(stream);
+
+	soc_hdac_link_stream_updatel(stream, PPLCCTL, 0, AZX_PPLCCTL_STRST);
+	udelay(3);
+	timeout = 50;
+	do {
+		val = soc_hdac_link_stream_readl(stream, PPLCCTL) &
+			AZX_PPLCCTL_STRST;
+		if (val)
+			break;
+		udelay(3);
+	} while (--timeout);
+	val &= ~AZX_PPLCCTL_STRST;
+	soc_hdac_link_stream_writel(stream, PPLCCTL, val);
+	udelay(3);
+
+	timeout = 50;
+	/* waiting for hardware to report that the stream is out of reset */
+	do {
+		val = soc_hdac_link_stream_readl(stream, PPLCCTL) &
+			AZX_PPLCCTL_STRST;
+		if (!val)
+			break;
+		udelay(3);
+	} while (--timeout);
+
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_link_stream_reset);
+
+/**
+ * snd_soc_hdac_link_stream_setup -  set up the SD for streaming
+ * @stream: HD-audio soc core stream to set up
+ * @fmt: stream format
+ */
+int snd_soc_hdac_link_stream_setup(struct soc_hdac_stream *stream, int fmt)
+{
+	struct hdac_stream *hstream = &stream->hstream;
+	unsigned int val;
+
+	/* make sure the run bit is zero for SD */
+	snd_soc_hdac_link_stream_clear(stream);
+	/* program the stream_tag */
+	val = soc_hdac_link_stream_readl(stream, PPLCCTL);
+	val = (val & ~AZX_PPLCCTL_STRM_MASK) |
+		(hstream->stream_tag << AZX_PPLCCTL_STRM_SHIFT);
+	soc_hdac_link_stream_writel(stream, PPLCCTL, val);
+
+	/* program the stream format */
+	soc_hdac_link_stream_writew(stream, PPLCFMT, fmt);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_link_stream_setup);
+
+/**
+ * snd_soc_hdac_link_set_stream_id - maps stream id to link output
+ * @link: HD-audio soc link to set up
+ * @stream: stream id
+ */
+void snd_soc_hdac_link_set_stream_id(struct soc_hdac_link *link,
+				 int stream)
+{
+	soc_hdac_link_updatew(link, ML_LOSIDV, (1 << stream), 0);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_link_set_stream_id);
+
+/**
+ * snd_soc_hdac_link_clear_stream_id - maps stream id to link output
+ * @link: HD-audio soc link to set up
+ * @stream: stream id
+ */
+void snd_soc_hdac_link_clear_stream_id(struct soc_hdac_link *link,
+				 int stream)
+{
+	soc_hdac_link_updatew(link, ML_LOSIDV, 0, (1 << stream));
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_link_clear_stream_id);
+
+static struct soc_hdac_stream *
+soc_hdac_link_stream_assign(struct soc_hdac_bus *sbus,
+				struct snd_pcm_substream *substream)
+{
+	struct soc_hdac_stream *res = NULL;
+	struct hdac_stream *stream = NULL;
+	struct hdac_bus *hbus = &sbus->bus;
+
+	if (!sbus->ppcap) {
+		dev_err(hbus->dev, "stream type not supported\n");
+		return NULL;
+	}
+
+	list_for_each_entry(stream, &hbus->stream_list, list) {
+		struct soc_hdac_stream *hstream = container_of(stream,
+						struct soc_hdac_stream,
+						hstream);
+		if (stream->direction != substream->stream)
+			continue;
+
+		/* check if decoupled stream and not in use is available */
+		if (hstream->decoupled && !hstream->link_locked) {
+			res = hstream;
+			break;
+		}
+
+		if (!hstream->link_locked) {
+			snd_soc_hdac_stream_decouple(sbus, hstream, true);
+			res = hstream;
+			break;
+		}
+	}
+	if (res) {
+		spin_lock_irq(&hbus->reg_lock);
+		res->link_locked = 1;
+		res->link_substream = substream;
+		spin_unlock_irq(&hbus->reg_lock);
+	}
+	return res;
+}
+
+static struct soc_hdac_stream *
+soc_hdac_host_stream_assign(struct soc_hdac_bus *sbus,
+				struct snd_pcm_substream *substream)
+{
+	struct soc_hdac_stream *res = NULL;
+	struct hdac_stream *stream = NULL;
+	struct hdac_bus *hbus = &sbus->bus;
+	int key;
+
+	if (!sbus->ppcap) {
+		dev_err(hbus->dev, "stream type not supported\n");
+		return NULL;
+	}
+
+	/* make a non-zero unique key for the substream */
+	key = (substream->pcm->device << 16) | (substream->number << 2) |
+			(substream->stream + 1);
+
+	list_for_each_entry(stream, &hbus->stream_list, list) {
+		struct soc_hdac_stream *hstream = container_of(stream,
+						struct soc_hdac_stream,
+						hstream);
+		if (stream->direction != substream->stream)
+			continue;
+
+		if (stream->assigned_key == key) {
+			if (!hstream->decoupled)
+				snd_soc_hdac_stream_decouple(sbus, hstream, true);
+			res = hstream;
+			break;
+		}
+	}
+	if (res) {
+		spin_lock_irq(&hbus->reg_lock);
+		res->hstream.opened = 1;
+		res->hstream.running = 0;
+		res->hstream.assigned_key = key;
+		res->hstream.substream = substream;
+		spin_unlock_irq(&hbus->reg_lock);
+	}
+	return res;
+}
+
+/**
+ * snd_soc_hdac_stream_assign - assign a stream for the PCM
+ * @sbus: HD-audio soc core bus
+ * @substream: PCM substream to assign
+ * @type: type of stream (coupled, host or link stream)
+ *
+ * This assigns the stream based on the type (coupled/host/link), for the
+ * given PCM substream, assigns it and returns the stream object
+ *
+ * coupled: Looks for an unused stream
+ * host: Looks for an unused decoupled host stream
+ * link: Looks for an unused decoupled link stream
+ *
+ * If no stream is free, returns NULL. The function tries to keep using
+ * the same stream object when it's used beforehand.  when a stream is
+ * decoupled, it becomes a host stream and link stream.
+ */
+struct soc_hdac_stream *snd_soc_hdac_stream_assign(struct soc_hdac_bus *sbus,
+					   struct snd_pcm_substream *substream,
+					   int type)
+{
+	struct soc_hdac_stream *hstream = NULL;
+	struct hdac_stream *stream = NULL;
+	struct hdac_bus *hbus = &sbus->bus;
+
+	switch (type) {
+	case HDAC_STREAM_TYPE_COUPLED:
+		stream = snd_hdac_stream_assign(hbus, substream);
+		if (stream)
+			hstream = container_of(stream,
+					struct soc_hdac_stream, hstream);
+		return hstream;
+
+	case HDAC_STREAM_TYPE_HOST:
+		return soc_hdac_host_stream_assign(sbus, substream);
+
+	case HDAC_STREAM_TYPE_LINK:
+		return soc_hdac_link_stream_assign(sbus, substream);
+
+	default:
+		return NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_stream_assign);
+
+/**
+ * snd_soc_hdac_stream_release - release the assigned stream
+ * @stream: HD-audio soc core stream to release
+ * @type: type of stream (coupled, host or link stream)
+ *
+ * Release the stream that has been assigned by snd_soc_hdac_stream_assign().
+ */
+void snd_soc_hdac_stream_release(struct soc_hdac_stream *stream, int type)
+{
+	struct hdac_bus *bus = stream->hstream.bus;
+	struct soc_hdac_bus *sbus = bus_to_soc_hdac_bus(bus);
+
+	switch (type) {
+	case HDAC_STREAM_TYPE_COUPLED:
+		snd_hdac_stream_release(&stream->hstream);
+		break;
+
+	case HDAC_STREAM_TYPE_HOST:
+		if (stream->decoupled && !stream->link_locked) {
+			snd_soc_hdac_stream_decouple(sbus, stream, false);
+			snd_hdac_stream_release(&stream->hstream);
+		}
+		break;
+	case HDAC_STREAM_TYPE_LINK:
+		if (stream->decoupled)
+			snd_soc_hdac_stream_decouple(sbus, stream, false);
+		spin_lock_irq(&bus->reg_lock);
+		stream->link_locked = 0;
+		stream->link_substream = NULL;
+		spin_unlock_irq(&bus->reg_lock);
+		break;
+	default:
+		dev_dbg(bus->dev, "Invalid type\n");
+	}
+
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_stream_release);
+
+/**
+ * snd_soc_hdac_stream_spbcap_enable - enable SPIB for a stream
+ * @sbus: HD-audio soc core bus
+ * @enable: flag to enable/disable SPIB
+ * @index: stream index for which SPIB need to be enabled
+ */
+void snd_soc_hdac_stream_spbcap_enable(struct soc_hdac_bus *sbus,
+				 bool enable, int index)
+{
+	u32 mask = 0;
+	u32 register_mask = 0;
+	struct hdac_bus *bus = &sbus->bus;
+
+	if (!sbus->spbcap) {
+		dev_err(bus->dev, "Address of SPB capability is NULL");
+		return;
+	}
+
+	mask |= (1 << index);
+
+	register_mask = soc_hdac_bus_spbcap_readl(sbus, SPB_SPBFCCTL);
+
+	mask |= register_mask;
+
+	if (enable)
+		soc_hdac_bus_spbcap_updatel(sbus, SPB_SPBFCCTL, 0, mask);
+	else
+		soc_hdac_bus_spbcap_updatel(sbus, SPB_SPBFCCTL, mask, 0);
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_stream_spbcap_enable);
+
+/**
+ * snd_soc_hdac_stop_all_stream - stop all stream if running
+ * @sbus: HD-audio soc core bus
+ */
+void snd_soc_hdac_stop_streams(struct soc_hdac_bus *sbus)
+{
+	struct hdac_bus *bus = hdac_bus(sbus);
+	struct hdac_stream *stream;
+
+	if (bus->chip_init) {
+		list_for_each_entry(stream, &bus->stream_list, list)
+			snd_hdac_stream_stop(stream);
+		snd_hdac_bus_stop_chip(bus);
+	}
+}
+EXPORT_SYMBOL_GPL(snd_soc_hdac_stop_streams);
-- 
1.9.1

Re: [PATCH v4 3/7] ASoC: hda - add asoc hda core bus, controller and stream helpers

[Mark Brown]

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

On Mon, May 11, 2015 at 04:24:01PM +0530, Vinod Koul wrote:

> +#ifndef __SOUND_SOC_HDAUDIO_H
> +#define __SOUND_SOC_HDAUDIO_H
> +
> +#include <sound/core.h>
> +#include <sound/initval.h>
> +#include <sound/hda_register.h>
> +#include <sound/hdaudio.h>

...

> +#include <linux/kernel.h>
> +#include <linux/delay.h>
> +#include <linux/export.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <sound/core.h>
> +#include <sound/soc-hdaudio.h>

...

> +#include <linux/kernel.h>
> +#include <linux/delay.h>
> +#include <linux/export.h>
> +#include <sound/core.h>
> +#include <sound/pcm.h>
> +#include <sound/hdaudio.h>
> +#include <sound/hda_register.h>
> +#include <sound/soc-hdaudio.h>

This includes no ASoC specific headers, what is ASoC specific about it?

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

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

Re: [PATCH v4 3/7] ASoC: hda - add asoc hda core bus, controller and stream helpers

[Vinod Koul]

On Tue, May 26, 2015 at 07:51:51PM +0100, Mark Brown wrote:
> On Mon, May 11, 2015 at 04:24:01PM +0530, Vinod Koul wrote:
> 
> > +#ifndef __SOUND_SOC_HDAUDIO_H
> > +#define __SOUND_SOC_HDAUDIO_H
> > +
> > +#include <sound/core.h>
> > +#include <sound/initval.h>
> > +#include <sound/hda_register.h>
> > +#include <sound/hdaudio.h>
> 
> ...
> 
> > +#include <linux/kernel.h>
> > +#include <linux/delay.h>
> > +#include <linux/export.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <sound/core.h>
> > +#include <sound/soc-hdaudio.h>
> 
> ...
> 
> > +#include <linux/kernel.h>
> > +#include <linux/delay.h>
> > +#include <linux/export.h>
> > +#include <sound/core.h>
> > +#include <sound/pcm.h>
> > +#include <sound/hdaudio.h>
> > +#include <sound/hda_register.h>
> > +#include <sound/soc-hdaudio.h>
> 
> This includes no ASoC specific headers, what is ASoC specific about it?
Hi Mark,

This patch adds the hda helpers for enhanced capability added in SKL
HDA and these are not part of base HDA specification.

So we kept it out of Intel/ dir code as these and further enhancements can
be added/used by other vendors too..

Thanks
-- 
~Vinod

[PATCH v4 4/7] ASoC: intel - add Skylake HDA platform driver

[Vinod Koul]

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

This patch starts to add the Skylake HDA platform driver by defining SoC cpu
dais, DMA driver ops and implements ALSA operations

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/hda-skl-pcm.c | 586 ++++++++++++++++++++++++++++++++++
 1 file changed, 586 insertions(+)
 create mode 100644 sound/soc/intel/skylake/hda-skl-pcm.c

diff --git a/sound/soc/intel/skylake/hda-skl-pcm.c b/sound/soc/intel/skylake/hda-skl-pcm.c
new file mode 100644
index 000000000000..8f895c0c1777
--- /dev/null
+++ b/sound/soc/intel/skylake/hda-skl-pcm.c
@@ -0,0 +1,586 @@
+/*
+ *  hda-skl-pcm.c -ASoC HDA Platform driver file implementing PCM functionality
+ *
+ *  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 published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  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/pci.h>
+#include <linux/pm_runtime.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include "hda-skl.h"
+
+#define HDA_MONO 1
+#define HDA_STEREO 2
+
+static struct snd_pcm_hardware azx_pcm_hw = {
+	.info =			(SNDRV_PCM_INFO_MMAP |
+				 SNDRV_PCM_INFO_INTERLEAVED |
+				 SNDRV_PCM_INFO_BLOCK_TRANSFER |
+				 SNDRV_PCM_INFO_MMAP_VALID |
+				 SNDRV_PCM_INFO_PAUSE |
+				 SNDRV_PCM_INFO_SYNC_START |
+				 SNDRV_PCM_INFO_HAS_WALL_CLOCK | /* legacy */
+				 SNDRV_PCM_INFO_HAS_LINK_ATIME |
+				 SNDRV_PCM_INFO_NO_PERIOD_WAKEUP),
+	.formats =		SNDRV_PCM_FMTBIT_S16_LE,
+	.rates =		SNDRV_PCM_RATE_48000,
+	.rate_min =		48000,
+	.rate_max =		48000,
+	.channels_min =		2,
+	.channels_max =		2,
+	.buffer_bytes_max =	AZX_MAX_BUF_SIZE,
+	.period_bytes_min =	128,
+	.period_bytes_max =	AZX_MAX_BUF_SIZE / 2,
+	.periods_min =		2,
+	.periods_max =		AZX_MAX_FRAG,
+	.fifo_size =		0,
+};
+
+static inline
+struct soc_hdac_stream *get_hdac_stream(struct snd_pcm_substream *substream)
+{
+	return substream->runtime->private_data;
+}
+
+static struct soc_hdac_bus *get_bus_ctx(struct snd_pcm_substream *substream)
+{
+	struct soc_hdac_stream *stream = get_hdac_stream(substream);
+	struct hdac_stream *hstream = hdac_stream(stream);
+	struct hdac_bus *bus = hstream->bus;
+
+	return bus_to_soc_hdac_bus(bus);
+}
+
+static int substream_alloc_pages(struct soc_hdac_bus *sbus,
+				 struct snd_pcm_substream *substream,
+				 size_t size)
+{
+	struct soc_hdac_stream *stream = get_hdac_stream(substream);
+	int ret;
+
+	hdac_stream(stream)->bufsize = 0;
+	hdac_stream(stream)->period_bytes = 0;
+	hdac_stream(stream)->format_val = 0;
+
+	ret = snd_pcm_lib_malloc_pages(substream, size);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int substream_free_pages(struct hdac_bus *bus,
+				struct snd_pcm_substream *substream)
+{
+	return snd_pcm_lib_free_pages(substream);
+}
+
+static void soc_hda_set_pcm_constrains(struct soc_hdac_bus *sbus,
+				 struct snd_pcm_runtime *runtime)
+{
+	snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
+
+	/* avoid wrap-around with wall-clock */
+	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_TIME,
+				     20, 178000000);
+}
+
+static int soc_hda_pcm_open(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	struct soc_hdac_bus *sbus = dev_get_drvdata(dai->dev);
+	struct soc_hdac_stream *stream;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct soc_hda_dma_params *dma_params;
+
+	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
+	pm_runtime_get_sync(dai->dev);
+
+	stream = snd_soc_hdac_stream_assign(sbus, substream,
+					HDAC_STREAM_TYPE_COUPLED);
+	if (stream == NULL)
+		return -EBUSY;
+
+	soc_hda_set_pcm_constrains(sbus, runtime);
+
+	/*
+	 * disable WALLCLOCK timestamps for capture streams
+	 * until we figure out how to handle digital inputs
+	 */
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
+		runtime->hw.info &= ~SNDRV_PCM_INFO_HAS_WALL_CLOCK; /* legacy */
+		runtime->hw.info &= ~SNDRV_PCM_INFO_HAS_LINK_ATIME;
+	}
+
+	runtime->private_data = stream;
+
+	dma_params = kzalloc(sizeof(*dma_params), GFP_KERNEL);
+	if (!dma_params)
+		return -ENOMEM;
+
+	dma_params->stream_tag = hdac_stream(stream)->stream_tag;
+	snd_soc_dai_set_dma_data(dai, substream, (void *)dma_params);
+
+	dev_dbg(dai->dev, "stream tag set in dma params=%d\n",
+				 dma_params->stream_tag);
+	snd_pcm_set_sync(substream);
+
+	return 0;
+}
+
+static int hda_get_format(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = snd_pcm_substream_chip(substream);
+	struct soc_hda_dma_params *dma_params;
+	int format_val = 0;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+
+	dma_params  = (struct soc_hda_dma_params *)
+			snd_soc_dai_get_dma_data(codec_dai, substream);
+
+	if (!dma_params)
+		format_val = dma_params->format;
+
+	return format_val;
+}
+
+static int soc_hda_pcm_prepare(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	struct soc_hdac_stream *stream = get_hdac_stream(substream);
+	unsigned int format_val;
+	int err;
+
+	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
+	if (hdac_stream(stream)->prepared) {
+		dev_dbg(dai->dev, "already stream is prepared - returning\n");
+		return 0;
+	}
+
+	format_val = hda_get_format(substream, dai);
+	dev_dbg(dai->dev, "stream_tag=%d formatvalue=%d\n",
+				hdac_stream(stream)->stream_tag, format_val);
+	snd_hdac_stream_reset(hdac_stream(stream));
+
+	err = snd_hdac_stream_set_params(hdac_stream(stream), format_val);
+
+	if (err < 0)
+		return err;
+
+	snd_hdac_stream_setup(hdac_stream(stream));
+	hdac_stream(stream)->prepared = 1;
+
+	return err;
+}
+
+static int soc_hda_pcm_hw_params(struct snd_pcm_substream *substream,
+				struct snd_pcm_hw_params *params,
+				struct snd_soc_dai *dai)
+{
+	struct soc_hdac_bus *sbus = dev_get_drvdata(dai->dev);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	int ret;
+
+	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
+	ret = substream_alloc_pages(sbus, substream,
+					  params_buffer_bytes(params));
+
+	if (ret < 0)
+		return ret;
+
+	if (substream->runtime->dma_area)
+		memset(substream->runtime->dma_area, 0,
+				params_buffer_bytes(params));
+
+	dev_dbg(dai->dev, "format_val, rate=%d, ch=%d, format=%d\n",
+			runtime->rate, runtime->channels, runtime->format);
+
+	return ret;
+}
+
+static void soc_hda_pcm_close(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	struct soc_hdac_stream *stream = get_hdac_stream(substream);
+	struct soc_hda_dma_params *dma_params = NULL;
+
+	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
+	snd_soc_hdac_stream_release(stream, HDAC_STREAM_TYPE_COUPLED);
+
+	dma_params  = (struct soc_hda_dma_params *)
+			snd_soc_dai_get_dma_data(dai, substream);
+
+	pm_runtime_mark_last_busy(dai->dev);
+	pm_runtime_put_autosuspend(dai->dev);
+	kfree(dma_params);
+}
+
+static int soc_hda_pcm_hw_free(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	struct soc_hdac_bus *sbus = dev_get_drvdata(dai->dev);
+	struct soc_hdac_stream *stream = get_hdac_stream(substream);
+
+	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
+
+	snd_hdac_stream_cleanup(hdac_stream(stream));
+	hdac_stream(stream)->prepared = 0;
+
+	return substream_free_pages(hdac_bus(sbus), substream);
+}
+
+static struct snd_soc_dai_ops hda_pcm_dai_ops = {
+	.startup = soc_hda_pcm_open,
+	.shutdown = soc_hda_pcm_close,
+	.prepare = soc_hda_pcm_prepare,
+	.hw_params = soc_hda_pcm_hw_params,
+	.hw_free = soc_hda_pcm_hw_free,
+};
+
+static struct snd_soc_dai_driver soc_hda_platform_dai[] = {
+{
+	.name = "System Pin",
+	.ops = &hda_pcm_dai_ops,
+	.playback = {
+		.stream_name = "System Playback",
+		.channels_min = HDA_MONO,
+		.channels_max = HDA_STEREO,
+		.rates = SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_8000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+	},
+	.capture = {
+		.stream_name = "System Capture",
+		.channels_min = HDA_MONO,
+		.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 = "Deepbuffer Pin",
+	.ops = &hda_pcm_dai_ops,
+	.playback = {
+		.stream_name = "Deepbuffer Playback",
+		.channels_min = HDA_STEREO,
+		.channels_max = HDA_STEREO,
+		.rates = SNDRV_PCM_RATE_48000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+	},
+},
+{
+	.name = "LowLatency Pin",
+	.ops = &hda_pcm_dai_ops,
+	.playback = {
+		.stream_name = "Low Latency Playback",
+		.channels_min = HDA_STEREO,
+		.channels_max = HDA_STEREO,
+		.rates = SNDRV_PCM_RATE_48000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+	},
+},
+};
+
+static int soc_hda_platform_open(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai_link *dai_link = rtd->dai_link;
+
+	dev_dbg(rtd->cpu_dai->dev, "In %s:%s\n", __func__,
+					dai_link->cpu_dai_name);
+
+	runtime = substream->runtime;
+	snd_soc_set_runtime_hwparams(substream, &azx_pcm_hw);
+	return 0;
+}
+
+static int soc_hda_platform_pcm_trigger(struct snd_pcm_substream *substream,
+					int cmd)
+{
+	struct soc_hdac_bus *sbus = get_bus_ctx(substream);
+	struct hdac_bus *bus = hdac_bus(sbus);
+	struct soc_hdac_stream *stream;
+	struct snd_pcm_substream *s;
+	bool start;
+	int sbits = 0;
+	unsigned long cookie;
+	struct hdac_stream *hstr;
+
+	stream = get_hdac_stream(substream);
+	hstr = hdac_stream(stream);
+
+	dev_dbg(bus->dev, "In %s cmd=%d\n", __func__, cmd);
+
+	if (!hstr->prepared)
+		return -EPIPE;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+	case SNDRV_PCM_TRIGGER_RESUME:
+		start = true;
+		break;
+
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_STOP:
+		start = false;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	snd_pcm_group_for_each_entry(s, substream) {
+		if (s->pcm->card != substream->pcm->card)
+			continue;
+		stream = get_hdac_stream(s);
+		sbits |= 1 << hdac_stream(stream)->index;
+		snd_pcm_trigger_done(s, substream);
+	}
+
+	spin_lock_irqsave(&bus->reg_lock, cookie);
+
+	/* first, set SYNC bits of corresponding streams */
+	snd_hdac_stream_sync_trigger(hstr, true, sbits, AZX_REG_SSYNC);
+
+	snd_pcm_group_for_each_entry(s, substream) {
+		if (s->pcm->card != substream->pcm->card)
+			continue;
+		stream = get_hdac_stream(s);
+		if (start)
+			snd_hdac_stream_start(hdac_stream(stream), true);
+		else
+			snd_hdac_stream_stop(hdac_stream(stream));
+	}
+	spin_unlock_irqrestore(&bus->reg_lock, cookie);
+
+	snd_hdac_stream_sync(hstr, start, sbits);
+
+	spin_lock_irqsave(&bus->reg_lock, cookie);
+
+	/* reset SYNC bits */
+	snd_hdac_stream_sync_trigger(hstr, false, sbits, AZX_REG_SSYNC);
+	if (start)
+		snd_hdac_stream_timecounter_init(hstr, sbits);
+	spin_unlock_irqrestore(&bus->reg_lock, cookie);
+	return 0;
+}
+
+/* calculate runtime delay from LPIB */
+static int hda_get_delay_from_lpib(struct soc_hdac_bus *sbus,
+				struct soc_hdac_stream *sstream,
+				unsigned int pos)
+{
+	struct hdac_bus *bus = hdac_bus(sbus);
+	struct hdac_stream *hstream = hdac_stream(sstream);
+	struct snd_pcm_substream *substream = hstream->substream;
+	int stream = substream->stream;
+	unsigned int lpib_pos = snd_hdac_stream_get_pos_lpib(hstream);
+	int delay;
+
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+		delay = pos - lpib_pos;
+	else
+		delay = lpib_pos - pos;
+	if (delay < 0) {
+		if (delay >= hstream->delay_negative_threshold)
+			delay = 0;
+		else
+			delay += hstream->bufsize;
+	}
+
+	if (delay >= hstream->period_bytes) {
+		dev_info(bus->dev,
+			 "Unstable LPIB (%d >= %d); disabling LPIB delay counting\n",
+			 delay, hstream->period_bytes);
+		delay = 0;
+	}
+
+	return bytes_to_frames(substream->runtime, delay);
+}
+
+static unsigned int hda_get_position(struct soc_hdac_stream *hstream,
+					int codec_delay)
+{
+	struct hdac_stream *hstr = hdac_stream(hstream);
+	struct snd_pcm_substream *substream = hstr->substream;
+	struct soc_hdac_bus *sbus = get_bus_ctx(substream);
+	unsigned int pos;
+	int delay = 0;
+
+	/* use the position buffer as default */
+	pos = snd_hdac_stream_get_pos_posbuf(hdac_stream(hstream));
+
+	if (pos >= hdac_stream(hstream)->bufsize)
+		pos = 0;
+
+	if (substream->runtime) {
+		delay = hda_get_delay_from_lpib(sbus, hstream, pos)
+						 + codec_delay;
+		substream->runtime->delay += delay;
+	}
+
+	return pos;
+}
+
+static snd_pcm_uframes_t soc_hda_platform_pcm_pointer
+			(struct snd_pcm_substream *substream)
+{
+	struct soc_hdac_stream *hstream = get_hdac_stream(substream);
+
+	return bytes_to_frames(substream->runtime,
+			       hda_get_position(hstream, 0));
+}
+
+static u64 soc_azx_adjust_codec_delay(struct snd_pcm_substream *substream,
+				u64 nsec)
+{
+	struct snd_soc_pcm_runtime *rtd = snd_pcm_substream_chip(substream);
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	u64 codec_frames, codec_nsecs;
+
+	if (!codec_dai->driver->ops->delay)
+		return nsec;
+
+	codec_frames = codec_dai->driver->ops->delay(substream, codec_dai);
+	codec_nsecs = div_u64(codec_frames * 1000000000LL,
+			      substream->runtime->rate);
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+		return nsec + codec_nsecs;
+
+	return (nsec > codec_nsecs) ? nsec - codec_nsecs : 0;
+}
+
+static int soc_hda_get_time_info(struct snd_pcm_substream *substream,
+			struct timespec *system_ts, struct timespec *audio_ts,
+			struct snd_pcm_audio_tstamp_config *audio_tstamp_config,
+			struct snd_pcm_audio_tstamp_report *audio_tstamp_report)
+{
+	struct soc_hdac_stream *sstream = get_hdac_stream(substream);
+	struct hdac_stream *hstr = hdac_stream(sstream);
+	u64 nsec;
+
+	if ((substream->runtime->hw.info & SNDRV_PCM_INFO_HAS_LINK_ATIME) &&
+		(audio_tstamp_config->type_requested == SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK)) {
+
+		snd_pcm_gettime(substream->runtime, system_ts);
+
+		nsec = timecounter_read(&hstr->tc);
+		nsec = div_u64(nsec, 3); /* can be optimized */
+		if (audio_tstamp_config->report_delay)
+			nsec = soc_azx_adjust_codec_delay(substream, nsec);
+
+		*audio_ts = ns_to_timespec(nsec);
+
+		audio_tstamp_report->actual_type = SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK;
+		audio_tstamp_report->accuracy_report = 1; /* rest of struct is valid */
+		audio_tstamp_report->accuracy = 42; /* 24MHzWallClk == 42ns resolution */
+
+	} else
+		audio_tstamp_report->actual_type = SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT;
+
+	return 0;
+}
+
+static struct snd_pcm_ops soc_hda_platform_ops = {
+	.open = soc_hda_platform_open,
+	.ioctl = snd_pcm_lib_ioctl,
+	.trigger = soc_hda_platform_pcm_trigger,
+	.pointer = soc_hda_platform_pcm_pointer,
+	.get_time_info =  soc_hda_get_time_info,
+	.mmap = snd_pcm_lib_default_mmap,
+	.page = snd_pcm_sgbuf_ops_page,
+};
+
+static void soc_hda_pcm_free(struct snd_pcm *pcm)
+{
+	snd_pcm_lib_preallocate_free_for_all(pcm);
+}
+
+#define MAX_PREALLOC_SIZE	(32 * 1024 * 1024)
+
+static int soc_hda_pcm_new(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_dai *dai = rtd->cpu_dai;
+	struct soc_hdac_bus *sbus = dev_get_drvdata(dai->dev);
+	struct snd_pcm *pcm = rtd->pcm;
+	unsigned int size;
+	int retval = 0;
+	struct hda_skl *hda = to_hda_skl(sbus);
+
+	dev_dbg(dai->dev, "In %s\n", __func__);
+	if (dai->driver->playback.channels_min ||
+		dai->driver->capture.channels_min) {
+		/* buffer pre-allocation */
+		size = CONFIG_SND_SOC_HDA_PREALLOC_SIZE * 1024;
+		if (size > MAX_PREALLOC_SIZE)
+			size = MAX_PREALLOC_SIZE;
+		retval = snd_pcm_lib_preallocate_pages_for_all(pcm,
+						SNDRV_DMA_TYPE_DEV_SG,
+						snd_dma_pci_data(hda->pci),
+						size, MAX_PREALLOC_SIZE);
+		if (retval) {
+			dev_err(dai->dev, "dma buffer allocationf fail\n");
+			return retval;
+		}
+	}
+	return retval;
+}
+
+static struct snd_soc_platform_driver soc_hda_platform_drv  = {
+	.ops		= &soc_hda_platform_ops,
+	.pcm_new	= soc_hda_pcm_new,
+	.pcm_free	= soc_hda_pcm_free,
+};
+
+static const struct snd_soc_component_driver soc_hda_component = {
+	.name           = "pcm",
+};
+
+int soc_hda_platform_register(struct device *dev)
+{
+	int ret;
+
+	ret = snd_soc_register_platform(dev, &soc_hda_platform_drv);
+	if (ret) {
+		dev_err(dev, "soc platform registration failed %d\n", ret);
+		return ret;
+	}
+	ret = snd_soc_register_component(dev, &soc_hda_component,
+				soc_hda_platform_dai,
+				ARRAY_SIZE(soc_hda_platform_dai));
+	if (ret) {
+		dev_err(dev, "soc component registration failed %d\n", ret);
+		snd_soc_unregister_platform(dev);
+	}
+
+	return ret;
+
+}
+
+int soc_hda_platform_unregister(struct device *dev)
+{
+	snd_soc_unregister_component(dev);
+	snd_soc_unregister_platform(dev);
+	return 0;
+}
-- 
1.9.1

[PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Vinod Koul]

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

This patch follows up by adding the HDA controller operations. This code is
mostly derived from Intel HDA PCI driver without legacy bits

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/hda-skl.c | 649 ++++++++++++++++++++++++++++++++++++++
 sound/soc/intel/skylake/hda-skl.h |  73 +++++
 2 files changed, 722 insertions(+)
 create mode 100644 sound/soc/intel/skylake/hda-skl.c
 create mode 100644 sound/soc/intel/skylake/hda-skl.h

diff --git a/sound/soc/intel/skylake/hda-skl.c b/sound/soc/intel/skylake/hda-skl.c
new file mode 100644
index 000000000000..3a8e6f702bac
--- /dev/null
+++ b/sound/soc/intel/skylake/hda-skl.c
@@ -0,0 +1,649 @@
+/*
+ *  hda-skl.c - Implementation of primary ASoC Intel HD Audio driver
+ *
+ *  Copyright (C) 2014-2015 Intel Corp
+ *  Author: Jeeja KP <jeeja.kp@intel.com>
+ *
+ *  Derived mostly from Intel HDA driver with following copyrights:
+ *  Copyright (c) 2004 Takashi Iwai <tiwai@suse.de>
+ *                     PeiSen Hou <pshou@realtek.com.tw>
+ *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  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/delay.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/pci.h>
+#include <linux/mutex.h>
+#include <linux/io.h>
+#include <linux/pm_runtime.h>
+
+#include <linux/platform_device.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/hda_register.h>
+#include <sound/hdaudio.h>
+#include "hda-skl.h"
+
+/*
+ * initialize the PCI registers
+ */
+static void update_pci_byte(struct pci_dev *pci, unsigned int reg,
+			    unsigned char mask, unsigned char val)
+{
+	unsigned char data;
+
+	pci_read_config_byte(pci, reg, &data);
+	data &= ~mask;
+	data |= (val & mask);
+	pci_write_config_byte(pci, reg, data);
+}
+
+static void azx_init_pci(struct hda_skl *hda)
+{
+	struct soc_hdac_bus *sbus = &hda->sbus;
+
+	/*
+	 * Clear bits 0-2 of PCI register TCSEL (at offset 0x44)
+	 * TCSEL == Traffic Class Select Register, which sets PCI express QOS
+	 * Ensuring these bits are 0 clears playback static on some HD Audio
+	 * codecs.
+	 * The PCI register TCSEL is defined in the Intel manuals.
+	 */
+	dev_dbg(hdac_bus(sbus)->dev, "Clearing TCSEL\n");
+	update_pci_byte(hda->pci, AZX_PCIREG_TCSEL, 0x07, 0);
+}
+
+/* called from IRQ */
+static void stream_update(struct hdac_bus *bus, struct hdac_stream *hstr)
+{
+	snd_pcm_period_elapsed(hstr->substream);
+}
+
+static irqreturn_t azx_interrupt(int irq, void *dev_id)
+{
+	struct soc_hdac_bus *sbus = dev_id;
+	struct hdac_bus *bus = hdac_bus(sbus);
+	u32 status;
+
+#ifdef CONFIG_PM
+	if (!pm_runtime_active(bus->dev))
+		return IRQ_NONE;
+#endif
+
+	spin_lock(&bus->reg_lock);
+
+	status = snd_hdac_chip_readl(bus, INTSTS);
+	if (status == 0 || status == 0xffffffff) {
+		spin_unlock(&bus->reg_lock);
+		return IRQ_NONE;
+	}
+
+	/* clear rirb int */
+	status = snd_hdac_chip_readb(bus, RIRBSTS);
+	if (status & RIRB_INT_MASK) {
+		if (status & RIRB_INT_RESPONSE)
+			snd_hdac_bus_update_rirb(bus);
+		snd_hdac_chip_writeb(bus, RIRBSTS, RIRB_INT_MASK);
+	}
+
+	spin_unlock(&bus->reg_lock);
+
+	return snd_hdac_chip_readl(bus, INTSTS) ? IRQ_WAKE_THREAD : IRQ_HANDLED;
+}
+
+static irqreturn_t azx_threaded_handler(int irq, void *dev_id)
+{
+	struct soc_hdac_bus *sbus = dev_id;
+	struct hdac_bus *bus = hdac_bus(sbus);
+	u32 status;
+	unsigned long cookie;
+
+	status = snd_hdac_chip_readl(bus, INTSTS);
+	spin_lock_irqsave(&bus->reg_lock, cookie);
+
+	snd_hdac_bus_handle_stream_irq(bus, status, stream_update);
+
+	spin_unlock_irqrestore(&bus->reg_lock, cookie);
+
+	return IRQ_HANDLED;
+}
+
+/* initialize SD streams, use seprate streeam tag for PB and CP */
+static int azx_init_stream(struct soc_hdac_bus *sbus, int num_stream, int dir)
+{
+	int i, tag;
+	int stream_tag = 0;
+
+	for (i = 0; i < num_stream; i++) {
+		struct soc_hdac_stream *stream =
+				kzalloc(sizeof(*stream), GFP_KERNEL);
+		if (!stream)
+			return -ENOMEM;
+		tag = ++stream_tag;
+		snd_soc_hdac_stream_init(sbus, stream, i, dir, tag);
+	}
+	return 0;
+}
+
+static void azx_free_streams(struct soc_hdac_bus *sbus)
+{
+	struct hdac_stream *s;
+	struct soc_hdac_stream *stream;
+	struct hdac_bus *bus = hdac_bus(sbus);
+
+	while (!list_empty(&bus->stream_list)) {
+		s = list_first_entry(&bus->stream_list, struct hdac_stream, list);
+		stream = stream_to_soc_hdac_stream(s);
+		list_del(&s->list);
+		kfree(stream);
+	}
+}
+
+static void azx_free_hda_links(struct soc_hdac_bus *sbus)
+{
+	struct soc_hdac_link *l;
+
+	while (!list_empty(&sbus->hlink_list)) {
+		l = list_first_entry(&sbus->hlink_list, struct soc_hdac_link, list);
+		list_del(&l->list);
+		kfree(l);
+	}
+}
+
+static int azx_acquire_irq(struct soc_hdac_bus *sbus, int do_disconnect)
+{
+	struct hda_skl *hda = to_hda_skl(sbus);
+	struct hdac_bus *bus = hdac_bus(sbus);
+	int ret = 0;
+
+	ret = request_threaded_irq(hda->pci->irq, azx_interrupt,
+			azx_threaded_handler,
+			hda->msi ? 0 : IRQF_SHARED,
+			KBUILD_MODNAME, sbus);
+	if (ret) {
+		dev_err(bus->dev,
+			"unable to grab IRQ %d, disabling device\n",
+			hda->pci->irq);
+		return ret;
+	}
+
+	bus->irq = hda->pci->irq;
+	pci_intx(hda->pci, !hda->msi);
+	return ret;
+}
+
+#ifdef CONFIG_PM_SLEEP
+/*
+ * power management
+ */
+static int azx_suspend(struct device *dev)
+{
+	struct pci_dev *pci = to_pci_dev(dev);
+	struct soc_hdac_bus *sbus = pci_get_drvdata(pci);
+	struct hdac_bus *bus = hdac_bus(sbus);
+	struct hda_skl *hda = to_hda_skl(sbus);
+
+	snd_hdac_bus_stop_chip(bus);
+	snd_hdac_bus_enter_link_reset(bus);
+	if (bus->irq >= 0) {
+		free_irq(bus->irq, bus);
+		bus->irq = -1;
+	}
+
+	if (hda->msi)
+		pci_disable_msi(pci);
+	return 0;
+}
+
+static int azx_resume(struct device *dev)
+{
+	struct pci_dev *pci = to_pci_dev(dev);
+	struct soc_hdac_bus *sbus = pci_get_drvdata(pci);
+	struct hdac_bus *bus = hdac_bus(sbus);
+	struct hda_skl *hda = to_hda_skl(sbus);
+
+	if (hda->msi)
+		if (pci_enable_msi(pci) < 0)
+			hda->msi = 0;
+	if (azx_acquire_irq(sbus, 1) < 0)
+		return -EIO;
+	azx_init_pci(hda);
+
+	snd_hdac_bus_init_chip(bus, 1);
+	return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_PM
+static int azx_runtime_suspend(struct device *dev)
+{
+	struct pci_dev *pci = to_pci_dev(dev);
+	struct soc_hdac_bus *sbus = pci_get_drvdata(pci);
+	struct hdac_bus *bus = hdac_bus(sbus);
+
+	dev_dbg(bus->dev, "in %s\n", __func__);
+
+	/* enable controller wake up event */
+	snd_hdac_chip_updatew(bus, WAKEEN, 0, STATESTS_INT_MASK);
+
+	snd_hdac_bus_stop_chip(bus);
+	snd_hdac_bus_enter_link_reset(bus);
+	return 0;
+}
+
+static int azx_runtime_resume(struct device *dev)
+{
+	struct pci_dev *pci = to_pci_dev(dev);
+	struct soc_hdac_bus *sbus = pci_get_drvdata(pci);
+	struct hdac_bus *bus = hdac_bus(sbus);
+	struct hda_skl *hda = to_hda_skl(sbus);
+	int status;
+
+	dev_dbg(bus->dev, "in %s\n", __func__);
+
+	/* Read STATESTS before controller reset */
+	status = snd_hdac_chip_readw(bus, STATESTS);
+
+	azx_init_pci(hda);
+	snd_hdac_bus_init_chip(bus, true);
+	/* disable controller Wake Up event */
+	snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, 0);
+	return 0;
+}
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops azx_pm = {
+	SET_SYSTEM_SLEEP_PM_OPS(azx_suspend, azx_resume)
+	SET_RUNTIME_PM_OPS(azx_runtime_suspend, azx_runtime_resume, NULL)
+};
+
+/*
+ * destructor
+ */
+static int azx_free(struct soc_hdac_bus *sbus)
+{
+	struct hda_skl *hda = to_hda_skl(sbus);
+	struct hdac_bus *bus = hdac_bus(sbus);
+
+	hda->init_failed = 1; /* to be sure */
+
+	snd_soc_hdac_stop_streams(sbus);
+
+	if (bus->irq >= 0)
+		free_irq(bus->irq, (void *)bus);
+	if (hda->msi)
+		pci_disable_msi(hda->pci);
+	if (bus->remap_addr)
+		iounmap(bus->remap_addr);
+
+	snd_hdac_bus_free_stream_pages(bus);
+	azx_free_streams(sbus);
+	azx_free_hda_links(sbus);
+	pci_release_regions(hda->pci);
+	pci_disable_device(hda->pci);
+
+	snd_soc_hdac_bus_exit(sbus);
+	return 0;
+}
+
+static int hda_dmic_device_register(struct hda_skl *hda)
+{
+	struct hdac_bus *bus = hdac_bus(&hda->sbus);
+	struct platform_device *pdev;
+	int ret;
+
+	pdev = platform_device_alloc("dmic-codec", -1);
+	if (!pdev) {
+		dev_err(bus->dev, "failed to allocate dmic device\n");
+		return -1;
+	}
+
+	ret = platform_device_add(pdev);
+	if (ret) {
+		dev_err(bus->dev, "failed to add hda codec device\n");
+		platform_device_put(pdev);
+		return -1;
+	}
+	hda->dmic_dev = pdev;
+	return 0;
+}
+
+static void hda_dmic_device_unregister(struct hda_skl *hda)
+{
+
+	if (hda->dmic_dev)
+		platform_device_unregister(hda->dmic_dev);
+}
+
+/*
+ * Probe the given codec address
+ */
+static int probe_codec(struct soc_hdac_bus *sbus, int addr)
+{
+	struct hdac_bus *bus = hdac_bus(sbus);
+	unsigned int cmd = (addr << 28) | (AC_NODE_ROOT << 20) |
+		(AC_VERB_PARAMETERS << 8) | AC_PAR_VENDOR_ID;
+	unsigned int res;
+
+	mutex_lock(&bus->cmd_mutex);
+	snd_hdac_bus_send_cmd(bus, cmd);
+	snd_hdac_bus_get_response(bus, addr, &res);
+	mutex_unlock(&bus->cmd_mutex);
+	if (res == -1)
+		return -EIO;
+	dev_dbg(bus->dev, "codec #%d probed OK\n", addr);
+	return snd_soc_hdac_bus_device_init(sbus, addr);
+}
+
+/* Codec initialization */
+static int azx_codec_create(struct soc_hdac_bus *sbus)
+{
+	struct hdac_bus *bus = hdac_bus(sbus);
+	int c, max_slots;
+
+	max_slots = HDA_MAX_CODECS;
+
+	/* First try to probe all given codec slots */
+	for (c = 0; c < max_slots; c++) {
+		if ((bus->codec_mask & (1 << c))) {
+			if (probe_codec(sbus, c) < 0) {
+				/* Some BIOSen give you wrong codec addresses
+				 * that don't exist
+				 */
+				dev_warn(bus->dev,
+					 "Codec #%d probe error; disabling it...\n", c);
+				bus->codec_mask &= ~(1 << c);
+				/* More badly, accessing to a non-existing
+				 * codec often screws up the controller bus,
+				 * and disturbs the further communications.
+				 * Thus if an error occurs during probing,
+				 * better to reset the controller bus to
+				 * get back to the sanity state.
+				 */
+				snd_hdac_bus_stop_chip(bus);
+				snd_hdac_bus_init_chip(bus, true);
+			}
+		}
+	}
+	return 0;
+}
+
+static const struct hdac_bus_ops bus_core_ops = {
+	.command = snd_hdac_bus_send_cmd,
+	.get_response = snd_hdac_bus_get_response,
+};
+
+/*
+ * constructor
+ */
+static int azx_create(struct pci_dev *pci,
+		      const struct hdac_io_ops *io_ops,
+		      struct hda_skl **rhda)
+{
+	struct hda_skl *hda;
+	struct soc_hdac_bus *sbus;
+
+	int err;
+
+	*rhda = NULL;
+
+	err = pci_enable_device(pci);
+	if (err < 0)
+		return err;
+
+	hda = devm_kzalloc(&pci->dev, sizeof(*hda), GFP_KERNEL);
+	if (!hda) {
+		pci_disable_device(pci);
+		return -ENOMEM;
+	}
+	sbus = &hda->sbus;
+	snd_soc_hdac_bus_init(sbus, &pci->dev, &bus_core_ops, io_ops);
+	sbus->bus.use_posbuf = 1;
+	hda->pci = pci;
+	hda->msi = 1;
+
+	sbus->bus.bdl_pos_adj = 0;
+
+	*rhda = hda;
+	return 0;
+}
+
+static int azx_first_init(struct soc_hdac_bus *sbus)
+{
+	struct hda_skl *hda = to_hda_skl(sbus);
+	struct hdac_bus *bus = hdac_bus(sbus);
+	struct pci_dev *pci = hda->pci;
+	int err;
+	unsigned short gcap;
+	int capture_streams, playback_streams;
+
+	err = pci_request_regions(pci, "Skylake HD audio");
+	if (err < 0)
+		return err;
+
+	bus->addr = pci_resource_start(pci, 0);
+	bus->remap_addr = pci_ioremap_bar(pci, 0);
+	if (bus->remap_addr == NULL) {
+		dev_err(bus->dev, "ioremap error\n");
+		return -ENXIO;
+	}
+
+	if (hda->msi)
+		if (pci_enable_msi(pci) < 0)
+			hda->msi = 0;
+
+	if (azx_acquire_irq(sbus, 0) < 0)
+		return -EBUSY;
+
+	pci_set_master(pci);
+	synchronize_irq(bus->irq);
+
+	gcap = snd_hdac_chip_readw(bus, GCAP);
+	dev_dbg(bus->dev, "chipset global capabilities = 0x%x\n", gcap);
+
+	/* allow 64bit DMA address if supported by H/W */
+	if (!dma_set_mask(bus->dev, DMA_BIT_MASK(64)))
+		dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(64));
+	else {
+		dma_set_mask(bus->dev, DMA_BIT_MASK(32));
+		dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(32));
+	}
+
+	/* read number of streams from GCAP register instead of using
+	 * hardcoded value
+	 */
+	capture_streams = (gcap >> 8) & 0x0f;
+	playback_streams = (gcap >> 12) & 0x0f;
+	if (!playback_streams && !capture_streams)
+		return -EIO;
+	sbus->num_streams = capture_streams + playback_streams;
+	/* initialize streams */
+	azx_init_stream(sbus, capture_streams, SNDRV_PCM_STREAM_CAPTURE);
+	azx_init_stream(sbus, playback_streams, SNDRV_PCM_STREAM_PLAYBACK);
+
+	err = snd_hdac_bus_alloc_stream_pages(bus);
+	if (err < 0)
+		return err;
+
+	/* initialize chip */
+	azx_init_pci(hda);
+
+	snd_hdac_bus_init_chip(bus, true);
+
+	/* codec detection */
+	if (!bus->codec_mask) {
+		dev_err(bus->dev, "no codecs found!\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+/* PCI register access. */
+static void pci_azx_writel(u32 value, u32 __iomem *addr)
+{
+	writel(value, addr);
+}
+
+static u32 pci_azx_readl(u32 __iomem *addr)
+{
+	return readl(addr);
+}
+
+static void pci_azx_writew(u16 value, u16 __iomem *addr)
+{
+	writew(value, addr);
+}
+
+static u16 pci_azx_readw(u16 __iomem *addr)
+{
+	return readw(addr);
+}
+
+static void pci_azx_writeb(u8 value, u8 __iomem *addr)
+{
+	writeb(value, addr);
+}
+
+static u8 pci_azx_readb(u8 __iomem *addr)
+{
+	return readb(addr);
+}
+
+/* DMA page allocation helpers.  */
+static int dma_alloc_pages(struct hdac_bus *bus,
+			   int type,
+			   size_t size,
+			   struct snd_dma_buffer *buf)
+{
+	int err;
+
+	err = snd_dma_alloc_pages(type,
+				  bus->dev,
+				  size, buf);
+	if (err < 0)
+		return err;
+	return 0;
+}
+
+static void dma_free_pages(struct hdac_bus *bus, struct snd_dma_buffer *buf)
+{
+	snd_dma_free_pages(buf);
+}
+
+static const struct hdac_io_ops hda_io_ops = {
+	.reg_writel = pci_azx_writel,
+	.reg_readl = pci_azx_readl,
+	.reg_writew = pci_azx_writew,
+	.reg_readw = pci_azx_readw,
+	.reg_writeb = pci_azx_writeb,
+	.reg_readb = pci_azx_readb,
+	.dma_alloc_pages = dma_alloc_pages,
+	.dma_free_pages = dma_free_pages,
+};
+
+static int azx_probe(struct pci_dev *pci,
+		     const struct pci_device_id *pci_id)
+{
+	struct hda_skl *hda;
+	struct soc_hdac_bus *sbus = NULL;
+	struct hdac_bus *bus = NULL;
+	int err;
+
+	err = azx_create(pci, &hda_io_ops, &hda);
+	if (err < 0)
+		return err;
+
+	sbus = &hda->sbus;
+	bus = hdac_bus(sbus);
+
+	err = azx_first_init(sbus);
+	if (err < 0)
+		goto out_free;
+
+	/*create device for soc dmic*/
+	err = hda_dmic_device_register(hda);
+	if (err < 0)
+		goto out_free;
+
+	/* register platform dai and controls */
+	err = soc_hda_platform_register(bus->dev);
+	if (err < 0)
+		goto out_dmic_free;
+	/* create codec instances */
+	err = azx_codec_create(sbus);
+	if (err < 0)
+		goto out_unregister;
+
+	pci_set_drvdata(hda->pci, sbus);
+
+	/*configure PM */
+	pm_runtime_set_autosuspend_delay(bus->dev, HDA_SKL_SUSPEND_DELAY);
+	pm_runtime_use_autosuspend(bus->dev);
+	pm_runtime_put_noidle(bus->dev);
+	pm_runtime_allow(bus->dev);
+
+	pci_set_drvdata(hda->pci, sbus);
+	return 0;
+
+out_unregister:
+	soc_hda_platform_unregister(bus->dev);
+out_dmic_free:
+	hda_dmic_device_unregister(hda);
+out_free:
+	hda->init_failed = 1;
+	azx_free(sbus);
+	pci_set_drvdata(hda->pci, NULL);
+	return err;
+}
+
+static void azx_remove(struct pci_dev *pci)
+{
+	struct soc_hdac_bus *sbus = pci_get_drvdata(pci);
+	struct hda_skl *hda = to_hda_skl(sbus);
+
+	if (pci_dev_run_wake(pci))
+		pm_runtime_get_noresume(&pci->dev);
+	pci_dev_put(pci);
+	soc_hda_platform_unregister(&pci->dev);
+	hda_dmic_device_unregister(hda);
+	azx_free(sbus);
+	dev_set_drvdata(&pci->dev, NULL);
+}
+
+/* PCI IDs */
+static const struct pci_device_id azx_ids[] = {
+	/* Sunrise Point-LP */
+	{ PCI_DEVICE(0x8086, 0x9d70), 0},
+	{ 0, }
+};
+MODULE_DEVICE_TABLE(pci, azx_ids);
+
+/* pci_driver definition */
+static struct pci_driver azx_driver = {
+	.name = KBUILD_MODNAME,
+	.id_table = azx_ids,
+	.probe = azx_probe,
+	.remove = azx_remove,
+	.driver = {
+		.pm = &azx_pm,
+	},
+};
+module_pci_driver(azx_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Intel Skylake ASoC HDA driver");
diff --git a/sound/soc/intel/skylake/hda-skl.h b/sound/soc/intel/skylake/hda-skl.h
new file mode 100644
index 000000000000..ed6ccd31b198
--- /dev/null
+++ b/sound/soc/intel/skylake/hda-skl.h
@@ -0,0 +1,73 @@
+/*
+ *  hda-skl.h - HD Audio skylake defintions.
+ *
+ *  Copyright (C) 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 published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  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.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ */
+
+#ifndef __SOUND_SOC_HDA_SKL_H
+#define __SOUND_SOC_HDA_SKL_H
+
+#include <sound/core.h>
+#include <sound/initval.h>
+#include <sound/hda_register.h>
+#include <sound/hdaudio.h>
+#include <sound/soc-hdaudio.h>
+
+#define HDA_SKL_SUSPEND_DELAY 2000
+
+/* Vendor Specific Registers */
+#define AZX_REG_VS_EM1			0x1000
+#define AZX_REG_VS_INRC			0x1004
+#define AZX_REG_VS_OUTRC		0x1008
+#define AZX_REG_VS_FIFOTRK		0x100C
+#define AZX_REG_VS_FIFOTRK2		0x1010
+#define AZX_REG_VS_EM2			0x1030
+#define AZX_REG_VS_EM3L			0x1038
+#define AZX_REG_VS_EM3U			0x103C
+#define AZX_REG_VS_EM4L			0x1040
+#define AZX_REG_VS_EM4U			0x1044
+#define AZX_REG_VS_LTRC			0x1048
+#define AZX_REG_VS_D0I3C		0x104A
+#define AZX_REG_VS_PCE			0x104B
+#define AZX_REG_VS_L2MAGC		0x1050
+#define AZX_REG_VS_L2LAHPT		0x1054
+#define AZX_REG_VS_SDXDPIB_XBASE	0x1084
+#define AZX_REG_VS_SDXDPIB_XINTERVAL	0x20
+#define AZX_REG_VS_SDXEFIFOS_XBASE	0x1094
+#define AZX_REG_VS_SDXEFIFOS_XINTERVAL	0x20
+struct hda_skl {
+	struct soc_hdac_bus sbus;
+	struct pci_dev *pci;
+
+	unsigned int init_failed:1; /* delayed init failed */
+	unsigned int msi:1;
+	struct platform_device *dmic_dev;
+};
+
+#define soc_hdac_bus(s)	(&(s)->sbus)
+#define to_hda_skl(sbus) \
+	container_of(sbus, struct hda_skl, sbus)
+
+/* to pass dai dma data */
+struct soc_hda_dma_params {
+	u32 format;
+	u8 stream_tag;
+};
+
+int soc_hda_platform_unregister(struct device *dev);
+int soc_hda_platform_register(struct device *dev);
+#endif /* __SOUND_SOC_HDA_SKL_H */
-- 
1.9.1

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Mark Brown]

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

On Mon, May 11, 2015 at 04:24:03PM +0530, Vinod Koul wrote:

> +static void azx_free_streams(struct soc_hdac_bus *sbus)
> +{
> +	struct hdac_stream *s;
> +	struct soc_hdac_stream *stream;
> +	struct hdac_bus *bus = hdac_bus(sbus);
> +
> +	while (!list_empty(&bus->stream_list)) {
> +		s = list_first_entry(&bus->stream_list, struct hdac_stream, list);
> +		stream = stream_to_soc_hdac_stream(s);
> +		list_del(&s->list);
> +		kfree(stream);
> +	}
> +}

Why is this (and the equivalent link function) device specific code?
They look very generic...

> +static int azx_acquire_irq(struct soc_hdac_bus *sbus, int do_disconnect)
> +{
> +	struct hda_skl *hda = to_hda_skl(sbus);
> +	struct hdac_bus *bus = hdac_bus(sbus);
> +	int ret = 0;
> +
> +	ret = request_threaded_irq(hda->pci->irq, azx_interrupt,
> +			azx_threaded_handler,
> +			hda->msi ? 0 : IRQF_SHARED,
> +			KBUILD_MODNAME, sbus);

Why not just always request the interrupt as shared - we don't seem to
care in the interrupt handling code?

> +/* PCI register access. */
> +static void pci_azx_writel(u32 value, u32 __iomem *addr)
> +{
> +	writel(value, addr);
> +}

These wrappers are setting off alarm bells - why can't we just use the
called functions directly, and given the parameters (which have just a
raw pointer and value) what else could the implementation be?

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

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

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Takashi Iwai]

At Fri, 29 May 2015 18:41:15 +0100,
Mark Brown wrote:
> 
> > +/* PCI register access. */
> > +static void pci_azx_writel(u32 value, u32 __iomem *addr)
> > +{
> > +	writel(value, addr);
> > +}
> 
> These wrappers are setting off alarm bells - why can't we just use the
> called functions directly, and given the parameters (which have just a
> raw pointer and value) what else could the implementation be?

Tegra has no direct byte access but only aligned 32bit accesses.
That's why we have the redirection.


Takashi

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Mark Brown]

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

On Fri, May 29, 2015 at 08:25:01PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:

> > > +/* PCI register access. */
> > > +static void pci_azx_writel(u32 value, u32 __iomem *addr)
> > > +{
> > > +	writel(value, addr);
> > > +}

> > These wrappers are setting off alarm bells - why can't we just use the
> > called functions directly, and given the parameters (which have just a
> > raw pointer and value) what else could the implementation be?

> Tegra has no direct byte access but only aligned 32bit accesses.
> That's why we have the redirection.

Ugh, and the functions are macros so can't be used directly.  I'd still
expect to see these ops be defined in some central place and reused.

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

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

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Takashi Iwai]

At Tue, 2 Jun 2015 11:45:16 +0100,
Mark Brown wrote:
> 
> On Fri, May 29, 2015 at 08:25:01PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > > +/* PCI register access. */
> > > > +static void pci_azx_writel(u32 value, u32 __iomem *addr)
> > > > +{
> > > > +	writel(value, addr);
> > > > +}
> 
> > > These wrappers are setting off alarm bells - why can't we just use the
> > > called functions directly, and given the parameters (which have just a
> > > raw pointer and value) what else could the implementation be?
> 
> > Tegra has no direct byte access but only aligned 32bit accesses.
> > That's why we have the redirection.
> 
> Ugh, and the functions are macros so can't be used directly.  I'd still
> expect to see these ops be defined in some central place and reused.

Maybe we can lift up some time later, but it's nothing more than an
optimization, so in a low priority for now.


Takashi

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Mark Brown]

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

On Tue, Jun 02, 2015 at 12:53:00PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:

> > Ugh, and the functions are macros so can't be used directly.  I'd still
> > expect to see these ops be defined in some central place and reused.

> Maybe we can lift up some time later, but it's nothing more than an
> optimization, so in a low priority for now.

It's not like it's hard to do and given that my concerns around this
series mostly center around code reuse and abstraction I'd rather not
have big warning signs type issues sitting there obscuring other things.

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

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

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Takashi Iwai]

At Tue, 2 Jun 2015 12:07:32 +0100,
Mark Brown wrote:
> 
> On Tue, Jun 02, 2015 at 12:53:00PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > Ugh, and the functions are macros so can't be used directly.  I'd still
> > > expect to see these ops be defined in some central place and reused.
> 
> > Maybe we can lift up some time later, but it's nothing more than an
> > optimization, so in a low priority for now.
> 
> It's not like it's hard to do and given that my concerns around this
> series mostly center around code reuse and abstraction I'd rather not
> have big warning signs type issues sitting there obscuring other things.

Well, the page allocator ops of snd-hda-intel have workarounds for
some chips that need the cache type changes of allocated pages.
Tegra and SKL don't need such mess.

Of course, we may split ops again to several pieces, but it doesn't
look good, either.  Or, a bit more code refactoring is needed; use the
standard allocator but apply the post-process and pre-process for
allocation and release of pages to fiddle with the memory caches on
demand.


Takashi

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Vinod Koul]

On Tue, Jun 02, 2015 at 12:07:32PM +0100, Mark Brown wrote:
> On Tue, Jun 02, 2015 at 12:53:00PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > Ugh, and the functions are macros so can't be used directly.  I'd still
> > > expect to see these ops be defined in some central place and reused.
> 
> > Maybe we can lift up some time later, but it's nothing more than an
> > optimization, so in a low priority for now.
> 
> It's not like it's hard to do and given that my concerns around this
> series mostly center around code reuse and abstraction I'd rather not
> have big warning signs type issues sitting there obscuring other things.
well these are read/write to registers, for SKL hidden behind writel, but
in code do help in providing functionality of reading/writing bytes/words

This is fairly *common* in drivers to use wrappers for hw accesses and yes
for overall HDA we can perhaps optimize these as Takashi pointed out but
hardly a worrying aspect/sign

Thanks
-- 
~Vinod

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Mark Brown]

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

On Tue, Jun 02, 2015 at 06:09:43PM +0530, Vinod Koul wrote:

> This is fairly *common* in drivers to use wrappers for hw accesses and yes
> for overall HDA we can perhaps optimize these as Takashi pointed out but
> hardly a worrying aspect/sign

It's not the fact that there are wrappers that is worrying, it's the
contents of the wrappers combined with the interfaces.  The interfaces
make the wrappers complete noops yet the driver has to manually go
through and rewrap everything.

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

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

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Vinod Koul]

On Fri, May 29, 2015 at 06:41:15PM +0100, Mark Brown wrote:
> On Mon, May 11, 2015 at 04:24:03PM +0530, Vinod Koul wrote:
> 
> > +static void azx_free_streams(struct soc_hdac_bus *sbus)
> > +{
> > +	struct hdac_stream *s;
> > +	struct soc_hdac_stream *stream;
> > +	struct hdac_bus *bus = hdac_bus(sbus);
> > +
> > +	while (!list_empty(&bus->stream_list)) {
> > +		s = list_first_entry(&bus->stream_list, struct hdac_stream, list);
> > +		stream = stream_to_soc_hdac_stream(s);
> > +		list_del(&s->list);
> > +		kfree(stream);
> > +	}
> > +}
> 
> Why is this (and the equivalent link function) device specific code?
> They look very generic...
In HDA we will have only one stream, but here we can have multiple links so
need to free each of them.
Also if you notice most of the code in this patch uses hdac_xxx helpers to
do the actual job

> 
> > +static int azx_acquire_irq(struct soc_hdac_bus *sbus, int do_disconnect)
> > +{
> > +	struct hda_skl *hda = to_hda_skl(sbus);
> > +	struct hdac_bus *bus = hdac_bus(sbus);
> > +	int ret = 0;
> > +
> > +	ret = request_threaded_irq(hda->pci->irq, azx_interrupt,
> > +			azx_threaded_handler,
> > +			hda->msi ? 0 : IRQF_SHARED,
> > +			KBUILD_MODNAME, sbus);
> 
> Why not just always request the interrupt as shared - we don't seem to
> care in the interrupt handling code?
I think we can move this to shared only

-- 
~Vinod

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Takashi Iwai]

At Mon, 1 Jun 2015 10:43:58 +0530,
Vinod Koul wrote:
> 
> > > +static int azx_acquire_irq(struct soc_hdac_bus *sbus, int do_disconnect)
> > > +{
> > > +	struct hda_skl *hda = to_hda_skl(sbus);
> > > +	struct hdac_bus *bus = hdac_bus(sbus);
> > > +	int ret = 0;
> > > +
> > > +	ret = request_threaded_irq(hda->pci->irq, azx_interrupt,
> > > +			azx_threaded_handler,
> > > +			hda->msi ? 0 : IRQF_SHARED,
> > > +			KBUILD_MODNAME, sbus);
> > 
> > Why not just always request the interrupt as shared - we don't seem to
> > care in the interrupt handling code?
> I think we can move this to shared only

MSI is the exclusive irq, thus shouldn't be shared.


Takashi

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Mark Brown]

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

On Mon, Jun 01, 2015 at 07:32:14AM +0200, Takashi Iwai wrote:
> Vinod Koul wrote:

> > > > +	ret = request_threaded_irq(hda->pci->irq, azx_interrupt,
> > > > +			azx_threaded_handler,
> > > > +			hda->msi ? 0 : IRQF_SHARED,
> > > > +			KBUILD_MODNAME, sbus);

> > > Why not just always request the interrupt as shared - we don't seem to
> > > care in the interrupt handling code?

> > I think we can move this to shared only

> MSI is the exclusive irq, thus shouldn't be shared.

Why does the driver care though?  IRQF_SHARED is advertising the
capabilities of the hander, not a requirement on the hardware - if the
interrupt physically can't be shared then the ability to share it will
never get used but that shouldn't matter.

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

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

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Takashi Iwai]

At Tue, 2 Jun 2015 11:42:33 +0100,
Mark Brown wrote:
> 
> On Mon, Jun 01, 2015 at 07:32:14AM +0200, Takashi Iwai wrote:
> > Vinod Koul wrote:
> 
> > > > > +	ret = request_threaded_irq(hda->pci->irq, azx_interrupt,
> > > > > +			azx_threaded_handler,
> > > > > +			hda->msi ? 0 : IRQF_SHARED,
> > > > > +			KBUILD_MODNAME, sbus);
> 
> > > > Why not just always request the interrupt as shared - we don't seem to
> > > > care in the interrupt handling code?
> 
> > > I think we can move this to shared only
> 
> > MSI is the exclusive irq, thus shouldn't be shared.
> 
> Why does the driver care though?  IRQF_SHARED is advertising the
> capabilities of the hander, not a requirement on the hardware - if the
> interrupt physically can't be shared then the ability to share it will
> never get used but that shouldn't matter.

Because the kernel doesn't guarantee the exclusiveness of irq handler
registration as long as you pass IRQF_SHARED.  That is, if we keep
IRQF_SHARED and another driver tries to request_irq() for the same irq
with again IRQF_SHARED.  But this shouldn't happen with MSI.


Takashi

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Mark Brown]

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

On Tue, Jun 02, 2015 at 12:48:50PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:

> > Why does the driver care though?  IRQF_SHARED is advertising the
> > capabilities of the hander, not a requirement on the hardware - if the
> > interrupt physically can't be shared then the ability to share it will
> > never get used but that shouldn't matter.

> Because the kernel doesn't guarantee the exclusiveness of irq handler
> registration as long as you pass IRQF_SHARED.  That is, if we keep
> IRQF_SHARED and another driver tries to request_irq() for the same irq
> with again IRQF_SHARED.  But this shouldn't happen with MSI.

Sure, but how could that happen (given that the interrupt physically
can't be shared) and surely individual client drivers are the wrong
place to do this - it's not like MSI is the only interrupt type that has
trouble with sharability, if this is an issue we need to have checks and
enforcement for I'd expect the interrupt controller to be flagging
itself as unsharable.

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

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

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Takashi Iwai]

At Tue, 2 Jun 2015 12:10:34 +0100,
Mark Brown wrote:
> 
> On Tue, Jun 02, 2015 at 12:48:50PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > Why does the driver care though?  IRQF_SHARED is advertising the
> > > capabilities of the hander, not a requirement on the hardware - if the
> > > interrupt physically can't be shared then the ability to share it will
> > > never get used but that shouldn't matter.
> 
> > Because the kernel doesn't guarantee the exclusiveness of irq handler
> > registration as long as you pass IRQF_SHARED.  That is, if we keep
> > IRQF_SHARED and another driver tries to request_irq() for the same irq
> > with again IRQF_SHARED.  But this shouldn't happen with MSI.
> 
> Sure, but how could that happen (given that the interrupt physically
> can't be shared) and surely individual client drivers are the wrong
> place to do this -

Oh how can you trust BIOS setup? :)  A wrong numbered IRQ is a most
frequently seen problem (mostly not about MSI, though).

> it's not like MSI is the only interrupt type that has
> trouble with sharability, if this is an issue we need to have checks and
> enforcement for I'd expect the interrupt controller to be flagging
> itself as unsharable.

Rather the sharable interrupt is exceptional, I'd say.  That's the
reason we have IRQF_SHARED, not IRQF_EXCLUSIVE.


Takashi

Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Vinod Koul]

On Tue, Jun 02, 2015 at 01:44:58PM +0200, Takashi Iwai wrote:
> At Tue, 2 Jun 2015 12:10:34 +0100,
> Mark Brown wrote:
> > 
> > On Tue, Jun 02, 2015 at 12:48:50PM +0200, Takashi Iwai wrote:
> > > Mark Brown wrote:
> > 
> > > > Why does the driver care though?  IRQF_SHARED is advertising the
> > > > capabilities of the hander, not a requirement on the hardware - if the
> > > > interrupt physically can't be shared then the ability to share it will
> > > > never get used but that shouldn't matter.
> > 
> > > Because the kernel doesn't guarantee the exclusiveness of irq handler
> > > registration as long as you pass IRQF_SHARED.  That is, if we keep
> > > IRQF_SHARED and another driver tries to request_irq() for the same irq
> > > with again IRQF_SHARED.  But this shouldn't happen with MSI.
> > 
> > Sure, but how could that happen (given that the interrupt physically
> > can't be shared) and surely individual client drivers are the wrong
> > place to do this -
> 
> Oh how can you trust BIOS setup? :)  A wrong numbered IRQ is a most
> frequently seen problem (mostly not about MSI, though).
well for SKL the systems are supposed to be not using MSI(my orignal thought
removing msi part and going to shared irq only), but we know how easily
these things get messed up, so having this sounds better here

-- 
~Vinod
> 
> > it's not like MSI is the only interrupt type that has
> > trouble with sharability, if this is an issue we need to have checks and
> > enforcement for I'd expect the interrupt controller to be flagging
> > itself as unsharable.
> 
> Rather the sharable interrupt is exceptional, I'd say.  That's the
> reason we have IRQF_SHARED, not IRQF_EXCLUSIVE.
> 
> 
> Takashi

-- 

[PATCH v4 6/7] ASoC: intel - add makefile support for SKL driver

[Vinod Koul]

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

Add makefile and Kconfig to enable Skylake HD audio driver

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/Kconfig          | 17 +++++++++++++++++
 sound/soc/intel/Makefile         |  1 +
 sound/soc/intel/skylake/Makefile |  3 +++
 3 files changed, 21 insertions(+)
 create mode 100644 sound/soc/intel/skylake/Makefile

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index ee03dbdda235..2b7d65733b28 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -121,3 +121,20 @@ config SND_SOC_INTEL_CHT_BSW_RT5645_MACH
 	  This adds support for ASoC machine driver for Intel(R) Cherrytrail & Braswell
 	  platforms with RT5645 audio codec.
 	  If unsure select "N".
+
+config SND_SOC_HDA_PREALLOC_SIZE
+	int "Pre-allocated buffer size for skylake HD-audio driver"
+	range 0 32768
+	default 64
+	help
+	  Specifies the default pre-allocated buffer-size in kB for the
+	  HD-audio driver.  A larger buffer (e.g. 2048) is preferred
+	  for systems using PulseAudio.  The default 64 is chosen just
+	  for compatibility reasons.
+
+config SND_SOC_INTEL_HDA_SKYLAKE
+	tristate
+	select SND_SOC_HDA_CORE
+	help
+	  Say Y here to include support for ASoC Intel "High Definition
+	  Audio" (Skylake) and its compatible devices.
diff --git a/sound/soc/intel/Makefile b/sound/soc/intel/Makefile
index cd9aee9871a3..564a8d6e2195 100644
--- a/sound/soc/intel/Makefile
+++ b/sound/soc/intel/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_SND_SOC_INTEL_SST) += common/
 obj-$(CONFIG_SND_SOC_INTEL_HASWELL) += haswell/
 obj-$(CONFIG_SND_SOC_INTEL_BAYTRAIL) += baytrail/
 obj-$(CONFIG_SND_SOC_INTEL_BAYTRAIL) += atom/
+obj-$(CONFIG_SND_SOC_INTEL_HDA_SKYLAKE) += skylake/
 
 # Machine support
 obj-$(CONFIG_SND_SOC_INTEL_SST) += boards/
diff --git a/sound/soc/intel/skylake/Makefile b/sound/soc/intel/skylake/Makefile
new file mode 100644
index 000000000000..1848ac809519
--- /dev/null
+++ b/sound/soc/intel/skylake/Makefile
@@ -0,0 +1,3 @@
+snd-soc-hda-skl-objs := hda-skl.o hda-skl-pcm.o
+
+obj-$(CONFIG_SND_SOC_INTEL_HDA_SKYLAKE) += snd-soc-hda-skl.o
-- 
1.9.1

[PATCH v4 7/7] ASoC: intel - adds support for decoupled mode in skl driver

[Vinod Koul]

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

Decoupled mode is where audio link is broken to frontend HDA and backend
(hda/i2s/dmic/hdmi) links. This patch adds support for decoupled mode and
then adds dais, dai ops for be/fe cpu dais and interrupt handler change to
support decoupled mode

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/intel/skylake/hda-skl-pcm.c | 367 +++++++++++++++++++++++++++++++++-
 sound/soc/intel/skylake/hda-skl.c     |  23 ++-
 sound/soc/intel/skylake/hda-skl.h     |   1 +
 3 files changed, 382 insertions(+), 9 deletions(-)

diff --git a/sound/soc/intel/skylake/hda-skl-pcm.c b/sound/soc/intel/skylake/hda-skl-pcm.c
index 8f895c0c1777..1ecf1f68a834 100644
--- a/sound/soc/intel/skylake/hda-skl-pcm.c
+++ b/sound/soc/intel/skylake/hda-skl-pcm.c
@@ -102,6 +102,14 @@ static void soc_hda_set_pcm_constrains(struct soc_hdac_bus *sbus,
 				     20, 178000000);
 }
 
+static enum hdac_stream_type hda_get_host_stream_type(struct soc_hdac_bus *sbus)
+{
+	if (sbus->ppcap)
+		return HDAC_STREAM_TYPE_HOST;
+	else
+		return HDAC_STREAM_TYPE_COUPLED;
+}
+
 static int soc_hda_pcm_open(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *dai)
 {
@@ -114,7 +122,7 @@ static int soc_hda_pcm_open(struct snd_pcm_substream *substream,
 	pm_runtime_get_sync(dai->dev);
 
 	stream = snd_soc_hdac_stream_assign(sbus, substream,
-					HDAC_STREAM_TYPE_COUPLED);
+					hda_get_host_stream_type(sbus));
 	if (stream == NULL)
 		return -EBUSY;
 
@@ -149,15 +157,27 @@ static int hda_get_format(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *dai)
 {
 	struct snd_soc_pcm_runtime *rtd = snd_pcm_substream_chip(substream);
+	struct soc_hdac_bus *sbus = dev_get_drvdata(dai->dev);
 	struct soc_hda_dma_params *dma_params;
 	int format_val = 0;
-	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 
-	dma_params  = (struct soc_hda_dma_params *)
+	if (sbus->ppcap) {
+		struct snd_pcm_runtime *runtime = substream->runtime;
+
+		format_val = snd_hdac_calc_stream_format(runtime->rate,
+						runtime->channels,
+						runtime->format,
+						32, 0);
+
+	} else {
+		struct snd_soc_dai *codec_dai = rtd->codec_dai;
+
+		dma_params  = (struct soc_hda_dma_params *)
 			snd_soc_dai_get_dma_data(codec_dai, substream);
 
-	if (!dma_params)
-		format_val = dma_params->format;
+		if (!dma_params)
+			format_val = dma_params->format;
+	}
 
 	return format_val;
 }
@@ -196,8 +216,9 @@ static int soc_hda_pcm_hw_params(struct snd_pcm_substream *substream,
 				struct snd_soc_dai *dai)
 {
 	struct soc_hdac_bus *sbus = dev_get_drvdata(dai->dev);
+	struct soc_hdac_stream *stream = get_hdac_stream(substream);
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	int ret;
+	int ret, dma_id;
 
 	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
 	ret = substream_alloc_pages(sbus, substream,
@@ -213,17 +234,22 @@ static int soc_hda_pcm_hw_params(struct snd_pcm_substream *substream,
 	dev_dbg(dai->dev, "format_val, rate=%d, ch=%d, format=%d\n",
 			runtime->rate, runtime->channels, runtime->format);
 
+	dma_id = hdac_stream(stream)->stream_tag;
+	dev_dbg(dai->dev, "dma_id=%d\n", dma_id);
+
 	return ret;
 }
 
 static void soc_hda_pcm_close(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *dai)
 {
+	struct soc_hdac_bus *sbus = dev_get_drvdata(dai->dev);
 	struct soc_hdac_stream *stream = get_hdac_stream(substream);
 	struct soc_hda_dma_params *dma_params = NULL;
 
 	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
-	snd_soc_hdac_stream_release(stream, HDAC_STREAM_TYPE_COUPLED);
+
+	snd_soc_hdac_stream_release(stream, hda_get_host_stream_type(sbus));
 
 	dma_params  = (struct soc_hda_dma_params *)
 			snd_soc_dai_get_dma_data(dai, substream);
@@ -247,6 +273,169 @@ static int soc_hda_pcm_hw_free(struct snd_pcm_substream *substream,
 	return substream_free_pages(hdac_bus(sbus), substream);
 }
 
+static int hda_be_dmic_prepare(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *dai)
+{
+	struct snd_pcm_hw_params params = {0};
+	struct snd_interval *channels, *rate;
+
+	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
+
+	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);
+
+	return 0;
+}
+
+static int soc_hda_be_link_hw_params(struct snd_pcm_substream *substream,
+				struct snd_pcm_hw_params *params,
+				struct snd_soc_dai *dai)
+{
+	struct soc_hdac_bus *sbus = dev_get_drvdata(dai->dev);
+	struct soc_hdac_stream *link_dev;
+	struct snd_soc_pcm_runtime *rtd = snd_pcm_substream_chip(substream);
+	struct soc_hda_dma_params *dma_params;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	int dma_id;
+
+	pr_debug("%s\n", __func__);
+	link_dev = snd_soc_hdac_stream_assign(sbus, substream,
+					HDAC_STREAM_TYPE_LINK);
+	if (!link_dev)
+		return -EBUSY;
+
+	snd_soc_dai_set_dma_data(dai, substream, (void *)link_dev);
+
+	/* set the stream tag in the codec dai dma params  */
+	dma_params = (struct soc_hda_dma_params *)
+			snd_soc_dai_get_dma_data(codec_dai, 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;
+
+	return 0;
+}
+
+static int soc_hda_be_link_pcm_prepare(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = snd_pcm_substream_chip(substream);
+	struct soc_hdac_bus *sbus = dev_get_drvdata(dai->dev);
+	struct soc_hdac_stream *link_dev =
+			snd_soc_dai_get_dma_data(dai, substream);
+	unsigned int format_val = 0;
+	struct soc_hda_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 soc_hdac_link *link;
+
+	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
+	if (link_dev->link_prepared) {
+		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 soc_hda_dma_params *)
+			snd_soc_dai_get_dma_data(codec_dai, substream);
+	if (dma_params)
+		format_val = dma_params->format;
+	dev_dbg(dai->dev, "stream_tag=%d formatvalue=%d codec_dai_name=%s\n",
+			hdac_stream(link_dev)->stream_tag, format_val, codec_dai->name);
+
+	snd_soc_hdac_link_stream_reset(link_dev);
+
+	snd_soc_hdac_link_stream_setup(link_dev, format_val);
+
+	link = snd_soc_hdac_bus_get_link(sbus, rtd->codec->component.name);
+	if (!link)
+		return -EINVAL;
+
+	snd_soc_hdac_link_set_stream_id(link, hdac_stream(link_dev)->stream_tag);
+	link_dev->link_prepared = 1;
+
+	return 0;
+}
+
+static int soc_hda_be_link_pcm_trigger(struct snd_pcm_substream *substream,
+	int cmd, struct snd_soc_dai *dai)
+{
+	struct soc_hdac_stream *link_dev =
+				snd_soc_dai_get_dma_data(dai, substream);
+
+	dev_dbg(dai->dev, "In %s cmd=%d\n", __func__, cmd);
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+	case SNDRV_PCM_TRIGGER_RESUME:
+		snd_soc_hdac_link_stream_start(link_dev);
+		break;
+
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_STOP:
+		snd_soc_hdac_link_stream_clear(link_dev);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int soc_hda_be_link_hw_free(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	struct soc_hdac_bus *sbus = dev_get_drvdata(dai->dev);
+	struct snd_soc_pcm_runtime *rtd = snd_pcm_substream_chip(substream);
+	struct soc_hdac_stream *link_dev =
+				snd_soc_dai_get_dma_data(dai, substream);
+	struct soc_hdac_link *link;
+
+	dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name);
+
+	link_dev->link_prepared = 0;
+
+	link = snd_soc_hdac_bus_get_link(sbus, rtd->codec->component.name);
+	if (!link)
+		return -EINVAL;
+
+	snd_soc_hdac_link_clear_stream_id(link, hdac_stream(link_dev)->stream_tag);
+	snd_soc_hdac_stream_release(link_dev, HDAC_STREAM_TYPE_LINK);
+	return 0;
+}
+
+static int soc_hda_be_startup(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	return pm_runtime_get_sync(dai->dev);
+}
+
+static void soc_hda_be_shutdown(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	pm_runtime_mark_last_busy(dai->dev);
+	pm_runtime_put_autosuspend(dai->dev);
+}
+
 static struct snd_soc_dai_ops hda_pcm_dai_ops = {
 	.startup = soc_hda_pcm_open,
 	.shutdown = soc_hda_pcm_close,
@@ -255,6 +444,21 @@ static struct snd_soc_dai_ops hda_pcm_dai_ops = {
 	.hw_free = soc_hda_pcm_hw_free,
 };
 
+static struct snd_soc_dai_ops hda_be_dmic_dai_ops = {
+	.startup = soc_hda_be_startup,
+	.prepare = hda_be_dmic_prepare,
+	.shutdown = soc_hda_be_shutdown,
+};
+
+static struct snd_soc_dai_ops hda_be_link_dai_ops = {
+	.startup = soc_hda_be_startup,
+	.prepare = soc_hda_be_link_pcm_prepare,
+	.hw_params = soc_hda_be_link_hw_params,
+	.hw_free = soc_hda_be_link_hw_free,
+	.trigger = soc_hda_be_link_pcm_trigger,
+	.shutdown = soc_hda_be_shutdown,
+};
+
 static struct snd_soc_dai_driver soc_hda_platform_dai[] = {
 {
 	.name = "System Pin",
@@ -275,6 +479,17 @@ static struct snd_soc_dai_driver soc_hda_platform_dai[] = {
 	},
 },
 {
+	.name = "Reference Pin",
+	.ops = &hda_pcm_dai_ops,
+	.capture = {
+		.stream_name = "Reference Capture",
+		.channels_min = HDA_MONO,
+		.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 = "Deepbuffer Pin",
 	.ops = &hda_pcm_dai_ops,
 	.playback = {
@@ -296,6 +511,80 @@ static struct snd_soc_dai_driver soc_hda_platform_dai[] = {
 		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
 	},
 },
+/* BE CPU  Dais */
+{
+	.name = "iDisp Pin",
+	.ops = &hda_be_link_dai_ops,
+	.playback = {
+		.stream_name = "iDisp Tx",
+		.channels_min = HDA_STEREO,
+		.channels_max = HDA_STEREO,
+		.rates = SNDRV_PCM_RATE_8000|SNDRV_PCM_RATE_16000|SNDRV_PCM_RATE_48000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+	},
+},
+{
+	.name = "DMIC01 Pin",
+	.ops = &hda_be_dmic_dai_ops,
+	.capture = {
+		.stream_name = "DMIC01 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 = "DMIC23 Pin",
+	.ops = &hda_be_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 = &hda_be_link_dai_ops,
+	.playback = {
+		.stream_name = "HD-Codec Tx",
+		.channels_min = HDA_STEREO,
+		.channels_max = HDA_STEREO,
+		.rates = SNDRV_PCM_RATE_48000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+	},
+	.capture = {
+		.stream_name = "HD-Codec Rx",
+		.channels_min = HDA_STEREO,
+		.channels_max = HDA_STEREO,
+		.rates = SNDRV_PCM_RATE_48000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+	},
+},
+{
+	.name = "HD-Codec-SPK Pin",
+	.ops = &hda_be_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 = &hda_be_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 soc_hda_platform_open(struct snd_pcm_substream *substream)
@@ -312,7 +601,7 @@ static int soc_hda_platform_open(struct snd_pcm_substream *substream)
 	return 0;
 }
 
-static int soc_hda_platform_pcm_trigger(struct snd_pcm_substream *substream,
+static int soc_hda_pcm_trigger(struct snd_pcm_substream *substream,
 					int cmd)
 {
 	struct soc_hdac_bus *sbus = get_bus_ctx(substream);
@@ -385,6 +674,68 @@ static int soc_hda_platform_pcm_trigger(struct snd_pcm_substream *substream,
 	return 0;
 }
 
+static int soc_hda_dsp_trigger(struct snd_pcm_substream *substream,
+		int cmd)
+{
+	struct soc_hdac_bus *sbus = get_bus_ctx(substream);
+	struct hdac_bus *bus = hdac_bus(sbus);
+	struct snd_soc_pcm_runtime *rtd = snd_pcm_substream_chip(substream);
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	struct soc_hdac_stream *stream;
+	int start;
+	unsigned long cookie;
+	struct hdac_stream *hstr;
+
+	dev_dbg(bus->dev, "In %s cmd=%d streamname=%s\n", __func__, cmd, cpu_dai->name);
+
+	stream = get_hdac_stream(substream);
+	hstr = hdac_stream(stream);
+
+	if (!hstr->prepared)
+		return -EPIPE;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+	case SNDRV_PCM_TRIGGER_RESUME:
+		start = 1;
+		break;
+
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_STOP:
+		start = 0;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&bus->reg_lock, cookie);
+
+	if (start)
+		snd_hdac_stream_start(hdac_stream(stream), true);
+	else
+		snd_hdac_stream_stop(hdac_stream(stream));
+
+	if (start)
+		snd_hdac_stream_timecounter_init(hstr, 0);
+
+	spin_unlock_irqrestore(&bus->reg_lock, cookie);
+
+	return 0;
+}
+static int soc_hda_platform_pcm_trigger(struct snd_pcm_substream *substream,
+					int cmd)
+{
+	struct soc_hdac_bus *sbus = get_bus_ctx(substream);
+
+	if (sbus->ppcap)
+		return soc_hda_dsp_trigger(substream, cmd);
+	else
+		return soc_hda_pcm_trigger(substream, cmd);
+}
+
 /* calculate runtime delay from LPIB */
 static int hda_get_delay_from_lpib(struct soc_hdac_bus *sbus,
 				struct soc_hdac_stream *sstream,
diff --git a/sound/soc/intel/skylake/hda-skl.c b/sound/soc/intel/skylake/hda-skl.c
index 3a8e6f702bac..a0d72f8393f2 100644
--- a/sound/soc/intel/skylake/hda-skl.c
+++ b/sound/soc/intel/skylake/hda-skl.c
@@ -169,10 +169,16 @@ static int azx_acquire_irq(struct soc_hdac_bus *sbus, int do_disconnect)
 	struct hda_skl *hda = to_hda_skl(sbus);
 	struct hdac_bus *bus = hdac_bus(sbus);
 	int ret = 0;
+	unsigned long flags = 0;
+
+	if (sbus->ppcap)
+		flags = IRQF_SHARED;
+	else
+		flags = hda->msi ? 0 : IRQF_SHARED;
 
 	ret = request_threaded_irq(hda->pci->irq, azx_interrupt,
 			azx_threaded_handler,
-			hda->msi ? 0 : IRQF_SHARED,
+			flags,
 			KBUILD_MODNAME, sbus);
 	if (ret) {
 		dev_err(bus->dev,
@@ -346,6 +352,10 @@ static int probe_codec(struct soc_hdac_bus *sbus, int addr)
 	if (res == -1)
 		return -EIO;
 	dev_dbg(bus->dev, "codec #%d probed OK\n", addr);
+
+	if (sbus->mlcap)
+		snd_soc_hdac_bus_map_codec_to_link(sbus, addr);
+
 	return snd_soc_hdac_bus_device_init(sbus, addr);
 }
 
@@ -446,6 +456,8 @@ static int azx_first_init(struct soc_hdac_bus *sbus)
 		if (pci_enable_msi(pci) < 0)
 			hda->msi = 0;
 
+	snd_soc_hdac_bus_parse_capabilities(sbus);
+
 	if (azx_acquire_irq(sbus, 0) < 0)
 		return -EBUSY;
 
@@ -575,6 +587,15 @@ static int azx_probe(struct pci_dev *pci,
 	if (err < 0)
 		goto out_free;
 
+	/* check if dsp is there */
+	if (sbus->ppcap) {
+		/* TODO register with dsp lib */
+		dev_dbg(bus->dev, "Register dsp\n");
+	}
+
+	if (sbus->mlcap)
+		snd_soc_hdac_bus_get_ml_capabilities(sbus);
+
 	/*create device for soc dmic*/
 	err = hda_dmic_device_register(hda);
 	if (err < 0)
diff --git a/sound/soc/intel/skylake/hda-skl.h b/sound/soc/intel/skylake/hda-skl.h
index ed6ccd31b198..7793111e42e5 100644
--- a/sound/soc/intel/skylake/hda-skl.h
+++ b/sound/soc/intel/skylake/hda-skl.h
@@ -23,6 +23,7 @@
 
 #include <sound/core.h>
 #include <sound/initval.h>
+#include <sound/soc.h>
 #include <sound/hda_register.h>
 #include <sound/hdaudio.h>
 #include <sound/soc-hdaudio.h>
-- 
1.9.1

Re: [PATCH v4 0/7] ASoC: intel - add skylake PCM driver

[Vinod Koul]

On Mon, May 11, 2015 at 04:23:58PM +0530, Vinod Koul wrote:
> SKL has HDA controller based audio subsystem with DSP and support for I2S,
> HDA, PDM links. The hda core code has been moved to sound/hda/ by Takashi
> which current HDA drivers use and will also be used by ASoC SKL driver.
> 
> The SKL platform driver will load and create the soc_hdac_bus which embeds
> the hdac_bus, same for hdac_device (hda codecs) and hdac_stream (pcms) This
> is on top of hdac code in Takashi's topic/hda
> 
> This patch provides the match function for asoc type hda codecs and let's
> them get enumerated by hdac. The second patch in this series adds the
> controller specific soc code. Common parts are in hdac core with changes
> introduced as part of SKL controller in soc part. Then we add the rest of
> controller PCM driver code (still HDA) and last patch breaks the HDA streams
> to host and link which will allow insertion of DSP in between these links.
> 
> The subsequent series will add IPC driver for SKL (using common IPC
> routines), then DSP topology handlers, DSP code with I2S support and then
> lastly when DFW is accepted then its handlers.
> 
> This patch series adds the hda codec match functions followed by asoc hda
> controller routines, then SKL PCM driver and last decouples the controller
> for splitting the links

Hey Mark,

Any word on this series ..

Thanks
-- 
~Vinod
> 
> Fixes in v4:
>   Updates changelog in patch1 and few other patches
>   Address Takashi's comment
>   Address Marks comments
> 
> Jeeja KP (7):
>   ASoC: hda - add ASoC HDA codec match function
>   ALSA: hda - add new HDA registers
>   ASoC: hda - add asoc hda core bus, controller and stream helpers
>   ASoC: intel - add Skylake HDA platform driver
>   ASoC: intel - add Skylake HDA audio driver
>   ASoC: intel - add makefile support for SKL driver
>   ASoC: intel - adds support for decoupled mode in skl driver
> 
>  include/sound/hda_register.h          |  88 ++++
>  include/sound/soc-hda-codec.h         |  49 ++
>  include/sound/soc-hdaudio.h           | 360 +++++++++++++
>  sound/soc/Kconfig                     |   1 +
>  sound/soc/Makefile                    |   1 +
>  sound/soc/hda/Kconfig                 |   3 +
>  sound/soc/hda/Makefile                |   4 +
>  sound/soc/hda/soc-hda-codec.c         |  89 ++++
>  sound/soc/hda/soc-hdac-bus.c          | 115 +++++
>  sound/soc/hda/soc-hdac-controller.c   | 296 +++++++++++
>  sound/soc/hda/soc-hdac-stream.c       | 409 +++++++++++++++
>  sound/soc/intel/Kconfig               |  17 +
>  sound/soc/intel/Makefile              |   1 +
>  sound/soc/intel/skylake/Makefile      |   3 +
>  sound/soc/intel/skylake/hda-skl-pcm.c | 937 ++++++++++++++++++++++++++++++++++
>  sound/soc/intel/skylake/hda-skl.c     | 670 ++++++++++++++++++++++++
>  sound/soc/intel/skylake/hda-skl.h     |  74 +++
>  17 files changed, 3117 insertions(+)
>  create mode 100644 include/sound/soc-hda-codec.h
>  create mode 100644 include/sound/soc-hdaudio.h
>  create mode 100644 sound/soc/hda/Kconfig
>  create mode 100644 sound/soc/hda/Makefile
>  create mode 100644 sound/soc/hda/soc-hda-codec.c
>  create mode 100644 sound/soc/hda/soc-hdac-bus.c
>  create mode 100644 sound/soc/hda/soc-hdac-controller.c
>  create mode 100644 sound/soc/hda/soc-hdac-stream.c
>  create mode 100644 sound/soc/intel/skylake/Makefile
>  create mode 100644 sound/soc/intel/skylake/hda-skl-pcm.c
>  create mode 100644 sound/soc/intel/skylake/hda-skl.c
>  create mode 100644 sound/soc/intel/skylake/hda-skl.h
> 
> -- 
> 1.9.1
> 

-- 

Re: [PATCH v4 0/7] ASoC: intel - add skylake PCM driver

[Mark Brown]

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

On Fri, May 22, 2015 at 05:50:49PM +0530, Vinod Koul wrote:
> On Mon, May 11, 2015 at 04:23:58PM +0530, Vinod Koul wrote:

> Any word on this series ..

Don't send contentless pings.  They just add to mail volume and take
even more time.  In this case you're chasing after less than two weeks
which is very fast especially for large, new bits of code that need
careful review.

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

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

Re: [PATCH v4 0/7] ASoC: intel - add skylake PCM driver

[Takashi Iwai]

At Mon, 11 May 2015 16:23:58 +0530,
Vinod Koul wrote:
> 
> SKL has HDA controller based audio subsystem with DSP and support for I2S,
> HDA, PDM links. The hda core code has been moved to sound/hda/ by Takashi
> which current HDA drivers use and will also be used by ASoC SKL driver.
> 
> The SKL platform driver will load and create the soc_hdac_bus which embeds
> the hdac_bus, same for hdac_device (hda codecs) and hdac_stream (pcms) This
> is on top of hdac code in Takashi's topic/hda
> 
> This patch provides the match function for asoc type hda codecs and let's
> them get enumerated by hdac. The second patch in this series adds the
> controller specific soc code. Common parts are in hdac core with changes
> introduced as part of SKL controller in soc part. Then we add the rest of
> controller PCM driver code (still HDA) and last patch breaks the HDA streams
> to host and link which will allow insertion of DSP in between these links.
> 
> The subsequent series will add IPC driver for SKL (using common IPC
> routines), then DSP topology handlers, DSP code with I2S support and then
> lastly when DFW is accepted then its handlers.
> 
> This patch series adds the hda codec match functions followed by asoc hda
> controller routines, then SKL PCM driver and last decouples the controller
> for splitting the links

Merging this without the codec driver would be rather confusing for
users, since it gives a conflicting driver that doesn't work at all.
I suppose that the merge should be pending until the complete
implementation of ASoC HDA.

Comments?


Takashi

> 
> Fixes in v4:
>   Updates changelog in patch1 and few other patches
>   Address Takashi's comment
>   Address Marks comments
> 
> Jeeja KP (7):
>   ASoC: hda - add ASoC HDA codec match function
>   ALSA: hda - add new HDA registers
>   ASoC: hda - add asoc hda core bus, controller and stream helpers
>   ASoC: intel - add Skylake HDA platform driver
>   ASoC: intel - add Skylake HDA audio driver
>   ASoC: intel - add makefile support for SKL driver
>   ASoC: intel - adds support for decoupled mode in skl driver
> 
>  include/sound/hda_register.h          |  88 ++++
>  include/sound/soc-hda-codec.h         |  49 ++
>  include/sound/soc-hdaudio.h           | 360 +++++++++++++
>  sound/soc/Kconfig                     |   1 +
>  sound/soc/Makefile                    |   1 +
>  sound/soc/hda/Kconfig                 |   3 +
>  sound/soc/hda/Makefile                |   4 +
>  sound/soc/hda/soc-hda-codec.c         |  89 ++++
>  sound/soc/hda/soc-hdac-bus.c          | 115 +++++
>  sound/soc/hda/soc-hdac-controller.c   | 296 +++++++++++
>  sound/soc/hda/soc-hdac-stream.c       | 409 +++++++++++++++
>  sound/soc/intel/Kconfig               |  17 +
>  sound/soc/intel/Makefile              |   1 +
>  sound/soc/intel/skylake/Makefile      |   3 +
>  sound/soc/intel/skylake/hda-skl-pcm.c | 937 ++++++++++++++++++++++++++++++++++
>  sound/soc/intel/skylake/hda-skl.c     | 670 ++++++++++++++++++++++++
>  sound/soc/intel/skylake/hda-skl.h     |  74 +++
>  17 files changed, 3117 insertions(+)
>  create mode 100644 include/sound/soc-hda-codec.h
>  create mode 100644 include/sound/soc-hdaudio.h
>  create mode 100644 sound/soc/hda/Kconfig
>  create mode 100644 sound/soc/hda/Makefile
>  create mode 100644 sound/soc/hda/soc-hda-codec.c
>  create mode 100644 sound/soc/hda/soc-hdac-bus.c
>  create mode 100644 sound/soc/hda/soc-hdac-controller.c
>  create mode 100644 sound/soc/hda/soc-hdac-stream.c
>  create mode 100644 sound/soc/intel/skylake/Makefile
>  create mode 100644 sound/soc/intel/skylake/hda-skl-pcm.c
>  create mode 100644 sound/soc/intel/skylake/hda-skl.c
>  create mode 100644 sound/soc/intel/skylake/hda-skl.h
> 
> -- 
> 1.9.1
> 

Re: [PATCH v4 0/7] ASoC: intel - add skylake PCM driver

[Vinod Koul]

On Mon, May 25, 2015 at 08:57:05AM +0200, Takashi Iwai wrote:
> At Mon, 11 May 2015 16:23:58 +0530,
> Vinod Koul wrote:
> > 
> > SKL has HDA controller based audio subsystem with DSP and support for I2S,
> > HDA, PDM links. The hda core code has been moved to sound/hda/ by Takashi
> > which current HDA drivers use and will also be used by ASoC SKL driver.
> > 
> > The SKL platform driver will load and create the soc_hdac_bus which embeds
> > the hdac_bus, same for hdac_device (hda codecs) and hdac_stream (pcms) This
> > is on top of hdac code in Takashi's topic/hda
> > 
> > This patch provides the match function for asoc type hda codecs and let's
> > them get enumerated by hdac. The second patch in this series adds the
> > controller specific soc code. Common parts are in hdac core with changes
> > introduced as part of SKL controller in soc part. Then we add the rest of
> > controller PCM driver code (still HDA) and last patch breaks the HDA streams
> > to host and link which will allow insertion of DSP in between these links.
> > 
> > The subsequent series will add IPC driver for SKL (using common IPC
> > routines), then DSP topology handlers, DSP code with I2S support and then
> > lastly when DFW is accepted then its handlers.
> > 
> > This patch series adds the hda codec match functions followed by asoc hda
> > controller routines, then SKL PCM driver and last decouples the controller
> > for splitting the links
> 
> Merging this without the codec driver would be rather confusing for
> users, since it gives a conflicting driver that doesn't work at all.
> I suppose that the merge should be pending until the complete
> implementation of ASoC HDA.
Well users wont notice till we have a machine driver which selects this
driver. That is why machine driver will come last and needs to be
merged only after controller and codec drivers are merged for end
users to notice

-- 
~Vinod

Re: [PATCH v4 0/7] ASoC: intel - add skylake PCM driver

[Takashi Iwai]

At Mon, 25 May 2015 16:54:58 +0530,
Vinod Koul wrote:
> 
> On Mon, May 25, 2015 at 08:57:05AM +0200, Takashi Iwai wrote:
> > At Mon, 11 May 2015 16:23:58 +0530,
> > Vinod Koul wrote:
> > > 
> > > SKL has HDA controller based audio subsystem with DSP and support for I2S,
> > > HDA, PDM links. The hda core code has been moved to sound/hda/ by Takashi
> > > which current HDA drivers use and will also be used by ASoC SKL driver.
> > > 
> > > The SKL platform driver will load and create the soc_hdac_bus which embeds
> > > the hdac_bus, same for hdac_device (hda codecs) and hdac_stream (pcms) This
> > > is on top of hdac code in Takashi's topic/hda
> > > 
> > > This patch provides the match function for asoc type hda codecs and let's
> > > them get enumerated by hdac. The second patch in this series adds the
> > > controller specific soc code. Common parts are in hdac core with changes
> > > introduced as part of SKL controller in soc part. Then we add the rest of
> > > controller PCM driver code (still HDA) and last patch breaks the HDA streams
> > > to host and link which will allow insertion of DSP in between these links.
> > > 
> > > The subsequent series will add IPC driver for SKL (using common IPC
> > > routines), then DSP topology handlers, DSP code with I2S support and then
> > > lastly when DFW is accepted then its handlers.
> > > 
> > > This patch series adds the hda codec match functions followed by asoc hda
> > > controller routines, then SKL PCM driver and last decouples the controller
> > > for splitting the links
> > 
> > Merging this without the codec driver would be rather confusing for
> > users, since it gives a conflicting driver that doesn't work at all.
> > I suppose that the merge should be pending until the complete
> > implementation of ASoC HDA.
> Well users wont notice till we have a machine driver which selects this
> driver. That is why machine driver will come last and needs to be
> merged only after controller and codec drivers are merged for end
> users to notice

We still don't provide a way to lead to the right driver.  With your
patch 5, it creates another SKL PCI driver that conflicts with the
existing driver entry of snd-hda-intel.  So, at this moment, it just
give a conflict for non-working driver, and it's not good to leave so
unless the complete driver set is provided in a week or so.


Takashi

Re: [PATCH v4 0/7] ASoC: intel - add skylake PCM driver

[Vinod Koul]

On Mon, May 25, 2015 at 01:58:58PM +0200, Takashi Iwai wrote:
> > > Merging this without the codec driver would be rather confusing for
> > > users, since it gives a conflicting driver that doesn't work at all.
> > > I suppose that the merge should be pending until the complete
> > > implementation of ASoC HDA.
> > Well users wont notice till we have a machine driver which selects this
> > driver. That is why machine driver will come last and needs to be
> > merged only after controller and codec drivers are merged for end
> > users to notice
> 
> We still don't provide a way to lead to the right driver.  With your
> patch 5, it creates another SKL PCI driver that conflicts with the
> existing driver entry of snd-hda-intel.  So, at this moment, it just
> give a conflict for non-working driver, and it's not good to leave so
> unless the complete driver set is provided in a week or so.
Hi Takashi,

You are right in that but the point is that user cannot select this driver

+config SND_SOC_INTEL_HDA_SKYLAKE
+       tristate
+       select SND_SOC_HDA_CORE
+       help
+         Say Y here to include support for ASoC Intel "High Definition
+         Audio" (Skylake) and its compatible devices.

So unless someone selects this symbol (machine driver) it wont be built for
anyone (expect you and me to test :D ) and wont conflict.
Or did I miss something ?

Also anyway I will add the flag in this driver as follow on to disable
registering this device so that users don't see this device getting
registered, unless we have complete story merged.

Thanks
-- 
~Vinod

Re: [PATCH v4 0/7] ASoC: intel - add skylake PCM driver

[Takashi Iwai]

At Tue, 26 May 2015 09:44:36 +0530,
Vinod Koul wrote:
> 
> On Mon, May 25, 2015 at 01:58:58PM +0200, Takashi Iwai wrote:
> > > > Merging this without the codec driver would be rather confusing for
> > > > users, since it gives a conflicting driver that doesn't work at all.
> > > > I suppose that the merge should be pending until the complete
> > > > implementation of ASoC HDA.
> > > Well users wont notice till we have a machine driver which selects this
> > > driver. That is why machine driver will come last and needs to be
> > > merged only after controller and codec drivers are merged for end
> > > users to notice
> > 
> > We still don't provide a way to lead to the right driver.  With your
> > patch 5, it creates another SKL PCI driver that conflicts with the
> > existing driver entry of snd-hda-intel.  So, at this moment, it just
> > give a conflict for non-working driver, and it's not good to leave so
> > unless the complete driver set is provided in a week or so.
> Hi Takashi,
> 
> You are right in that but the point is that user cannot select this driver
> 
> +config SND_SOC_INTEL_HDA_SKYLAKE
> +       tristate
> +       select SND_SOC_HDA_CORE
> +       help
> +         Say Y here to include support for ASoC Intel "High Definition
> +         Audio" (Skylake) and its compatible devices.
> 
> So unless someone selects this symbol (machine driver) it wont be built for
> anyone (expect you and me to test :D ) and wont conflict.
> Or did I miss something ?

Distros tend to select all possible options, especially if it can be
built as a module.  Even if it's written in the help text, not
everybody reads it.

> Also anyway I will add the flag in this driver as follow on to disable
> registering this device so that users don't see this device getting
> registered, unless we have complete story merged.

Yeah, we should prepare some way before the actual conflict is seen.


thanks,

Takashi