[PATCH 0/5] Improve AMD ACP Vangogh audio support for Steam Deck OLED

[PATCH 0/5] Improve AMD ACP Vangogh audio support for Steam Deck OLED

[Cristian Ciocaltea]

This patch series provides several fixes and improvements to AMD ACP drivers
targeting the Vangogh platform, as found on the Valve's new Steam Deck OLED.

Although in theory the board should have been supported by both SOF and legacy
ACP drivers, as of next-20231208 the audio seems to be completely broken.

Please note this only restores the legacy support, while SOF will be handled in
a separate series.

Cristian Ciocaltea (5):
  ASoC: amd: vangogh: Drop conflicting ACPI-based probing
  ASoC: amd: vangogh: Allow probing ACP PCI when SOF is disabled
  ASoC: amd: vangogh: Switch to {RUNTIME,SYSTEM_SLEEP}_PM_OPS
  ASoC: amd: acp-config: Add missing MODULE_DESCRIPTION
  ASoC: amd: acp: Add missing MODULE_DESCRIPTION in mach-common

 sound/soc/amd/acp-config.c          |  1 +
 sound/soc/amd/acp/acp-mach-common.c |  1 +
 sound/soc/amd/vangogh/acp5x-mach.c  | 35 +++++++++++------------------
 sound/soc/amd/vangogh/pci-acp5x.c   | 19 +++++++++-------
 4 files changed, 26 insertions(+), 30 deletions(-)

-- 
2.43.0

[PATCH 1/5] ASoC: amd: vangogh: Drop conflicting ACPI-based probing

[Cristian Ciocaltea]

The Vangogh machine driver variant based on the MAX98388 amplifier, as
found on Valve's Steam Deck OLED, relies on probing via an ACPI match
table.  This worked fine until commit 197b1f7f0df1 ("ASoC: amd: Add new
dmi entries to config entry") enabled SOF support for the target machine
(i.e. Galileo product), causing the sound card to enter the deferred
probe state indefinitely:

$ cat /sys/kernel/debug/devices_deferred
AMDI8821:00	acp5x_mach: Register card (acp5x-max98388) failed

The issue is related to commit e89f45edb747 ("ASoC: amd: vangogh: Add
check for acp config flags in vangogh platform"), which tries to
mitigate potential conflicts between SOF and generic ACP Vangogh
drivers, due to sharing the PCI device IDs.

However, the solution is effective only if the machine driver is
directly probed by pci-acp5x through platform_device_register_full().

Hence, remove the conflicting ACPI based probing and rely exclusively on
DMI quirks for sound card setup.

Fixes: dba22efd0d17 ("ASoC: amd: vangogh: Add support for NAU8821/MAX98388 variant")
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 sound/soc/amd/vangogh/acp5x-mach.c | 35 +++++++++++-------------------
 1 file changed, 13 insertions(+), 22 deletions(-)

diff --git a/sound/soc/amd/vangogh/acp5x-mach.c b/sound/soc/amd/vangogh/acp5x-mach.c
index de4b478a983d..7878e061ecb9 100644
--- a/sound/soc/amd/vangogh/acp5x-mach.c
+++ b/sound/soc/amd/vangogh/acp5x-mach.c
@@ -439,7 +439,15 @@ static const struct dmi_system_id acp5x_vg_quirk_table[] = {
 		.matches = {
 			DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Valve"),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Jupiter"),
-		}
+		},
+		.driver_data = (void *)&acp5x_8821_35l41_card,
+	},
+	{
+		.matches = {
+			DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Valve"),
+			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Galileo"),
+		},
+		.driver_data = (void *)&acp5x_8821_98388_card,
 	},
 	{}
 };
@@ -452,25 +460,15 @@ static int acp5x_probe(struct platform_device *pdev)
 	struct snd_soc_card *card;
 	int ret;
 
-	card = (struct snd_soc_card *)device_get_match_data(dev);
-	if (!card) {
-		/*
-		 * This is normally the result of directly probing the driver
-		 * in pci-acp5x through platform_device_register_full(), which
-		 * is necessary for the CS35L41 variant, as it doesn't support
-		 * ACPI probing and relies on DMI quirks.
-		 */
-		dmi_id = dmi_first_match(acp5x_vg_quirk_table);
-		if (!dmi_id)
-			return -ENODEV;
-
-		card = &acp5x_8821_35l41_card;
-	}
+	dmi_id = dmi_first_match(acp5x_vg_quirk_table);
+	if (!dmi_id || !dmi_id->driver_data)
+		return -ENODEV;
 
 	machine = devm_kzalloc(dev, sizeof(*machine), GFP_KERNEL);
 	if (!machine)
 		return -ENOMEM;
 
