[PATCH] ALSA: core: Remove trigger_tstamp_latched

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



The trigger_tstamp_latched hook was introduced to allow drivers to
provide their own trigger timestamp instead of the default generated
one for higher accuracy. This makes sense in theory, but in practice
the only place that uses this is the hda core, and:

* The custom timestamp there does not seem to be a meaningful
  improvement over the default one; There is virtually no code in
  between them, so I measured only a difference of around 300ns in a
  KVM VM with ich9-intel-hda device.
* It is also bugged as it does not set a timestamp when the stream
  stops.
* It creates a pitfall for hda driver writers; Calling
  snd_hdac_stream_timecounter_init implicitly makes them responsible
  for generating these timestamps.

Since there is no real good use of this facility, I propose to remove
it.

I reported the bug initially on github (see below), there one can also
find a reproducer userspace app, as well as some other potential ways
to fix this issue, in case this removal is not accepted.

Cc'ing the Intel ASoC maintainers, as the skl-pcm.c is using the
snd_hdac_stream_timecounter_init function this patch modifies.

Closes: https://github.com/alsa-project/alsa-lib/issues/387
Signed-off-by: Zeno Endemann <zeno.endemann@xxxxxxxxxxx>
---
 include/sound/pcm.h     | 1 -
 sound/core/pcm_native.c | 4 +---
 sound/hda/hdac_stream.c | 6 ------
 3 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index ac8f3aef9205..3539af9f733e 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -361,7 +361,6 @@ struct snd_pcm_runtime {
 	snd_pcm_state_t suspended_state; /* suspended stream state */
 	struct snd_pcm_substream *trigger_master;
 	struct timespec64 trigger_tstamp;	/* trigger timestamp */
-	bool trigger_tstamp_latched;     /* trigger timestamp latched in low-level driver/hardware */
 	int overrange;
 	snd_pcm_uframes_t avail_max;
 	snd_pcm_uframes_t hw_ptr_base;	/* Position at buffer restart */
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 4057f9f10aee..ced5bd2d7ebb 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1194,8 +1194,7 @@ static void snd_pcm_trigger_tstamp(struct snd_pcm_substream *substream)
 	if (runtime->trigger_master == NULL)
 		return;
 	if (runtime->trigger_master == substream) {
-		if (!runtime->trigger_tstamp_latched)
-			snd_pcm_gettime(runtime, &runtime->trigger_tstamp);
+		snd_pcm_gettime(runtime, &runtime->trigger_tstamp);
 	} else {
 		snd_pcm_trigger_tstamp(runtime->trigger_master);
 		runtime->trigger_tstamp = runtime->trigger_master->runtime->trigger_tstamp;
@@ -1422,7 +1421,6 @@ static int snd_pcm_pre_start(struct snd_pcm_substream *substream,
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
 	    !snd_pcm_playback_data(substream))
 		return -EPIPE;
-	runtime->trigger_tstamp_latched = false;
 	runtime->trigger_master = substream;
 	return 0;
 }
diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index b53de020309f..271d42b765fc 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -660,14 +660,11 @@ static void azx_timecounter_init(struct hdac_stream *azx_dev,
  *
  * Initializes the time counter of streams marked by the bit flags (each
  * bit corresponds to the stream index).
- * The trigger timestamp of PCM substream assigned to the given stream is
- * updated accordingly, too.
  */
 void snd_hdac_stream_timecounter_init(struct hdac_stream *azx_dev,
 				      unsigned int streams)
 {
 	struct hdac_bus *bus = azx_dev->bus;
-	struct snd_pcm_runtime *runtime = azx_dev->substream->runtime;
 	struct hdac_stream *s;
 	bool inited = false;
 	u64 cycle_last = 0;
@@ -681,9 +678,6 @@ void snd_hdac_stream_timecounter_init(struct hdac_stream *azx_dev,
 			}
 		}
 	}
-
-	snd_pcm_gettime(runtime, &runtime->trigger_tstamp);
-	runtime->trigger_tstamp_latched = true;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_stream_timecounter_init);
 
-- 
2.46.0





[Index of Archives]     [Pulseaudio]     [Linux Audio Users]     [ALSA Devel]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux