[alsa-devel] [RFC] disabling ALSA period interrupts
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
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 ?
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
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.
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
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.
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
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
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
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
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.
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.
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
--- 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; }
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.
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
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.
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.
- 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.
On Mon, 17 May 2010, pl bossart wrote:
- 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.
- 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?
On Mon, 17 May 2010, pl bossart wrote:
- 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.
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:
- 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 }
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;
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,
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.
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;
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
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
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
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
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
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
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
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.
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.
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.
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.
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?
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.
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.
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=4df443bbe682055a41e7c224...
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".
Refer to your patch "add rewind-safeguard parameter"
http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=4df443bbe682055a41e7c224...
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.
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=4df443bbe682055a41e7c224...
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
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=4df443bbe682055a41e7c224...
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
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=4df443bbe682055a41e7c224...
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
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.
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.
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.
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.
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
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.
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.
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.
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
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
participants (10)
-
Clemens Ladisch -
James Courtier-Dutton -
Jaroslav Kysela -
Jassi Brar -
Lennart Poettering -
Liam Girdwood -
Mark Brown -
pl bossart -
Raymond Yau -
Takashi Iwai