[PATCH v2 0/3] ASoC: fix SoundWire audio on HP OmniBook X Flip 14 (ACP7.1 + TAS2783)

[PATCH v2 0/3] ASoC: fix SoundWire audio on HP OmniBook X Flip 14 (ACP7.1 + TAS2783)

[jml]

On the HP OmniBook X Flip 14-kc0xxx (board 8EA1, ACP PCI 1022:15e2 rev
0x71) no analog audio works: no speakers, no headset jack, no headset
microphone. Only HDMI and the PDM microphone come up.

The machine has two TI TAS2783 SmartAmps on SoundWire link 0 (unique_id
0x9 and 0xC) and a Realtek RT712-VB on link 1. A matching entry already
exists in snd_soc_acpi_amd_acp70_sdw_machines[], and the TAS2783
codec_info entry already carries .component_name, so those parts are
done. Three problems remain.

Patch 1 is board specific: the BIOS never exposes
"acp-audio-config-flag", so the ACP config lookup falls back to
FLAG_AMD_LEGACY_ONLY_DMIC, snd_pci_ps declines to probe and snd_acp_pci
binds instead. SoundWire never comes up at all, so the existing machine
entry is never reached.

Patches 2 and 3 are not board specific:

 - 2 makes the driver find factory calibration data that some OEMs store
   under a different vendor GUID. Affected machines run the amplifiers
   on generic defaults with nothing to indicate anything is wrong; the
   symptom is distortion at higher volumes.

 - 3 fixes silent amplifiers after system sleep for aggregated TAS2783
   pairs.

All three have been running on the affected machine: speakers, headset
jack and microphones work, and audio survives suspend/resume.

To be clear about what was tested where: the changes were developed and
runtime tested against v7.0 (Ubuntu 7.0.0-30), then rebased onto
for-next. The rebased form is checkpatch clean and builds warning-free
(sound/soc/codecs/tas2783-sdw.o and sound/soc/amd/acp-config.o), but the
runtime testing was done with the v7.0 form of the same changes - I have
not booted a for-next kernel on this machine.

Two things I did not turn into patches, because I would rather ask.

First, firmware lookup. tas_generate_fw_name() builds a flat name, so on
this board it asks for "8EA1-0-0x9.bin" and then "8EA1-0-9.bin".
linux-firmware ships these blobs as ti/audio/tas2783/8EXX-X-9.bin and
ti/audio/tas2783/8EXX-X-C.bin, i.e. under a subdirectory and with what
look like wildcard components for the board id and the link. The driver
prepends no path and implements no 8EXX fallback, so the firmware is
never found and playback fails with -EINVAL. Locally I worked around it
with symlinks under /lib/firmware/updates. Should the driver prepend
ti/audio/tas2783/ and fall back to the 8EXX-X- form, or is
linux-firmware expected to ship per-board names?

Second, channel order. tas2783x2_l0u9c_adr[] maps unique_id 0xC to
spk_l_endpoint and 0x9 to spk_r_endpoint. On this unit that appears to
be the wrong way round: with 0x9 as left and 0xC as right, a
left-channel-only tone comes out of the left speaker. I have only one
machine, so I have not sent a patch for it - can anyone with the same
board confirm?

jml (3):
  ASoC: amd: acp-config: override ACP config flag on HP OmniBook X Flip
    14
  ASoC: tas2783: also look for calibration data under the OEM GUID
  ASoC: tas2783: re-initialise amplifiers from .startup after system
    sleep

 sound/soc/amd/acp-config.c     |   7 ++
 sound/soc/codecs/tas2783-sdw.c | 130 ++++++++++++++++++++++++++++-----
 2 files changed, 120 insertions(+), 17 deletions(-)

-- 
2.53.0

jml (3):
  ASoC: amd: acp-config: override ACP config flag on HP OmniBook X Flip
    14
  ASoC: tas2783: also look for calibration data under the OEM GUID
  ASoC: tas2783: re-initialise amplifiers from .startup after system
    sleep

 sound/soc/amd/acp-config.c     |   7 ++
 sound/soc/codecs/tas2783-sdw.c | 130 ++++++++++++++++++++++++++++-----
 2 files changed, 120 insertions(+), 17 deletions(-)

-- 
2.53.0

[PATCH v2 1/3] ASoC: amd: acp-config: override ACP config flag on HP OmniBook X Flip 14

[jml]

The HP OmniBook X Flip 14-kc0xxx (board 8EA1, ACP 7.1) does not expose
the "acp-audio-config-flag" ACPI property on its ACP device, so
snd_amd_acp_acpi_find_config() falls back to its
FLAG_AMD_LEGACY_ONLY_DMIC default. snd_pci_ps then declines to probe and
snd_acp_pci binds instead, which has no SoundWire support at all.

The result is a machine with only the PDM microphone and HDMI: the
SoundWire codecs driving the speakers and the headset jack never come
up, even though a matching entry for this board already exists in
snd_soc_acpi_amd_acp70_sdw_machines[].

Add the board to acp70_acpi_flag_override_table so the config lookup
returns 0 and snd_pci_ps can claim the device.

Signed-off-by: jml <jml@carbonforge.ai>
---
 sound/soc/amd/acp-config.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sound/soc/amd/acp-config.c b/sound/soc/amd/acp-config.c
index 3021dc498..a545dd679 100644
--- a/sound/soc/amd/acp-config.c
+++ b/sound/soc/amd/acp-config.c
@@ -30,6 +30,13 @@ static const struct dmi_system_id acp70_acpi_flag_override_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "HN7306EA"),
 		},
 	},
