[RFC] disabling ALSA period interrupts

[RFC] disabling ALSA period interrupts

[pl bossart]

[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]

Howdy,
When PulseAudio is used and all PCM is routed through PulseAudio
(Fedora, Meego, etc), the notion of ALSA periods isn't very useful.
PulseAudio uses a timer to refill buffers and the period interrupts
are not used at all.

So why not disable them entirely to reduce the number of wakeups? This
is possible with a number of existing DMA controllers. The simple
patch attached shows a proof-of-concept on HDAudio, it's been working
for 5 hours on my Fedora12 laptop without any glitches and powertop
does show _zero_ wakeups from the HDAudio controller (except on
startup). I am told by my colleagues working in Windows environments
that this is what's done on Vista/Windows7 btw. This isn't to say
Windows is great but that artificial generation of not-needed
interrupts is avoidable.

There are probably some cases where you don't want this type of
behavior (broken hardware, legacy code with multiple-buffering,
disabled timer in PulseAudio), so I think it would make sense to
request the disabling of interrupts when hw_params are set, since this
is also the time when period sizes are set. I am aware that some
changes would be needed in pcm_lib.c, where all the error checks are
done. Takashi, Jaroslav, Lennart, what do you think?
Feedback and suggestions welcome.
Cheers
- Pierre

[-- Attachment #2: 0001-hda-remove-HDA-interrupts-when-buffer-is-processed.patch --]
[-- Type: application/octet-stream, Size: 2348 bytes --]

From 1edc402dc0a1f44117b910e4a42c5f6e6bc6b27f Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart <pierre-louis.bossart@intel.com>
Date: Thu, 29 Apr 2010 17:00:59 -0500
Subject: [PATCH] [hda] remove HDA interrupts when buffer is processed
 This patch assumes a timer is used to wake-up the system and refill buffers
 (as PulseAudio does)

---
 core/pcm_lib.c      |    2 ++
 pci/hda/hda_intel.c |   14 ++++++++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/core/pcm_lib.c b/core/pcm_lib.c
index a2ff861..d1d5f77 100644
--- a/core/pcm_lib.c
+++ b/core/pcm_lib.c
@@ -420,6 +420,7 @@ 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 0 // skip when interrupts are not enabled
 	if (delta > runtime->period_size + runtime->period_size / 2) {
 		hw_ptr_error(substream,
 			     "Lost interrupts? %s"
@@ -430,6 +431,7 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 			     (long)new_hw_ptr,
 			     (long)old_hw_ptr);
 	}
+#endif
 
 	if (runtime->status->hw_ptr == new_hw_ptr)
 		return 0;
diff --git a/pci/hda/hda_intel.c b/pci/hda/hda_intel.c
index 8b6815d..c64106e 100644
--- a/pci/hda/hda_intel.c
+++ b/pci/hda/hda_intel.c
@@ -1158,6 +1158,11 @@ static int setup_bdle(struct snd_pcm_substream *substream,
 {
 	u32 *bdl = *bdlp;
 
+#define NO_INTERRUPTS
+#ifdef  NO_INTERRUPTS
+    snd_printk(KERN_ERR SFX "plb- setup_bdle: interrupts disabled\n");
+#endif
+
 	while (size > 0) {
 		dma_addr_t addr;
 		int chunk;
@@ -1176,7 +1181,13 @@ static int setup_bdle(struct snd_pcm_substream *substream,
 		 * only when the whole fragment is processed
 		 */
 		size -= chunk;
+
+#ifdef NO_INTERRUPTS
+		bdl[3] = 0; /* no interrupts when buffer is processed */
+#else
 		bdl[3] = (size || !with_ioc) ? 0 : cpu_to_le32(0x01);
+#endif
+
 		bdl += 4;
 		azx_dev->frags++;
 		ofs += chunk;
@@ -1937,6 +1948,9 @@ static void azx_irq_pending_work(struct work_struct *work)
 			if (azx_position_ok(chip, azx_dev)) {
 				azx_dev->irq_pending = 0;
 				spin_unlock(&chip->reg_lock);
+#ifdef NO_INTERRUPTS
+				snd_printk(KERN_ERR SFX "plb- period elapsed: THIS SHOULD NOT HAPPEN \n");
+#endif
 				snd_pcm_period_elapsed(azx_dev->substream);
 				spin_lock(&chip->reg_lock);
 			} else
-- 
1.6.6.1


[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/4/30 pl bossart <bossart.nospam@gmail.com>

> Howdy,
> When PulseAudio is used and all PCM is routed through PulseAudio
> (Fedora, Meego, etc), the notion of ALSA periods isn't very useful.
> PulseAudio uses a timer to refill buffers and the period interrupts
> are not used at all.
>
> So why not disable them entirely to reduce the number of wakeups? This
> is possible with a number of existing DMA controllers. The simple
> patch attached shows a proof-of-concept on HDAudio, it's been working
> for 5 hours on my Fedora12 laptop without any glitches and powertop
> does show _zero_ wakeups from the HDAudio controller (except on
> startup). I am told by my colleagues working in Windows environments
> that this is what's done on Vista/Windows7 btw. This isn't to say
> Windows is great but that artificial generation of not-needed
> interrupts is avoidable.
>
> There are probably some cases where you don't want this type of
> behavior (broken hardware, legacy code with multiple-buffering,
> disabled timer in PulseAudio), so I think it would make sense to
> request the disabling of interrupts when hw_params are set, since this
> is also the time when period sizes are set. I am aware that some
> changes would be needed in pcm_lib.c, where all the error checks are
> done. Takashi, Jaroslav, Lennart, what do you think?
> Feedback and suggestions welcome.
> Cheers
> - Pierre
>

do you mean that it only work on HDaudio and not intel8x0 ?

How about other drivers ?

How pulseaudio and alsa driver resume/suspend from hibernation seem to be a
mystery ?

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/4/30 pl bossart <bossart.nospam@gmail.com>

> > do you mean that it only work on HDaudio and not intel8x0 ?
> >
> > How about other drivers ?
>
> It's driver/hardware specific, I guess everyone would have to enable
> this in their drivers. I don't understand what 8x0 does...You're
> welcome to test and do the same for 8x0 if you have the specs and know
> how to disable interrupts.
>
>
Any side effect of this disabling interrupt  ?

Do you mean that all applications must use pulse api instead of alsa-pulse
plugin ?

Did your test included those voip application which require low latency ?

did your test only play one audio stream to the sound server ?

Do those alsa application still work with "hw" device when pulseaudio is
disabled ?

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/4/30 pl bossart <bossart.nospam@gmail.com>

> Howdy,
> When PulseAudio is used and all PCM is routed through PulseAudio
> (Fedora, Meego, etc), the notion of ALSA periods isn't very useful.
> PulseAudio uses a timer to refill buffers and the period interrupts
> are not used at all.
>
> So why not disable them entirely to reduce the number of wakeups? This
> is possible with a number of existing DMA controllers. The simple
> patch attached shows a proof-of-concept on HDAudio, it's been working
> for 5 hours on my Fedora12 laptop without any glitches and powertop
> does show _zero_ wakeups from the HDAudio controller (except on
> startup). I am told by my colleagues working in Windows environments
> that this is what's done on Vista/Windows7 btw. This isn't to say
> Windows is great but that artificial generation of not-needed
> interrupts is avoidable.
>

Do your test need the patch which you have posted to pulseaudio developer
mailing list

http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/6671

it seem that fix should be in driver side if it is hardware specific

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/4/30 pl bossart <bossart.nospam@gmail.com>

> Howdy,
> When PulseAudio is used and all PCM is routed through PulseAudio
> (Fedora, Meego, etc), the notion of ALSA periods isn't very useful.
> PulseAudio uses a timer to refill buffers and the period interrupts
> are not used at all.
>
> So why not disable them entirely to reduce the number of wakeups? This
> is possible with a number of existing DMA controllers. The simple
> patch attached shows a proof-of-concept on HDAudio, it's been working
> for 5 hours on my Fedora12 laptop without any glitches and powertop
> does show _zero_ wakeups from the HDAudio controller (except on
> startup). I am told by my colleagues working in Windows environments
> that this is what's done on Vista/Windows7 btw. This isn't to say
> Windows is great but that artificial generation of not-needed
> interrupts is avoidable.
>
> There are probably some cases where you don't want this type of
> behavior (broken hardware, legacy code with multiple-buffering,
> disabled timer in PulseAudio), so I think it would make sense to
> request the disabling of interrupts when hw_params are set, since this
> is also the time when period sizes are set. I am aware that some
> changes would be needed in pcm_lib.c, where all the error checks are
> done. Takashi, Jaroslav, Lennart, what do you think?
> Feedback and suggestions welcome.
> Cheers
> - Pierre
>

How about SPDIF/HDMI and AC3 passthrough which PA seem not supported ?

what is your definition of legacy code with multiple buffering ?

How low latency can the application have when this mode can achieved ?

cases which may not want this type of behaviour
1) desktop computer which power saving is not main concern
2) players want to play interactive game or voip which require low latency
3) Consumers also want the ability to play back two different audio tracks,
such as a CD and a DVD simultaneously, which can't be done using current
audio solutions. Intel HD Audio features multi-streaming capabilities that
give users the ability to send two or more different audio streams to
different locations at the same time, from the same PC.

http://www.intel.com/design/chipsets/hdaudio.htm

Re: [RFC] disabling ALSA period interrupts

[Clemens Ladisch]

pl bossart wrote:
> When PulseAudio is used and all PCM is routed through PulseAudio
> (Fedora, Meego, etc), the notion of ALSA periods isn't very useful.
> So why not disable them entirely to reduce the number of wakeups? ...
> There are probably some cases where you don't want this type of
> behavior (broken hardware, legacy code with multiple-buffering,
> disabled timer in PulseAudio),

It's interesting that all ALSA applications except PA are "legacy".  :)

> so I think it would make sense to request the disabling of interrupts
> when hw_params are set, since this is also the time when period sizes
> are set.

Yes.  For compatibility with the existing ALSA API, this needs to be
a flag that is not enabled by default.

> I am aware that some changes would be needed in pcm_lib.c, where all
> the error checks are done.

Plus all the API changes in the ALSA kernel framework, the ALSA kernel/
userspace interface, and the alsa-lib interface.

I'll see if I can get this done over the weekend ...


Regards,
Clemens

Re: [RFC] disabling ALSA period interrupts

[pl bossart]

Hi Clemens,

>> When PulseAudio is used and all PCM is routed through PulseAudio
>> (Fedora, Meego, etc), the notion of ALSA periods isn't very useful.
>> So why not disable them entirely to reduce the number of wakeups? ...
>> There are probably some cases where you don't want this type of
>> behavior (broken hardware, legacy code with multiple-buffering,
>> disabled timer in PulseAudio),
>
> It's interesting that all ALSA applications except PA are "legacy".  :)

Ha. Nice slip. I didn't mean legacy was bad... It's perfectly ok to
use multiple buffers and interrupts in a variety of apps.

>> so I think it would make sense to request the disabling of interrupts
>> when hw_params are set, since this is also the time when period sizes
>> are set.
>
> Yes.  For compatibility with the existing ALSA API, this needs to be
> a flag that is not enabled by default.

Agreed. This shouldn't even be mandatory since this option might not
be possible in all platforms.

>> I am aware that some changes would be needed in pcm_lib.c, where all
>> the error checks are done.
>
> Plus all the API changes in the ALSA kernel framework, the ALSA kernel/
> userspace interface, and the alsa-lib interface.

I am not following this point. If you add a simple flag to an existing
interface, why would we need to change the kernel/userspace inferface?
This change should be possible in a backwards compatible manner as an
additional option provided to the application developer.
Cheers
-Pierre

Re: [RFC] disabling ALSA period interrupts

[pl bossart]

>
> Do your test need the patch which you have posted to pulseaudio developer
> mailing list
>
> http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/6671
>
> it seem that fix should be in driver side if it is hardware specific

No it's totally unrelated.

Re: [RFC] disabling ALSA period interrupts

[Liam Girdwood]

On Thu, 2010-04-29 at 17:38 -0500, pl bossart wrote:
> Howdy,
> When PulseAudio is used and all PCM is routed through PulseAudio
> (Fedora, Meego, etc), the notion of ALSA periods isn't very useful.
> PulseAudio uses a timer to refill buffers and the period interrupts
> are not used at all.
> 
> So why not disable them entirely to reduce the number of wakeups? This
> is possible with a number of existing DMA controllers. The simple
> patch attached shows a proof-of-concept on HDAudio, it's been working
> for 5 hours on my Fedora12 laptop without any glitches and powertop
> does show _zero_ wakeups from the HDAudio controller (except on
> startup). I am told by my colleagues working in Windows environments
> that this is what's done on Vista/Windows7 btw. This isn't to say
> Windows is great but that artificial generation of not-needed
> interrupts is avoidable.
> 
> There are probably some cases where you don't want this type of
> behavior (broken hardware, legacy code with multiple-buffering,
> disabled timer in PulseAudio), so I think it would make sense to
> request the disabling of interrupts when hw_params are set, since this
> is also the time when period sizes are set. I am aware that some
> changes would be needed in pcm_lib.c, where all the error checks are
> done. Takashi, Jaroslav, Lennart, what do you think?
> Feedback and suggestions welcome.
> Cheers
> - Pierre

How do you handle any clock drift here between the HDA hardware
interface clock and the PA timer ?

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [RFC] disabling ALSA period interrupts

[pl bossart]

Hi Liam,

> How do you handle any clock drift here between the HDA hardware
> interface clock and the PA timer ?

Good question. This is already handled by PulseAudio. The timer isn't
programmed with a fixed value but is adapted precisely to track
differences between system time and audio time. PulseAudio relies on
snd_pcm_avail() to query the number of samples played, and correlates
timer values with samples. There's a longer blurb on this at [1]. The
use of timers has pretty much broken most of the drivers as the values
returned by snd_pcm_avail weren't precise enough. However it's been a
year and a half now and things are ok on most hardware. The main
reason why this is interesting is that you can vary the latency on the
fly simply by reprogramming the timer. Having to define a fixed period
size for all use cases is a real problem for us. You end-up with a
compromise between latency and power that makes things suboptimal
across the board.
Cheers
-Pierre

[1] http://0pointer.de/blog/projects/pulse-glitch-free.html

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/4/30 pl bossart <bossart.nospam@gmail.com>

> >
> > Do your test need the patch which you have posted to pulseaudio developer
> > mailing list
> >
> > http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/6671
> >
> > it seem that fix should be in driver side if it is hardware specific
>
> No it's totally unrelated.
>

>> On my HDAudio test system, 128 bytes was already more than enough to
prevent audible noises.

>> Rewinding the ring buffer completely causes audible issues with DMAs.
Previous solution didn't work with tsched=0, and used tsched_watermark
for guardband, which isn't linked to hardware and could become really high
if underflows occurred.

But the rewind issue is not limiting to (PCIE) HDaudio since all PCI sound
cards would have the same problem ?

i.e. snd_pcm_rewindable() return wrong value

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/5/1 pl bossart <bossart.nospam@gmail.com>

> Hi Liam,
>
> > How do you handle any clock drift here between the HDA hardware
> > interface clock and the PA timer ?
>
> Good question. This is already handled by PulseAudio. The timer isn't
> programmed with a fixed value but is adapted precisely to track
> differences between system time and audio time.


seem underrun occur during this adjustment but PA just print "cutting
sleeping time" and did not provide any information about the sleeping time
for debugging

PulseAudio relies on
> snd_pcm_avail() to query the number of samples played, and correlates
> timer values with samples.


This mean that cs46xx and those drivers which cannot provide accurate number
of sample played will not work with PA ?


-Pierre
>

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/4/30 pl bossart <bossart.nospam@gmail.com>

> Hi Clemens,
>
> >> When PulseAudio is used and all PCM is routed through PulseAudio
> >> (Fedora, Meego, etc), the notion of ALSA periods isn't very useful.
> >> So why not disable them entirely to reduce the number of wakeups? ...
> >> There are probably some cases where you don't want this type of
> >> behavior (broken hardware, legacy code with multiple-buffering,
> >> disabled timer in PulseAudio),
> >
> > It's interesting that all ALSA applications except PA are "legacy".  :)
>
> Ha. Nice slip. I didn't mean legacy was bad... It's perfectly ok to
> use multiple buffers and interrupts in a variety of apps.
>
> >> so I think it would make sense to request the disabling of interrupts
> >> when hw_params are set, since this is also the time when period sizes
> >> are set.
> >
> > Yes.  For compatibility with the existing ALSA API, this needs to be
> > a flag that is not enabled by default.
>
> Agreed. This shouldn't even be mandatory since this option might not
> be possible in all platforms.
>
> >> I am aware that some changes would be needed in pcm_lib.c, where all
> >> the error checks are done.
> >
> > Plus all the API changes in the ALSA kernel framework, the ALSA kernel/
> > userspace interface, and the alsa-lib interface.
>
> I am not following this point. If you add a simple flag to an existing
> interface, why would we need to change the kernel/userspace inferface?
> This change should be possible in a backwards compatible manner as an
> additional option provided to the application developer.
> Cheers
> -Pierre
>

Do your proposal allow one application use "leagcy" method and the other
application use PA since my HDA codec seem allow multi streamming capture ?


card 1: Intel [HDA Intel], device 0: AD198x Analog [AD198x Analog]
  Subdevices: 3/3
  Subdevice #0: subdevice #0
  Subdevice #1: subdevice #1
  Subdevice #2: subdevice #2

Re: [alsa-devel] [RFC] disabling ALSA period interrupts

[Lennart Poettering]

On Thu, 29.04.10 17:38, pl bossart (bossart.nospam@gmail.com) wrote:

> Howdy,
> When PulseAudio is used and all PCM is routed through PulseAudio
> (Fedora, Meego, etc), the notion of ALSA periods isn't very useful.
> PulseAudio uses a timer to refill buffers and the period interrupts
> are not used at all.

This patch looks very interesting and desirable. This is something have
long been waiting for.

I wonder how this actually relates to
snd_pcm_sw_params_set_period_event() though.

> So why not disable them entirely to reduce the number of wakeups? This
> is possible with a number of existing DMA controllers. The simple
> patch attached shows a proof-of-concept on HDAudio, it's been working
> for 5 hours on my Fedora12 laptop without any glitches and powertop
> does show _zero_ wakeups from the HDAudio controller (except on
> startup). I am told by my colleagues working in Windows environments
> that this is what's done on Vista/Windows7 btw. This isn't to say
> Windows is great but that artificial generation of not-needed
> interrupts is avoidable.
> 
> There are probably some cases where you don't want this type of
> behavior (broken hardware, legacy code with multiple-buffering,
> disabled timer in PulseAudio), so I think it would make sense to
> request the disabling of interrupts when hw_params are set, since this
> is also the time when period sizes are set. I am aware that some
> changes would be needed in pcm_lib.c, where all the error checks are
> done. Takashi, Jaroslav, Lennart, what do you think?

I am sold!

Lennart

-- 
Lennart Poettering                        Red Hat, Inc.
lennart [at] poettering [dot] net
http://0pointer.net/lennart/           GnuPG 0x1A015CC4

Re: [RFC] disabling ALSA period interrupts

[pl bossart]

> This patch looks very interesting and desirable. This is something have
> long been waiting for.
>
> I wonder how this actually relates to
> snd_pcm_sw_params_set_period_event() though.

snd_pcm_sw_params_set_period_event() defines whether or not you will
have poll wakeups when a period elapses. But if you decide not to have
poll wakeups, you still end-up with interrupts that were generated for
no good reason. You might think that this could be extended to disable
interrupts as well, but this would be hard since this can only be done
when the DMA linked list is created, ie when the hw_params are set.
Plus the sw_params may be changed on-the-fly, which isn't the case for
hw_params/interrupts. We would really need a flag used with
snd_pcm_open or when hw_params are set.

>>  Takashi, Jaroslav, Lennart, what do you think?
> I am sold!
Good to hear.
Cheers
-Pierre

Re: [RFC] disabling ALSA period interrupts

[pl bossart]

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

Here's a proposal for an alsa-lib modification to allow applications
to disable interrupts (if the hardware can do so).
I used the flag field in hw_params, this looked like a good candidate
to convey this information needed by the driver. I don't really like
adding two new routines, but I don't think there's any other way.
Still 2 points to be worked out:
- I couldn't figure out how to make these two new symbols known to
PulseAudio. Somehow there's a missing symbol that prevents PA modules
from loading, not sure where it needs to be declare
- I thought I could make use of the same flag on the driver side, but
the code in pcm_native.c makes a peculiar use of hw_params->flag. It
looks like this field is used as an internal variable.

for (k = 0; k < constrs->rules_num; k++) {
			struct snd_pcm_hw_rule *r = &constrs->rules[k];
			unsigned int d;
			int doit = 0;
			if (r->cond && !(r->cond & params->flags))
				continue;
There's too much history for me to figure out what this is supposed to
mean. Am I following a false lead and should I use the more complex
mask array instead?
Thanks for your feedback.
- Pierre

[-- Attachment #2: alsa-lib-disable-interrupts.patch --]
[-- Type: application/octet-stream, Size: 4297 bytes --]

diff --git a/configure.in b/configure.in
index abc4687..e930621 100644
--- a/configure.in
+++ b/configure.in
@@ -12,7 +12,7 @@ dnl add API = c+1:0:a+1
 dnl remove API = c+1:0:0
 dnl *************************************************
 AC_CANONICAL_HOST
-AM_INIT_AUTOMAKE(alsa-lib, 1.0.23)
+AM_INIT_AUTOMAKE(alsa-lib, 1.0.24rc1)
 eval LIBTOOL_VERSION_INFO="2:0:0"
 dnl *************************************************
 AM_CONDITIONAL(INSTALL_M4, test -n "${ACLOCAL}")
diff --git a/include/pcm.h b/include/pcm.h
index f3618c3..5157877 100644
--- a/include/pcm.h
+++ b/include/pcm.h
@@ -626,7 +626,8 @@ int snd_pcm_hw_params_set_rate_resample(snd_pcm_t *pcm, snd_pcm_hw_params_t *par
 int snd_pcm_hw_params_get_rate_resample(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val);
 int snd_pcm_hw_params_set_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val);
 int snd_pcm_hw_params_get_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val);
-
+int snd_pcm_hw_params_set_disable_interrupts(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val);
+int snd_pcm_hw_params_get_disable_interrupts(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val);
 int snd_pcm_hw_params_get_period_time(const snd_pcm_hw_params_t *params, unsigned int *val, int *dir);
 int snd_pcm_hw_params_get_period_time_min(const snd_pcm_hw_params_t *params, unsigned int *val, int *dir);
 int snd_pcm_hw_params_get_period_time_max(const snd_pcm_hw_params_t *params, unsigned int *val, int *dir);
diff --git a/include/sound/asound.h b/include/sound/asound.h
index fa88938..35d3e75 100644
--- a/include/sound/asound.h
+++ b/include/sound/asound.h
@@ -346,6 +346,7 @@ enum sndrv_pcm_hw_param {
 
 #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_DISABLE_INTERRUPTS	(1<<2)	/* disable period interrupts */
 
 struct sndrv_interval {
 	unsigned int min, max;
diff --git a/src/Versions.in b/src/Versions.in
index 8d2dd11..faa8055 100644
--- a/src/Versions.in
+++ b/src/Versions.in
@@ -129,3 +129,8 @@ ALSA_0.9.7 {
     @SYMBOL_PREFIX@alsa_lisp_*;
 } ALSA_0.9.5;
 
+ALSA_1.0.24rc1 {
+  global:
+    @SYMBOL_PREFIX@snd_pcm_hw_params_set_disable_interrupts;
+    @SYMBOL_PREFIX@snd_pcm_hw_params_get_disable_interrupts;
+} ALSA_0.9.7;
diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index f910189..e7c254c 100644
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -4204,6 +4204,39 @@ int snd_pcm_hw_params_get_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *par
 }
 
 /**
+ * \brief Restrict a configuration space to disable period interrupts 
+ * \param pcm PCM handle
+ * \param params Configuration space
+ * \param val 1 = disable interrupts, 0 = enable (default) interrupts
+ * \return 0 otherwise a negative error code
+ */
+int snd_pcm_hw_params_set_disable_interrupts(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val)
+{
+	assert(pcm && params);
+
+	if (val)
+        params->flags |= SND_PCM_HW_PARAMS_DISABLE_INTERRUPTS;
+	else
+        params->flags &= ~SND_PCM_HW_PARAMS_DISABLE_INTERRUPTS;
+	return snd_pcm_hw_refine(pcm, params);
+}
+
+/**
+ * \brief Extract interrupt disable info from a configuration space
+ * \param pcm PCM handle
+ * \param params Configuration space
+ * \param val 1 = disable, 0 = enable interrupts
+ * \return 0
+ */
+int snd_pcm_hw_params_get_interrupt_disable(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val)
+{
+	assert(pcm && params && val);
+	*val = params->flags & SND_PCM_HW_PARAMS_DISABLE_INTERRUPTS ? 1 : 0;
+	return 0;
+}
+
+
+/**
  * \brief Extract period time from a configuration space
  * \param params Configuration space
  * \param val Returned approximate period duration in us
diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h
index 9aa81e1..31b5b69 100644
--- a/src/pcm/pcm_local.h
+++ b/src/pcm/pcm_local.h
@@ -94,6 +94,7 @@ typedef enum sndrv_pcm_hw_param snd_pcm_hw_param_t;
 
 #define SND_PCM_HW_PARAMS_NORESAMPLE SNDRV_PCM_HW_PARAMS_NORESAMPLE
 #define SND_PCM_HW_PARAMS_EXPORT_BUFFER SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER
+#define SND_PCM_HW_PARAMS_DISABLE_INTERRUPTS SNDRV_PCM_HW_PARAMS_DISABLE_INTERRUPTS
 
 #define SND_PCM_INFO_MONOTONIC	0x80000000
 

[-- Attachment #3: pulseaudio-disable-interrupts.patch --]
[-- Type: application/octet-stream, Size: 789 bytes --]

diff --git a/src/modules/alsa/alsa-util.c b/src/modules/alsa/alsa-util.c
index 1cbb3f3..3c7037a 100644
--- a/src/modules/alsa/alsa-util.c
+++ b/src/modules/alsa/alsa-util.c
@@ -250,6 +250,16 @@ int pa_alsa_set_hw_params(
 
     if (!pa_alsa_pcm_is_hw(pcm_handle))
         _use_tsched = FALSE;
+    else {
+        if(_use_tsched) {
+
+            /* try to disable period interrupts if hardware can do so */
+            if ((ret = snd_pcm_hw_params_set_disable_interrupts(pcm_handle, hwparams, 1)) < 0) {
+                pa_log_debug("snd_pcm_hw_params_set_disable_interrupts() failed: %s", pa_alsa_strerror(ret));
+                /* don't bail, keep going */
+            }
+        }
+    }
 
     if ((ret = set_format(pcm_handle, hwparams, &_ss.format)) < 0)
         goto finish;

[-- Attachment #4: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [RFC] disabling ALSA period interrupts

[James Courtier-Dutton]

On 8 May 2010 00:25, Lennart Poettering <mznyfn@0pointer.de> wrote:
> On Thu, 29.04.10 17:38, pl bossart (bossart.nospam@gmail.com) wrote:
>
>> Howdy,
>> When PulseAudio is used and all PCM is routed through PulseAudio
>> (Fedora, Meego, etc), the notion of ALSA periods isn't very useful.
>> PulseAudio uses a timer to refill buffers and the period interrupts
>> are not used at all.
>
> This patch looks very interesting and desirable. This is something have
> long been waiting for.
>
> I wonder how this actually relates to
> snd_pcm_sw_params_set_period_event() though.
>
>> So why not disable them entirely to reduce the number of wakeups? This
>> is possible with a number of existing DMA controllers. The simple
>> patch attached shows a proof-of-concept on HDAudio, it's been working
>> for 5 hours on my Fedora12 laptop without any glitches and powertop
>> does show _zero_ wakeups from the HDAudio controller (except on
>> startup). I am told by my colleagues working in Windows environments
>> that this is what's done on Vista/Windows7 btw. This isn't to say
>> Windows is great but that artificial generation of not-needed
>> interrupts is avoidable.
>>
>> There are probably some cases where you don't want this type of
>> behavior (broken hardware, legacy code with multiple-buffering,
>> disabled timer in PulseAudio), so I think it would make sense to
>> request the disabling of interrupts when hw_params are set, since this
>> is also the time when period sizes are set. I am aware that some
>> changes would be needed in pcm_lib.c, where all the error checks are
>> done. Takashi, Jaroslav, Lennart, what do you think?
>
> I am sold!
>
> Lennart
>

Some care would need to be taken with regards to detecting xruns.
I think the alsa code currently uses the interrupt callback to detect this.
I have seen a Windows 7 machine happily loop the audio buffer
uncontrollably, so I assume it has problems detecting xruns as well.
Some sound card hardware only updates the hw pointer at around dma
interrupt event time, so again using the interrupt is used to improve
the accuracy of the hw pointer with interpolation used between
interrupts.
Some sound card hardware has very small hardware buffers, so PA will
have to be waking up as often as the dma interrupts in order to keep
the audio hardware buffers full enough.
In how many cased would PA have to wake up less often than the DMA interrupt?

Re: [RFC] disabling ALSA period interrupts

[pl bossart]

> Some care would need to be taken with regards to detecting xruns.
> I think the alsa code currently uses the interrupt callback to detect this.
> I have seen a Windows 7 machine happily loop the audio buffer
> uncontrollably, so I assume it has problems detecting xruns as well.

When the PulseAudio timer fires, the use of snd_pcm_avail() will force
a call to .pointer and will detect underflows. PulseAudio modilfies
its watermark when underflows occur so that more time is allocated to
refilling the ring buffer.
There are probably some cases I didn't plan for, but on paper I don't
really see a show-stopper here.

> Some sound card hardware only updates the hw pointer at around dma
> interrupt event time, so again using the interrupt is used to improve
> the accuracy of the hw pointer with interpolation used between
> interrupts.

If the hardware doesn't provide an accurate hw pointer, then the
timer-based scheduling should not be used I agree.

> Some sound card hardware has very small hardware buffers, so PA will
> have to be waking up as often as the dma interrupts in order to keep
> the audio hardware buffers full enough.
> In how many cased would PA have to wake up less often than the DMA interrupt?

This patch is mainly for music playback where you have more than 2s
buffered in the ring buffer. PulseAudio will wake-up after 1.9s or so,
as the ring buffer becomes empty, and when it does the wake-up may be
grouped with other system events with the timer slack. Having one or
more interrupts in the middle or the ring buffer will reduce the
efficiency of sleep modes that are being introduced with Moorestown
and future Atom-based SOCs.
So again this patch isn't for everyone. All standard disclaimers
apply, if you have a pre-existing conditon tell your doctor etc. If
there's a 4K ring buffer, this is not useful. If your hardware is
broken, stay the course with the current solution. If PulseAudio is
disabled in your distro, this is of no interest to you. In all these
cases, this patch doesn't change anything, the behavior is the same.
But if you want to optimize for power and latency isn't a concern,
then these interrupts can be disabled and need to.

Re: [RFC] disabling ALSA period interrupts

[Jaroslav Kysela]

On Tue, 11 May 2010, pl bossart wrote:

> Here's a proposal for an alsa-lib modification to allow applications
> to disable interrupts (if the hardware can do so).

Please, use tabs for block indention for alsa-lib.

> I used the flag field in hw_params, this looked like a good candidate
> to convey this information needed by the driver. I don't really like
> adding two new routines, but I don't think there's any other way.
> Still 2 points to be worked out:
> - I couldn't figure out how to make these two new symbols known to
> PulseAudio. Somehow there's a missing symbol that prevents PA modules
> from loading, not sure where it needs to be declare

Are you sure that you're using new libraries? 'ldd' and 'nm' tools will 
help you to determine what's wrong with symbols.

> - I thought I could make use of the same flag on the driver side, but
> the code in pcm_native.c makes a peculiar use of hw_params->flag. It
> looks like this field is used as an internal variable.
>
> for (k = 0; k < constrs->rules_num; k++) {
> 			struct snd_pcm_hw_rule *r = &constrs->rules[k];
> 			unsigned int d;
> 			int doit = 0;
> 			if (r->cond && !(r->cond & params->flags))
> 				continue;

Ignore this. I don't know about any place where r->cond is used (!= 0) in 
the current ALSA driver, so eating one more bit from params->flags is OK.
Just put the kernel asound.h in sync with library asound.h.

 						Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [RFC] disabling ALSA period interrupts

[Jassi Brar]

On Fri, Apr 30, 2010 at 7:38 AM, pl bossart <bossart.nospam@gmail.com> wrote:
> Howdy,
> When PulseAudio is used and all PCM is routed through PulseAudio
> (Fedora, Meego, etc), the notion of ALSA periods isn't very useful.
> PulseAudio uses a timer to refill buffers and the period interrupts
> are not used at all.

It seems the requirement is just to have as least wakeups as possible
in order to
maximize power savings.
If so, then how about setting the period size slightly smaller than
the ring-buffer: the
difference being just enough to refill the ring buffer. Of course, you
would have to
enforce full-buffer-size as the start threshold.
We do just that to implement low-power-audio-mode for latest Samsung
SoC's I2S blocks.

Re: [alsa-devel] [RFC] disabling ALSA period interrupts

[pl bossart]

> It seems the requirement is just to have as least wakeups as possible
> in order to
> maximize power savings.
> If so, then how about setting the period size slightly smaller than
> the ring-buffer: the
> difference being just enough to refill the ring buffer. Of course, you
> would have to
> enforce full-buffer-size as the start threshold.
> We do just that to implement low-power-audio-mode for latest Samsung
> SoC's I2S blocks.

Thanks for the heads-up Jassi, this is interesting info that does show
the need to reduce the number of wakeups in embedded low-power
solutions...
This might be almost equivalent to the timer approach in terms of # of
wakeups, however the timer can be reprogrammed on-the-fly whereas
periods can only be changed by closing and reopening the device. You
can also adjust the timer shall underflows occur. And the timer slack
lets the kernel group events. Not to mention that you will need
specific apps written to make use of this mode. The only drawback of
the timer approach is that you need to keep track of drifts between
system and audio clock and that you need the hardware and driver to
report the hw_ptr with precision.

Re: [alsa-devel] [RFC] disabling ALSA period interrupts

[Jassi Brar]

On Wed, May 12, 2010 at 10:50 PM, pl bossart <bossart.nospam@gmail.com> wrote:
>> It seems the requirement is just to have as least wakeups as possible
>> in order to
>> maximize power savings.
>> If so, then how about setting the period size slightly smaller than
>> the ring-buffer: the
>> difference being just enough to refill the ring buffer. Of course, you
>> would have to
>> enforce full-buffer-size as the start threshold.
>> We do just that to implement low-power-audio-mode for latest Samsung
>> SoC's I2S blocks.
>
> Thanks for the heads-up Jassi, this is interesting info that does show
> the need to reduce the number of wakeups in embedded low-power
> solutions...
> This might be almost equivalent to the timer approach in terms of # of
> wakeups, however the timer can be reprogrammed on-the-fly whereas
> periods can only be changed by closing and reopening the device. You
> can also adjust the timer shall underflows occur. And the timer slack
> lets the kernel group events. Not to mention that you will need
> specific apps written to make use of this mode. The only drawback of
> the timer approach is that you need to keep track of drifts between
> system and audio clock and that you need the hardware and driver to
> report the hw_ptr with precision.
.... another negative is having to hack the alsa stack.
I couldn't use the timer thing because the system has to be woken up
from suspend(and I2S intr is one of the wakeup sources) and maybe
for moorstown/atom also you'd go to sleep/suspend? make sure if the
timer based approach could work there.
Also, at least for embedded systems, such audio mode switching is done
at a level coarse enough(the user sets the 'Hold' switch) that overall power
savings(unloading unnecessary driver modules upon first LPAM suspend
and reloading only after the last) is much more than a minor click in playback
due to close->open of the audio stream.

Re: [RFC] disabling ALSA period interrupts

[Mark Brown]

On Wed, May 12, 2010 at 08:50:42AM -0500, pl bossart wrote:

> This might be almost equivalent to the timer approach in terms of # of
> wakeups, however the timer can be reprogrammed on-the-fly whereas
> periods can only be changed by closing and reopening the device. You
> can also adjust the timer shall underflows occur. And the timer slack
> lets the kernel group events. Not to mention that you will need

The timer is also useful for allowing PulseAudio to dynamically adjust
the period size as the runtime situation changes - if you're running
high latency streams and have a low latency application start up then
PulseAudio can just change the timer to get more frequent events.

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/5/12 pl bossart <bossart.nospam@gmail.com>

> > It seems the requirement is just to have as least wakeups as possible
> > in order to
> > maximize power savings.
> > If so, then how about setting the period size slightly smaller than
> > the ring-buffer: the
> > difference being just enough to refill the ring buffer. Of course, you
> > would have to
> > enforce full-buffer-size as the start threshold.
> > We do just that to implement low-power-audio-mode for latest Samsung
> > SoC's I2S blocks.
>
> Thanks for the heads-up Jassi, this is interesting info that does show
> the need to reduce the number of wakeups in embedded low-power
> solutions...
> This might be almost equivalent to the timer approach in terms of # of
> wakeups, however the timer can be reprogrammed on-the-fly whereas
> periods can only be changed by closing and reopening the device. You
> can also adjust the timer shall underflows occur. And the timer slack
> lets the kernel group events. Not to mention that you will need
> specific apps written to make use of this mode. The only drawback of
> the timer approach is that you need to keep track of drifts between
> system and audio clock and that you need the hardware and driver to
> report the hw_ptr with precision.
>

The problem is PA require hw_ptr with precision but PA server did not
provide same precision to PA client through alsa-pulseaudio plugin , this
inaccuracy lead to underrun easily

Re: [alsa-devel] [RFC] disabling ALSA period interrupts

[pl bossart]

> Please, use tabs for block indention for alsa-lib.

Right, I used the same .emacs style for pa and alsa-lib...Fixed now

> Are you sure that you're using new libraries? 'ldd' and 'nm' tools will help
> you to determine what's wrong with symbols.

Argh.. For some reason Fedora has a preinstalled libasound in
/lib...So when I installed the new alsa-lib in /usr, PA knew about the
include file but used the old lib. Thanks for the tip.


> Ignore this. I don't know about any place where r->cond is used (!= 0) in
> the current ALSA driver, so eating one more bit from params->flags is OK.
> Just put the kernel asound.h in sync with library asound.h.

ok. It works just fine now. Can I send a patch against alsa-kmirror or
do you prefer against alsa-kernel? I use the former to recompile only
the audio modules.

Re: [RFC] disabling ALSA period interrupts

[Jaroslav Kysela]

On Wed, 12 May 2010, pl bossart wrote:

> ok. It works just fine now. Can I send a patch against alsa-kmirror or
> do you prefer against alsa-kernel? I use the former to recompile only
> the audio modules.

It does not matter. I accept both forms (the diffence between trees is 
minimal). Also, you can use alsa-driver with alsa-kernel trees too now 
(the latest gitcompile script in alsa-driver handles this option as well).

 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/5/12 Mark Brown <broonie@opensource.wolfsonmicro.com>

> On Wed, May 12, 2010 at 08:50:42AM -0500, pl bossart wrote:
>
> > This might be almost equivalent to the timer approach in terms of # of
> > wakeups, however the timer can be reprogrammed on-the-fly whereas
> > periods can only be changed by closing and reopening the device. You
> > can also adjust the timer shall underflows occur. And the timer slack
> > lets the kernel group events. Not to mention that you will need
>
> The timer is also useful for allowing PulseAudio to dynamically adjust
> the period size as the runtime situation changes - if you're running
> high latency streams and have a low latency application start up then
> PulseAudio can just change the timer to get more frequent events.
>

how low latency can PA provide ?

any tool to measure the latency ?

seem alsa-lib/test/latency did not work on pulse device

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/5/12 pl bossart <bossart.nospam@gmail.com>

> > Some care would need to be taken with regards to detecting xruns.
> > I think the alsa code currently uses the interrupt callback to detect
> this.
> > I have seen a Windows 7 machine happily loop the audio buffer
> > uncontrollably, so I assume it has problems detecting xruns as well.
>
> When the PulseAudio timer fires, the use of snd_pcm_avail() will force
> a call to .pointer and will detect underflows. PulseAudio modilfies
> its watermark when underflows occur so that more time is allocated to
> refilling the ring buffer.
> There are probably some cases I didn't plan for, but on paper I don't
> really see a show-stopper here.
>
> > Some sound card hardware only updates the hw pointer at around dma
> > interrupt event time, so again using the interrupt is used to improve
> > the accuracy of the hw pointer with interpolation used between
> > interrupts.
>
> If the hardware doesn't provide an accurate hw pointer, then the
> timer-based scheduling should not be used I agree.
>

How accurate do timer-based scheduling need ?

e.g. accuracy up to +/- 5% of period size

or the watermark is too low for those sound card

can PA provide option to change the initial watermark ?


>
> > Some sound card hardware has very small hardware buffers, so PA will
> > have to be waking up as often as the dma interrupts in order to keep
> > the audio hardware buffers full enough.
> > In how many cased would PA have to wake up less often than the DMA
> interrupt?
>
> This patch is mainly for music playback where you have more than 2s
> buffered in the ring buffer.


Do you mean that  those application require low latency will fail with
underrun ?

seem the usuage of PA is quite limited

Can PA provide an option to increase the number of wake up for those desktop
user which power consumption is not important ? ( default- fragments )

it seem (disable timer option ) of new PA version is not as same as the
Traditional mode of old PA version anymore


> PulseAudio will wake-up after 1.9s or so,
> as the ring buffer becomes empty, and when it does the wake-up may be
> grouped with other system events with the timer slack. Having one or
> more interrupts in the middle or the ring buffer will reduce the
> efficiency of sleep modes that are being introduced with Moorestown
> and future Atom-based SOCs.
> So again this patch isn't for everyone. All standard disclaimers
> apply, if you have a pre-existing conditon tell your doctor etc. If
> there's a 4K ring buffer, this is not useful.


au88x0 has 4 set of hardware registers each with 4 K buffers ( i.e. total 16
K bytes)

PA work quite nicely with 2 periods per buffer on PA 1.0.13 but some user
reported cracking noise when new PA 1.0.14 or later version use 8 periods
per buffer


> If your hardware is
> broken, stay the course with the current solution. If PulseAudio is
> disabled in your distro, this is of no interest to you. In all these
> cases, this patch doesn't change anything, the behavior is the same.
> But if you want to optimize for power and latency isn't a concern,
> then these interrupts can be disabled and need to.
>
>

Re: [RFC] disabling ALSA period interrupts

[pl bossart]

>> ok. It works just fine now. Can I send a patch against alsa-kmirror or
>> do you prefer against alsa-kernel? I use the former to recompile only
>> the audio modules.
>
> It does not matter. I accept both forms (the diffence between trees is
> minimal). Also, you can use alsa-driver with alsa-kernel trees too now (the
> latest gitcompile script in alsa-driver handles this option as well).

Good to know. I was using alsa-kmirror and alsa-driver with the
insert/remove scripts re-added. But this new option looks much better.
Will give it a try tomorrow.

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/5/12 pl bossart <bossart.nospam@gmail.com>

> > It seems the requirement is just to have as least wakeups as possible
> > in order to
> > maximize power savings.
> > If so, then how about setting the period size slightly smaller than
> > the ring-buffer: the
> > difference being just enough to refill the ring buffer. Of course, you
> > would have to
> > enforce full-buffer-size as the start threshold.
> > We do just that to implement low-power-audio-mode for latest Samsung
> > SoC's I2S blocks.
>
> Thanks for the heads-up Jassi, this is interesting info that does show
> the need to reduce the number of wakeups in embedded low-power
> solutions...
> This might be almost equivalent to the timer approach in terms of # of
> wakeups, however the timer can be reprogrammed on-the-fly whereas
> periods can only be changed by closing and reopening the device. You
> can also adjust the timer shall underflows occur. And the timer slack
> lets the kernel group events. Not to mention that you will need
> specific apps written to make use of this mode. The only drawback of
> the timer approach is that you need to keep track of drifts between
> system and audio clock and that you need the hardware and driver to
> report the hw_ptr with precision.
>

it depend on the ratio of watermark and the buffer time

PA use 20 ms watermark and 2 s buffer time which require the driver report
hw_ptr with precision . if the watermark is 50% of the buffer time ,this
will fall back to the traditional model two periods per buffer

so it depend on whether PA allow user to adjust the watermark for their
desktop or laptop

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/5/12 pl bossart <bossart.nospam@gmail.com>

> > Some care would need to be taken with regards to detecting xruns.
> > I think the alsa code currently uses the interrupt callback to detect
> this.
> > I have seen a Windows 7 machine happily loop the audio buffer
> > uncontrollably, so I assume it has problems detecting xruns as well.
>
> When the PulseAudio timer fires, the use of snd_pcm_avail() will force
> a call to .pointer and will detect underflows. PulseAudio modilfies
> its watermark when underflows occur so that more time is allocated to
> refilling the ring buffer.
> There are probably some cases I didn't plan for, but on paper I don't
> really see a show-stopper here.
>

Refer to your patch "add rewind-safeguard parameter"

http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=4df443bbe682055a41e7c2248877dcc7682a69b8

you mention glitch occur because application cannot rewind appl pointer to
hardware ptr.

In another word, app ptr must be 128/256 bytes ahead of hardware ptr at any
time to prevent glitch occur. this mean that the glitch was due to DMA brust
size since "PulseAudio modilfies its watermark when underflows occur".

Re: [RFC] disabling ALSA period interrupts

[pl bossart]

> Refer to your patch "add rewind-safeguard parameter"
>
> http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=4df443bbe682055a41e7c2248877dcc7682a69b8
>
> you mention glitch occur because application cannot rewind appl pointer to
> hardware ptr.
>
> In another word, app ptr must be 128/256 bytes ahead of hardware ptr at any
> time to prevent glitch occur. this mean that the glitch was due to DMA brust
> size since "PulseAudio modilfies its watermark when underflows occur".

Man, you are seriously confused. You ought to do some reading, look at
the code, use your brain and figure things out instead of spamming
this thread.

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/5/14 pl bossart <bossart.nospam@gmail.com>

> > Refer to your patch "add rewind-safeguard parameter"
> >
> >
> http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=4df443bbe682055a41e7c2248877dcc7682a69b8
> >
> > you mention glitch occur because application cannot rewind appl pointer
> to
> > hardware ptr.
> >
> > In another word, app ptr must be 128/256 bytes ahead of hardware ptr at
> any
> > time to prevent glitch occur. this mean that the glitch was due to DMA
> brust
> > size since "PulseAudio modilfies its watermark when underflows occur".
>
> Man, you are seriously confused. You ought to do some reading, look at
> the code, use your brain and figure things out instead of spamming
> this thread.
>

underrun is application pointer behind hardware pointer

"Rewinding the ring buffer completely causes audible issues with DMAs."

rewinding ring buffer compleletly mean hardware pointer = application
pointer

so gltich also occur when application pointer is behind hardware pointer

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/5/14 pl bossart <bossart.nospam@gmail.com>

> > Refer to your patch "add rewind-safeguard parameter"
> >
> >
> http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=4df443bbe682055a41e7c2248877dcc7682a69b8
> >
> > you mention glitch occur because application cannot rewind appl pointer
> to
> > hardware ptr.
> >
> > In another word, app ptr must be 128/256 bytes ahead of hardware ptr at
> any
> > time to prevent glitch occur. this mean that the glitch was due to DMA
> brust
> > size since "PulseAudio modilfies its watermark when underflows occur".
>
> Man, you are seriously confused. You ought to do some reading, look at
> the code, use your brain and figure things out instead of spamming
> this thread.
>


underrun is application pointer behind hardware pointer

"Rewinding the ring buffer completely causes audible issues with DMAs."

rewinding ring buffer compleletly mean hardware pointer = application
pointer

"Glitch free" can only be achieved when PA adjust the watermark before any
unnder run occur not "until underrrun occur" since it is too late

Re: [RFC] disabling ALSA period interrupts

[Raymond Yau]

2010/5/14 pl bossart <bossart.nospam@gmail.com>

> > Refer to your patch "add rewind-safeguard parameter"
> >
> >
> http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=4df443bbe682055a41e7c2248877dcc7682a69b8
> >
> > you mention glitch occur because application cannot rewind appl pointer
> to
> > hardware ptr.
> >
> > In another word, app ptr must be 128/256 bytes ahead of hardware ptr at
> any
> > time to prevent glitch occur. this mean that the glitch was due to DMA
> brust
> > size since "PulseAudio modilfies its watermark when underflows occur".
>
> Man, you are seriously confused. You ought to do some reading, look at
> the code, use your brain and figure things out instead of spamming
> this thread.
>

your patch are adjust the watermark before unnder occur  to prevent glitch ,
not as you said "PA adjust watermark until underrun occur"

" PulseAudio modilfies its watermark when underflows occur so that more time
is allocated to refilling the ring buffer."

This model cannot be called as "Glitch free model" ,

A "Glicth free model" have to prevent any underrun occur at any time

Re: [RFC] disabling ALSA period interrupts

[Jassi Brar]

On Wed, May 12, 2010 at 11:16 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, May 12, 2010 at 08:50:42AM -0500, pl bossart wrote:
>
>> This might be almost equivalent to the timer approach in terms of # of
>> wakeups, however the timer can be reprogrammed on-the-fly whereas
>> periods can only be changed by closing and reopening the device. You
>> can also adjust the timer shall underflows occur. And the timer slack
>> lets the kernel group events. Not to mention that you will need
>
> The timer is also useful for allowing PulseAudio to dynamically adjust
> the period size as the runtime situation changes - if you're running
> high latency streams and have a low latency application start up then
> PulseAudio can just change the timer to get more frequent events.

AFAIU, the only issue is lack of ability to fine-tune period size of
DMA runtime.
Otherwise, for the requirement, having period-size almost equal
to ring-buffer serves better than disabling interrupts and using timer
based updating, more so for VMs with inaccurate timer source.

Then I think, rather then providing a way to disable hw-intr, we'd
better provide
a way to modify runtime period-size at DMA Driver level, and
snd_pcm_period_elapsed seems already capable to handle that.
That way, disabled interrupts would just be a special case with
period-size := ULONG_MAX.

Re: [RFC] disabling ALSA period interrupts

[pl bossart]

> AFAIU, the only issue is lack of ability to fine-tune period size of
> DMA runtime.
> Otherwise, for the requirement, having period-size almost equal
> to ring-buffer serves better than disabling interrupts and using timer
> based updating, more so for VMs with inaccurate timer source.
>
> Then I think, rather then providing a way to disable hw-intr, we'd
> better provide
> a way to modify runtime period-size at DMA Driver level, and
> snd_pcm_period_elapsed seems already capable to handle that.
> That way, disabled interrupts would just be a special case with
> period-size := ULONG_MAX.

Is this a realistic option? With the majority of existing hardware
period interrupts are programmed with a flag set in a descriptor when
the DMA linked list is created. That includes HDAudio and numerous
others. I am skeptical that one could reliably modify these
descriptors at run-time, specifically in the case where the controller
caches the descriptors. Modifying the linked list would likewise
generate race conditions.
I still view timers are the lesser of two evils.

Re: [RFC] disabling ALSA period interrupts

[Jassi Brar]

On Fri, May 14, 2010 at 1:39 PM, pl bossart <bossart.nospam@gmail.com> wrote:
>> AFAIU, the only issue is lack of ability to fine-tune period size of
>> DMA runtime.
>> Otherwise, for the requirement, having period-size almost equal
>> to ring-buffer serves better than disabling interrupts and using timer
>> based updating, more so for VMs with inaccurate timer source.
>>
>> Then I think, rather then providing a way to disable hw-intr, we'd
>> better provide
>> a way to modify runtime period-size at DMA Driver level, and
>> snd_pcm_period_elapsed seems already capable to handle that.
>> That way, disabled interrupts would just be a special case with
>> period-size := ULONG_MAX.
>
> Is this a realistic option?
We have to see as it is certainly a more desirable solution.

> With the majority of existing hardware
> period interrupts are programmed with a flag set in a descriptor when
> the DMA linked list is created. That includes HDAudio and numerous
> others. I am skeptical that one could reliably modify these
> descriptors at run-time, specifically in the case where the controller
> caches the descriptors.
As we all understand, the requirement is expected to be met only by
some h/w that has the capability.
Among those candidates, cards that don't support period resize may
respond only to "period-size := ULONG_MAX" (i.e interrupt disabling),
while more flexible cards can exploit the period-resize feature.

Also, let us not forget that almost every embedded device
use general purpose DMA that can be re-programmed for different
period-size without much trouble.

Re: [RFC] disabling ALSA period interrupts

[Takashi Iwai]

At Fri, 30 Apr 2010 08:44:24 -0500,
pl bossart wrote:
> 
> >> I am aware that some changes would be needed in pcm_lib.c, where all
> >> the error checks are done.
> >
> > Plus all the API changes in the ALSA kernel framework, the ALSA kernel/
> > userspace interface, and the alsa-lib interface.
> 
> I am not following this point. If you add a simple flag to an existing
> interface, why would we need to change the kernel/userspace inferface?
> This change should be possible in a backwards compatible manner as an
> additional option provided to the application developer.

The biggest problem I can foresee is the handling of PCM position.
In the current implementation, the PCM position continues to go over
the buffer size until the certain boundary that is close to long int
max.  Without interrupts (i.e. snd_pcm_period_elapsed()), this
position update won't work reliably.  If we reduce boundary size as
buffer size, certainly some code in alsa-lib (or kernel PCM) would be
confused.

Thus, before disabling the interrupts completely, we need to revisit
the PCM position handling all over places.  But in general, I think
it's fine to implement such a mode -- it's more intuitive than the
current free-wheel mode we have now.


BTW, I'm still wondering whether disabling the IRQ would really give
so much gain compared with periods=1 or 2 case.  I thought all this
was about the power-saving, no?  Did someone measure a significant
difference between periods=0 and periods=2 in this regard?


thanks,

Takashi

Re: [RFC] disabling ALSA period interrupts

[pl bossart]

> The biggest problem I can foresee is the handling of PCM position.
> In the current implementation, the PCM position continues to go over
> the buffer size until the certain boundary that is close to long int
> max.  Without interrupts (i.e. snd_pcm_period_elapsed()), this
> position update won't work reliably.  If we reduce boundary size as
> buffer size, certainly some code in alsa-lib (or kernel PCM) would be
> confused.

Good catch. I thought the processing was the same whether you called
snd_pcm_period_elapsed or snd_pcm_avail when the timer fires, since in
both cases you call the .pointer routine, but there's some code in
snd_pcm_update_hw_ptr0 that is only executed in an interrupt context.
Likewise there are a bunch of fixes for hw_ptr position that make use
of this boundary field (which I have to admit I don't understand too
much).

> BTW, I'm still wondering whether disabling the IRQ would really give
> so much gain compared with periods=1 or 2 case.  I thought all this
> was about the power-saving, no?  Did someone measure a significant
> difference between periods=0 and periods=2 in this regard?

I don't have data I can post on this mailing list without running into
trouble with legal. For low-power playback where we chase milliwatts,
waking up for no good reason isn't very compatible with the S0i2 modes
defined from Moorestown onwards. In S0i2 the platform is mostly off
which the exception of the audio rendering cluster that is
clocked/powered independently. The entry/exit latency is higher than
for regular C states and the transition costs make you want avoid
isolated interrupts. There's a slew of patches coming from Intel to
introduce timer slacks everywhere, my proposal follows the same idea
of avoiding wakeups and grouping events.
We also have a different use case where we want the ring buffer to be
refilled when a modem provides data. The modem events and ALSA periods
may not be aligned, the delta between events isn't predictable, and
that leads to doubling the number of wake-ups since we need to handle
modem events and ALSA events. Again this interferes with cpuidle
heuristics and prevents the entry in sleep states.

Re: [RFC] disabling ALSA period interrupts

[pl bossart]

[-- Attachment #1: Type: text/plain, Size: 363 bytes --]

Here is my latest set of patches before I forget about them.
Still some work to be done on the alsa-lib one, for some reason the
hw_param->flags field I used gets overwritten if I don't use the
hw_device. I suspect this is due to some black magic with the
pcm->hw_flags when slave devices are used.
And as Takashi mention there's even more work on sample counts.

[-- Attachment #2: 0001-add-support-for-disabling-of-period-interrupts.patch --]
[-- Type: application/octet-stream, Size: 4993 bytes --]

From 2475eaa833dc9a4961a3fcc8b6d6364188b0c4cb Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart <pierre-louis.bossart@intel.com>
Date: Wed, 12 May 2010 11:46:11 -0500
Subject: [PATCH] add support for disabling of period interrupts


Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@intel.com>
---
 configure.in           |    2 +-
 include/pcm.h          |    3 ++-
 include/sound/asound.h |    1 +
 src/Versions.in        |    5 +++++
 src/pcm/pcm.c          |   33 +++++++++++++++++++++++++++++++++
 src/pcm/pcm_local.h    |    1 +
 6 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/configure.in b/configure.in
index abc4687..e930621 100644
--- a/configure.in
+++ b/configure.in
@@ -12,7 +12,7 @@ dnl add API = c+1:0:a+1
 dnl remove API = c+1:0:0
 dnl *************************************************
 AC_CANONICAL_HOST
-AM_INIT_AUTOMAKE(alsa-lib, 1.0.23)
+AM_INIT_AUTOMAKE(alsa-lib, 1.0.24rc1)
 eval LIBTOOL_VERSION_INFO="2:0:0"
 dnl *************************************************
 AM_CONDITIONAL(INSTALL_M4, test -n "${ACLOCAL}")
diff --git a/include/pcm.h b/include/pcm.h
index f3618c3..5157877 100644
--- a/include/pcm.h
+++ b/include/pcm.h
@@ -626,7 +626,8 @@ int snd_pcm_hw_params_set_rate_resample(snd_pcm_t *pcm, snd_pcm_hw_params_t *par
 int snd_pcm_hw_params_get_rate_resample(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val);
 int snd_pcm_hw_params_set_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val);
 int snd_pcm_hw_params_get_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val);
-
+int snd_pcm_hw_params_set_disable_interrupts(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val);
+int snd_pcm_hw_params_get_disable_interrupts(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val);
 int snd_pcm_hw_params_get_period_time(const snd_pcm_hw_params_t *params, unsigned int *val, int *dir);
 int snd_pcm_hw_params_get_period_time_min(const snd_pcm_hw_params_t *params, unsigned int *val, int *dir);
 int snd_pcm_hw_params_get_period_time_max(const snd_pcm_hw_params_t *params, unsigned int *val, int *dir);
diff --git a/include/sound/asound.h b/include/sound/asound.h
index fa88938..0686da4 100644
--- a/include/sound/asound.h
+++ b/include/sound/asound.h
@@ -346,6 +346,7 @@ enum sndrv_pcm_hw_param {
 
 #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_DISABLE_PERIOD_INTERRUPTS	(1<<2)	/* disable period interrupts */
 
 struct sndrv_interval {
 	unsigned int min, max;
diff --git a/src/Versions.in b/src/Versions.in
index 8d2dd11..f907912 100644
--- a/src/Versions.in
+++ b/src/Versions.in
@@ -129,3 +129,8 @@ ALSA_0.9.7 {
     @SYMBOL_PREFIX@alsa_lisp_*;
 } ALSA_0.9.5;
 
+ALSA_1.0.24rc1 {
+  global:
+    @SYMBOL_PREFIX@snd_pcm_hw_params_set_disable_period_interrupts;
+    @SYMBOL_PREFIX@snd_pcm_hw_params_get_disable_period_interrupts;
+} ALSA_0.9.7;
diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index f910189..55a67cd 100644
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -4204,6 +4204,39 @@ int snd_pcm_hw_params_get_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *par
 }
 
 /**
+ * \brief Restrict a configuration space to disable period interrupts 
+ * \param pcm PCM handle
+ * \param params Configuration space
+ * \param val 1 = disable interrupts, 0 = enable (default) interrupts
+ * \return 0 otherwise a negative error code
+ */
+int snd_pcm_hw_params_set_disable_period_interrupts(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val)
+{
+	assert(pcm && params);
+
+        if (val)
+                params->flags |= SND_PCM_HW_PARAMS_DISABLE_PERIOD_INTERRUPTS;
+	else
+                params->flags &= ~SND_PCM_HW_PARAMS_DISABLE_PERIOD_INTERRUPTS;
+	return snd_pcm_hw_refine(pcm, params);
+}
+
+/**
+ * \brief Extract interrupt disable info from a configuration space
+ * \param pcm PCM handle
+ * \param params Configuration space
+ * \param val 1 = disable, 0 = enable interrupts
+ * \return 0
+ */
+int snd_pcm_hw_params_get_interrupt_disable(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val)
+{
+	assert(pcm && params && val);
+	*val = params->flags & SND_PCM_HW_PARAMS_DISABLE_PERIOD_INTERRUPTS ? 1 : 0;
+	return 0;
+}
+
+
+/**
  * \brief Extract period time from a configuration space
  * \param params Configuration space
  * \param val Returned approximate period duration in us
diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h
index 9aa81e1..6e9929f 100644
--- a/src/pcm/pcm_local.h
+++ b/src/pcm/pcm_local.h
@@ -94,6 +94,7 @@ typedef enum sndrv_pcm_hw_param snd_pcm_hw_param_t;
 
 #define SND_PCM_HW_PARAMS_NORESAMPLE SNDRV_PCM_HW_PARAMS_NORESAMPLE
 #define SND_PCM_HW_PARAMS_EXPORT_BUFFER SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER
+#define SND_PCM_HW_PARAMS_DISABLE_PERIOD_INTERRUPTS SNDRV_PCM_HW_PARAMS_DISABLE_PERIOD_INTERRUPTS
 
 #define SND_PCM_INFO_MONOTONIC	0x80000000
 
-- 
1.6.6.1


[-- Attachment #3: 0001-alsa-support-for-disabling-of-period-interrupts.patch --]
[-- Type: application/octet-stream, Size: 6729 bytes --]

From fdccbdfdafad6de80ae651758a3d01e8433ee5d8 Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart <pierre-louis.bossart@intel.com>
Date: Fri, 14 May 2010 15:38:18 -0500
Subject: [PATCH] alsa: support for disabling of period interrupts


Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@intel.com>
---
 include/sound/asound.h    |    1 +
 include/sound/pcm.h       |    2 ++
 sound/core/pcm_lib.c      |   22 ++++++++++++----------
 sound/core/pcm_native.c   |    7 ++++++-
 sound/pci/hda/hda_intel.c |   28 +++++++++++++++++++++++++---
 5 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/include/sound/asound.h b/include/sound/asound.h
index 0985955..8d9e97a 100644
--- a/include/sound/asound.h
+++ b/include/sound/asound.h
@@ -330,6 +330,7 @@ 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_DISABLE_PERIOD_INTERRUPTS	(1<<2)	/* disable period interrupts */
 
 struct snd_interval {
 	unsigned int min, max;
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 8b611a5..a5ec8c1 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -55,6 +55,7 @@ struct snd_pcm_hardware {
 	unsigned int periods_min;	/* min # of periods */
 	unsigned int periods_max;	/* max # of periods */
 	size_t fifo_size;		/* fifo size in bytes */
+        unsigned int can_disable_period_interrupts;
 };
 
 struct snd_pcm_substream;
@@ -276,6 +277,7 @@ struct snd_pcm_runtime {
 	snd_pcm_sframes_t delay;	/* extra delay; typically FIFO size */
 
 	/* -- HW params -- */
+        unsigned int flags;
 	snd_pcm_access_t access;	/* access mode */
 	snd_pcm_format_t format;	/* SNDRV_PCM_FORMAT_* */
 	snd_pcm_subformat_t subformat;	/* subformat */
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index a2ff861..ea5194e 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -420,16 +420,18 @@ 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) {
-		hw_ptr_error(substream,
-			     "Lost interrupts? %s"
-			     "(stream=%i, delta=%ld, new_hw_ptr=%ld, "
-			     "old_hw_ptr=%ld)\n",
-			     in_interrupt ? "[Q] " : "",
-			     substream->stream, (long)delta,
-			     (long)new_hw_ptr,
-			     (long)old_hw_ptr);
-	}
+        if (!(runtime->flags & SNDRV_PCM_HW_PARAMS_DISABLE_PERIOD_INTERRUPTS)) {
+                if (delta > runtime->period_size + runtime->period_size / 2) {
+                        hw_ptr_error(substream,
+                                     "Lost interrupts? %s"
+                                     "(stream=%i, delta=%ld, new_hw_ptr=%ld, "
+                                     "old_hw_ptr=%ld)\n",
+                                     in_interrupt ? "[Q] " : "",
+                                     substream->stream, (long)delta,
+                                     (long)new_hw_ptr,
+                                     (long)old_hw_ptr);
+                }
+        }
 
 	if (runtime->status->hw_ptr == new_hw_ptr)
 		return 0;
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index c22ebb0..135131c 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -427,7 +427,7 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
 		if (atomic_read(&substream->mmap_count))
 			return -EBADFD;
 
-	params->rmask = ~0U;
+        params->rmask = ~0U;
 	err = snd_pcm_hw_refine(substream, params);
 	if (err < 0)
 		goto _error;
@@ -442,6 +442,11 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
 			goto _error;
 	}
 
+        if (substream->runtime->hw.can_disable_period_interrupts) {
+                runtime->flags  |= (params->flags&SNDRV_PCM_HW_PARAMS_DISABLE_PERIOD_INTERRUPTS);
+        } else {
+                runtime->flags &= ~SNDRV_PCM_HW_PARAMS_DISABLE_PERIOD_INTERRUPTS;
+        }
 	runtime->access = params_access(params);
 	runtime->format = params_format(params);
 	runtime->subformat = params_subformat(params);
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 170610e..7360b37 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -116,6 +116,12 @@ module_param(power_save_controller, bool, 0644);
 MODULE_PARM_DESC(power_save_controller, "Reset controller in power save mode.");
 #endif
 
+static int disable_period_interrupts = 1;
+module_param(disable_period_interrupts, bool, 0644);
+MODULE_PARM_DESC(disable_period_interrupts, "Disable period interrupts"
+		 "(default true, set to false to force interrupts).");
+
+
 MODULE_LICENSE("GPL");
 MODULE_SUPPORTED_DEVICE("{{Intel, ICH6},"
 			 "{Intel, ICH6M},"
@@ -1157,7 +1163,12 @@ static int setup_bdle(struct snd_pcm_substream *substream,
 {
 	u32 *bdl = *bdlp;
 
-	while (size > 0) {
+        if (substream->runtime->flags & SNDRV_PCM_HW_PARAMS_DISABLE_PERIOD_INTERRUPTS) {
+                /* override ioc */
+                with_ioc = 0;
+        }
+
+        while (size > 0) {
 		dma_addr_t addr;
 		int chunk;
 
@@ -1519,6 +1530,7 @@ static struct snd_pcm_hardware azx_pcm_hw = {
 	.periods_min =		2,
 	.periods_max =		AZX_MAX_FRAG,
 	.fifo_size =		0,
+        .can_disable_period_interrupts =   1,
 };
 
 struct azx_pcm {
@@ -1554,7 +1566,13 @@ static int azx_pcm_open(struct snd_pcm_substream *substream)
 				   128);
 	snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_BYTES,
 				   128);
-	snd_hda_power_up(apcm->codec);
+
+        /* check if module parameter prevents interrupt disable */
+        if (disable_period_interrupts == 0) {
+                runtime->hw.can_disable_period_interrupts = 0;
+        }
+
+        snd_hda_power_up(apcm->codec);
 	err = hinfo->ops.open(hinfo, apcm->codec, substream);
 	if (err < 0) {
 		azx_release_device(azx_dev);
@@ -1947,7 +1965,11 @@ static void azx_irq_pending_work(struct work_struct *work)
 			if (ok > 0) {
 				azx_dev->irq_pending = 0;
 				spin_unlock(&chip->reg_lock);
-				snd_pcm_period_elapsed(azx_dev->substream);
+                                if( azx_dev->substream->runtime->flags & SNDRV_PCM_HW_PARAMS_DISABLE_PERIOD_INTERRUPTS)
+                                        printk(KERN_WARNING
+                                               "hda-intel: IRQ received but period interrupts were disabled. This should not happen \n");
+                                else
+                                        snd_pcm_period_elapsed(azx_dev->substream);
 				spin_lock(&chip->reg_lock);
 			} else if (ok < 0) {
 				pending = 0;	/* too early */
-- 
1.6.6.1


[-- Attachment #4: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [RFC] disabling ALSA period interrupts

[Clemens Ladisch]

pl bossart wrote:
> Takashi Iwai wrote:
> > The biggest problem I can foresee is the handling of PCM position.
> > In the current implementation, the PCM position continues to go over
> > the buffer size until the certain boundary that is close to long int
> > max.  Without interrupts (i.e. snd_pcm_period_elapsed()), this
> > position update won't work reliably.

Pointer updates (snd_pcm_avail or implicitly when writing samples) are
still necessary, and the interval between them can never be more than
the buffer time if you want to avoid xruns.

It seems to work fine with a simple test program.

> Good catch. I thought the processing was the same whether you called
> snd_pcm_period_elapsed or snd_pcm_avail when the timer fires, since in
> both cases you call the .pointer routine, but there's some code in
> snd_pcm_update_hw_ptr0 that is only executed in an interrupt context.

When an interrupt happens, we know that the current position must be at
least at the end of the period that was started at the last interrupt.
This code does the following: If it looks as if the current position is
less than that, we know that the period interrupt was delayed and that
an entire buffer was played before the position wrapped around to the
current position, so we have to add one buffer size to get the correct
position.

In practice, this implies an underrun.  Without interrupts, this
underrun could not be detected (except by the fact your own timer
has arrived too late).

> Likewise there are a bunch of fixes for hw_ptr position that make use
> of this boundary field (which I have to admit I don't understand too
> much).

The boundary is a multiple of the buffer size, so you can use
ptr%buffer_size to get the position in the actual hardware buffer.
The boundary is larger than the buffer size to allow computations on
pointer values even when long xruns happen.

> Here is my latest set of patches before I forget about them.

Instead of a new field in snd_pcm_hardware, you should better use a new
flag so that userspace also knows about this capability.

Following are my own patches that I wrote before I saw yours; they look
essentially the same.

> Still some work to be done on the alsa-lib one, for some reason the
> hw_param->flags field I used gets overwritten if I don't use the
> hw_device. I suspect this is due to some black magic with the
> pcm->hw_flags when slave devices are used.

Until now this field has been used only for flags used internally in
alsa-lib, so this might be a bug in alsa-lib.


Regards,
Clemens

[PATCH 1/3] add API to allow disabling period interrupts

[Clemens Ladisch]

---
 alsa-kernel/include/sound/asound.h  |    4 +
 alsa-kernel/include/sound/pcm.h     |    1
 alsa-kernel/sound/core/pcm_lib.c    |    8 ++-
 alsa-kernel/sound/core/pcm_native.c |    2
 alsa-lib/include/pcm.h              |    4 +
 alsa-lib/include/sound/asound.h     |    3 +
 alsa-lib/src/pcm/pcm.c              |   61 ++++++++++++++++++++++++++++
 alsa-lib/src/pcm/pcm_direct.c       |    2
 alsa-lib/src/pcm/pcm_local.h        |    3 +
 alsa-lib/src/pcm/pcm_multi.c        |    2
 10 files changed, 86 insertions(+), 4 deletions(-)

--- a/alsa-kernel/include/sound/asound.h
+++ b/alsa-kernel/include/sound/asound.h
@@ -255,6 +255,7 @@ typedef int __bitwise snd_pcm_subformat_
 #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;
@@ -330,6 +331,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;
--- a/alsa-kernel/include/sound/pcm.h
+++ b/alsa-kernel/include/sound/pcm.h
@@ -291,6 +291,7 @@ struct snd_pcm_runtime {
 	unsigned int info;
 	unsigned int rate_num;
 	unsigned int rate_den;
+	bool no_period_irq;
 
 	/* -- SW params -- */
 	int tstamp_mode;		/* mmap timestamp is updated */
--- a/alsa-kernel/sound/core/pcm_native.c
+++ b/alsa-kernel/sound/core/pcm_native.c
@@ -453,6 +453,8 @@ static int snd_pcm_hw_params(struct snd_
 	runtime->info = params->info;
 	runtime->rate_num = params->rate_num;
 	runtime->rate_den = params->rate_den;
+	runtime->no_period_irq =
+			!!(params->flags & SNDRV_PCM_HW_PARAMS_NO_PERIOD_IRQ);
 
 	bits = snd_pcm_format_physical_width(runtime->format);
 	runtime->sample_bits = bits;
--- a/alsa-kernel/sound/core/pcm_lib.c
+++ b/alsa-kernel/sound/core/pcm_lib.c
@@ -363,7 +363,8 @@ static int snd_pcm_update_hw_ptr0(struct
 			   (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, "
@@ -384,6 +385,8 @@ static int snd_pcm_update_hw_ptr0(struct
 	 */
 	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;
@@ -420,7 +423,8 @@ static int snd_pcm_update_hw_ptr0(struct
 		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, "
--- a/alsa-lib/include/sound/asound.h
+++ b/alsa-lib/include/sound/asound.h
@@ -278,6 +278,7 @@ enum sndrv_pcm_subformat {
 #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 */
 
 enum sndrv_pcm_state {
 	SNDRV_PCM_STATE_OPEN = 0,	/* stream is open */
@@ -346,6 +347,8 @@ enum sndrv_pcm_hw_param {
 
 #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 sndrv_interval {
 	unsigned int min, max;
--- a/alsa-lib/include/pcm.h
+++ b/alsa-lib/include/pcm.h
@@ -531,6 +531,7 @@ int snd_pcm_hw_params_can_resume(const s
 int snd_pcm_hw_params_is_half_duplex(const snd_pcm_hw_params_t *params);
 int snd_pcm_hw_params_is_joint_duplex(const snd_pcm_hw_params_t *params);
 int snd_pcm_hw_params_can_sync_start(const snd_pcm_hw_params_t *params);
+int snd_pcm_hw_params_can_disable_period_irq(const snd_pcm_hw_params_t *params);
 int snd_pcm_hw_params_get_rate_numden(const snd_pcm_hw_params_t *params,
 				      unsigned int *rate_num,
 				      unsigned int *rate_den);
@@ -622,10 +623,13 @@ int snd_pcm_hw_params_set_rate_minmax(sn
 int snd_pcm_hw_params_set_rate_near(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val, int *dir);
 int snd_pcm_hw_params_set_rate_first(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val, int *dir);
 int snd_pcm_hw_params_set_rate_last(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val, int *dir);
+
 int snd_pcm_hw_params_set_rate_resample(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val);
 int snd_pcm_hw_params_get_rate_resample(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val);
 int snd_pcm_hw_params_set_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val);
 int snd_pcm_hw_params_get_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val);
+int snd_pcm_hw_params_set_period_irq(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val);
+int snd_pcm_hw_params_get_period_irq(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val);
 
 int snd_pcm_hw_params_get_period_time(const snd_pcm_hw_params_t *params, unsigned int *val, int *dir);
 int snd_pcm_hw_params_get_period_time_min(const snd_pcm_hw_params_t *params, unsigned int *val, int *dir);
--- a/alsa-lib/src/pcm/pcm.c
+++ b/alsa-lib/src/pcm/pcm.c
@@ -3085,6 +3085,27 @@ int snd_pcm_hw_params_can_sync_start(con
 }
 
 /**
+ * \brief Check if hardware can disable period interrupts
+ * \param params Configuration space
+ * \return Boolean value
+ * \retval 0 Hardware cannot disable period interrupts
+ * \retval 1 Hardware can disable period interrupts
+ *
+ * It is not allowed to call this function when given configuration is not exactly one.
+ * Usually, #snd_pcm_hw_params() function chooses one configuration
+ * from the configuration space.
+ */
+int snd_pcm_hw_params_can_disable_period_irq(const snd_pcm_hw_params_t *params)
+{
+	assert(params);
+	if (CHECK_SANITY(params->info == ~0U)) {
+		SNDMSG("invalid PCM info field");
+		return 0; /* FIXME: should be a negative error? */
+	}
+	return !!(params->info & SNDRV_PCM_INFO_NO_PERIOD_IRQ);
+}
+
+/**
  * \brief Get rate exact info from a configuration space
  * \param params Configuration space
  * \param rate_num Pointer to returned rate numerator
@@ -4203,6 +4224,46 @@ int snd_pcm_hw_params_get_export_buffer(
 	return 0;
 }
 
+/**
+ * \brief Restrict a configuration space to settings without period interrupts
+ * \param pcm PCM handle
+ * \param params Configuration space
+ * \param val 0 = disable, 1 = enable (default) period interrupts
+ * \return Zero on success, otherwise a negative error code.
+ *
+ * This function should be called only on devices where
+ * #snd_pcm_hw_params_can_disable_period_irq() returns true. (too late, FIXME)
+ *
+ * Even with disabled period interrupts, the period size/time/count parameters
+ * are valid; it is suggested to use #snd_pcm_hw_params_set_period_size_last().
+ *
+ * When period interrupts are disabled, the application must not use poll() or
+ * any functions that could block on this device.
+ */
+int snd_pcm_hw_params_set_period_irq(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val)
+{
+	assert(pcm && params);
+	if (!val)
+		params->flags |= SND_PCM_HW_PARAMS_NO_PERIOD_IRQ;
+	else
+		params->flags &= ~SND_PCM_HW_PARAMS_NO_PERIOD_IRQ;
+	return snd_pcm_hw_refine(pcm, params);
+}
+
+/**
+ * \brief Extract period interrupt mask from a configuration space
+ * \param pcm PCM handle
+ * \param params Configuration space
+ * \param val 0 = disable, 1 = enable period interrupts
+ * \return Zero on success, otherwise a negative error code.
+ */
+int snd_pcm_hw_params_get_period_irq(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val)
+{
+	assert(pcm && params && val);
+	*val = params->flags & SND_PCM_HW_PARAMS_NO_PERIOD_IRQ ? 0 : 1;
+	return 0;
+}
+
 /**
  * \brief Extract period time from a configuration space
  * \param params Configuration space
--- a/alsa-lib/src/pcm/pcm_local.h
+++ b/alsa-lib/src/pcm/pcm_local.h
@@ -91,9 +91,12 @@ typedef enum sndrv_pcm_hw_param snd_pcm_
 #define SND_PCM_INFO_JOINT_DUPLEX SNDRV_PCM_INFO_JOINT_DUPLEX
 /** device can do a kind of synchronized start */
 #define SND_PCM_INFO_SYNC_START SNDRV_PCM_INFO_SYNC_START
+/** device can disable period interrupts */
+#define SND_PCM_INFO_NO_PERIOD_IRQ SNDRV_PCM_INFO_NO_PERIOD_IRQ
 
 #define SND_PCM_HW_PARAMS_NORESAMPLE SNDRV_PCM_HW_PARAMS_NORESAMPLE
 #define SND_PCM_HW_PARAMS_EXPORT_BUFFER SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER
+#define SND_PCM_HW_PARAMS_NO_PERIOD_IRQ SNDRV_PCM_HW_PARAMS_NO_PERIOD_IRQ
 
 #define SND_PCM_INFO_MONOTONIC	0x80000000
 
--- a/alsa-lib/src/pcm/pcm_direct.c
+++ b/alsa-lib/src/pcm/pcm_direct.c
@@ -741,7 +741,7 @@ int snd_pcm_direct_hw_refine(snd_pcm_t *
 			changed |= err;
 		} while (changed);
 	}
-	params->info = dshare->shmptr->s.info;
+	params->info = dshare->shmptr->s.info & ~SND_PCM_INFO_NO_PERIOD_IRQ;
 #ifdef REFINE_DEBUG
 	snd_output_puts(log, "DMIX REFINE (end):\n");
 	snd_pcm_hw_params_dump(params, log);
--- a/alsa-lib/src/pcm/pcm_multi.c
+++ b/alsa-lib/src/pcm/pcm_multi.c
@@ -157,7 +157,7 @@ static int snd_pcm_multi_hw_refine_cprep
 				    multi->channels_count, 0);
 	if (err < 0)
 		return err;
-	params->info = ~0U;
+	params->info = ~SND_PCM_INFO_NO_PERIOD_IRQ;
 	return 0;
 }

[PATCH 2/3] ALSA: hda-intel: add support for disabling period irq

[Clemens Ladisch]

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1224,7 +1224,8 @@ static int azx_setup_periods(struct azx
 			pos_adj = 0;
 		} else {
 			ofs = setup_bdle(substream, azx_dev,
-					 &bdl, ofs, pos_adj, 1);
+					 &bdl, ofs, pos_adj,
+					 !substream->runtime->no_period_irq);
 			if (ofs < 0)
 				goto error;
 		}
@@ -1236,7 +1237,8 @@ static int azx_setup_periods(struct azx
 					 period_bytes - pos_adj, 0);
 		else
 			ofs = setup_bdle(substream, azx_dev, &bdl, ofs,
-					 period_bytes, 1);
+					 period_bytes,
+					 !substream->runtime->no_period_irq);
 		if (ofs < 0)
 			goto error;
 	}
@@ -1505,7 +1507,8 @@ static struct snd_pcm_hardware azx_pcm_h
 				 /* No full-resume yet implemented */
 				 /* SNDRV_PCM_INFO_RESUME |*/
 				 SNDRV_PCM_INFO_PAUSE |
-				 SNDRV_PCM_INFO_SYNC_START),
+				 SNDRV_PCM_INFO_SYNC_START |
+				 SNDRV_PCM_INFO_NO_PERIOD_IRQ),
 	.formats =		SNDRV_PCM_FMTBIT_S16_LE,
 	.rates =		SNDRV_PCM_RATE_48000,
 	.rate_min =		48000,

[PATCH 3/3] ALSA: oxygen: add support for disabling period irq

[Clemens Ladisch]

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

--- a/sound/pci/oxygen/oxygen_pcm.c
+++ b/sound/pci/oxygen/oxygen_pcm.c
@@ -39,7 +39,8 @@ static const struct snd_pcm_hardware oxy
 		SNDRV_PCM_INFO_MMAP_VALID |
 		SNDRV_PCM_INFO_INTERLEAVED |
 		SNDRV_PCM_INFO_PAUSE |
-		SNDRV_PCM_INFO_SYNC_START,
+		SNDRV_PCM_INFO_SYNC_START |
+		SNDRV_PCM_INFO_NO_PERIOD_IRQ,
 	.formats = SNDRV_PCM_FMTBIT_S16_LE |
 		   SNDRV_PCM_FMTBIT_S32_LE,
 	.rates = SNDRV_PCM_RATE_32000 |
@@ -65,7 +66,8 @@ static const struct snd_pcm_hardware oxy
 		SNDRV_PCM_INFO_MMAP_VALID |
 		SNDRV_PCM_INFO_INTERLEAVED |
 		SNDRV_PCM_INFO_PAUSE |
-		SNDRV_PCM_INFO_SYNC_START,
+		SNDRV_PCM_INFO_SYNC_START |
+		SNDRV_PCM_INFO_NO_PERIOD_IRQ,
 	.formats = SNDRV_PCM_FMTBIT_S16_LE |
 		   SNDRV_PCM_FMTBIT_S32_LE,
 	.rates = SNDRV_PCM_RATE_32000 |
@@ -91,7 +93,8 @@ static const struct snd_pcm_hardware oxy
 		SNDRV_PCM_INFO_MMAP_VALID |
 		SNDRV_PCM_INFO_INTERLEAVED |
 		SNDRV_PCM_INFO_PAUSE |
-		SNDRV_PCM_INFO_SYNC_START,
+		SNDRV_PCM_INFO_SYNC_START |
+		SNDRV_PCM_INFO_NO_PERIOD_IRQ,
 	.formats = SNDRV_PCM_FMTBIT_S16_LE,
 	.rates = SNDRV_PCM_RATE_48000,
 	.rate_min = 48000,
@@ -530,7 +533,10 @@ static int oxygen_prepare(struct snd_pcm
 	oxygen_set_bits8(chip, OXYGEN_DMA_FLUSH, channel_mask);
 	oxygen_clear_bits8(chip, OXYGEN_DMA_FLUSH, channel_mask);
 
-	chip->interrupt_mask |= channel_mask;
+	if (substream->runtime->no_period_irq)
+		chip->interrupt_mask &= ~channel_mask;
+	else
+		chip->interrupt_mask |= channel_mask;
 	oxygen_write16(chip, OXYGEN_INTERRUPT_MASK, chip->interrupt_mask);
 	spin_unlock_irq(&chip->reg_lock);
 	return 0;

Re: [PATCH 1/3] add API to allow disabling period interrupts

[Jassi Brar]

On Mon, May 17, 2010 at 6:14 PM, Clemens Ladisch <clemens@ladisch.de> wrote:
Hi,
 I am in favor of support variable hw_interrupt at lowest level, i.e, in
 ring buffer driver, instead of disabling hw_interrupt altogether.
 That would provide even more flexibility for at least future embedded
  systems that support special low-power-audio-mode involving
  long durations of audio playback while the system is in suspend mode.

 Please have a look at
  http://mailman.alsa-project.org/pipermail/alsa-devel/2010-May/027674.html
 and consider use case for embedded systems too.

Thanks.

Re: [PATCH 1/3] add API to allow disabling period interrupts

[Clemens Ladisch]

Jassi Brar wrote:
> I am in favor of support variable hw_interrupt at lowest level, i.e, in
> ring buffer driver, instead of disabling hw_interrupt altogether.

Removing the constant-sized periods restriction would certainly be
useful.  However, it doesn't look as if anybody has the time to redesign
the ALSA API, the kernel framework and all the drivers.


Regards,
Clemens

Re: [PATCH 1/3] add API to allow disabling period interrupts

[Jassi Brar]

On Mon, May 17, 2010 at 8:16 PM, Clemens Ladisch <clemens@ladisch.de> wrote:
> Jassi Brar wrote:
>> I am in favor of support variable hw_interrupt at lowest level, i.e, in
>> ring buffer driver, instead of disabling hw_interrupt altogether.
>
> Removing the constant-sized periods restriction would certainly be
> useful.  However, it doesn't look as if anybody has the time to redesign
> the ALSA API, the kernel framework and all the drivers.
No need to redesign ALSA API and certainly no need to change _any_ driver,
just like this interrupt disable call is optional so would period resize be.

I only ask to make this newly added call as period-resize rather than
a special case of period-disable.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/3] add API to allow disabling period interrupts

[pl bossart]

Looks good. Using the .info field is probably better than adding a new
one as I did. One correction though:

> +       runtime->no_period_irq =
> +                       !!(params->flags & SNDRV_PCM_HW_PARAMS_NO_PERIOD_IRQ);

An additional test is needed here so that the capabilities of the
hardware are double-checked

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;

Re: [PATCH 2/3] ALSA: hda-intel: add support for disabling period irq

[pl bossart]

Pretty much the same code as mine. How many ways can one find to
program a bit...
We may want to keep a module parameter to disable this functionality
for debug purposes,same for adding a trace message shall an interrupt
still happen.

Re: [RFC] disabling ALSA period interrupts

[pl bossart]

> Instead of a new field in snd_pcm_hardware, you should better use a new
> flag so that userspace also knows about this capability.

Yes this is much better indeed.

Re: [PATCH 1/3] add API to allow disabling period interrupts

[Jaroslav Kysela]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1858 bytes --]

On Mon, 17 May 2010, Jassi Brar wrote:

> On Mon, May 17, 2010 at 8:16 PM, Clemens Ladisch <clemens@ladisch.de> wrote:
>> Jassi Brar wrote:
>>> I am in favor of support variable hw_interrupt at lowest level, i.e, in
>>> ring buffer driver, instead of disabling hw_interrupt altogether.
>>
>> Removing the constant-sized periods restriction would certainly be
>> useful.  However, it doesn't look as if anybody has the time to redesign
>> the ALSA API, the kernel framework and all the drivers.
> No need to redesign ALSA API and certainly no need to change _any_ driver,
> just like this interrupt disable call is optional so would period resize be.
>
> I only ask to make this newly added call as period-resize rather than
> a special case of period-disable.

This is very good point. But I have two comments:

1) Period-disable function is OK, but it should not have a name "no
    period irq": SNDRV_PCM_INFO_PERIOD_DISABLE or DISABLE_PERIOD looks
    better. This change does imply to set the period size automatically in
    the driver - probably to highest value (applications cannot choose/set
    the period size in this operation mode - it's useless anyway).
2) The avail_min parameter in sw_params was overlooked. The lowlevel
    drivers can use this value to compute the wake-up point and set hw
    appropriately, to do wake-up at requested time. We can add a support
    functions like "return how many samples are expected to be transferred
    for next wake-up point" to linux/sound/pcm.h. In case when this value
    is high, no interrupts (wake ups) will be processed in the driver. If
    hardware cannot do the precise transfers, we can program a system
    timer as the wake-up source.

 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/3] add API to allow disabling period interrupts

[pl bossart]

> 2) The avail_min parameter in sw_params was overlooked. The lowlevel
>   drivers can use this value to compute the wake-up point and set hw
>   appropriately, to do wake-up at requested time. We can add a support
>   functions like "return how many samples are expected to be transferred
>   for next wake-up point" to linux/sound/pcm.h. In case when this value
>   is high, no interrupts (wake ups) will be processed in the driver. If
>   hardware cannot do the precise transfers, we can program a system
>   timer as the wake-up source.

Isn't the interrupt-related behavior defined when you setup the DMA
linked list. i.e when hw_params are frozen? The problem with sw_params
is that they can change at any time, and the hardware may not support
this. I have no idea how you would modify the HDAudio controller
behavior dynamically for example.

Re: [PATCH 1/3] add API to allow disabling period interrupts

[Jaroslav Kysela]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1150 bytes --]

On Mon, 17 May 2010, pl bossart wrote:

>> 2) The avail_min parameter in sw_params was overlooked. The lowlevel
>>   drivers can use this value to compute the wake-up point and set hw
>>   appropriately, to do wake-up at requested time. We can add a support
>>   functions like "return how many samples are expected to be transferred
>>   for next wake-up point" to linux/sound/pcm.h. In case when this value
>>   is high, no interrupts (wake ups) will be processed in the driver. If
>>   hardware cannot do the precise transfers, we can program a system
>>   timer as the wake-up source.
>
> Isn't the interrupt-related behavior defined when you setup the DMA
> linked list. i.e when hw_params are frozen? The problem with sw_params
> is that they can change at any time, and the hardware may not support
> this. I have no idea how you would modify the HDAudio controller
> behavior dynamically for example.

Look my last sentence - we should use another timing source like system 
timer in this case.

 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/3] add API to allow disabling period interrupts

[pl bossart]

>>> 2) The avail_min parameter in sw_params was overlooked. The lowlevel
>>>   drivers can use this value to compute the wake-up point and set hw
>>>   appropriately, to do wake-up at requested time. We can add a support
>>>   functions like "return how many samples are expected to be transferred
>>>   for next wake-up point" to linux/sound/pcm.h. In case when this value
>>>   is high, no interrupts (wake ups) will be processed in the driver. If
>>>   hardware cannot do the precise transfers, we can program a system
>>>   timer as the wake-up source.
>>
>> Isn't the interrupt-related behavior defined when you setup the DMA
>> linked list. i.e when hw_params are frozen? The problem with sw_params
>> is that they can change at any time, and the hardware may not support
>> this. I have no idea how you would modify the HDAudio controller
>> behavior dynamically for example.
>
> Look my last sentence - we should use another timing source like system
> timer in this case.

Sorry, I misunderstood what you meant by 'precise transfers'. If we
use a system timer, we would need to keep track of the drift between
audio clock and system time as PulseAudio does it. Would your proposal
entail moving this interpolation code into the kernel and let
PulseAudio only program avail_min?

Re: [PATCH 1/3] add API to allow disabling period interrupts

[Jaroslav Kysela]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1809 bytes --]

On Mon, 17 May 2010, pl bossart wrote:

>>>> 2) The avail_min parameter in sw_params was overlooked. The lowlevel
>>>>   drivers can use this value to compute the wake-up point and set hw
>>>>   appropriately, to do wake-up at requested time. We can add a support
>>>>   functions like "return how many samples are expected to be transferred
>>>>   for next wake-up point" to linux/sound/pcm.h. In case when this value
>>>>   is high, no interrupts (wake ups) will be processed in the driver. If
>>>>   hardware cannot do the precise transfers, we can program a system
>>>>   timer as the wake-up source.
>>>
>>> Isn't the interrupt-related behavior defined when you setup the DMA
>>> linked list. i.e when hw_params are frozen? The problem with sw_params
>>> is that they can change at any time, and the hardware may not support
>>> this. I have no idea how you would modify the HDAudio controller
>>> behavior dynamically for example.
>>
>> Look my last sentence - we should use another timing source like system
>> timer in this case.
>
> Sorry, I misunderstood what you meant by 'precise transfers'. If we
> use a system timer, we would need to keep track of the drift between
> audio clock and system time as PulseAudio does it. Would your proposal
> entail moving this interpolation code into the kernel and let
> PulseAudio only program avail_min?

I think that this is not job for the driver. The driver should just
obtain the current DMA position at the wake-up time and eventually store 
the monotonic clock for a drift handling in the user space. Note that 
even with IRQs, the actual DMA position can be delayed a bit (irq 
processing).

 						Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/3] add API to allow disabling period interrupts

[Jassi Brar]

On Mon, May 17, 2010 at 11:42 PM, Jaroslav Kysela <perex@perex.cz> wrote:
> On Mon, 17 May 2010, Jassi Brar wrote:
>
>> On Mon, May 17, 2010 at 8:16 PM, Clemens Ladisch <clemens@ladisch.de>
>> wrote:
>>>
>>> Jassi Brar wrote:
>>>>
>>>> I am in favor of support variable hw_interrupt at lowest level, i.e, in
>>>> ring buffer driver, instead of disabling hw_interrupt altogether.
>>>
>>> Removing the constant-sized periods restriction would certainly be
>>> useful.  However, it doesn't look as if anybody has the time to redesign
>>> the ALSA API, the kernel framework and all the drivers.
>>
>> No need to redesign ALSA API and certainly no need to change _any_ driver,
>> just like this interrupt disable call is optional so would period resize
>> be.
>>
>> I only ask to make this newly added call as period-resize rather than
>> a special case of period-disable.
>
> This is very good point. But I have two comments:
>
> 1) Period-disable function is OK, but it should not have a name "no
>   period irq": SNDRV_PCM_INFO_PERIOD_DISABLE or DISABLE_PERIOD looks
>   better. This change does imply to set the period size automatically in
>   the driver - probably to highest value (applications cannot choose/set
>   the period size in this operation mode - it's useless anyway).
> 2) The avail_min parameter in sw_params was overlooked. The lowlevel
>   drivers can use this value to compute the wake-up point and set hw
>   appropriately, to do wake-up at requested time. We can add a support
>   functions like "return how many samples are expected to be transferred
>   for next wake-up point" to linux/sound/pcm.h. In case when this value
>   is high, no interrupts (wake ups) will be processed in the driver. If
>   hardware cannot do the precise transfers, we can program a system
>   timer as the wake-up source.
Sounds good, though I had a different implementation in mind...
A new _optionally_ supported call to set the ratio of h/w period
to ring buffer.
The fields are returned with actually set ratio by the low
level driver upon call return. The boundary case of say 0/0
can be interpreted as intr disable and n/n as period := ring
The call can be used to query current ratio by asking for
a/b where a > b i.e, invalid ratio.
Depending upon the capability of the h/w and it's driver,
fine-tuning can be achieved to max possible extent.


something like....

snd_pcm_uframes_t prd, ring;

/* Get current ratio */
prd = 257, ring = 256; /* Invalid ratio */
snd_pcm_set_ratio(&prd, &ring);

/* Increase the period by desirable amount */
prd += incr;
snd_pcm_set_ratio(&prd, &ring);

/* Disable period interrupts */
prd = 0, ring = 0;
snd_pcm_set_ratio(&prd, &ring);
if (!prd && !ring) {
   Interrupts are successfull disabled;
} else {
    prd/ring ratio is max supported by h/w
}
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [RFC] disabling ALSA period interrupts

[Clemens Ladisch]

pl bossart wrote:
> Still some work to be done on the alsa-lib one, for some reason the
> hw_param->flags field I used gets overwritten if I don't use the
> hw_device.

In the plugins, the are separate hw_params structures for the slave and
the virtual device; the *_schange and *_cchange function are responsible
for copying parameters from one to the other.  These functions currently
do not copy the flags field.


Regards,
Clemens