* [PATCH v3 0/2] ASoC: fix audio on the Microsoft Surface Pro 11 (Intel)
@ 2026-08-30 15:15 Sergey Lebedev
2026-08-30 15:15 ` [PATCH v3 1/2] ASoC: rt1320: run the initialisation preset on the first hardware init Sergey Lebedev
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Sergey Lebedev @ 2026-08-30 15:15 UTC (permalink / raw)
To: Vinod Koul, Bard Liao, Pierre-Louis Bossart, Oder Chiou,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai
Cc: Bard Liao, Amaan Lalani, linux-sound, linux-kernel
The Microsoft Surface Pro 11 for Business (Intel, Lunar Lake) has no working
audio under Linux: the speakers are silent while every layer reports success.
These two patches fix it, and the machine then works with the stock
sof-soundwire UCM profile and the shipped topologies - no board quirk, no new
match entry, no local configuration of any kind.
Changes since v2: none to the code. v2 was sent in reply to the v1 series,
which Mark Brown asked me not to do because it buries the current patches in
an old thread and confuses tooling. He is right, and this is the same two
patches sent as their own thread. Sorry for the extra round.
v1: https://lore.kernel.org/linux-sound/20260804225853.31585-1-lsa.uz@pm.me/
v2: https://lore.kernel.org/linux-sound/20260830084500.6123-1-lsa.uz@pm.me/
Changes since v1: two of the three patches are replaced by a single DMI quirk,
which is what Bard Liao and Pierre-Louis Bossart asked for in review, and which
is both smaller and safer than what it replaces.
The board carries one physical RT1320 amplifier on SoundWire link 0 and
describes it twice:
SWRA _ADR 0x000030025D132000 SDCA class 0
SWRB _ADR 0x000030025D132001 SDCA class 1
Identical apart from the class id: same link, same manufacturer, part and
version, same unique id 0. The part reports class 1, so only SWRB ever
enumerates. SWRA stays UNATTACHED on every boot and on every firmware version
tested, including the November 2025 bundle, and the firmware is signed vendor
firmware we cannot have corrected at the source.
That ghost broke two things at once. It consumed an amplifier index, so the
real part was named "rt1320-2" and the stock UCM enabled switches on a device
that is not there; and its endpoints reached create_sdw_dailink(), which builds
DAI link names from link id and function type alone, so its SmartMic collided
with the real one and the card failed to register at all with -EEXIST.
v1 fixed those two symptoms in two places, each with its own way of noticing
the ghost. Removing the ghost at enumeration instead fixes both at once and
needs no runtime presence test:
1/2 rt1320: the amplifier's preset never runs, because the driver waits for
FUNCTION_NEEDS_INITIALIZATION and this part never sets it. rt712-sdca and
rt722-sdca already handle this by also running the preset on the first
hardware init; rt1320 is the odd one out. One line, unchanged from v1.
2/2 dmi-quirks: remap the ghost _ADR to zero so sdw_acpi_find_slaves() never
creates the peripheral, as ghost_realtek and global_ghost_adr already do.
Matched on DMI_PRODUCT_SKU, not the product name, so a later batch with a
different RT1320 version - and therefore a different _ADR - cannot be
caught by a remap it was never verified against.
Testing. Surface Pro 11 for Business (Intel Core Ultra 7 268V). Verified on the
machine's own kernel, 7.0.0-30 (Ubuntu 26.04), with 2/2 backported to that
tree: its dmi-quirks.c predates ghost_realtek, but the table entry is identical
and the mechanism is unchanged - slave.c drops a peripheral whose overridden
_ADR is zero in both trees. 1/2 is byte-identical to v1's 1/3, which was built
and booted on 7.1.0-rc7.
- /sys/bus/soundwire/devices/ shows only sdw:0:0:025d:1320:01; the class-0
ghost is gone
- amplifier named rt1320-1, controls "rt1320-1 OT23 L/R Switch"
- card registers as sof-soundwire, 4 playback + 1 capture
- no -EEXIST, no -61 link startup errors
- speakers audible, internal microphone captures signal
- stock alsa-ucm-conf and firmware-sof-signed, no local configuration
- Secure Boot enabled with module signature enforcement, no rejections
Related work already on the list, so nobody reviews this twice. Amaan Lalani
sent a series for the same machine on 2026-08-28:
[PATCH 0/2] ASoC: fix SoundWire enumeration on Surface Pro 11
https://lore.kernel.org/linux-sound/BY5PR02MB63701A05A58EF6B4D6CAAC31A0AC2@BY5PR02MB6370.namprd02.prod.outlook.com/
In substance it is my v1 2/3 and 3/3: name-prefix reuse for _ADR entries that
differ only in class id, plus a runtime SDW_SLAVE_UNATTACHED test. The review
Bard and Pierre-Louis gave me therefore applies to it unchanged - it detects
the ghost at runtime rather than removing it at enumeration. Their version
hooks both the count and parse paths where mine hooked only parse, and carries
Fixes: tags, which is worth keeping if that approach is ever preferred.
There is also a body of downstream work for this machine in a linux-surface
fork (https://github.com/linux-surface/linux-surface/pull/1990), whose audio
fix takes the traditional shape of a DMI-gated machine entry. I wrote up the
comparison, including why the generic path appears to serve the microphone
better here, in a message on the v2 thread:
https://lore.kernel.org/linux-sound/20260830122606.47028-1-lsa.uz@pm.me/
I have not run that branch on this device, so that part is their reports
against my measurements rather than one machine tested both ways. I have
offered to do it and will report the result.
One thing the review process turned up that is worth recording. The v1 cover
letter said the DAI link name collision was "no longer reachable on this
machine and we cannot demonstrate it". That was wrong: during v2 testing a boot
where the quirk did not take effect reproduced it exactly, and it is fatal.
create_sdw_dailink()'s naming scheme is still not unique in general. This series
does not address that - it removes the ghost before the naming code sees it -
and I am happy to send a separate patch if you would like it fixed.
checkpatch --strict is clean on both.
Sergey Lebedev (2):
ASoC: rt1320: run the initialisation preset on the first hardware init
soundwire: dmi-quirks: drop the ghost RT1320 on the Surface Pro 11
(Intel)
drivers/soundwire/dmi-quirks.c | 28 ++++++++++++++++++++++++++++
sound/soc/codecs/rt1320-sdw.c | 2 +-
2 files changed, 29 insertions(+), 1 deletion(-)
base-commit: 7e9e0409cd57924c4099090879154300c07b8643
--
2.50.1 (Apple Git-155)
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v3 1/2] ASoC: rt1320: run the initialisation preset on the first hardware init 2026-08-30 15:15 [PATCH v3 0/2] ASoC: fix audio on the Microsoft Surface Pro 11 (Intel) Sergey Lebedev @ 2026-08-30 15:15 ` Sergey Lebedev 2026-08-30 15:15 ` [PATCH v3 2/2] soundwire: dmi-quirks: drop the ghost RT1320 on the Surface Pro 11 (Intel) Sergey Lebedev 2026-08-30 17:47 ` [PATCH v3 0/2] ASoC: fix audio on the Microsoft " Sergey Lebedev 2 siblings, 0 replies; 9+ messages in thread From: Sergey Lebedev @ 2026-08-30 15:15 UTC (permalink / raw) To: Vinod Koul, Bard Liao, Pierre-Louis Bossart, Oder Chiou, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai Cc: Bard Liao, Amaan Lalani, linux-sound, linux-kernel rt1320_io_init() applies the vendor initialisation preset only when the amplifier's SDCA function status has FUNCTION_NEEDS_INITIALIZATION set: if ((amp_func_status & FUNCTION_NEEDS_INITIALIZATION)) { Its two sibling drivers guard the same write differently, also running the preset on the first hardware init: rt712-sdca.c: if ((amp_func_status & FUNCTION_NEEDS_INITIALIZATION) || (!rt712->first_hw_init)) { rt722-sdca.c: if ((amp_func_status & FUNCTION_NEEDS_INITIALIZATION) || (!rt722->first_hw_init)) { On the Microsoft Surface Pro 11 (Intel) the RT1320 never sets that bit. Its function status reads back 0x41 on every boot, cold or warm: rt1320-sdca sdw:0:0:025d:1320:01: rt1320_io_init amp func_status=0x41 which is NEWLY_ATTACHED | FUNCTION_HAS_BEEN_RESET: the function reports that it has been reset and does not consider itself in need of initialisation. Bit 5 is never set, so the preset never runs, rt1320_vc_preset() and the MCU patch load are skipped, and the amplifier is left unprogrammed. rt712 and rt722 would have run it via their first_hw_init fallback. Add the same fallback. With it rt1320_vc_preset() executes and the amplifier reports RT1320_KR0_INT_READY=0x1f where previously it did not. Signed-off-by: Sergey Lebedev <lsa.uz@pm.me> --- sound/soc/codecs/rt1320-sdw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/codecs/rt1320-sdw.c b/sound/soc/codecs/rt1320-sdw.c index 13493b85f..d1f3b160a 100644 --- a/sound/soc/codecs/rt1320-sdw.c +++ b/sound/soc/codecs/rt1320-sdw.c @@ -1900,7 +1900,7 @@ static int rt1320_io_init(struct device *dev, struct sdw_slave *slave) dev_dbg(dev, "%s amp func_status=0x%x\n", __func__, amp_func_status); /* initialization write */ - if ((amp_func_status & FUNCTION_NEEDS_INITIALIZATION)) { + if ((amp_func_status & FUNCTION_NEEDS_INITIALIZATION) || !rt1320->first_hw_init) { switch (rt1320->dev_id) { case RT1320_DEV_ID: if (rt1320->version_id < RT1320_VC) -- 2.50.1 (Apple Git-155) ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/2] soundwire: dmi-quirks: drop the ghost RT1320 on the Surface Pro 11 (Intel) 2026-08-30 15:15 [PATCH v3 0/2] ASoC: fix audio on the Microsoft Surface Pro 11 (Intel) Sergey Lebedev 2026-08-30 15:15 ` [PATCH v3 1/2] ASoC: rt1320: run the initialisation preset on the first hardware init Sergey Lebedev @ 2026-08-30 15:15 ` Sergey Lebedev 2026-08-30 21:12 ` Sergey Lebedev 2026-09-04 7:51 ` Pierre-Louis Bossart 2026-08-30 17:47 ` [PATCH v3 0/2] ASoC: fix audio on the Microsoft " Sergey Lebedev 2 siblings, 2 replies; 9+ messages in thread From: Sergey Lebedev @ 2026-08-30 15:15 UTC (permalink / raw) To: Vinod Koul, Bard Liao, Pierre-Louis Bossart, Oder Chiou, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai Cc: Bard Liao, Amaan Lalani, linux-sound, linux-kernel The Microsoft Surface Pro 11 for Business (Intel, Lunar Lake) carries a single physical RT1320 amplifier on link 0 and describes it twice: SWRA _ADR 0x000030025D132000 SDCA class 0 SWRB _ADR 0x000030025D132001 SDCA class 1 Identical apart from the class id: same link, same manufacturer, part and version, same unique id 0. The part reports class 1, so only SWRB enumerates. SWRA never attaches on any boot or firmware version tested, including the November 2025 bundle. The ghost is not harmless. It consumes an amplifier index in find_acpi_adr_device(), so the real part is named "rt1320-2" and the stock sof-soundwire UCM profile enables switches on a device that is not there. Its endpoints also reach create_sdw_dailink(), which builds DAI link names from link id and function type alone; the ghost declares the same SmartMic function as the real part, so the card fails to register at all: sysfs: cannot create duplicate filename '.../sof_sdw/SDW0-Capture-SmartMic' kobject_add_internal failed for SDW0-Capture-SmartMic with -EEXIST sof_sdw sof_sdw: probe with driver sof_sdw failed with error -12 Remap its _ADR to zero so sdw_acpi_find_slaves() never creates the peripheral, which is how ghost devices are handled already - see ghost_realtek and global_ghost_adr in this file. Both failures then disappear, because neither path is reached. Matched on DMI_PRODUCT_SKU rather than the product name. A later batch of the same model could carry a different RT1320 version, which would change the _ADR; keying on the SKU keeps this remap to the hardware it was verified on, and follows dell_sku_0A3E above. Suggested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev> Suggested-by: Bard Liao <bard.liao@intel.com> Signed-off-by: Sergey Lebedev <lsa.uz@pm.me> --- drivers/soundwire/dmi-quirks.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/drivers/soundwire/dmi-quirks.c b/drivers/soundwire/dmi-quirks.c index d75037558..8b3c5c8d1 100644 --- a/drivers/soundwire/dmi-quirks.c +++ b/drivers/soundwire/dmi-quirks.c @@ -111,6 +111,25 @@ static const struct adr_remap ghost_realtek[] = { {} }; +/* + * The Microsoft Surface Pro 11 (Intel) describes its single physical RT1320 + * amplifier twice on link 0, as two _ADR entries differing only in SDCA class + * id and sharing the same unique id. Only the class 1 entry enumerates; the + * class 0 entry is a ghost. + * + * Matched on the product SKU rather than the product name: a later batch of the + * same model could carry a different RT1320 version, which would change the + * _ADR. Keying on the SKU keeps this remap to the hardware it was verified on. + */ +static const struct adr_remap microsoft_sp11_intel[] = { + /* ghost rt1320 on link0 */ + { + 0x000030025d132000ull, + 0x0000000000000000ull + }, + {} +}; + static const struct dmi_system_id adr_remap_quirk_table[] = { /* TGL devices */ { @@ -207,6 +226,15 @@ static const struct dmi_system_id adr_remap_quirk_table[] = { }, .driver_data = (void *)ghost_realtek, }, + /* LNL devices */ + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"), + DMI_EXACT_MATCH(DMI_PRODUCT_SKU, + "Surface_Pro_11th_Edition_With_Intel_For_Business_2103") + }, + .driver_data = (void *)microsoft_sp11_intel, + }, {} }; -- 2.50.1 (Apple Git-155) ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] soundwire: dmi-quirks: drop the ghost RT1320 on the Surface Pro 11 (Intel) 2026-08-30 15:15 ` [PATCH v3 2/2] soundwire: dmi-quirks: drop the ghost RT1320 on the Surface Pro 11 (Intel) Sergey Lebedev @ 2026-08-30 21:12 ` Sergey Lebedev 2026-09-04 7:51 ` Pierre-Louis Bossart 1 sibling, 0 replies; 9+ messages in thread From: Sergey Lebedev @ 2026-08-30 21:12 UTC (permalink / raw) To: Vinod Koul, Bard Liao, Pierre-Louis Bossart, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai Cc: Bard Liao, Peter Ujfalusi, Amaan Lalani, linux-sound, linux-kernel A second Surface Pro 11 (Intel) has reproduced the ghost independently and confirms this patch removes it. The report is from leihua-dev on the linux-surface tracker; I am relaying it here because it is a different unit with a different BIOS from mine, and because this series has had no review of the code since 2026-08-05. Unit: Surface Pro for Business 11th Edition with Intel, SKU Surface_Pro_11th_Edition_With_Intel_For_Business_2103, Core Ultra 7 266V, BIOS 17.100.143. Mine is the same SKU with a Core Ultra 7 268V and a different BIOS. Before the quirk, both _ADRs present and ACPI enumerating each SDCA function twice: acpi device:28: find_sdca_function: SDCA function SmartMic (type 3) at 0x2 acpi device:29: find_sdca_function: SDCA function SmartAmp (type 1) at 0x4 acpi device:2b: find_sdca_function: SDCA function SmartMic (type 3) at 0x2 acpi device:2c: find_sdca_function: SDCA function SmartAmp (type 1) at 0x4 with sdw:0:0:025d:1320:00 permanently UNATTACHED. After the patch only the 2b/2c pair remains. Their DMI_PRODUCT_SKU matches the quirk key exactly. That is the evidence I did not have when I chose SKU over product-name matching in v2 — at the time it was one machine and an argument. Two limits on the report, stated so nobody has to discover them: - They run a 6.18 tree, so the dmi-quirks.c hunk was context-adapted; that branch has no ghost_realtek table yet. The logic is unchanged. - They applied five changes at once and did not bisect. That does not weaken this patch specifically: the duplicate _ADR and the doubled enumeration are both observable before any topology loads, and their before/after shows them changing. Full report, with their kernel and userspace versions: https://github.com/linux-surface/linux-surface/issues/1876#issuecomment-5470909030 Separately, one hazard that surfaced in the same comparison and may be worth knowing on the list. Backporting a machine entry whose .sof_tplg_filename names a dummy topology onto a pre-6.19 kernel yields no sound card at all — probe fails with -ENOENT before anything registers, speakers included — because 225d70b80745 ("ASoC: SOF: don't check the existence of dummy topology") is not there to skip the existence check. Upstream is fine: sof_test_topology_file() tests the name and has no idea where the name came from, so from 6.19 a machine entry and the generic path are treated alike. It is purely a backport hazard, and it cost that reporter a boot with no audio at all. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] soundwire: dmi-quirks: drop the ghost RT1320 on the Surface Pro 11 (Intel) 2026-08-30 15:15 ` [PATCH v3 2/2] soundwire: dmi-quirks: drop the ghost RT1320 on the Surface Pro 11 (Intel) Sergey Lebedev 2026-08-30 21:12 ` Sergey Lebedev @ 2026-09-04 7:51 ` Pierre-Louis Bossart 2026-09-04 12:36 ` Sergey Lebedev 1 sibling, 1 reply; 9+ messages in thread From: Pierre-Louis Bossart @ 2026-09-04 7:51 UTC (permalink / raw) To: Sergey Lebedev, Vinod Koul, Bard Liao, Oder Chiou, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai Cc: Bard Liao, Amaan Lalani, linux-sound, linux-kernel On 8/30/26 17:15, Sergey Lebedev wrote: > The Microsoft Surface Pro 11 for Business (Intel, Lunar Lake) carries a > single physical RT1320 amplifier on link 0 and describes it twice: > > SWRA _ADR 0x000030025D132000 SDCA class 0 > SWRB _ADR 0x000030025D132001 SDCA class 1 > > Identical apart from the class id: same link, same manufacturer, part and > version, same unique id 0. The part reports class 1, so only SWRB > enumerates. SWRA never attaches on any boot or firmware version tested, > including the November 2025 bundle. > > The ghost is not harmless. It consumes an amplifier index in > find_acpi_adr_device(), so the real part is named "rt1320-2" and the stock > sof-soundwire UCM profile enables switches on a device that is not there. > Its endpoints also reach create_sdw_dailink(), which builds DAI link names > from link id and function type alone; the ghost declares the same SmartMic > function as the real part, so the card fails to register at all: > > sysfs: cannot create duplicate filename > '.../sof_sdw/SDW0-Capture-SmartMic' > kobject_add_internal failed for SDW0-Capture-SmartMic with -EEXIST > sof_sdw sof_sdw: probe with driver sof_sdw failed with error -12 > > Remap its _ADR to zero so sdw_acpi_find_slaves() never creates the > peripheral, which is how ghost devices are handled already - see > ghost_realtek and global_ghost_adr in this file. Both failures then > disappear, because neither path is reached. > > Matched on DMI_PRODUCT_SKU rather than the product name. A later batch of > the same model could carry a different RT1320 version, which would change > the _ADR; keying on the SKU keeps this remap to the hardware it was > verified on, and follows dell_sku_0A3E above. > > Suggested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev> > Suggested-by: Bard Liao <bard.liao@intel.com> > Signed-off-by: Sergey Lebedev <lsa.uz@pm.me> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev> the other 1/2 patch should be skipped for now until the problem with the NEEDS_INITIALIZATION flag is understood. > --- > drivers/soundwire/dmi-quirks.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/drivers/soundwire/dmi-quirks.c b/drivers/soundwire/dmi-quirks.c > index d75037558..8b3c5c8d1 100644 > --- a/drivers/soundwire/dmi-quirks.c > +++ b/drivers/soundwire/dmi-quirks.c > @@ -111,6 +111,25 @@ static const struct adr_remap ghost_realtek[] = { > {} > }; > > +/* > + * The Microsoft Surface Pro 11 (Intel) describes its single physical RT1320 > + * amplifier twice on link 0, as two _ADR entries differing only in SDCA class > + * id and sharing the same unique id. Only the class 1 entry enumerates; the > + * class 0 entry is a ghost. > + * > + * Matched on the product SKU rather than the product name: a later batch of the > + * same model could carry a different RT1320 version, which would change the > + * _ADR. Keying on the SKU keeps this remap to the hardware it was verified on. > + */ > +static const struct adr_remap microsoft_sp11_intel[] = { > + /* ghost rt1320 on link0 */ > + { > + 0x000030025d132000ull, > + 0x0000000000000000ull > + }, > + {} > +}; > + > static const struct dmi_system_id adr_remap_quirk_table[] = { > /* TGL devices */ > { > @@ -207,6 +226,15 @@ static const struct dmi_system_id adr_remap_quirk_table[] = { > }, > .driver_data = (void *)ghost_realtek, > }, > + /* LNL devices */ > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"), > + DMI_EXACT_MATCH(DMI_PRODUCT_SKU, > + "Surface_Pro_11th_Edition_With_Intel_For_Business_2103") > + }, > + .driver_data = (void *)microsoft_sp11_intel, > + }, > {} > }; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] soundwire: dmi-quirks: drop the ghost RT1320 on the Surface Pro 11 (Intel) 2026-09-04 7:51 ` Pierre-Louis Bossart @ 2026-09-04 12:36 ` Sergey Lebedev 2026-09-04 14:09 ` Pierre-Louis Bossart 0 siblings, 1 reply; 9+ messages in thread From: Sergey Lebedev @ 2026-09-04 12:36 UTC (permalink / raw) To: Pierre-Louis Bossart, Vinod Koul, Bard Liao, Oder Chiou, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai Cc: Bard Liao, Shuming Fan, Amaan Lalani, linux-sound, linux-kernel Pierre-Louis, Thank you for the review of 2/2. On 1/2 I have to correct the record, and the confusion is mine. That patch is already upstream: 9b30521074f0 ("ASoC: rt1320: run the initialisation preset on the first hardware init") applied from v1 on 2026-08-10, merged in asoc-v7.3 on 2026-08-18 — twelve days before I re-sent it as v3 1/2. I rebuilt the series after the v1 review and did not notice that one of the three had already been taken; the cover letter says only that 1/2 is byte-identical to v1's 1/3, which reads as "unchanged" rather than "already applied". So there is nothing to skip: my own 2026-08-30 request not to apply it, and your agreement with it, both concern code that shipped three weeks ago, and AUTOSEL has since proposed it for 6.18. That is my error and I am sorry for the wasted attention. Which leaves two real questions: whether it belongs in the tree, and what the flag is actually doing. I have answers for both now, and they point in opposite directions. The change is a revert, not a workaround ---------------------------------------- The fallback is what the driver shipped with: bad0a07a7e61 rt1320 driver added 2024-05-21 v6.11 if ((amp_func_status & FUNCTION_NEEDS_INITIALIZATION) || (!rt1320->first_hw_init)) { f465d10cd731 "Add support for version C" 2024-09-02 v6.12 if ((amp_func_status & FUNCTION_NEEDS_INITIALIZATION)) { 9b30521074f0 restored 2026-08-04 v7.3 f465d10cd731 is +2124/-23 and its commit message reads, in full, "This patch added the support for version C". That condition was rewritten because its body had to grow a VC branch, and the `||` half did not survive the edit. Nothing in the message, and nothing I can find on the list, suggests it was meant to go. The siblings have never been without it, and all four commits are Shuming Fan's, which is why I have put him on Cc — he is better placed than I am to say whether the removal was meant: bad0a07a7e61 rt1320 2024-05-21 gate introduced, with the fallback 936abb09c1c7 rt712 2024-06-20 gate introduced, with the fallback (x3) f465d10cd731 rt1320 2024-09-02 fallback dropped 91f4ca732495 rt722 2025-04-16 gate introduced, with the fallback (x3) In rt712 and rt722 the fallback is present in the very first line that ever consulted the flag there — three times each, for DMIC, jack and amp. Six live instances across two drivers, and rt1320 the only one that lost it, eight months after being the first to have it. So I do not think anything about the flag needs to be understood before that change is allowed to stand: it restores the driver's original behaviour and makes it consistent with its siblings. What it lacks is the tag saying so, which I should have found in August and did not: Fixes: f465d10cd731 ("ASoC: rt1320: Add support for version C") That would put the window at v6.12 through v7.2. Whether it is worth backporting past 6.18 depends on whether any machine actually lands in the gap, and — see below — I can no longer demonstrate one, so I am not asking for that. What the flag is doing, measured today -------------------------------------- I went back to the hardware rather than argue from July's logs. Surface Pro 11 for Business, Ubuntu 7.0.0-30, with snd_soc_rt1320_sdw.dyndbg="func rt1320_io_init +p" on the kernel command line — the dynamic-debug control file cannot be written on this machine, because Secure Boot puts the kernel in lockdown "integrity". Two of its modules are local builds rather than the distro's, and it matters for reading what follows: snd-soc-rt1320-sdw carries 9b30521074f0, and the v3 2/2 quirk is backported into soundwire-intel, which is where dmi-quirks.o lands in this tree. I checked the first of those in the disassembly rather than trusting the build directory, which turned out to hold different source: the gate reads the status register, tests BIT(5), and falls through to a byte test of first_hw_init with both branches entering the same block. So the fallback is present. The ghost is absent on this machine today for the same reason — the quirk is loaded. The func_status value is read and printed before that gate, so the readings themselves are independent of which version of the condition is compiled in: condition amp func_status BIT(5) cold boot, systemd poweroff to S5, RTC alarm wake 0x61 set warm reboot 0x61 set resume from s2idle 0x41 absent driver unbind/bind 0x41 absent 0x61 is NEWLY_ATTACHED | NEEDS_INITIALIZATION | HAS_BEEN_RESET; 0x41 is the same without BIT(5). On this machine the flag is therefore behaving as intended: raised on the attach that follows a genuine power-on, absent on later re-attaches where the part has kept its state. The resume row is the one that says something about the hardware regardless of the patch, because first_hw_init is already true there — so the preset is skipped on resume under the stock condition and under the restored one alike. It is skipped, and the speakers still work. I checked that by measurement rather than by log, because silence with every layer reporting success is the failure mode here: a 1 kHz tone played to the speaker and recorded on the internal microphone puts the 1 kHz bin 1697x over ambient after a resume. The amplifier retains its programming across s2idle. I cannot make the same claim from the unbind/bind row. There first_hw_init goes back to false, so on this module the fallback fires and the preset runs; sound after a rebind shows only that it ran. I think 0x41 is also explained, and it is not a second reading of "this part has been reset". All three drivers acknowledge exactly one bit: regmap_write(..., RT1320_SDCA_CTL_FUNC_STATUS, 0), FUNCTION_NEEDS_INITIALIZATION); DEVICE_NEWLY_ATTACHED and FUNCTION_HAS_BEEN_RESET are never written back by rt1320, rt712 or rt722 — none of the three mentions HAS_BEEN_RESET at all. So after the first attach clears BIT(5), what is left standing is 0x41, for the rest of the boot. That is a question worth asking on its own: are those bits meant to be acknowledged by the host, and is anything downstream entitled to believe them? If they are write-1-to-clear status and nobody clears them, then every reader after the first sees a function permanently claiming to be newly attached and freshly reset. It also means the patch I nearly sent instead — treat HAS_BEEN_RESET like NEEDS_INITIALIZATION — would have been wrong. It would re-run the full preset and the MCU patch load on every resume and every rebind, driven by a bit that nobody ever clears. I am glad I measured before writing it. What I still cannot explain --------------------------- In July this machine read 0x41 where it now reads 0x61, the amplifier was unprogrammed, and the speakers were silent. Those logs are real and so is today's table. I do not have a mechanism that turns one into the other, the kernel and firmware were ruled out at the time, and I am not going to invent one to tidy up the story. What I can say is that the case for 9b30521074f0 no longer rests on this laptop at all — it rests on f465d10cd731. I still have the hardware and the instrumentation is now scripted, so if there is a register or a moment you would want looked at, I would rather measure what you ask for than keep guessing at what matters. 2/2 is the one that still needs applying. Run fully stock on 2026-08-30, with distro modules and no quirk, this machine had no audio device at all: "aplay -l" reported zero cards. It is not an optimisation. That it has one today is only because the backported quirk is loaded. Sergey ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] soundwire: dmi-quirks: drop the ghost RT1320 on the Surface Pro 11 (Intel) 2026-09-04 12:36 ` Sergey Lebedev @ 2026-09-04 14:09 ` Pierre-Louis Bossart 2026-09-04 15:10 ` Sergey Lebedev 0 siblings, 1 reply; 9+ messages in thread From: Pierre-Louis Bossart @ 2026-09-04 14:09 UTC (permalink / raw) To: Sergey Lebedev, Vinod Koul, Bard Liao, Oder Chiou, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai Cc: Bard Liao, Shuming Fan, Amaan Lalani, linux-sound, linux-kernel On 9/4/26 14:36, Sergey Lebedev wrote: > Pierre-Louis, > > Thank you for the review of 2/2. > > On 1/2 I have to correct the record, and the confusion is mine. That patch > is already upstream: > > 9b30521074f0 ("ASoC: rt1320: run the initialisation preset on the first > hardware init") > > applied from v1 on 2026-08-10, merged in asoc-v7.3 on 2026-08-18 — twelve > days before I re-sent it as v3 1/2. I rebuilt the series after the v1 > review and did not notice that one of the three had already been taken; the > cover letter says only that 1/2 is byte-identical to v1's 1/3, which reads > as "unchanged" rather than "already applied". So there is nothing to skip: > my own 2026-08-30 request not to apply it, and your agreement with it, both > concern code that shipped three weeks ago, and AUTOSEL has since proposed > it for 6.18. That is my error and I am sorry for the wasted attention. > > Which leaves two real questions: whether it belongs in the tree, and what > the flag is actually doing. I have answers for both now, and they point in > opposite directions. > > > The change is a revert, not a workaround > ---------------------------------------- > > The fallback is what the driver shipped with: > > bad0a07a7e61 rt1320 driver added 2024-05-21 v6.11 > if ((amp_func_status & FUNCTION_NEEDS_INITIALIZATION) || > (!rt1320->first_hw_init)) { > > f465d10cd731 "Add support for version C" 2024-09-02 v6.12 > if ((amp_func_status & FUNCTION_NEEDS_INITIALIZATION)) { > > 9b30521074f0 restored 2026-08-04 v7.3 > > f465d10cd731 is +2124/-23 and its commit message reads, in full, "This > patch added the support for version C". That condition was rewritten > because its body had to grow a VC branch, and the `||` half did not survive > the edit. Nothing in the message, and nothing I can find on the list, > suggests it was meant to go. > > The siblings have never been without it, and all four commits are Shuming > Fan's, which is why I have put him on Cc — he is better placed than I am to > say whether the removal was meant: > > bad0a07a7e61 rt1320 2024-05-21 gate introduced, with the fallback > 936abb09c1c7 rt712 2024-06-20 gate introduced, with the fallback (x3) > f465d10cd731 rt1320 2024-09-02 fallback dropped > 91f4ca732495 rt722 2025-04-16 gate introduced, with the fallback (x3) > > In rt712 and rt722 the fallback is present in the very first line that ever > consulted the flag there — three times each, for DMIC, jack and amp. Six > live instances across two drivers, and rt1320 the only one that lost it, > eight months after being the first to have it. > > So I do not think anything about the flag needs to be understood before > that change is allowed to stand: it restores the driver's original > behaviour and makes it consistent with its siblings. What it lacks is the > tag saying so, which I should have found in August and did not: > > Fixes: f465d10cd731 ("ASoC: rt1320: Add support for version C") > > That would put the window at v6.12 through v7.2. Whether it is worth > backporting past 6.18 depends on whether any machine actually lands in the > gap, and — see below — I can no longer demonstrate one, so I am not asking > for that. > > > What the flag is doing, measured today > -------------------------------------- > > I went back to the hardware rather than argue from July's logs. Surface Pro > 11 for Business, Ubuntu 7.0.0-30, with snd_soc_rt1320_sdw.dyndbg="func > rt1320_io_init +p" on the kernel command line — the dynamic-debug control > file cannot be written on this machine, because Secure Boot puts the kernel > in lockdown "integrity". > > Two of its modules are local builds rather than the distro's, and it > matters for reading what follows: snd-soc-rt1320-sdw carries 9b30521074f0, > and the v3 2/2 quirk is backported into soundwire-intel, which is where > dmi-quirks.o lands in this tree. I checked the first of those in the > disassembly rather than trusting the build directory, which turned out to > hold different source: the gate reads the status register, tests BIT(5), > and falls through to a byte test of first_hw_init with both branches > entering the same block. So the fallback is present. The ghost is absent on > this machine today for the same reason — the quirk is loaded. > > The func_status value is read and printed before that gate, so the readings > themselves are independent of which version of the condition is compiled > in: > > condition amp func_status BIT(5) > cold boot, systemd poweroff to S5, > RTC alarm wake 0x61 set > warm reboot 0x61 set > resume from s2idle 0x41 absent > driver unbind/bind 0x41 absent > > 0x61 is NEWLY_ATTACHED | NEEDS_INITIALIZATION | HAS_BEEN_RESET; 0x41 is the > same without BIT(5). > > On this machine the flag is therefore behaving as intended: raised on the > attach that follows a genuine power-on, absent on later re-attaches where > the part has kept its state. > > The resume row is the one that says something about the hardware regardless > of the patch, because first_hw_init is already true there — so the preset > is skipped on resume under the stock condition and under the restored one > alike. It is skipped, and the speakers still work. I checked that by > measurement rather than by log, because silence with every layer reporting > success is the failure mode here: a 1 kHz tone played to the speaker and > recorded on the internal microphone puts the 1 kHz bin 1697x over ambient > after a resume. The amplifier retains its programming across s2idle. > > I cannot make the same claim from the unbind/bind row. There first_hw_init > goes back to false, so on this module the fallback fires and the preset > runs; sound after a rebind shows only that it ran. > > I think 0x41 is also explained, and it is not a second reading of "this > part has been reset". All three drivers acknowledge exactly one bit: > > regmap_write(..., RT1320_SDCA_CTL_FUNC_STATUS, 0), > FUNCTION_NEEDS_INITIALIZATION); > > DEVICE_NEWLY_ATTACHED and FUNCTION_HAS_BEEN_RESET are never written back by > rt1320, rt712 or rt722 — none of the three mentions HAS_BEEN_RESET at all. > So after the first attach clears BIT(5), what is left standing is 0x41, for > the rest of the boot. That is a question worth asking on its own: are those > bits meant to be acknowledged by the host, and is anything downstream > entitled to believe them? If they are write-1-to-clear status and nobody > clears them, then every reader after the first sees a function permanently > claiming to be newly attached and freshly reset. > > It also means the patch I nearly sent instead — treat HAS_BEEN_RESET like > NEEDS_INITIALIZATION — would have been wrong. It would re-run the full > preset and the MCU patch load on every resume and every rebind, driven by a > bit that nobody ever clears. I am glad I measured before writing it. > > > What I still cannot explain > --------------------------- > > In July this machine read 0x41 where it now reads 0x61, the amplifier was > unprogrammed, and the speakers were silent. Those logs are real and so is > today's table. I do not have a mechanism that turns one into the other, the > kernel and firmware were ruled out at the time, and I am not going to > invent one to tidy up the story. What I can say is that the case for > 9b30521074f0 no longer rests on this laptop at all — it rests on > f465d10cd731. > > I still have the hardware and the instrumentation is now scripted, so if > there is a register or a moment you would want looked at, I would rather > measure what you ask for than keep guessing at what matters. > > 2/2 is the one that still needs applying. Run fully stock on 2026-08-30, > with distro modules and no quirk, this machine had no audio device at all: > "aplay -l" reported zero cards. It is not an optimisation. That it has one > today is only because the backported quirk is loaded. It's rather hard to follow the references to patches and versions, and if I am honest there are too many details... If you have two versions of code where you get two different reads of the same registers, then git bisect is your friend. On these bitfields, the definitions of HAS_BEEN_RESET and NEWLY_ATTACHED were controversial from the beginning when they were discussed in the MIPI WG. My personal opinion was that they didn't make sense... NEWLY_ATTACHED is a duplicate at the SDCA level of what is already defined for the SoundWire enumeration process. I am not sure if there is any merit in trying to deal with this bitfield, you could end-up in a a situation where the dev_num is non-zero after enumeration but the NEWLY_ATTACHED bit is still set. It's not obvious at all for how long this NEWLY_ATTACHED bit is supposed to be set by hardware. Same for HAS_BEEN_RESET. There are existing definitions at the SoundWire level for sync loss, device reset with a register write and bus reset. It's not clear for how long the HAS_BEEN_RESET bit is set in the SDCA status register after a SoundWire-level event...I vaguely remember that this bit was intended to deal with imp-def reset or to provide a placeholder for future extensions, but for now this bit doesn't seem too useful... I would skip both definitions for now. The only one that really matters is NEEDS_INITIALIZATION, since this could lead to 'no audio' problems. The premise is that the SDCA hardware layers know if the context is preserved or not, depending on what power rails are maintained during the suspend phases. The idea was that the driver should only try to re-download all the initialization tables - also defined as blind writes - when the hardware sets this bit. ut as usual placing trust in the hardware is risky if said hardware doesn't implement this logic, i.e. it's reasonable to expect that some chips will always set this bit even if it's not strictly required, or never set it... I think the logic to download the tables on the first_hw_init OR when hardware sets this bit is perfectly reasonable, it can't think of any reasons why drivers would have different tests. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] soundwire: dmi-quirks: drop the ghost RT1320 on the Surface Pro 11 (Intel) 2026-09-04 14:09 ` Pierre-Louis Bossart @ 2026-09-04 15:10 ` Sergey Lebedev 0 siblings, 0 replies; 9+ messages in thread From: Sergey Lebedev @ 2026-09-04 15:10 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: bard.liao, broonie, lgirdwood, linux-kernel, linux-sound, oder_chiou, perex, shumingf, tiwai, vkoul, yung-chuan.liao, zlzzm2014 Pierre-Louis, Point taken on the length, and thank you for the MIPI WG history — that is the part I had no way to find, and it settles what I was asking. I will leave NEWLY_ATTACHED and HAS_BEEN_RESET alone. On bisect: the status register is read before the condition is evaluated, so no version of that test can change the value — bisecting the driver would read the same thing throughout. What differs is the machine, two months apart, and I cannot recreate July to walk between them. So it stays unexplained, and by your account it does not need to be: only NEEDS_INITIALIZATION matters, and first_hw_init OR the bit is what the driver now does. Sergey ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/2] ASoC: fix audio on the Microsoft Surface Pro 11 (Intel) 2026-08-30 15:15 [PATCH v3 0/2] ASoC: fix audio on the Microsoft Surface Pro 11 (Intel) Sergey Lebedev 2026-08-30 15:15 ` [PATCH v3 1/2] ASoC: rt1320: run the initialisation preset on the first hardware init Sergey Lebedev 2026-08-30 15:15 ` [PATCH v3 2/2] soundwire: dmi-quirks: drop the ghost RT1320 on the Surface Pro 11 (Intel) Sergey Lebedev @ 2026-08-30 17:47 ` Sergey Lebedev 2 siblings, 0 replies; 9+ messages in thread From: Sergey Lebedev @ 2026-08-30 17:47 UTC (permalink / raw) To: Vinod Koul, Bard Liao, Pierre-Louis Bossart, Oder Chiou, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai Cc: Bard Liao, Shuming Fan, Amaan Lalani, linux-sound, linux-kernel Please do not apply 1/2. I cannot reproduce the condition it fixes, and I would rather say so myself than have it found in review. The patch rests on one claim: that this part never sets FUNCTION_NEEDS_INITIALIZATION, so rt1320_io_init() never runs the vendor preset and the amplifier is never programmed. Every func_status line I recorded in July reads 0x41 - NEWLY_ATTACHED | FUNCTION_HAS_BEEN_RESET, without BIT(5). Re-tested tonight on the same machine, with the stock rt1320 driver and my patch not loaded: rt1320-sdca sdw:0:0:025d:1320:01: rt1320_io_init amp func_status=0x61 rt1320-sdca sdw:0:0:025d:1320:01: rt1320_vc_preset, RT1320_KR0_INT_READY=0x0 rt1320-sdca sdw:0:0:025d:1320:01: rt1320_vc_preset, RT1320_KR0_INT_READY=0x1f 0x61 is 0x41 | BIT(5). The bit is set, the stock driver runs the preset, and the patch is a no-op. Same reading in three configurations - with the ghost _ADR removed, with a machine-entry approach that leaves it enumerated, and fully stock - and on a genuine cold boot, not a reboot: the previous boot ended in systemd-poweroff and an RTC alarm brought the machine back from S5. The speakers work without the patch, and I checked that by measurement rather than by reading logs, because silence with every layer reporting success is the exact failure mode here. Playing a 1 kHz tone and recording the internal microphone, the 1 kHz bin rises 185x over ambient with the patch absent, 610x with it present. Both are unambiguously audible; the difference is room noise. What I ruled out: - the gate is unchanged. if ((amp_func_status & FUNCTION_NEEDS_INITIALIZATION)) is identical in v7.0 and v7.1-rc7, the tree this series targets - Ubuntu changed nothing relevant between 7.0.0-28 and 7.0.0-30: no rt1320, sdca, soundwire or sdw entry in the changelog - firmware predates the July readings. firmware-sof-signed went to 2025.12.2 on 2026-07-27 and the Surface firmware bundle was applied 2026-07-28 - the ghost is not involved: fully stock, it never reaches rt1320_io_init() at all, and the real device still reports 0x61 So I have no explanation for the change, and I am not going to invent one. If someone knows how an SDCA part stops asserting that status bit between one month and the next, I would like to hear it - that is worth more to the subsystem than the patch was. I still have the hardware and will test whatever is useful. 2/2 is unaffected and I would ask that it still be considered. I re-verified it the same night by running fully stock: sysfs: cannot create duplicate filename '.../sof_sdw/SDW0-Capture-SmartMic' kobject_add_internal failed for SDW0-Capture-SmartMic with -EEXIST sof_sdw sof_sdw: probe with driver sof_sdw failed with error -12 aplay -l reports zero cards. Without the quirk this machine has no audio device at all, so that patch is not an optimisation. One more result from the same night, since it bears on the approach. I compared this series against the downstream machine-entry patch that Surface Pro 11 users run today, on one machine and one kernel, swapping only the audio modules. Both end up with the same configuration - same three function topologies, 4 playback + 1 capture, rt1320-1, working microphone. The difference is what each costs: the machine entry does not match at all unless class_id is added to SDW_CODEC_ADR_MASK and to the peripheral comparison in soc-acpi.c, because generic matching sees two peripherals for the one address it declares. That is shared code for every SoundWire machine. Removing the ghost at enumeration needs no such change. Sorry for the noise on 1/2. Thanks, Sergey ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-09-04 15:10 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-30 15:15 [PATCH v3 0/2] ASoC: fix audio on the Microsoft Surface Pro 11 (Intel) Sergey Lebedev 2026-08-30 15:15 ` [PATCH v3 1/2] ASoC: rt1320: run the initialisation preset on the first hardware init Sergey Lebedev 2026-08-30 15:15 ` [PATCH v3 2/2] soundwire: dmi-quirks: drop the ghost RT1320 on the Surface Pro 11 (Intel) Sergey Lebedev 2026-08-30 21:12 ` Sergey Lebedev 2026-09-04 7:51 ` Pierre-Louis Bossart 2026-09-04 12:36 ` Sergey Lebedev 2026-09-04 14:09 ` Pierre-Louis Bossart 2026-09-04 15:10 ` Sergey Lebedev 2026-08-30 17:47 ` [PATCH v3 0/2] ASoC: fix audio on the Microsoft " Sergey Lebedev
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox