Skip to content

ASoC: SOF: Intel: hda: Fixes for ACE2+ HD-DMA stream constraints for PPLC_LLP corruption #5745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ujfalusi wants to merge 4 commits into
base: topic/sof-dev
Choose a base branch
from
Open

ASoC: SOF: Intel: hda: Fixes for ACE2+ HD-DMA stream constraints for PPLC_LLP corruption #5745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c15e0d7
ASoC: SOF: Intel: hda: Fold mlink enumeration into hda_dsp_ctrl_init_…
ujfalusi Apr 17, 2026
3225a6f
ASoC: SOF: Intel: hda: Keep non-alt mlinks powered at probe on ACE2+
ujfalusi Apr 17, 2026
da4a638
ASoC: SOF: Intel: hda: Remove unused hda_bus_ml_put_all()
ujfalusi Apr 22, 2026
d0bfef8
ASoC: SOF: Intel: hda: Avoid ACE2+ link DMA stream allocation hazards
ujfalusi Apr 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions include/sound/hda-mlink.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,22 @@
struct hdac_bus;
struct hdac_ext_link;

/**
* enum hda_bus_ml_link_type - mlink link type, used by SOF link DMA
* allocator constraints (see struct sof_intel_hda_dev).
*
* @HDA_BUS_ML_LINK_HDA: non-alt link, i.e. HDA codec or iDisp
* @HDA_BUS_ML_LINK_SDW: alt link, SoundWire
* @HDA_BUS_ML_LINK_UAOL: alt link, USB Audio Offload
* @HDA_BUS_ML_LINK_OTHER: alt link, SSP or DMIC
*/
enum hda_bus_ml_link_type {
HDA_BUS_ML_LINK_HDA,
HDA_BUS_ML_LINK_SDW,
HDA_BUS_ML_LINK_UAOL,
HDA_BUS_ML_LINK_OTHER,
};

#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_MLINK)

int hda_bus_ml_init(struct hdac_bus *bus);
Expand Down Expand Up @@ -49,11 +65,12 @@ int hdac_bus_eml_sdw_set_lsdiid(struct hdac_bus *bus, int sublink, int dev_num);
int hdac_bus_eml_sdw_map_stream_ch(struct hdac_bus *bus, int sublink, int y,
int channel_mask, int stream_id, int dir);

void hda_bus_ml_put_all(struct hdac_bus *bus);
void hda_bus_ml_reset_losidv(struct hdac_bus *bus);
int hda_bus_ml_resume(struct hdac_bus *bus);
int hda_bus_ml_suspend(struct hdac_bus *bus);

enum hda_bus_ml_link_type hda_bus_ml_link_get_type(struct hdac_ext_link *hlink);

struct hdac_ext_link *hdac_bus_eml_ssp_get_hlink(struct hdac_bus *bus);
struct hdac_ext_link *hdac_bus_eml_dmic_get_hlink(struct hdac_bus *bus);
struct hdac_ext_link *hdac_bus_eml_sdw_get_hlink(struct hdac_bus *bus);
Expand Down Expand Up @@ -169,11 +186,13 @@ hdac_bus_eml_sdw_map_stream_ch(struct hdac_bus *bus, int sublink, int y,
return 0;
}

static inline void hda_bus_ml_put_all(struct hdac_bus *bus) { }
static inline void hda_bus_ml_reset_losidv(struct hdac_bus *bus) { }
static inline int hda_bus_ml_resume(struct hdac_bus *bus) { return 0; }
static inline int hda_bus_ml_suspend(struct hdac_bus *bus) { return 0; }

static inline enum hda_bus_ml_link_type
hda_bus_ml_link_get_type(struct hdac_ext_link *hlink) { return HDA_BUS_ML_LINK_HDA; }

static inline struct hdac_ext_link *
hdac_bus_eml_ssp_get_hlink(struct hdac_bus *bus) { return NULL; }

