Skip to content

Topic/asoc ch map mask fix - #5887

Draft
rfvirgil wants to merge 5 commits into
thesofproject:topic/sof-devfrom
CirrusLogic:topic/asoc-ch-map-mask-fix
Draft

Topic/asoc ch map mask fix#5887
rfvirgil wants to merge 5 commits into
thesofproject:topic/sof-devfrom
CirrusLogic:topic/asoc-ch-map-mask-fix

Conversation

@rfvirgil

Copy link
Copy Markdown

Fixes for ASoC not applying a channel count restriction to the codec end of a channel map.

Rename the ch_mask member of snd_soc_dai_link_ch_map to cpu_ch_mask,
as that is what it is used for.

The CPU and codec channel masks are not necessarily the same, and are
quite likely different. SoundWire and I2S/TDM both support assigning
different sample slots to each codec, so for example channel 0 on each
codec could map to different channels at the CPU. So it's quite normal
that the channel mask at the CPU end is different for each codec, but
the codec channel masks are the same for each codec.

Change-Id: Ia40e1f924320b1426ecbf4e04d3e2d8dc9e4a901
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Add a codec_ch_mask member to snd_soc_dai_link_ch_map.

The CPU and codec channel masks are not necessarily the same, and are
quite likely different. SoundWire and I2S/TDM both support assigning
different sample slots to each codec, so for example channel 0 on each
codec could map to different channels at the CPU. So it's quite normal
that the channel mask at the CPU end is different for each codec, but
the codec channel masks are the same for each codec.

Change-Id: Ie2b30fc2dab17a5b2062634fc6bc6439bf83f8e6
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
…arams

In __soc_pcm_hw_params() if there is a snd_soc_dai_link_ch_map with
non-zero codec_ch_mask, use that channel mask to restrict which channels
are enabled on the codec. But only if there isn't a TDM mask.

It is possible that a snd_soc_dai_link_ch_map could include the same codec
multiple times on different CPUs so the for_each_rtd_ch_maps() loop
accumulates the channel masks for all entries of that codec.

If a TDM mask was also set, it takes priority and is used instead of any
possible snd_soc_dai_link_ch_map entries. (They cannot be ANDed together
because the bit positions are indicating different things: TDM is a bit
for each TDM slot, codec_ch_mask is a bit for each codec channel.)

This fixes a problem of incorrect TX channels enabled on the codec when
multiple codecs are aggregated on a single capture link. For example:

- Two CPUs with six 4-channel codecs.
- The machine driver chooses to assign one channel from each codec to
  one channel on the CPU
- But the codec hw_params() would be passed a channel count of 6, which
  (a) is more channels than the codec has and (b) allows enabling channels
  that should not be driving the audio bus.

Change-Id: I54fbdbe6768bec2cb2f2b50e922ac477b7317d90
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
In asoc_sdw_hw_params() set the codec_ch_mask member of struct
snd_soc_dai_link_ch_map for capture streams. ASoC will then pass the
correct number of channels to each codec hw_params(). This prevents
trying to enable more channels on the codec DP than have been allocated
bitslots in the SoundWire frame, which would cause bus clash errors.

In theory codec_ch_mask could also be set for playback streams, but for
those the CPU is the only sender so there is no risk of bus clash.
For playback streams codec_ch_mask is set to 0 to preserve the existing
behavior and avoid introducing bugs.

Change-Id: I0b6e362abda9d91c654140c2470546bc9b6a62b6
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
…ixup

Delete the asoc_sdw_cs_spk_feedback_rtd_init(). This is not needed now
that the ASoC bug it was working around has been fixed. And it was broken
anyway, creating a confusing mapping of amp channels to capture channels.

This code was added to avoid a problem where multiple codec DP outputs
were mapped to the same SoundWire frame bit slot. This would allow a
user to break the SoundWire bus just by enabling mixer outputs using
ALSA controls.

As no production system has used the capture stream, this workaround
was of little consequence and the problem of conflicting DP mappings
was not investigated.

The ASoC bug that enabled too many channels on each codec has now been
fixed. So this workaround can be completely deleted.

Change-Id: Ia4d193bf7db33179277d23a09090178918d2848a
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
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.

1 participant