alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] disable period wakeups
@ 2010-11-10 21:42 pl bossart
  2010-11-15  9:58 ` Clemens Ladisch
  0 siblings, 1 reply; 6+ messages in thread
From: pl bossart @ 2010-11-10 21:42 UTC (permalink / raw)
  To: alsa-devel

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

Here's the second set of patches based on feedback from Takashi,
Jaroslav and Clemens. Hope this is fine now. How many patches does it
take to flip a bit :-) ?
-Pierre

[-- Attachment #2: 0001-ALSA-support-for-period-wakeup-disabling.patch --]
[-- Type: application/octet-stream, Size: 8120 bytes --]

From 355c4b50dc63d93214ed99d42100ff897d8522b8 Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart <pierre-louis.bossart@intel.com>
Date: Wed, 10 Nov 2010 15:34:23 -0600
Subject: [PATCH] ALSA: support for period wakeup disabling

Merged and cleaned patch based on earlier patches posted
on alsa-devel by Clemens Ladisch <clemens@ladisch.de> and
Pierre-Louis Bossart <pierre-louis.bossart@intel.com>

This patch disables period interrupts which are not
needed when the application relies on a system timer
to wake-up and refill the ring buffer. The behavior of
the driver is left unchanged, and interrupts are only
disabled if the application requests this configuration.
The behavior in case of underruns is slightly different,
instead of being detected during the period interrupts the
underruns are detected when the application calls
snd_pcm_update_avail, which in turns forces a refresh of the
hw pointer and shows the buffer is empty.

More specifically this patch makes a lot of sense when
PulseAudio relies on timer-based scheduling to access audio
devices such as HDAudio or Intel SST. Disabling interrupts
removes two unwanted wake-ups due to period elapsed events
in low-power playback modes. It also simplifies PulseAudio
voice modules used for speech calls.

To quote Lennart "This patch looks very interesting and
desirable. This is something have long been waiting for."

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@intel.com>
---
 core/pcm_lib.c          |    5 +++++
 core/pcm_native.c       |    6 ++++++
 include/asound.h        |    6 +++++-
 include/pcm.h           |    1 +
 pci/hda/hda_intel.c     |    9 ++++++---
 pci/oxygen/oxygen_pcm.c |   14 ++++++++++----
 6 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/core/pcm_lib.c b/core/pcm_lib.c
index a1707cc..1ccea4c 100644
--- a/core/pcm_lib.c
+++ b/core/pcm_lib.c
@@ -373,6 +373,10 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 			   (unsigned long)new_hw_ptr,
 			   (unsigned long)runtime->hw_ptr_base);
 	}
+
+        if (runtime->no_period_wakeup)
+                goto no_jiffies_check_if_no_period_wakeup;
+
 	/* something must be really wrong */
 	if (delta >= runtime->buffer_size + runtime->period_size) {
 		hw_ptr_error(substream,
@@ -442,6 +446,7 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 			     (long)old_hw_ptr);
 	}
 
+ no_jiffies_check_if_no_period_wakeup:
 	if (runtime->status->hw_ptr == new_hw_ptr)
 		return 0;
 
diff --git a/core/pcm_native.c b/core/pcm_native.c
index 8bc7cb3..68bde2b 100644
--- a/core/pcm_native.c
+++ b/core/pcm_native.c
@@ -412,6 +412,12 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
 			goto _error;
 	}
 
+        if (params->info & SNDRV_PCM_INFO_NO_PERIOD_WAKEUP) 
+                runtime->no_period_wakeup = !!(params->flags &
+                                               SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP);
+        else
+                runtime->no_period_wakeup = 0;
+        
 	runtime->access = params_access(params);
 	runtime->format = params_format(params);
 	runtime->subformat = params_subformat(params);
diff --git a/include/asound.h b/include/asound.h
index a1803ec..60b63b2 100644
--- a/include/asound.h
+++ b/include/asound.h
@@ -138,7 +138,7 @@ struct snd_hwdep_dsp_image {
  *                                                                           *
  *****************************************************************************/
 
-#define SNDRV_PCM_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 10)
+#define SNDRV_PCM_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 11)
 
 typedef unsigned long snd_pcm_uframes_t;
 typedef signed long snd_pcm_sframes_t;
@@ -259,6 +259,7 @@ typedef int __bitwise snd_pcm_subformat_t;
 #define SNDRV_PCM_INFO_HALF_DUPLEX	0x00100000	/* only half duplex */
 #define SNDRV_PCM_INFO_JOINT_DUPLEX	0x00200000	/* playback and capture stream are somewhat correlated */
 #define SNDRV_PCM_INFO_SYNC_START	0x00400000	/* pcm support some kind of sync go */
+#define SNDRV_PCM_INFO_NO_PERIOD_WAKEUP 0x00800000      /* period wakeup can be disabled */
 #define SNDRV_PCM_INFO_FIFO_IN_FRAMES	0x80000000	/* internal kernel flag - FIFO size is in frames */
 
 typedef int __bitwise snd_pcm_state_t;
@@ -334,6 +335,9 @@ typedef int snd_pcm_hw_param_t;
 #define	SNDRV_PCM_HW_PARAM_LAST_INTERVAL	SNDRV_PCM_HW_PARAM_TICK_TIME
 
 #define SNDRV_PCM_HW_PARAMS_NORESAMPLE	(1<<0)	/* avoid rate resampling */
+#define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER      (1<<1)  /* export buffer */
+#define SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP   (1<<2)  /* disable period
+                                                          wakeups */
 
 struct snd_interval {
 	unsigned int min, max;
diff --git a/include/pcm.h b/include/pcm.h
index dfd9b76..b3ec506 100644
--- a/include/pcm.h
+++ b/include/pcm.h
@@ -297,6 +297,7 @@ struct snd_pcm_runtime {
 	unsigned int info;
 	unsigned int rate_num;
 	unsigned int rate_den;
+        unsigned int no_period_wakeup;
 
 	/* -- SW params -- */
 	int tstamp_mode;		/* mmap timestamp is updated */
diff --git a/pci/hda/hda_intel.c b/pci/hda/hda_intel.c
index 38b063e..77fc81f 100644
--- a/pci/hda/hda_intel.c
+++ b/pci/hda/hda_intel.c
@@ -1227,7 +1227,8 @@ static int azx_setup_periods(struct azx *chip,
 			pos_adj = 0;
 		} else {
 			ofs = setup_bdle(substream, azx_dev,
-					 &bdl, ofs, pos_adj, 1);
+					 &bdl, ofs, pos_adj,
+                                         !substream->runtime->no_period_wakeup);
 			if (ofs < 0)
 				goto error;
 		}
@@ -1239,7 +1240,8 @@ static int azx_setup_periods(struct azx *chip,
 					 period_bytes - pos_adj, 0);
 		else
 			ofs = setup_bdle(substream, azx_dev, &bdl, ofs,
-					 period_bytes, 1);
+					 period_bytes,
+                                         !substream->runtime->no_period_wakeup);
 		if (ofs < 0)
 			goto error;
 	}
@@ -1507,7 +1509,8 @@ static struct snd_pcm_hardware azx_pcm_hw = {
 				 /* 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_WAKEUP),
 	.formats =		SNDRV_PCM_FMTBIT_S16_LE,
 	.rates =		SNDRV_PCM_RATE_48000,
 	.rate_min =		48000,
diff --git a/pci/oxygen/oxygen_pcm.c b/pci/oxygen/oxygen_pcm.c
index 8146674..dc8f592 100644
--- a/pci/oxygen/oxygen_pcm.c
+++ b/pci/oxygen/oxygen_pcm.c
@@ -39,7 +39,8 @@ static const struct snd_pcm_hardware oxygen_stereo_hardware = {
 		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_WAKEUP,
 	.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 oxygen_multichannel_hardware = {
 		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_WAKEUP,
 	.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 oxygen_ac97_hardware = {
 		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_WAKEUP,
 	.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_substream *substream)
 	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_wakeup)
+                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;
-- 
1.7.2.3


[-- Attachment #3: 0001-pcm-pass-hw_params-flags-to-slave.patch --]
[-- Type: application/octet-stream, Size: 914 bytes --]

From fa64d9458adb16fc149f66a02c96d738a2e8d4c4 Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart <pierre-louis.bossart@intel.com>
Date: Tue, 26 Oct 2010 14:18:23 -0500
Subject: [PATCH 1/2] pcm: pass hw_params flags to slave

fix required before interrupt disabling routines patch can be applied.
Without this fix, the interrupts are only disabled when accessing
directly hw devices.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@intel.com>
---
 src/pcm/pcm_params.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/pcm/pcm_params.c b/src/pcm/pcm_params.c
index 3a90bcd..416b081 100644
--- a/src/pcm/pcm_params.c
+++ b/src/pcm/pcm_params.c
@@ -2126,6 +2126,7 @@ int _snd_pcm_hw_params_refine(snd_pcm_hw_params_t *params,
 			err = changed;
 	}
 	params->info &= src->info;
+        params->flags = src->flags; /* propagate flags to slave */
 	return err;
 }
 
-- 
1.7.2.3


[-- Attachment #4: 0002-add-API-to-disable-period-wakeups.patch --]
[-- Type: application/octet-stream, Size: 6156 bytes --]

From 4ff9ea5681af5730ab6236337a7920bb2837965b Mon Sep 17 00:00:00 2001
From: Pierre-Louis Bossart <pierre-louis.bossart@intel.com>
Date: Wed, 10 Nov 2010 15:09:18 -0600
Subject: [PATCH 2/2] add API to disable period wakeups

2nd cleaned-up version of the patch posted on May 17, 2010 by
Clemens Ladisch <clemens@ladisch.de>
(No filtering in pcm_multi and pcm_direct info fields)

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@intel.com>
---
 include/pcm.h          |    2 +
 include/sound/asound.h |    5 +++-
 src/pcm/pcm.c          |   57 +++++++++++++++++++++++++++++++++++++++++++++++-
 src/pcm/pcm_local.h    |    1 +
 4 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/include/pcm.h b/include/pcm.h
index f3618c3..4ecf5fa 100644
--- a/include/pcm.h
+++ b/include/pcm.h
@@ -626,6 +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_period_wakeup(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val);
+int snd_pcm_hw_params_get_period_wakeup(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);
diff --git a/include/sound/asound.h b/include/sound/asound.h
index fa88938..a6b0fe3 100644
--- a/include/sound/asound.h
+++ b/include/sound/asound.h
@@ -278,6 +278,8 @@ 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_WAKEUP 0x00800000      /* period wakeup can be disabled */
+#define SNDRV_PCM_INFO_FIFO_IN_FRAMES	0x80000000	/* internal kernel flag - FIFO size is in frames */
 
 enum sndrv_pcm_state {
 	SNDRV_PCM_STATE_OPEN = 0,	/* stream is open */
@@ -346,7 +348,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_WAKEUP    (1<<2)  /* disable period
+                                                          wakeups */
 struct sndrv_interval {
 	unsigned int min, max;
 	unsigned int openmin:1,
diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index a49b5b9..3dd61f6 100644
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -726,8 +726,13 @@ int snd_pcm_nonblock(snd_pcm_t *pcm, int nonblock)
 		return err;
 	if (nonblock)
 		pcm->mode |= SND_PCM_NONBLOCK;
-	else
+	else {
+                if (pcm->hw_flags & SND_PCM_HW_PARAMS_NO_PERIOD_WAKEUP)
+                        /* can't use blocking mode if wakeup where disabled */
+                        return -EINVAL;
+                
 		pcm->mode &= ~SND_PCM_NONBLOCK;
+        }
 	return 0;
 }
 
@@ -4200,6 +4205,56 @@ int snd_pcm_hw_params_get_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *par
 }
 
 /**
+ * \brief Restrict a configuration space to settings without period wakeups
+ * \param pcm PCM handle
+ * \param params Configuration space
+ * \param val 0 = disable, 1 = enable (default) period wakeup
+ * \return Zero on success, otherwise a negative error code.
+ *
+ * This function should be called only on devices where non-blocking mode
+ * is enabled. After finalizing the hw_params, the application should verify
+ * if wakeups are indeed disabled by calling
+ * #snd_pcm_hw_params_get_period_wakeup(). If the hardware does not support
+ * this mode, standard period wakeups will be generated.
+ *
+ * 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 wakeups are disabled, the application must not use any functions
+ * that could block on this device. The use of poll should be limited to error
+ * cases. The application needs to use an external event or a timer to
+ * check the state of the ring buffer and refill it apropriately.
+ */
+int snd_pcm_hw_params_set_period_wakeup(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val)
+{
+       assert(pcm && params);
+       
+       /* make sure non-blocking mode is enabled */
+       if (!(pcm->mode & SND_PCM_NONBLOCK))
+               return -EINVAL;
+       
+       if (!val)
+               params->flags |= SND_PCM_HW_PARAMS_NO_PERIOD_WAKEUP;
+       else
+               params->flags &= ~SND_PCM_HW_PARAMS_NO_PERIOD_WAKEUP;
+       return snd_pcm_hw_refine(pcm, params);
+}
+
+/**
+ * \brief Extract period wakeup mask from a configuration space
+ * \param pcm PCM handle
+ * \param params Configuration space
+ * \param val overwritten with 0 = disabled, 1 = enabled period wakeups
+ * \return Zero 
+ */
+int snd_pcm_hw_params_get_period_wakeup(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_WAKEUP ? 0 : 1;
+       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 2f6fcd2..38d9267 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_NO_PERIOD_WAKEUP SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP
 
 #define SND_PCM_INFO_MONOTONIC	0x80000000
 
-- 
1.7.2.3


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

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] disable period wakeups
  2010-11-10 21:42 [PATCH] disable period wakeups pl bossart
@ 2010-11-15  9:58 ` Clemens Ladisch
  2010-11-15 10:26   ` Jaroslav Kysela
  0 siblings, 1 reply; 6+ messages in thread
From: Clemens Ladisch @ 2010-11-15  9:58 UTC (permalink / raw)
  To: pl bossart; +Cc: alsa-devel

pl bossart wrote:
> Here's the second set of patches based on feedback from Takashi,
> Jaroslav and Clemens. Hope this is fine now.

Committed, with fixed whitespace (your mailer or editor replaced tabs
with spaces in all new lines), added back
snd_pcm_hw_params_can_disable_period_wakeup(), and small changes.


Regards,
Clemens

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] disable period wakeups
  2010-11-15  9:58 ` Clemens Ladisch