Expand Down
19 changes: 19 additions & 0 deletions sound/soc/sof/intel/hda-ctrl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,24 @@ EXPORT_SYMBOL_NS(hda_dsp_ctrl_clock_power_gating, "SND_SOC_SOF_INTEL_HDA_COMMON"
int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool detect_codec)
{
struct hdac_bus *bus = sof_to_bus(sdev);
struct sof_intel_hda_dev *sof_hda = bus_to_sof_hda(bus);
struct hdac_stream *stream;
int sd_offset, ret = 0;
u32 gctl;

if (bus->chip_init)
return 0;

/*
* The controller reset clears the ACE2+ link DMA stream allocation
* constraints; reset the masks to reflect this.
*/
memset(sof_hda->link_dma_active_sdw_mask, 0,
sizeof(sof_hda->link_dma_active_sdw_mask));
memset(sof_hda->link_dma_active_multi_mask, 0,
sizeof(sof_hda->link_dma_active_multi_mask));
sof_hda->link_dma_out_hda_used_mask = 0;

hda_codec_set_codec_wakeup(sdev, true);

hda_dsp_ctrl_misc_clock_gating(sdev, false);
Expand Down Expand Up @@ -223,6 +234,14 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool detect_codec)
/* Accept unsolicited responses */
snd_hdac_chip_updatel(bus, GCTL, AZX_GCTL_UNSOL, AZX_GCTL_UNSOL);

/* Perform a one-time enumeration of the Multi-Link capability */
ret = hda_bus_ml_init(bus);
if (ret < 0) {
dev_err(sdev->dev, "%s: failed to enumerate multi-links\n",
__func__);
goto err;
}

if (detect_codec)
hda_codec_detect_mask(sdev);

Expand Down
76 changes: 70 additions & 6 deletions sound/soc/sof/intel/hda-dai-ops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
/* These ops are only applicable for the HDA DAI's in their current form */
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_LINK)
/*
* This function checks if the host dma channel corresponding
* This function checks if the host DMA stream corresponding
* to the link DMA stream_tag argument is assigned to one
* of the FEs connected to the BE DAI.
*/
Expand All @@ -42,23 +42,53 @@ static bool hda_check_fes(struct snd_soc_pcm_runtime *rtd,
}

