[PATCH v2 1/3] ALSA: pcm: add snd_pcm_period_elapsed() variant without acquiring lock of PCM substream

[Takashi Iwai]

On Thu, 10 Jun 2021 01:16:23 +0200,
Takashi Sakamoto wrote:
> 
> On Wed, Jun 09, 2021 at 05:27:29PM +0200, Takashi Iwai wrote:
> > On Wed, 09 Jun 2021 16:31:43 +0200,
> > Takashi Sakamoto wrote:
> > > diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
> > > index b7e3d8f44511..3488ec1e3674 100644
> > > --- a/sound/core/pcm_lib.c
> > > +++ b/sound/core/pcm_lib.c
> > > @@ -1778,27 +1778,41 @@ int snd_pcm_lib_ioctl(struct snd_pcm_substream *substream,
> > >  EXPORT_SYMBOL(snd_pcm_lib_ioctl);
> > >  
> > >  /**
> > > - * snd_pcm_period_elapsed - update the pcm status for the next period
> > > - * @substream: the pcm substream instance
> > > + * snd_pcm_period_elapsed_under_stream_lock() - update the status of runtime for the next period
> > > + *						under acquired lock of PCM substream.
> > > + * @substream: the instance of pcm substream.
> > > + *
> > > + * This function is called when the batch of audio data frames as the same size as the period of
> > > + * buffer is already processed in audio data transmission.
> > > + *
> > > + * The call of function updates the status of runtime with the latest position of audio data
> > > + * transmission, checks overrun and underrun over buffer, awaken user processes from waiting for
> > > + * available audio data frames, sampling audio timestamp, and performs stop or drain the PCM
> > > + * substream according to configured threshold.
> > > + *
> > > + * The function is intended to use for the case that PCM driver operates audio data frames under
> > > + * acquired lock of PCM substream; e.g. in callback of any operation of &snd_pcm_ops in process
> > > + * context. In any interrupt context, it's preferrable to use ``snd_pcm_period_elapsed()`` instead
> > > + * since lock of PCM substream should be acquired in advance.
> > >   *
> > > - * This function is called from the interrupt handler when the
> > > - * PCM has processed the period size.  It will update the current
> > > - * pointer, wake up sleepers, etc.
> > > + * Developer should pay enough attention that some callbacks in &snd_pcm_ops are done by the call of
> > > + * function:
> > >   *
> > > - * Even if more than one periods have elapsed since the last call, you
> > > - * have to call this only once.
> > > + * - .pointer - to retrieve current position of audio data transmission by frame count or XRUN state.
> > > + * - .trigger - with SNDRV_PCM_TRIGGER_STOP at XRUN or DRAINING state.
> > > + * - .get_time_info - to retrieve audio time stamp if needed.
> > > + *
> > > + * Even if more than one periods have elapsed since the last call, you have to call this only once.
> > > + *
> > > + * Context: Any context in which lock of PCM substream is already acquired. This function may not
> > > + * sleep.
> > 
> > Hm, this text still remains here.  Overlooked?
> 
> It's my intension for documentation of
> snd_pcm_period_elapsed_under_stream_lock() since it's expected to call
> it under acquired lock. Its implementation doesn't yield processor
> voluntarily by itself. If it yielded, it would depend on implementation
> of each driver for struct snd_pcm_ops.{pointer, trigger, get_time_info},
> but it's not preferable implementation of driver, in my opinion.

My point is again about the sleep.  This function may sleep in the
nonatomic mode.  The type of the PCM stream lock depends on it.


Takashi