On Mon, 1 Nov 2010, Pierre-Louis Bossart wrote:
> Merged and cleaned patch based on earlier patches posted
> on alsa-devel by Clemens Ladisch <clemens@xxxxxxxxxx> and
> Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxx>
>
> 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.
>
> More specifically this patch makes a lot of sense when
> PulseAudio relies on timer-based scheduling to access audio
> devices such as HDAudio or Intel SST. Disabling interrupts
> removes two unwanted wake-ups due to period elapsed events
> in low-power playback modes. It also simplifies PulseAudio
> voice modules used for speech calls.
>
> To quote Lennart "This patch looks very interesting and
> desirable. This is something have long been waiting for."
>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxx>
> ---
> core/pcm_lib.c | 8 ++++++--
> core/pcm_native.c | 6 ++++++
> include/asound.h | 4 ++++
> include/pcm.h | 1 +
> pci/hda/hda_intel.c | 9 ++++++---
> pci/oxygen/oxygen_pcm.c | 14 ++++++++++----
> 6 files changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/core/pcm_lib.c b/core/pcm_lib.c
> index a1707cc..4963d8f 100644
> --- a/core/pcm_lib.c
> +++ b/core/pcm_lib.c
> @@ -374,7 +374,8 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
> (unsigned long)runtime->hw_ptr_base);
> }
> /* something must be really wrong */
> - if (delta >= runtime->buffer_size + runtime->period_size) {
> + if (delta >= runtime->buffer_size + runtime->period_size &&
> + !runtime->no_period_irq) {
> hw_ptr_error(substream,
> "Unexpected hw_pointer value %s"
> "(stream=%i, pos=%ld, new_hw_ptr=%ld, "
> @@ -395,6 +396,8 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
> */
> if (runtime->hw.info & SNDRV_PCM_INFO_BATCH)
> goto no_jiffies_check;
> + if (runtime->no_period_irq)
> + goto no_jiffies_check;
> hdelta = delta;
> if (hdelta < runtime->delay)
> goto no_jiffies_check;
> @@ -431,7 +434,8 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
> hw_base = new_hw_ptr - (new_hw_ptr % runtime->buffer_size);
> }
> no_jiffies_check:
> - if (delta > runtime->period_size + runtime->period_size / 2) {
> + if (delta > runtime->period_size + runtime->period_size / 2 &&
> + !runtime->no_period_irq) {
> hw_ptr_error(substream,
> "Lost interrupts? %s"
> "(stream=%i, delta=%ld, new_hw_ptr=%ld, "
Use one 'if' and a goto behind the last condition to simplify things.
> diff --git a/core/pcm_native.c b/core/pcm_native.c
> index 8bc7cb3..92c8c59 100644
> --- a/core/pcm_native.c
> +++ b/core/pcm_native.c
> @@ -412,6 +412,12 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
> goto _error;
> }
>
> + if (params->info & SNDRV_PCM_INFO_NO_PERIOD_IRQ)
> + runtime->no_period_irq = !!(params->flags &
> + SNDRV_PCM_HW_PARAMS_NO_PERIOD_IRQ);
> + else
> + runtime->no_period_irq = 0;
> +
> runtime->access = params_access(params);
> runtime->format = params_format(params);
> runtime->subformat = params_subformat(params);
> diff --git a/include/asound.h b/include/asound.h
> index a1803ec..d4b5f2d 100644
> --- a/include/asound.h
> +++ b/include/asound.h
> @@ -259,6 +259,7 @@ typedef int __bitwise snd_pcm_subformat_t;
> #define SNDRV_PCM_INFO_HALF_DUPLEX 0x00100000 /* only half duplex */
> #define SNDRV_PCM_INFO_JOINT_DUPLEX 0x00200000 /* playback and capture stream are somewhat correlated */
> #define SNDRV_PCM_INFO_SYNC_START 0x00400000 /* pcm support some kind of sync go */
> +#define SNDRV_PCM_INFO_NO_PERIOD_IRQ 0x00800000 /* period interrupt can be disabled */
> #define SNDRV_PCM_INFO_FIFO_IN_FRAMES 0x80000000 /* internal kernel flag - FIFO size is in frames */
>
> typedef int __bitwise snd_pcm_state_t;
> @@ -334,6 +335,9 @@ typedef int snd_pcm_hw_param_t;
> #define SNDRV_PCM_HW_PARAM_LAST_INTERVAL SNDRV_PCM_HW_PARAM_TICK_TIME
>
> #define SNDRV_PCM_HW_PARAMS_NORESAMPLE (1<<0) /* avoid rate resampling */
> +#define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER (1<<1) /* export buffer */
> +#define SNDRV_PCM_HW_PARAMS_NO_PERIOD_IRQ (1<<2) /* disable period
> + interrupts */
>
> struct snd_interval {
> unsigned int min, max;
The PCM protocol version should be increased.
> diff --git a/include/pcm.h b/include/pcm.h
> index dfd9b76..9abb4aa 100644
> --- a/include/pcm.h
> +++ b/include/pcm.h
> @@ -297,6 +297,7 @@ struct snd_pcm_runtime {
> unsigned int info;
> unsigned int rate_num;
> unsigned int rate_den;
> + bool no_period_irq;
I would use an unsigned int bit field rather than bool in this case to
make addition in-line with other structures in this file.
Jaroslav
-----
Jaroslav Kysela <perex@xxxxxxxx>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