@ 2010-11-15 10:26   ` Jaroslav Kysela
  2010-11-16  9:19     ` Clemens Ladisch
  0 siblings, 1 reply; 6+ messages in thread
From: Jaroslav Kysela @ 2010-11-15 10:26 UTC (permalink / raw)
  To: Clemens Ladisch; +Cc: pl bossart, alsa-devel

On Mon, 15 Nov 2010, Clemens Ladisch wrote:

> pl bossart wrote:
>> Here's the second set of patches based on feedback from Takashi,
>> Jaroslav and Clemens. Hope this is fine now.
>
> Committed, with fixed whitespace (your mailer or editor replaced tabs
> with spaces in all new lines), added back
> snd_pcm_hw_params_can_disable_period_wakeup(), and small changes.

The problem in the kernel side is that there is no ring buffer boundary 
check using system jiffies. Without this check, the actual implementation 
does not guarantee the consistency of hw_ptr.

 					Jaroslav

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] disable period wakeups
  2010-11-15 10:26   ` Jaroslav Kysela
@ 2010-11-16  9:19     ` Clemens Ladisch
  2010-11-18  8:35       ` Jaroslav Kysela
  0 siblings, 1 reply; 6+ messages in thread
From: Clemens Ladisch @ 2010-11-16  9:19 UTC (permalink / raw)
  To: Jaroslav Kysela; +Cc: pl bossart, alsa-devel

Jaroslav Kysela wrote:
>> pl bossart wrote:
>>> Here's the second set of patches based on feedback from Takashi,
>>> Jaroslav and Clemens. Hope this is fine now.
> 
> The problem in the kernel side is that there is no ring buffer boundary 
> check using system jiffies. Without this check, the actual implementation 
> does not guarantee the consistency of hw_ptr.

How about this?

--8<---------------------------------------------------------------->8--
ALSA: pcm: detect xruns in no-period-wakeup mode

When period wakeups are disabled, successive calls to the pointer update
function do not have a maximum allowed distance, so xruns cannot be
detected with the pointer value only.

To detect xruns, compare the actually elapsed time with the time that
should have theoretically elapsed since the last update.  When the
hardware pointer has wrapped around due to an xrun, the actually elapsed
time will be too big by about hw_ptr_buffer_jiffies.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
---
 sound/core/pcm_lib.c |   18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

--- alsa-kernel/sound/core/pcm_lib.c
+++ alsa-kernel/sound/core/pcm_lib.c
@@ -374,9 +374,23 @@ static int snd_pcm_update_hw_ptr0(struct
 			   (unsigned long)runtime->hw_ptr_base);
 	}
 
-	/* without period interrupts, there are no regular pointer updates */
-	if (runtime->no_period_wakeup)
+	if (runtime->no_period_wakeup) {
+		/*
+		 * Without regular period interrupts, we have to check
+		 * the elapsed time to detect xruns.
+		 */
+		jdelta = jiffies - runtime->hw_ptr_jiffies;
+		hdelta = jdelta - delta * HZ / runtime->rate;
+		while (hdelta > runtime->hw_ptr_buffer_jiffies / 2 + 1) {
+			delta += runtime->buffer_size;
+			hw_base += runtime->buffer_size;
+			if (hw_base >= runtime->boundary)
+				hw_base = 0;
+			new_hw_ptr = hw_base + pos;
+			hdelta -= runtime->hw_ptr_buffer_jiffies;
+		}
 		goto no_delta_check;
+	}
 
 	/* something must be really wrong */
 	if (delta >= runtime->buffer_size + runtime->period_size) {

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] disable period wakeups
  2010-11-16  9:19     ` Clemens Ladisch
