[PATCH 00/19] ASoC: SOF: improvements for ABI checks and Intel code

[PATCH 00/19] ASoC: SOF: improvements for ABI checks and Intel code

[Pierre-Louis Bossart]

This series is a set of relatively small SOF changes after the big
batch merged last week (Thanks!). Since we are very close to the merge
window and in May where most of the world takes time off, it'd be
perfectly understandable if those patches were queued for 5.3, after
feedback and corrections as needed.

First we added optional stricter ABI checks when the firmware,
topology and kernel ABI levels differ, which can happen when patches
are merged at different times on Github. This is mainly for developers
and the Github CI to track disconnects, should they happen despite our
new process to keep evolutions under control. This option has no
impact on the usual problem of updating the kernel without updating
the firmware files.

The Intel changes are mainly
a) fixes for the slave mode suppport
b) clean-ups such as removal of a static index for HDAudio support or
removal of unneeded include file
c) use of a workqueue to defer period elapsed events after the
IPC completion.
d) simplification of the IRQ code
e) fixes to deal with resume on HDaudio links. In the previous
patchset we removed the RESUME_INFO flag but missed the need to
explicitly set the hw_params on resume.
f) a set of routines to dump the Intel IPC registers, mainly for early
platform enablement.

Keyon Jie (7):
  ASoC: SOF: Intel: cnl: add pointer ops to use DPIB position
  ASoC: SOF: PCM: add period_elapsed work to fix race condition in
    interrupt context
  ASoC: SOF: Intel: use snd_sof_pcm_period_elapsed
  ASoC: SOF: ipc: use snd_sof_pcm_period_elapsed
  ASoC: SOF: Intel: hda-ipc: simplify handling of IPC IRQ
  ASoC: SOF: Intel: hda-stream: store stream capabilities
  ASoC: SOF: Intel: hda-stream: handle real stream interrupts only

Pan Xiuli (3):
  ASoC: SOF: IPC: add ipc dump function
  ASoC: SOF: Intel: APL: add ipc dump function
  ASoC: SOF: Intel: CNL: add ipc dump function

Pierre-Louis Bossart (4):
  ASoC: SOF: add Kconfig option for strict ABI checks
  ASOC: SOF: ipc: add support for stricter ABI checks
  ASoC: SOF: topology: add support for stricter ABI checks
  ASoC: SOF: Intel: hda-pcm: remove useless dependency on hdac_ext

Ranjani Sridharan (1):
  ASoC: SOF: intel: hda: add hw_params_upon_resume flag for hda stream

Zhu Yingjiang (4):
  ASoC: SOF: Intel: hda: add the SSP Host Device memory space
  ASoC: SOF: Intel: hda: add SSP info to the chip info struct
  ASoC: SOF: Intel: hda: set I2S slave before enabling DSP
  ASoC: SOF: Intel: hda: set bus->idx as 0

 sound/soc/sof/Kconfig            | 15 ++++++++++
 sound/soc/sof/intel/apl.c        |  4 +++
 sound/soc/sof/intel/cnl.c        | 27 +++++++++++++++---
 sound/soc/sof/intel/hda-bus.c    |  9 ++++--
 sound/soc/sof/intel/hda-dai.c    | 23 +++++++++------
 sound/soc/sof/intel/hda-dsp.c    | 16 +++++++++++
 sound/soc/sof/intel/hda-ipc.c    | 13 ++++-----
 sound/soc/sof/intel/hda-loader.c | 11 ++++++++
 sound/soc/sof/intel/hda-pcm.c    |  1 -
 sound/soc/sof/intel/hda-stream.c | 15 ++++++++--
 sound/soc/sof/intel/hda.c        | 18 ++++++++++++
 sound/soc/sof/intel/hda.h        | 23 +++++++++++++++
 sound/soc/sof/intel/shim.h       |  2 ++
 sound/soc/sof/ipc.c              | 12 +++++++-
 sound/soc/sof/ops.h              | 12 ++++++++
 sound/soc/sof/pcm.c              | 48 ++++++++++++++++++++++++++++++++
 sound/soc/sof/pm.c               |  3 ++
 sound/soc/sof/sof-priv.h         |  5 +++-
 sound/soc/sof/topology.c         | 43 +++++++++++++++++++---------
 19 files changed, 257 insertions(+), 43 deletions(-)

-- 
2.17.1

[PATCH 01/19] ASoC: SOF: add Kconfig option for strict ABI checks

[Pierre-Louis Bossart]

When the kernel is more recent than firmware files, it will always
behave in backwards-compatible ways.

Add optional behavior to check if the kernel is older than the
firmware files, so that the kernel fails early instead of attempting
to use new functionality it does not support.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/Kconfig | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
index eaa380092c3b..a1a9ffe605dc 100644
--- a/sound/soc/sof/Kconfig
+++ b/sound/soc/sof/Kconfig
@@ -53,6 +53,21 @@ config SND_SOC_SOF_NOCODEC
 	  Say Y if you need this nocodec fallback option
 	  If unsure select "N".
 
+config SND_SOC_SOF_STRICT_ABI_CHECKS
+	bool "SOF strict ABI checks"
+	help
+	  This option enables strict ABI checks for firmware and topology
+	  files.
+	  When these files are more recent than the kernel, the kernel
+	  will handle the functionality it supports and may report errors
+	  during topology creation or run-time usage if new functionality
+	  is invoked.
+	  This option will stop topology creation and firmware load upfront.
+	  It is intended for SOF CI/releases and not for users or distros.
+	  Say Y if you want strict ABI checks for an SOF release
+	  If you are not involved in SOF releases and CI development
+	  select "N".
+
 config SND_SOC_SOF_DEBUG
 	bool "SOF debugging features"
 	help
-- 
2.17.1

[PATCH 02/19] ASOC: SOF: ipc: add support for stricter ABI checks

[Pierre-Louis Bossart]

Fail early if firmware is more recent than kernel and Kconfig is
selected.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/ipc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c
index d00373ceca12..bf58d58e7379 100644
--- a/sound/soc/sof/ipc.c
+++ b/sound/soc/sof/ipc.c
@@ -762,6 +762,15 @@ int snd_sof_ipc_valid(struct snd_sof_dev *sdev)
 		return -EINVAL;
 	}
 