static struct hdac_ext_stream *
hda_link_stream_assign(struct hdac_bus *bus, struct snd_pcm_substream *substream)
hda_link_stream_assign(struct hdac_bus *bus, struct snd_pcm_substream *substream,
enum hda_bus_ml_link_type link_type)
{
struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
struct sof_intel_hda_dev *sof_hda = bus_to_sof_hda(bus);
struct sof_intel_hda_stream *hda_stream;
const struct sof_intel_dsp_desc *chip;
struct snd_sof_dev *sdev;
struct hdac_ext_stream *res = NULL;
struct hdac_stream *hstream = NULL;

int stream_dir = substream->stream;
bool is_multi = link_type == HDA_BUS_ML_LINK_HDA || link_type == HDA_BUS_ML_LINK_UAOL;
bool is_play = stream_dir == SNDRV_PCM_STREAM_PLAYBACK;
bool is_sdw = link_type == HDA_BUS_ML_LINK_SDW;
bool is_hda = link_type == HDA_BUS_ML_LINK_HDA;
u32 concur_block_mask = 0;
u32 seq_block_mask = 0;
unsigned int stream_idx;

if (!bus->ppcap) {
dev_err(bus->dev, "stream type not supported\n");
return NULL;
}

/*
* On ACE2+ the link DMA stream allocator must avoid two HW errata,
* see the comment on struct sof_intel_hda_dev.
*
* - Concurrent cross-direction: SoundWire conflicts with HDA, iDisp
* and UAOL on the same physical stream index; SSP and DMIC are safe.
* - Sequential playback: a stream index previously used by an HDA/iDisp
* link cannot drive any non-HDA/iDisp link in the same direction
* until the next controller reset.
*
* The masks are protected by bus->reg_lock; sample them inside the
* lock together with the stream walk to keep the decision atomic
* with concurrent allocations and releases.
*/
guard(spinlock_irq)(&bus->reg_lock);

if (is_sdw)
concur_block_mask = sof_hda->link_dma_active_multi_mask[!stream_dir];
else if (is_multi)
concur_block_mask = sof_hda->link_dma_active_sdw_mask[!stream_dir];
if (is_play && !is_hda)
Comment on lines +85 to +89
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using !stream_dir to select the opposite direction relies on SNDRV_PCM_STREAM_PLAYBACK/CAPTURE being exactly 0/1. Consider using stream_dir ^ 1 or an explicit conditional to avoid coupling this logic to the exact numeric values of the ALSA constants.

Copilot uses AI. Check for mistakes.
seq_block_mask = sof_hda->link_dma_out_hda_used_mask;

list_for_each_entry(hstream, &bus->stream_list, list) {
struct hdac_ext_stream *hext_stream =
stream_to_hdac_ext_stream(hstream);
Expand All @@ -69,6 +99,12 @@ hda_link_stream_assign(struct hdac_bus *bus, struct snd_pcm_substream *substream
sdev = hda_stream->sdev;
chip = get_chip_info(sdev->pdata);

stream_idx = hstream->stream_tag - 1;

/* skip streams blocked by the ACE2+ allocator constraints */
if ((concur_block_mask | seq_block_mask) & BIT(stream_idx))
continue;

/* check if link is available */
if (!hext_stream->link_locked) {
/*
Expand All @@ -95,7 +131,7 @@ hda_link_stream_assign(struct hdac_bus *bus, struct snd_pcm_substream *substream

/*
* This must be a hostless stream.
* So reserve the host DMA channel.
* So reserve the host DMA stream.
*/
hda_stream->host_reserved = 1;
break;
Expand All @@ -109,6 +145,16 @@ hda_link_stream_assign(struct hdac_bus *bus, struct snd_pcm_substream *substream

res->link_locked = 1;
res->link_substream = substream;

stream_idx = res->hstream.stream_tag - 1;
if (is_sdw)
sof_hda->link_dma_active_sdw_mask[stream_dir] |= BIT(stream_idx);
else if (is_multi)
sof_hda->link_dma_active_multi_mask[stream_dir] |= BIT(stream_idx);

/* persistent OUT HDA/iDisp shadow, cleared only on CRST# */
if (is_hda && is_play)
sof_hda->link_dma_out_hda_used_mask |= BIT(stream_idx);
}

return res;
Expand Down Expand Up @@ -143,11 +189,13 @@ static struct hdac_ext_stream *hda_ipc4_get_hext_stream(struct snd_sof_dev *sdev

static struct hdac_ext_stream *hda_assign_hext_stream(struct snd_sof_dev *sdev,
struct snd_soc_dai *cpu_dai,
struct snd_pcm_substream *substream)
struct snd_pcm_substream *substream,
struct hdac_ext_link *hlink)
{
struct hdac_ext_stream *hext_stream;
enum hda_bus_ml_link_type link_type = hda_bus_ml_link_get_type(hlink);

hext_stream = hda_link_stream_assign(sof_to_bus(sdev), substream);
hext_stream = hda_link_stream_assign(sof_to_bus(sdev), substream, link_type);
if (!hext_stream)
return NULL;

Expand All @@ -160,6 +208,22 @@ static void hda_release_hext_stream(struct snd_sof_dev *sdev, struct snd_soc_dai
struct snd_pcm_substream *substream)
{
struct hdac_ext_stream *hext_stream = hda_get_hext_stream(sdev, cpu_dai, substream);
struct sof_intel_hda_dev *sof_hda = sdev->pdata->hw_pdata;
struct hdac_bus *bus = sof_to_bus(sdev);
int dir = substream->stream;
unsigned int stream_idx = hext_stream->hstream.stream_tag - 1;

/*
* Drop the stream index from the per-direction active concurrency masks.
* The two masks are mutually exclusive for a given stream/direction
* (and a stream of the SSP/DMIC kind appears in neither), so a blind
* clear of both is safe and lets us avoid having to remember the
* link type at allocation time.
*/
scoped_guard(spinlock_irq, &bus->reg_lock) {
sof_hda->link_dma_active_sdw_mask[dir] &= ~BIT(stream_idx);
sof_hda->link_dma_active_multi_mask[dir] &= ~BIT(stream_idx);
}

snd_soc_dai_set_dma_data(cpu_dai, substream, NULL);
snd_hdac_ext_stream_release(hext_stream, HDAC_EXT_STREAM_TYPE_LINK);
Expand Down
2 changes: 1 addition & 1 deletion sound/soc/sof/intel/hda-dai.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ static int hda_link_dma_hw_params(struct snd_pcm_substream *substream,

if (!hext_stream) {
if (ops->assign_hext_stream)
hext_stream = ops->assign_hext_stream(sdev, cpu_dai, substream);
hext_stream = ops->assign_hext_stream(sdev, cpu_dai, substream, hlink);
}

if (!hext_stream)
Expand Down
35 changes: 22 additions & 13 deletions sound/soc/sof/intel/hda-mlink.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,10 @@ int hda_bus_ml_init(struct hdac_bus *bus)
if (!bus->mlcap)
return 0;

/* Enumeration is a one time operation, skip if already done */
if (!list_empty(&bus->hlink_list))
return 0;

link_count = readl(bus->mlcap + AZX_REG_ML_MLCD) + 1;

dev_dbg(bus->dev, "HDAudio Multi-Link count: %d\n", link_count);
Expand Down Expand Up @@ -880,19 +884,6 @@ int hdac_bus_eml_sdw_map_stream_ch(struct hdac_bus *bus, int sublink, int y,
return 0;
} EXPORT_SYMBOL_NS(hdac_bus_eml_sdw_map_stream_ch, "SND_SOC_SOF_HDA_MLINK");

void hda_bus_ml_put_all(struct hdac_bus *bus)
{
struct hdac_ext_link *hlink;

list_for_each_entry(hlink, &bus->hlink_list, list) {
struct hdac_ext2_link *h2link = hdac_ext_link_to_ext2(hlink);

if (!h2link->alt)
snd_hdac_ext_bus_link_put(bus, hlink);
}
}
EXPORT_SYMBOL_NS(hda_bus_ml_put_all, "SND_SOC_SOF_HDA_MLINK");

void hda_bus_ml_reset_losidv(struct hdac_bus *bus)
{
struct hdac_ext_link *hlink;
Expand All @@ -903,6 +894,24 @@ void hda_bus_ml_reset_losidv(struct hdac_bus *bus)
}
EXPORT_SYMBOL_NS(hda_bus_ml_reset_losidv, "SND_SOC_SOF_HDA_MLINK");

enum hda_bus_ml_link_type hda_bus_ml_link_get_type(struct hdac_ext_link *hlink)
{
struct hdac_ext2_link *h2link = hdac_ext_link_to_ext2(hlink);

if (!h2link->alt)
return HDA_BUS_ML_LINK_HDA;

switch (h2link->elid) {
case AZX_REG_ML_LEPTR_ID_SDW:
return HDA_BUS_ML_LINK_SDW;
case AZX_REG_ML_LEPTR_ID_INTEL_UAOL:
return HDA_BUS_ML_LINK_UAOL;
default:
return HDA_BUS_ML_LINK_OTHER;
}
}
EXPORT_SYMBOL_NS(hda_bus_ml_link_get_type, "SND_SOC_SOF_HDA_MLINK");

int hda_bus_ml_resume(struct hdac_bus *bus)
{
struct hdac_ext_link *hlink;
Expand Down
4 changes: 0 additions & 4 deletions sound/soc/sof/intel/hda.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,8 +625,6 @@ static int hda_init_caps(struct snd_sof_dev *sdev)
return ret;
}

hda_bus_ml_init(bus);

/* Skip SoundWire if it is not supported */
if (!(interface_mask & BIT(SOF_DAI_INTEL_ALH)))
goto skip_soundwire;
Expand Down Expand Up @@ -670,8 +668,6 @@ static int hda_init_caps(struct snd_sof_dev *sdev)
if (!HDA_IDISP_CODEC(bus->codec_mask))
hda_codec_i915_display_power(sdev, false);

Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hda_bus_ml_put_all() is no longer called anywhere in the tree (search shows only the definition), which means non-alternate links keep their initial ref_count = 1 and will remain logically “in use” for the lifetime of the driver. This changes power-management behavior (e.g., hda_bus_ml_resume() will always power up all non-alt links after suspend). If the intent is only to keep links powered through first use, consider adding a later/conditional hda_bus_ml_put_all() (or equivalent refcount drop) once the PPLC/LLP issue is no longer a risk, or gating the behavior to the affected platforms only.

Suggested change
/*
* Drop the initial non-alternate multi-link references once the
* first-use initialization sequence is complete so links are not
* kept logically in use for the lifetime of the driver.
*/
hda_bus_ml_put_all(bus);

Copilot uses AI. Check for mistakes.
hda_bus_ml_put_all(bus);
Copy link
Copy Markdown
Collaborator

@kv2019i kv2019i Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ujfalusi Just to double check, this does not keep the display codec powered up (if only SDW used), right? It shouldn't if only the link is powered. I'll do some local testing on this as well...


return 0;
}

Expand Down
26 changes: 25 additions & 1 deletion sound/soc/sof/intel/hda.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,29 @@ struct sof_intel_hda_dev {
/* the maximum number of streams (playback + capture) supported */
u32 stream_max;

/*
* ACE2+ link DMA stream allocation constraints (stream index =
* stream_tag - 1, shared between input and output directions). All
* masks are cleared by hda_dsp_ctrl_init_chip() on controller reset
* (CRST#).
*
* - Concurrent (cross-direction) constraint: a SoundWire stream and
* a HDA/iDisp/UAOL stream cannot share a physical stream index
* across directions, the resulting LLP/timestamp values are wrong.
* link_dma_active_sdw_mask and link_dma_active_multi_mask
* (indexed by SNDRV_PCM_STREAM_*) track currently allocated
* streams per direction in each of the conflicting groups; SSP
* and DMIC do not participate. Bits are cleared on stream release.
*
* - Sequential (playback only) constraint: once a HDA/iDisp link
* has used a playback stream index, that index cannot drive a
* non-HDA/iDisp link in the same direction until the next CRST#.
* link_dma_out_hda_used_mask records this.
*/
u32 link_dma_active_sdw_mask[SNDRV_PCM_STREAM_LAST + 1];
u32 link_dma_active_multi_mask[SNDRV_PCM_STREAM_LAST + 1];
u32 link_dma_out_hda_used_mask;

/* PM related */
bool l1_disabled;/* is DMI link L1 disabled? */

Expand Down Expand Up @@ -1030,7 +1053,8 @@ struct hda_dai_widget_dma_ops {
struct snd_pcm_substream *substream);
struct hdac_ext_stream *(*assign_hext_stream)(struct snd_sof_dev *sdev,
struct snd_soc_dai *cpu_dai,
struct snd_pcm_substream *substream);
struct snd_pcm_substream *substream,
struct hdac_ext_link *hlink);
void (*release_hext_stream)(struct snd_sof_dev *sdev, struct snd_soc_dai *cpu_dai,
struct snd_pcm_substream *substream);
void (*setup_hext_stream)(struct snd_sof_dev *sdev, struct hdac_ext_stream *hext_stream,
Expand Down
Loading