The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Yu-Hsuan Hsu <yuhsuan@chromium.org>
Cc: linux-kernel@vger.kernel.org, "Jaroslav Kysela" <perex@perex.cz>,
	"Takashi Iwai" <tiwai@suse.com>,
	"Cássio Gabriel" <cassiogabrielcontato@gmail.com>,
	linux-sound@vger.kernel.org
Subject: Re: [PATCH] ALSA: aloop: Add 'hrtimer' option to timer_source
Date: Fri, 03 Jul 2026 13:12:03 +0200	[thread overview]
Message-ID: <87se6072f0.wl-tiwai@suse.de> (raw)
In-Reply-To: <20260702170356.3418432-1-yuhsuan@chromium.org>

On Thu, 02 Jul 2026 19:03:48 +0200,
Yu-Hsuan Hsu wrote:
> 
> The snd-aloop driver currently defaults to using the system jiffies
> timer (struct timer_list). On systems configured with a low timer
> interrupt frequency (e.g., CONFIG_HZ=250), the jiffies resolution
> (4ms per tick) is insufficient for precise audio timing. For
> example, a 10ms audio period requires 2.5 jiffies ticks, causing
> timing jitter that leads to capture underruns.
> 
> Introduce "hrtimer" as a supported timer_source option. When
> timer_source="hrtimer" is set, aloop uses high-resolution timers
> (hrtimer) to drive period updates. This provides nanosecond-level
> accuracy regardless of CONFIG_HZ and operates independently of other
> hardware audio cards.
> 
> Signed-off-by: Yu-Hsuan Hsu <yuhsuan@chromium.org>

I guess the code depends on CONFIG_HIGH_RES_TIMERS, so we'd need
either an additional Kconfig dependency or some ifdefs to build
properly.

Other than that, I find the idea fine.


thanks,

Takashi