+	if (v->abi_version > SOF_ABI_VERSION) {
+		if (!IS_ENABLED(CONFIG_SND_SOC_SOF_STRICT_ABI_CHECKS)) {
+			dev_warn(sdev->dev, "warn: FW ABI is more recent than kernel\n");
+		} else {
+			dev_err(sdev->dev, "error: FW ABI is more recent than kernel\n");
+			return -EINVAL;
+		}
+	}
+
 	if (ready->debug.bits.build) {
 		dev_info(sdev->dev,
 			 "Firmware debug build %d on %s-%s - options:\n"
-- 
2.17.1

[PATCH 03/19] ASoC: SOF: topology: add support for stricter ABI checks

[Pierre-Louis Bossart]

Fail early if topology is more recent than kernel and Kconfig is
selected.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/topology.c | 43 ++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
index 2b9de1b97447..b04b99cc0ff8 100644
--- a/sound/soc/sof/topology.c
+++ b/sound/soc/sof/topology.c
@@ -3051,6 +3051,7 @@ static int sof_manifest(struct snd_soc_component *scomp, int index,
 {
 	struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
 	u32 size;
+	u32 abi_version;
 
 	size = le32_to_cpu(man->priv.size);
 
@@ -3060,20 +3061,36 @@ static int sof_manifest(struct snd_soc_component *scomp, int index,
 		return 0;
 	}
 
-	if (size == SOF_TPLG_ABI_SIZE) {
-		dev_info(sdev->dev,
-			 "Topology: ABI %d:%d:%d Kernel ABI %d:%d:%d\n",
-			 man->priv.data[0], man->priv.data[1],
-			 man->priv.data[2], SOF_ABI_MAJOR, SOF_ABI_MINOR,
-			 SOF_ABI_PATCH);
-		if (SOF_ABI_VER(man->priv.data[0], man->priv.data[1],
-				man->priv.data[2]) <= SOF_ABI_VERSION)
-			return 0;
+	if (size != SOF_TPLG_ABI_SIZE) {
+		dev_err(sdev->dev, "error: invalid topology ABI size\n");
+		return -EINVAL;
 	}
-	dev_err(sdev->dev,
-		"error: Incompatible ABI version %d:%d:%d\n",
-		man->priv.data[0], man->priv.data[1], man->priv.data[2]);
-	return -EINVAL;
+
+	dev_info(sdev->dev,
+		 "Topology: ABI %d:%d:%d Kernel ABI %d:%d:%d\n",
+		 man->priv.data[0], man->priv.data[1],
+		 man->priv.data[2], SOF_ABI_MAJOR, SOF_ABI_MINOR,
+		 SOF_ABI_PATCH);
+
+	abi_version = SOF_ABI_VER(man->priv.data[0],
+				  man->priv.data[1],
+				  man->priv.data[2]);
+
+	if (SOF_ABI_VERSION_INCOMPATIBLE(SOF_ABI_VERSION, abi_version)) {
+		dev_err(sdev->dev, "error: incompatible topology ABI version\n");
+		return -EINVAL;
+	}
+
+	if (abi_version > SOF_ABI_VERSION) {
+		if (!IS_ENABLED(CONFIG_SND_SOC_SOF_STRICT_ABI_CHECKS)) {
+			dev_warn(sdev->dev, "warn: topology ABI is more recent than kernel\n");
+		} else {
+			dev_err(sdev->dev, "error: topology ABI is more recent than kernel\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
 }
 
 /* vendor specific kcontrol handlers available for binding */
-- 
2.17.1

[PATCH 04/19] ASoC: SOF: Intel: cnl: add pointer ops to use DPIB position

[Pierre-Louis Bossart]

From: Keyon Jie <yang.jie@linux.intel.com>

Add .pcm_pointer ops for cannonlake to read DPIB/posbuf and get pointer
for ALSA, to align with apollolake.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/cnl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c
index 3e95c1e5e491..36ae9b88d0dc 100644
--- a/sound/soc/sof/intel/cnl.c
+++ b/sound/soc/sof/intel/cnl.c
@@ -199,6 +199,7 @@ const struct snd_sof_dsp_ops sof_cnl_ops = {
 	.pcm_close	= hda_dsp_pcm_close,
 	.pcm_hw_params	= hda_dsp_pcm_hw_params,
 	.pcm_trigger	= hda_dsp_pcm_trigger,
+	.pcm_pointer	= hda_dsp_pcm_pointer,
 
 	/* firmware loading */
 	.load_firmware = snd_sof_load_firmware_raw,
-- 
2.17.1

[PATCH 05/19] ASoC: SOF: Intel: hda: add the SSP Host Device memory space

[Pierre-Louis Bossart]

From: Zhu Yingjiang <yingjiang.zhu@linux.intel.com>

The DSP SSP device memory can be conditionally accessed by
the host(depending on access policy).

Add the SSP base memory offset of APL and CNL.

Signed-off-by: Zhu Yingjiang <yingjiang.zhu@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/hda.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 4efcf85af6b5..c4550f2e493d 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -338,6 +338,13 @@
 #define HDA_ADSP_FW_STATUS_SKL		HDA_ADSP_SRAM0_BASE_SKL
 #define HDA_ADSP_ERROR_CODE_SKL		(HDA_ADSP_FW_STATUS_SKL + 0x4)
 
+/* Host Device Memory Space */
+#define APL_SSP_BASE_OFFSET	0x2000
+#define CNL_SSP_BASE_OFFSET	0x10000
+
+/* Host Device Memory Size of a Single SSP */
+#define SSP_DEV_MEM_SIZE	0x1000
+
 #define HDA_IDISP_CODEC(x) ((x) & BIT(2))
 
 struct sof_intel_dsp_bdl {
-- 
2.17.1

[PATCH 06/19] ASoC: SOF: Intel: hda: add SSP info to the chip info struct

[Pierre-Louis Bossart]

From: Zhu Yingjiang <yingjiang.zhu@linux.intel.com>

add SSP info of APL and CNL, to the sof_intel_dsp_desc
structure. The max SSP count the platform support and
the SSP base address.

Signed-off-by: Zhu Yingjiang <yingjiang.zhu@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/apl.c  | 2 ++
 sound/soc/sof/intel/cnl.c  | 2 ++
 sound/soc/sof/intel/hda.h  | 4 ++++
 sound/soc/sof/intel/shim.h | 2 ++
 4 files changed, 10 insertions(+)

diff --git a/sound/soc/sof/intel/apl.c b/sound/soc/sof/intel/apl.c
index 8c6282606944..026dde839621 100644
--- a/sound/soc/sof/intel/apl.c
+++ b/sound/soc/sof/intel/apl.c
@@ -105,5 +105,7 @@ const struct sof_intel_dsp_desc apl_chip_info = {
 	.ipc_ack_mask = HDA_DSP_REG_HIPCIE_DONE,
 	.ipc_ctl = HDA_DSP_REG_HIPCCTL,
 	.rom_init_timeout	= 150,
+	.ssp_count = APL_SSP_COUNT,
+	.ssp_base_offset = APL_SSP_BASE_OFFSET,
 };
 EXPORT_SYMBOL(apl_chip_info);
diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c
index 36ae9b88d0dc..3afd96d9c925 100644
--- a/sound/soc/sof/intel/cnl.c
+++ b/sound/soc/sof/intel/cnl.c
@@ -246,5 +246,7 @@ const struct sof_intel_dsp_desc cnl_chip_info = {
 	.ipc_ack_mask = CNL_DSP_REG_HIPCIDA_DONE,
 	.ipc_ctl = CNL_DSP_REG_HIPCCTL,
 	.rom_init_timeout	= 300,
+	.ssp_count = CNL_SSP_COUNT,
+	.ssp_base_offset = CNL_SSP_BASE_OFFSET,
 };
 EXPORT_SYMBOL(cnl_chip_info);
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index c4550f2e493d..b2cf6fa15575 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -345,6 +345,10 @@
 /* Host Device Memory Size of a Single SSP */
 #define SSP_DEV_MEM_SIZE	0x1000
 
+/* SSP Count of the Platform */
+#define APL_SSP_COUNT		6
+#define CNL_SSP_COUNT		3
+
 #define HDA_IDISP_CODEC(x) ((x) & BIT(2))
 
 struct sof_intel_dsp_bdl {
diff --git a/sound/soc/sof/intel/shim.h b/sound/soc/sof/intel/shim.h
index 11fd77cb4e6d..f7a3f62e45d4 100644
--- a/sound/soc/sof/intel/shim.h
+++ b/sound/soc/sof/intel/shim.h
@@ -162,6 +162,8 @@ struct sof_intel_dsp_desc {
 	int ipc_ack_mask;
 	int ipc_ctl;
 	int rom_init_timeout;
+	int ssp_count;			/* ssp count of the platform */
+	int ssp_base_offset;		/* base address of the SSPs */
 };
 
 extern const struct snd_sof_dsp_ops sof_tng_ops;
-- 
2.17.1

[PATCH 07/19] ASoC: SOF: Intel: hda: set I2S slave before enabling DSP

[Pierre-Louis Bossart]

From: Zhu Yingjiang <yingjiang.zhu@linux.intel.com>

By default, the I2S ports are configured in master mode during
DSP powerup sequences, the FS and BCLK lines will be driven on
startup, even when the topology file explicitly requires the
SSP to be slave.

This may be problematic for external components configured in
master mode who don't expect the Intel SOC/PCH to drive. Fix by
configuring the SSP as slave before the SSP outputs are enabled
to avoid this transient behavior.

When the topology file configures the SSP as clock master, the
initial slave configuration will be overridden.

Signed-off-by: Zhu Yingjiang <yingjiang.zhu@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/hda-loader.c | 11 +++++++++++
 sound/soc/sof/intel/hda.h        |  6 ++++++
 2 files changed, 17 insertions(+)

diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c
index 6a44bc349e44..6427f0b3a2f1 100644
--- a/sound/soc/sof/intel/hda-loader.c
+++ b/sound/soc/sof/intel/hda-loader.c
@@ -84,6 +84,7 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, const void *fwdata,
 	const struct sof_intel_dsp_desc *chip = hda->desc;
 	unsigned int status;
 	int ret;
+	int i;
 
 	/* step 1: power up corex */
 	ret = hda_dsp_core_power_up(sdev, chip->cores_mask);
@@ -92,6 +93,16 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, const void *fwdata,
 		goto err;
 	}
 
+	/* DSP is powered up, set all SSPs to slave mode */
+	for (i = 0; i < chip->ssp_count; i++) {
+		snd_sof_dsp_update_bits_unlocked(sdev, HDA_DSP_BAR,
+						 chip->ssp_base_offset
+						 + i * SSP_DEV_MEM_SIZE
+						 + SSP_SSC1_OFFSET,
+						 SSP_SET_SLAVE,
+						 SSP_SET_SLAVE);
+	}
+
 	/* step 2: purge FW request */
 	snd_sof_dsp_write(sdev, HDA_DSP_BAR, chip->ipc_req,
 			  chip->ipc_req_mask | (HDA_DSP_IPC_PURGE_FW |
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index b2cf6fa15575..3799a3518106 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -349,6 +349,12 @@
 #define APL_SSP_COUNT		6
 #define CNL_SSP_COUNT		3
 
+/* SSP Registers */
+#define SSP_SSC1_OFFSET		0x4
+#define SSP_SET_SCLK_SLAVE	BIT(25)
+#define SSP_SET_SFRM_SLAVE	BIT(24)
+#define SSP_SET_SLAVE		(SSP_SET_SCLK_SLAVE | SSP_SET_SFRM_SLAVE)
+
 #define HDA_IDISP_CODEC(x) ((x) & BIT(2))
 
 struct sof_intel_dsp_bdl {
-- 
2.17.1

[PATCH 08/19] ASoC: SOF: Intel: hda: set bus->idx as 0

[Pierre-Louis Bossart]

From: Zhu Yingjiang <yingjiang.zhu@linux.intel.com>

Setting the bus->idx as 0, for we only have one HDA
bus atm. This need to be fixed when there are more
than one HDA bus.

Signed-off-by: Zhu Yingjiang <yingjiang.zhu@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/hda-bus.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/sound/soc/sof/intel/hda-bus.c b/sound/soc/sof/intel/hda-bus.c
index 62cc9921bb55..a7e6d8227df6 100644
--- a/sound/soc/sof/intel/hda-bus.c
+++ b/sound/soc/sof/intel/hda-bus.c
@@ -80,8 +80,6 @@ static const struct hdac_io_ops io_ops = {
 void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev,
 		      const struct hdac_ext_bus_ops *ext_ops)
 {
-	static int idx;
-
 	memset(bus, 0, sizeof(*bus));
 	bus->dev = dev;
 
@@ -90,7 +88,12 @@ void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev,
 
 	bus->irq = -1;
 	bus->ext_ops = ext_ops;
-	bus->idx = idx++;
+
+	/*
+	 * There is only one HDA bus atm. keep the index as 0.
+	 * Need to fix when there are more than one HDA bus.
+	 */
+	bus->idx = 0;
 
 	spin_lock_init(&bus->reg_lock);
 
-- 
2.17.1

[PATCH 09/19] ASoC: SOF: Intel: hda-pcm: remove useless dependency on hdac_ext

[Pierre-Louis Bossart]

Nothing depends on definitions in hdaudio_ext.h, don't include it

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/hda-pcm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/sof/intel/hda-pcm.c b/sound/soc/sof/intel/hda-pcm.c
index 5714a79fbe1a..9b730f183529 100644
--- a/sound/soc/sof/intel/hda-pcm.c
+++ b/sound/soc/sof/intel/hda-pcm.c
@@ -15,7 +15,6 @@
  * Hardware interface for generic Intel audio DSP HDA IP
  */
 
-#include <sound/hdaudio_ext.h>
 #include <sound/hda_register.h>
 #include <sound/pcm_params.h>
 #include "../ops.h"
-- 
2.17.1

[PATCH 10/19] ASoC: SOF: PCM: add period_elapsed work to fix race condition in interrupt context

[Pierre-Louis Bossart]

From: Keyon Jie <yang.jie@linux.intel.com>

The IPC implementation in SOF requires sending IPCs serially: we should
not send a new IPC command to the firmware before we get an ACK (or time
out) from firmware, and the IRQ processing is complete.

snd_pcm_period_elapsed() can be called in interrupt context before
IRQ_HANDLED is returned. When the PCM is done draining, a STOP
IPC will then be sent, which breaks the expectation that IPCs are
handled serially and leads to IPC timeouts.

This patch adds a workqueue to defer the call to snd_pcm_elapsed() after
the IRQ is handled.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/pcm.c      | 48 ++++++++++++++++++++++++++++++++++++++++
 sound/soc/sof/sof-priv.h |  2 ++
 2 files changed, 50 insertions(+)

diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
index be4984c4da4e..649968841dad 100644
--- a/sound/soc/sof/pcm.c
+++ b/sound/soc/sof/pcm.c
@@ -52,6 +52,48 @@ static int sof_pcm_dsp_params(struct snd_sof_pcm *spcm, struct snd_pcm_substream
 	return ret;
 }
 
+/*
+ * sof pcm period elapse work
+ */
+static void sof_pcm_period_elapsed_work(struct work_struct *work)
+{
+	struct snd_sof_pcm_stream *sps =
+		container_of(work, struct snd_sof_pcm_stream,
+			     period_elapsed_work);
+
+	snd_pcm_period_elapsed(sps->substream);
+}
+
+/*
+ * sof pcm period elapse, this could be called at irq thread context.
+ */
+void snd_sof_pcm_period_elapsed(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_component *component =
+		snd_soc_rtdcom_lookup(rtd, DRV_NAME);
+	struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component);
+	struct snd_sof_pcm *spcm;
+
+	spcm = snd_sof_find_spcm_dai(sdev, rtd);
+	if (!spcm) {
+		dev_err(sdev->dev,
+			"error: period elapsed for unknown stream!\n");
+		return;
+	}
+
+	/*
+	 * snd_pcm_period_elapsed() can be called in interrupt context
+	 * before IRQ_HANDLED is returned. Inside snd_pcm_period_elapsed(),
+	 * when the PCM is done draining or xrun happened, a STOP IPC will
+	 * then be sent and this IPC will hit IPC timeout.
+	 * To avoid sending IPC before the previous IPC is handled, we
+	 * schedule delayed work here to call the snd_pcm_period_elapsed().
+	 */
+	schedule_work(&spcm->stream[substream->stream].period_elapsed_work);
+}
+EXPORT_SYMBOL(snd_sof_pcm_period_elapsed);
+
 /* this may get called several times by oss emulation */
 static int sof_pcm_hw_params(struct snd_pcm_substream *substream,
 			     struct snd_pcm_hw_params *params)
@@ -169,6 +211,9 @@ static int sof_pcm_hw_params(struct snd_pcm_substream *substream,
 	/* save pcm hw_params */
 	memcpy(&spcm->params[substream->stream], params, sizeof(*params));
 
+	INIT_WORK(&spcm->stream[substream->stream].period_elapsed_work,
+		  sof_pcm_period_elapsed_work);
+
 	return ret;
 }
 
@@ -203,6 +248,9 @@ static int sof_pcm_hw_free(struct snd_pcm_substream *substream)
 				 sizeof(stream), &reply, sizeof(reply));
 
 	snd_pcm_lib_free_pages(substream);
+
+	cancel_work_sync(&spcm->stream[substream->stream].period_elapsed_work);
+
 	return ret;
 }
 
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index 35e78ffecce2..675bb10c82f5 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -274,6 +274,7 @@ struct snd_sof_pcm_stream {
 	struct snd_dma_buffer page_table;
 	struct sof_ipc_stream_posn posn;
 	struct snd_pcm_substream *substream;
+	struct work_struct period_elapsed_work;
 };
 
 /* ALSA SOF PCM device */
@@ -495,6 +496,7 @@ struct snd_sof_pcm *snd_sof_find_spcm_comp(struct snd_sof_dev *sdev,
 					   int *direction);
 struct snd_sof_pcm *snd_sof_find_spcm_pcm_id(struct snd_sof_dev *sdev,
 					     unsigned int pcm_id);
+void snd_sof_pcm_period_elapsed(struct snd_pcm_substream *substream);
 
 /*
  * Stream IPC
-- 
2.17.1

[PATCH 11/19] ASoC: SOF: Intel: use snd_sof_pcm_period_elapsed

[Pierre-Louis Bossart]

From: Keyon Jie <yang.jie@linux.intel.com>

Switch to a wrapper function which schedules the actual call of
snd_pcm_period_elapsed after the current IPC is completed.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/hda-stream.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c
index 6290b2df5e62..47eff161c60e 100644
--- a/sound/soc/sof/intel/hda-stream.c
+++ b/sound/soc/sof/intel/hda-stream.c
@@ -465,8 +465,8 @@ irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context)
 {
 	struct hdac_bus *bus = context;
 	struct sof_intel_hda_dev *sof_hda = bus_to_sof_hda(bus);
-	struct hdac_stream *s;
 	u32 status = snd_hdac_chip_readl(bus, INTSTS);
+	struct hdac_stream *s;
 	u32 sd_status;
 
 	/* check streams */
@@ -486,7 +486,7 @@ irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context)
 
 			/* Inform ALSA only in case not do that with IPC */
 			if (sof_hda->no_ipc_position)
-				snd_pcm_period_elapsed(s->substream);
+				snd_sof_pcm_period_elapsed(s->substream);
 
 		}
 	}
-- 
2.17.1

[PATCH 12/19] ASoC: SOF: ipc: use snd_sof_pcm_period_elapsed

[Pierre-Louis Bossart]

From: Keyon Jie <yang.jie@linux.intel.com>

Switch to a wrapper function which schedules the actual call of
snd_pcm_period_elapsed after the current IPC is completed.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/ipc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c
index bf58d58e7379..437e80a07bc2 100644
--- a/sound/soc/sof/ipc.c
+++ b/sound/soc/sof/ipc.c
@@ -445,7 +445,7 @@ static void ipc_period_elapsed(struct snd_sof_dev *sdev, u32 msg_id)
 
 	/* only inform ALSA for period_wakeup mode */
 	if (!stream->substream->runtime->no_period_wakeup)
-		snd_pcm_period_elapsed(stream->substream);
+		snd_sof_pcm_period_elapsed(stream->substream);
 }
 
 /* DSP notifies host of an XRUN within FW */
-- 
2.17.1

[PATCH 13/19] ASoC: SOF: Intel: hda-ipc: simplify handling of IPC IRQ

[Pierre-Louis Bossart]

From: Keyon Jie <yang.jie@linux.intel.com>

When using a shared IRQ between IPC interrupt and stream IOC interrupt,
the interrupt handlers need to check the interrupt source before
scheduling their respective IRQ threads. In the case of IPC handler, it
should check if it is an IPC interrupt before waking up the IPC IRQ
thread.

The IPC IRQ thread, once scheduled, does not need to check the IRQ
source again. So, remove the superfluous check in the thread. Remove the
irq_status field from snd_sof_dev struct also as it is no longer needed.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/cnl.c     |  4 ----
 sound/soc/sof/intel/hda-ipc.c | 13 +++++--------
 sound/soc/sof/sof-priv.h      |  1 -
 3 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c
index 3afd96d9c925..d128839b2450 100644
--- a/sound/soc/sof/intel/cnl.c
+++ b/sound/soc/sof/intel/cnl.c
@@ -39,10 +39,6 @@ static irqreturn_t cnl_ipc_irq_thread(int irq, void *context)
 	u32 msg_ext;
 	irqreturn_t ret = IRQ_NONE;
 
-	/* here we handle IPC interrupts only */
-	if (!(sdev->irq_status & HDA_DSP_ADSPIS_IPC))
-		return ret;
-
 	hipcida = snd_sof_dsp_read(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDA);
 	hipcctl = snd_sof_dsp_read(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCCTL);
 	hipctdr = snd_sof_dsp_read(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCTDR);
diff --git a/sound/soc/sof/intel/hda-ipc.c b/sound/soc/sof/intel/hda-ipc.c
index a938f568dbb1..73ead7070cde 100644
--- a/sound/soc/sof/intel/hda-ipc.c
+++ b/sound/soc/sof/intel/hda-ipc.c
@@ -145,10 +145,6 @@ irqreturn_t hda_dsp_ipc_irq_thread(int irq, void *context)
 	u32 msg;
 	u32 msg_ext;
 
-	/* here we handle IPC interrupts only */
-	if (!(sdev->irq_status & HDA_DSP_ADSPIS_IPC))
-		return IRQ_NONE;
-
 	/* read IPC status */
 	hipcie = snd_sof_dsp_read(sdev, HDA_DSP_BAR,
 				  HDA_DSP_REG_HIPCIE);
@@ -234,19 +230,20 @@ irqreturn_t hda_dsp_ipc_irq_handler(int irq, void *context)
 {
 	struct snd_sof_dev *sdev = context;
 	int ret = IRQ_NONE;
+	u32 irq_status;
 
 	spin_lock(&sdev->hw_lock);
 
 	/* store status */
-	sdev->irq_status = snd_sof_dsp_read(sdev, HDA_DSP_BAR,
-					    HDA_DSP_REG_ADSPIS);
+	irq_status = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_REG_ADSPIS);
+	dev_vdbg(sdev->dev, "irq handler: irq_status:0x%x\n", irq_status);
 
 	/* invalid message ? */
-	if (sdev->irq_status == 0xffffffff)
+	if (irq_status == 0xffffffff)
 		goto out;
 
 	/* IPC message ? */
-	if (sdev->irq_status & HDA_DSP_ADSPIS_IPC) {
+	if (irq_status & HDA_DSP_ADSPIS_IPC) {
 		/* disable IPC interrupt */
 		snd_sof_dsp_update_bits_unlocked(sdev, HDA_DSP_BAR,
 						 HDA_DSP_REG_ADSPIC,
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index 675bb10c82f5..bbc285018f9a 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -366,7 +366,6 @@ struct snd_sof_dev {
 	struct snd_sof_mailbox host_box;	/* Host initiated IPC */
 	struct snd_sof_mailbox stream_box;	/* Stream position update */
 	struct snd_sof_ipc_msg *msg;
-	u64 irq_status;
 	int ipc_irq;
 	u32 next_comp_id; /* monotonic - reset during S3 */
 
-- 
2.17.1

[PATCH 14/19] ASoC: SOF: Intel: hda-stream: store stream capabilities

[Pierre-Louis Bossart]

From: Keyon Jie <yang.jie@linux.intel.com>

Add stream_max into struct sof_intel_hda_dev to store the total hda
stream number that the platform can support, and initialize it at
stream_init.

This can be used later e.g. for stream bitmask.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/hda-stream.c | 4 ++++
 sound/soc/sof/intel/hda.h        | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c
index 47eff161c60e..39f4212bde4b 100644
--- a/sound/soc/sof/intel/hda-stream.c
+++ b/sound/soc/sof/intel/hda-stream.c
@@ -500,6 +500,7 @@ int hda_dsp_stream_init(struct snd_sof_dev *sdev)
 	struct hdac_ext_stream *stream;
 	struct hdac_stream *hstream;
 	struct pci_dev *pci = to_pci_dev(sdev->dev);
+	struct sof_intel_hda_dev *sof_hda = bus_to_sof_hda(bus);
 	int sd_offset;
 	int i, num_playback, num_capture, num_total, ret;
 	u32 gcap;
@@ -657,6 +658,9 @@ int hda_dsp_stream_init(struct snd_sof_dev *sdev)
 		list_add_tail(&hstream->list, &bus->stream_list);
 	}
 
+	/* store total stream count (playback + capture) from GCAP */
+	sof_hda->stream_max = num_total;
+
 	return 0;
 }
 
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 3799a3518106..7cb2fb92c4eb 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -383,6 +383,9 @@ struct sof_intel_hda_dev {
 	/* if position update IPC needed */
 	u32 no_ipc_position;
 
+	/* the maximum number of streams (playback + capture) supported */
+	u32 stream_max;
+
 	int irq;
 
 	/* DMIC device */
-- 
2.17.1

[PATCH 15/19] ASoC: SOF: Intel: hda-stream: handle real stream interrupts only

[Pierre-Louis Bossart]

From: Keyon Jie <yang.jie@linux.intel.com>

The stream and IPC share the same interrupt. The stream interrupt
handler mistakenly uses the ipc interrupt and return IRQ_HANDLED,
causing the ipc interrupt to be missed.

Make sure the stream interrupt handler only deals with stream-related
interrupts.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/hda-stream.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c
index 39f4212bde4b..c92006f89499 100644
--- a/sound/soc/sof/intel/hda-stream.c
+++ b/sound/soc/sof/intel/hda-stream.c
@@ -433,6 +433,8 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev,
 irqreturn_t hda_dsp_stream_interrupt(int irq, void *context)
 {
 	struct hdac_bus *bus = context;
+	struct sof_intel_hda_dev *sof_hda = bus_to_sof_hda(bus);
+	u32 stream_mask;
 	u32 status;
 
 	if (!pm_runtime_active(bus->dev))
@@ -441,7 +443,10 @@ irqreturn_t hda_dsp_stream_interrupt(int irq, void *context)
 	spin_lock(&bus->reg_lock);
 
 	status = snd_hdac_chip_readl(bus, INTSTS);
-	if (status == 0 || status == 0xffffffff) {
+	stream_mask = GENMASK(sof_hda->stream_max - 1, 0) | AZX_INT_CTRL_EN;
+
+	/* Not stream interrupt or register inaccessible, ignore it.*/
+	if (!(status & stream_mask) || status == 0xffffffff) {
 		spin_unlock(&bus->reg_lock);
 		return IRQ_NONE;
 	}
-- 
2.17.1

[PATCH 16/19] ASoC: SOF: intel: hda: add hw_params_upon_resume flag for hda stream

[Pierre-Louis Bossart]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

The prepare() ioctl for BE dai link gets called both
when the stream is started and when it is resumed from
suspend. SOF uses this ioctl to set the hw params
again only if the stream has been suspended.

When the stream is started, the hw_params ioctl gets called
before prepare() and hw_params is set for the BE dai link.
So the prepare call does not need to do anything further.

When the stream resumes after system suspend, SOF requires
that the hw_params be set again for the BE dai. In order
to determine which streams should set the hw params
during prepare(), an internal flag called "hw_params_upon_resume"
is introduced in struct sof_intel_hda_stream. The flag is set
for hda streams when the sof device suspends and is
cleared after hw_params is set.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/apl.c     |  1 +
 sound/soc/sof/intel/cnl.c     |  1 +
 sound/soc/sof/intel/hda-dai.c | 23 ++++++++++++++---------
 sound/soc/sof/intel/hda-dsp.c | 16 ++++++++++++++++
 sound/soc/sof/intel/hda.h     |  2 ++
 sound/soc/sof/ops.h           |  6 ++++++
 sound/soc/sof/pm.c            |  3 +++
 sound/soc/sof/sof-priv.h      |  1 +
 8 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/sound/soc/sof/intel/apl.c b/sound/soc/sof/intel/apl.c
index 026dde839621..d1fb2f2ba1e1 100644
--- a/sound/soc/sof/intel/apl.c
+++ b/sound/soc/sof/intel/apl.c
@@ -91,6 +91,7 @@ const struct snd_sof_dsp_ops sof_apl_ops = {
 	.resume			= hda_dsp_resume,
 	.runtime_suspend	= hda_dsp_runtime_suspend,
 	.runtime_resume		= hda_dsp_runtime_resume,
+	.set_hw_params_upon_resume = hda_dsp_set_hw_params_upon_resume,
 };
 EXPORT_SYMBOL(sof_apl_ops);
 
diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c
index d128839b2450..2eac20bbcaea 100644
--- a/sound/soc/sof/intel/cnl.c
+++ b/sound/soc/sof/intel/cnl.c
@@ -225,6 +225,7 @@ const struct snd_sof_dsp_ops sof_cnl_ops = {
 	.resume			= hda_dsp_resume,
 	.runtime_suspend	= hda_dsp_runtime_suspend,
 	.runtime_resume		= hda_dsp_runtime_resume,
+	.set_hw_params_upon_resume = hda_dsp_set_hw_params_upon_resume,
 };
 EXPORT_SYMBOL(sof_cnl_ops);
 
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 830328af19c5..e1decf25aeac 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -131,12 +131,17 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream,
 	struct hdac_ext_stream *link_dev;
 	struct snd_soc_pcm_runtime *rtd = snd_pcm_substream_chip(substream);
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	struct sof_intel_hda_stream *hda_stream;
 	struct hda_pipe_params p_params = {0};
 	struct hdac_ext_link *link;
 	int stream_tag;
 
 	link_dev = snd_soc_dai_get_dma_data(dai, substream);
 
+	hda_stream = container_of(link_dev, struct sof_intel_hda_stream,
+				  hda_stream);
+	hda_stream->hw_params_upon_resume = 0;
+
 	link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name);
 	if (!link)
 		return -EINVAL;
@@ -168,22 +173,22 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream,
 static int hda_link_pcm_prepare(struct snd_pcm_substream *substream,
 				struct snd_soc_dai *dai)
 {
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct hdac_ext_stream *link_dev =
+				snd_soc_dai_get_dma_data(dai, substream);
+	struct sof_intel_hda_stream *hda_stream;
 	struct snd_sof_dev *sdev =
-		snd_soc_component_get_drvdata(dai->component);
-	struct snd_sof_pcm *spcm;
+				snd_soc_component_get_drvdata(dai->component);
+	struct snd_soc_pcm_runtime *rtd = snd_pcm_substream_chip(substream);
 	int stream = substream->stream;
 
-	spcm = snd_sof_find_spcm_dai(sdev, rtd);
-	if (!spcm)
-		return -EINVAL;
+	hda_stream = container_of(link_dev, struct sof_intel_hda_stream,
+				  hda_stream);
 
 	/* setup hw_params again only if resuming from system suspend */
-	if (!spcm->hw_params_upon_resume[stream])
+	if (!hda_stream->hw_params_upon_resume)
 		return 0;
 
-	dev_dbg(sdev->dev, "hda: prepare stream %d dir %d\n",
-		spcm->pcm.pcm_id, substream->stream);
+	dev_dbg(sdev->dev, "hda: prepare stream dir %d\n", substream->stream);
 
 	return hda_link_hw_params(substream, &rtd->dpcm[stream].hw_params,
 				  dai);
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 311fed502e09..5b73115a0b78 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -453,3 +453,19 @@ int hda_dsp_suspend(struct snd_sof_dev *sdev, int state)
 
 	return 0;
 }
+
+void hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev)
+{
+	struct hdac_bus *bus = sof_to_bus(sdev);
+	struct sof_intel_hda_stream *hda_stream;
+	struct hdac_ext_stream *stream;
+	struct hdac_stream *s;
+
+	/* set internal flag for BE */
+	list_for_each_entry(s, &bus->stream_list, list) {
+		stream = stream_to_hdac_ext_stream(s);
+		hda_stream = container_of(stream, struct sof_intel_hda_stream,
+					  hda_stream);
+		hda_stream->hw_params_upon_resume = 1;
+	}
+}
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 7cb2fb92c4eb..4b00e5d88565 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -409,6 +409,7 @@ static inline struct hda_bus *sof_to_hbus(struct snd_sof_dev *s)
 struct sof_intel_hda_stream {
 	struct hdac_ext_stream hda_stream;
 	struct sof_intel_stream stream;
+	int hw_params_upon_resume; /* set up hw_params upon resume */
 };
 
 #define bus_to_sof_hda(bus) \
@@ -443,6 +444,7 @@ int hda_dsp_suspend(struct snd_sof_dev *sdev, int state);
 int hda_dsp_resume(struct snd_sof_dev *sdev);
 int hda_dsp_runtime_suspend(struct snd_sof_dev *sdev, int state);
 int hda_dsp_runtime_resume(struct snd_sof_dev *sdev);
+void hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev);
 void hda_dsp_dump_skl(struct snd_sof_dev *sdev, u32 flags);
 void hda_dsp_dump(struct snd_sof_dev *sdev, u32 flags);
 
diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h
index 2a5d4c63f160..b8e2bf1fee24 100644
--- a/sound/soc/sof/ops.h
+++ b/sound/soc/sof/ops.h
@@ -134,6 +134,12 @@ static inline int snd_sof_dsp_runtime_suspend(struct snd_sof_dev *sdev,
 	return 0;
 }
 
+static inline void snd_sof_dsp_hw_params_upon_resume(struct snd_sof_dev *sdev)
+{
+	if (sof_ops(sdev)->set_hw_params_upon_resume)
+		sof_ops(sdev)->set_hw_params_upon_resume(sdev);
+}
+
 static inline int snd_sof_dsp_set_clk(struct snd_sof_dev *sdev, u32 freq)
 {
 	if (sof_ops(sdev)->set_clk)
diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c
index fc599e1b6f65..8ef1d51025d8 100644
--- a/sound/soc/sof/pm.c
+++ b/sound/soc/sof/pm.c
@@ -227,6 +227,9 @@ static void sof_set_hw_params_upon_resume(struct snd_sof_dev *sdev)
 				spcm->hw_params_upon_resume[dir] = 1;
 		}
 	}
+
+	/* set internal flag for BE */
+	snd_sof_dsp_hw_params_upon_resume(sdev);
 }
 
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_ENABLE_DEBUGFS_CACHE)
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index bbc285018f9a..170adc21ef17 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -166,6 +166,7 @@ struct snd_sof_dsp_ops {
 	int (*runtime_suspend)(struct snd_sof_dev *sof_dev,
 			       int state); /* optional */
 	int (*runtime_resume)(struct snd_sof_dev *sof_dev); /* optional */
+	void (*set_hw_params_upon_resume)(struct snd_sof_dev *sdev); /* optional */
 
 	/* DSP clocking */
 	int (*set_clk)(struct snd_sof_dev *sof_dev, u32 freq); /* optional */
-- 
2.17.1

[PATCH 17/19] ASoC: SOF: IPC: add ipc dump function

[Pierre-Louis Bossart]

From: Pan Xiuli <xiuli.pan@linux.intel.com>

Dump IPC status when IPC timed out. IPC status is platform specific and
need bind to plaform.

Signed-off-by: Pan Xiuli <xiuli.pan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/ipc.c      | 1 +
 sound/soc/sof/ops.h      | 6 ++++++
 sound/soc/sof/sof-priv.h | 1 +
 3 files changed, 8 insertions(+)

diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c
index 437e80a07bc2..ba1bb17a8d1e 100644
--- a/sound/soc/sof/ipc.c
+++ b/sound/soc/sof/ipc.c
@@ -207,6 +207,7 @@ static int tx_wait_done(struct snd_sof_ipc *ipc, struct snd_sof_ipc_msg *msg,
 		dev_err(sdev->dev, "error: ipc timed out for 0x%x size %d\n",
 			hdr->cmd, hdr->size);
 		snd_sof_dsp_dbg_dump(ipc->sdev, SOF_DBG_REGS | SOF_DBG_MBOX);
+		snd_sof_ipc_dump(ipc->sdev);
 		snd_sof_trace_notify_for_error(ipc->sdev);
 		ret = -ETIMEDOUT;
 	} else {
diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h
index b8e2bf1fee24..80fc3b374c2b 100644
--- a/sound/soc/sof/ops.h
+++ b/sound/soc/sof/ops.h
@@ -155,6 +155,12 @@ static inline void snd_sof_dsp_dbg_dump(struct snd_sof_dev *sdev, u32 flags)
 		return sof_ops(sdev)->dbg_dump(sdev, flags);
 }
 
+static inline void snd_sof_ipc_dump(struct snd_sof_dev *sdev)
+{
+	if (sof_ops(sdev)->ipc_dump)
+		return sof_ops(sdev)->ipc_dump(sdev);
+}
+
 /* register IO */
 static inline void snd_sof_dsp_write(struct snd_sof_dev *sdev, u32 bar,
 				     u32 offset, u32 value)
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index 170adc21ef17..1e85d6f9c5c3 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -176,6 +176,7 @@ struct snd_sof_dsp_ops {
 	int debug_map_count; /* optional */
 	void (*dbg_dump)(struct snd_sof_dev *sof_dev,
 			 u32 flags); /* optional */
+	void (*ipc_dump)(struct snd_sof_dev *sof_dev); /* optional */
 
 	/* host DMA trace initialization */
 	int (*trace_init)(struct snd_sof_dev *sdev,
-- 
2.17.1

[PATCH 18/19] ASoC: SOF: Intel: APL: add ipc dump function

[Pierre-Louis Bossart]

From: Pan Xiuli <xiuli.pan@linux.intel.com>

Add IPC dump function for APL plaform

Signed-off-by: Pan Xiuli <xiuli.pan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/apl.c |  1 +
 sound/soc/sof/intel/hda.c | 18 ++++++++++++++++++
 sound/soc/sof/intel/hda.h |  1 +
 3 files changed, 20 insertions(+)

diff --git a/sound/soc/sof/intel/apl.c b/sound/soc/sof/intel/apl.c
index d1fb2f2ba1e1..f215d80dce2c 100644
--- a/sound/soc/sof/intel/apl.c
+++ b/sound/soc/sof/intel/apl.c
@@ -55,6 +55,7 @@ const struct snd_sof_dsp_ops sof_apl_ops = {
 	.debug_map	= apl_dsp_debugfs,
 	.debug_map_count	= ARRAY_SIZE(apl_dsp_debugfs),
 	.dbg_dump	= hda_dsp_dump,
+	.ipc_dump	= hda_ipc_dump,
 
 	/* stream callbacks */
 	.pcm_open	= hda_dsp_pcm_open,
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index b8fc19790f3b..c7d419d340c8 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -179,6 +179,24 @@ void hda_dsp_dump(struct snd_sof_dev *sdev, u32 flags)
 	}
 }
 
+void hda_ipc_dump(struct snd_sof_dev *sdev)
+{
+	u32 hipcie;
+	u32 hipct;
+	u32 hipcctl;
+
+	/* read IPC status */
+	hipcie = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_REG_HIPCIE);
+	hipct = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_REG_HIPCT);
+	hipcctl = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_REG_HIPCCTL);
+
+	/* dump the IPC regs */
+	/* TODO: parse the raw msg */
+	dev_err(sdev->dev,
+		"error: host status 0x%8.8x dsp status 0x%8.8x mask 0x%8.8x\n",
+		hipcie, hipct, hipcctl);
+}
+
 static int hda_init(struct snd_sof_dev *sdev)
 {
 	struct hda_bus *hbus;
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 4b00e5d88565..92d45c43b4b1 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -447,6 +447,7 @@ int hda_dsp_runtime_resume(struct snd_sof_dev *sdev);
 void hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev);
 void hda_dsp_dump_skl(struct snd_sof_dev *sdev, u32 flags);
 void hda_dsp_dump(struct snd_sof_dev *sdev, u32 flags);
+void hda_ipc_dump(struct snd_sof_dev *sdev);
 
 /*
  * DSP PCM Operations.
-- 
2.17.1

[PATCH 19/19] ASoC: SOF: Intel: CNL: add ipc dump function

[Pierre-Louis Bossart]

From: Pan Xiuli <xiuli.pan@linux.intel.com>

Add ipc dump function to CNL+ platforms.

Signed-off-by: Pan Xiuli <xiuli.pan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/cnl.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c
index 2eac20bbcaea..08a1a3d3c08d 100644
--- a/sound/soc/sof/intel/cnl.c
+++ b/sound/soc/sof/intel/cnl.c
@@ -158,6 +158,24 @@ static int cnl_ipc_send_msg(struct snd_sof_dev *sdev,
 	return 0;
 }
 
+static void cnl_ipc_dump(struct snd_sof_dev *sdev)
+{
+	u32 hipcctl;
+	u32 hipcida;
+	u32 hipctdr;
+
+	/* read IPC status */
+	hipcida = snd_sof_dsp_read(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDA);
+	hipcctl = snd_sof_dsp_read(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCCTL);
+	hipctdr = snd_sof_dsp_read(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCTDR);
+
+	/* dump the IPC regs */
+	/* TODO: parse the raw msg */
+	dev_err(sdev->dev,
+		"error: host status 0x%8.8x dsp status 0x%8.8x mask 0x%8.8x\n",
+		hipcida, hipctdr, hipcctl);
+}
+
 /* cannonlake ops */
 const struct snd_sof_dsp_ops sof_cnl_ops = {
 	/* probe and remove */
@@ -189,6 +207,7 @@ const struct snd_sof_dsp_ops sof_cnl_ops = {
 	.debug_map	= cnl_dsp_debugfs,
 	.debug_map_count	= ARRAY_SIZE(cnl_dsp_debugfs),
 	.dbg_dump	= hda_dsp_dump,
+	.ipc_dump	= cnl_ipc_dump,
 
 	/* stream callbacks */
 	.pcm_open	= hda_dsp_pcm_open,
-- 
2.17.1

Re: [PATCH 00/19] ASoC: SOF: improvements for ABI checks and Intel code

[Pierre-Louis Bossart]

On 5/3/19 12:40 AM, Mark Brown wrote:
> On Tue, Apr 30, 2019 at 06:09:15PM -0500, Pierre-Louis Bossart wrote:
>> This series is a set of relatively small SOF changes after the big
>> batch merged last week (Thanks!). Since we are very close to the merge
>> window and in May where most of the world takes time off, it'd be
>> perfectly understandable if those patches were queued for 5.3, after
>> feedback and corrections as needed.
> 
> The headline thing I see when this pops up in my inbox is yet another 20
> patch series for these DSPs, I'm not going to notice the size of the
> patches on a first pass.  As I think you've noticed there's a lot of
> reviewer fatigue setting in with this stuff, one thing that'd really
> help here is if there were some help from Intel people with review for
> the DPCM code.

I can certainly understand reviewer fatigue, i've had to put a time 
limit on daily reviews to keep my sanity, but I don't get your last 
point. These patches were submitted and reviewed by Intel people on 
GitHub, what you see here is the result of multiple iterations precisely 
to make sure the patches are acceptable for upstream. we've set the goal 
of having two Intel aprovers for each patch. Can you elaborate on how we 
can make your life simpler?
FYI we are going to have a number of fixes coming in the next 2 months 
for HDaudio support, changes to IPC/power flows to support D0ix states 
and SoundWire, 3rd party firmware module support, etc, so I really want 
to make sure you are comfortable with SOF-related patches, their level 
of maturity and the submission rate.
Thanks!
-Pierre

Re: [PATCH 00/19] ASoC: SOF: improvements for ABI checks and Intel code

[Pierre-Louis Bossart]

On 5/5/19 10:51 PM, Mark Brown wrote:
> On Fri, May 03, 2019 at 09:47:39AM -0500, Pierre-Louis Bossart wrote:
>> On 5/3/19 12:40 AM, Mark Brown wrote:
> 
>>> reviewer fatigue setting in with this stuff, one thing that'd really
>>> help here is if there were some help from Intel people with review for
>>> the DPCM code.
> 
>> I can certainly understand reviewer fatigue, i've had to put a time limit on
>> daily reviews to keep my sanity, but I don't get your last point. These
>> patches were submitted and reviewed by Intel people on GitHub, what you see
>> here is the result of multiple iterations precisely to make sure the patches
>> are acceptable for upstream. we've set the goal of having two Intel aprovers
>> for each patch. Can you elaborate on how we can make your life simpler?
> 
> The last point there is that Intel only write patches, nobody from Intel
> is reviewing other people's patches especially in areas of the code like
> DPCM which are complex, fragile and where Intel is by far the most
> active user.

It's a valid point, and for now we indeed only check for non-regressions 
and provide point solutions without looking at the bigger picture. We 
have a couple of people ramping up (Ranjani, Libin, Guennadi, Jaska) and 
hopefully at some point we'll be able to review and improve.

Re: [PATCH 00/19] ASoC: SOF: improvements for ABI checks and Intel code

[Mark Brown]

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

On Mon, May 06, 2019 at 09:59:24AM -0500, Pierre-Louis Bossart wrote:
> On 5/5/19 10:51 PM, Mark Brown wrote:

> > The last point there is that Intel only write patches, nobody from Intel
> > is reviewing other people's patches especially in areas of the code like
> > DPCM which are complex, fragile and where Intel is by far the most
> > active user.

> It's a valid point, and for now we indeed only check for non-regressions and
> provide point solutions without looking at the bigger picture. We have a
> couple of people ramping up (Ranjani, Libin, Guennadi, Jaska) and hopefully
> at some point we'll be able to review and improve.

Even just testing (rather than reviewing, though obviously reviewing
would be good!) patches off the list would be very helpful and is close
to the regression testing stuff you're already doing.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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