[PATCH 0/8] ALSA: hda: document/simplify hdac_ext handling

[PATCH 0/8] ALSA: hda: document/simplify hdac_ext handling

[Pierre-Louis Bossart]

This sound/hda/ext library is a confusing magic black box that very
few understand. It needn't be that way, we can document/simplify and
make the code clearer.

Pierre-Louis Bossart (8):
  ALSA: hda: make snd_hdac_stream_clear() static
  ALSA: hda: document state machine for hdac_streams
  ALSA: hda: ext: make snd_hdac_ext_stream_init() static
  ALSA: hda: Use hdac_ext prefix in snd_hdac_stream_free_all() for
    clarity
  ALSA: hda: add snd_hdac_stop_streams() helper
  ALSA: hda: ext: simplify logic for stream assignment
  ALSA: hda: ext: fix locking in stream_release
  ALSA: hda: ext: remove always-true conditions on host and link release

 include/sound/hdaudio.h         |  3 +-
 include/sound/hdaudio_ext.h     |  5 +--
 sound/hda/ext/hdac_ext_stream.c | 34 +++++++--------
 sound/hda/hdac_stream.c         | 74 +++++++++++++++++++++++++++++----
 sound/pci/hda/hda_controller.c  |  4 +-
 sound/soc/intel/avs/core.c      |  4 +-
 sound/soc/intel/skylake/skl.c   |  2 +-
 7 files changed, 87 insertions(+), 39 deletions(-)

-- 
2.34.1

[PATCH 1/8] ALSA: hda: make snd_hdac_stream_clear() static

[Pierre-Louis Bossart]

This helper has no users outside of hdac_stream.c. External users
should only use snd_hdac_stream_start() and snd_hdac_stream_stop().

No functional change beyond making the function static and removing
the symbol export.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 include/sound/hdaudio.h | 1 -
 sound/hda/hdac_stream.c | 5 ++---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 6e74aeafeda41..24c731e53ccb6 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -561,7 +561,6 @@ int snd_hdac_stream_setup_periods(struct hdac_stream *azx_dev);
 int snd_hdac_stream_set_params(struct hdac_stream *azx_dev,
 				unsigned int format_val);
 void snd_hdac_stream_start(struct hdac_stream *azx_dev, bool fresh_start);
-void snd_hdac_stream_clear(struct hdac_stream *azx_dev);
 void snd_hdac_stream_stop(struct hdac_stream *azx_dev);
 void snd_hdac_stop_streams_and_chip(struct hdac_bus *bus);
 void snd_hdac_stream_reset(struct hdac_stream *azx_dev);
diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index bdf6d4db67694..2dbde3d1cf683 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -112,10 +112,10 @@ void snd_hdac_stream_start(struct hdac_stream *azx_dev, bool fresh_start)
 EXPORT_SYMBOL_GPL(snd_hdac_stream_start);
 
 /**
- * snd_hdac_stream_clear - stop a stream DMA
+ * snd_hdac_stream_clear - helper to clear stream registers and stop DMA transfers
  * @azx_dev: HD-audio core stream to stop
  */
-void snd_hdac_stream_clear(struct hdac_stream *azx_dev)
+static void snd_hdac_stream_clear(struct hdac_stream *azx_dev)
 {
 	snd_hdac_stream_updateb(azx_dev, SD_CTL,
 				SD_CTL_DMA_START | SD_INT_MASK, 0);
@@ -124,7 +124,6 @@ void snd_hdac_stream_clear(struct hdac_stream *azx_dev)
 		snd_hdac_stream_updateb(azx_dev, SD_CTL_3B, SD_CTL_STRIPE_MASK, 0);
 	azx_dev->running = false;
 }
