Skip to content

ASoC: SOF: ipc4-topology: set playback channel mask#5527

Merged
lgirdwood merged 1 commit intothesofproject:topic/sof-devfrom
bardliao:playback-ch-mask
Oct 22, 2025
Merged

ASoC: SOF: ipc4-topology: set playback channel mask#5527
lgirdwood merged 1 commit intothesofproject:topic/sof-devfrom
bardliao:playback-ch-mask

Conversation

@bardliao
Copy link
Copy Markdown
Collaborator

@bardliao bardliao commented Sep 4, 2025

Currently, we send all channels to all amps and copy the channel_mask to all ALH DMAs in playback. However, the amp may not have the capability to run any process and SOF may need to split the channels and send specific data channel to each amp. In that case, we need to split the channel_mask in ALH DMA.
Copy the channel mask only if the widget channel count is the same the FE channels for playback, otherwise, split the channels among the aggregated DAIs. Like what we did in capture.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes channel mask handling in SOF (Sound Open Firmware) IPC4 topology for playback scenarios. It addresses cases where amplifiers cannot process all channels and SOF needs to split channels among DAIs.

  • Updates playback channel mask logic to conditionally use full channel mask or split channels
  • Removes hardcoded playbook channel mask assignment in favor of calculated masks
  • Aligns playback behavior with existing capture channel splitting logic

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +2305 to +2307
if (swidget->id == snd_soc_dapm_dai_in && ch_count == out_ref_channels) {
mask = ch_mask;
Copy link

Copilot AI Sep 4, 2025

Choose a reason for hiding this comment

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

The condition checks for snd_soc_dapm_dai_in (playbook) but sets step = 0 which effectively disables the shifting logic in the channel mask calculation below. This magic number should be documented or made more explicit with a named constant or explanatory comment about why step is zeroed for this condition.

Suggested change
if (swidget->id == snd_soc_dapm_dai_in && ch_count == out_ref_channels) {
mask = ch_mask;
mask = ch_mask;
/*
* For DAI input widgets where all channels are used,
* disable the shifting logic in the channel mask calculation
* by setting step to 0. This ensures the same mask is applied
* to all devices.
*/

Copilot uses AI. Check for mistakes.
Comment on lines +2344 to +2346
* Set the same channel mask if the widget channel count is the same
* as the FE channels for playback as the audio data is duplicated
* for all speakers in the case. Otherwise, split the channels
Copy link

Copilot AI Sep 4, 2025

Choose a reason for hiding this comment

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

There's a grammatical error in the comment. 'in the case' should be 'in this case' for proper English grammar.

Suggested change
* Set the same channel mask if the widget channel count is the same
* as the FE channels for playback as the audio data is duplicated
* for all speakers in the case. Otherwise, split the channels
* for all speakers in this case. Otherwise, split the channels

Copilot uses AI. Check for mistakes.
mask = GENMASK(step - 1, 0);

/* Set the same mask to all devices if ch_count == out_ref_channels */
if (swidget->id == snd_soc_dapm_dai_in && ch_count == out_ref_channels) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would rather:

if (swidget->id == snd_soc_dapm_dai_in && ch_count == out_ref_channels) {
	/* Set the same mask to all devices if ch_count == out_ref_channels */
	step = 0;
	mask = ch_mask;
} else {
	step = ch_count / blob->alh_cfg.device_count;
	mask =  GENMASK(step - 1, 0);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

but the comment is somehow strange...
"Set the same mask to all devices" and swidget->id == snd_soc_dapm_dai_in, so it is not all devices, but DAIs..

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

but the comment is somehow strange... "Set the same mask to all devices" and swidget->id == snd_soc_dapm_dai_in, so it is not all devices, but DAIs..

I mean blob->alh_cfg.mapping[i].channel_mask, commit message updated.

Currently, we send all channels to all amps and copy the channel_mask
to all ALH DMAs in playback. However, the amp may not have the
capability to run any process and SOF may need to split the channels
and send specific data channel to each amp. In that case, we need
to split the channel_mask in ALH DMA.
Copy the channel mask only if the widget channel count is the same
the FE channels for playback, otherwise, split the channels among the
aggregated DAIs. Like what we did in capture.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
@bardliao
Copy link
Copy Markdown
Collaborator Author

@ujfalusi @ranj063 @lgirdwood This PR is for 4 channel playback. However, the request was postponed. Do we want to merge it now or wait until we really need it?

Copy link
Copy Markdown
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

@singalsu good for your use cases ?

@lgirdwood lgirdwood merged commit 3386645 into thesofproject:topic/sof-dev Oct 22, 2025
8 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants