[PATCH 0/5] ASoC: SOF: Intel: add flags to turn on SSP clocks early

[PATCH 0/5] ASoC: SOF: Intel: add flags to turn on SSP clocks early

[Pierre-Louis Bossart]

With the chip shortage, some GeminiLake Intel-based designs were
respun and now rely on codecs that need the SSP bit clock turned on in
the hw_params stage, not the trigger stage. This patchset mirrors the
flags added in the SOF DAI_CONFIG IPC, and sets the flags when this
capability is indicated as necessary in the topology files where the
SSP configuration is stored.

We initially considered a more generic solution with an on-demand SSP
clock activation using the common clock framework. This would be a
more elegant solution indeed, but it would have required more
intrusive changes that would conflict with the SOF multi-client
support (in-development), and more backport hassles on product
branches. The on-demand activation of clocks is still a desired
feature that will be enabled at a later point.

Bard Liao (1):
  ASoC: SOF: dai-intel: add SOF_DAI_INTEL_SSP_CLKCTRL_MCLK/BCLK_ES bits

Pierre-Louis Bossart (4):
  ASoC: SOF: dai: mirror group_id definition added in firmware
  ASoC: SOF: dai: include new flags for DAI_CONFIG
  ASoC: SOF: Intel: hda: add new flags for DAI_CONFIG
  ASoC: SOF: Intel: hda-dai: improve SSP DAI handling for dynamic
    pipelines

 include/sound/sof/dai-intel.h |  4 ++
 include/sound/sof/dai.h       | 10 ++++-
 sound/soc/sof/intel/hda-dai.c | 82 ++++++++++++++++++++++++++++++++++-
 sound/soc/sof/intel/hda.c     |  6 +++
 sound/soc/sof/sof-audio.c     |  4 ++
 5 files changed, 103 insertions(+), 3 deletions(-)

-- 
2.25.1

[PATCH 1/5] ASoC: SOF: dai: mirror group_id definition added in firmware

[Pierre-Louis Bossart]

This was added in ABI 3.17 but never added to the kernel tree. The
group_id is not currently used but this patch is required before
additional changes.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <bard.liao@intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Brent Lu <brent.lu@intel.com>
---
 include/sound/sof/dai.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/sound/sof/dai.h b/include/sound/sof/dai.h
