[alsa-devel] [PATCH] ALSA: add support for disabling period irq

[Takashi Iwai]

At Tue, 2 Nov 2010 10:22:32 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Tue, 2 Nov 2010, Takashi Iwai wrote:
> 
> > At Tue, 2 Nov 2010 10:02:49 +0100 (CET),
> > Jaroslav Kysela wrote:
> >>
> >> On Tue, 2 Nov 2010, Takashi Iwai wrote:
> >>
> >>> At Tue, 2 Nov 2010 09:26:48 +0100 (CET),
> >>> Jaroslav Kysela wrote:
> >>>>
> >>>> On Tue, 2 Nov 2010, Takashi Iwai wrote:
> >>>>
> >>>>> At Tue, 2 Nov 2010 09:17:33 +0100 (CET),
> >>>>> Jaroslav Kysela wrote:
> >>>>>>
> >>>>>> On Tue, 2 Nov 2010, Takashi Iwai wrote:
> >>>>>>
> >>>>>>> At Mon,  1 Nov 2010 17:12:53 -0500,
> >>>>>>> Pierre-Louis Bossart wrote:
> >>>>>>>>
> >>>>>>>> Merged and cleaned patch based on earlier patches posted
> >>>>>>>> on alsa-devel by Clemens Ladisch <clemens at ladisch.de> and
> >>>>>>>> Pierre-Louis Bossart <pierre-louis.bossart at intel.com>
> >>>>>>>>
> >>>>>>>> This patch disables period interrupts which are not
> >>>>>>>> needed when the application relies on a system timer
> >>>>>>>> to wake-up and refill the ring buffer. The behavior of
> >>>>>>>> the driver is left unchanged, and interrupts are only
> >>>>>>>> disabled if the application requests this configuration.
> >>>>>>>> The behavior in case of underruns is slightly different,
> >>>>>>>> instead of being detected during the period interrupts the
> >>>>>>>> underruns are detected when the application calls
> >>>>>>>> snd_pcm_update_avail, which in turns forces a refresh of the
> >>>>>>>> hw pointer and shows the buffer is empty.
> >>>>>>>
> >>>>>>> So, this silently assumes that the applications do call
> >>>>>>> snd_pcm_update_avail() appropriately at the right timing?
> >>>>>>> If so, any sense to check XRUN in the driver at all...?
> >>>>>>>
> >>>>>>> And, even more, any sense to report the incremental position by this
> >>>>>>> approach?  The only reliable information in this case is the offset in
> >>>>>>> the ring buffer.  The linear position as the current ALSA API provides
> >>>>>>> isn't guaranteed without the period irq.
> >>>>>>
> >>>>>> We can detect the buffer size crossing using jiffies, but I agree, it's
> >>>>>> something which should be added to the patch to not break hw_ptr in
> >>>>>> case when the application does not call the update function in time.
> >>>>>> We have both values in runtime->hw_ptr_jiffies and
> >>>>>> runtime->hw_ptr_buffer_jiffies.
> >>>>>
> >>>>> But I thought this patch also disabled the jiffies check?
> >>>>
> >>>> These values are not related only to the debug jiffies check. I'm using
> >>>> these values also to detect the double irq acks which were discovered
> >>>> recently.
> >>>
> >>> Ah, OK, I overlooked it.
> >>>
> >>>>> I feel that some bottom-line check is needed if we keep the linear
> >>>>> position over the buffer size even without the period irq.
> >>>>> If the app doesn't care, OK fine, the driver shouldn't care, too.
> >>>>> But then it doesn't make sense to keep the linear position, either.
> >>>>
> >>>> But if we can keep the linear position using the light-weight jiffies
> >>>> check, it's probably OK to keep it rather than changing the hw_ptr
> >>>> behaviour.
> >>>
> >>> Well, but the point of the patch is to avoid wakeups as much as
> >>> possible.  The jiffies-check adds an unconditional wakeup for each
> >>> buffer size, thus it's against the purpose of the patch.
> >>
> >> I don't see any wakeup.
> >
> > The CPU is woken up.  This is to be avoided.
> 
> Nope. No timers are used. The jiffies check and hw_ptr correction should 
> be done from the avail_update call invoked from an application.

OK, point taken.


Takashi