* Re: ASoC: soc-pcm: Don't zero TDM masks in __soc_pcm_open() breaks SOF Audio in Lenovo laptops
[not found] ` <a31e0bee-6972-0c0f-7579-449fb412a0b2@linux.intel.com>
@ 2022-12-09 14:55 ` Richard Fitzgerald
[not found] ` <b54b48cf-3680-9f0c-8ca9-db904e4a57ec@linux.intel.com>
1 sibling, 0 replies; 4+ messages in thread
From: Richard Fitzgerald @ 2022-12-09 14:55 UTC (permalink / raw)
To: Pierre-Louis Bossart, Péter Ujfalusi, Mark Brown, Joakim Tjernlund
Cc: sashal, alsa-devel, Kai Vehmanen, patches, Ranjani Sridharan, Bard Liao
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ASoC: soc-pcm: Don't zero TDM masks in __soc_pcm_open() breaks SOF Audio in Lenovo laptops
[not found] ` <Y5Nfyo7VARU0TKoB@sirena.org.uk>
@ 2022-12-09 16:26 ` Richard Fitzgerald
2022-12-13 16:57 ` Joakim Tjernlund
0 siblings, 1 reply; 4+ messages in thread
From: Richard Fitzgerald @ 2022-12-09 16:26 UTC (permalink / raw)
To: Mark Brown, Péter Ujfalusi
Cc: sashal, alsa-devel, Kai Vehmanen, patches, Pierre-Louis Bossart,
Ranjani Sridharan, Joakim Tjernlund, Bard Liao
On 9/12/22 16:18, Mark Brown wrote:
> On Fri, Dec 09, 2022 at 05:38:54PM +0200, Péter Ujfalusi wrote:
>> On 09/12/2022 16:42, Pierre-Louis Bossart wrote:
>
>>> 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()?
>
>> I would vote for b) unless other platforms report regressions.
>
> I don't really care either way given the backport policy.
Neither do I.
If you want to minimize risk, revert the patch in older backports.
Lessons learned:
Don't hijack a data member to pass something different from what it
is intended to hold.
Don't depend on a bug.
Don't assume all code is using a data member for what is supposed to be
in that member.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ASoC: soc-pcm: Don't zero TDM masks in __soc_pcm_open() breaks SOF Audio in Lenovo laptops
2022-12-09 16:26 ` Richard Fitzgerald
@ 2022-12-13 16:57 ` Joakim Tjernlund
2022-12-13 17:12 ` Pierre-Louis Bossart
0 siblings, 1 reply; 4+ messages in thread
From: Joakim Tjernlund @ 2022-12-13 16:57 UTC (permalink / raw)
To: rf, broonie, peter.ujfalusi
Cc: sashal, alsa-devel, kai.vehmanen, patches, pierre-louis.bossart,
ranjani.sridharan, yung-chuan.liao
On Fri, 2022-12-09 at 16:26 +0000, Richard Fitzgerald wrote:
> On 9/12/22 16:18, Mark Brown wrote:
> > On Fri, Dec 09, 2022 at 05:38:54PM +0200, Péter Ujfalusi wrote:
> > > On 09/12/2022 16:42, Pierre-Louis Bossart wrote:
> >
> > > > 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()?
> >
> > > I would vote for b) unless other platforms report regressions.
> >
> > I don't really care either way given the backport policy.
>
> Neither do I.
> If you want to minimize risk, revert the patch in older backports.
>
> Lessons learned:
> Don't hijack a data member to pass something different from what it
> is intended to hold.
> Don't depend on a bug.
> Don't assume all code is using a data member for what is supposed to be
> in that member.
Did you reach a conclusion w.r.t what the should be?
Jocke
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ASoC: soc-pcm: Don't zero TDM masks in __soc_pcm_open() breaks SOF Audio in Lenovo laptops
2022-12-13 16:57 ` Joakim Tjernlund
@ 2022-12-13 17:12 ` Pierre-Louis Bossart
0 siblings, 0 replies; 4+ messages in thread
From: Pierre-Louis Bossart @ 2022-12-13 17:12 UTC (permalink / raw)
To: Joakim Tjernlund, rf, broonie, peter.ujfalusi
Cc: sashal, alsa-devel, kai.vehmanen, patches, ranjani.sridharan,
yung-chuan.liao
On 12/13/22 10:57, Joakim Tjernlund wrote:
> On Fri, 2022-12-09 at 16:26 +0000, Richard Fitzgerald wrote:
>> On 9/12/22 16:18, Mark Brown wrote:
>>> On Fri, Dec 09, 2022 at 05:38:54PM +0200, Péter Ujfalusi wrote:
>>>> On 09/12/2022 16:42, Pierre-Louis Bossart wrote:
>>>
>>>>> 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()?
>>>
>>>> I would vote for b) unless other platforms report regressions.
>>>
>>> I don't really care either way given the backport policy.
>>
>> Neither do I.
>> If you want to minimize risk, revert the patch in older backports.
>>
>> Lessons learned:
>> Don't hijack a data member to pass something different from what it
>> is intended to hold.
>> Don't depend on a bug.
>> Don't assume all code is using a data member for what is supposed to be
>> in that member.
>
> Did you reach a conclusion w.r.t what the should be?
I read the consensus is for a backport of the 'set_stream' stuff. You're
welcome to submit a patch.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-12-13 17:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <f7a515a0d5429b41348341874b78f3c3254970e6.camel@infinera.com>
[not found] ` <Y5IOEAw5pjAvGgiN@sirena.org.uk>
[not found] ` <8fa316d5-b525-2207-9092-da14f1d77232@linux.intel.com>
[not found] ` <ade9deca-2f15-a3a6-97c3-7198f1cf0da0@linux.intel.com>
[not found] ` <a31e0bee-6972-0c0f-7579-449fb412a0b2@linux.intel.com>
2022-12-09 14:55 ` ASoC: soc-pcm: Don't zero TDM masks in __soc_pcm_open() breaks SOF Audio in Lenovo laptops Richard Fitzgerald
[not found] ` <b54b48cf-3680-9f0c-8ca9-db904e4a57ec@linux.intel.com>
[not found] ` <Y5Nfyo7VARU0TKoB@sirena.org.uk>
2022-12-09 16:26 ` Richard Fitzgerald
2022-12-13 16:57 ` Joakim Tjernlund
2022-12-13 17:12 ` Pierre-Louis Bossart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.