[PATCH v6 0/3] ALSA: HDA: add extended HDA

[PATCH v6 0/3] ALSA: HDA: add extended HDA

[Vinod Koul]

New HDA controllers from Intel have extended HDA capabilities like multilink,
pipe processing, SPIB, GTS etc In order to use them we create an extended
HDA bus, controller and stream which uses the extended configuration

Vinod Koul (3):
  ALSA: hdac_ext: add extended HDA bus
  ALSA: hdac_ext: add hdac extended controller
  ALSA: hdac_ext: add extended stream capabilities

 include/sound/hdaudio_ext.h         | 385 +++++++++++++++++++++++++++++++++
 sound/hda/Kconfig                   |   4 +
 sound/hda/Makefile                  |   3 +
 sound/hda/ext/Makefile              |   3 +
 sound/hda/ext/hdac_ext_bus.c        | 115 ++++++++++
 sound/hda/ext/hdac_ext_controller.c | 295 +++++++++++++++++++++++++
 sound/hda/ext/hdac_ext_stream.c     | 414 ++++++++++++++++++++++++++++++++++++
 7 files changed, 1219 insertions(+)
 create mode 100644 include/sound/hdaudio_ext.h
 create mode 100644 sound/hda/ext/Makefile
 create mode 100644 sound/hda/ext/hdac_ext_bus.c
 create mode 100644 sound/hda/ext/hdac_ext_controller.c
 create mode 100644 sound/hda/ext/hdac_ext_stream.c

-- 
1.9.1

[PATCH v6 1/3] ALSA: hdac_ext: add extended HDA bus

[Vinod Koul]

The new HDA controllers from Intel support new capabilities like multilink,
pipe processing, SPIB, GTS etc In order to use them we create an extended HDA
bus which embed the hdac bus and contains the fields for extended
configurations

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 include/sound/hdaudio_ext.h  |  93 ++++++++++++++++++++++++++++++++++
 sound/hda/Kconfig            |   4 ++
 sound/hda/Makefile           |   3 ++
 sound/hda/ext/Makefile       |   3 ++
 sound/hda/ext/hdac_ext_bus.c | 115 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 218 insertions(+)
 create mode 100644 include/sound/hdaudio_ext.h
 create mode 100644 sound/hda/ext/Makefile
 create mode 100644 sound/hda/ext/hdac_ext_bus.c

diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
new file mode 100644
index 000000000000..df1f8ae64693
--- /dev/null
+++ b/include/sound/hdaudio_ext.h
@@ -0,0 +1,93 @@
+#ifndef __SOUND_HDAUDIO_EXT_H
+#define __SOUND_HDAUDIO_EXT_H
+
+#include <sound/core.h>
+#include <sound/initval.h>
+#include <sound/hda_register.h>
+#include <sound/hdaudio.h>
+
+/**
+ * hdac_ext_bus: HDAC extended bus for extended HDA caps
+ *
+ * @bus: hdac bus
+ * @num_streams: streams supported
+ * @ppcap: HDA ext post processing caps supported
+ * @spbcap: HDA ext Software Position in Buffer cap support
+ * @mlcap: HDA ext MultiLink cap support
+ * @gtscap: HDA ext Global Time Sync cap support
+ * @ppcap_addr: pp capabilities pointer
+ * @spbcap_addr: SPIB capabilities pointer
+ * @mlcap_addr: MultiLink capabilities pointer
+ * @gtscap_addr: gts capabilities pointer
+ * @hlink_list: link list of HDA links
+ */
+struct hdac_ext_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;
+
+	struct list_head hlink_list;
+};
+
+int snd_hdac_ext_bus_init(struct hdac_ext_bus *sbus, struct device *dev,
+		      const struct hdac_bus_ops *ops,
+		      const struct hdac_io_ops *io_ops);
+
+void snd_hdac_ext_bus_exit(struct hdac_ext_bus *sbus);
+int snd_hdac_ext_bus_device_init(struct hdac_ext_bus *sbus, int addr);
+void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev);
+
+#define hdac_bus(sbus)	(&(sbus)->bus)
+#define bus_to_hdac_ext_bus(_bus) \
+	container_of(_bus, struct hdac_ext_bus, bus)
+
+int snd_hdac_ext_bus_parse_capabilities(struct hdac_ext_bus *sbus);
+void snd_hdac_ext_bus_ppcap_enable(struct hdac_ext_bus *chip, bool enable);
+void snd_hdac_ext_bus_ppcap_int_enable(struct hdac_ext_bus *chip, bool enable);
+
+/*
+ * macros for ppcap register read/write
+ */
+#define _snd_hdac_ext_bus_ppcap_write(type, dev, reg, value)		\
+	((dev)->bus.io_ops->reg_write ## type(value, (dev)->ppcap_addr + (reg)))
+#define _snd_hdac_ext_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 snd_hdac_ext_bus_ppcap_writel(dev, reg, value) \
+	_snd_hdac_ext_bus_ppcap_write(l, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_bus_ppcap_writew(dev, reg, value) \
+	_snd_hdac_ext_bus_ppcap_write(w, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_bus_ppcap_writeb(dev, reg, value) \
+	_snd_hdac_ext_bus_ppcap_write(b, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_bus_ppcap_readl(dev, reg) \
+	_snd_hdac_ext_bus_ppcap_read(l, dev, AZX_REG_ ## reg)
+#define snd_hdac_ext_bus_ppcap_readw(dev, reg) \
+	_snd_hdac_ext_bus_ppcap_read(w, dev, AZX_REG_ ## reg)
+#define snd_hdac_ext_bus_ppcap_readb(dev, reg) \
+	_snd_hdac_ext_bus_ppcap_read(b, dev, AZX_REG_ ## reg)
+
+/* update a register, pass without AZX_REG_ prefix */
+#define snd_hdac_ext_bus_ppcap_updatel(dev, reg, mask, val) \
+	snd_hdac_ext_bus_ppcap_writel(dev, reg, \
+			       (snd_hdac_ext_bus_ppcap_readl(dev, reg) & \
+				~(mask)) | (val))
+#define snd_hdac_ext_bus_ppcap_updatew(dev, reg, mask, val) \
+	snd_hdac_ext_bus_ppcap_writew(dev, reg, \
+			       (snd_hdac_ext_bus_ppcap_readw(dev, reg) & \
+				~(mask)) | (val))
+#define snd_hdac_ext_bus_ppcap_updateb(dev, reg, mask, val) \
+	snd_hdac_ext_bus_ppcap_writeb(dev, reg, \
+			       (snd_hdac_ext_bus_ppcap_readb(dev, reg) & \
+				~(mask)) | (val))
+
+#endif /* __SOUND_HDAUDIO_EXT_H */
diff --git a/sound/hda/Kconfig b/sound/hda/Kconfig
index ac5ffac2a272..6dc3914fd28b 100644
--- a/sound/hda/Kconfig
+++ b/sound/hda/Kconfig
@@ -10,3 +10,7 @@ config SND_HDA_I915
 	default y
 	depends on DRM_I915
 	depends on SND_HDA_CORE
+
+config SND_HDA_EXT_CORE
+       tristate
+       select SND_HDA_CORE
diff --git a/sound/hda/Makefile b/sound/hda/Makefile
index 55dd465c7042..7e999c995cdc 100644
--- a/sound/hda/Makefile
+++ b/sound/hda/Makefile
@@ -8,3 +8,6 @@ CFLAGS_trace.o := -I$(src)
 snd-hda-core-$(CONFIG_SND_HDA_I915) += hdac_i915.o
 
 obj-$(CONFIG_SND_HDA_CORE) += snd-hda-core.o
+
+#extended hda
+obj-$(CONFIG_SND_HDA_EXT_CORE) += ext/
diff --git a/sound/hda/ext/Makefile b/sound/hda/ext/Makefile
new file mode 100644
index 000000000000..2e583e664916
--- /dev/null
+++ b/sound/hda/ext/Makefile
@@ -0,0 +1,3 @@
+snd-hda-ext-core-objs := hdac_ext_bus.o
+
+obj-$(CONFIG_SND_HDA_EXT_CORE) += snd-hda-ext-core.o
diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c
new file mode 100644
index 000000000000..b5dbf33be72d
--- /dev/null
+++ b/sound/hda/ext/hdac_ext_bus.c
@@ -0,0 +1,115 @@
+/*
+ *  hdac-ext-bus.c - HD-audio extended 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/hdaudio_ext.h>
+
+/**
+ * snd_hdac_ext_bus_init - initialize a HD-audio extended bus
+ * @sbus: the pointer to extended 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_hdac_ext_bus_init(struct hdac_ext_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_hdac_ext_bus_init);
+
+/**
+ * snd_hdac_ext_bus_exit - clean up a HD-audio extended bus
+ * @sbus: the pointer to extended bus object
+ */
+void snd_hdac_ext_bus_exit(struct hdac_ext_bus *sbus)
+{
+	snd_hdac_bus_exit(&sbus->bus);
+	WARN_ON(!list_empty(&sbus->hlink_list));
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_exit);
+
+static void default_release(struct device *dev)
+{
+	snd_hdac_ext_bus_device_exit(container_of(dev, struct hdac_device, dev));
+}
+
+/**
+ * snd_hdac_ext_device_init - initialize the HDA extended codec base device
+ * @sbus: hdac extended bus to attach to
+ * @addr: codec address
+ *
+ * Returns zero for success or a negative error code.
+ */
+int snd_hdac_ext_bus_device_init(struct hdac_ext_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), "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_hdac_ext_bus_device_exit(hdev);
+		return ret;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_init);
+
+/**
+ * snd_hdac_ext_bus_device_exit - clean up a HD-audio extended codec base device
+ * @hdev: hdac device to clean up
+ */
+void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev)
+{
+	snd_hdac_device_exit(hdev);
+	kfree(hdev);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_exit);
-- 
1.9.1

[PATCH v6 2/3] ALSA: hdac_ext: add hdac extended controller

[Vinod Koul]

The controller needs to support the new capabilities and allow reading,
parsing and initializing of these capabilities, so this patch does it

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 include/sound/hdaudio_ext.h         | 174 +++++++++++++++++++++
 sound/hda/ext/Makefile              |   2 +-
 sound/hda/ext/hdac_ext_controller.c | 295 ++++++++++++++++++++++++++++++++++++
 3 files changed, 470 insertions(+), 1 deletion(-)
 create mode 100644 sound/hda/ext/hdac_ext_controller.c

diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
index df1f8ae64693..b17d7f91f6ac 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -90,4 +90,178 @@ void snd_hdac_ext_bus_ppcap_int_enable(struct hdac_ext_bus *chip, bool enable);
 			       (snd_hdac_ext_bus_ppcap_readb(dev, reg) & \
 				~(mask)) | (val))
 
+void snd_hdac_ext_stream_spbcap_enable(struct hdac_ext_bus *chip,
+				 bool enable, int index);
+/*
+ * macros for spcap register read/write
+ */
+#define _snd_hdac_ext_bus_spbcap_write(type, dev, reg, value)	\
+	((dev)->bus.io_ops->reg_write ## type(value,		\
+	(dev)->spbcap_addr + (reg)))
+#define _snd_hdac_ext_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 snd_hdac_ext_bus_spbcap_writel(dev, reg, value) \
+	_snd_hdac_ext_bus_spbcap_write(l, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_bus_spbcap_writew(dev, reg, value) \
+	_snd_hdac_ext_bus_spbcap_write(w, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_bus_spbcap_writeb(dev, reg, value) \
+	_snd_hdac_ext_bus_spbcap_write(b, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_bus_spbcap_readl(dev, reg) \
+	_snd_hdac_ext_bus_spbcap_read(l, dev, AZX_REG_ ## reg)
+#define snd_hdac_ext_bus_spbcap_readw(dev, reg) \
+	_snd_hdac_ext_bus_spbcap_read(w, dev, AZX_REG_ ## reg)
+#define snd_hdac_ext_bus_spbcap_readb(dev, reg) \
+	_snd_hdac_ext_bus_spbcap_read(b, dev, AZX_REG_ ## reg)
+
+/* update a register, pass without AZX_REG_ prefix */
+#define snd_hdac_ext_bus_spbcap_updatel(dev, reg, mask, val) \
+	snd_hdac_ext_bus_spbcap_writel(dev, reg, \
+			       (snd_hdac_ext_bus_spbcap_readl(dev, reg) & \
+				~(mask)) | (val))
+#define snd_hdac_ext_bus_spbcap_updatew(dev, reg, mask, val) \
+	snd_hdac_ext_bus_spbcap_writew(dev, reg, \
+			       (snd_hdac_ext_bus_spbcap_readw(dev, reg) & \
+				~(mask)) | (val))
+#define snd_hdac_ext_bus_spbcap_updateb(dev, reg, mask, val) \
+	snd_hdac_ext_bus_spbcap_writeb(dev, reg, \
+			       (snd_hdac_ext_bus_spbcap_readb(dev, reg) & \
+				~(mask)) | (val))
+
+int snd_hdac_ext_bus_get_ml_capabilities(struct hdac_ext_bus *bus);
+int snd_hdac_ext_bus_map_codec_to_link(struct hdac_ext_bus *bus, int addr);
+struct hdac_ext_link *snd_hdac_ext_bus_get_link(struct hdac_ext_bus *bus,
+						const char *codec_name);
+
+/*
+ * macros for mlcap register read/write
+ */
+#define _snd_hdac_ext_bus_mlcap_write(type, dev, reg, value)		\
+	((dev)->bus.io_ops->reg_write ## type(value, (dev)->mlcap_addr + (reg)))
+#define _snd_hdac_ext_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 snd_hdac_ext_bus_mlcap_writel(dev, reg, value) \
+	_snd_hdac_ext_bus_mlcap_write(l, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_bus_mlcap_writew(dev, reg, value) \
+	_snd_hdac_ext_bus_mlcap_write(w, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_bus_mlcap_writeb(dev, reg, value) \
+	_snd_hdac_ext_bus_mlcap_write(b, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_bus_mlcap_readl(dev, reg) \
+	_snd_hdac_ext_bus_mlcap_read(l, dev, AZX_REG_ ## reg)
+#define snd_hdac_ext_bus_mlcap_readw(dev, reg) \
+	_snd_hdac_ext_bus_mlcap_read(w, dev, AZX_REG_ ## reg)
+#define snd_hdac_ext_bus_mlcap_readb(dev, reg) \
+	_snd_hdac_ext_bus_mlcap_read(b, dev, AZX_REG_ ## reg)
+
+/* update a register, pass without AZX_REG_ prefix */
+#define snd_hdac_ext_bus_mlcap_updatel(dev, reg, mask, val) \
+	snd_hdac_ext_bus_mlcap_writel(dev, reg, \
+			       (snd_hdac_ext_bus_mlcap_readl(dev, reg) & \
+				~(mask)) | (val))
+#define snd_hdac_ext_bus_mlcap_updatew(dev, reg, mask, val) \
+	snd_hdac_ext_bus_mlcap_writew(dev, reg, \
+			       (snd_hdac_ext_bus_mlcap_readw(dev, reg) & \
+				~(mask)) | (val))
+#define snd_hdac_ext_bus_mlcap_updateb(dev, reg, mask, val) \
+	snd_hdac_ext_bus_mlcap_writeb(dev, reg, \
+			       (snd_hdac_ext_bus_mlcap_readb(dev, reg) & \
+				~(mask)) | (val))
+
+/*
+ * macros for gtscap register read/write
+ */
+#define _snd_hdac_ext_bus_gtscap_write(type, dev, reg, value)	\
+	((dev)->bus.io_ops->reg_write ## type(value,		\
+	(dev)->gtscap_addr + (reg)))
+#define _snd_hdac_ext_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 snd_hdac_ext_bus_gtscap_writel(dev, reg, value) \
+	_snd_hdac_ext_bus_gtscap_write(l, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_bus_gtscap_writew(dev, reg, value) \
+	_snd_hdac_ext_bus_gtscap_write(w, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_bus_gtscap_writeb(dev, reg, value) \
+	_snd_hdac_ext_bus_gtscap_write(b, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_bus_gtscap_readl(dev, reg) \
+	_snd_hdac_ext_bus_gtscap_read(l, dev, AZX_REG_ ## reg)
+#define snd_hdac_ext_bus_gtscap_readw(dev, reg) \
+	_snd_hdac_ext_bus_gtscap_read(w, dev, AZX_REG_ ## reg)
+#define snd_hdac_ext_bus_gtscap_readb(dev, reg) \
+	_snd_hdac_ext_bus_gtscap_read(b, dev, AZX_REG_ ## reg)
+
+/* update a register, pass without AZX_REG_ prefix */
+#define snd_hdac_ext_bus_gtscap_updatel(dev, reg, mask, val) \
+	snd_hdac_ext_bus_gtscap_writel(dev, reg, \
+			       (snd_hdac_ext_bus_gtscap_readl(dev, reg) & \
+				~(mask)) | (val))
+#define snd_hdac_ext_bus_gtscap_updatew(dev, reg, mask, val) \
+	snd_hdac_ext_bus_gtscap_writew(dev, reg, \
+			       (snd_hdac_ext_bus_gtscap_readw(dev, reg) & \
+				~(mask)) | (val))
+#define snd_hdac_ext_bus_gtscap_updateb(dev, reg, mask, val) \
+	snd_hdac_ext_bus_gtscap_writeb(dev, reg, \
+			       (snd_hdac_ext_bus_gtscap_readb(dev, reg) & \
+				~(mask)) | (val))
+enum hdac_ext_stream_type {
+	HDAC_EXT_STREAM_TYPE_COUPLED = 0,
+	HDAC_EXT_STREAM_TYPE_HOST,
+	HDAC_EXT_STREAM_TYPE_LINK
+};
+
+struct hdac_ext_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_hdac_ext_bus_link_power_up(struct hdac_ext_link *link);
+int snd_hdac_ext_bus_link_power_down(struct hdac_ext_link *link);
+void snd_hdac_ext_link_set_stream_id(struct hdac_ext_link *link,
+				 int stream);
+void snd_hdac_ext_link_clear_stream_id(struct hdac_ext_link *link,
+				 int stream);
+
+/*
+ * macros for mutilink register read/ write
+ */
+#define _snd_hdac_ext_link_write(type, dev, reg, value)			\
+	((dev)->bus->io_ops->reg_write ## type(value, (dev)->ml_addr + (reg)))
+#define _snd_hdac_ext_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 snd_hdac_ext_link_writel(dev, reg, value) \
+	_snd_hdac_ext_link_write(l, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_link_writew(dev, reg, value) \
+	_snd_hdac_ext_link_write(w, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_link_writeb(dev, reg, value) \
+	_snd_hdac_ext_link_write(b, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_link_readl(dev, reg) \
+	_snd_hdac_ext_link_read(l, dev, AZX_REG_ ## reg)
+#define snd_hdac_ext_link_readw(dev, reg) \
+	_snd_hdac_ext_link_read(w, dev, AZX_REG_ ## reg)
+#define snd_hdac_ext_link_readb(dev, reg) \
+	_snd_hdac_ext_link_read(b, dev, AZX_REG_ ## reg)
+
+/* update a register, pass without AZX_REG_ prefix */
+#define snd_hdac_ext_link_updatel(dev, reg, mask, val) \
+	snd_hdac_ext_link_writel(dev, reg, \
+			       (snd_hdac_ext_link_readl(dev, reg) & \
+				~(mask)) | (val))
+#define snd_hdac_ext_link_updatew(dev, reg, mask, val) \
+	snd_hdac_ext_link_writew(dev, reg, \
+			       (snd_hdac_ext_link_readw(dev, reg) & \
+				~(mask)) | (val))
+#define snd_hdac_ext_link_updateb(dev, reg, mask, val) \
+	snd_hdac_ext_link_writeb(dev, reg, \
+			       (snd_hdac_ext_link_readb(dev, reg) & \
+				~(mask)) | (val))
 #endif /* __SOUND_HDAUDIO_EXT_H */
diff --git a/sound/hda/ext/Makefile b/sound/hda/ext/Makefile
index 2e583e664916..934b7bf5d87f 100644
--- a/sound/hda/ext/Makefile
+++ b/sound/hda/ext/Makefile
@@ -1,3 +1,3 @@
-snd-hda-ext-core-objs := hdac_ext_bus.o
+snd-hda-ext-core-objs := hdac_ext_bus.o hdac_ext_controller.o
 
 obj-$(CONFIG_SND_HDA_EXT_CORE) += snd-hda-ext-core.o
diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c
new file mode 100644
index 000000000000..ab68d8165085
--- /dev/null
+++ b/sound/hda/ext/hdac_ext_controller.c
@@ -0,0 +1,295 @@
+/*
+ *  hdac-ext-controller.c - HD-audio extended 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/hdaudio_ext.h>
+
+/**
+ * snd_hdac_ext_bus_parse_capabilities - parse capablity structure
+ * @sbus: the pointer to extended bus object
+ *
+ * Returns 0 if successful, or a negative error code.
+ */
+int snd_hdac_ext_bus_parse_capabilities(struct hdac_ext_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_hdac_ext_bus_parse_capabilities);
+
+/*
+ * processing pipe helpers - these helpers are useful for dealing with HDA
+ * new capability of processing pipelines
+ */
+
+/**
+ * snd_hdac_ext_bus_ppcap_enable - enable/disable processing pipe capability
+ * @sbus: HD-audio extended core bus
+ * @enable: flag to turn on/off the capability
+ */
+void snd_hdac_ext_bus_ppcap_enable(struct hdac_ext_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)
+		snd_hdac_ext_bus_ppcap_updatel(sbus, PP_PPCTL, 0, AZX_PPCTL_GPROCEN);
+	else
+		snd_hdac_ext_bus_ppcap_updatel(sbus, PP_PPCTL, AZX_PPCTL_GPROCEN, 0);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_ppcap_enable);
+
+/**
+ * snd_hdac_ext_bus_ppcap_int_enable - ppcap interrupt enable/disable
+ * @sbus: HD-audio extended core bus
+ * @enable: flag to enable/disable interrupt
+ */
+void snd_hdac_ext_bus_ppcap_int_enable(struct hdac_ext_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)
+		snd_hdac_ext_bus_ppcap_updatel(sbus, PP_PPCTL, 0, AZX_PPCTL_PIE);
+	else
+		snd_hdac_ext_bus_ppcap_updatel(sbus, PP_PPCTL, AZX_PPCTL_PIE, 0);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_ppcap_int_enable);
+
+/*
+ * Multilink helpers - these helpers are useful for dealing with HDA
+ * new multilink capability
+ */
+
+/**
+ * snd_hdac_ext_bus_get_ml_capabilities - get multilink capability
+ * @sbus: HD-audio extended core bus
+ *
+ * This will parse all links and read the mlink capabilities and add them
+ * in hlink_list of extended hdac bus
+ * Note: this will be freed on bus exit by driver
+ */
+int snd_hdac_ext_bus_get_ml_capabilities(struct hdac_ext_bus *sbus)
+{
+	int idx;
+	u32 link_count;
+	struct hdac_ext_link *hlink;
+	struct hdac_bus *bus = &sbus->bus;
+
+	link_count = snd_hdac_ext_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  = snd_hdac_ext_link_readl(hlink, ML_LCAP);
+		hlink->lsdiid = snd_hdac_ext_link_readw(hlink, ML_LSDIID);
+
+		list_add_tail(&hlink->list, &sbus->hlink_list);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_get_ml_capabilities);
+
+/**
+ * snd_hdac_ext_bus_map_codec_to_link - maps codec to link
+ * @sbus: HD-audio extended core bus
+ * @addr: codec address
+ */
+int snd_hdac_ext_bus_map_codec_to_link(struct hdac_ext_bus *sbus, int addr)
+{
+	struct hdac_ext_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_hdac_ext_bus_map_codec_to_link);
+
+/**
+ * snd_hdac_ext_bus_get_link_index - get link based on codec name
+ * @sbus: HD-audio extended core bus
+ * @codec_name: codec name
+ */
+struct hdac_ext_link *snd_hdac_ext_bus_get_link(struct hdac_ext_bus *sbus,
+						 const char *codec_name)
+{
+	int i;
+	struct hdac_ext_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_hdac_ext_bus_get_link);
+
+static int check_hdac_link_power_active(struct hdac_ext_link *link, bool enable)
+{
+	int timeout;
+	u32 val;
+	int mask = (1 << AZX_MLCTL_CPA);
+
+	udelay(3);
+	timeout = 50;
+
+	do {
+		val = snd_hdac_ext_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_hdac_ext_bus_link_power_up -power up hda link
+ * @link: HD-audio extended link
+ */
+int snd_hdac_ext_bus_link_power_up(struct hdac_ext_link *link)
+{
+	snd_hdac_ext_link_updatel(link, ML_LCTL, 0, AZX_MLCTL_SPA);
+
+	return check_hdac_link_power_active(link, true);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_power_up);
+
+/**
+ * snd_hdac_ext_bus_link_power_down -power down hda link
+ * @link: HD-audio extended link
+ */
+int snd_hdac_ext_bus_link_power_down(struct hdac_ext_link *link)
+{
+	snd_hdac_ext_link_updatel(link, ML_LCTL, AZX_MLCTL_SPA, 0);
+
+	return check_hdac_link_power_active(link, false);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_power_down);
+
+MODULE_DESCRIPTION("HDA extended core");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH v6 3/3] ALSA: hdac_ext: add extended stream capabilities

[Vinod Koul]

Now we have the bus and controller code added to find and initialize the
extended capabilities. Now we need to use them in stream code to decouple
stream, manage links etc

So this patch adds the stream handling code for extended capabilities
introduced in preceding patches

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 include/sound/hdaudio_ext.h     | 118 ++++++++++++
 sound/hda/ext/Makefile          |   2 +-
 sound/hda/ext/hdac_ext_stream.c | 414 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 533 insertions(+), 1 deletion(-)
 create mode 100644 sound/hda/ext/hdac_ext_stream.c

diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
index b17d7f91f6ac..05982dc7e3cc 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -212,6 +212,124 @@ enum hdac_ext_stream_type {
 	HDAC_EXT_STREAM_TYPE_LINK
 };
 
+/**
+ * hdac_ext_stream: HDAC extended stream for extended HDA caps
+ *
+ * @hstream: hdac_stream
+ * @pphc_addr: processing pipe host stream pointer
+ * @pplc_addr: processing pipe link stream pointer
+ * @decoupled: stream host and link is decoupled
+ * @link_locked: link is locked
+ * @link_prepared: link is prepared
+ * link_substream: link substream
+ */
+struct hdac_ext_stream {
+	struct hdac_stream hstream;
+
+	void __iomem *pphc_addr;
+	void __iomem *pplc_addr;
+
+	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_hdac_ext_stream(s) \
+	container_of(s, struct hdac_ext_stream, hstream)
+
+void snd_hdac_ext_stream_init(struct hdac_ext_bus *bus,
+				struct hdac_ext_stream *stream, int idx,
+				int direction, int tag);
+struct hdac_ext_stream *snd_hdac_ext_stream_assign(struct hdac_ext_bus *bus,
+					   struct snd_pcm_substream *substream,
+					   int type);
+void snd_hdac_ext_stream_release(struct hdac_ext_stream *azx_dev, int type);
+void snd_hdac_ext_stream_decouple(struct hdac_ext_bus *bus,
+				struct hdac_ext_stream *azx_dev, bool decouple);
+void snd_hdac_ext_stop_streams(struct hdac_ext_bus *sbus);
+
+/*
+ * macros for host stream register read/write
+ */
+#define _snd_hdac_ext_host_stream_write(type, dev, reg, value)	\
+	((dev)->hstream.bus->io_ops->reg_write ## type(value,	\
+	(dev)->pphc_addr + (reg)))
+#define _snd_hdac_ext_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 snd_hdac_ext_host_stream_writel(dev, reg, value) \
+	_snd_hdac_ext_host_stream_write(l, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_host_stream_writew(dev, reg, value) \
+	_snd_hdac_ext_host_stream_write(w, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_host_stream_writeb(dev, reg, value) \
+	_snd_hdac_ext_host_stream_write(b, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_host_stream_readl(dev, reg) \
+	_snd_hdac_ext_host_stream_read(l, dev, AZX_REG_ ## reg)
+#define snd_hdac_ext_host_stream_readw(dev, reg) \
+	_snd_hdac_ext_host_stream_read(w, dev, AZX_REG_ ## reg)
+#define snd_hdac_ext_host_stream_readb(dev, reg) \
+	_snd_hdac_ext_host_stream_read(b, dev, AZX_REG_ ## reg)
+
+/* update a register, pass without AZX_REG_ prefix */
+#define snd_hdac_ext_host_stream_updatel(dev, reg, mask, val) \
+	snd_hdac_ext_host_stream_writel(dev, reg, \
+			       (snd_hdac_ext_host_stream_readl(dev, reg) & \
+				~(mask)) | (val))
+#define snd_hdac_ext_host_stream_updatew(dev, reg, mask, val) \
+	snd_hdac_ext_host_stream_writew(dev, reg, \
+			       (snd_hdac_ext_host_stream_readw(dev, reg) & \
+				~(mask)) | (val))
+#define snd_hdac_ext_host_stream_updateb(dev, reg, mask, val) \
+	snd_hdac_ext_host_stream_writeb(dev, reg, \
+			       (snd_hdac_ext_host_stream_readb(dev, reg) & \
+				~(mask)) | (val))
+
+void snd_hdac_ext_link_stream_start(struct hdac_ext_stream *hstream);
+void snd_hdac_ext_link_stream_clear(struct hdac_ext_stream *hstream);
+void snd_hdac_ext_link_stream_reset(struct hdac_ext_stream *hstream);
+int snd_hdac_ext_link_stream_setup(struct hdac_ext_stream *stream, int fmt);
+
+/*
+ * macros for link stream register read/write
+ */
+#define _snd_hdac_ext_link_stream_write(type, dev, reg, value)	\
+	((dev)->hstream.bus->io_ops->reg_write ## type(value,	\
+	(dev)->pplc_addr + (reg)))
+#define _snd_hdac_ext_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 snd_hdac_ext_link_stream_writel(dev, reg, value) \
+	_snd_hdac_ext_link_stream_write(l, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_link_stream_writew(dev, reg, value) \
+	_snd_hdac_ext_link_stream_write(w, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_link_stream_writeb(dev, reg, value) \
+	_snd_hdac_ext_link_stream_write(b, dev, AZX_REG_ ## reg, value)
+#define snd_hdac_ext_link_stream_readl(dev, reg) \
+	_snd_hdac_ext_link_stream_read(l, dev, AZX_REG_ ## reg)
+#define snd_hdac_ext_link_stream_readw(dev, reg) \
+	_snd_hdac_ext_link_stream_read(w, dev, AZX_REG_ ## reg)
+#define snd_hdac_ext_link_stream_readb(dev, reg) \
+	_snd_hdac_ext_link_stream_read(b, dev, AZX_REG_ ## reg)
+
+/* update a register, pass without AZX_REG_ prefix */
+#define snd_hdac_ext_link_stream_updatel(dev, reg, mask, val) \
+	snd_hdac_ext_link_stream_writel(dev, reg, \
+			       (snd_hdac_ext_link_stream_readl(dev, reg) & \
+				~(mask)) | (val))
+#define snd_hdac_ext_link_stream_updatew(dev, reg, mask, val) \
+	snd_hdac_ext_link_stream_writew(dev, reg, \
+			       (snd_hdac_ext_link_stream_readw(dev, reg) & \
+				~(mask)) | (val))
+#define snd_hdac_ext_link_stream_updateb(dev, reg, mask, val) \
+	snd_hdac_ext_link_stream_writeb(dev, reg, \
+			       (snd_hdac_ext_link_stream_readb(dev, reg) & \
+				~(mask)) | (val))
 struct hdac_ext_link {
 	struct hdac_bus *bus;
 	int index;
diff --git a/sound/hda/ext/Makefile b/sound/hda/ext/Makefile
index 934b7bf5d87f..9b6f641c7777 100644
--- a/sound/hda/ext/Makefile
+++ b/sound/hda/ext/Makefile
@@ -1,3 +1,3 @@
-snd-hda-ext-core-objs := hdac_ext_bus.o hdac_ext_controller.o
+snd-hda-ext-core-objs := hdac_ext_bus.o hdac_ext_controller.o hdac_ext_stream.o
 
 obj-$(CONFIG_SND_HDA_EXT_CORE) += snd-hda-ext-core.o
diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c
new file mode 100644
index 000000000000..f95dce6b9d0a
--- /dev/null
+++ b/sound/hda/ext/hdac_ext_stream.c
@@ -0,0 +1,414 @@
+/*
+ *  hdac-ext-stream.c - HD-audio extended 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/hdaudio_ext.h>
+
+/**
+ * snd_hdac_ext_stream_init - initialize each stream (aka device)
+ * @sbus: HD-audio ext core bus
+ * @stream: HD-audio ext 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
+ *
+ * initialize teh stream, if ppcap is enabled then init those and then
+ * invoke hdac stream initialization routine
+ */
+void snd_hdac_ext_stream_init(struct hdac_ext_bus *sbus,
+				struct hdac_ext_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_hdac_ext_stream_init);
+
+/**
+ * snd_hdac_ext_stream_decouple - decouple the hdac stream
+ * @sbus: HD-audio ext core bus
+ * @stream: HD-audio ext core stream object to initialize
+ * @decouple: flag to decouple
+ */
+void snd_hdac_ext_stream_decouple(struct hdac_ext_bus *sbus,
+				struct hdac_ext_stream *stream, bool decouple)
+{
+	struct hdac_stream *hstream = &stream->hstream;
+	struct hdac_bus *bus = &sbus->bus;
+
+	spin_lock_irq(&bus->reg_lock);
+	if (decouple)
+		snd_hdac_ext_bus_ppcap_updatel(sbus, PP_PPCTL, 0,
+						AZX_PPCTL_PROCEN(hstream->index));
+	else
+		snd_hdac_ext_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_hdac_ext_stream_decouple);
+
+/**
+ * snd_hdac_ext_linkstream_start - start a stream
+ * @stream: HD-audio ext core stream to start
+ */
+void snd_hdac_ext_link_stream_start(struct hdac_ext_stream *stream)
+{
+	snd_hdac_ext_link_stream_updatel(stream, PPLCCTL,
+				0, AZX_PPLCCTL_RUN);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_start);
+
+/**
+ * snd_hdac_ext_link_stream_clear - stop a stream DMA
+ * @stream: HD-audio ext core stream to stop
+ */
+void snd_hdac_ext_link_stream_clear(struct hdac_ext_stream *stream)
+{
+	snd_hdac_ext_link_stream_updatel(stream, PPLCCTL,
+					AZX_PPLCCTL_RUN, 0);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_clear);
+
+/**
+ * snd_hdac_ext_link_stream_reset - reset a stream
+ * @stream: HD-audio ext core stream to reset
+ */
+void snd_hdac_ext_link_stream_reset(struct hdac_ext_stream *stream)
+{
+	unsigned char val;
+	int timeout;
+
+	snd_hdac_ext_link_stream_clear(stream);
+
+	snd_hdac_ext_link_stream_updatel(stream, PPLCCTL, 0, AZX_PPLCCTL_STRST);
+	udelay(3);
+	timeout = 50;
+	do {
+		val = snd_hdac_ext_link_stream_readl(stream, PPLCCTL) &
+			AZX_PPLCCTL_STRST;
+		if (val)
+			break;
+		udelay(3);
+	} while (--timeout);
+	val &= ~AZX_PPLCCTL_STRST;
+	snd_hdac_ext_link_stream_writel(stream, PPLCCTL, val);
+	udelay(3);
+
+	timeout = 50;
+	/* waiting for hardware to report that the stream is out of reset */
+	do {
+		val = snd_hdac_ext_link_stream_readl(stream, PPLCCTL) &
+			AZX_PPLCCTL_STRST;
+		if (!val)
+			break;
+		udelay(3);
+	} while (--timeout);
+
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_reset);
+
+/**
+ * snd_hdac_ext_link_stream_setup -  set up the SD for streaming
+ * @stream: HD-audio ext core stream to set up
+ * @fmt: stream format
+ */
+int snd_hdac_ext_link_stream_setup(struct hdac_ext_stream *stream, int fmt)
+{
+	struct hdac_stream *hstream = &stream->hstream;
+	unsigned int val;
+
+	/* make sure the run bit is zero for SD */
+	snd_hdac_ext_link_stream_clear(stream);
+	/* program the stream_tag */
+	val = snd_hdac_ext_link_stream_readl(stream, PPLCCTL);
+	val = (val & ~AZX_PPLCCTL_STRM_MASK) |
+		(hstream->stream_tag << AZX_PPLCCTL_STRM_SHIFT);
+	snd_hdac_ext_link_stream_writel(stream, PPLCCTL, val);
+
+	/* program the stream format */
+	snd_hdac_ext_link_stream_writew(stream, PPLCFMT, fmt);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_setup);
+
+/**
+ * snd_hdac_ext_link_set_stream_id - maps stream id to link output
+ * @link: HD-audio ext link to set up
+ * @stream: stream id
+ */
+void snd_hdac_ext_link_set_stream_id(struct hdac_ext_link *link,
+				 int stream)
+{
+	snd_hdac_ext_link_updatew(link, ML_LOSIDV, (1 << stream), 0);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_link_set_stream_id);
+
+/**
+ * snd_hdac_ext_link_clear_stream_id - maps stream id to link output
+ * @link: HD-audio ext link to set up
+ * @stream: stream id
+ */
+void snd_hdac_ext_link_clear_stream_id(struct hdac_ext_link *link,
+				 int stream)
+{
+	snd_hdac_ext_link_updatew(link, ML_LOSIDV, 0, (1 << stream));
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_link_clear_stream_id);
+
+static struct hdac_ext_stream *
+hdac_ext_link_stream_assign(struct hdac_ext_bus *sbus,
+				struct snd_pcm_substream *substream)
+{
+	struct hdac_ext_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 hdac_ext_stream *hstream = container_of(stream,
+						struct hdac_ext_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_hdac_ext_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 hdac_ext_stream *
+hdac_ext_host_stream_assign(struct hdac_ext_bus *sbus,
+				struct snd_pcm_substream *substream)
+{
+	struct hdac_ext_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 hdac_ext_stream *hstream = container_of(stream,
+						struct hdac_ext_stream,
+						hstream);
+		if (stream->direction != substream->stream)
+			continue;
+
+		if (stream->opened) {
+			if (!hstream->decoupled)
+				snd_hdac_ext_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_hdac_ext_stream_assign - assign a stream for the PCM
+ * @sbus: HD-audio ext 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 hdac_ext_stream *snd_hdac_ext_stream_assign(struct hdac_ext_bus *sbus,
+					   struct snd_pcm_substream *substream,
+					   int type)
+{
+	struct hdac_ext_stream *hstream = NULL;
+	struct hdac_stream *stream = NULL;
+	struct hdac_bus *hbus = &sbus->bus;
+
+	switch (type) {
+	case HDAC_EXT_STREAM_TYPE_COUPLED:
+		stream = snd_hdac_stream_assign(hbus, substream);
+		if (stream)
+			hstream = container_of(stream,
+					struct hdac_ext_stream, hstream);
+		return hstream;
+
+	case HDAC_EXT_STREAM_TYPE_HOST:
+		return hdac_ext_host_stream_assign(sbus, substream);
+
+	case HDAC_EXT_STREAM_TYPE_LINK:
+		return hdac_ext_link_stream_assign(sbus, substream);
+
+	default:
+		return NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_assign);
+
+/**
+ * snd_hdac_ext_stream_release - release the assigned stream
+ * @stream: HD-audio ext core stream to release
+ * @type: type of stream (coupled, host or link stream)
+ *
+ * Release the stream that has been assigned by snd_hdac_ext_stream_assign().
+ */
+void snd_hdac_ext_stream_release(struct hdac_ext_stream *stream, int type)
+{
+	struct hdac_bus *bus = stream->hstream.bus;
+	struct hdac_ext_bus *sbus = bus_to_hdac_ext_bus(bus);
+
+	switch (type) {
+	case HDAC_EXT_STREAM_TYPE_COUPLED:
+		snd_hdac_stream_release(&stream->hstream);
+		break;
+
+	case HDAC_EXT_STREAM_TYPE_HOST:
+		if (stream->decoupled) {
+			snd_hdac_ext_stream_decouple(sbus, stream, false);
+			snd_hdac_stream_release(&stream->hstream);
+		}
+		break;
+
+	case HDAC_EXT_STREAM_TYPE_LINK:
+		if (stream->decoupled)
+			snd_hdac_ext_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 %d\n", type);
+	}
+
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_release);
+
+/**
+ * snd_hdac_ext_stream_spbcap_enable - enable SPIB for a stream
+ * @sbus: HD-audio ext core bus
+ * @enable: flag to enable/disable SPIB
+ * @index: stream index for which SPIB need to be enabled
+ */
+void snd_hdac_ext_stream_spbcap_enable(struct hdac_ext_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 = snd_hdac_ext_bus_spbcap_readl(sbus, SPB_SPBFCCTL);
+
+	mask |= register_mask;
+
+	if (enable)
+		snd_hdac_ext_bus_spbcap_updatel(sbus, SPB_SPBFCCTL, 0, mask);
+	else
+		snd_hdac_ext_bus_spbcap_updatel(sbus, SPB_SPBFCCTL, mask, 0);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_spbcap_enable);
+
+/**
+ * snd_hdac_ext_stop_streams - stop all stream if running
+ * @sbus: HD-audio ext core bus
+ */
+void snd_hdac_ext_stop_streams(struct hdac_ext_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_hdac_ext_stop_streams);
-- 
1.9.1

Re: [PATCH v6 1/3] ALSA: hdac_ext: add extended HDA bus

[Takashi Iwai]

At Thu,  4 Jun 2015 15:23:20 +0530,
Vinod Koul wrote:
> 
> The new HDA controllers from Intel support new capabilities like multilink,
> pipe processing, SPIB, GTS etc In order to use them we create an extended HDA
> bus which embed the hdac bus and contains the fields for extended
> configurations
> 
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  include/sound/hdaudio_ext.h  |  93 ++++++++++++++++++++++++++++++++++
>  sound/hda/Kconfig            |   4 ++
>  sound/hda/Makefile           |   3 ++
>  sound/hda/ext/Makefile       |   3 ++
>  sound/hda/ext/hdac_ext_bus.c | 115 +++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 218 insertions(+)
>  create mode 100644 include/sound/hdaudio_ext.h
>  create mode 100644 sound/hda/ext/Makefile
>  create mode 100644 sound/hda/ext/hdac_ext_bus.c
> 
> diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
> new file mode 100644
> index 000000000000..df1f8ae64693
> --- /dev/null
> +++ b/include/sound/hdaudio_ext.h
> @@ -0,0 +1,93 @@
> +#ifndef __SOUND_HDAUDIO_EXT_H
> +#define __SOUND_HDAUDIO_EXT_H
> +
> +#include <sound/core.h>
> +#include <sound/initval.h>

Do we need this header?

> +#include <sound/hda_register.h>
> +#include <sound/hdaudio.h>
> +
> +/**
> + * hdac_ext_bus: HDAC extended bus for extended HDA caps
> + *
> + * @bus: hdac bus
> + * @num_streams: streams supported
> + * @ppcap: HDA ext post processing caps supported
> + * @spbcap: HDA ext Software Position in Buffer cap support
> + * @mlcap: HDA ext MultiLink cap support
> + * @gtscap: HDA ext Global Time Sync cap support
> + * @ppcap_addr: pp capabilities pointer
> + * @spbcap_addr: SPIB capabilities pointer
> + * @mlcap_addr: MultiLink capabilities pointer
> + * @gtscap_addr: gts capabilities pointer
> + * @hlink_list: link list of HDA links
> + */
> +struct hdac_ext_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;
> +
> +	struct list_head hlink_list;
> +};
> +
> +int snd_hdac_ext_bus_init(struct hdac_ext_bus *sbus, struct device *dev,
> +		      const struct hdac_bus_ops *ops,
> +		      const struct hdac_io_ops *io_ops);
> +
> +void snd_hdac_ext_bus_exit(struct hdac_ext_bus *sbus);
> +int snd_hdac_ext_bus_device_init(struct hdac_ext_bus *sbus, int addr);
> +void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev);
> +
> +#define hdac_bus(sbus)	(&(sbus)->bus)

This macro name is a bit too ambiguous.  If it were really a generic
cast, I wouldn't mind.  But it's specific to this type.

> +#define bus_to_hdac_ext_bus(_bus) \
> +	container_of(_bus, struct hdac_ext_bus, bus)
> +
> +int snd_hdac_ext_bus_parse_capabilities(struct hdac_ext_bus *sbus);
> +void snd_hdac_ext_bus_ppcap_enable(struct hdac_ext_bus *chip, bool enable);
> +void snd_hdac_ext_bus_ppcap_int_enable(struct hdac_ext_bus *chip, bool enable);
> +
> +/*
> + * macros for ppcap register read/write
> + */
> +#define _snd_hdac_ext_bus_ppcap_write(type, dev, reg, value)		\
> +	((dev)->bus.io_ops->reg_write ## type(value, (dev)->ppcap_addr + (reg)))
> +#define _snd_hdac_ext_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 snd_hdac_ext_bus_ppcap_writel(dev, reg, value) \
> +	_snd_hdac_ext_bus_ppcap_write(l, dev, AZX_REG_ ## reg, value)
> +#define snd_hdac_ext_bus_ppcap_writew(dev, reg, value) \
> +	_snd_hdac_ext_bus_ppcap_write(w, dev, AZX_REG_ ## reg, value)
> +#define snd_hdac_ext_bus_ppcap_writeb(dev, reg, value) \
> +	_snd_hdac_ext_bus_ppcap_write(b, dev, AZX_REG_ ## reg, value)
> +#define snd_hdac_ext_bus_ppcap_readl(dev, reg) \
> +	_snd_hdac_ext_bus_ppcap_read(l, dev, AZX_REG_ ## reg)
> +#define snd_hdac_ext_bus_ppcap_readw(dev, reg) \
> +	_snd_hdac_ext_bus_ppcap_read(w, dev, AZX_REG_ ## reg)
> +#define snd_hdac_ext_bus_ppcap_readb(dev, reg) \
> +	_snd_hdac_ext_bus_ppcap_read(b, dev, AZX_REG_ ## reg)
> +
> +/* update a register, pass without AZX_REG_ prefix */
> +#define snd_hdac_ext_bus_ppcap_updatel(dev, reg, mask, val) \
> +	snd_hdac_ext_bus_ppcap_writel(dev, reg, \
> +			       (snd_hdac_ext_bus_ppcap_readl(dev, reg) & \
> +				~(mask)) | (val))
> +#define snd_hdac_ext_bus_ppcap_updatew(dev, reg, mask, val) \
> +	snd_hdac_ext_bus_ppcap_writew(dev, reg, \
> +			       (snd_hdac_ext_bus_ppcap_readw(dev, reg) & \
> +				~(mask)) | (val))
> +#define snd_hdac_ext_bus_ppcap_updateb(dev, reg, mask, val) \
> +	snd_hdac_ext_bus_ppcap_writeb(dev, reg, \
> +			       (snd_hdac_ext_bus_ppcap_readb(dev, reg) & \
> +				~(mask)) | (val))

It's not necessarily good to wrap all with such macros.
For azx_write*(), I kept them as is for reducing the amount of useless
code rewrites.  But for new codes, I don't think it's always worth...


thanks,

Takashi


> +#endif /* __SOUND_HDAUDIO_EXT_H */
> diff --git a/sound/hda/Kconfig b/sound/hda/Kconfig
> index ac5ffac2a272..6dc3914fd28b 100644
> --- a/sound/hda/Kconfig
> +++ b/sound/hda/Kconfig
> @@ -10,3 +10,7 @@ config SND_HDA_I915
>  	default y
>  	depends on DRM_I915
>  	depends on SND_HDA_CORE
> +
> +config SND_HDA_EXT_CORE
> +       tristate
> +       select SND_HDA_CORE
> diff --git a/sound/hda/Makefile b/sound/hda/Makefile
> index 55dd465c7042..7e999c995cdc 100644
> --- a/sound/hda/Makefile
> +++ b/sound/hda/Makefile
> @@ -8,3 +8,6 @@ CFLAGS_trace.o := -I$(src)
>  snd-hda-core-$(CONFIG_SND_HDA_I915) += hdac_i915.o
>  
>  obj-$(CONFIG_SND_HDA_CORE) += snd-hda-core.o
> +
> +#extended hda
> +obj-$(CONFIG_SND_HDA_EXT_CORE) += ext/
> diff --git a/sound/hda/ext/Makefile b/sound/hda/ext/Makefile
> new file mode 100644
> index 000000000000..2e583e664916
> --- /dev/null
> +++ b/sound/hda/ext/Makefile
> @@ -0,0 +1,3 @@
> +snd-hda-ext-core-objs := hdac_ext_bus.o
> +
> +obj-$(CONFIG_SND_HDA_EXT_CORE) += snd-hda-ext-core.o
> diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c
> new file mode 100644
> index 000000000000..b5dbf33be72d
> --- /dev/null
> +++ b/sound/hda/ext/hdac_ext_bus.c
> @@ -0,0 +1,115 @@
> +/*
> + *  hdac-ext-bus.c - HD-audio extended 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/hdaudio_ext.h>
> +
> +/**
> + * snd_hdac_ext_bus_init - initialize a HD-audio extended bus
> + * @sbus: the pointer to extended 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_hdac_ext_bus_init(struct hdac_ext_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_hdac_ext_bus_init);
> +
> +/**
> + * snd_hdac_ext_bus_exit - clean up a HD-audio extended bus
> + * @sbus: the pointer to extended bus object
> + */
> +void snd_hdac_ext_bus_exit(struct hdac_ext_bus *sbus)
> +{
> +	snd_hdac_bus_exit(&sbus->bus);
> +	WARN_ON(!list_empty(&sbus->hlink_list));
> +}
> +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_exit);
> +
> +static void default_release(struct device *dev)
> +{
> +	snd_hdac_ext_bus_device_exit(container_of(dev, struct hdac_device, dev));
> +}
> +
> +/**
> + * snd_hdac_ext_device_init - initialize the HDA extended codec base device
> + * @sbus: hdac extended bus to attach to
> + * @addr: codec address
> + *
> + * Returns zero for success or a negative error code.
> + */
> +int snd_hdac_ext_bus_device_init(struct hdac_ext_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), "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_hdac_ext_bus_device_exit(hdev);
> +		return ret;
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_init);
> +
> +/**
> + * snd_hdac_ext_bus_device_exit - clean up a HD-audio extended codec base device
> + * @hdev: hdac device to clean up
> + */
> +void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev)
> +{
> +	snd_hdac_device_exit(hdev);
> +	kfree(hdev);
> +}
> +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_exit);
> -- 
> 1.9.1
> 

Re: [PATCH v6 1/3] ALSA: hdac_ext: add extended HDA bus

[Takashi Iwai]

One thing forgot...

At Thu,  4 Jun 2015 15:23:20 +0530,
Vinod Koul wrote:
> 
> +/**
> + * snd_hdac_ext_device_init - initialize the HDA extended codec base device
> + * @sbus: hdac extended bus to attach to
> + * @addr: codec address
> + *
> + * Returns zero for success or a negative error code.
> + */
> +int snd_hdac_ext_bus_device_init(struct hdac_ext_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), "hda-codec#%03x", addr);
> +
> +	ret  = snd_hdac_device_init(hdev, bus, name, addr);

The device name must be unique to the whole system.  Using only the
codec address as an id would conflict if there are multiple cards.
The legacy HDA device consists of "hdaudioC%dD%d" indicating both the
card number and the codec address, for example.


Takashi

Re: [PATCH v6 2/3] ALSA: hdac_ext: add hdac extended controller

[Takashi Iwai]

At Thu,  4 Jun 2015 15:23:21 +0530,
Vinod Koul wrote:
> 
> The controller needs to support the new capabilities and allow reading,
> parsing and initializing of these capabilities, so this patch does it
> 
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  include/sound/hdaudio_ext.h         | 174 +++++++++++++++++++++
>  sound/hda/ext/Makefile              |   2 +-
>  sound/hda/ext/hdac_ext_controller.c | 295 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 470 insertions(+), 1 deletion(-)
>  create mode 100644 sound/hda/ext/hdac_ext_controller.c
> 
> diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
> index df1f8ae64693..b17d7f91f6ac 100644
> --- a/include/sound/hdaudio_ext.h
> +++ b/include/sound/hdaudio_ext.h
> @@ -90,4 +90,178 @@ void snd_hdac_ext_bus_ppcap_int_enable(struct hdac_ext_bus *chip, bool enable);
>  			       (snd_hdac_ext_bus_ppcap_readb(dev, reg) & \
>  				~(mask)) | (val))
>  
> +void snd_hdac_ext_stream_spbcap_enable(struct hdac_ext_bus *chip,
> +				 bool enable, int index);
> +/*
> + * macros for spcap register read/write
> + */
> +#define _snd_hdac_ext_bus_spbcap_write(type, dev, reg, value)	\
> +	((dev)->bus.io_ops->reg_write ## type(value,		\
> +	(dev)->spbcap_addr + (reg)))
> +#define _snd_hdac_ext_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 snd_hdac_ext_bus_spbcap_writel(dev, reg, value) \
> +	_snd_hdac_ext_bus_spbcap_write(l, dev, AZX_REG_ ## reg, value)
> +#define snd_hdac_ext_bus_spbcap_writew(dev, reg, value) \
> +	_snd_hdac_ext_bus_spbcap_write(w, dev, AZX_REG_ ## reg, value)
> +#define snd_hdac_ext_bus_spbcap_writeb(dev, reg, value) \
> +	_snd_hdac_ext_bus_spbcap_write(b, dev, AZX_REG_ ## reg, value)
> +#define snd_hdac_ext_bus_spbcap_readl(dev, reg) \
> +	_snd_hdac_ext_bus_spbcap_read(l, dev, AZX_REG_ ## reg)
> +#define snd_hdac_ext_bus_spbcap_readw(dev, reg) \
> +	_snd_hdac_ext_bus_spbcap_read(w, dev, AZX_REG_ ## reg)
> +#define snd_hdac_ext_bus_spbcap_readb(dev, reg) \
> +	_snd_hdac_ext_bus_spbcap_read(b, dev, AZX_REG_ ## reg)
> +
> +/* update a register, pass without AZX_REG_ prefix */
> +#define snd_hdac_ext_bus_spbcap_updatel(dev, reg, mask, val) \
> +	snd_hdac_ext_bus_spbcap_writel(dev, reg, \
> +			       (snd_hdac_ext_bus_spbcap_readl(dev, reg) & \
> +				~(mask)) | (val))
> +#define snd_hdac_ext_bus_spbcap_updatew(dev, reg, mask, val) \
> +	snd_hdac_ext_bus_spbcap_writew(dev, reg, \
> +			       (snd_hdac_ext_bus_spbcap_readw(dev, reg) & \
> +				~(mask)) | (val))
> +#define snd_hdac_ext_bus_spbcap_updateb(dev, reg, mask, val) \
> +	snd_hdac_ext_bus_spbcap_writeb(dev, reg, \
> +			       (snd_hdac_ext_bus_spbcap_readb(dev, reg) & \
> +				~(mask)) | (val))

The same question for these macros: whether we really want to wrap
all.  (Ditto for mlcap, and others.)

> +struct hdac_ext_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 */

Do we need to keep the codec name string?  Isn't it just codec address
we'd like to check the match...?  If so, we may use bitmasks instead,
too.


> +/**
> + * snd_hdac_ext_bus_parse_capabilities - parse capablity structure
> + * @sbus: the pointer to extended bus object
> + *
> + * Returns 0 if successful, or a negative error code.
> + */
> +int snd_hdac_ext_bus_parse_capabilities(struct hdac_ext_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;

Can be flags (mlcap, gtscap, etc) removed by just NULL-checking the
corresponding address?



> +			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);

Better to have a loop counter for avoiding the endless loop.
You should never trust the hardware.

> +MODULE_DESCRIPTION("HDA extended core");
> +MODULE_LICENSE("GPL v2");

These should have been added in the first patch.


thanks,

Takashi

Re: [PATCH v6 1/3] ALSA: hdac_ext: add extended HDA bus

[Vinod Koul]

On Mon, Jun 08, 2015 at 11:00:28AM +0200, Takashi Iwai wrote:
> At Thu,  4 Jun 2015 15:23:20 +0530,
> Vinod Koul wrote:
> > 
> > The new HDA controllers from Intel support new capabilities like multilink,
> > pipe processing, SPIB, GTS etc In order to use them we create an extended HDA
> > bus which embed the hdac bus and contains the fields for extended
> > configurations
> > 
> > Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> > ---
> >  include/sound/hdaudio_ext.h  |  93 ++++++++++++++++++++++++++++++++++
> >  sound/hda/Kconfig            |   4 ++
> >  sound/hda/Makefile           |   3 ++
> >  sound/hda/ext/Makefile       |   3 ++
> >  sound/hda/ext/hdac_ext_bus.c | 115 +++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 218 insertions(+)
> >  create mode 100644 include/sound/hdaudio_ext.h
> >  create mode 100644 sound/hda/ext/Makefile
> >  create mode 100644 sound/hda/ext/hdac_ext_bus.c
> > 
> > diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
> > new file mode 100644
> > index 000000000000..df1f8ae64693
> > --- /dev/null
> > +++ b/include/sound/hdaudio_ext.h
> > @@ -0,0 +1,93 @@
> > +#ifndef __SOUND_HDAUDIO_EXT_H
> > +#define __SOUND_HDAUDIO_EXT_H
> > +
> > +#include <sound/core.h>
> > +#include <sound/initval.h>
> 
> Do we need this header?
I dont think so will confirm and update this

> 
> > +#include <sound/hda_register.h>
> > +#include <sound/hdaudio.h>
> > +
> > +/**
> > + * hdac_ext_bus: HDAC extended bus for extended HDA caps
> > + *
> > + * @bus: hdac bus
> > + * @num_streams: streams supported
> > + * @ppcap: HDA ext post processing caps supported
> > + * @spbcap: HDA ext Software Position in Buffer cap support
> > + * @mlcap: HDA ext MultiLink cap support
> > + * @gtscap: HDA ext Global Time Sync cap support
> > + * @ppcap_addr: pp capabilities pointer
> > + * @spbcap_addr: SPIB capabilities pointer
> > + * @mlcap_addr: MultiLink capabilities pointer
> > + * @gtscap_addr: gts capabilities pointer
> > + * @hlink_list: link list of HDA links
> > + */
> > +struct hdac_ext_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;
> > +
> > +	struct list_head hlink_list;
> > +};
> > +
> > +int snd_hdac_ext_bus_init(struct hdac_ext_bus *sbus, struct device *dev,
> > +		      const struct hdac_bus_ops *ops,
> > +		      const struct hdac_io_ops *io_ops);
> > +
> > +void snd_hdac_ext_bus_exit(struct hdac_ext_bus *sbus);
> > +int snd_hdac_ext_bus_device_init(struct hdac_ext_bus *sbus, int addr);
> > +void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev);
> > +
> > +#define hdac_bus(sbus)	(&(sbus)->bus)
> 
> This macro name is a bit too ambiguous.  If it were really a generic
> cast, I wouldn't mind.  But it's specific to this type.
Yes would ebus_to_hdac_bus() make it better or something else

> 
> > +#define bus_to_hdac_ext_bus(_bus) \
> > +	container_of(_bus, struct hdac_ext_bus, bus)
> > +
> > +int snd_hdac_ext_bus_parse_capabilities(struct hdac_ext_bus *sbus);
> > +void snd_hdac_ext_bus_ppcap_enable(struct hdac_ext_bus *chip, bool enable);
> > +void snd_hdac_ext_bus_ppcap_int_enable(struct hdac_ext_bus *chip, bool enable);
> > +
> > +/*
> > + * macros for ppcap register read/write
> > + */
> > +#define _snd_hdac_ext_bus_ppcap_write(type, dev, reg, value)		\
> > +	((dev)->bus.io_ops->reg_write ## type(value, (dev)->ppcap_addr + (reg)))
> > +#define _snd_hdac_ext_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 snd_hdac_ext_bus_ppcap_writel(dev, reg, value) \
> > +	_snd_hdac_ext_bus_ppcap_write(l, dev, AZX_REG_ ## reg, value)
> > +#define snd_hdac_ext_bus_ppcap_writew(dev, reg, value) \
> > +	_snd_hdac_ext_bus_ppcap_write(w, dev, AZX_REG_ ## reg, value)
> > +#define snd_hdac_ext_bus_ppcap_writeb(dev, reg, value) \
> > +	_snd_hdac_ext_bus_ppcap_write(b, dev, AZX_REG_ ## reg, value)
> > +#define snd_hdac_ext_bus_ppcap_readl(dev, reg) \
> > +	_snd_hdac_ext_bus_ppcap_read(l, dev, AZX_REG_ ## reg)
> > +#define snd_hdac_ext_bus_ppcap_readw(dev, reg) \
> > +	_snd_hdac_ext_bus_ppcap_read(w, dev, AZX_REG_ ## reg)
> > +#define snd_hdac_ext_bus_ppcap_readb(dev, reg) \
> > +	_snd_hdac_ext_bus_ppcap_read(b, dev, AZX_REG_ ## reg)
> > +
> > +/* update a register, pass without AZX_REG_ prefix */
> > +#define snd_hdac_ext_bus_ppcap_updatel(dev, reg, mask, val) \
> > +	snd_hdac_ext_bus_ppcap_writel(dev, reg, \
> > +			       (snd_hdac_ext_bus_ppcap_readl(dev, reg) & \
> > +				~(mask)) | (val))
> > +#define snd_hdac_ext_bus_ppcap_updatew(dev, reg, mask, val) \
> > +	snd_hdac_ext_bus_ppcap_writew(dev, reg, \
> > +			       (snd_hdac_ext_bus_ppcap_readw(dev, reg) & \
> > +				~(mask)) | (val))
> > +#define snd_hdac_ext_bus_ppcap_updateb(dev, reg, mask, val) \
> > +	snd_hdac_ext_bus_ppcap_writeb(dev, reg, \
> > +			       (snd_hdac_ext_bus_ppcap_readb(dev, reg) & \
> > +				~(mask)) | (val))
> 
> It's not necessarily good to wrap all with such macros.
> For azx_write*(), I kept them as is for reducing the amount of useless
> code rewrites.  But for new codes, I don't think it's always worth...
Actually while updating the patch for ext I was wondering about this too.

So we cna remove these and use snd_hdac_chip_writel/w/b here 

let me update it here and for other caps

-- 
~Vinod

Re: [PATCH v6 1/3] ALSA: hdac_ext: add extended HDA bus

[Vinod Koul]

On Mon, Jun 08, 2015 at 11:03:12AM +0200, Takashi Iwai wrote:
> One thing forgot...
> 
> At Thu,  4 Jun 2015 15:23:20 +0530,
> Vinod Koul wrote:
> > 
> > +/**
> > + * snd_hdac_ext_device_init - initialize the HDA extended codec base device
> > + * @sbus: hdac extended bus to attach to
> > + * @addr: codec address
> > + *
> > + * Returns zero for success or a negative error code.
> > + */
> > +int snd_hdac_ext_bus_device_init(struct hdac_ext_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), "hda-codec#%03x", addr);
> > +
> > +	ret  = snd_hdac_device_init(hdev, bus, name, addr);
> 
> The device name must be unique to the whole system.  Using only the
> codec address as an id would conflict if there are multiple cards.
> The legacy HDA device consists of "hdaudioC%dD%d" indicating both the
> card number and the codec address, for example.
Sorry my bad, you did comment on it before as well. I will fix it now, by
adding card name here.

-- 
~Vinod

Re: [PATCH v6 1/3] ALSA: hdac_ext: add extended HDA bus

[Vinod Koul]

On Mon, Jun 08, 2015 at 03:40:08PM +0530, Vinod Koul wrote:
> On Mon, Jun 08, 2015 at 11:03:12AM +0200, Takashi Iwai wrote:
> > One thing forgot...
> > 
> > At Thu,  4 Jun 2015 15:23:20 +0530,
> > Vinod Koul wrote:
> > > 
> > > +/**
> > > + * snd_hdac_ext_device_init - initialize the HDA extended codec base device
> > > + * @sbus: hdac extended bus to attach to
> > > + * @addr: codec address
> > > + *
> > > + * Returns zero for success or a negative error code.
> > > + */
> > > +int snd_hdac_ext_bus_device_init(struct hdac_ext_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), "hda-codec#%03x", addr);
> > > +
> > > +	ret  = snd_hdac_device_init(hdev, bus, name, addr);
> > 
> > The device name must be unique to the whole system.  Using only the
> > codec address as an id would conflict if there are multiple cards.
> > The legacy HDA device consists of "hdaudioC%dD%d" indicating both the
> > card number and the codec address, for example.
> Sorry my bad, you did comment on it before as well. I will fix it now, by
> adding card name here.
Okay and we have an issue here :(, We actually dont know the card number
here and card number will be created after this enumeration is complete
and we register the DAI with ASoC and then ASoC would create card later.

So how do we solve this?

-- 
~Vinod

Re: [PATCH v6 1/3] ALSA: hdac_ext: add extended HDA bus

[Vinod Koul]

On Mon, Jun 08, 2015 at 03:38:22PM +0530, Vinod Koul wrote:
> > > +#define snd_hdac_ext_bus_ppcap_updateb(dev, reg, mask, val) \
> > > +	snd_hdac_ext_bus_ppcap_writeb(dev, reg, \
> > > +			       (snd_hdac_ext_bus_ppcap_readb(dev, reg) & \
> > > +				~(mask)) | (val))
> > 
> > It's not necessarily good to wrap all with such macros.
> > For azx_write*(), I kept them as is for reducing the amount of useless
> > code rewrites.  But for new codes, I don't think it's always worth...
> Actually while updating the patch for ext I was wondering about this too.
> 
> So we cna remove these and use snd_hdac_chip_writel/w/b here
As Jeeja pointed we can't use snd_hdac_chip_writel as we need to use a
different base. So we cna move this to use plain writel only

Any other ideas?

-- 
~Vinod

Re: [PATCH v6 2/3] ALSA: hdac_ext: add hdac extended controller

[Vinod Koul]

On Mon, Jun 08, 2015 at 11:12:48AM +0200, Takashi Iwai wrote:
> > +struct hdac_ext_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 */
> 
> Do we need to keep the codec name string?  Isn't it just codec address
> we'd like to check the match...?  If so, we may use bitmasks instead,
> too.
This is not for matching stuff (this is link structure and not codec)
So we keep name of codec found on this link. When ASoC BE triggers we use
codec name to find the link and use

> > +/**
> > + * snd_hdac_ext_bus_parse_capabilities - parse capablity structure
> > + * @sbus: the pointer to extended bus object
> > + *
> > + * Returns 0 if successful, or a negative error code.
> > + */
> > +int snd_hdac_ext_bus_parse_capabilities(struct hdac_ext_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;
> 
> Can be flags (mlcap, gtscap, etc) removed by just NULL-checking the
> corresponding address?
Yes, removed these

> > +			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);
> 
> Better to have a loop counter for avoiding the endless loop.
> You should never trust the hardware.
So we have 4 caps, so am going to put 10 as count for now and bail out on
10th one...

> 
> > +MODULE_DESCRIPTION("HDA extended core");
> > +MODULE_LICENSE("GPL v2");
> 
> These should have been added in the first patch.
Yes will move

-- 
~Vinod

Re: [PATCH v6 1/3] ALSA: hdac_ext: add extended HDA bus

[Takashi Iwai]

At Mon, 8 Jun 2015 20:54:23 +0530,
Vinod Koul wrote:
> 
> On Mon, Jun 08, 2015 at 03:40:08PM +0530, Vinod Koul wrote:
> > On Mon, Jun 08, 2015 at 11:03:12AM +0200, Takashi Iwai wrote:
> > > One thing forgot...
> > > 
> > > At Thu,  4 Jun 2015 15:23:20 +0530,
> > > Vinod Koul wrote:
> > > > 
> > > > +/**
> > > > + * snd_hdac_ext_device_init - initialize the HDA extended codec base device
> > > > + * @sbus: hdac extended bus to attach to
> > > > + * @addr: codec address
> > > > + *
> > > > + * Returns zero for success or a negative error code.
> > > > + */
> > > > +int snd_hdac_ext_bus_device_init(struct hdac_ext_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), "hda-codec#%03x", addr);
> > > > +
> > > > +	ret  = snd_hdac_device_init(hdev, bus, name, addr);
> > > 
> > > The device name must be unique to the whole system.  Using only the
> > > codec address as an id would conflict if there are multiple cards.
> > > The legacy HDA device consists of "hdaudioC%dD%d" indicating both the
> > > card number and the codec address, for example.
> > Sorry my bad, you did comment on it before as well. I will fix it now, by
> > adding card name here.
> Okay and we have an issue here :(, We actually dont know the card number
> here and card number will be created after this enumeration is complete
> and we register the DAI with ASoC and then ASoC would create card later.
> 
> So how do we solve this?

Instead of the card number, assign some unique index (either a simple
static counter or use idr) to each bus object and use it instead of
the card number.  After all, we just need a unique mapping.

Alternatively, you may rename via dev_set_name() just before
registration.  But, of course, this is inconsistent in some areas,
e.g. dev_err() shows differently before and after registration.


Takashi

Re: [PATCH v6 1/3] ALSA: hdac_ext: add extended HDA bus

[Takashi Iwai]

At Mon, 8 Jun 2015 21:00:14 +0530,
Vinod Koul wrote:
> 
> On Mon, Jun 08, 2015 at 03:38:22PM +0530, Vinod Koul wrote:
> > > > +#define snd_hdac_ext_bus_ppcap_updateb(dev, reg, mask, val) \
> > > > +	snd_hdac_ext_bus_ppcap_writeb(dev, reg, \
> > > > +			       (snd_hdac_ext_bus_ppcap_readb(dev, reg) & \
> > > > +				~(mask)) | (val))
> > > 
> > > It's not necessarily good to wrap all with such macros.
> > > For azx_write*(), I kept them as is for reducing the amount of useless
> > > code rewrites.  But for new codes, I don't think it's always worth...
> > Actually while updating the patch for ext I was wondering about this too.
> > 
> > So we cna remove these and use snd_hdac_chip_writel/w/b here
> As Jeeja pointed we can't use snd_hdac_chip_writel as we need to use a
> different base. So we cna move this to use plain writel only
> 
> Any other ideas?

I don't think you need to access via io_ops redirection as these are
SKL specific registers.  Use plain readl()/writel() and keep things
as simple as possible.

(And better to avoid w and b variants.)


Takashi

Re: [PATCH v6 2/3] ALSA: hdac_ext: add hdac extended controller

[Takashi Iwai]

At Mon, 8 Jun 2015 21:02:59 +0530,
Vinod Koul wrote:
> 
> On Mon, Jun 08, 2015 at 11:12:48AM +0200, Takashi Iwai wrote:
> > > +struct hdac_ext_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 */
> > 
> > Do we need to keep the codec name string?  Isn't it just codec address
> > we'd like to check the match...?  If so, we may use bitmasks instead,
> > too.
> This is not for matching stuff (this is link structure and not codec)
> So we keep name of codec found on this link. When ASoC BE triggers we use
> codec name to find the link and use

But each codec name consists of just a few bits.  Why do you need to
keep each codec name string?


Takashi

Re: [PATCH v6 1/3] ALSA: hdac_ext: add extended HDA bus

[Vinod Koul]

On Mon, Jun 08, 2015 at 05:37:03PM +0200, Takashi Iwai wrote:
> At Mon, 8 Jun 2015 20:54:23 +0530,
> Vinod Koul wrote:
> > 
> > On Mon, Jun 08, 2015 at 03:40:08PM +0530, Vinod Koul wrote:
> > > On Mon, Jun 08, 2015 at 11:03:12AM +0200, Takashi Iwai wrote:
> > > > One thing forgot...
> > > > 
> > > > At Thu,  4 Jun 2015 15:23:20 +0530,
> > > > Vinod Koul wrote:
> > > > > 
> > > > > +/**
> > > > > + * snd_hdac_ext_device_init - initialize the HDA extended codec base device
> > > > > + * @sbus: hdac extended bus to attach to
> > > > > + * @addr: codec address
> > > > > + *
> > > > > + * Returns zero for success or a negative error code.
> > > > > + */
> > > > > +int snd_hdac_ext_bus_device_init(struct hdac_ext_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), "hda-codec#%03x", addr);
> > > > > +
> > > > > +	ret  = snd_hdac_device_init(hdev, bus, name, addr);
> > > > 
> > > > The device name must be unique to the whole system.  Using only the
> > > > codec address as an id would conflict if there are multiple cards.
> > > > The legacy HDA device consists of "hdaudioC%dD%d" indicating both the
> > > > card number and the codec address, for example.
> > > Sorry my bad, you did comment on it before as well. I will fix it now, by
> > > adding card name here.
> > Okay and we have an issue here :(, We actually dont know the card number
> > here and card number will be created after this enumeration is complete
> > and we register the DAI with ASoC and then ASoC would create card later.
> > 
> > So how do we solve this?
> 
> Instead of the card number, assign some unique index (either a simple
> static counter or use idr) to each bus object and use it instead of
> the card number.  After all, we just need a unique mapping.
That is doable, lets maintain a counter then :) Also we are thinking of using
"hdaudio%dD%d" or "ehdaudio%dD%d" dropping Card charcter
> 
> Alternatively, you may rename via dev_set_name() just before
> registration.  But, of course, this is inconsistent in some areas,
> e.g. dev_err() shows differently before and after registration.
Yes lets avoid it

-- 
~Vinod

Re: [PATCH v6 2/3] ALSA: hdac_ext: add hdac extended controller

[Vinod Koul]

On Mon, Jun 08, 2015 at 05:42:25PM +0200, Takashi Iwai wrote:
> At Mon, 8 Jun 2015 21:02:59 +0530,
> Vinod Koul wrote:
> > 
> > On Mon, Jun 08, 2015 at 11:12:48AM +0200, Takashi Iwai wrote:
> > > > +struct hdac_ext_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 */
> > > 
> > > Do we need to keep the codec name string?  Isn't it just codec address
> > > we'd like to check the match...?  If so, we may use bitmasks instead,
> > > too.
> > This is not for matching stuff (this is link structure and not codec)
> > So we keep name of codec found on this link. When ASoC BE triggers we use
> > codec name to find the link and use
> 
> But each codec name consists of just a few bits.  Why do you need to
> keep each codec name string?
Yes got your point now, we can indeed generate the codec name and avoid
char array

-- 
~Vinod

Re: [PATCH v6 1/3] ALSA: hdac_ext: add extended HDA bus

[Vinod Koul]

On Mon, Jun 08, 2015 at 05:40:46PM +0200, Takashi Iwai wrote:
> At Mon, 8 Jun 2015 21:00:14 +0530,
> Vinod Koul wrote:
> > 
> > On Mon, Jun 08, 2015 at 03:38:22PM +0530, Vinod Koul wrote:
> > > > > +#define snd_hdac_ext_bus_ppcap_updateb(dev, reg, mask, val) \
> > > > > +	snd_hdac_ext_bus_ppcap_writeb(dev, reg, \
> > > > > +			       (snd_hdac_ext_bus_ppcap_readb(dev, reg) & \
> > > > > +				~(mask)) | (val))
> > > > 
> > > > It's not necessarily good to wrap all with such macros.
> > > > For azx_write*(), I kept them as is for reducing the amount of useless
> > > > code rewrites.  But for new codes, I don't think it's always worth...
> > > Actually while updating the patch for ext I was wondering about this too.
> > > 
> > > So we cna remove these and use snd_hdac_chip_writel/w/b here
> > As Jeeja pointed we can't use snd_hdac_chip_writel as we need to use a
> > different base. So we cna move this to use plain writel only
> > 
> > Any other ideas?
> 
> I don't think you need to access via io_ops redirection as these are
> SKL specific registers.  Use plain readl()/writel() and keep things
> as simple as possible.
> 
> (And better to avoid w and b variants.)
Okay I have removed all these macros but ended up defining one generic update
macros (most of the places we are doing read and write, so better to use
update macro

/* update register macro */
#define snd_hdac_updatel(addr, reg, mask, val) \
	writel(addr, reg, (readl(addr, reg) & ~(mask)) | (val))

yes its updatel to signify that it uses writel and readl

Let me know if you are fine with this approach

-- 
~Vinod

Re: [PATCH v6 1/3] ALSA: hdac_ext: add extended HDA bus

[Takashi Iwai]

At Tue, 9 Jun 2015 15:36:51 +0530,
Vinod Koul wrote:
> 
> On Mon, Jun 08, 2015 at 05:40:46PM +0200, Takashi Iwai wrote:
> > At Mon, 8 Jun 2015 21:00:14 +0530,
> > Vinod Koul wrote:
> > > 
> > > On Mon, Jun 08, 2015 at 03:38:22PM +0530, Vinod Koul wrote:
> > > > > > +#define snd_hdac_ext_bus_ppcap_updateb(dev, reg, mask, val) \
> > > > > > +	snd_hdac_ext_bus_ppcap_writeb(dev, reg, \
> > > > > > +			       (snd_hdac_ext_bus_ppcap_readb(dev, reg) & \
> > > > > > +				~(mask)) | (val))
> > > > > 
> > > > > It's not necessarily good to wrap all with such macros.
> > > > > For azx_write*(), I kept them as is for reducing the amount of useless
> > > > > code rewrites.  But for new codes, I don't think it's always worth...
> > > > Actually while updating the patch for ext I was wondering about this too.
> > > > 
> > > > So we cna remove these and use snd_hdac_chip_writel/w/b here
> > > As Jeeja pointed we can't use snd_hdac_chip_writel as we need to use a
> > > different base. So we cna move this to use plain writel only
> > > 
> > > Any other ideas?
> > 
> > I don't think you need to access via io_ops redirection as these are
> > SKL specific registers.  Use plain readl()/writel() and keep things
> > as simple as possible.
> > 
> > (And better to avoid w and b variants.)
> Okay I have removed all these macros but ended up defining one generic update
> macros (most of the places we are doing read and write, so better to use
> update macro
> 
> /* update register macro */
> #define snd_hdac_updatel(addr, reg, mask, val) \
> 	writel(addr, reg, (readl(addr, reg) & ~(mask)) | (val))
> 
> yes its updatel to signify that it uses writel and readl
> 
> Let me know if you are fine with this approach

This looks good to me.


thanks,

Takashi