@ 2010-11-18  8:35       ` Jaroslav Kysela
  2010-11-18  8:57         ` Clemens Ladisch
  0 siblings, 1 reply; 6+ messages in thread
From: Jaroslav Kysela @ 2010-11-18  8:35 UTC (permalink / raw)
  To: Clemens Ladisch; +Cc: pl bossart, alsa-devel

On Tue, 16 Nov 2010, Clemens Ladisch wrote:

> Jaroslav Kysela wrote:
>>> pl bossart wrote:
>>>> Here's the second set of patches based on feedback from Takashi,
>>>> Jaroslav and Clemens. Hope this is fine now.
>>
>> The problem in the kernel side is that there is no ring buffer boundary
>> check using system jiffies. Without this check, the actual implementation
>> does not guarantee the consistency of hw_ptr.
>
> How about this?

The patch bellow looks good. I would probably add some lightweight 
condition on top like:

 	if (jdelta < runtime->hw_ptr_buffer_jiffies / 2 + 1)
 		goto no_delta_check;

But it's just an optimization. Please, commit.

 					Thanks,
 						Jaroslav

> --8<---------------------------------------------------------------->8--
> ALSA: pcm: detect xruns in no-period-wakeup mode
>
> When period wakeups are disabled, successive calls to the pointer update
> function do not have a maximum allowed distance, so xruns cannot be
> detected with the pointer value only.
>
> To detect xruns, compare the actually elapsed time with the time that
> should have theoretically elapsed since the last update.  When the
> hardware pointer has wrapped around due to an xrun, the actually elapsed
> time will be too big by about hw_ptr_buffer_jiffies.
>
> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
> ---
> sound/core/pcm_lib.c |   18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> --- alsa-kernel/sound/core/pcm_lib.c
> +++ alsa-kernel/sound/core/pcm_lib.c
> @@ -374,9 +374,23 @@ static int snd_pcm_update_hw_ptr0(struct
> 			   (unsigned long)runtime->hw_ptr_base);
> 	}
>
> -	/* without period interrupts, there are no regular pointer updates */
> -	if (runtime->no_period_wakeup)
> +	if (runtime->no_period_wakeup) {
> +		/*
> +		 * Without regular period interrupts, we have to check
> +		 * the elapsed time to detect xruns.
> +		 */
> +		jdelta = jiffies - runtime->hw_ptr_jiffies;
> +		hdelta = jdelta - delta * HZ / runtime->rate;
> +		while (hdelta > runtime->hw_ptr_buffer_jiffies / 2 + 1) {
> +			delta += runtime->buffer_size;
> +			hw_base += runtime->buffer_size;
> +			if (hw_base >= runtime->boundary)
> +				hw_base = 0;
> +			new_hw_ptr = hw_base + pos;
> +			hdelta -= runtime->hw_ptr_buffer_jiffies;
> +		}
> 		goto no_delta_check;
> +	}
>
> 	/* something must be really wrong */
> 	if (delta >= runtime->buffer_size + runtime->period_size) {

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] disable period wakeups
  2010-11-18  8:35       ` Jaroslav Kysela
@ 2010-11-18  8:57         ` Clemens Ladisch
  0 siblings, 0 replies; 6+ messages in thread
From: Clemens Ladisch @ 2010-11-18  8:57 UTC (permalink / raw)
  To: Jaroslav Kysela; +Cc: pl bossart, alsa-devel

Jaroslav Kysela wrote:
> On Tue, 16 Nov 2010, Clemens Ladisch wrote:
> > How about this?
> 
> The patch bellow looks good. I would probably add some lightweight
> condition on top like:
> 
>  	if (jdelta < runtime->hw_ptr_buffer_jiffies / 2 + 1)
>  		goto no_delta_check;
> 
> But it's just an optimization. Please, commit.

Done.


Regards,
Clemens

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-11-18  8:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-10 21:42 [PATCH] disable period wakeups pl bossart
2010-11-15  9:58 ` Clemens Ladisch
2010-11-15 10:26   ` Jaroslav Kysela
2010-11-16  9:19     ` Clemens Ladisch
2010-11-18  8:35       ` Jaroslav Kysela
2010-11-18  8:57         ` Clemens Ladisch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).