+	card = dmi_id->driver_data;
 	card->dev = dev;
 	platform_set_drvdata(pdev, card);
 	snd_soc_card_set_drvdata(card, machine);
@@ -482,17 +480,10 @@ static int acp5x_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct acpi_device_id acp5x_acpi_match[] = {
-	{ "AMDI8821", (kernel_ulong_t)&acp5x_8821_98388_card },
-	{},
-};
-MODULE_DEVICE_TABLE(acpi, acp5x_acpi_match);
-
 static struct platform_driver acp5x_mach_driver = {
 	.driver = {
 		.name = DRV_NAME,
 		.pm = &snd_soc_pm_ops,
-		.acpi_match_table = acp5x_acpi_match,
 	},
 	.probe = acp5x_probe,
 };
-- 
2.43.0

[PATCH 2/5] ASoC: amd: vangogh: Allow probing ACP PCI when SOF is disabled

[Cristian Ciocaltea]

Since commit e89f45edb747 ("ASoC: amd: vangogh: Add check for acp config
flags in vangogh platform"), the Vangogh ACP PCI driver could not be used
anymore for boards which happen to have a matching entry in acp-config
list.

Commit f18818eb0dbe ("ASoC: amd: vangogh: Add condition check for acp
config flag") slightly changed the behaviour to permit loading the
driver if AMD_LEGACY flag is set.  However, for AMD_SOF flag the probing
is still denied, even if SOF support is disabled in kernel
configuration.

While this helps preventing conflicts between SOF and generic ACP
drivers, there are cases where a fallback to the generic non-SOF support
would still be needed or useful, e.g. SOF firmware is not available or
doesn't work properly, SOF driver is broken or doesn't provide full
support for a particular hardware, or simply for testing/debugging the
alternative solution.  A real-life example is Steam Deck OLED, which
works with both drivers.

Prevent returning from probe() when ACP config indicates SOF support for
the current board *and* the Vangogh SOF driver is not enabled in kernel
configuration.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 sound/soc/amd/vangogh/pci-acp5x.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/sound/soc/amd/vangogh/pci-acp5x.c b/sound/soc/amd/vangogh/pci-acp5x.c
index 3826443d77b9..10755c07949c 100644
--- a/sound/soc/amd/vangogh/pci-acp5x.c
+++ b/sound/soc/amd/vangogh/pci-acp5x.c
@@ -130,9 +130,13 @@ static int snd_acp5x_probe(struct pci_dev *pci,
 	int ret, i;
 	u32 addr, val;
 
-	/* Return if acp config flag is defined */
+	/*
+	 * Return if ACP config flag is defined, except when board
+	 * supports SOF while it is not being enabled in kernel config.
+	 */
 	flag = snd_amd_acp_find_config(pci);
-	if (flag != FLAG_AMD_LEGACY)
+	if (flag != FLAG_AMD_LEGACY &&
+	    (flag != FLAG_AMD_SOF || IS_ENABLED(CONFIG_SND_SOC_SOF_AMD_VANGOGH)))
 		return -ENODEV;
 
 	irqflags = IRQF_SHARED;
-- 
2.43.0

[PATCH 3/5] ASoC: amd: vangogh: Switch to {RUNTIME,SYSTEM_SLEEP}_PM_OPS

[Cristian Ciocaltea]

Replace the old SET_{RUNTIME,SYSTEM_SLEEP}_PM_OPS() helpers with their
modern alternatives and drop the now unnecessary __maybe_unused
qualifier in the suspend and resume functions.

Additionally, make use of pm_ptr() to ensure the PM ops are dropped when
building with CONFIG_PM disabled.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 sound/soc/amd/vangogh/pci-acp5x.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/sound/soc/amd/vangogh/pci-acp5x.c b/sound/soc/amd/vangogh/pci-acp5x.c
index 10755c07949c..af56ff09f02a 100644
--- a/sound/soc/amd/vangogh/pci-acp5x.c
+++ b/sound/soc/amd/vangogh/pci-acp5x.c
@@ -264,7 +264,7 @@ static int snd_acp5x_probe(struct pci_dev *pci,
 	return ret;
 }
 
-static int __maybe_unused snd_acp5x_suspend(struct device *dev)
+static int snd_acp5x_suspend(struct device *dev)
 {
 	int ret;
 	struct acp5x_dev_data *adata;
@@ -279,7 +279,7 @@ static int __maybe_unused snd_acp5x_suspend(struct device *dev)
 	return ret;
 }
 
-static int __maybe_unused snd_acp5x_resume(struct device *dev)
+static int snd_acp5x_resume(struct device *dev)
 {
 	int ret;
 	struct acp5x_dev_data *adata;
@@ -294,9 +294,8 @@ static int __maybe_unused snd_acp5x_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops acp5x_pm = {
-	SET_RUNTIME_PM_OPS(snd_acp5x_suspend,
-			   snd_acp5x_resume, NULL)
-	SET_SYSTEM_SLEEP_PM_OPS(snd_acp5x_suspend, snd_acp5x_resume)
+	RUNTIME_PM_OPS(snd_acp5x_suspend, snd_acp5x_resume, NULL)
+	SYSTEM_SLEEP_PM_OPS(snd_acp5x_suspend, snd_acp5x_resume)
 };
 
 static void snd_acp5x_remove(struct pci_dev *pci)
@@ -332,7 +331,7 @@ static struct pci_driver acp5x_driver  = {
 	.probe = snd_acp5x_probe,
 	.remove = snd_acp5x_remove,
 	.driver = {
-		.pm = &acp5x_pm,
+		.pm = pm_ptr(&acp5x_pm),
 	}
 };
 
-- 
2.43.0

[PATCH 4/5] ASoC: amd: acp-config: Add missing MODULE_DESCRIPTION

[Cristian Ciocaltea]

Add the missing MODULE_DESCRIPTION() to avoid the following warning when
building with W=1:

WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/amd/snd-acp-config.o

Fixes: f1bdd8d385a8 ("ASoC: amd: Add module to determine ACP configuration")
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 sound/soc/amd/acp-config.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/amd/acp-config.c b/sound/soc/amd/acp-config.c
index 5c8cf75be009..42c2322cd11b 100644
--- a/sound/soc/amd/acp-config.c
+++ b/sound/soc/amd/acp-config.c
@@ -321,4 +321,5 @@ struct snd_soc_acpi_mach snd_soc_acpi_amd_acp63_sof_machines[] = {
 };
 EXPORT_SYMBOL(snd_soc_acpi_amd_acp63_sof_machines);
 
+MODULE_DESCRIPTION("AMD ACP Machine Configuration Module");
 MODULE_LICENSE("Dual BSD/GPL");
-- 
2.43.0

[PATCH 5/5] ASoC: amd: acp: Add missing MODULE_DESCRIPTION in mach-common

[Cristian Ciocaltea]

Add a MODULE_DESCRIPTION() in the generic ACP machine driver to avoid
the following warning when building with W=1:

WARNING: modpost: missing MODULE_DESCRIPTION() in sound/soc/amd/acp/snd-acp-mach.o

Fixes: d4c750f2c7d4 ("ASoC: amd: acp: Add generic machine driver support for ACP cards")
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 sound/soc/amd/acp/acp-mach-common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/amd/acp/acp-mach-common.c b/sound/soc/amd/acp/acp-mach-common.c
index f7bcf210f0fd..c90ec3419247 100644
--- a/sound/soc/amd/acp/acp-mach-common.c
+++ b/sound/soc/amd/acp/acp-mach-common.c
@@ -1773,4 +1773,5 @@ int acp_legacy_dai_links_create(struct snd_soc_card *card)
 }
 EXPORT_SYMBOL_NS_GPL(acp_legacy_dai_links_create, SND_SOC_AMD_MACH);
 
+MODULE_DESCRIPTION("AMD ACP Common Machine driver");
 MODULE_LICENSE("GPL v2");
-- 
2.43.0

Re: [PATCH 0/5] Improve AMD ACP Vangogh audio support for Steam Deck OLED

[Emil Velikov]

On 2023/12/09, Cristian Ciocaltea wrote:
> This patch series provides several fixes and improvements to AMD ACP drivers
> targeting the Vangogh platform, as found on the Valve's new Steam Deck OLED.
> 
> Although in theory the board should have been supported by both SOF and legacy
> ACP drivers, as of next-20231208 the audio seems to be completely broken.
> 
> Please note this only restores the legacy support, while SOF will be handled in
> a separate series.
> 
> Cristian Ciocaltea (5):
>   ASoC: amd: vangogh: Drop conflicting ACPI-based probing
>   ASoC: amd: vangogh: Allow probing ACP PCI when SOF is disabled
>   ASoC: amd: vangogh: Switch to {RUNTIME,SYSTEM_SLEEP}_PM_OPS
>   ASoC: amd: acp-config: Add missing MODULE_DESCRIPTION
>   ASoC: amd: acp: Add missing MODULE_DESCRIPTION in mach-common
> 

If it were me, I'd drop the Fixes tag in the latter two patches since
the warning is harmless and as-is it'll get into the -stable
maintainers' list, who are already pretty swamped with patches.

Regardless, for the series:
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

HTH
Emil

Re: [PATCH 0/5] Improve AMD ACP Vangogh audio support for Steam Deck OLED

[Cristian Ciocaltea]

On 12/11/23 17:12, Emil Velikov wrote:
> On 2023/12/09, Cristian Ciocaltea wrote:
>> This patch series provides several fixes and improvements to AMD ACP drivers
>> targeting the Vangogh platform, as found on the Valve's new Steam Deck OLED.
>>
>> Although in theory the board should have been supported by both SOF and legacy
>> ACP drivers, as of next-20231208 the audio seems to be completely broken.
>>
>> Please note this only restores the legacy support, while SOF will be handled in
>> a separate series.
>>
>> Cristian Ciocaltea (5):
>>   ASoC: amd: vangogh: Drop conflicting ACPI-based probing
>>   ASoC: amd: vangogh: Allow probing ACP PCI when SOF is disabled
>>   ASoC: amd: vangogh: Switch to {RUNTIME,SYSTEM_SLEEP}_PM_OPS
>>   ASoC: amd: acp-config: Add missing MODULE_DESCRIPTION
>>   ASoC: amd: acp: Add missing MODULE_DESCRIPTION in mach-common
>>
> 
> If it were me, I'd drop the Fixes tag in the latter two patches since
> the warning is harmless and as-is it'll get into the -stable
> maintainers' list, who are already pretty swamped with patches.

Sure, I can drop them if there is a need for v2.

> Regardless, for the series:
> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

Thanks for the review,
Cristian

> HTH
> Emil

Re: [PATCH 0/5] Improve AMD ACP Vangogh audio support for Steam Deck OLED

[Mark Brown]

On Sat, 09 Dec 2023 22:32:18 +0200, Cristian Ciocaltea wrote:
> This patch series provides several fixes and improvements to AMD ACP drivers
> targeting the Vangogh platform, as found on the Valve's new Steam Deck OLED.
> 
> Although in theory the board should have been supported by both SOF and legacy
> ACP drivers, as of next-20231208 the audio seems to be completely broken.
> 
> Please note this only restores the legacy support, while SOF will be handled in
> a separate series.
> 
> [...]

Applied to

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

Thanks!

[1/5] ASoC: amd: vangogh: Drop conflicting ACPI-based probing
      commit: ddd1ee12a8fb6e4d6f86eddeba64c135eee56623
[2/5] ASoC: amd: vangogh: Allow probing ACP PCI when SOF is disabled
      commit: 2cef11ec3dfd5f14d8ddef917682408ed01e5805
[3/5] ASoC: amd: vangogh: Switch to {RUNTIME,SYSTEM_SLEEP}_PM_OPS
      commit: 78d3924675d4e076faa5600b48b8565fcb135ee0
[4/5] ASoC: amd: acp-config: Add missing MODULE_DESCRIPTION
      commit: 6e202e758b4b8d85dfb909c8eb710db8c6160303
[5/5] ASoC: amd: acp: Add missing MODULE_DESCRIPTION in mach-common
      commit: 576f3aef47f42f368db08fd5e3f49880c4493bf5

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