[PATCH 0/8] SOF fixes and updates for FW boot

[PATCH 0/8] SOF fixes and updates for FW boot

[Ranjani Sridharan]

This series includes fixes and updates for the FW boot sequence for
Intel platforms.

Ranjani Sridharan (7):
  ALSA: hda: fix VS_LTRC register name
  ASoC: SOF: Intel: hda: Add helper function to program ICCMAX stream
  ASoC: SOF: Intel: hda: modify the signature of get_stream_with_tag()
  ASoC: SOF: Intel: hda: define macro for code loader stream format
  ASoC: SOF: Intel: hda: Define FW boot sequence with ICCMAX
  ASoC: SOF: Intel: hda: Add sof_tgl_ops for TGL platforms
  ASoC: SOF: Intel: hda: Simplify error handling during FW boot

Yong Zhi (1):
  ASoC: SOF: Intel: hda: Remove unused parameters in cl_dsp_init()

 include/sound/hda_register.h     |   2 +-
 sound/soc/sof/intel/Makefile     |   2 +-
 sound/soc/sof/intel/cnl.c        |  23 +----
 sound/soc/sof/intel/hda-ipc.h    |   4 +
 sound/soc/sof/intel/hda-loader.c | 145 ++++++++++++++++++-------------
 sound/soc/sof/intel/hda-stream.c |  69 +++++++++++++++
 sound/soc/sof/intel/hda.h        |   6 ++
 sound/soc/sof/intel/tgl.c        | 137 +++++++++++++++++++++++++++++
 sound/soc/sof/sof-pci-dev.c      |   2 +-
 9 files changed, 306 insertions(+), 84 deletions(-)
 create mode 100644 sound/soc/sof/intel/tgl.c

-- 
2.25.1

[PATCH 1/8] ALSA: hda: fix VS_LTRC register name

[Ranjani Sridharan]

It should be called VS_LTRP instead.

Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 include/sound/hda_register.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h
index 057d2a2d0bd0..4f987b1f32f7 100644
--- a/include/sound/hda_register.h
+++ b/include/sound/hda_register.h
@@ -119,7 +119,7 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
 #define AZX_REG_VS_EM3U			0x103C
 #define AZX_REG_VS_EM4L			0x1040
 #define AZX_REG_VS_EM4U			0x1044
-#define AZX_REG_VS_LTRC			0x1048
+#define AZX_REG_VS_LTRP			0x1048
 #define AZX_REG_VS_D0I3C		0x104A
 #define AZX_REG_VS_PCE			0x104B
 #define AZX_REG_VS_L2MAGC		0x1050
-- 
2.25.1

[PATCH 2/8] ASoC: SOF: Intel: hda: Remove unused parameters in cl_dsp_init()

[Ranjani Sridharan]

From: Yong Zhi <yong.zhi@intel.com>

cl_dsp_init() doesn't use the fwdata and fwsize parameters.
Remove it, and update caller accordingly.

Signed-off-by: Yong Zhi <yong.zhi@intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/sof/intel/hda-loader.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c
index 9cb219b87b3e..914699f550b1 100644
--- a/sound/soc/sof/intel/hda-loader.c
+++ b/sound/soc/sof/intel/hda-loader.c
@@ -78,8 +78,7 @@ static int cl_stream_prepare(struct snd_sof_dev *sdev, unsigned int format,
  * status on core 1, so power up core 1 also momentarily, keep it in
  * reset/stall and then turn it off
  */
-static int cl_dsp_init(struct snd_sof_dev *sdev, const void *fwdata,
-		       u32 fwsize, int stream_tag, int iteration)
+static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag, int iteration)
 {
 	struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
 	const struct sof_intel_dsp_desc *chip = hda->desc;
@@ -340,8 +339,7 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev)
 		dev_dbg(sdev->dev,
 			"Attempting iteration %d of Core En/ROM load...\n", i);
 
-		ret = cl_dsp_init(sdev, stripped_firmware.data,
-				  stripped_firmware.size, tag, i + 1);
+		ret = cl_dsp_init(sdev, tag, i + 1);
 
 		/* don't retry anymore if successful */
 		if (!ret)
-- 
2.25.1

[PATCH 3/8] ASoC: SOF: Intel: hda: Add helper function to program ICCMAX stream

[Ranjani Sridharan]

For some platforms, the recommended HW sequence for FW boot involves
starting a specially crafted capture stream before powering
on the DSP cores. Add a helper function to define the minimal
recommended stream programming sequence for this stream.

Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/sof/intel/hda-stream.c | 69 ++++++++++++++++++++++++++++++++
 sound/soc/sof/intel/hda.h        |  4 ++
 2 files changed, 73 insertions(+)

diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c
index 1bda14c3590c..0e09ede922c7 100644
--- a/sound/soc/sof/intel/hda-stream.c
+++ b/sound/soc/sof/intel/hda-stream.c
@@ -23,6 +23,8 @@
 #include "../sof-audio.h"
 #include "hda.h"
 
+#define HDA_LTRP_GB_VALUE_US	95
+
 /*
  * set up one of BDL entries for a stream
  */
@@ -322,6 +324,73 @@ int hda_dsp_stream_trigger(struct snd_sof_dev *sdev,
 	return 0;
 }
 
