Hello Mike, AFAIU, this patch is wrong. Please correct me, maybe I misunderstand something. > The problem that this patch seeks to resolve is that when userland asks for > the delay The userspace asks not for delay, but for the pointer. You modify the function, which is called `snd_bcm2835_pcm_pointer`. Here you are supposed to increase `alsa_stream->pos` with the proper offset. Instead, you imitate a delay, but in fact the delay is not increased. So, the proper solution should be to fix the reported pointer. As a result, userspace will recieve the correct delay, instead of these crazy 10 ms. > FYI, there is > a discussion of the effects of a downstream equivalent of this suggested patch > at: > https://github.com/raspberrypi/firmware/issues/1026#issuecomment-415746016. Thank you for the link, it clarified for me what you try to achieve. On 10/22/18 21:17, Mike Brady wrote: > When the BCM2835 audio output is used, userspace sees a jitter up to 10ms > in the audio position, aka "delay" -- the number of frames that must > be output before a new frame would be played. > Make this a bit nicer for userspace by interpolating the position > using the CPU clock. > The overhead is small -- an extra ktime_get() every time a GPU message > is sent -- and another call and a few calculations whenever the delay > is sought from userland. > At 48,000 frames per second, i.e. approximately 20 microseconds per > frame, it would take a clock inaccuracy of > 20 microseconds in 10 milliseconds -- 2,000 parts per million -- > to result in an inaccurate estimate, whereas > crystal- or resonator-based clocks typically have an > inaccuracy of 10s to 100s of parts per million. > > Signed-off-by: Mike Brady <mikebrady@xxxxxxxxxx> > --- > Changes in v2 -- remove inappropriate addition of SNDRV_PCM_INFO_BATCH flag > > .../vc04_services/bcm2835-audio/bcm2835-pcm.c | 20 +++++++++++++++++++ > .../vc04_services/bcm2835-audio/bcm2835.h | 1 + > 2 files changed, 21 insertions(+) > > diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c > index e66da11af5cf..9053b996cada 100644 > --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c > +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c > @@ -74,6 +74,7 @@ void bcm2835_playback_fifo(struct bcm2835_alsa_stream *alsa_stream, > atomic_set(&alsa_stream->pos, pos); > > alsa_stream->period_offset += bytes; > + alsa_stream->interpolate_start = ktime_get(); > if (alsa_stream->period_offset >= alsa_stream->period_size) { > alsa_stream->period_offset %= alsa_stream->period_size; > snd_pcm_period_elapsed(substream); > @@ -243,6 +244,7 @@ static int snd_bcm2835_pcm_prepare(struct snd_pcm_substream *substream) > atomic_set(&alsa_stream->pos, 0); > alsa_stream->period_offset = 0; > alsa_stream->draining = false; > + alsa_stream->interpolate_start = ktime_get(); > > return 0; > } > @@ -292,6 +294,24 @@ snd_bcm2835_pcm_pointer(struct snd_pcm_substream *substream) > { > struct snd_pcm_runtime *runtime = substream->runtime; > struct bcm2835_alsa_stream *alsa_stream = runtime->private_data; > + ktime_t now = ktime_get(); > + > + /* Give userspace better delay reporting by interpolating between GPU > + * notifications, assuming audio speed is close enough to the clock > + * used for ktime > + */ > + > + if ((ktime_to_ns(alsa_stream->interpolate_start)) && > + (ktime_compare(alsa_stream->interpolate_start, now) < 0)) { > + u64 interval = > + (ktime_to_ns(ktime_sub(now, > + alsa_stream->interpolate_start))); > + u64 frames_output_in_interval = > + div_u64((interval * runtime->rate), 1000000000); > + snd_pcm_sframes_t frames_output_in_interval_sized = > + -frames_output_in_interval; > + runtime->delay = frames_output_in_interval_sized; > + } > > return snd_pcm_indirect_playback_pointer(substream, > &alsa_stream->pcm_indirect, > diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h > index e13435d1c205..595ad584243f 100644 > --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h > +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h > @@ -78,6 +78,7 @@ struct bcm2835_alsa_stream { > unsigned int period_offset; > unsigned int buffer_size; > unsigned int period_size; > + ktime_t interpolate_start; > > struct bcm2835_audio_instance *instance; > int idx; > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
