[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87frqyorzi.wl-tiwai@suse.de>
Date: Wed, 21 Aug 2024 16:44:33 +0200
From: Takashi Iwai <tiwai@...e.de>
To: Zeno Endemann <zeno.endemann@...lbox.org>
Cc: Takashi Iwai <tiwai@...e.de>,
Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
linux-sound@...r.kernel.org,
linux-kernel@...r.kernel.org,
Jaroslav Kysela <perex@...ex.cz>,
Takashi Iwai <tiwai@...e.com>,
Cezary Rojewski <cezary.rojewski@...el.com>,
Christian Brauner <brauner@...nel.org>,
Mark Brown <broonie@...nel.org>,
Pavel Hofman <pavel.hofman@...tera.com>,
David Howells
<dhowells@...hat.com>,
Liam Girdwood <liam.r.girdwood@...ux.intel.com>,
Peter Ujfalusi <peter.ujfalusi@...ux.intel.com>,
Bard Liao <yung-chuan.liao@...ux.intel.com>,
Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>,
Kai Vehmanen <kai.vehmanen@...ux.intel.com>
Subject: Re: [PATCH] ALSA: core: Remove trigger_tstamp_latched
On Wed, 21 Aug 2024 16:27:43 +0200,
Zeno Endemann wrote:
>
> Takashi Iwai wrote on 13.08.24 16:05:
> > On Tue, 13 Aug 2024 15:58:13 +0200,
> > Zeno Endemann wrote:
> >>
> >> Takashi Iwai wrote on 13.08.24 15:41:
> >>> On Tue, 13 Aug 2024 14:54:42 +0200,
> >>> Zeno Endemann wrote:
> >>>>
> >>>> Pierre-Louis Bossart wrote on 13.08.24 10:04:
> >>>>> by focusing on the trigger timestamp I think you're looking at the wrong
> >>>>> side of the problem. The timestamping is improved by using the same
> >>>>> hardware counter for the trigger AND regular timestamp during
> >>>>> playback/capture. If you look at a hardware counter during
> >>>>> playback/capture but the start position is recorded with another method,
> >>>>> would you agree that there's a systematic non-reproducible offset at
> >>>>> each run? You want the trigger and regular timestamps to be measured in
> >>>>> the same way to avoid measurement differences.
> >>>>
> >>>> I am not sure what you are talking about. I have not seen any place in the
> >>>> code where the trigger timestamp is taken in any other more sophisticated
> >>>> way than what the default is doing, i.e. calling snd_pcm_gettime. So I do
> >>>> not see how your custom *trigger* timestamps are done "with another method".
> >>>>
> >>>>> I will not disagree that most applications do not need precise
> >>>>> timestamping, but if you want to try to enable time-of-flight
> >>>>> measurements for presence or gesture detection you will need higher
> >>>>> sampling rates and micro-second level accuracy.
> >>>>
> >>>> I don't know, this sounds very theoretical at best to me. However I do not
> >>>> have the desire to try to further argue and convince you otherwise.
> >>>>
> >>>> Do you want to propose a different solution for the stop trigger timestamp
> >>>> bug? That is my main goal after all.
> >>>
> >>> Ah, I guess that the discussion drifted because of misunderstanding.
> >>>
> >>> This isn't about the accuracy of the audio timestamp, but rather the
> >>> timing of trigger tstamp. The commit 2b79d7a6bf34 ("ALSA: pcm: allow
> >>> for trigger_tstamp snapshot in .trigger") allowed the trigger_tstamp
> >>> taken in the driver's trigger callback. But, the effectiveness of
> >>> this change is dubious, because the timestamp taken in the usual code
> >>> path in PCM core is right after the trigger callback, hence the
> >>> difference should be negligible -- that's the argument.
> >>
> >> Exactly. Sorry if my communication was not clear on that.
> >>
> >>>
> >>> No matter how the fix will be, could you put the Fixes tag pointing to
> >>> the culprit commit(s) at the next submission?
> >>
> >> Will do. I guess I'll have to look up which commit actually enabled the
> >> trigger_tstamp_latched in hda, as 2b79d7a6bf34 has no driver using that
> >> yet, so is not technically the culprit?
> >
> > You can take the HD-audio side, the commit ed610af86a71 ("ALSA: hda:
> > read trigger_timestamp immediately after starting DMA") instead, too.
> > Maybe it doesn't matter much which commit is chosen; both should
> > appear in the same kernel version.
>
> Well, I think I've waited a decent amount of time now for more comments.
> How do we proceed?
>
> I'm still of the opinion that the removal is the most sensible solution,
> so if we agree I could prepare a V2 where I just improve the commit message
> a bit further.
>
> But if we don't have a good enough consensus on this, I'd need some guidance
> which alternate path should be taken to at least fix the bug of bad stop
> trigger timestamps for hda devices (e.g. should I try to fix it also for
> soc/intel/skylake without any testing? That seems to me the only other place
> that should be affected, apart from the generic pci hda code).
IIUC, the achievement of the timestamp at the exact timing was the
goal of that change (which caused a regression unfortunately), so
keeping that feature may still make sense. I'd rather try to fix in
HD-audio side at first.
If Pierre agrees with the removal of the local timestamp call, we can
revert to there afterwards, too.
thanks,
Takashi
Powered by blists - more mailing lists