+/* minimal recommended programming for ICCMAX stream */
+int hda_dsp_iccmax_stream_hw_params(struct snd_sof_dev *sdev, struct hdac_ext_stream *stream,
+				    struct snd_dma_buffer *dmab,
+				    struct snd_pcm_hw_params *params)
+{
+	struct hdac_bus *bus = sof_to_bus(sdev);
+	struct hdac_stream *hstream = &stream->hstream;
+	int sd_offset = SOF_STREAM_SD_OFFSET(hstream);
+	int ret;
+	u32 mask = 0x1 << hstream->index;
+
+	if (!stream) {
+		dev_err(sdev->dev, "error: no stream available\n");
+		return -ENODEV;
+	}
+
+	if (hstream->posbuf)
+		*hstream->posbuf = 0;
+
+	/* reset BDL address */
+	snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR,
+			  sd_offset + SOF_HDA_ADSP_REG_CL_SD_BDLPL,
+			  0x0);
+	snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR,
+			  sd_offset + SOF_HDA_ADSP_REG_CL_SD_BDLPU,
+			  0x0);
+
+	hstream->frags = 0;
+
+	ret = hda_dsp_stream_setup_bdl(sdev, dmab, hstream);
+	if (ret < 0) {
+		dev_err(sdev->dev, "error: set up of BDL failed\n");
+		return ret;
+	}
+
+	/* program BDL address */
+	snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR,
+			  sd_offset + SOF_HDA_ADSP_REG_CL_SD_BDLPL,
+			  (u32)hstream->bdl.addr);
+	snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR,
+			  sd_offset + SOF_HDA_ADSP_REG_CL_SD_BDLPU,
+			  upper_32_bits(hstream->bdl.addr));
+
+	/* program cyclic buffer length */
+	snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR,
+			  sd_offset + SOF_HDA_ADSP_REG_CL_SD_CBL,
+			  hstream->bufsize);
+
+	/* program last valid index */
+	snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
+				sd_offset + SOF_HDA_ADSP_REG_CL_SD_LVI,
+				0xffff, (hstream->frags - 1));
+
+	/* decouple host and link DMA, enable DSP features */
+	snd_sof_dsp_update_bits(sdev, HDA_DSP_PP_BAR, SOF_HDA_REG_PP_PPCTL,
+				mask, mask);
+
+	/* Follow HW recommendation to set the guardband value to 95us during FW boot */
+	snd_hdac_chip_updateb(bus, VS_LTRP, HDA_VS_INTEL_LTRP_GB_MASK, HDA_LTRP_GB_VALUE_US);
+
+	/* start DMA */
+	snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, sd_offset,
+				SOF_HDA_SD_CTL_DMA_START, SOF_HDA_SD_CTL_DMA_START);
+
+	return 0;
+}
+
 /*
  * prepare for common hdac registers settings, for both code loader
  * and normal stream.
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index fe452f0d0ec7..100eaaeecb4d 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -246,6 +246,7 @@
 /* Intel Vendor Specific Registers */
 #define HDA_VS_INTEL_EM2		0x1030
 #define HDA_VS_INTEL_EM2_L1SEN		BIT(13)
+#define HDA_VS_INTEL_LTRP_GB_MASK	0x3F
 
 /*  HIPCI */
 #define HDA_DSP_REG_HIPCI_BUSY		BIT(31)
@@ -546,6 +547,9 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev,
 			     struct hdac_ext_stream *stream,
 			     struct snd_dma_buffer *dmab,
 			     struct snd_pcm_hw_params *params);
+int hda_dsp_iccmax_stream_hw_params(struct snd_sof_dev *sdev, struct hdac_ext_stream *stream,
+				    struct snd_dma_buffer *dmab,
+				    struct snd_pcm_hw_params *params);
 int hda_dsp_stream_trigger(struct snd_sof_dev *sdev,
 			   struct hdac_ext_stream *stream, int cmd);
 irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context);
-- 
2.25.1

[PATCH 4/8] ASoC: SOF: Intel: hda: modify the signature of get_stream_with_tag()

[Ranjani Sridharan]

Modify the signature of get_stream_with_tag() to add the direction
as an argument to extend it for using with capture streams.

Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/sof/intel/hda-loader.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c
index 914699f550b1..804f5f64aa33 100644
--- a/sound/soc/sof/intel/hda-loader.c
+++ b/sound/soc/sof/intel/hda-loader.c
@@ -205,18 +205,15 @@ static int cl_trigger(struct snd_sof_dev *sdev,
 }
 
 static struct hdac_ext_stream *get_stream_with_tag(struct snd_sof_dev *sdev,
-						   int tag)
+						   int tag, int direction)
 {
 	struct hdac_bus *bus = sof_to_bus(sdev);
 	struct hdac_stream *s;
 
 	/* get stream with tag */
-	list_for_each_entry(s, &bus->stream_list, list) {
-		if (s->direction == SNDRV_PCM_STREAM_PLAYBACK &&
-		    s->stream_tag == tag) {
+	list_for_each_entry(s, &bus->stream_list, list)
+		if (s->direction == direction && s->stream_tag == tag)
 			return stream_to_hdac_ext_stream(s);
-		}
-	}
 
 	return NULL;
 }
@@ -322,7 +319,7 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev)
 	}
 
 	/* get stream with tag */
