Re: ASoC: soc-pcm: Don't zero TDM masks in __soc_pcm_open() breaks SOF Audio in Lenovo laptops

[Richard Fitzgerald <rf@opensource.cirrus.com>]

On 9/12/22 14:42, Pierre-Louis Bossart wrote:
> 
> 
> On 12/9/22 01:37, Péter Ujfalusi wrote:
>>
>>
>> On 08/12/2022 18:37, Pierre-Louis Bossart wrote:
>>>
>>>
>>> On 12/8/22 10:17, Mark Brown wrote:
>>>> On Thu, Dec 08, 2022 at 02:02:02PM +0000, Joakim Tjernlund wrote:
>>>>> Several of our Lenovo laptops lost PC audio output in Teams in 5.15.81(also in 5.15.82)
>>>>> Revering above patch:https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.15.y&id=b2ddd76237121155dcadfc4ae77ca1775dfc99f7
>>>>> fixes it.
>>>>>
>>>>> Any idea what the real fix is?
>>>>
>>>> Adding the Intel people.  I've no idea if there's issues with
>>>> dependencies, missing quirks for the hardare or anything in that stable
>>>> version.
>>>
>>> Humm, yes in the past we used the TDM masks to convey the 'stream_tag'
>>> for HDaudio.
>>>
>>> I can still see this in v5.15.85 in sound/soc/sof/intel/hda-dai.c
>>>
>>> 	/* set the stream tag in the codec dai dma params */
>>> 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>>> 		snd_soc_dai_set_tdm_slot(codec_dai, stream_tag, 0, 0, 0);
>>> 	else
>>> 		snd_soc_dai_set_tdm_slot(codec_dai, 0, stream_tag, 0, 0);
>>>
>>>
>>> That was changed in
>>> 636110411ca72 ASoC: Intel/SOF: use set_stream() instead of
>>> set_tdm_slots() for HDAudio
>>>
>>> snd_soc_dai_set_stream(codec_dai, hdac_stream(hext_stream),
>>> substream->stream);
>>>
>>> So my guess is that zeroing out TDM masks has a side effect on older
>>> stable kernels, and that effect is not seen on newer kernels.
>>>
>>> I don't really understand what the side effect might be though.
>>
>> The reason is that on the the tdm mask now became persistent and the HDA
>> code relied on the fact that it is volatile, it is reset for each stream
>> open.
>> The core would do some fixup if the tx/rx_mask is set:
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/sound/soc/soc-pcm.c?h=linux-5.15.y&#n963
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/sound/soc/soc-pcm.c?h=linux-5.15.y&#n968
>>
>> This did not happened when the tx/rx_mask was reset at open - and later
>> set bny the HDA driver, I guess after these checks, so the fixup did not
>> happened, but without resetting them we would use the previously set
>> masks to do fixups for the number of channels (!) based on a stream_tag
>> value, which has nothing to do with channels.
> 
> Agree with the analysis, so what would be the least bad recommendation?
> a) revert the "don't zero TDM masks" patch
> b) backport the change to use set_stream()?

Adding alsa-devel@alsa-project.org and patches@opensource.cirrus.com.
Please add relevant lists when sending emails so that other people who
might need to know, or have an opinion, are aware of the discussion.