ASoC: SOF: amd: SoundWire support and fixes for ACP7.B/7.F - #5931
vijendarmukunda wants to merge 12 commits into
Conversation
amd_sof_acp7x_suspend() is registered only for ACP7.B and ACP7.F platforms. If an unexpected PCI revision reaches the switch statement the default case previously fell through, leaving ACP_CONTROL with a stale value (enable=false) before writing ZSC_DSP_CTRL=1. Return -EINVAL instead to surface the programming error immediately. Fixes: 1c9646f ("ASoC: SOF: amd: add system and runtime PM ops for ACP7x") Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Call free_irq() before amd_sof_sdw_exit() in amd_sof_acp_remove(). amd_sof_sdw_exit() sets adata->sdw to NULL; if an IRQ fires between that point and the subsequent free_irq(), acp_irq_handler() may dereference the now-NULL pdev[] pointer, causing a use-after-free. Fixes: d948218 ("ASoC: SOF: amd: add code for invoking soundwire manager helper functions") Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Call amd_sof_sdw_exit() after free_irq() in the free_ipc_irq error label of amd_sof_acp_probe(). Without this, an IRQ that fires between sdw context teardown and free_irq() can dereference freed SoundWire resources. Fixes: d948218 ("ASoC: SOF: amd: add code for invoking soundwire manager helper functions") Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Add acp_sof_scan_pdm_devices() to read the acp-audio-ep-port ACPI _DSD property from the PDM child device on ACP7.B/7.F platforms. Value 4 selects PDM0 (ACP7X_PDM_DMIC0), value 5 selects PDM1 (ACP7X_PDM_DMIC1). Unrecognized values are reported via dev_warn(). The selected controller is stored in acp_dev_data.pdm_sel and propagated to the machine driver via mach->pdata in amd_sof_machine_select() so the machine driver probe can register the correct SOF DMIC DAI link. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
…CP7.B/7.F Populate the SoundWire descriptor fields in acp7x_chip_info so the SOF core can locate and enumerate the four SoundWire managers on ACP7.B/7.F platforms. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Select SND_SOC_SOF_AMD_SOUNDWIRE_LINK_BASELINE in Kconfig for SND_SOC_SOF_AMD_ACP7X so that the SoundWire stack is built when ACP7.B/7.F SOF support is enabled. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
In amd_sof_acp7x_probe(), scan DSDT for SoundWire peripherals via acp_sof_scan_sdw_devices() and call amd_sof_sdw_probe() on success. In the error unwind, call free_irq() before amd_sof_sdw_exit() so the IRQ cannot fire after the SoundWire context is freed. In amd_sof_acp7x_remove(), call amd_sof_sdw_exit() after free_irq() to prevent a use-after-free: the IRQ handler dereferences pdev[] entries freed by amd_sof_sdw_exit(). Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
…enable_status Extend check_acp_sdw_enable_status() with a pci_rev switch so that ACP7.B/7.F, which has 4 SoundWire manager instances, reads all four SW_EN registers to determine whether any manager is in clock-stop mode. The existing two-manager read (ACP_SW0_EN, ACP_SW1_EN) is preserved for ACP63/ACP70/ACP71/ACP72 platforms. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
|
test please. |
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness issues in the updated selection/PM logic (stale sdw_en_stat on unknown revisions and overwriting existing mach->pdata used by ACP quirk flags).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds SoundWire support for AMD ACP7.B/7.F (4 manager instances) and fixes teardown / PM ordering issues in the AMD SOF ACP driver, including new ACPI scanning to propagate PDM endpoint selection to the machine driver.
Changes:
- Extend ACP7x chip descriptor and Kconfig to enable SoundWire baseline support.
- Add ACP7x-specific SoundWire IRQ handling (4 managers), wake/PME handling, and clock-stop aware suspend/resume paths.
- Scan ACPI DSDT for PDM endpoint selection and pass the selection to the machine driver via
mach->pdata.
File summaries
| File | Description |
|---|---|
| sound/soc/sof/amd/pci-acp7x.c | Populate ACP7x SoundWire descriptor fields (max links, ACPI addr). |
| sound/soc/sof/amd/Kconfig | Select SoundWire link baseline dependency for ACP7x SOF. |
| sound/soc/sof/amd/acp7x.h | Add ACP7x SoundWire error/wake register table definitions. |
| sound/soc/sof/amd/acp.h | Add ACP7x manager count, PDM selection constants, and per-manager wake flags in acp_dev_data. |
| sound/soc/sof/amd/acp.c | Implement ACP7x SoundWire IRQ/wake/error handling, ACPI scanning for SoundWire/PDM, and PM callback updates including clock-stop path. |
| sound/soc/sof/amd/acp-dsp-offset.h | Define ACP7x SoundWire register offsets/bitmasks. |
| sound/soc/sof/amd/acp-common.c | Copy selected machine entry and attach PDM selection platform data for machine driver use. |
| sound/soc/amd/acp/soc_amd_sdw_common.h | Define amd_pdm_pdata payload passed via mach->pdata. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@vijendarmukunda The change is all in the sound/soc/sof/amd directory. Please go ahead to submit it to mailing list. |
|
@bardliao : Thanks will post the patch series to mailing list. |
ACP7.B/7.F exposes four SoundWire managers (SDW0-SDW3), each with dedicated interrupt, wake-enable, PME status and error registers. Add acp7x_irq_handler() to dispatch: - Per-manager data IRQs via sof_acp7x_handle_sdw_manager_irq() - Host-wake and PME events via sof_amd_check_and_handle_acp7x_sdw_wake_irq() which checks both sources independently per manager so simultaneous host-wake and PME events are not missed - Error conditions via sof_amd_acp7x_clear_sdw_err_regs() which writes back the updated error status to clear only the affected manager bit Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
During acp_init() for ACP7.B/7.F, read ACP7X_SW_WAKE_EN and enable the corresponding host-wake interrupt mask in ACP7X_EXTERNAL_INTR_CNTL1 only for managers that have wake-enable set (SW_WAKE_EN bit i enables ACP7X_SW_HOST_WAKE_MASK << i in INTR_CNTL1). This ensures only the active managers arm their host-wake interrupt when ACP enters D0. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Add handle_amd_sof_acp7x_sdw_pme_event() to iterate over all four SoundWire managers on runtime PM resume, clear PME status and wake-enable bits (under acp_lock to prevent races with the IRQ path), and request runtime resume for each active manager. Extend amd_sof_acp7x_suspend() with a clock-stop fast path: when SoundWire managers are in clock-stop mode, write ZSC_DSP_CTRL=1 and call acp_dsp_reset() instead of the full acp_reset() to preserve SoundWire bus state. Extend amd_sof_acp7x_resume() with a matching clock-stop resume path that restores ZSC_DSP_CTRL and re-arms PME_EN. Introduce amd_sof_acp7x_suspend_runtime() and amd_sof_acp7x_resume_runtime() as separate runtime PM callbacks. The runtime resume path calls handle_amd_sof_acp7x_sdw_pme_event() after re-initialising hardware to clear any PME state that arrived during the suspend window. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Extend the AMD SOF machine driver to propagate the PCI subsystem Vendor and Device IDs so that they may be subsequently used as an SSID. Store the subsystem IDs in acp_dev_data during probe (both amd_sof_acp_probe and amd_sof_acp7x_probe), and propagate them to mach_params in the SoundWire machine select path. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
2652268 to
bbfe903
Compare
|
@ujfalusi : Please help to review. if you don't have further review comments, then i will post this patch series as v2 version to the mailing list. |
| #define WAKE_IRQ_HANDLED 1 | ||
| #define WAKE_IRQ_NONE 0 | ||
|
|
||
| /* ACP7X SoundWire IO structures */ |
There was a problem hiding this comment.
this comment does not add much context.
There was a problem hiding this comment.
These structures are being used in ACP7X IRQ handler for SoundWire IO interrupts.
| ACP7X_SW_WAKE_EN_MASK << 3, 3 }, | ||
| }; | ||
|
|
||
| /* ACP7X SoundWire IO interrupt and wake helpers */ |
There was a problem hiding this comment.
I'm not sure if this comment helps to frame the code, 'ACP7X SoundWire IO interrupt and wake helpers' from here till the end of the file? Included acp_power_on()?
There was a problem hiding this comment.
These are included as part of IRQ handling patch and being invoked from acp7x irq handler code.
|
@ujfalusi:In this patch series except top commit, rest of the 11 commits are got merged into ASoC tree for-next branch. We will send the nit pick fixes as an incremental patch along with 12th patch. |
@vijendarmukunda, that is great, congrats! Send the update as you see fit. |
This series adds SoundWire support for the ACP7.B and ACP7.F platforms
and fixes three pre-existing bugs in the ACP SOF driver.
Patches 1-3 are standalone bug fixes:
Patch 1 corrects the error-return path in amd_sof_acp7x_suspend() so
that an unrecognised PCI revision returns -EINVAL instead of silently
leaving ACP_CONTROL with a stale value.
Patch 2 fixes the teardown ordering in amd_sof_acp_remove(): free_irq()
is moved before amd_sof_sdw_exit() to prevent a use-after-free when an
IRQ fires after the SoundWire context has been freed.
Patch 3 fixes the equivalent ordering in the error unwind path of
amd_sof_acp_probe() for the same reason.
Patches 4-11 add SoundWire support for ACP7.B/7.F:
ACP7.B/7.F exposes four SoundWire manager instances (SDW0-SDW3),
compared to the two managers found on ACP63/ACP70/ACP71/ACP72.
Patch 4 scans the ACPI DSDT for PDM controller endpoints and propagates
the selection to the machine driver via mach->pdata.
Patch 5 populates the SoundWire descriptor fields in acp7x_chip_info so
the SOF core can enumerate the four managers.
Patch 6 enables the SND_SOC_SOF_AMD_SOUNDWIRE_LINK_BASELINE Kconfig
dependency for SND_SOC_SOF_AMD_ACP7X.
Patch 7 wires amd_sof_sdw_probe() and amd_sof_sdw_exit() into the
ACP7.B/7.F probe and remove paths with the correct IRQ ordering.
Patch 8 extends check_acp_sdw_enable_status() to read all four SW_EN
registers on ACP7.B/7.F for clock-stop detection.
Patch 9 adds the ACP7.B/7.F SoundWire IRQ handler, dispatching
per-manager data IRQs, host-wake and PME events, and error conditions
across all four managers.
Patch 10 enables the host-wake interrupt mask in acp_init() for each
ACP7.B/7.F manager that has SW_WAKE_EN set.
Patch 11 adds SoundWire PM callbacks for ACP7.B/7.F: a clock-stop fast
path through suspend/resume that preserves SoundWire bus state, and
separate runtime PM callbacks that clear PME state on runtime resume.
Patch 12 propagates the PCI subsystem Vendor and Device IDs so they may
be used as an SSID: the subsystem IDs are stored in acp_dev_data during
probe (both amd_sof_acp_probe() and amd_sof_acp7x_probe()) and forwarded
to mach_params in the SoundWire machine-select path.