[alsa-devel] [PATCH 0/5] ASoC: SOF: Intel: Soundwire integration
This patchset applies on top of the series "[PATCH 0/4] soundwire: update ASoC interfaces". The SOF/Intel code makes use of the interfaces defined for initialization.
Build support for SoundWire is not provided for now, all Soundwire-related code will be handled with a dummy fallback. We will enable SoundWire interfaces in the Kconfigs when the functionality is enabled in the soundwire tree.
In short, if the interfaces are agreed on, there is no risk with the integration of these patches on the ASoC side.
Pierre-Louis Bossart (5): ASoC: SOF: Intel: add SoundWire configuration interface ASoC: SOF: IPC: dai-intel: move ALH declarations in header file ASoC: SOF: Intel: hda: add SoundWire stream config/free callbacks ASoC: SOF: Intel: hda: initial SoundWire machine driver autodetect ASoC: SOF: Intel: hda: disable SoundWire interrupts on suspend
include/sound/sof/dai-intel.h | 18 +-- sound/soc/sof/intel/hda-dsp.c | 2 + sound/soc/sof/intel/hda-loader.c | 13 ++ sound/soc/sof/intel/hda.c | 230 ++++++++++++++++++++++++++++++- sound/soc/sof/intel/hda.h | 44 ++++++ 5 files changed, 295 insertions(+), 12 deletions(-)
Now that the SoundWire core supports the multi-step initialization, call the relevant APIs.
The actual hardware enablement can be done in two places, ideally we'd want to startup the SoundWire IP as soon as possible (while still taking power rail dependencies into account)
However when suspend/resume is implemented, the DSP device will be resumed first, and only when the DSP firmware is downloaded/booted would the SoundWire child devices be resumed, so there are only marginal benefits in starting the IP earlier for the first probe.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-loader.c | 13 ++++ sound/soc/sof/intel/hda.c | 121 ++++++++++++++++++++++++++++++- sound/soc/sof/intel/hda.h | 44 +++++++++++ 3 files changed, 177 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index b1783360fe10..2f5cd2c1ea3c 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -397,6 +397,19 @@ int hda_dsp_pre_fw_run(struct snd_sof_dev *sdev) /* post fw run operations */ int hda_dsp_post_fw_run(struct snd_sof_dev *sdev) { + int ret; + + if (sdev->first_boot) { + ret = hda_sdw_startup(sdev); + if (ret < 0) { + dev_err(sdev->dev, + "error: could not startup SoundWire links\n"); + return ret; + } + } + + hda_sdw_int_enable(sdev, true); + /* re-enable clock gating and power gating */ return hda_dsp_ctrl_clock_power_gating(sdev, true); } diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 103f4273c4d3..7e36f3bd6b39 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -18,7 +18,9 @@ #include <sound/hdaudio_ext.h> #include <sound/hda_register.h>
+#include <linux/acpi.h> #include <linux/module.h> +#include <linux/soundwire/sdw_intel.h> #include <sound/intel-nhlt.h> #include <sound/sof.h> #include <sound/sof/xtensa.h> @@ -34,6 +36,98 @@
#define EXCEPT_MAX_HDR_SIZE 0x400
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE) + +void hda_sdw_int_enable(struct snd_sof_dev *sdev, bool enable) +{ + sdw_intel_enable_irq(sdev->bar[HDA_DSP_BAR], enable); +} + +static int hda_sdw_acpi_scan(struct snd_sof_dev *sdev) +{ + struct sof_intel_hda_dev *hdev; + acpi_handle handle; + int ret; + + handle = ACPI_HANDLE(sdev->dev); + + /* save ACPI info for the probe step */ + hdev = sdev->pdata->hw_pdata; + + ret = sdw_intel_acpi_scan(handle, &hdev->info); + if (ret < 0) { + dev_err(sdev->dev, "%s failed\n", __func__); + return -EINVAL; + } + + return 0; +} + +static int hda_sdw_probe(struct snd_sof_dev *sdev) +{ + struct sof_intel_hda_dev *hdev; + struct sdw_intel_res res; + acpi_handle handle; + void *sdw; + + handle = ACPI_HANDLE(sdev->dev); + + hdev = sdev->pdata->hw_pdata; + + memset(&res, 0, sizeof(res)); + + res.mmio_base = sdev->bar[HDA_DSP_BAR]; + res.irq = sdev->ipc_irq; + res.handle = hdev->info.handle; + res.parent = sdev->dev; + + /* + * ops and arg fields are not populated for now, + * they will be needed when the DAI callbacks are + * provided + */ + + /* we could filter links here if needed, e.g for quirks */ + res.count = hdev->info.count; + res.link_mask = hdev->info.link_mask; + + sdw = sdw_intel_probe(&res); + if (!sdw) { + dev_err(sdev->dev, "error: SoundWire probe failed\n"); + return -EINVAL; + } + + /* save context */ + hdev->sdw = sdw; + + return 0; +} + +int hda_sdw_startup(struct snd_sof_dev *sdev) +{ + struct sof_intel_hda_dev *hdev; + + hdev = sdev->pdata->hw_pdata; + + return sdw_intel_startup(hdev->sdw); +} + +static int hda_sdw_exit(struct snd_sof_dev *sdev) +{ + struct sof_intel_hda_dev *hdev; + + hdev = sdev->pdata->hw_pdata; + + hda_sdw_int_enable(sdev, false); + + if (hdev->sdw) + sdw_intel_exit(hdev->sdw); + hdev->sdw = NULL; + + return 0; +} +#endif + /* * Debug */ @@ -337,11 +431,11 @@ static const char *fixup_tplg_name(struct snd_sof_dev *sdev, static int hda_init_caps(struct snd_sof_dev *sdev) { struct hdac_bus *bus = sof_to_bus(sdev); + struct snd_sof_pdata *pdata = sdev->pdata; #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) struct hdac_ext_link *hlink; struct snd_soc_acpi_mach_params *mach_params; struct snd_soc_acpi_mach *hda_mach; - struct snd_sof_pdata *pdata = sdev->pdata; struct snd_soc_acpi_mach *mach; const char *tplg_filename; const char *idisp_str; @@ -350,6 +444,8 @@ static int hda_init_caps(struct snd_sof_dev *sdev) int codec_num = 0; int i; #endif + struct sof_intel_hda_dev *hdev = pdata->hw_pdata; + u32 link_mask; int ret = 0;
device_disable_async_suspend(bus->dev); @@ -378,6 +474,27 @@ static int hda_init_caps(struct snd_sof_dev *sdev) return ret; }
+ /* scan SoundWire capabilities exposed by DSDT */ + ret = hda_sdw_acpi_scan(sdev); + if (ret < 0) { + dev_err(sdev->dev, "error: SoundWire ACPI scan error\n"); + return ret; + } + + link_mask = hdev->info.link_mask; + if (!link_mask) { + /* + * probe/allocated SoundWire resources. + * The hardware configuration takes place in hda_sdw_startup + * after power rails are enabled. + */ + ret = hda_sdw_probe(sdev); + if (ret < 0) { + dev_err(sdev->dev, "error: SoundWire probe error\n"); + return ret; + } + } + #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) if (bus->mlcap) snd_hdac_ext_bus_get_ml_capabilities(bus); @@ -682,6 +799,8 @@ int hda_dsp_remove(struct snd_sof_dev *sdev) snd_hdac_ext_bus_device_remove(bus); #endif
+ hda_sdw_exit(sdev); + if (!IS_ERR_OR_NULL(hda->dmic_dev)) platform_device_unregister(hda->dmic_dev);
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 0e7c366b8f71..e63630f5c547 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -11,6 +11,8 @@ #ifndef __SOF_INTEL_HDA_H #define __SOF_INTEL_HDA_H
+#include <linux/soundwire/sdw.h> +#include <linux/soundwire/sdw_intel.h> #include <sound/hda_codec.h> #include <sound/hdaudio_ext.h> #include "shim.h" @@ -408,6 +410,12 @@ struct sof_intel_hda_dev {
/* DMIC device */ struct platform_device *dmic_dev; + + /* ACPI information stored between scan and probe steps */ + struct sdw_intel_acpi_info info; + + /* sdw context allocated by SoundWire driver */ + struct sdw_intel_ctx *sdw; };
static inline struct hdac_bus *sof_to_bus(struct snd_sof_dev *s) @@ -598,6 +606,42 @@ int hda_dsp_trace_init(struct snd_sof_dev *sdev, u32 *stream_tag); int hda_dsp_trace_release(struct snd_sof_dev *sdev); int hda_dsp_trace_trigger(struct snd_sof_dev *sdev, int cmd);
+/* + * SoundWire support + */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE) + +int hda_sdw_startup(struct snd_sof_dev *sdev); +void hda_sdw_int_enable(struct snd_sof_dev *sdev, bool enable); + +#else + +static inline int hda_sdw_acpi_scan(struct snd_sof_dev *sdev) +{ + return 0; +} + +static inline int hda_sdw_probe(struct snd_sof_dev *sdev) +{ + return 0; +} + +static inline int hda_sdw_startup(struct snd_sof_dev *sdev) +{ + return 0; +} + +static inline int hda_sdw_exit(struct snd_sof_dev *sdev) +{ + return 0; +} + +static inline void hda_sdw_int_enable(struct snd_sof_dev *sdev, bool enable) +{ +} + +#endif + /* common dai driver */ extern struct snd_soc_dai_driver skl_dai[];
ALH was inserted in the wrong place during integration, add after DMIC to mirror the file used by SOF firmware.
No functional change, just text move in the same file to better track changes, if any.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- include/sound/sof/dai-intel.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/sound/sof/dai-intel.h b/include/sound/sof/dai-intel.h index 5f1ef5565be6..04e48227f542 100644 --- a/include/sound/sof/dai-intel.h +++ b/include/sound/sof/dai-intel.h @@ -87,6 +87,15 @@ struct sof_ipc_dai_hda_params { uint32_t link_dma_ch; } __packed;
+/* ALH Configuration Request - SOF_IPC_DAI_ALH_CONFIG */ +struct sof_ipc_dai_alh_params { + struct sof_ipc_hdr hdr; + uint32_t stream_id; + + /* reserved for future use */ + uint32_t reserved[15]; +} __packed; + /* DMIC Configuration Request - SOF_IPC_DAI_DMIC_CONFIG */
/* This struct is defined per 2ch PDM controller available in the platform. @@ -179,13 +188,4 @@ struct sof_ipc_dai_dmic_params { struct sof_ipc_dai_dmic_pdm_ctrl pdm[0]; } __packed;
-/* ALH Configuration Request - SOF_IPC_DAI_ALH_CONFIG */ -struct sof_ipc_dai_alh_params { - struct sof_ipc_hdr hdr; - uint32_t stream_id; - - /* reserved for future use */ - uint32_t reserved[15]; -} __packed; - #endif
These callbacks are invoked when a matching hw_params/hw_free() DAI operation takes place, and will result in IPC operations with the SOF firmware.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda.c | 70 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+)
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 7e36f3bd6b39..98ac38ca0afa 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -38,6 +38,74 @@
#if IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE)
+static int sdw_params_stream(struct device *dev, + struct sdw_intel_stream_params_data *params_data) +{ + struct snd_sof_dev *sdev = dev_get_drvdata(dev); + struct snd_soc_dai *d = params_data->dai; + struct sof_ipc_dai_config config; + struct sof_ipc_reply reply; + int link_id = params_data->link_id; + int alh_stream_id = params_data->alh_stream_id; + int ret; + u32 size = sizeof(config); + + memset(&config, 0, size); + config.hdr.size = size; + config.hdr.cmd = SOF_IPC_GLB_DAI_MSG | SOF_IPC_DAI_CONFIG; + config.type = SOF_DAI_INTEL_ALH; + config.dai_index = (link_id << 8) | (d->id); + config.alh.stream_id = alh_stream_id; + + /* send message to DSP */ + ret = sof_ipc_tx_message(sdev->ipc, + config.hdr.cmd, &config, size, &reply, + sizeof(reply)); + if (ret < 0) { + dev_err(sdev->dev, + "error: failed to set DAI hw_params for link %d dai->id %d ALH %d\n", + link_id, d->id, alh_stream_id); + } + + return ret; +} + +static int sdw_free_stream(struct device *dev, + struct sdw_intel_stream_free_data *free_data) +{ + struct snd_sof_dev *sdev = dev_get_drvdata(dev); + struct snd_soc_dai *d = free_data->dai; + struct sof_ipc_dai_config config; + struct sof_ipc_reply reply; + int link_id = free_data->link_id; + int ret; + u32 size = sizeof(config); + + memset(&config, 0, size); + config.hdr.size = size; + config.hdr.cmd = SOF_IPC_GLB_DAI_MSG | SOF_IPC_DAI_CONFIG; + config.type = SOF_DAI_INTEL_ALH; + config.dai_index = (link_id << 8) | d->id; + config.alh.stream_id = 0xFFFF; /* invalid value on purpose */ + + /* send message to DSP */ + ret = sof_ipc_tx_message(sdev->ipc, + config.hdr.cmd, &config, size, &reply, + sizeof(reply)); + if (ret < 0) { + dev_err(sdev->dev, + "error: failed to free stream for link %d dai->id %d\n", + link_id, d->id); + } + + return ret; +} + +static const struct sdw_intel_ops sdw_callback = { + .params_stream = sdw_params_stream, + .free_stream = sdw_free_stream, +}; + void hda_sdw_int_enable(struct snd_sof_dev *sdev, bool enable) { sdw_intel_enable_irq(sdev->bar[HDA_DSP_BAR], enable); @@ -80,6 +148,8 @@ static int hda_sdw_probe(struct snd_sof_dev *sdev) res.irq = sdev->ipc_irq; res.handle = hdev->info.handle; res.parent = sdev->dev; + res.ops = &sdw_callback; + res.dev = sdev->dev;
/* * ops and arg fields are not populated for now,
For now we have a limited number of machine driver configurations, and we can detect them based on the link configuration returned after checking hardware and firmware (BIOS) configurations.
It's likely that in the future we will need to check for _ADR match as well, which can easily be done by extending the acpi_info structure.
There is a chance that in extreme cases where the BIOS contains too much information we would need to detect which Slave devices actually report as 'attached'. This would be more accurate than static table-based solutions, but it also introduces timing dependencies since we don't know when those devices might become attached, so will only be only be looked at if we see limitations with static methods and the usual quirks based e.g. on DMI information.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda.c | 61 ++++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 13 deletions(-)
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 98ac38ca0afa..5f614ec8de1d 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -179,6 +179,9 @@ int hda_sdw_startup(struct snd_sof_dev *sdev)
hdev = sdev->pdata->hw_pdata;
+ if (!hdev->sdw) + return 0; + return sdw_intel_startup(hdev->sdw); }
@@ -502,11 +505,11 @@ static int hda_init_caps(struct snd_sof_dev *sdev) { struct hdac_bus *bus = sof_to_bus(sdev); struct snd_sof_pdata *pdata = sdev->pdata; + struct snd_soc_acpi_mach *mach; #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) struct hdac_ext_link *hlink; - struct snd_soc_acpi_mach_params *mach_params; struct snd_soc_acpi_mach *hda_mach; - struct snd_soc_acpi_mach *mach; + struct snd_soc_acpi_mach_params *mach_params; const char *tplg_filename; const char *idisp_str; const char *dmic_str; @@ -547,24 +550,56 @@ static int hda_init_caps(struct snd_sof_dev *sdev) /* scan SoundWire capabilities exposed by DSDT */ ret = hda_sdw_acpi_scan(sdev); if (ret < 0) { - dev_err(sdev->dev, "error: SoundWire ACPI scan error\n"); - return ret; + dev_dbg(sdev->dev, "skipping SoundWire, ACPI scan error\n"); + goto skip_soundwire; }
link_mask = hdev->info.link_mask; if (!link_mask) { - /* - * probe/allocated SoundWire resources. - * The hardware configuration takes place in hda_sdw_startup - * after power rails are enabled. - */ - ret = hda_sdw_probe(sdev); - if (ret < 0) { - dev_err(sdev->dev, "error: SoundWire probe error\n"); - return ret; + dev_dbg(sdev->dev, "skipping SoundWire, no links enabled\n"); + goto skip_soundwire; + } + + /* + * probe/allocate SoundWire resources. + * The hardware configuration takes place in hda_sdw_startup + * after power rails are enabled. + * It's entirely possible to have a mix of I2S/DMIC/SoundWire + * devices, so we allocate the resources in all cases. + */ + ret = hda_sdw_probe(sdev); + if (ret < 0) { + dev_err(sdev->dev, "error: SoundWire probe error\n"); + return ret; + } + + /* + * Select SoundWire machine driver if needed using the + * alternate tables. This case deals with SoundWire-only + * machines, for mixed cases with I2C/I2S the detection relies + * on the HID list. + */ + if (!pdata->machine) { + mach = pdata->desc->alt_machines; + while (mach && mach->link_mask && mach->link_mask != link_mask) + mach++; + if (mach && mach->link_mask) { + dev_dbg(bus->dev, + "SoundWire machine driver %s topology %s\n", + mach->drv_name, + mach->sof_tplg_filename); + pdata->machine = mach; + mach->mach_params.platform = dev_name(sdev->dev); + pdata->fw_filename = mach->sof_fw_filename; + pdata->tplg_filename = mach->sof_tplg_filename; + } else { + dev_info(sdev->dev, + "No SoundWire machine driver found\n"); } }
+skip_soundwire: + #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) if (bus->mlcap) snd_hdac_ext_bus_get_ml_capabilities(bus);
Doing this avoid conflicts and errors reported on the bus.
The interrupts are only re-enabled on resume after the firmware is downloaded, so the behavior is not fully symmetric
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dsp.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index fa2f1f66c72c..a7509e8a0e30 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -361,6 +361,8 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend) #endif int ret;
+ hda_sdw_int_enable(sdev, false); + /* disable IPC interrupts */ hda_dsp_ipc_int_disable(sdev);
On 10/23/19 4:14 PM, Pierre-Louis Bossart wrote:
This patchset applies on top of the series "[PATCH 0/4] soundwire: update ASoC interfaces". The SOF/Intel code makes use of the interfaces defined for initialization.
Build support for SoundWire is not provided for now, all Soundwire-related code will be handled with a dummy fallback. We will enable SoundWire interfaces in the Kconfigs when the functionality is enabled in the soundwire tree.
In short, if the interfaces are agreed on, there is no risk with the integration of these patches on the ASoC side.
Mark, Vinod, any comments/objections on the suggested interfaces? I tried to make your life simpler with a clean separation between SoundWire and ASoC/SOF.
Pierre-Louis Bossart (5): ASoC: SOF: Intel: add SoundWire configuration interface ASoC: SOF: IPC: dai-intel: move ALH declarations in header file ASoC: SOF: Intel: hda: add SoundWire stream config/free callbacks ASoC: SOF: Intel: hda: initial SoundWire machine driver autodetect ASoC: SOF: Intel: hda: disable SoundWire interrupts on suspend
include/sound/sof/dai-intel.h | 18 +-- sound/soc/sof/intel/hda-dsp.c | 2 + sound/soc/sof/intel/hda-loader.c | 13 ++ sound/soc/sof/intel/hda.c | 230 ++++++++++++++++++++++++++++++- sound/soc/sof/intel/hda.h | 44 ++++++ 5 files changed, 295 insertions(+), 12 deletions(-)
participants (1)
-
Pierre-Louis Bossart