[PATCH 0/5] ALSA: hdac: More hdac core fixes and updates

[PATCH 0/5] ALSA: hdac: More hdac core fixes and updates

[Vinod Koul]

Hi Takashi,

We had few more hdac core fixes & enhancements so senidng those.

First one is fix for register pointers for spcap and link registers. This is
required for SKL driver when we use link registers.

rest add new APIs for powering down all links, getting stream from stream
tag and direction, enabling SPIB.

Last one remove keys as we dont need them for host streams

Jeeja KP (5):
  ALSA: hdac: Fix to read the correct offset of spcap/link register
  ALSA: hdac: Add snd_hdac_ext_bus_link_power_down_all()
  ALSA: hdac: Add snd_hdac_get_hdac_stream()
  ALSA: hdac: Add support to enable SPIB for hdac ext stream
  ALSA: hdac: Remove the usage of key for host stream

 include/sound/hda_register.h        |  4 +++
 include/sound/hdaudio.h             |  2 ++
 include/sound/hdaudio_ext.h         | 11 +++++++
 sound/hda/ext/hdac_ext_controller.c | 30 +++++++++++++++---
 sound/hda/ext/hdac_ext_stream.c     | 62 ++++++++++++++++++++++++++++++++-----
 sound/hda/hdac_stream.c             | 19 ++++++++++++
 6 files changed, 116 insertions(+), 12 deletions(-)

Thanks
-- 
~Vinod

[PATCH 1/5] ALSA: hdac: Fix to read the correct offset of spcap/link register

[Vinod Koul]

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

SPCAP and Mutilink register offset were incorrect as offset needs
to be based on capability offset. So correct the offset for
read/write of spcap/link register.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/hda/ext/hdac_ext_controller.c | 10 +++++-----
 sound/hda/ext/hdac_ext_stream.c     |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c
index 358f16195483..d3bb112754f2 100644
--- a/sound/hda/ext/hdac_ext_controller.c
+++ b/sound/hda/ext/hdac_ext_controller.c
@@ -177,8 +177,8 @@ int snd_hdac_ext_bus_get_ml_capabilities(struct hdac_ext_bus *ebus)
 		hlink->bus = bus;
 		hlink->ml_addr = ebus->mlcap + AZX_ML_BASE +
 					(AZX_ML_INTERVAL * idx);
-		hlink->lcaps  = snd_hdac_chip_readl(bus, ML_LCAP);
-		hlink->lsdiid = snd_hdac_chip_readw(bus, ML_LSDIID);
+		hlink->lcaps  = readl(hlink->ml_addr + AZX_REG_ML_LCAP);
+		hlink->lsdiid = readw(hlink->ml_addr + AZX_REG_ML_LSDIID);
 
 		list_add_tail(&hlink->list, &ebus->hlink_list);
 	}
@@ -243,7 +243,7 @@ static int check_hdac_link_power_active(struct hdac_ext_link *link, bool enable)
 	timeout = 50;
 
 	do {
-		val = snd_hdac_chip_readl(link->bus, ML_LCTL);
+		val = readl(link->ml_addr + AZX_REG_ML_LCTL);
 		if (enable) {
 			if (((val & mask) >> AZX_MLCTL_CPA))
 				return 0;
@@ -263,7 +263,7 @@ static int check_hdac_link_power_active(struct hdac_ext_link *link, bool enable)
  */
 int snd_hdac_ext_bus_link_power_up(struct hdac_ext_link *link)
 {
-	snd_hdac_chip_updatel(link->bus, ML_LCTL, 0, AZX_MLCTL_SPA);
+	snd_hdac_updatel(link->ml_addr, AZX_REG_ML_LCTL, 0, AZX_MLCTL_SPA);
 
 	return check_hdac_link_power_active(link, true);
 }
@@ -275,7 +275,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_power_up);
  */
 int snd_hdac_ext_bus_link_power_down(struct hdac_ext_link *link)
 {
-	snd_hdac_chip_updatel(link->bus, ML_LCTL, AZX_MLCTL_SPA, 0);
+	snd_hdac_updatel(link->ml_addr, AZX_REG_ML_LCTL, AZX_MLCTL_SPA, 0);
 
 	return check_hdac_link_power_active(link, false);
 }
diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c
index 3de47dd1a76d..b649625f43a4 100644
--- a/sound/hda/ext/hdac_ext_stream.c
+++ b/sound/hda/ext/hdac_ext_stream.c
@@ -423,7 +423,7 @@ void snd_hdac_ext_stream_spbcap_enable(struct hdac_ext_bus *ebus,
 
 	mask |= (1 << index);
 
-	register_mask = snd_hdac_chip_readl(bus, SPB_SPBFCCTL);
+	register_mask = readl(ebus->spbcap + AZX_REG_SPB_SPBFCCTL);
 
 	mask |= register_mask;
 
-- 
1.9.1

[PATCH 2/5] ALSA: hdac: Add snd_hdac_ext_bus_link_power_down_all()

[Vinod Koul]

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

New HDA controllers like Skylake sport multiple HDA links, so we need a
helper to turn off all the links in one go while suspending the device so
add snd_hdac_ext_bus_link_power_down_all() API

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 include/sound/hdaudio_ext.h         |  1 +
 sound/hda/ext/hdac_ext_controller.c | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
index 55e2fc36177f..160160d9bffc 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -116,6 +116,7 @@ struct hdac_ext_link {
 
 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);
+int snd_hdac_ext_bus_link_power_down_all(struct hdac_ext_bus *ebus);
 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,
diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c
index d3bb112754f2..63215b17247c 100644
--- a/sound/hda/ext/hdac_ext_controller.c
+++ b/sound/hda/ext/hdac_ext_controller.c
@@ -280,3 +280,23 @@ int snd_hdac_ext_bus_link_power_down(struct hdac_ext_link *link)
 	return check_hdac_link_power_active(link, false);
 }
 EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_power_down);
