[alsa-devel] [PATCH 1/2] ALSA: pcm - Ensure wakeup at each periodic interrupt

[jassi brar]

On Thu, Dec 17, 2009 at 4:02 PM, Takashi Iwai <tiwai at suse.de> wrote:
> At Thu, 17 Dec 2009 15:00:02 +0900,
> jassisinghbrar at gmail.com wrote:
>>
>> From: Jassi Brar <jassi.brar at samsung.com>
>>
>> The check for at least 'avail_min' available data before calling wake_up
>> doesn't always hold good as it does not guarantee callbacks at each periodic
>> interrupt.
>
> Well, avail_min can be greater than period_size.  And, avail_min won't be
> less than period size.
>
> For example, when avail_min = 2.5 x period_size, the driver wakes up
> in periods like 3, 2, 3, 2, ...
correct, but if we ensure wake_up's after each period and let the 'sleepers'
track if the data available is enough or not, we will have more fine grained
control.
The point is:- Waking up _after_ avail_min is working, but does waking up before
avail_min(but at period boundary) break the system?

>
>> An example of such situation is snd_pcm_lib_read1/write1 consuming some space
>> of the period and going to sleep from wait_for_avail_min upon syncing with
>> the DMA pointer. Clearly just the remainder of period size is needed, but
>> wake_up is called only after _two_ periodic interrupts from that point.
>
> In that case, the original behavior is correct.
going by current implementation, that is correct, but is that desirable?

thanks.

>
> thanks,
>
> Takashi
>
>>
>> Signed-off-by: Jassi Brar <jassi.brar at samsung.com>
>> ---
>>  sound/core/pcm_lib.c |   10 ++++++++--
>>  1 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
>> index 30f4108..ee2d7b9 100644
>> --- a/sound/core/pcm_lib.c
>> +++ b/sound/core/pcm_lib.c
>> @@ -189,7 +189,7 @@ snd_pcm_update_hw_ptr_pos(struct snd_pcm_substream *substream,
>>  static int snd_pcm_update_hw_ptr_post(struct snd_pcm_substream *substream,
>>                                     struct snd_pcm_runtime *runtime)
>>  {
>> -     snd_pcm_uframes_t avail;
>> +     snd_pcm_uframes_t avail, prd_short;
>>
>>       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>>               avail = snd_pcm_playback_avail(runtime);
>> @@ -208,8 +208,14 @@ static int snd_pcm_update_hw_ptr_post(struct snd_pcm_substream *substream,
>>                       return -EPIPE;
>>               }
>>       }
>> -     if (avail >= runtime->control->avail_min)
>> +
>> +     prd_short = runtime->period_size
>> +                     - runtime->status->hw_ptr % runtime->period_size;
>> +
>> +     if (avail >= runtime->control->avail_min
>> +                     || avail >= prd_short)
>>               wake_up(&runtime->sleep);
>> +
>>       return 0;
>>  }
>>
>> --
>> 1.6.2.5
>>
>