> ---
>  sound/drivers/aloop.c | 101 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 98 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/drivers/aloop.c b/sound/drivers/aloop.c
> index 06bfe09eae1a..35018c33a92b 100644
> --- a/sound/drivers/aloop.c
> +++ b/sound/drivers/aloop.c
> @@ -23,6 +23,7 @@
>  #include <linux/wait.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/hrtimer.h>
>  #include <sound/core.h>
>  #include <sound/control.h>
>  #include <sound/pcm.h>
> @@ -55,7 +56,7 @@ MODULE_PARM_DESC(pcm_substreams, "PCM substreams # (1-8) for loopback driver.");
>  module_param_array(pcm_notify, int, NULL, 0444);
>  MODULE_PARM_DESC(pcm_notify, "Break capture when PCM format/rate/channels changes.");
>  module_param_array(timer_source, charp, NULL, 0444);
> -MODULE_PARM_DESC(timer_source, "Sound card name or number and device/subdevice number of timer to be used. Empty string for jiffies timer [default].");
> +MODULE_PARM_DESC(timer_source, "Sound card name or number and device/subdevice number of timer to be used. Empty string for jiffies timer [default], 'hrtimer' for high-resolution timer.");
>  
>  #define NO_PITCH 100000
>  
> @@ -161,8 +162,9 @@ struct loopback_pcm {
>  	unsigned int period_size_frac;	/* period size in jiffies ticks */
>  	unsigned int last_drift;
>  	unsigned long last_jiffies;
> -	/* If jiffies timer is used */
> +	/* If jiffies / hrtimer is used */
>  	struct timer_list timer;
> +	struct hrtimer hrtimer;
>  
>  	/* size of per channel buffer in case of non-interleaved access */
>  	unsigned int channel_buf_n;
> @@ -232,6 +234,29 @@ static int loopback_jiffies_timer_start(struct loopback_pcm *dpcm)
>  	return 0;
>  }
>  
> +/* call in cable->lock */
> +static int loopback_hrtimer_start(struct loopback_pcm *dpcm)
> +{
> +	unsigned long tick;
> +	unsigned int rate_shift = get_rate_shift(dpcm);
> +
> +	if (rate_shift != dpcm->pcm_rate_shift) {
> +		dpcm->pcm_rate_shift = rate_shift;
> +		dpcm->period_size_frac = frac_pos(dpcm, dpcm->pcm_period_size);
> +	}
> +	if (dpcm->period_size_frac <= dpcm->irq_pos) {
> +		dpcm->irq_pos %= dpcm->period_size_frac;
> +		dpcm->period_update_pending = 1;
> +	}
> +	tick = dpcm->period_size_frac - dpcm->irq_pos;
> +	tick = DIV_ROUND_UP(tick, dpcm->pcm_bps);
> +	hrtimer_start(&dpcm->hrtimer,
> +		      ns_to_ktime(div_u64((u64)tick * NSEC_PER_SEC, HZ)),
> +		      HRTIMER_MODE_REL_SOFT);
> +
> +	return 0;
> +}
> +
>  /* call in cable->lock */
>  static int loopback_snd_timer_start(struct loopback_pcm *dpcm)
>  {
> @@ -270,6 +295,14 @@ static inline int loopback_jiffies_timer_stop(struct loopback_pcm *dpcm)
>  	return 0;
>  }
>  
> +/* call in cable->lock */
> +static inline int loopback_hrtimer_stop(struct loopback_pcm *dpcm)
> +{
> +	hrtimer_cancel(&dpcm->hrtimer);
> +
> +	return 0;
> +}
> +
>  /* call in cable->lock */
>  static int loopback_snd_timer_stop(struct loopback_pcm *dpcm)
>  {
> @@ -300,6 +333,13 @@ static inline int loopback_jiffies_timer_stop_sync(struct loopback_pcm *dpcm)
>  	return 0;
>  }
>  
> +static inline int loopback_hrtimer_stop_sync(struct loopback_pcm *dpcm)
> +{
> +	hrtimer_cancel(&dpcm->hrtimer);
> +
> +	return 0;
> +}
> +
>  /* call in loopback->cable_lock */
>  static int loopback_snd_timer_close_cable(struct loopback_pcm *dpcm)
>  {
> @@ -734,6 +774,28 @@ static void loopback_jiffies_timer_function(struct timer_list *t)
>  		snd_pcm_period_elapsed(dpcm->substream);
>  }
>  
> +static enum hrtimer_restart loopback_hrtimer_function(struct hrtimer *t)
> +{
> +	struct loopback_pcm *dpcm = container_of(t, struct loopback_pcm, hrtimer);
> +	bool period_elapsed = false;
> +
> +	scoped_guard(spinlock_irqsave, &dpcm->cable->lock) {
> +		if (loopback_jiffies_timer_pos_update(dpcm->cable) &
> +		    (1 << dpcm->substream->stream)) {
> +			loopback_hrtimer_start(dpcm);
> +			if (dpcm->period_update_pending) {
> +				dpcm->period_update_pending = 0;
> +				period_elapsed = true;
> +			}
> +		}
> +	}
> +
> +	if (period_elapsed)
> +		snd_pcm_period_elapsed(dpcm->substream);
> +
> +	return HRTIMER_NORESTART;
> +}
> +
>  /* call in cable->lock */
>  static int loopback_snd_timer_check_resolution(struct snd_pcm_runtime *runtime,
>  					       unsigned long resolution)
> @@ -907,6 +969,19 @@ static void loopback_jiffies_timer_dpcm_info(struct loopback_pcm *dpcm,
>  	snd_iprintf(buffer, "    timer_expires:\t%lu\n", dpcm->timer.expires);
>  }
>  
> +static void loopback_hrtimer_dpcm_info(struct loopback_pcm *dpcm,
> +				       struct snd_info_buffer *buffer)
> +{
> +	snd_iprintf(buffer, "    update_pending:\t%u\n",
> +		    dpcm->period_update_pending);
> +	snd_iprintf(buffer, "    irq_pos:\t\t%u\n", dpcm->irq_pos);
> +	snd_iprintf(buffer, "    period_frac:\t%u\n", dpcm->period_size_frac);
> +	snd_iprintf(buffer, "    last_jiffies:\t%lu (%lu)\n",
> +		    dpcm->last_jiffies, jiffies);
> +	snd_iprintf(buffer, "    timer_expires:\t%llu\n",
> +		    ktime_to_ns(hrtimer_get_expires(&dpcm->hrtimer)));
> +}
> +
>  static void loopback_snd_timer_dpcm_info(struct loopback_pcm *dpcm,
>  					 struct snd_info_buffer *buffer)
>  {
> @@ -1097,6 +1172,24 @@ static const struct loopback_ops loopback_jiffies_timer_ops = {
>  	.dpcm_info = loopback_jiffies_timer_dpcm_info,
>  };
>  
> +static int loopback_hrtimer_open(struct loopback_pcm *dpcm)
> +{
> +	hrtimer_setup(&dpcm->hrtimer, loopback_hrtimer_function,
> +		      CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
> +
> +	return 0;
> +}
> +
> +static const struct loopback_ops loopback_hrtimer_ops = {
> +	.open = loopback_hrtimer_open,
> +	.start = loopback_hrtimer_start,
> +	.stop = loopback_hrtimer_stop,
> +	.stop_sync = loopback_hrtimer_stop_sync,
> +	.close_substream = loopback_hrtimer_stop_sync,
> +	.pos_update = loopback_jiffies_timer_pos_update,
> +	.dpcm_info = loopback_hrtimer_dpcm_info,
> +};
> +
>  static int loopback_parse_timer_id(const char *str,
>  				   struct snd_timer_id *tid)
>  {
> @@ -1274,7 +1367,9 @@ static int loopback_open(struct snd_pcm_substream *substream)
>  		spin_lock_init(&cable->lock);
>  		snd_refcount_init(&cable->stop_count);
>  		cable->hw = loopback_pcm_hardware;
> -		if (loopback->timer_source)
> +		if (loopback->timer_source && !strcmp(loopback->timer_source, "hrtimer"))
> +			cable->ops = &loopback_hrtimer_ops;
> +		else if (loopback->timer_source && loopback->timer_source[0])
>  			cable->ops = &loopback_snd_timer_ops;
>  		else
>  			cable->ops = &loopback_jiffies_timer_ops;
> -- 
> 2.55.0.rc0.799.gd6f94ed593-goog
> 

      reply	other threads:[~2026-07-03 11:12 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-02 17:03 [PATCH] ALSA: aloop: Add 'hrtimer' option to timer_source Yu-Hsuan Hsu
2026-07-03 11:12 ` Takashi Iwai [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87se6072f0.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=cassiogabrielcontato@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    --cc=yuhsuan@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox