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
>
prev parent 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