index 6bb403e8c5ee..ea6dc970c18f 100644
--- a/include/sound/sof/dai.h
+++ b/include/sound/sof/dai.h
@@ -69,7 +69,8 @@ struct sof_ipc_dai_config {
 
 	/* physical protocol and clocking */
 	uint16_t format;	/**< SOF_DAI_FMT_ */
-	uint16_t reserved16;	/**< alignment */
+	uint8_t group_id;	/**< group ID, 0 means no group (ABI 3.17) */
+	uint8_t reserved8;	/**< alignment */
 
 	/* reserved for future use */
 	uint32_t reserved[8];
-- 
2.25.1

[PATCH 2/5] ASoC: SOF: dai: include new flags for DAI_CONFIG

[Pierre-Louis Bossart]

Mirror changes done in SOF tree. The changes do not rely on
BIT/GENMASK on purpose to keep the structure and flags common with the
firmware tree.

The DAI_CONFIG IPC is currently used in multiple ways. It is sent to
the DSP firmware when enabling static or dynamic pipelines, in
hw_params or prepare callbacks for Intel SSP, HDaudio and ALH, on
trigger_stop and hw_free.

This IPC has been abused a bit in the past, i.e. the values used for
some of the DAI-specific fields are used to either allocate or free
resources. Two typical examples are Intel HDaudio and SoundWire/ALH
DAIs, where using a zero DMA channel number or stream tag signals to
the firmware the DMA channels or tags allocated earlier can be freed.

Rather than add a new IPC for 'hw_params' and 'hw_free', this patch
suggests supporting a 2-bit value conveying the 'stage' information in
an existing IPC structure. Only 3 possible values are used.

The mapping between HW_PARAMS and HW_FREE flags and ALSA definitions
is not strictly 1:1, e.g. in some cases the HW_PARAM flag might be set
during the .prepare callback, while the HW_FREE might be sent during the
ALSA .trigger for stop/suspend.

The semantics of the flags is to reserve and start/stop all needed
resources, typically hardware related such as DMAs or clocks, when the
HW_PARAMS is set, while the HW_FREE flag allows the firmware to
release the resources allocated. The data transfers are still
controlled within the firmware through the propagation of the trigger
command.

The driver can then pass information that the DAI_CONFIG was invoked
in e.g. a pipeline/DAI setup, hw_params or hw_free stage without
having to use a special DAI-specific encoding. Unfortunately we can't
remove old encodings due to backwards-compatibility requirements but
for new cases, such as the SSP in follow-up patches, we can make the
IPC less cryptic.

This change is tagged as ABI 3.19 and is completely backwards compatible.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <bard.liao@intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Brent Lu <brent.lu@intel.com>
---
 include/sound/sof/dai.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/sound/sof/dai.h b/include/sound/sof/dai.h
index ea6dc970c18f..9625f47557b8 100644
--- a/include/sound/sof/dai.h
+++ b/include/sound/sof/dai.h
@@ -50,6 +50,13 @@
 #define SOF_DAI_FMT_INV_MASK		0x0f00
 #define SOF_DAI_FMT_CLOCK_PROVIDER_MASK	0xf000
 
+/* DAI_CONFIG flags */
+#define SOF_DAI_CONFIG_FLAGS_MASK	0x3
+#define SOF_DAI_CONFIG_FLAGS_NONE	(0 << 0) /**< DAI_CONFIG sent without stage information */
+#define SOF_DAI_CONFIG_FLAGS_HW_PARAMS	(1 << 0) /**< DAI_CONFIG sent during hw_params stage */
+#define SOF_DAI_CONFIG_FLAGS_HW_FREE	(2 << 0) /**< DAI_CONFIG sent during hw_free stage */
+#define SOF_DAI_CONFIG_FLAGS_RFU	(3 << 0) /**< not used, reserved for future use */
+
 /** \brief Types of DAI */
 enum sof_ipc_dai_type {
 	SOF_DAI_INTEL_NONE = 0,		/**< None */
@@ -70,7 +77,7 @@ struct sof_ipc_dai_config {
 	/* physical protocol and clocking */
 	uint16_t format;	/**< SOF_DAI_FMT_ */
 	uint8_t group_id;	/**< group ID, 0 means no group (ABI 3.17) */
-	uint8_t reserved8;	/**< alignment */
+	uint8_t flags;		/**< SOF_DAI_CONFIG_FLAGS_ (ABI 3.19) */
 
 	/* reserved for future use */
 	uint32_t reserved[8];
-- 
2.25.1

[PATCH 3/5] ASoC: SOF: Intel: hda: add new flags for DAI_CONFIG

[Pierre-Louis Bossart]

The DAI_CONFIG is used for both hw_params and hw_free. Use flags to
specify what stage the configuration applies to.

the DAI_CONFIG IPC may be sent also during the widget setup so each
flag is cleared after the IPC to restore the state.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <bard.liao@intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Brent Lu <brent.lu@intel.com>
---
 sound/soc/sof/intel/hda.c | 6 ++++++
 sound/soc/sof/sof-audio.c | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 066e90506ae6..1463f3de01bc 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -71,6 +71,9 @@ int hda_ctrl_dai_widget_setup(struct snd_soc_dapm_widget *w)
 		return ret;
 	}
 
+	/* set HW_PARAMS flag */
+	config->flags = FIELD_PREP(SOF_DAI_CONFIG_FLAGS_MASK, SOF_DAI_CONFIG_FLAGS_HW_PARAMS);
+
 	/* send DAI_CONFIG IPC */
 	ret = sof_ipc_tx_message(sdev->ipc, config->hdr.cmd, config, config->hdr.size,
 				 &reply, sizeof(reply));
@@ -107,6 +110,9 @@ int hda_ctrl_dai_widget_free(struct snd_soc_dapm_widget *w)
 
 	config = &sof_dai->dai_config[sof_dai->current_config];
 
+	/* set HW_FREE flag */
+	config->flags = FIELD_PREP(SOF_DAI_CONFIG_FLAGS_MASK, SOF_DAI_CONFIG_FLAGS_HW_FREE);
+
 	ret = sof_ipc_tx_message(sdev->ipc, config->hdr.cmd, config, config->hdr.size,
 				 &reply, sizeof(reply));
 	if (ret < 0)
diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index c4cabe26b157..262cb3ad4674 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -8,6 +8,7 @@
 // Author: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
 //
 
+#include <linux/bitfield.h>
 #include "sof-audio.h"
 #include "ops.h"
 
@@ -55,6 +56,9 @@ static int sof_dai_config_setup(struct snd_sof_dev *sdev, struct snd_sof_dai *da
 		return -EINVAL;
 	}
 
+	/* set NONE flag to clear all previous settings */
+	config->flags = FIELD_PREP(SOF_DAI_CONFIG_FLAGS_MASK, SOF_DAI_CONFIG_FLAGS_NONE);
+
 	ret = sof_ipc_tx_message(sdev->ipc, config->hdr.cmd, config, config->hdr.size,
 				 &reply, sizeof(reply));
 
-- 
2.25.1

[PATCH 4/5] ASoC: SOF: dai-intel: add SOF_DAI_INTEL_SSP_CLKCTRL_MCLK/BCLK_ES bits

[Pierre-Louis Bossart]

From: Bard Liao <yung-chuan.liao@linux.intel.com>

Add two clks_control bits. MCLK and/or BCLK will start during hw_params
and stop during hw_free if the corresponding bit is set.

While the kernel does not do anything with these bitfields, this is
also tagged as part of the ABI 3.19 changes.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <bard.liao@intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Brent Lu <brent.lu@intel.com>
---
 include/sound/sof/dai-intel.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/sound/sof/dai-intel.h b/include/sound/sof/dai-intel.h
index 136adf6686e2..7a266f41983c 100644
--- a/include/sound/sof/dai-intel.h
+++ b/include/sound/sof/dai-intel.h
@@ -48,6 +48,10 @@
 #define SOF_DAI_INTEL_SSP_CLKCTRL_FS_KA			BIT(4)
 /* bclk idle */
 #define SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_IDLE_HIGH	BIT(5)
+/* mclk early start */
+#define SOF_DAI_INTEL_SSP_CLKCTRL_MCLK_ES               BIT(6)
+/* bclk early start */
+#define SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES               BIT(7)
 
 /* DMIC max. four controllers for eight microphone channels */
 #define SOF_DAI_INTEL_DMIC_NUM_CTRL			4
-- 
2.25.1

[PATCH 5/5] ASoC: SOF: Intel: hda-dai: improve SSP DAI handling for dynamic pipelines

[Pierre-Louis Bossart]

In order to keep the widget use_count balanced, make sure the DAI
widgets are allocated once in hw_params and released in hw_free. A
'setup' status flag is used to deal with cases where the .hw_params
callback is invoked multiple times, and likewise with cases where
hw_free is invoked without hw_params being called first (which can
happen if the FE hw_params fails).

In addition, this patch frees the widgets in the suspend transition,
and reallocates them in the .prepare callback. The 'setup' flag helps
in this case differentiate between resume (setup needed) and
xruns (setup not needed).

This balanced operation was not needed previously but will be required
when SOF dynamic pipelines are enabled.

Co-developed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Bard Liao <bard.liao@intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/soc/sof/intel/hda-dai.c | 82 ++++++++++++++++++++++++++++++++++-
 1 file changed, 80 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 59d6750c6a20..dfd2df0b1bc3 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -440,6 +440,11 @@ static const struct snd_soc_dai_ops hda_link_dai_ops = {
 
 #endif
 
+/* only one flag used so far to harden hw_params/hw_free/trigger/prepare */
+struct ssp_dai_dma_data {
+	bool setup;
+};
+
 static int ssp_dai_setup_or_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai,
 				 bool setup)
 {
@@ -469,22 +474,95 @@ static int ssp_dai_setup_or_free(struct snd_pcm_substream *substream, struct snd
 	return hda_ctrl_dai_widget_free(w);
 }
 
+static int ssp_dai_startup(struct snd_pcm_substream *substream,
+			   struct snd_soc_dai *dai)
+{
+	struct ssp_dai_dma_data *dma_data;
+
+	dma_data = kzalloc(sizeof(*dma_data), GFP_KERNEL);
+	if (!dma_data)
+		return -ENOMEM;
+
+	snd_soc_dai_set_dma_data(dai, substream, dma_data);
+
+	return 0;
+}
+
+static int ssp_dai_setup(struct snd_pcm_substream *substream,
+			 struct snd_soc_dai *dai,
+			 bool setup)
+{
+	struct ssp_dai_dma_data *dma_data;
+	int ret = 0;
+
+	dma_data = snd_soc_dai_get_dma_data(dai, substream);
+	if (!dma_data) {
+		dev_err(dai->dev, "%s: failed to get dma_data\n", __func__);
+		return -EIO;
+	}
+
+	if (dma_data->setup != setup) {
+		ret = ssp_dai_setup_or_free(substream, dai, setup);
+		if (!ret)
+			dma_data->setup = setup;
+	}
+	return ret;
+}
+
 static int ssp_dai_hw_params(struct snd_pcm_substream *substream,
 			     struct snd_pcm_hw_params *params,
 			     struct snd_soc_dai *dai)
 {
-	return ssp_dai_setup_or_free(substream, dai, true);
+	/* params are ignored for now */
+	return ssp_dai_setup(substream, dai, true);
+}
+
+static int ssp_dai_prepare(struct snd_pcm_substream *substream,
+			   struct snd_soc_dai *dai)
+{
+	/*
+	 * the SSP will only be reconfigured during resume operations and
+	 * not in case of xruns
+	 */
+	return ssp_dai_setup(substream, dai, true);
+}
+
+static int ssp_dai_trigger(struct snd_pcm_substream *substream,
+			   int cmd, struct snd_soc_dai *dai)
+{
+	if (cmd != SNDRV_PCM_TRIGGER_SUSPEND)
+		return 0;
+
+	return ssp_dai_setup(substream, dai, false);
 }
 
 static int ssp_dai_hw_free(struct snd_pcm_substream *substream,
 			   struct snd_soc_dai *dai)
 {
-	return ssp_dai_setup_or_free(substream, dai, false);
+	return ssp_dai_setup(substream, dai, false);
+}
+
+static void ssp_dai_shutdown(struct snd_pcm_substream *substream,
+			     struct snd_soc_dai *dai)
+{
+	struct ssp_dai_dma_data *dma_data;
+
+	dma_data = snd_soc_dai_get_dma_data(dai, substream);
+	if (!dma_data) {
+		dev_err(dai->dev, "%s: failed to get dma_data\n", __func__);
+		return;
+	}
+	snd_soc_dai_set_dma_data(dai, substream, NULL);
+	kfree(dma_data);
 }
 
 static const struct snd_soc_dai_ops ssp_dai_ops = {
+	.startup = ssp_dai_startup,
 	.hw_params = ssp_dai_hw_params,
+	.prepare = ssp_dai_prepare,
+	.trigger = ssp_dai_trigger,
 	.hw_free = ssp_dai_hw_free,
+	.shutdown = ssp_dai_shutdown,
 };
 
 /*
-- 
2.25.1

Re: [PATCH 0/5] ASoC: SOF: Intel: add flags to turn on SSP clocks early

[Mark Brown]

On Mon, 4 Oct 2021 12:14:25 -0500, Pierre-Louis Bossart wrote:
> With the chip shortage, some GeminiLake Intel-based designs were
> respun and now rely on codecs that need the SSP bit clock turned on in
> the hw_params stage, not the trigger stage. This patchset mirrors the
> flags added in the SOF DAI_CONFIG IPC, and sets the flags when this
> capability is indicated as necessary in the topology files where the
> SSP configuration is stored.
> 
> [...]

Applied to

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

Thanks!

[1/5] ASoC: SOF: dai: mirror group_id definition added in firmware
      commit: 663742307fd7b695f34597e28a846afbc9d5f3e8
[2/5] ASoC: SOF: dai: include new flags for DAI_CONFIG
      commit: 21c51692fcdf9ceb36eeda48849e0ac155ff84f8
[3/5] ASoC: SOF: Intel: hda: add new flags for DAI_CONFIG
      commit: b30b60a26a2369d6cbb63d63245f3b13f0403449
[4/5] ASoC: SOF: dai-intel: add SOF_DAI_INTEL_SSP_CLKCTRL_MCLK/BCLK_ES bits
      commit: 68776b2fb06e7e438a2c4ebca5ca7f216e31d678
[5/5] ASoC: SOF: Intel: hda-dai: improve SSP DAI handling for dynamic pipelines
      commit: 84e3cfd16a72c7b7d569b72661093cdd16346d29

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