-	stream = get_stream_with_tag(sdev, tag);
+	stream = get_stream_with_tag(sdev, tag, SNDRV_PCM_STREAM_PLAYBACK);
 	if (!stream) {
 		dev_err(sdev->dev,
 			"error: could not get stream with stream tag %d\n",
-- 
2.25.1

[PATCH 5/8] ASoC: SOF: Intel: hda: define macro for code loader stream format

[Ranjani Sridharan]

This will be used for the ICCMAX stream as well.

Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/sof/intel/hda-loader.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c
index 804f5f64aa33..9b4844aa3023 100644
--- a/sound/soc/sof/intel/hda-loader.c
+++ b/sound/soc/sof/intel/hda-loader.c
@@ -22,6 +22,7 @@
 #include "hda.h"
 
 #define HDA_FW_BOOT_ATTEMPTS	3
+#define HDA_CL_STREAM_FORMAT 0x40
 
 static int cl_stream_prepare(struct snd_sof_dev *sdev, unsigned int format,
 			     unsigned int size, struct snd_dma_buffer *dmab,
@@ -309,7 +310,7 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev)
 	init_waitqueue_head(&sdev->boot_wait);
 
 	/* prepare DMA for code loader stream */
-	tag = cl_stream_prepare(sdev, 0x40, stripped_firmware.size,
+	tag = cl_stream_prepare(sdev, HDA_CL_STREAM_FORMAT, stripped_firmware.size,
 				&sdev->dmab, SNDRV_PCM_STREAM_PLAYBACK);
 
 	if (tag < 0) {
-- 
2.25.1

[PATCH 6/8] ASoC: SOF: Intel: hda: Define FW boot sequence with ICCMAX

[Ranjani Sridharan]

Define the FW boot sequence for platforms that are recommended
to use ICCMAX. This function uses the existing prepare and cleanup
functions for creating a specially crafted capture stream before
powering up the DSP cores.

Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/sof/intel/hda-loader.c | 92 ++++++++++++++++++++++++++------
 sound/soc/sof/intel/hda.h        |  1 +
 2 files changed, 78 insertions(+), 15 deletions(-)

diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c
index 9b4844aa3023..5464f899e16f 100644
--- a/sound/soc/sof/intel/hda-loader.c
+++ b/sound/soc/sof/intel/hda-loader.c
@@ -17,6 +17,7 @@
 
 #include <linux/firmware.h>
 #include <sound/hdaudio_ext.h>
+#include <sound/hda_register.h>
 #include <sound/sof.h>
 #include "../ops.h"
 #include "hda.h"
@@ -33,11 +34,6 @@ static int cl_stream_prepare(struct snd_sof_dev *sdev, unsigned int format,
 	struct pci_dev *pci = to_pci_dev(sdev->dev);
 	int ret;
 
-	if (direction != SNDRV_PCM_STREAM_PLAYBACK) {
-		dev_err(sdev->dev, "error: code loading DMA is playback only\n");
-		return -EINVAL;
-	}
-
 	dsp_stream = hda_dsp_stream_get(sdev, direction);
 
 	if (!dsp_stream) {
@@ -58,14 +54,21 @@ static int cl_stream_prepare(struct snd_sof_dev *sdev, unsigned int format,
 	hstream->format_val = format;
 	hstream->bufsize = size;
 
-	ret = hda_dsp_stream_hw_params(sdev, dsp_stream, dmab, NULL);
-	if (ret < 0) {
-		dev_err(sdev->dev, "error: hdac prepare failed: %x\n", ret);
-		goto error;
+	if (direction == SNDRV_PCM_STREAM_CAPTURE) {
+		ret = hda_dsp_iccmax_stream_hw_params(sdev, dsp_stream, dmab, NULL);
+		if (ret < 0) {
+			dev_err(sdev->dev, "error: iccmax stream prepare failed: %x\n", ret);
+			goto error;
+		}
+	} else {
+		ret = hda_dsp_stream_hw_params(sdev, dsp_stream, dmab, NULL);
+		if (ret < 0) {
+			dev_err(sdev->dev, "error: hdac prepare failed: %x\n", ret);
+			goto error;
+		}
+		hda_dsp_stream_spib_config(sdev, dsp_stream, HDA_DSP_SPIB_ENABLE, size);
 	}
 
-	hda_dsp_stream_spib_config(sdev, dsp_stream, HDA_DSP_SPIB_ENABLE, size);
-
 	return hstream->stream_tag;
 
 error:
@@ -224,12 +227,15 @@ static int cl_cleanup(struct snd_sof_dev *sdev, struct snd_dma_buffer *dmab,
 {
 	struct hdac_stream *hstream = &stream->hstream;
 	int sd_offset = SOF_STREAM_SD_OFFSET(hstream);
-	int ret;
+	int ret = 0;
 
-	ret = hda_dsp_stream_spib_config(sdev, stream, HDA_DSP_SPIB_DISABLE, 0);
+	if (hstream->direction == SNDRV_PCM_STREAM_PLAYBACK)
+		ret = hda_dsp_stream_spib_config(sdev, stream, HDA_DSP_SPIB_DISABLE, 0);
+	else
+		snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, sd_offset,
+					SOF_HDA_SD_CTL_DMA_START, 0);
 
-	hda_dsp_stream_put(sdev, SNDRV_PCM_STREAM_PLAYBACK,
-			   hstream->stream_tag);
+	hda_dsp_stream_put(sdev, hstream->direction, hstream->stream_tag);
 	hstream->running = 0;
 	hstream->substream = NULL;
 
@@ -287,6 +293,62 @@ static int cl_copy_fw(struct snd_sof_dev *sdev, struct hdac_ext_stream *stream)
 	return status;
 }
 
+int hda_dsp_cl_boot_firmware_iccmax(struct snd_sof_dev *sdev)
+{
+	struct snd_sof_pdata *plat_data = sdev->pdata;
+	struct hdac_ext_stream *iccmax_stream;
+	struct hdac_bus *bus = sof_to_bus(sdev);
+	struct firmware stripped_firmware;
+	int ret, ret1;
+	int iccmax_tag;
+	u8 original_gb;
+
+	/* save the original LTRP guardband value */
+	original_gb = snd_hdac_chip_readb(bus, VS_LTRP) & HDA_VS_INTEL_LTRP_GB_MASK;
+
+	if (plat_data->fw->size <= plat_data->fw_offset) {
+		dev_err(sdev->dev, "error: firmware size must be greater than firmware offset\n");
+		return -EINVAL;
+	}
+
+	stripped_firmware.size = plat_data->fw->size - plat_data->fw_offset;
+
+	/* prepare capture stream for ICCMAX */
+	iccmax_tag = cl_stream_prepare(sdev, HDA_CL_STREAM_FORMAT, stripped_firmware.size,
+				       &sdev->dmab_bdl, SNDRV_PCM_STREAM_CAPTURE);
+	if (iccmax_tag < 0) {
+		dev_err(sdev->dev, "error: dma prepare for ICCMAX %x\n", iccmax_tag);
+		return iccmax_tag;
+	}
+
+	/* get stream with tag */
+	iccmax_stream = get_stream_with_tag(sdev, iccmax_tag, SNDRV_PCM_STREAM_CAPTURE);
+	if (!iccmax_stream) {
+		dev_err(sdev->dev, "error: could not get stream with stream tag %d\n", iccmax_tag);
+		ret = -ENODEV;
+	} else {
+		ret = hda_dsp_cl_boot_firmware(sdev);
+	}
+
+	/*
+	 * Perform iccmax stream cleanup. This should be done even if firmware loading fails.
+	 * If the cleanup also fails, we return the initial error
+	 */
+	ret1 = cl_cleanup(sdev, &sdev->dmab_bdl, iccmax_stream);
+	if (ret1 < 0) {
+		dev_err(sdev->dev, "error: ICCMAX stream cleanup failed\n");
+
+		/* set return value to indicate cleanup failure */
+		if (!ret)
+			ret = ret1;
+	}
+
+	/* restore the original guardband value after FW boot */
+	snd_hdac_chip_updateb(bus, VS_LTRP, HDA_VS_INTEL_LTRP_GB_MASK, original_gb);
+
+	return ret;
+}
+
 int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev)
 {
 	struct snd_sof_pdata *plat_data = sdev->pdata;
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 100eaaeecb4d..a2e6050f0ef0 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -612,6 +612,7 @@ int hda_dsp_ipc_cmd_done(struct snd_sof_dev *sdev, int dir);
  * DSP Code loader.
  */
 int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev);
+int hda_dsp_cl_boot_firmware_iccmax(struct snd_sof_dev *sdev);
 int hda_dsp_cl_boot_firmware_skl(struct snd_sof_dev *sdev);
 
 /* pre and post fw run ops */
-- 
2.25.1

[PATCH 7/8] ASoC: SOF: Intel: hda: Add sof_tgl_ops for TGL platforms

[Ranjani Sridharan]

Separate the dsp ops for TGL ops to specify the use of ICCMAX
FW boot sequence in the run op. All other ops are identical.
Also separate the TGL descriptors into a separate file to make
it easier to follow.

Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/sof/intel/Makefile  |   2 +-
 sound/soc/sof/intel/cnl.c     |  23 +-----
 sound/soc/sof/intel/hda-ipc.h |   4 +
 sound/soc/sof/intel/hda.h     |   1 +
 sound/soc/sof/intel/tgl.c     | 137 ++++++++++++++++++++++++++++++++++
 sound/soc/sof/sof-pci-dev.c   |   2 +-
 6 files changed, 147 insertions(+), 22 deletions(-)
 create mode 100644 sound/soc/sof/intel/tgl.c

diff --git a/sound/soc/sof/intel/Makefile b/sound/soc/sof/intel/Makefile
index f7e9358f1f06..72d85b25df7d 100644
--- a/sound/soc/sof/intel/Makefile
+++ b/sound/soc/sof/intel/Makefile
@@ -8,7 +8,7 @@ snd-sof-intel-ipc-objs := intel-ipc.o
 snd-sof-intel-hda-common-objs := hda.o hda-loader.o hda-stream.o hda-trace.o \
 				 hda-dsp.o hda-ipc.o hda-ctrl.o hda-pcm.o \
 				 hda-dai.o hda-bus.o \
-				 apl.o cnl.o
+				 apl.o cnl.o tgl.o
 snd-sof-intel-hda-common-$(CONFIG_SND_SOC_SOF_HDA_PROBES) += hda-compress.o
 
 snd-sof-intel-hda-objs := hda-codec.o
diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c
index 95234ae59e42..70f14b2aa954 100644
--- a/sound/soc/sof/intel/cnl.c
+++ b/sound/soc/sof/intel/cnl.c
@@ -29,7 +29,7 @@ static const struct snd_sof_debugfs_map cnl_dsp_debugfs[] = {
 static void cnl_ipc_host_done(struct snd_sof_dev *sdev);
 static void cnl_ipc_dsp_done(struct snd_sof_dev *sdev);
 
-static irqreturn_t cnl_ipc_irq_thread(int irq, void *context)
+irqreturn_t cnl_ipc_irq_thread(int irq, void *context)
 {
 	struct snd_sof_dev *sdev = context;
 	u32 hipci;
@@ -163,8 +163,7 @@ static bool cnl_compact_ipc_compress(struct snd_sof_ipc_msg *msg,
 	return false;
 }
 
-static int cnl_ipc_send_msg(struct snd_sof_dev *sdev,
-			    struct snd_sof_ipc_msg *msg)
+int cnl_ipc_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
 {
 	struct sof_intel_hda_dev *hdev = sdev->pdata->hw_pdata;
 	struct sof_ipc_cmd_hdr *hdr;
@@ -211,7 +210,7 @@ static int cnl_ipc_send_msg(struct snd_sof_dev *sdev,
 	return 0;
 }
 
-static void cnl_ipc_dump(struct snd_sof_dev *sdev)
+void cnl_ipc_dump(struct snd_sof_dev *sdev)
 {
 	u32 hipcctl;
 	u32 hipcida;
@@ -369,22 +368,6 @@ const struct sof_intel_dsp_desc icl_chip_info = {
 };
 EXPORT_SYMBOL_NS(icl_chip_info, SND_SOC_SOF_INTEL_HDA_COMMON);
 
-const struct sof_intel_dsp_desc tgl_chip_info = {
-	/* Tigerlake */
-	.cores_num = 4,
-	.init_core_mask = 1,
-	.cores_mask = HDA_DSP_CORE_MASK(0),
-	.ipc_req = CNL_DSP_REG_HIPCIDR,
-	.ipc_req_mask = CNL_DSP_REG_HIPCIDR_BUSY,
-	.ipc_ack = CNL_DSP_REG_HIPCIDA,
-	.ipc_ack_mask = CNL_DSP_REG_HIPCIDA_DONE,
-	.ipc_ctl = CNL_DSP_REG_HIPCCTL,
-	.rom_init_timeout	= 300,
-	.ssp_count = ICL_SSP_COUNT,
-	.ssp_base_offset = CNL_SSP_BASE_OFFSET,
-};
-EXPORT_SYMBOL_NS(tgl_chip_info, SND_SOC_SOF_INTEL_HDA_COMMON);
-
 const struct sof_intel_dsp_desc ehl_chip_info = {
 	/* Elkhartlake */
 	.cores_num = 4,
diff --git a/sound/soc/sof/intel/hda-ipc.h b/sound/soc/sof/intel/hda-ipc.h
index ade4c3191a39..10fbca5939db 100644
--- a/sound/soc/sof/intel/hda-ipc.h
+++ b/sound/soc/sof/intel/hda-ipc.h
@@ -48,4 +48,8 @@
 #define HDA_PM_PG_STREAMING	BIT(1)
 #define HDA_PM_PG_RSVD		BIT(0)
 
+irqreturn_t cnl_ipc_irq_thread(int irq, void *context);
+int cnl_ipc_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg);
+void cnl_ipc_dump(struct snd_sof_dev *sdev);
+
 #endif
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index a2e6050f0ef0..5ee2f8354051 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -733,6 +733,7 @@ extern struct snd_soc_dai_driver skl_dai[];
  */
 extern const struct snd_sof_dsp_ops sof_apl_ops;
 extern const struct snd_sof_dsp_ops sof_cnl_ops;
+extern const struct snd_sof_dsp_ops sof_tgl_ops;
 
 extern const struct sof_intel_dsp_desc apl_chip_info;
 extern const struct sof_intel_dsp_desc cnl_chip_info;
diff --git a/sound/soc/sof/intel/tgl.c b/sound/soc/sof/intel/tgl.c
new file mode 100644
index 000000000000..d0e84b7747a0
--- /dev/null
+++ b/sound/soc/sof/intel/tgl.c
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+//
+// Copyright(c) 2020 Intel Corporation. All rights reserved.
+//
+// Authors: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
+//
+
+/*
+ * Hardware interface for audio DSP on Tigerlake.
+ */
+
+#include "../ops.h"
+#include "hda.h"
+#include "hda-ipc.h"
+#include "../sof-audio.h"
+
+static const struct snd_sof_debugfs_map tgl_dsp_debugfs[] = {
+	{"hda", HDA_DSP_HDA_BAR, 0, 0x4000, SOF_DEBUGFS_ACCESS_ALWAYS},
+	{"pp", HDA_DSP_PP_BAR,  0, 0x1000, SOF_DEBUGFS_ACCESS_ALWAYS},
+	{"dsp", HDA_DSP_BAR,  0, 0x10000, SOF_DEBUGFS_ACCESS_ALWAYS},
+};
+
+/* Tigerlake ops */
+const struct snd_sof_dsp_ops sof_tgl_ops = {
+	/* probe and remove */
+	.probe		= hda_dsp_probe,
+	.remove		= hda_dsp_remove,
+
+	/* Register IO */
+	.write		= sof_io_write,
+	.read		= sof_io_read,
+	.write64	= sof_io_write64,
+	.read64		= sof_io_read64,
+
+	/* Block IO */
+	.block_read	= sof_block_read,
+	.block_write	= sof_block_write,
+
+	/* doorbell */
+	.irq_thread	= cnl_ipc_irq_thread,
+
+	/* ipc */
+	.send_msg	= cnl_ipc_send_msg,
+	.fw_ready	= sof_fw_ready,
+	.get_mailbox_offset = hda_dsp_ipc_get_mailbox_offset,
+	.get_window_offset = hda_dsp_ipc_get_window_offset,
+
+	.ipc_msg_data	= hda_ipc_msg_data,
+	.ipc_pcm_params	= hda_ipc_pcm_params,
+
+	/* machine driver */
+	.machine_select = hda_machine_select,
+	.machine_register = sof_machine_register,
+	.machine_unregister = sof_machine_unregister,
+	.set_mach_params = hda_set_mach_params,
+
+	/* debug */
+	.debug_map	= tgl_dsp_debugfs,
+	.debug_map_count	= ARRAY_SIZE(tgl_dsp_debugfs),
+	.dbg_dump	= hda_dsp_dump,
+	.ipc_dump	= cnl_ipc_dump,
+
+	/* stream callbacks */
+	.pcm_open	= hda_dsp_pcm_open,
+	.pcm_close	= hda_dsp_pcm_close,
+	.pcm_hw_params	= hda_dsp_pcm_hw_params,
+	.pcm_hw_free	= hda_dsp_stream_hw_free,
+	.pcm_trigger	= hda_dsp_pcm_trigger,
+	.pcm_pointer	= hda_dsp_pcm_pointer,
+
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES)
+	/* probe callbacks */
+	.probe_assign	= hda_probe_compr_assign,
+	.probe_free	= hda_probe_compr_free,
+	.probe_set_params	= hda_probe_compr_set_params,
+	.probe_trigger	= hda_probe_compr_trigger,
+	.probe_pointer	= hda_probe_compr_pointer,
+#endif
+
+	/* firmware loading */
+	.load_firmware = snd_sof_load_firmware_raw,
+
+	/* pre/post fw run */
+	.pre_fw_run = hda_dsp_pre_fw_run,
+	.post_fw_run = hda_dsp_post_fw_run,
+
+	/* dsp core power up/down */
+	.core_power_up = hda_dsp_enable_core,
+	.core_power_down = hda_dsp_core_reset_power_down,
+
+	/* firmware run */
+	.run = hda_dsp_cl_boot_firmware_iccmax,
+
+	/* trace callback */
+	.trace_init = hda_dsp_trace_init,
+	.trace_release = hda_dsp_trace_release,
+	.trace_trigger = hda_dsp_trace_trigger,
+
+	/* DAI drivers */
+	.drv		= skl_dai,
+	.num_drv	= SOF_SKL_NUM_DAIS,
+
+	/* PM */
+	.suspend		= hda_dsp_suspend,
+	.resume			= hda_dsp_resume,
+	.runtime_suspend	= hda_dsp_runtime_suspend,
+	.runtime_resume		= hda_dsp_runtime_resume,
+	.runtime_idle		= hda_dsp_runtime_idle,
+	.set_hw_params_upon_resume = hda_dsp_set_hw_params_upon_resume,
+	.set_power_state	= hda_dsp_set_power_state,
+
+	/* ALSA HW info flags */
+	.hw_info =	SNDRV_PCM_INFO_MMAP |
+			SNDRV_PCM_INFO_MMAP_VALID |
+			SNDRV_PCM_INFO_INTERLEAVED |
+			SNDRV_PCM_INFO_PAUSE |
+			SNDRV_PCM_INFO_NO_PERIOD_WAKEUP,
+
+	.arch_ops = &sof_xtensa_arch_ops,
+};
+EXPORT_SYMBOL_NS(sof_tgl_ops, SND_SOC_SOF_INTEL_HDA_COMMON);
+
+const struct sof_intel_dsp_desc tgl_chip_info = {
+	/* Tigerlake */
+	.cores_num = 4,
+	.init_core_mask = 1,
+	.cores_mask = HDA_DSP_CORE_MASK(0),
+	.ipc_req = CNL_DSP_REG_HIPCIDR,
+	.ipc_req_mask = CNL_DSP_REG_HIPCIDR_BUSY,
+	.ipc_ack = CNL_DSP_REG_HIPCIDA,
+	.ipc_ack_mask = CNL_DSP_REG_HIPCIDA_DONE,
+	.ipc_ctl = CNL_DSP_REG_HIPCCTL,
+	.rom_init_timeout	= 300,
+	.ssp_count = ICL_SSP_COUNT,
+	.ssp_base_offset = CNL_SSP_BASE_OFFSET,
+};
+EXPORT_SYMBOL_NS(tgl_chip_info, SND_SOC_SOF_INTEL_HDA_COMMON);
diff --git a/sound/soc/sof/sof-pci-dev.c b/sound/soc/sof/sof-pci-dev.c
index 7b85c88cba9c..ec62d9337d68 100644
--- a/sound/soc/sof/sof-pci-dev.c
+++ b/sound/soc/sof/sof-pci-dev.c
@@ -221,7 +221,7 @@ static const struct sof_dev_desc tgl_desc = {
 	.default_tplg_path = "intel/sof-tplg",
 	.default_fw_filename = "sof-tgl.ri",
 	.nocodec_tplg_filename = "sof-tgl-nocodec.tplg",
-	.ops = &sof_cnl_ops,
+	.ops = &sof_tgl_ops,
 };
 #endif
 
-- 
2.25.1

[PATCH 8/8] ASoC: SOF: Intel: hda: Simplify error handling during FW boot

[Ranjani Sridharan]

Modify cl_stream_prepare() to return a pointer to the prepared stream
if successful or ERR_PTR() otherwise. This would simplify the error
paths in hda_dsp_cl_boot_firmware() and hda_dsp_cl_boot_firmware_iccmax()
to perform the stream cleanup after FW boot. This change also renders
the function get_stream_with_tag() redundant.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/sof/intel/hda-loader.c | 73 +++++++++-----------------------
 1 file changed, 19 insertions(+), 54 deletions(-)

diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c
index 5464f899e16f..70727495fdbf 100644
--- a/sound/soc/sof/intel/hda-loader.c
+++ b/sound/soc/sof/intel/hda-loader.c
@@ -25,9 +25,9 @@
 #define HDA_FW_BOOT_ATTEMPTS	3
 #define HDA_CL_STREAM_FORMAT 0x40
 
-static int cl_stream_prepare(struct snd_sof_dev *sdev, unsigned int format,
-			     unsigned int size, struct snd_dma_buffer *dmab,
-			     int direction)
+static struct hdac_ext_stream *cl_stream_prepare(struct snd_sof_dev *sdev, unsigned int format,
+						 unsigned int size, struct snd_dma_buffer *dmab,
+						 int direction)
 {
 	struct hdac_ext_stream *dsp_stream;
 	struct hdac_stream *hstream;
@@ -38,7 +38,7 @@ static int cl_stream_prepare(struct snd_sof_dev *sdev, unsigned int format,
 
 	if (!dsp_stream) {
 		dev_err(sdev->dev, "error: no stream available\n");
-		return -ENODEV;
+		return ERR_PTR(-ENODEV);
 	}
 	hstream = &dsp_stream->hstream;
 	hstream->substream = NULL;
@@ -69,12 +69,12 @@ static int cl_stream_prepare(struct snd_sof_dev *sdev, unsigned int format,
 		hda_dsp_stream_spib_config(sdev, dsp_stream, HDA_DSP_SPIB_ENABLE, size);
 	}
 
-	return hstream->stream_tag;
+	return dsp_stream;
 
 error:
 	hda_dsp_stream_put(sdev, direction, hstream->stream_tag);
 	snd_dma_free_pages(dmab);
-	return ret;
+	return ERR_PTR(ret);
 }
 
 /*
@@ -208,20 +208,6 @@ static int cl_trigger(struct snd_sof_dev *sdev,
 	}
 }
 
-static struct hdac_ext_stream *get_stream_with_tag(struct snd_sof_dev *sdev,
-						   int tag, int direction)
-{
-	struct hdac_bus *bus = sof_to_bus(sdev);
-	struct hdac_stream *s;
-
-	/* get stream with tag */
-	list_for_each_entry(s, &bus->stream_list, list)
-		if (s->direction == direction && s->stream_tag == tag)
-			return stream_to_hdac_ext_stream(s);
-
-	return NULL;
-}
-
 static int cl_cleanup(struct snd_sof_dev *sdev, struct snd_dma_buffer *dmab,
 		      struct hdac_ext_stream *stream)
 {
@@ -300,7 +286,6 @@ int hda_dsp_cl_boot_firmware_iccmax(struct snd_sof_dev *sdev)
 	struct hdac_bus *bus = sof_to_bus(sdev);
 	struct firmware stripped_firmware;
 	int ret, ret1;
-	int iccmax_tag;
 	u8 original_gb;
 
 	/* save the original LTRP guardband value */
@@ -314,21 +299,14 @@ int hda_dsp_cl_boot_firmware_iccmax(struct snd_sof_dev *sdev)
 	stripped_firmware.size = plat_data->fw->size - plat_data->fw_offset;
 
 	/* prepare capture stream for ICCMAX */
-	iccmax_tag = cl_stream_prepare(sdev, HDA_CL_STREAM_FORMAT, stripped_firmware.size,
-				       &sdev->dmab_bdl, SNDRV_PCM_STREAM_CAPTURE);
-	if (iccmax_tag < 0) {
-		dev_err(sdev->dev, "error: dma prepare for ICCMAX %x\n", iccmax_tag);
-		return iccmax_tag;
+	iccmax_stream = cl_stream_prepare(sdev, HDA_CL_STREAM_FORMAT, stripped_firmware.size,
+					  &sdev->dmab_bdl, SNDRV_PCM_STREAM_CAPTURE);
+	if (IS_ERR(iccmax_stream)) {
+		dev_err(sdev->dev, "error: dma prepare for ICCMAX stream failed\n");
+		return PTR_ERR(iccmax_stream);
 	}
 
-	/* get stream with tag */
-	iccmax_stream = get_stream_with_tag(sdev, iccmax_tag, SNDRV_PCM_STREAM_CAPTURE);
-	if (!iccmax_stream) {
-		dev_err(sdev->dev, "error: could not get stream with stream tag %d\n", iccmax_tag);
-		ret = -ENODEV;
-	} else {
-		ret = hda_dsp_cl_boot_firmware(sdev);
-	}
+	ret = hda_dsp_cl_boot_firmware(sdev);
 
 	/*
 	 * Perform iccmax stream cleanup. This should be done even if firmware loading fails.
@@ -356,7 +334,7 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev)
 	const struct sof_intel_dsp_desc *chip_info;
 	struct hdac_ext_stream *stream;
 	struct firmware stripped_firmware;
-	int ret, ret1, tag, i;
+	int ret, ret1, i;
 
 	chip_info = desc->chip_info;
 
@@ -372,23 +350,11 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev)
 	init_waitqueue_head(&sdev->boot_wait);
 
 	/* prepare DMA for code loader stream */
-	tag = cl_stream_prepare(sdev, HDA_CL_STREAM_FORMAT, stripped_firmware.size,
-				&sdev->dmab, SNDRV_PCM_STREAM_PLAYBACK);
-
-	if (tag < 0) {
-		dev_err(sdev->dev, "error: dma prepare for fw loading err: %x\n",
-			tag);
-		return tag;
-	}
-
-	/* get stream with tag */
-	stream = get_stream_with_tag(sdev, tag, SNDRV_PCM_STREAM_PLAYBACK);
-	if (!stream) {
-		dev_err(sdev->dev,
-			"error: could not get stream with stream tag %d\n",
-			tag);
-		ret = -ENODEV;
-		goto err;
+	stream = cl_stream_prepare(sdev, HDA_CL_STREAM_FORMAT, stripped_firmware.size,
+				   &sdev->dmab, SNDRV_PCM_STREAM_PLAYBACK);
+	if (IS_ERR(stream)) {
+		dev_err(sdev->dev, "error: dma prepare for fw loading failed\n");
+		return PTR_ERR(stream);
 	}
 
 	memcpy(sdev->dmab.area, stripped_firmware.data,
@@ -399,7 +365,7 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev)
 		dev_dbg(sdev->dev,
 			"Attempting iteration %d of Core En/ROM load...\n", i);
 
-		ret = cl_dsp_init(sdev, tag, i + 1);
+		ret = cl_dsp_init(sdev, stream->hstream.stream_tag, i + 1);
 
 		/* don't retry anymore if successful */
 		if (!ret)
@@ -468,7 +434,6 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev)
 		return chip_info->init_core_mask;
 
 	/* dump dsp registers and disable DSP upon error */
-err:
 	hda_dsp_dump(sdev, SOF_DBG_REGS | SOF_DBG_PCI | SOF_DBG_MBOX);
 
 	/* disable DSP */
-- 
2.25.1

Re: [PATCH 1/8] ALSA: hda: fix VS_LTRC register name

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 159 bytes --]

On Wed, Aug 26, 2020 at 11:45:25AM -0700, Ranjani Sridharan wrote:
> It should be called VS_LTRP instead.

Takashi, should I just apply this to the ASoC tree?

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

Re: [PATCH 1/8] ALSA: hda: fix VS_LTRC register name

[Takashi Iwai]

On Wed, 26 Aug 2020 20:50:25 +0200,
Mark Brown wrote:
> 
> On Wed, Aug 26, 2020 at 11:45:25AM -0700, Ranjani Sridharan wrote:
> > It should be called VS_LTRP instead.
> 
> Takashi, should I just apply this to the ASoC tree?

Yes, it's just a typo fix, feel free to take it.

Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

Re: [PATCH 0/8] SOF fixes and updates for FW boot

[Mark Brown]

On Wed, 26 Aug 2020 11:45:24 -0700, Ranjani Sridharan wrote:
> This series includes fixes and updates for the FW boot sequence for
> Intel platforms.
> 
> Ranjani Sridharan (7):
>   ALSA: hda: fix VS_LTRC register name
>   ASoC: SOF: Intel: hda: Add helper function to program ICCMAX stream
>   ASoC: SOF: Intel: hda: modify the signature of get_stream_with_tag()
>   ASoC: SOF: Intel: hda: define macro for code loader stream format
>   ASoC: SOF: Intel: hda: Define FW boot sequence with ICCMAX
>   ASoC: SOF: Intel: hda: Add sof_tgl_ops for TGL platforms
>   ASoC: SOF: Intel: hda: Simplify error handling during FW boot
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/8] ALSA: hda: fix VS_LTRC register name
      commit: c242766f286aa5a54cc3d987e71b8c952059132d
[2/8] ASoC: SOF: Intel: hda: Remove unused parameters in cl_dsp_init()
      commit: 0ff06df0be60db920f3d7218fa2bded9fd8de3f6
[3/8] ASoC: SOF: Intel: hda: Add helper function to program ICCMAX stream
      commit: aca961f196e5da10d99f4b19e5c27e5a5d7731a3
[4/8] ASoC: SOF: Intel: hda: modify the signature of get_stream_with_tag()
      commit: c07fa3fcbd28b6d8383a05f0570d472894c6e38f
[5/8] ASoC: SOF: Intel: hda: define macro for code loader stream format
      commit: d43e381390d0aa1992ce204bb6a9af791edf3d45
[6/8] ASoC: SOF: Intel: hda: Define FW boot sequence with ICCMAX
      commit: acf705a425f0e9cd5cab06231479c0a239e5a671
[7/8] ASoC: SOF: Intel: hda: Add sof_tgl_ops for TGL platforms
      commit: 8b98491a6b8c41a7e4334fb94a58c268d831458e
[8/8] ASoC: SOF: Intel: hda: Simplify error handling during FW boot
      commit: 01d42d5a0a70bdcece7228232590f177e7114368

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark