Re: [PATCH] ALSA: core: Remove trigger_tstamp_latched

[Zeno Endemann <zeno.endemann@xxxxxxxxxxx>]

Takashi Iwai wrote on 13.08.24 11:26:
> As long as I read the thread, there seems still use cases for this
> high resolution time counter, so I'm rather for fixing the bugs of
> bogus trigger tstamp than dropping the feature.

I still remain fully unconvinced that moving the point where the
timestamp is taken back by a couple of fairly trivial CPU instructions
does anything of value, but if that convinces you I suppose I'll have
to propose an alternative.

In that case I'd rather simply go with what also Jaroslav Kysela
suggested, and add a timestamp read in the azx_pcm_trigger when stopping.
There may be some Intel drivers that have their own trigger callback,
where this bug would still remain, but I believe it is fair that I do not
worry about that, as I wouldn't be able to test it anyway.

> I still wonder, though, why the trigger_tstamp becomes *earlier* when
> runtime->trigger_tstamp_latched is set.  I thought the same value
> would be kept, instead.  What am I overlooking?

The trigger timestamp is kept separately from the runtime timestamp. So
the runtime timestamp gets updated, but the trigger timestamp does not,
thus the stop trigger timestamp is the same as the start trigger timestamp,
which is earlier than any runtime timestamp.

> In anyway, if it's only about the missing trigger_tstamp update at PCM
> trigger actions other than START, it could be fixed simply by clearing
> trigger_tstamp_latched flag, too.  Alternatively, move the conditional
> call to snd_pcm_post_start() instead of snd_pcm_trigger_tstamp()
> itself, something like below.

I strongly advise against that. That would make the trigger_tstamp_latched
flag even more confusing, as then the driver would only be able to override
the start trigger timestamp, not any other.

If that flag should stay, it may also be a good idea to rename it to
something more descriptive, like "trigger_tstamp_managed_by_driver".
With simply "latched" it is unclear whether it is only latched for the
next timestamp that would be taken (and need to be re-raised by the driver)
or stays in effect permanently (or, as it is now, stays in effect until
the next start)...