+	{
+		/* HP OmniBook X Flip 14-kc0xxx (Krackan Point, ACP 7.1) */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "HP"),
+			DMI_MATCH(DMI_BOARD_NAME, "8EA1"),
+		},
+	},
 	{
 		/* ASUS Zenbook S16 UM5606GA (Strix Point, ACP 7.0) */
 		.matches = {
-- 
2.53.0

Re: [PATCH v2 1/3] ASoC: amd: acp-config: override ACP config flag on HP OmniBook X Flip 14

[Mukunda,Vijendar]

On 8/24/26 09:11, jml wrote:
> [You don't often get email from jml@carbonforge.ai. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> The HP OmniBook X Flip 14-kc0xxx (board 8EA1, ACP 7.1) does not expose
> the "acp-audio-config-flag" ACPI property on its ACP device, so
> snd_amd_acp_acpi_find_config() falls back to its
> FLAG_AMD_LEGACY_ONLY_DMIC default. snd_pci_ps then declines to probe and
> snd_acp_pci binds instead, which has no SoundWire support at all.
>
> The result is a machine with only the PDM microphone and HDMI: the
> SoundWire codecs driving the speakers and the headset jack never come
> up, even though a matching entry for this board already exists in
> snd_soc_acpi_amd_acp70_sdw_machines[].
>
> Add the board to acp70_acpi_flag_override_table so the config lookup
> returns 0 and snd_pci_ps can claim the device.
>
> Signed-off-by: jml <jml@carbonforge.ai>
Reviewed-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
> ---
>   sound/soc/amd/acp-config.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/sound/soc/amd/acp-config.c b/sound/soc/amd/acp-config.c
> index 3021dc498..a545dd679 100644
> --- a/sound/soc/amd/acp-config.c
> +++ b/sound/soc/amd/acp-config.c
> @@ -30,6 +30,13 @@ static const struct dmi_system_id acp70_acpi_flag_override_table[] = {
>                          DMI_MATCH(DMI_PRODUCT_NAME, "HN7306EA"),
>                  },
>          },
> +       {
> +               /* HP OmniBook X Flip 14-kc0xxx (Krackan Point, ACP 7.1) */
> +               .matches = {
> +                       DMI_MATCH(DMI_BOARD_VENDOR, "HP"),
> +                       DMI_MATCH(DMI_BOARD_NAME, "8EA1"),
> +               },
> +       },
>          {
>                  /* ASUS Zenbook S16 UM5606GA (Strix Point, ACP 7.0) */
>                  .matches = {
> --
> 2.53.0
>

[PATCH v2 2/3] ASoC: tas2783: also look for calibration data under the OEM GUID

[jml]

tas2783_update_calibdata() looks the SmartAmpCalibrationData UEFI
variable up under a single vendor GUID. At least one OEM stores the very
same payload under a different GUID: on the HP OmniBook X Flip
14-kc0xxx the variable is named SmartAmpCalibrationData but sits under
53559579-8753-4f5c-9130-e82acfb8d893.

efi.get_variable() matches on name *and* GUID, so the lookup fails, the
driver takes the silent dev_dbg("No calibration data in UEFI") path, and
the amplifiers run on generic defaults. The factory data is present and
valid - magic 2783, spk_count 2, CRC32 verified, per-speaker records
carrying the unique_ids of the two amplifiers fitted - it is simply
never read. Audibly this shows up as distortion at higher volumes, since
the protection algorithm has no real R0/TLim values for the drivers.

Try both GUIDs before giving up.

Signed-off-by: jml <jml@carbonforge.ai>
---
 sound/soc/codecs/tas2783-sdw.c | 52 +++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 17 deletions(-)

diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
index eaebb0ebb..2ca634a6e 100644
--- a/sound/soc/codecs/tas2783-sdw.c
+++ b/sound/soc/codecs/tas2783-sdw.c
@@ -46,6 +46,15 @@
 #define TAS2783_PROBE_TIMEOUT 5000
 #define TAS2783_CALI_GUID EFI_GUID(0x1f52d2a1, 0xbb3a, 0x457d, 0xbc, \
 				   0x09, 0x43, 0xa3, 0xf4, 0x31, 0x0a, 0x92)
+/*
+ * Some OEMs store the same SmartAmpCalibrationData payload under a different
+ * vendor GUID. Seen on the HP OmniBook X Flip 14-kc0xxx (board 8EA1): the
+ * variable name matches but the GUID does not, so the factory per-speaker
+ * calibration was silently ignored and the protection algorithm ran on
+ * defaults.
+ */
+#define TAS2783_CALI_GUID_ALT EFI_GUID(0x53559579, 0x8753, 0x4f5c, 0x91, \
+				       0x30, 0xe8, 0x2a, 0xcf, 0xb8, 0xd8, 0x93)
 
 static const u32 tas2783_cali_reg[] = {
 	TAS2783_CAL_R0,
@@ -702,8 +711,10 @@ static void tas2783_set_calib_params_to_device(struct tas2783_prv *tas_dev, u32
 
 static s32 tas2783_update_calibdata(struct tas2783_prv *tas_dev)
 {
-	efi_guid_t efi_guid = TAS2783_CALI_GUID;
-	u32 attr, i, *tmp_val;
+	static const efi_guid_t efi_guids[] = {
+		TAS2783_CALI_GUID, TAS2783_CALI_GUID_ALT };
+	efi_guid_t efi_guid;
+	u32 attr, i, g, *tmp_val;
 	unsigned long size;
 	s32 ret;
 	efi_status_t status;
@@ -717,22 +728,29 @@ static s32 tas2783_update_calibdata(struct tas2783_prv *tas_dev)
 	 * In some cases, the calibration is performed in Windows,
 	 * and data was saved in UEFI. Linux can access it.
 	 */
-	for (i = 0; i < ARRAY_SIZE(efi_names); i++) {
-		size = 0;
-		status = efi.get_variable(efi_names[i], &efi_guid, &attr,
-					  &size, NULL);
-		if (size > TAS2783_CALIB_DATA_SZ) {
-			dev_err(tas_dev->dev, "cali data too large\n");
-			break;
-		}
-
-		tas_dev->cali_data.read_sz = size;
-		if (status == EFI_BUFFER_TOO_SMALL) {
+	status = EFI_NOT_FOUND;
+	for (g = 0; g < ARRAY_SIZE(efi_guids); g++) {
+		efi_guid = efi_guids[g];
+		for (i = 0; i < ARRAY_SIZE(efi_names); i++) {
+			size = 0;
 			status = efi.get_variable(efi_names[i], &efi_guid, &attr,
-							&tas_dev->cali_data.read_sz,
-							tas_dev->cali_data.data);
-			dev_dbg(tas_dev->dev, "cali get %lu bytes result:%ld\n",
-				tas_dev->cali_data.read_sz, status);
+						  &size, NULL);
+			if (size > TAS2783_CALIB_DATA_SZ) {
+				dev_err(tas_dev->dev, "cali data too large\n");
+				status = EFI_NOT_FOUND;
+				break;
+			}
+
+			tas_dev->cali_data.read_sz = size;
+			if (status == EFI_BUFFER_TOO_SMALL) {
+				status = efi.get_variable(efi_names[i], &efi_guid, &attr,
+							  &tas_dev->cali_data.read_sz,
+							  tas_dev->cali_data.data);
+				dev_dbg(tas_dev->dev, "cali get %lu bytes result:%ld\n",
+					tas_dev->cali_data.read_sz, status);
+			}
+			if (status == EFI_SUCCESS)
+				break;
 		}
 		if (status == EFI_SUCCESS)
 			break;
-- 
2.53.0

[PATCH v2 3/3] ASoC: tas2783: re-initialise amplifiers from .startup after system sleep

[jml]

After system sleep the amplifiers come back silent. The firmware is in
fact re-downloaded correctly when the peripheral re-enumerates - the
download reports success and both fw_dl_success and hw_init are set -
but the part still produces no output. Only a firmware download
performed shortly before playback restores sound.

Doing that from .hw_params is not safe when the parts are used as an
aggregated pair on a single SoundWire stream, as they are on the HP
OmniBook X Flip 14-kc0xxx: by then the first amplifier's port
configuration is already in place, and resetting the second one knocks
the first out. Exactly one of the two speakers then works, and which one
varies from resume to resume.

Do the re-initialisation from .startup instead. ASoC calls .startup for
every codec DAI of a link before any of them reaches .hw_params, so both
amplifiers can be re-initialised while the stream is still unconfigured.

Gate it on a flag set from a PM notifier rather than on fw_dl_success:
whether a given peripheral re-enumerates before or after the notifier
runs is a race, so fw_dl_success is not a reliable indication that the
DSP still holds its program.

Signed-off-by: jml <jml@carbonforge.ai>
---
 sound/soc/codecs/tas2783-sdw.c | 78 ++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
index 2ca634a6e..4c850a857 100644
--- a/sound/soc/codecs/tas2783-sdw.c
+++ b/sound/soc/codecs/tas2783-sdw.c
@@ -24,6 +24,7 @@
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
+#include <linux/suspend.h>
 #include <linux/wait.h>
 #include <linux/soundwire/sdw.h>
 #include <linux/soundwire/sdw_registers.h>
@@ -112,8 +113,13 @@ struct tas2783_prv {
 	bool fw_dl_success;
 	/* use fallback fw name */
 	bool fw_use_fallback;
+	/* set across system sleep, cleared once the part has been re-inited */
+	bool force_reinit;
+	struct notifier_block pm_nb;
 };
 
+static s32 tas_io_init(struct device *dev, struct sdw_slave *slave);
+
 static const struct reg_default tas2783_reg_default[] = {
 	{TAS2783_AMP_LEVEL, 0x28},
 	{TASDEV_REG_SDW(0, 0, 0x04), 0x21},
@@ -1051,7 +1057,47 @@ static s32 tas_sdw_pcm_hw_free(struct snd_pcm_substream *substream,
 			    TAS2783_SDCA_POW_STATE_OFF);
 }
 
+/*
+ * The amplifier loses its DSP program across system sleep and only a
+ * firmware download performed shortly before playback makes it produce
+ * sound again. Doing that from .hw_params is not safe when the parts are
+ * used as an aggregated pair on one SoundWire stream: by then the first
+ * amplifier's port configuration is already in place, and resetting the
+ * second one knocks the first out, so exactly one of the two speakers
+ * works and which one varies from resume to resume.
+ *
+ * .startup runs for every codec DAI of the link before any of them
+ * reaches .hw_params, so both amplifiers can be re-initialised while the
+ * stream is still unconfigured.
+ */
+static int tas_sdw_startup(struct snd_pcm_substream *substream,
+			   struct snd_soc_dai *dai)
+{
+	struct snd_soc_component *component = dai->component;
+	struct tas2783_prv *tas_dev =
+		snd_soc_component_get_drvdata(component);
+	s32 ret;
+
+	if (!tas_dev->force_reinit && tas_dev->fw_dl_success)
+		return 0;
+
+	tas_dev->hw_init = false;
+	regcache_cache_only(tas_dev->regmap, false);
+
+	ret = tas_io_init(tas_dev->dev, tas_dev->sdw_peripheral);
+	if (ret || !tas_dev->fw_dl_success) {
+		dev_err(tas_dev->dev, "re-init before playback failed, err=%d\n",
+			ret);
+		return -EIO;
+	}
+
+	tas_dev->force_reinit = false;
+
+	return 0;
+}
+
 static const struct snd_soc_dai_ops tas_dai_ops = {
+	.startup	= tas_sdw_startup,
 	.hw_params	= tas_sdw_hw_params,
 	.hw_free	= tas_sdw_pcm_hw_free,
 	.set_stream	= tas_set_sdw_stream,
@@ -1391,6 +1437,32 @@ static void tas_remove(struct tas2783_prv *tas_dev)
 	snd_soc_unregister_component(tas_dev->dev);
 }
 
+/*
+ * fw_dl_success on its own is not a reliable indication that the DSP still
+ * holds its program: whether a given peripheral re-enumerates before or
+ * after this notifier runs is a race, and a download performed while the
+ * bus is still settling does not stick.
+ */
+static int tas2783_pm_notify(struct notifier_block *nb,
+			     unsigned long action, void *data)
+{
+	struct tas2783_prv *tas_dev =
+		container_of(nb, struct tas2783_prv, pm_nb);
+
+	switch (action) {
+	case PM_SUSPEND_PREPARE:
+	case PM_HIBERNATION_PREPARE:
+	case PM_POST_SUSPEND:
+	case PM_POST_HIBERNATION:
+		tas_dev->force_reinit = true;
+		break;
+	default:
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
 static s32 tas_sdw_probe(struct sdw_slave *peripheral,
 			 const struct sdw_device_id *id)
 {
@@ -1462,6 +1534,11 @@ static s32 tas_sdw_probe(struct sdw_slave *peripheral,
 	/* keep in cache until the device is fully initialized */
 	regcache_cache_only(regmap, true);
 	tas_dev->regmap = regmap;
+
+	tas_dev->pm_nb.notifier_call = tas2783_pm_notify;
+	if (register_pm_notifier(&tas_dev->pm_nb))
+		dev_warn(dev, "pm notifier registration failed");
+
 	return tas_init(tas_dev);
 }
 
@@ -1469,6 +1546,7 @@ static void tas_sdw_remove(struct sdw_slave *peripheral)
 {
 	struct tas2783_prv *tas_dev = dev_get_drvdata(&peripheral->dev);
 
+	unregister_pm_notifier(&tas_dev->pm_nb);
 	pm_runtime_disable(tas_dev->dev);
 	tas_remove(tas_dev);
 	mutex_destroy(&tas_dev->calib_lock);
-- 
2.53.0