-EXPORT_SYMBOL_GPL(snd_hdac_stream_clear);
 
 /**
  * snd_hdac_stream_stop - stop a stream
-- 
2.34.1

[PATCH 2/8] ALSA: hda: document state machine for hdac_streams

[Pierre-Louis Bossart]

The code in this library is far from self-explanatory, hopefully this
state diagram reverse-engineered from the code will help others
understand the expected transitions.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/hda/hdac_stream.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index 2dbde3d1cf683..2e98f5fd50e54 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -13,6 +13,39 @@
 #include <sound/hda_register.h>
 #include "trace.h"
 
+/*
+ * the hdac_stream library is intended to be used with the following
+ * transitions. The states are not formally defined in the code but loosely
+ * inspired by boolean variables. Note that the 'prepared' field is not used
+ * in this library but by the callers during the hw_params/prepare transitions
+ *
+ *			   |
+ *	stream_init()	   |
+ *			   v
+ *			+--+-------+
+ *			|  unused  |
+ *			+--+----+--+
+ *			   |    ^
+ *	stream_assign()	   | 	|    stream_release()
+ *			   v	|
+ *			+--+----+--+
+ *			|  opened  |
+ *			+--+----+--+
+ *			   |    ^
+ *	stream_reset()	   |    |
+ *	stream_setup()	   |	|    stream_cleanup()
+ *			   v	|
+ *			+--+----+--+
+ *			| prepared |
+ *			+--+----+--+
+ *			   |    ^
+ *	stream_start()	   | 	|    stream_stop()
+ *			   v	|
+ *			+--+----+--+
+ *			|  running |
+ *			+----------+
+ */
+
 /**
  * snd_hdac_get_stream_stripe_ctl - get stripe control value
  * @bus: HD-audio core bus
-- 
2.34.1

[PATCH 3/8] ALSA: hda: ext: make snd_hdac_ext_stream_init() static

[Pierre-Louis Bossart]

There are no external users of this helper, move to static and remove
sympol export. No functionality change.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 include/sound/hdaudio_ext.h     | 3 ---
 sound/hda/ext/hdac_ext_stream.c | 7 +++----
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
index 07231f0b93b50..4a4bd1d88612f 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -77,9 +77,6 @@ struct hdac_ext_stream {
 #define stream_to_hdac_ext_stream(s) \
 	container_of(s, struct hdac_ext_stream, hstream)
 
-void snd_hdac_ext_stream_init(struct hdac_bus *bus,
-			      struct hdac_ext_stream *hext_stream, int idx,
-			      int direction, int tag);
 int snd_hdac_ext_stream_init_all(struct hdac_bus *bus, int start_idx,
 				 int num_stream, int dir);
 void snd_hdac_stream_free_all(struct hdac_bus *bus);
diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c
index d2b5724b463ff..5c665b26f8533 100644
--- a/sound/hda/ext/hdac_ext_stream.c
+++ b/sound/hda/ext/hdac_ext_stream.c
@@ -26,9 +26,9 @@
  * initialize the stream, if ppcap is enabled then init those and then
  * invoke hdac stream initialization routine
  */
-void snd_hdac_ext_stream_init(struct hdac_bus *bus,
-			      struct hdac_ext_stream *hext_stream,
-			      int idx, int direction, int tag)
+static void snd_hdac_ext_stream_init(struct hdac_bus *bus,
+				     struct hdac_ext_stream *hext_stream,
+				     int idx, int direction, int tag)
 {
 	if (bus->ppcap) {
 		hext_stream->pphc_addr = bus->ppcap + AZX_PPHC_BASE +
@@ -56,7 +56,6 @@ void snd_hdac_ext_stream_init(struct hdac_bus *bus,
 	hext_stream->decoupled = false;
 	snd_hdac_stream_init(bus, &hext_stream->hstream, idx, direction, tag);
 }
-EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_init);
 
 /**
  * snd_hdac_ext_stream_init_all - create and initialize the stream objects
-- 
2.34.1

[PATCH 4/8] ALSA: hda: Use hdac_ext prefix in snd_hdac_stream_free_all() for clarity

[Pierre-Louis Bossart]

Make sure there's no ambiguity on layering with the appropriate prefix
added.

Pure rename, no functionality changed.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 include/sound/hdaudio_ext.h     | 2 +-
 sound/hda/ext/hdac_ext_stream.c | 6 +++---
 sound/soc/intel/avs/core.c      | 4 ++--
 sound/soc/intel/skylake/skl.c   | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
index 4a4bd1d88612f..83aed26ab1433 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -79,7 +79,7 @@ struct hdac_ext_stream {
 
 int snd_hdac_ext_stream_init_all(struct hdac_bus *bus, int start_idx,
 				 int num_stream, int dir);
-void snd_hdac_stream_free_all(struct hdac_bus *bus);
+void snd_hdac_ext_stream_free_all(struct hdac_bus *bus);
 void snd_hdac_link_free_all(struct hdac_bus *bus);
 struct hdac_ext_stream *snd_hdac_ext_stream_assign(struct hdac_bus *bus,
 					   struct snd_pcm_substream *substream,
diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c
index 5c665b26f8533..9419abd7fc036 100644
--- a/sound/hda/ext/hdac_ext_stream.c
+++ b/sound/hda/ext/hdac_ext_stream.c
@@ -87,11 +87,11 @@ int snd_hdac_ext_stream_init_all(struct hdac_bus *bus, int start_idx,
 EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_init_all);
 
 /**
- * snd_hdac_stream_free_all - free hdac extended stream objects
+ * snd_hdac_ext_stream_free_all - free hdac extended stream objects
  *
  * @bus: HD-audio core bus
  */
-void snd_hdac_stream_free_all(struct hdac_bus *bus)
+void snd_hdac_ext_stream_free_all(struct hdac_bus *bus)
 {
 	struct hdac_stream *s, *_s;
 	struct hdac_ext_stream *hext_stream;
@@ -103,7 +103,7 @@ void snd_hdac_stream_free_all(struct hdac_bus *bus)
 		kfree(hext_stream);
 	}
 }