+
+/**
+ * snd_hdac_ext_bus_link_power_down_all -power down all hda link
+ * @ebus: HD-audio extended bus
+ */
+int snd_hdac_ext_bus_link_power_down_all(struct hdac_ext_bus *ebus)
+{
+	struct hdac_ext_link *hlink = NULL;
+	int ret;
+
+	list_for_each_entry(hlink, &ebus->hlink_list, list) {
+		snd_hdac_updatel(hlink->ml_addr, AZX_REG_ML_LCTL, AZX_MLCTL_SPA, 0);
+		ret = check_hdac_link_power_active(hlink, false);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_power_down_all);
-- 
1.9.1

[PATCH 3/5] ALSA: hdac: Add snd_hdac_get_hdac_stream()

[Vinod Koul]

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

Add a helper to find the stream using stream tag and direction.
This is useful for drivers to query stream based on stream tag and
direction, fox example while downloading FW thru DSP loader code

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 include/sound/hdaudio.h |  2 ++
 sound/hda/hdac_stream.c | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index e37c3355ae8a..67f7068696b9 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -438,6 +438,8 @@ void snd_hdac_stream_init(struct hdac_bus *bus, struct hdac_stream *azx_dev,
 struct hdac_stream *snd_hdac_stream_assign(struct hdac_bus *bus,
 					   struct snd_pcm_substream *substream);
 void snd_hdac_stream_release(struct hdac_stream *azx_dev);
+struct hdac_stream *snd_hdac_get_hdac_stream(struct hdac_bus *bus,
+						int dir, int stream_tag);
 
 int snd_hdac_stream_setup(struct hdac_stream *azx_dev);
 void snd_hdac_stream_cleanup(struct hdac_stream *azx_dev);
diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index 4c15d0accc9e..02aed2e85504 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -286,6 +286,25 @@ void snd_hdac_stream_release(struct hdac_stream *azx_dev)
 }
 EXPORT_SYMBOL_GPL(snd_hdac_stream_release);
 
+/**
+ * snd_hdac_get_hdac_stream - return hdac_stream based on stream_tag and
+ * direction
+ *
+ * @bus: HD-audio core bus
+ */
+struct hdac_stream *snd_hdac_get_hdac_stream(struct hdac_bus *bus,
+						int dir, int stream_tag)
+{
+	struct hdac_stream *s;
+
+	list_for_each_entry(s, &bus->stream_list, list) {
+		if (s->direction == dir && s->stream_tag == stream_tag)
+			return s;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_get_hdac_stream);
 /*
  * set up a BDL entry
  */
-- 
1.9.1

[PATCH 4/5] ALSA: hdac: Add support to enable SPIB for hdac ext stream

[Vinod Koul]

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

The drivers need to set the spib and maxfifios values, so add
these new APIs snd_hdac_ext_stream_set_spib() and
snd_hdac_ext_stream_set_spbmaxfifo() APIs

For these APIs we also need to have spib and fifos pointer, so
add these to hdac_ext_stream and initialize them at stream init

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 include/sound/hda_register.h    |  4 +++
 include/sound/hdaudio_ext.h     | 10 ++++++++
 sound/hda/ext/hdac_ext_stream.c | 54 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+)

diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h
index ae995e523ff8..2ae8812d7b1a 100644
--- a/include/sound/hda_register.h
+++ b/include/sound/hda_register.h
@@ -160,6 +160,10 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
 #define AZX_SPB_BASE			0x08
 /* Interval used to calculate the iterating register offset */
 #define AZX_SPB_INTERVAL		0x08
+/* SPIB base */
+#define AZX_SPB_SPIB			0x00
+/* SPIB MAXFIFO base*/
+#define AZX_SPB_MAXFIFO			0x04
 
 /* registers of Global Time Synchronization Capability Structure */
 #define AZX_GTS_CAP_ID			0x1
diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
index 160160d9bffc..9385c99a6504 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -63,6 +63,8 @@ enum hdac_ext_stream_type {
  * @hstream: hdac_stream
  * @pphc_addr: processing pipe host stream pointer
  * @pplc_addr: processing pipe link stream pointer
+ * @spib_addr: software position in buffers stream pointer
+ * @fifo_addr: software position Max fifos stream pointer
  * @decoupled: stream host and link is decoupled
  * @link_locked: link is locked
  * @link_prepared: link is prepared
@@ -74,6 +76,9 @@ struct hdac_ext_stream {
 	void __iomem *pphc_addr;
 	void __iomem *pplc_addr;
 
+	void __iomem *spib_addr;
+	void __iomem *fifo_addr;
+
 	bool decoupled:1;
 	bool link_locked:1;
 	bool link_prepared;
@@ -100,6 +105,11 @@ 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);
 
+int snd_hdac_ext_stream_set_spib(struct hdac_ext_bus *ebus,
+				 struct hdac_ext_stream *stream, u32 value);
+int snd_hdac_ext_stream_set_spbmaxfifo(struct hdac_ext_bus *ebus,
+				 struct hdac_ext_stream *stream);
+
 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);
diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c
index b649625f43a4..a4f6bbe5da8f 100644
--- a/sound/hda/ext/hdac_ext_stream.c
+++ b/sound/hda/ext/hdac_ext_stream.c
@@ -49,6 +49,16 @@ void snd_hdac_ext_stream_init(struct hdac_ext_bus *ebus,
 				AZX_PPLC_INTERVAL * idx;
 	}
 
+	if (ebus->spbcap) {
+		stream->spib_addr = ebus->spbcap + AZX_SPB_BASE +
+					AZX_SPB_INTERVAL * idx +
+					AZX_SPB_SPIB;
+
+		stream->fifo_addr = ebus->spbcap + AZX_SPB_BASE +
+					AZX_SPB_INTERVAL * idx +
+					AZX_SPB_MAXFIFO;
+	}
+
 	stream->decoupled = false;
 	snd_hdac_stream_init(bus, &stream->hstream, idx, direction, tag);
 }
@@ -435,6 +445,50 @@ void snd_hdac_ext_stream_spbcap_enable(struct hdac_ext_bus *ebus,
 EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_spbcap_enable);
 
 /**
+ * snd_hdac_ext_stream_set_spib - sets the spib value of a stream
+ * @ebus: HD-audio ext core bus
+ * @stream: hdac_ext_stream
+ * @value: spib value to set
+ */
+int snd_hdac_ext_stream_set_spib(struct hdac_ext_bus *ebus,
+				 struct hdac_ext_stream *stream, u32 value)
+{
+	struct hdac_bus *bus = &ebus->bus;
+
+	if (!ebus->spbcap) {
+		dev_err(bus->dev, "Address of SPB capability is NULL");
+		return -EINVAL;
+	}
+
+	writel(value, stream->spib_addr);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_set_spib);
+
+/**
+ * snd_hdac_ext_stream_set_spbmaxfifo - sets the spib value of a stream
+ * @ebus: HD-audio ext core bus
+ * @stream: hdac_ext_stream
+ *
+ * Return maxfifo for the stream
+ */
+int snd_hdac_ext_stream_set_spbmaxfifo(struct hdac_ext_bus *ebus,
+				 struct hdac_ext_stream *stream)
+{
+	struct hdac_bus *bus = &ebus->bus;
+
+	if (!ebus->spbcap) {
+		dev_err(bus->dev, "Address of SPB capability is NULL");
+		return -EINVAL;
+	}
+
+	return readl(stream->fifo_addr);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_set_spbmaxfifo);
+
+
+/**
  * snd_hdac_ext_stop_streams - stop all stream if running
  * @ebus: HD-audio ext core bus
  */
-- 
1.9.1

[PATCH 5/5] ALSA: hdac: Remove the usage of key for host stream

[Vinod Koul]

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

hdac_ext_stream assign doesn't require key mapping as in case of
hdac_stream. So for host stream, the key to device mapping needs
to be removed.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/hda/ext/hdac_ext_stream.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c
index a4f6bbe5da8f..c978a9bb1db8 100644
--- a/sound/hda/ext/hdac_ext_stream.c
+++ b/sound/hda/ext/hdac_ext_stream.c
@@ -291,17 +291,12 @@ hdac_ext_host_stream_assign(struct hdac_ext_bus *ebus,
 	struct hdac_ext_stream *res = NULL;
 	struct hdac_stream *stream = NULL;
 	struct hdac_bus *hbus = &ebus->bus;
-	int key;
 
 	if (!ebus->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,
@@ -320,7 +315,6 @@ hdac_ext_host_stream_assign(struct hdac_ext_bus *ebus,
 		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);
 	}
-- 
1.9.1

Re: [PATCH 3/5] ALSA: hdac: Add snd_hdac_get_hdac_stream()

[Takashi Iwai]

On Fri, 21 Aug 2015 18:06:19 +0200,
Vinod Koul wrote:
> 
> From: Jeeja KP <jeeja.kp@intel.com>
> 
> Add a helper to find the stream using stream tag and direction.
> This is useful for drivers to query stream based on stream tag and
> direction, fox example while downloading FW thru DSP loader code
> 
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  include/sound/hdaudio.h |  2 ++
>  sound/hda/hdac_stream.c | 19 +++++++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
> index e37c3355ae8a..67f7068696b9 100644
> --- a/include/sound/hdaudio.h
> +++ b/include/sound/hdaudio.h
> @@ -438,6 +438,8 @@ void snd_hdac_stream_init(struct hdac_bus *bus, struct hdac_stream *azx_dev,
>  struct hdac_stream *snd_hdac_stream_assign(struct hdac_bus *bus,
>  					   struct snd_pcm_substream *substream);
>  void snd_hdac_stream_release(struct hdac_stream *azx_dev);
> +struct hdac_stream *snd_hdac_get_hdac_stream(struct hdac_bus *bus,
> +						int dir, int stream_tag);
>  
>  int snd_hdac_stream_setup(struct hdac_stream *azx_dev);
>  void snd_hdac_stream_cleanup(struct hdac_stream *azx_dev);
> diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
> index 4c15d0accc9e..02aed2e85504 100644
> --- a/sound/hda/hdac_stream.c
> +++ b/sound/hda/hdac_stream.c
> @@ -286,6 +286,25 @@ void snd_hdac_stream_release(struct hdac_stream *azx_dev)
>  }
>  EXPORT_SYMBOL_GPL(snd_hdac_stream_release);
>  
> +/**
> + * snd_hdac_get_hdac_stream - return hdac_stream based on stream_tag and
> + * direction
> + *
> + * @bus: HD-audio core bus
> + */

Please complete the description.

> +struct hdac_stream *snd_hdac_get_hdac_stream(struct hdac_bus *bus,
> +						int dir, int stream_tag)
> +{
> +	struct hdac_stream *s;
> +
> +	list_for_each_entry(s, &bus->stream_list, list) {
> +		if (s->direction == dir && s->stream_tag == stream_tag)
> +			return s;
> +	}
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(snd_hdac_get_hdac_stream);
>  /*

Put the empty line after EXPORT_SYMBOL_GPL().


thanks,

Takashi

Re: [PATCH 0/5] ALSA: hdac: More hdac core fixes and updates

[Takashi Iwai]

On Fri, 21 Aug 2015 18:06:16 +0200,
Vinod Koul wrote:
> 
> Hi Takashi,
> 
> We had few more hdac core fixes & enhancements so senidng those.
> 
> First one is fix for register pointers for spcap and link registers. This is
> required for SKL driver when we use link registers.
> 
> rest add new APIs for powering down all links, getting stream from stream
> tag and direction, enabling SPIB.
> 
> Last one remove keys as we dont need them for host streams
> 
> Jeeja KP (5):
>   ALSA: hdac: Fix to read the correct offset of spcap/link register
>   ALSA: hdac: Add snd_hdac_ext_bus_link_power_down_all()
>   ALSA: hdac: Add snd_hdac_get_hdac_stream()
>   ALSA: hdac: Add support to enable SPIB for hdac ext stream
>   ALSA: hdac: Remove the usage of key for host stream

I skipped the patch 3 due to issues I pointed in another mail, and
applied the rest four patches to for-next branch.  Please resubmit
only patch 3.


thanks,

Takashi


> 
>  include/sound/hda_register.h        |  4 +++
>  include/sound/hdaudio.h             |  2 ++
>  include/sound/hdaudio_ext.h         | 11 +++++++
>  sound/hda/ext/hdac_ext_controller.c | 30 +++++++++++++++---
>  sound/hda/ext/hdac_ext_stream.c     | 62 ++++++++++++++++++++++++++++++++-----
>  sound/hda/hdac_stream.c             | 19 ++++++++++++
>  6 files changed, 116 insertions(+), 12 deletions(-)
> 
> Thanks
> -- 
> ~Vinod
> 

Re: [PATCH 4/5] ALSA: hdac: Add support to enable SPIB for hdac ext stream

[Pierre-Louis Bossart]

On 8/21/15 11:06 AM, Vinod Koul wrote:
> From: Jeeja KP <jeeja.kp@intel.com>
>
> The drivers need to set the spib and maxfifios values, so add
> these new APIs snd_hdac_ext_stream_set_spib() and
> snd_hdac_ext_stream_set_spbmaxfifo() APIs
>
> For these APIs we also need to have spib and fifos pointer, so
> add these to hdac_ext_stream and initialize them at stream init
>
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>   include/sound/hda_register.h    |  4 +++
>   include/sound/hdaudio_ext.h     | 10 ++++++++
>   sound/hda/ext/hdac_ext_stream.c | 54 +++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 68 insertions(+)
>
> diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h
> index ae995e523ff8..2ae8812d7b1a 100644
> --- a/include/sound/hda_register.h
> +++ b/include/sound/hda_register.h
> @@ -160,6 +160,10 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
>   #define AZX_SPB_BASE			0x08
>   /* Interval used to calculate the iterating register offset */
>   #define AZX_SPB_INTERVAL		0x08
> +/* SPIB base */
> +#define AZX_SPB_SPIB			0x00
> +/* SPIB MAXFIFO base*/
> +#define AZX_SPB_MAXFIFO			0x04
>
>   /* registers of Global Time Synchronization Capability Structure */
>   #define AZX_GTS_CAP_ID			0x1
> diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
> index 160160d9bffc..9385c99a6504 100644
> --- a/include/sound/hdaudio_ext.h
> +++ b/include/sound/hdaudio_ext.h
> @@ -63,6 +63,8 @@ enum hdac_ext_stream_type {
>    * @hstream: hdac_stream
>    * @pphc_addr: processing pipe host stream pointer
>    * @pplc_addr: processing pipe link stream pointer
> + * @spib_addr: software position in buffers stream pointer
> + * @fifo_addr: software position Max fifos stream pointer
>    * @decoupled: stream host and link is decoupled
>    * @link_locked: link is locked
>    * @link_prepared: link is prepared
> @@ -74,6 +76,9 @@ struct hdac_ext_stream {
>   	void __iomem *pphc_addr;
>   	void __iomem *pplc_addr;
>
> +	void __iomem *spib_addr;
> +	void __iomem *fifo_addr;
> +
>   	bool decoupled:1;
>   	bool link_locked:1;
>   	bool link_prepared;
> @@ -100,6 +105,11 @@ 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);
>
> +int snd_hdac_ext_stream_set_spib(struct hdac_ext_bus *ebus,
> +				 struct hdac_ext_stream *stream, u32 value);
> +int snd_hdac_ext_stream_set_spbmaxfifo(struct hdac_ext_bus *ebus,
> +				 struct hdac_ext_stream *stream);
> +
>   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);
> diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c
> index b649625f43a4..a4f6bbe5da8f 100644
> --- a/sound/hda/ext/hdac_ext_stream.c
> +++ b/sound/hda/ext/hdac_ext_stream.c
> @@ -49,6 +49,16 @@ void snd_hdac_ext_stream_init(struct hdac_ext_bus *ebus,
>   				AZX_PPLC_INTERVAL * idx;
>   	}
>
> +	if (ebus->spbcap) {
> +		stream->spib_addr = ebus->spbcap + AZX_SPB_BASE +
> +					AZX_SPB_INTERVAL * idx +
> +					AZX_SPB_SPIB;
> +
> +		stream->fifo_addr = ebus->spbcap + AZX_SPB_BASE +
> +					AZX_SPB_INTERVAL * idx +
> +					AZX_SPB_MAXFIFO;
> +	}
> +
>   	stream->decoupled = false;
>   	snd_hdac_stream_init(bus, &stream->hstream, idx, direction, tag);
>   }
> @@ -435,6 +445,50 @@ void snd_hdac_ext_stream_spbcap_enable(struct hdac_ext_bus *ebus,
>   EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_spbcap_enable);
>
>   /**
> + * snd_hdac_ext_stream_set_spib - sets the spib value of a stream
> + * @ebus: HD-audio ext core bus
> + * @stream: hdac_ext_stream
> + * @value: spib value to set
> + */
> +int snd_hdac_ext_stream_set_spib(struct hdac_ext_bus *ebus,
> +				 struct hdac_ext_stream *stream, u32 value)
> +{
> +	struct hdac_bus *bus = &ebus->bus;
> +
> +	if (!ebus->spbcap) {
> +		dev_err(bus->dev, "Address of SPB capability is NULL");
> +		return -EINVAL;
> +	}
> +
> +	writel(value, stream->spib_addr);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_set_spib);
> +
> +/**
> + * snd_hdac_ext_stream_set_spbmaxfifo - sets the spib value of a stream

copy-paste issue, wrong comment

> + * @ebus: HD-audio ext core bus
> + * @stream: hdac_ext_stream
> + *
> + * Return maxfifo for the stream
> + */
> +int snd_hdac_ext_stream_set_spbmaxfifo(struct hdac_ext_bus *ebus,
> +				 struct hdac_ext_stream *stream)
> +{
> +	struct hdac_bus *bus = &ebus->bus;
> +
> +	if (!ebus->spbcap) {
> +		dev_err(bus->dev, "Address of SPB capability is NULL");
> +		return -EINVAL;
> +	}
> +
> +	return readl(stream->fifo_addr);
> +}

weird function name. a set_spbmaxfifo that reads a value? Shouldn't it 
be a get?

> +EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_set_spbmaxfifo);
> +
> +
> +/**
>    * snd_hdac_ext_stop_streams - stop all stream if running
>    * @ebus: HD-audio ext core bus
>    */
>

Re: [PATCH 3/5] ALSA: hdac: Add snd_hdac_get_hdac_stream()

[Vinod Koul]

On Fri, Aug 21, 2015 at 06:35:09PM +0200, Takashi Iwai wrote:
> On Fri, 21 Aug 2015 18:06:19 +0200,
> > +/**
> > + * snd_hdac_get_hdac_stream - return hdac_stream based on stream_tag and
> > + * direction
> > + *
> > + * @bus: HD-audio core bus
> > + */
> 
> Please complete the description.
Okay will add more details

> > +EXPORT_SYMBOL_GPL(snd_hdac_get_hdac_stream);
> >  /*
> 
> Put the empty line after EXPORT_SYMBOL_GPL().
Sure


-- 
~Vinod

Re: [PATCH 4/5] ALSA: hdac: Add support to enable SPIB for hdac ext stream

[Vinod Koul]

On Fri, Aug 21, 2015 at 12:38:02PM -0500, Pierre-Louis Bossart wrote:
> weird function name. a set_spbmaxfifo that reads a value? Shouldn't
> it be a get?
Sorry it is my bad as I inserted get while correcting few other things in
this patch. I will send an update fixing these things as Takashi applied
this

-- 
~Vinod