[alsa-devel] [PATCH 0/8] ASOC: SOF: Intel: improvements for Broadwell, HDA, OSS
This patchset provides general improvements for SOF on Intel platforms.
First we have good support for SOF on Broadwell now (DMA and stability issues seem to be being us). The legacy driver remains the recommendation until UCM and topologies are provided downstream, but this is promising.
Kai also fixed a couple of user-reported bugs for HDaudio/HDMI support when external graphics are enabled, and OSS emulation. Additional work is needed for the case where the Intel internal graphics solution is disabled.
Also included a change on error logs which apparently made some users nervous.
Bard liao (1): ASoC: SOF: Intel: lower print level to dbg if we will reinit DSP
Kai Vehmanen (3): ASoC: SOF: fix PCM playback through ALSA OSS emulation ASoC: SOF: Intel: fix HDA codec driver probe with multiple controllers ASoC: hdac_hda: Fix error in driver removal after failed probe
Pan Xiuli (1): ASoC: Intel: broadwell: change cpu_dai and platform components for SOF
Pierre-Louis Bossart (3): ASoC: Intel: bdw-rt5677: fix Kconfig dependencies ASoC: Intel: bdw-rt5677: change cpu_dai and platform components for SOF ASoC: Intel: bdw-rt5650: change cpu_dai and platform components for SOF
sound/soc/codecs/hdac_hda.c | 4 ++- sound/soc/intel/boards/Kconfig | 3 ++ sound/soc/intel/boards/bdw-rt5650.c | 10 +++++- sound/soc/intel/boards/bdw-rt5677.c | 10 +++++- sound/soc/intel/boards/broadwell.c | 10 +++++- sound/soc/sof/intel/hda-codec.c | 19 +++++++---- sound/soc/sof/intel/hda-loader.c | 6 ++-- sound/soc/sof/pcm.c | 53 +++++++++++++++++------------ 8 files changed, 79 insertions(+), 36 deletions(-)
The existing machine driver depends on SPI Master capabilities, but the Kconfig does not model this dependency and the SPI controller needs to be selected as well.
Without this patch the machine driver probe would fail with the spi-RT5677AA:00 component never registered by the ACPI/LPSS subsystem.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/intel/boards/Kconfig | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig index bddf04715f15..9ca2567d0059 100644 --- a/sound/soc/intel/boards/Kconfig +++ b/sound/soc/intel/boards/Kconfig @@ -62,6 +62,9 @@ config SND_SOC_INTEL_BDW_RT5677_MACH depends on I2C_DESIGNWARE_PLATFORM || COMPILE_TEST depends on GPIOLIB || COMPILE_TEST depends on X86_INTEL_LPSS || COMPILE_TEST + depends on SPI_MASTER + select SPI_PXA2XX + select SND_SOC_RT5677_SPI select SND_SOC_RT5677 help This adds support for Intel Broadwell platform based boards with
The patch
ASoC: Intel: bdw-rt5677: fix Kconfig dependencies
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.6
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
From 630db1549356f64424b8f532c028e7894df8e40b Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Date: Fri, 10 Jan 2020 17:57:44 -0600 Subject: [PATCH] ASoC: Intel: bdw-rt5677: fix Kconfig dependencies
The existing machine driver depends on SPI Master capabilities, but the Kconfig does not model this dependency and the SPI controller needs to be selected as well.
Without this patch the machine driver probe would fail with the spi-RT5677AA:00 component never registered by the ACPI/LPSS subsystem.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20200110235751.3404-2-pierre-louis.bossart@linux.i... Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/boards/Kconfig | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig index bddf04715f15..9ca2567d0059 100644 --- a/sound/soc/intel/boards/Kconfig +++ b/sound/soc/intel/boards/Kconfig @@ -62,6 +62,9 @@ config SND_SOC_INTEL_BDW_RT5677_MACH depends on I2C_DESIGNWARE_PLATFORM || COMPILE_TEST depends on GPIOLIB || COMPILE_TEST depends on X86_INTEL_LPSS || COMPILE_TEST + depends on SPI_MASTER + select SPI_PXA2XX + select SND_SOC_RT5677_SPI select SND_SOC_RT5677 help This adds support for Intel Broadwell platform based boards with
The legacy driver uses dummy cpu_dai and platform, SOF requires actual values to bind.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/intel/boards/bdw-rt5677.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c index 2af8e5a62da8..bb643c99069d 100644 --- a/sound/soc/intel/boards/bdw-rt5677.c +++ b/sound/soc/intel/boards/bdw-rt5677.c @@ -295,6 +295,14 @@ SND_SOC_DAILINK_DEF(platform, SND_SOC_DAILINK_DEF(be, DAILINK_COMP_ARRAY(COMP_CODEC("i2c-RT5677CE:00", "rt5677-aif1")));
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL) +SND_SOC_DAILINK_DEF(ssp0_port, + DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port"))); +#else +SND_SOC_DAILINK_DEF(ssp0_port, + DAILINK_COMP_ARRAY(COMP_DUMMY())); +#endif + /* Wake on voice interface */ SND_SOC_DAILINK_DEFS(dsp, DAILINK_COMP_ARRAY(COMP_CPU("spi-RT5677AA:00")), @@ -342,7 +350,7 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = { .dpcm_playback = 1, .dpcm_capture = 1, .init = bdw_rt5677_init, - SND_SOC_DAILINK_REG(dummy, be, dummy), + SND_SOC_DAILINK_REG(ssp0_port, be, platform), }, };
The patch
ASoC: Intel: bdw-rt5677: change cpu_dai and platform components for SOF
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.6
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
From 4865bde187b29e42b3f572d9610dfca9a57c30b9 Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Date: Fri, 10 Jan 2020 17:57:45 -0600 Subject: [PATCH] ASoC: Intel: bdw-rt5677: change cpu_dai and platform components for SOF
The legacy driver uses dummy cpu_dai and platform, SOF requires actual values to bind.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20200110235751.3404-3-pierre-louis.bossart@linux.i... Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/boards/bdw-rt5677.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c index 2af8e5a62da8..bb643c99069d 100644 --- a/sound/soc/intel/boards/bdw-rt5677.c +++ b/sound/soc/intel/boards/bdw-rt5677.c @@ -295,6 +295,14 @@ SND_SOC_DAILINK_DEF(platform, SND_SOC_DAILINK_DEF(be, DAILINK_COMP_ARRAY(COMP_CODEC("i2c-RT5677CE:00", "rt5677-aif1")));
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL) +SND_SOC_DAILINK_DEF(ssp0_port, + DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port"))); +#else +SND_SOC_DAILINK_DEF(ssp0_port, + DAILINK_COMP_ARRAY(COMP_DUMMY())); +#endif + /* Wake on voice interface */ SND_SOC_DAILINK_DEFS(dsp, DAILINK_COMP_ARRAY(COMP_CPU("spi-RT5677AA:00")), @@ -342,7 +350,7 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = { .dpcm_playback = 1, .dpcm_capture = 1, .init = bdw_rt5677_init, - SND_SOC_DAILINK_REG(dummy, be, dummy), + SND_SOC_DAILINK_REG(ssp0_port, be, platform), }, };
From: Pan Xiuli xiuli.pan@linux.intel.com
The legacy driver uses dummy cpu_dai and platform, SOF requires actual values to bind.
Signed-off-by: Pan Xiuli xiuli.pan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/intel/boards/broadwell.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/broadwell.c index db7e1e87156d..b9c12e24c70b 100644 --- a/sound/soc/intel/boards/broadwell.c +++ b/sound/soc/intel/boards/broadwell.c @@ -164,6 +164,14 @@ SND_SOC_DAILINK_DEF(platform, SND_SOC_DAILINK_DEF(codec, DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT343A:00", "rt286-aif1")));
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL) +SND_SOC_DAILINK_DEF(ssp0_port, + DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port"))); +#else +SND_SOC_DAILINK_DEF(ssp0_port, + DAILINK_COMP_ARRAY(COMP_DUMMY())); +#endif + /* broadwell digital audio interface glue - connects codec <--> CPU */ static struct snd_soc_dai_link broadwell_rt286_dais[] = { /* Front End DAI links */ @@ -218,7 +226,7 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] = { .ops = &broadwell_rt286_ops, .dpcm_playback = 1, .dpcm_capture = 1, - SND_SOC_DAILINK_REG(dummy, codec, dummy), + SND_SOC_DAILINK_REG(ssp0_port, codec, platform), }, };
The patch
ASoC: Intel: broadwell: change cpu_dai and platform components for SOF
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.6
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
From 64df6afa0dab5eda95cc4cc2269e3d4e83b6b6ce Mon Sep 17 00:00:00 2001
From: Pan Xiuli xiuli.pan@linux.intel.com Date: Fri, 10 Jan 2020 17:57:46 -0600 Subject: [PATCH] ASoC: Intel: broadwell: change cpu_dai and platform components for SOF
The legacy driver uses dummy cpu_dai and platform, SOF requires actual values to bind.
Signed-off-by: Pan Xiuli xiuli.pan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20200110235751.3404-4-pierre-louis.bossart@linux.i... Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/boards/broadwell.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/broadwell.c index db7e1e87156d..b9c12e24c70b 100644 --- a/sound/soc/intel/boards/broadwell.c +++ b/sound/soc/intel/boards/broadwell.c @@ -164,6 +164,14 @@ SND_SOC_DAILINK_DEF(platform, SND_SOC_DAILINK_DEF(codec, DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT343A:00", "rt286-aif1")));
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL) +SND_SOC_DAILINK_DEF(ssp0_port, + DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port"))); +#else +SND_SOC_DAILINK_DEF(ssp0_port, + DAILINK_COMP_ARRAY(COMP_DUMMY())); +#endif + /* broadwell digital audio interface glue - connects codec <--> CPU */ static struct snd_soc_dai_link broadwell_rt286_dais[] = { /* Front End DAI links */ @@ -218,7 +226,7 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] = { .ops = &broadwell_rt286_ops, .dpcm_playback = 1, .dpcm_capture = 1, - SND_SOC_DAILINK_REG(dummy, codec, dummy), + SND_SOC_DAILINK_REG(ssp0_port, codec, platform), }, };
The legacy driver uses dummy cpu_dai and platform, SOF requires actual values to bind.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/intel/boards/bdw-rt5650.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/bdw-rt5650.c b/sound/soc/intel/boards/bdw-rt5650.c index ba3fc1ef900a..1a302436d450 100644 --- a/sound/soc/intel/boards/bdw-rt5650.c +++ b/sound/soc/intel/boards/bdw-rt5650.c @@ -223,6 +223,14 @@ SND_SOC_DAILINK_DEF(platform, SND_SOC_DAILINK_DEF(be, DAILINK_COMP_ARRAY(COMP_CODEC("i2c-10EC5650:00", "rt5645-aif1")));
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL) +SND_SOC_DAILINK_DEF(ssp0_port, + DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port"))); +#else +SND_SOC_DAILINK_DEF(ssp0_port, + DAILINK_COMP_ARRAY(COMP_DUMMY())); +#endif + static struct snd_soc_dai_link bdw_rt5650_dais[] = { /* Front End DAI links */ { @@ -256,7 +264,7 @@ static struct snd_soc_dai_link bdw_rt5650_dais[] = { .dpcm_playback = 1, .dpcm_capture = 1, .init = bdw_rt5650_init, - SND_SOC_DAILINK_REG(dummy, be, dummy), + SND_SOC_DAILINK_REG(ssp0_port, be, platform), }, };
The patch
ASoC: Intel: bdw-rt5650: change cpu_dai and platform components for SOF
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.6
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
From a40acc6bfcebb59fdf6a71c4740c46880ed5c0b4 Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Date: Fri, 10 Jan 2020 17:57:47 -0600 Subject: [PATCH] ASoC: Intel: bdw-rt5650: change cpu_dai and platform components for SOF
The legacy driver uses dummy cpu_dai and platform, SOF requires actual values to bind.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20200110235751.3404-5-pierre-louis.bossart@linux.i... Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/boards/bdw-rt5650.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/bdw-rt5650.c b/sound/soc/intel/boards/bdw-rt5650.c index ba3fc1ef900a..1a302436d450 100644 --- a/sound/soc/intel/boards/bdw-rt5650.c +++ b/sound/soc/intel/boards/bdw-rt5650.c @@ -223,6 +223,14 @@ SND_SOC_DAILINK_DEF(platform, SND_SOC_DAILINK_DEF(be, DAILINK_COMP_ARRAY(COMP_CODEC("i2c-10EC5650:00", "rt5645-aif1")));
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL) +SND_SOC_DAILINK_DEF(ssp0_port, + DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port"))); +#else +SND_SOC_DAILINK_DEF(ssp0_port, + DAILINK_COMP_ARRAY(COMP_DUMMY())); +#endif + static struct snd_soc_dai_link bdw_rt5650_dais[] = { /* Front End DAI links */ { @@ -256,7 +264,7 @@ static struct snd_soc_dai_link bdw_rt5650_dais[] = { .dpcm_playback = 1, .dpcm_capture = 1, .init = bdw_rt5650_init, - SND_SOC_DAILINK_REG(dummy, be, dummy), + SND_SOC_DAILINK_REG(ssp0_port, be, platform), }, };
From: Bard liao yung-chuan.liao@linux.intel.com
We will reinit DSP in a loop when it fails to initialize the first time, as recommended. So, it is not an error before we finally give up. And reorder the trace to make it more readable.
Signed-off-by: Bard liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-loader.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index 1782f5092639..8852184a2569 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -328,13 +328,13 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev) if (!ret) break;
- dev_err(sdev->dev, "error: Error code=0x%x: FW status=0x%x\n", + dev_dbg(sdev->dev, "iteration %d of Core En/ROM load failed: %d\n", + i, ret); + dev_dbg(sdev->dev, "Error code=0x%x: FW status=0x%x\n", snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_SRAM_REG_ROM_ERROR), snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_SRAM_REG_ROM_STATUS)); - dev_err(sdev->dev, "error: iteration %d of Core En/ROM load failed: %d\n", - i, ret); }
if (i == HDA_FW_BOOT_ATTEMPTS) {
The patch
ASoC: SOF: Intel: lower print level to dbg if we will reinit DSP
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From ceca2197b2f1af4cb6e3f32bb7bd2879943406ef Mon Sep 17 00:00:00 2001
From: Bard liao yung-chuan.liao@linux.intel.com Date: Fri, 10 Jan 2020 17:57:48 -0600 Subject: [PATCH] ASoC: SOF: Intel: lower print level to dbg if we will reinit DSP
We will reinit DSP in a loop when it fails to initialize the first time, as recommended. So, it is not an error before we finally give up. And reorder the trace to make it more readable.
Signed-off-by: Bard liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20200110235751.3404-6-pierre-louis.bossart@linux.i... Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sof/intel/hda-loader.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index b1783360fe10..bae7ac3581e5 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -329,13 +329,13 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev) if (!ret) break;
- dev_err(sdev->dev, "error: Error code=0x%x: FW status=0x%x\n", + dev_dbg(sdev->dev, "iteration %d of Core En/ROM load failed: %d\n", + i, ret); + dev_dbg(sdev->dev, "Error code=0x%x: FW status=0x%x\n", snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_SRAM_REG_ROM_ERROR), snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_SRAM_REG_ROM_STATUS)); - dev_err(sdev->dev, "error: iteration %d of Core En/ROM load failed: %d\n", - i, ret); }
if (i == HDA_FW_BOOT_ATTEMPTS) {
From: Kai Vehmanen kai.vehmanen@linux.intel.com
Any app using ALSA OSS emulation on top of SOF will fail to error from OSS SNDCTL_DSP_SETFMT ioctl. Reported initially as an issue with xournalpp (application using PortAudio with an OSS backend), but applies more generally to other apps using OSS as well.
Problem is caused by SOF PCM not supporting repeated calls to hw_params(), without matching calls to pcm_free(). This is however exactly what the ALSA OSS PCM code is doing when it is handling the OSS ioctls.
The problem will lead to leaking of DSP resources and eventual failure of DSP PCM_PARAMS IPC.
BugLink: https://github.com/thesofproject/linux/issues/1510 Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/pcm.c | 53 ++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 22 deletions(-)
diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 9bb6388742e1..314f3095c12f 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -92,7 +92,27 @@ void snd_sof_pcm_period_elapsed(struct snd_pcm_substream *substream) } EXPORT_SYMBOL(snd_sof_pcm_period_elapsed);
-/* this may get called several times by oss emulation */ +static int sof_pcm_dsp_pcm_free(struct snd_pcm_substream *substream, + struct snd_sof_dev *sdev, + struct snd_sof_pcm *spcm) +{ + struct sof_ipc_stream stream; + struct sof_ipc_reply reply; + int ret; + + stream.hdr.size = sizeof(stream); + stream.hdr.cmd = SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_PCM_FREE; + stream.comp_id = spcm->stream[substream->stream].comp_id; + + /* send IPC to the DSP */ + ret = sof_ipc_tx_message(sdev->ipc, stream.hdr.cmd, &stream, + sizeof(stream), &reply, sizeof(reply)); + if (!ret) + spcm->prepared[substream->stream] = false; + + return ret; +} + static int sof_pcm_hw_params(struct snd_soc_component *component, struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) @@ -113,6 +133,16 @@ static int sof_pcm_hw_params(struct snd_soc_component *component, if (!spcm) return -EINVAL;
+ /* + * Handle repeated calls to hw_params() without free_pcm() in + * between. At least ALSA OSS emulation depends on this. + */ + if (spcm->prepared[substream->stream]) { + ret = sof_pcm_dsp_pcm_free(substream, sdev, spcm); + if (ret < 0) + return ret; + } + dev_dbg(component->dev, "pcm: hw params stream %d dir %d\n", spcm->pcm.pcm_id, substream->stream);
@@ -201,27 +231,6 @@ static int sof_pcm_hw_params(struct snd_soc_component *component, return ret; }
-static int sof_pcm_dsp_pcm_free(struct snd_pcm_substream *substream, - struct snd_sof_dev *sdev, - struct snd_sof_pcm *spcm) -{ - struct sof_ipc_stream stream; - struct sof_ipc_reply reply; - int ret; - - stream.hdr.size = sizeof(stream); - stream.hdr.cmd = SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_PCM_FREE; - stream.comp_id = spcm->stream[substream->stream].comp_id; - - /* send IPC to the DSP */ - ret = sof_ipc_tx_message(sdev->ipc, stream.hdr.cmd, &stream, - sizeof(stream), &reply, sizeof(reply)); - if (!ret) - spcm->prepared[substream->stream] = false; - - return ret; -} - static int sof_pcm_hw_free(struct snd_soc_component *component, struct snd_pcm_substream *substream) {
The patch
ASoC: SOF: fix PCM playback through ALSA OSS emulation
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.6
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
From cfe8191b1bbf2b41581b63eb97e56cd6bc3c34a1 Mon Sep 17 00:00:00 2001
From: Kai Vehmanen kai.vehmanen@linux.intel.com Date: Fri, 10 Jan 2020 17:57:49 -0600 Subject: [PATCH] ASoC: SOF: fix PCM playback through ALSA OSS emulation
Any app using ALSA OSS emulation on top of SOF will fail to error from OSS SNDCTL_DSP_SETFMT ioctl. Reported initially as an issue with xournalpp (application using PortAudio with an OSS backend), but applies more generally to other apps using OSS as well.
Problem is caused by SOF PCM not supporting repeated calls to hw_params(), without matching calls to pcm_free(). This is however exactly what the ALSA OSS PCM code is doing when it is handling the OSS ioctls.
The problem will lead to leaking of DSP resources and eventual failure of DSP PCM_PARAMS IPC.
BugLink: https://github.com/thesofproject/linux/issues/1510 Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20200110235751.3404-7-pierre-louis.bossart@linux.i... Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sof/pcm.c | 53 ++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 22 deletions(-)
diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 9bb6388742e1..314f3095c12f 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -92,7 +92,27 @@ void snd_sof_pcm_period_elapsed(struct snd_pcm_substream *substream) } EXPORT_SYMBOL(snd_sof_pcm_period_elapsed);
-/* this may get called several times by oss emulation */ +static int sof_pcm_dsp_pcm_free(struct snd_pcm_substream *substream, + struct snd_sof_dev *sdev, + struct snd_sof_pcm *spcm) +{ + struct sof_ipc_stream stream; + struct sof_ipc_reply reply; + int ret; + + stream.hdr.size = sizeof(stream); + stream.hdr.cmd = SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_PCM_FREE; + stream.comp_id = spcm->stream[substream->stream].comp_id; + + /* send IPC to the DSP */ + ret = sof_ipc_tx_message(sdev->ipc, stream.hdr.cmd, &stream, + sizeof(stream), &reply, sizeof(reply)); + if (!ret) + spcm->prepared[substream->stream] = false; + + return ret; +} + static int sof_pcm_hw_params(struct snd_soc_component *component, struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) @@ -113,6 +133,16 @@ static int sof_pcm_hw_params(struct snd_soc_component *component, if (!spcm) return -EINVAL;
+ /* + * Handle repeated calls to hw_params() without free_pcm() in + * between. At least ALSA OSS emulation depends on this. + */ + if (spcm->prepared[substream->stream]) { + ret = sof_pcm_dsp_pcm_free(substream, sdev, spcm); + if (ret < 0) + return ret; + } + dev_dbg(component->dev, "pcm: hw params stream %d dir %d\n", spcm->pcm.pcm_id, substream->stream);
@@ -201,27 +231,6 @@ static int sof_pcm_hw_params(struct snd_soc_component *component, return ret; }
-static int sof_pcm_dsp_pcm_free(struct snd_pcm_substream *substream, - struct snd_sof_dev *sdev, - struct snd_sof_pcm *spcm) -{ - struct sof_ipc_stream stream; - struct sof_ipc_reply reply; - int ret; - - stream.hdr.size = sizeof(stream); - stream.hdr.cmd = SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_PCM_FREE; - stream.comp_id = spcm->stream[substream->stream].comp_id; - - /* send IPC to the DSP */ - ret = sof_ipc_tx_message(sdev->ipc, stream.hdr.cmd, &stream, - sizeof(stream), &reply, sizeof(reply)); - if (!ret) - spcm->prepared[substream->stream] = false; - - return ret; -} - static int sof_pcm_hw_free(struct snd_soc_component *component, struct snd_pcm_substream *substream) {
From: Kai Vehmanen kai.vehmanen@linux.intel.com
In case system has multiple HDA controllers, it can happen that same HDA codec driver is used for codecs of multiple controllers. In this case, SOF may fail to probe the HDA driver and SOF initialization fails.
SOF HDA code currently relies that a call to request_module() will also run device matching logic to attach driver to the codec instance. However if driver for another HDA controller was already loaded and it already loaded the HDA codec driver, this breaks current logic in SOF. In this case the request_module() SOF does becomes a no-op and HDA Codec driver is not attached to the codec instance sitting on the HDA bus SOF is controlling. Typical scenario would be a system with both external and internal GPUs, with driver of the external GPU loaded first.
Fix this by adding similar logic as is used in legacy HDA driver where an explicit device_attach() call is done after request_module().
Also add logic to propagate errors reported by device_attach() back to caller. This also works in the case where drivers are not built as modules.
Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-codec.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c index 5514e6191ba4..78dfd5f5c034 100644 --- a/sound/soc/sof/intel/hda-codec.c +++ b/sound/soc/sof/intel/hda-codec.c @@ -24,19 +24,18 @@ #define IDISP_VID_INTEL 0x80860000
/* load the legacy HDA codec driver */ -#ifdef MODULE -static void hda_codec_load_module(struct hda_codec *codec) +static int hda_codec_load_module(struct hda_codec *codec) { +#ifdef MODULE char alias[MODULE_NAME_LEN]; const char *module = alias;
snd_hdac_codec_modalias(&codec->core, alias, sizeof(alias)); dev_dbg(&codec->core.dev, "loading codec module: %s\n", module); request_module(module); -} -#else -static void hda_codec_load_module(struct hda_codec *codec) {} #endif + return device_attach(hda_codec_dev(codec)); +}
/* enable controller wake up event for all codecs with jack connectors */ void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev) @@ -124,10 +123,16 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address, if (hda_codec_use_common_hdmi || (resp & 0xFFFF0000) != IDISP_VID_INTEL) { hdev->type = HDA_DEV_LEGACY; - hda_codec_load_module(&hda_priv->codec); + ret = hda_codec_load_module(&hda_priv->codec); + /* + * handle ret==0 (no driver bound) as an error, but pass + * other return codes without modification + */ + if (ret == 0) + ret = -ENOENT; }
- return 0; + return ret; #else hdev = devm_kzalloc(sdev->dev, sizeof(*hdev), GFP_KERNEL); if (!hdev)
From: Kai Vehmanen kai.vehmanen@linux.intel.com
In case system has multiple HDA codecs, and codec probe fails for at least one but not all codecs, driver will end up cancelling a non-initialized timer context upon driver removal.
Call trace of typical case:
[ 60.593646] WARNING: CPU: 1 PID: 1147 at kernel/workqueue.c:3032 __flush_work+0x18b/0x1a0 [...] [ 60.593670] __cancel_work_timer+0x11f/0x1a0 [ 60.593673] hdac_hda_dev_remove+0x25/0x30 [snd_soc_hdac_hda] [ 60.593674] device_release_driver_internal+0xe0/0x1c0 [ 60.593675] bus_remove_device+0xd6/0x140 [ 60.593677] device_del+0x175/0x3e0 [ 60.593679] ? widget_tree_free.isra.7+0x90/0xb0 [snd_hda_core] [ 60.593680] snd_hdac_device_unregister+0x34/0x50 [snd_hda_core] [ 60.593682] snd_hdac_ext_bus_device_remove+0x2a/0x60 [snd_hda_ext_core] [ 60.593684] hda_dsp_remove+0x26/0x100 [snd_sof_intel_hda_common] [ 60.593686] snd_sof_device_remove+0x84/0xa0 [snd_sof] [ 60.593687] sof_pci_remove+0x10/0x30 [snd_sof_pci] [ 60.593689] pci_device_remove+0x36/0xb0
Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/codecs/hdac_hda.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c index 4e0f4afe6ddc..de003acb1951 100644 --- a/sound/soc/codecs/hdac_hda.c +++ b/sound/soc/codecs/hdac_hda.c @@ -604,7 +604,9 @@ static int hdac_hda_dev_remove(struct hdac_device *hdev) struct hdac_hda_priv *hda_pvt;
hda_pvt = dev_get_drvdata(&hdev->dev); - cancel_delayed_work_sync(&hda_pvt->codec.jackpoll_work); + if (hda_pvt && hda_pvt->codec.registered) + cancel_delayed_work_sync(&hda_pvt->codec.jackpoll_work); + return 0; }
The patch
ASoC: hdac_hda: Fix error in driver removal after failed probe
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From 552b1a85da9f63856e7e341b81c16e0e078204f1 Mon Sep 17 00:00:00 2001
From: Kai Vehmanen kai.vehmanen@linux.intel.com Date: Fri, 10 Jan 2020 17:57:51 -0600 Subject: [PATCH] ASoC: hdac_hda: Fix error in driver removal after failed probe
In case system has multiple HDA codecs, and codec probe fails for at least one but not all codecs, driver will end up cancelling a non-initialized timer context upon driver removal.
Call trace of typical case:
[ 60.593646] WARNING: CPU: 1 PID: 1147 at kernel/workqueue.c:3032 __flush_work+0x18b/0x1a0 [...] [ 60.593670] __cancel_work_timer+0x11f/0x1a0 [ 60.593673] hdac_hda_dev_remove+0x25/0x30 [snd_soc_hdac_hda] [ 60.593674] device_release_driver_internal+0xe0/0x1c0 [ 60.593675] bus_remove_device+0xd6/0x140 [ 60.593677] device_del+0x175/0x3e0 [ 60.593679] ? widget_tree_free.isra.7+0x90/0xb0 [snd_hda_core] [ 60.593680] snd_hdac_device_unregister+0x34/0x50 [snd_hda_core] [ 60.593682] snd_hdac_ext_bus_device_remove+0x2a/0x60 [snd_hda_ext_core] [ 60.593684] hda_dsp_remove+0x26/0x100 [snd_sof_intel_hda_common] [ 60.593686] snd_sof_device_remove+0x84/0xa0 [snd_sof] [ 60.593687] sof_pci_remove+0x10/0x30 [snd_sof_pci] [ 60.593689] pci_device_remove+0x36/0xb0
Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20200110235751.3404-9-pierre-louis.bossart@linux.i... Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/hdac_hda.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c index 6803d39e09a5..43110151e928 100644 --- a/sound/soc/codecs/hdac_hda.c +++ b/sound/soc/codecs/hdac_hda.c @@ -588,7 +588,9 @@ static int hdac_hda_dev_remove(struct hdac_device *hdev) struct hdac_hda_priv *hda_pvt;
hda_pvt = dev_get_drvdata(&hdev->dev); - cancel_delayed_work_sync(&hda_pvt->codec.jackpoll_work); + if (hda_pvt && hda_pvt->codec.registered) + cancel_delayed_work_sync(&hda_pvt->codec.jackpoll_work); + return 0; }
participants (2)
-
Mark Brown -
Pierre-Louis Bossart