-EXPORT_SYMBOL_GPL(snd_hdac_stream_free_all);
+EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_free_all);
 
 void snd_hdac_ext_stream_decouple_locked(struct hdac_bus *bus,
 					 struct hdac_ext_stream *hext_stream,
diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
index c50c20fd681a1..bb0719c58ca49 100644
--- a/sound/soc/intel/avs/core.c
+++ b/sound/soc/intel/avs/core.c
@@ -466,7 +466,7 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
 
 err_acquire_irq:
 	snd_hdac_bus_free_stream_pages(bus);
-	snd_hdac_stream_free_all(bus);
+	snd_hdac_ext_stream_free_all(bus);
 err_init_streams:
 	iounmap(adev->dsp_ba);
 err_remap_bar4:
@@ -502,7 +502,7 @@ static void avs_pci_remove(struct pci_dev *pci)
 		snd_hda_codec_unregister(hdac_to_hda_codec(hdev));
 
 	snd_hdac_bus_free_stream_pages(bus);
-	snd_hdac_stream_free_all(bus);
+	snd_hdac_ext_stream_free_all(bus);
 	/* reverse ml_capabilities */
 	snd_hdac_link_free_all(bus);
 	snd_hdac_ext_bus_exit(bus);
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 52a041d6144c9..0122926f9c58b 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -444,7 +444,7 @@ static int skl_free(struct hdac_bus *bus)
 	if (bus->irq >= 0)
 		free_irq(bus->irq, (void *)bus);
 	snd_hdac_bus_free_stream_pages(bus);
-	snd_hdac_stream_free_all(bus);
+	snd_hdac_ext_stream_free_all(bus);
 	snd_hdac_link_free_all(bus);
 
 	if (bus->remap_addr)
-- 
2.34.1

[PATCH 5/8] ALSA: hda: add snd_hdac_stop_streams() helper

[Pierre-Louis Bossart]

Minor code reuse, no functionality change.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 include/sound/hdaudio.h        |  1 +
 sound/hda/hdac_stream.c        | 17 ++++++++++++++---
 sound/pci/hda/hda_controller.c |  4 +---
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 24c731e53ccb6..35459d740f008 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -562,6 +562,7 @@ int snd_hdac_stream_set_params(struct hdac_stream *azx_dev,
 				unsigned int format_val);
 void snd_hdac_stream_start(struct hdac_stream *azx_dev, bool fresh_start);
 void snd_hdac_stream_stop(struct hdac_stream *azx_dev);
+void snd_hdac_stop_streams(struct hdac_bus *bus);
 void snd_hdac_stop_streams_and_chip(struct hdac_bus *bus);
 void snd_hdac_stream_reset(struct hdac_stream *azx_dev);
 void snd_hdac_stream_sync_trigger(struct hdac_stream *azx_dev, bool set,
diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index 2e98f5fd50e54..c056bcc5543d1 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -174,17 +174,28 @@ void snd_hdac_stream_stop(struct hdac_stream *azx_dev)
 }
 EXPORT_SYMBOL_GPL(snd_hdac_stream_stop);
 
+/**
+ * snd_hdac_stop_streams - stop all streams
+ * @bus: HD-audio core bus
+ */
+void snd_hdac_stop_streams(struct hdac_bus *bus)
+{
+	struct hdac_stream *stream;
+
+	list_for_each_entry(stream, &bus->stream_list, list)
+		snd_hdac_stream_stop(stream);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_stop_streams);
+
 /**
  * snd_hdac_stop_streams_and_chip - stop all streams and chip if running
  * @bus: HD-audio core bus
  */
 void snd_hdac_stop_streams_and_chip(struct hdac_bus *bus)
 {
-	struct hdac_stream *stream;
 
 	if (bus->chip_init) {
-		list_for_each_entry(stream, &bus->stream_list, list)
-			snd_hdac_stream_stop(stream);
+		snd_hdac_stop_streams(bus);
 		snd_hdac_bus_stop_chip(bus);
 	}
 }
diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index 75dcb14ff20ad..0ff286b7b66be 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -1033,10 +1033,8 @@ EXPORT_SYMBOL_GPL(azx_init_chip);
 void azx_stop_all_streams(struct azx *chip)
 {
 	struct hdac_bus *bus = azx_bus(chip);
-	struct hdac_stream *s;
 
-	list_for_each_entry(s, &bus->stream_list, list)
-		snd_hdac_stream_stop(s);
+	snd_hdac_stop_streams(bus);
 }
 EXPORT_SYMBOL_GPL(azx_stop_all_streams);
 
-- 
2.34.1

[PATCH 6/8] ALSA: hda: ext: simplify logic for stream assignment

[Pierre-Louis Bossart]

The logic is needlessly complicated, the basic rule is:

The host streams can be found by checking the 'opened' boolean.
The link streams can be found by checking the 'link_locked' boolean.

Once a stream is found, it can be unconditionally decoupled. The
snd_hdac_ext_stream_decouple_locked() routine will make sure the
register status is modified as needed and the 'decoupled' boolean set.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/hda/ext/hdac_ext_stream.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c
index 9419abd7fc036..254df9a67bd2b 100644
--- a/sound/hda/ext/hdac_ext_stream.c
+++ b/sound/hda/ext/hdac_ext_stream.c
@@ -267,19 +267,15 @@ hdac_ext_link_stream_assign(struct hdac_bus *bus,
 		if (hstream->direction != substream->stream)
 			continue;
 
-		/* check if decoupled stream and not in use is available */
-		if (hext_stream->decoupled && !hext_stream->link_locked) {
-			res = hext_stream;
-			break;
-		}
-
+		/* check if link stream is available */
 		if (!hext_stream->link_locked) {
-			snd_hdac_ext_stream_decouple_locked(bus, hext_stream, true);
 			res = hext_stream;
 			break;
 		}
+
 	}
 	if (res) {
+		snd_hdac_ext_stream_decouple_locked(bus, res, true);
 		res->link_locked = 1;
 		res->link_substream = substream;
 	}
@@ -308,13 +304,12 @@ hdac_ext_host_stream_assign(struct hdac_bus *bus,
 			continue;
 
 		if (!hstream->opened) {
-			if (!hext_stream->decoupled)
-				snd_hdac_ext_stream_decouple_locked(bus, hext_stream, true);
 			res = hext_stream;
 			break;
 		}
 	}
 	if (res) {
+		snd_hdac_ext_stream_decouple_locked(bus, res, true);
 		res->hstream.opened = 1;
 		res->hstream.running = 0;
 		res->hstream.substream = substream;
-- 
2.34.1

[PATCH 7/8] ALSA: hda: ext: fix locking in stream_release

[Pierre-Louis Bossart]

The snd_hdac_ext_stream_release() routine uses the bus reg_lock, but
releases it before calling snd_hdac_stream_release() where the bus
reg_lock is taken again.

This creates a timing window where the link stream release could test
an invalid 'opened' boolean status and fail to recouple the host and
link parts.

Fix by exposing a locked version of snd_hdac_stream_release() and use
it without releasing the spinlock.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 include/sound/hdaudio.h         |  1 +
 sound/hda/ext/hdac_ext_stream.c |  2 +-
 sound/hda/hdac_stream.c         | 19 ++++++++++++++++---
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 35459d740f008..ddff03e546e9f 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -551,6 +551,7 @@ void snd_hdac_stream_init(struct hdac_bus *bus, struct hdac_stream *azx_dev,
 			  int idx, int direction, int tag);
 struct hdac_stream *snd_hdac_stream_assign(struct hdac_bus *bus,
 					   struct snd_pcm_substream *substream);
+void snd_hdac_stream_release_locked(struct hdac_stream *azx_dev);
 void snd_hdac_stream_release(struct hdac_stream *azx_dev);
 struct hdac_stream *snd_hdac_get_stream(struct hdac_bus *bus,
 					int dir, int stream_tag);
diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c
index 254df9a67bd2b..9a2bc7e803dd6 100644
--- a/sound/hda/ext/hdac_ext_stream.c
+++ b/sound/hda/ext/hdac_ext_stream.c
@@ -384,8 +384,8 @@ void snd_hdac_ext_stream_release(struct hdac_ext_stream *hext_stream, int type)
 		spin_lock_irq(&bus->reg_lock);
 		if (hext_stream->decoupled && !hext_stream->link_locked)
 			snd_hdac_ext_stream_decouple_locked(bus, hext_stream, false);
+		snd_hdac_stream_release_locked(&hext_stream->hstream);
 		spin_unlock_irq(&bus->reg_lock);
-		snd_hdac_stream_release(&hext_stream->hstream);
 		break;
 
 	case HDAC_EXT_STREAM_TYPE_LINK:
diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index c056bcc5543d1..1b8be39c38a96 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -365,6 +365,21 @@ struct hdac_stream *snd_hdac_stream_assign(struct hdac_bus *bus,
 }
 EXPORT_SYMBOL_GPL(snd_hdac_stream_assign);
 
+/**
+ * snd_hdac_stream_release_locked - release the assigned stream
+ * @azx_dev: HD-audio core stream to release
+ *
+ * Release the stream that has been assigned by snd_hdac_stream_assign().
+ * The bus->reg_lock needs to be taken at a higher level
+ */
+void snd_hdac_stream_release_locked(struct hdac_stream *azx_dev)
+{
+	azx_dev->opened = 0;
+	azx_dev->running = 0;
+	azx_dev->substream = NULL;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_stream_release_locked);
+
 /**
  * snd_hdac_stream_release - release the assigned stream
  * @azx_dev: HD-audio core stream to release
@@ -376,9 +391,7 @@ void snd_hdac_stream_release(struct hdac_stream *azx_dev)
 	struct hdac_bus *bus = azx_dev->bus;
 
 	spin_lock_irq(&bus->reg_lock);
-	azx_dev->opened = 0;
-	azx_dev->running = 0;
-	azx_dev->substream = NULL;
+	snd_hdac_stream_release_locked(azx_dev);
 	spin_unlock_irq(&bus->reg_lock);
 }
 EXPORT_SYMBOL_GPL(snd_hdac_stream_release);
-- 
2.34.1

[PATCH 8/8] ALSA: hda: ext: remove always-true conditions on host and link release

[Pierre-Louis Bossart]

By construction a host and link DMA are always decoupled. This
decoupling happens in the assign() phase. There's no point in checking
if the two parts are decoupled, this is by-design always-true.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/hda/ext/hdac_ext_stream.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c
index 9a2bc7e803dd6..70f3ad71aaf0d 100644
--- a/sound/hda/ext/hdac_ext_stream.c
+++ b/sound/hda/ext/hdac_ext_stream.c
@@ -382,7 +382,8 @@ void snd_hdac_ext_stream_release(struct hdac_ext_stream *hext_stream, int type)
 
 	case HDAC_EXT_STREAM_TYPE_HOST:
 		spin_lock_irq(&bus->reg_lock);
-		if (hext_stream->decoupled && !hext_stream->link_locked)
+		/* couple link only if not in use */
+		if (!hext_stream->link_locked)
 			snd_hdac_ext_stream_decouple_locked(bus, hext_stream, false);
 		snd_hdac_stream_release_locked(&hext_stream->hstream);
 		spin_unlock_irq(&bus->reg_lock);
@@ -390,7 +391,8 @@ void snd_hdac_ext_stream_release(struct hdac_ext_stream *hext_stream, int type)
 
 	case HDAC_EXT_STREAM_TYPE_LINK:
 		spin_lock_irq(&bus->reg_lock);
-		if (hext_stream->decoupled && !hext_stream->hstream.opened)
+		/* couple host only if not in use */
+		if (!hext_stream->hstream.opened)
 			snd_hdac_ext_stream_decouple_locked(bus, hext_stream, false);
 		hext_stream->link_locked = 0;
 		hext_stream->link_substream = NULL;
-- 
2.34.1

Re: [PATCH 0/8] ALSA: hda: document/simplify hdac_ext handling

[Takashi Iwai]

On Mon, 19 Sep 2022 14:10:33 +0200,
Pierre-Louis Bossart wrote:
> 
> This sound/hda/ext library is a confusing magic black box that very
> few understand. It needn't be that way, we can document/simplify and
> make the code clearer.
> 
> Pierre-Louis Bossart (8):
>   ALSA: hda: make snd_hdac_stream_clear() static
>   ALSA: hda: document state machine for hdac_streams
>   ALSA: hda: ext: make snd_hdac_ext_stream_init() static
>   ALSA: hda: Use hdac_ext prefix in snd_hdac_stream_free_all() for
>     clarity
>   ALSA: hda: add snd_hdac_stop_streams() helper
>   ALSA: hda: ext: simplify logic for stream assignment
>   ALSA: hda: ext: fix locking in stream_release
>   ALSA: hda: ext: remove always-true conditions on host and link release

Applied all patches now.  Thanks.


Takashi