[PATCH 0/5] ASoC: SOF: Intel: add flags to turn on SSP clocks early
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(-)
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];
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];
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));
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
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, };
/*
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
participants (2)
-
Mark Brown -
Pierre-Louis Bossart