From nobody Tue Oct 7 10:15:22 2025 Received: from mail-ej1-f73.google.com (mail-ej1-f73.google.com [209.85.218.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 770B2279DB8 for ; Fri, 11 Jul 2025 09:37:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.73 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1752226636; cv=none; b=Oa+52vUXTWSU/l5+ep0Hi017A0kCe3IWZupaZ2HFd0lmKmOyi7mFuokzu4TxZSmf6aDAFKb7NVj+UhVGlyz5ftYHhBOxFAkqJWNcnoz5UAM9pjQEfMX1t6GTxE2lWnuKaQxfJa+WLDh2MzckJ0Tcef+TBDnCRyrj181FUxSHuC8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1752226636; c=relaxed/simple; bh=BF4pLdBjpwXhp9214H0A3l/lQbnRhG7cLFEFj657yF8=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=PDtIKB+3KGziR+i+aQvZQV2udXZE5YF9VUHL+Y19eadS7Qmc9oObAdSUh7iVsBadagjjh/qKKmCnaSW/N7EvbaQRCP8LXnPC6anBfbV1KirebsvkWKHdAwM/6Fax8Ct4GzqN8NH566YfsYJzd38GeJgA9lTlHTKWeEBHTCaZGJ8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--verhaegen.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=ebuAlyLv; arc=none smtp.client-ip=209.85.218.73 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--verhaegen.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="ebuAlyLv" Received: by mail-ej1-f73.google.com with SMTP id a640c23a62f3a-ae6d4c5f798so129196966b.0 for ; Fri, 11 Jul 2025 02:37:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1752226633; x=1752831433; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=wBhpozoKO994tZaJby3EUAdOYCx2fn320yq/NpQfjfg=; b=ebuAlyLvQ9/mByoP9sjoRlXpDJoFV4kSmZV/3z9BXaA0j4eiL5jRndcJus51Mtt4hu OvxFtceeJvwgvPIb8demYbc5kK7uupB5c5JrEww8joWjvNxFsVr/RXwMa6kfz9B+viNh URq6/X47FXCZEtOCROfAUMsLFyS/gIvMr2x1BVUkIXnni/M06+euaMyRzGbXvc1/DgrG 2fG847ARPT7gwqquCI/xVoh2IguMePkm/pZgPmHDVzfxRJBNUOCcipX5ZW+2zm12sMBQ 67v0KiBad+l0eZiTJwHK5zly6zC6YazNJAHY67MLqVrMzaNdutuqKosz1L/Esxbnmx+L DJOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1752226633; x=1752831433; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=wBhpozoKO994tZaJby3EUAdOYCx2fn320yq/NpQfjfg=; b=VBTHM0vOr53G2WWDIuMbXi3+PfJRGRl6/rJ8H9mz47lXIIqcmvGU7wQPiEhpoqYOYI kEDdXZ1rHmzTZHB2XretoExkH1bPyJduHcqGlF9Av4V1QRfd9cl6xNuWTIzL7khu5cGj 1SNoqZ/HdlwBQTowACi0zSHwL20OjdXAe6o8oen6FbLmNFoTgQwqv80PmWejq5L9NA5Q Y4ttufw79OLBaXbR15wiKr0nwCXP/OLBSOHv35+dWjUMqbYooY7OUfvaB7epReRv46ck umXesMKr4TLEINpb3xUsu+TC4hcIn6s+u7T6tgH11Q4ZuNzbQDH6iuJ/g4GdwC1gxWXP fvxg== X-Forwarded-Encrypted: i=1; AJvYcCVlugvwPVjqzKcrPg43vetJ7cNVXkqfKcFuRS/FDkghxUSkd3E9oLinlweobgLYG3CbFSFsfh4AZnCUUMk=@vger.kernel.org X-Gm-Message-State: AOJu0YyD6cx6W2/giU7Egx8W23RlFXy5pr+hY3RUCe8rVzq7e+8RD/mx uUGvwPLCS/riaJS6byjPLFQhZ2ONL17y74bGNuwu/sEMVUtn9xpKqkOeclODcVl82sJ0JNiwi2b QbqUI5FpZ1XTXZVfoLQ== X-Google-Smtp-Source: AGHT+IGIdc1uDMfWkRcYXH2QBFFa8MaZ4zaOUnOEybovNZBaLXnRsrqkGYBocqlyoisFxGvLN/JgmGhrEpyjbXY= X-Received: from ejaw15.prod.google.com ([2002:a17:906:33cf:b0:ae4:a43:f265]) (user=verhaegen job=prod-delivery.src-stubby-dispatcher) by 2002:a17:907:970f:b0:ae0:da2d:44b9 with SMTP id a640c23a62f3a-ae6fbc2654fmr271916766b.7.1752226632845; Fri, 11 Jul 2025 02:37:12 -0700 (PDT) Date: Fri, 11 Jul 2025 10:36:27 +0100 In-Reply-To: <20250711093636.28204-1-verhaegen@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20250711093636.28204-1-verhaegen@google.com> X-Mailer: git-send-email 2.50.0.727.gbf7dc18ff4-goog Message-ID: <20250711093636.28204-2-verhaegen@google.com> Subject: [PATCH v2 1/4] ALSA: compress_offload: Add 64-bit safe timestamp infrastructure From: Joris Verhaegen To: Vinod Koul , Jaroslav Kysela , Takashi Iwai , Liam Girdwood , Mark Brown , Charles Keepax , Richard Fitzgerald , David Rhodes , Cezary Rojewski , Peter Ujfalusi , Bard Liao , Ranjani Sridharan , Kai Vehmanen , Pierre-Louis Bossart , Srinivas Kandagatla , Daniel Baluta , Orson Zhai , Baolin Wang , Chunyan Zhang , Kunihiko Hayashi , Masami Hiramatsu Cc: Joris Verhaegen , kernel-team@android.com, linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org, patches@opensource.cirrus.com, linux-arm-msm@vger.kernel.org, sound-open-firmware@alsa-project.org, linux-arm-kernel@lists.infradead.org, David Li , Miller Liang Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" The copied_total field in struct snd_compr_tstamp is a 32-bit value that can overflow on long-running high-bitrate streams, leading to incorrect calculations for buffer availablility. This patch adds a 64-bit safe timestamping mechanism. It introduces struct snd_compr_tstamp64 in the UAPI header which uses __u64 for cumulative counters. A new .pointer64 operation is added to the relevant ASoC and core compress ops structures. Corresponding ASoC wrapper functions are also added. The core timestamping logic is refactored to use u64 for its internal total_bytes_transferred counter, fixing the internal state overflow. The logic now attempts to use the new .pointer64 op first, falling back to the legacy 32-bit path if the new op is not implemented by a driver. Signed-off-by: Joris Verhaegen Tested-by: Joris Verhaegen Reviewed-by: David Li Reviewed-by: Miller Liang --- include/sound/compress_driver.h | 3 ++ include/sound/soc-component.h | 5 ++ include/sound/soc-dai.h | 6 +++ include/uapi/sound/compress_offload.h | 19 +++++++ sound/core/compress_offload.c | 78 +++++++++++++++++++++------ sound/soc/soc-component.c | 20 +++++++ sound/soc/soc-compress.c | 21 ++++++++ sound/soc/soc-dai.c | 14 +++++ 8 files changed, 150 insertions(+), 16 deletions(-) diff --git a/include/sound/compress_driver.h b/include/sound/compress_drive= r.h index b55c9eeb2b54..a047ff14a420 100644 --- a/include/sound/compress_driver.h +++ b/include/sound/compress_driver.h @@ -136,6 +136,7 @@ struct snd_compr_stream { * @trigger: Trigger operations like start, pause, resume, drain, stop. * This callback is mandatory * @pointer: Retrieve current h/w pointer information. Mandatory + * @pointer64: Retrieve current h/w pointer information in 64 bit. Optional * @copy: Copy the compressed data to/from userspace, Optional * Can't be implemented if DSP supports mmap * @mmap: DSP mmap method to mmap DSP memory @@ -162,6 +163,8 @@ struct snd_compr_ops { int (*trigger)(struct snd_compr_stream *stream, int cmd); int (*pointer)(struct snd_compr_stream *stream, struct snd_compr_tstamp *tstamp); + int (*pointer64)(struct snd_compr_stream *stream, + struct snd_compr_tstamp64 *tstamp); int (*copy)(struct snd_compr_stream *stream, char __user *buf, size_t count); int (*mmap)(struct snd_compr_stream *stream, diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h index 61534ac0edd1..07d9f0ff7b1c 100644 --- a/include/sound/soc-component.h +++ b/include/sound/soc-component.h @@ -48,6 +48,9 @@ struct snd_compress_ops { int (*pointer)(struct snd_soc_component *component, struct snd_compr_stream *stream, struct snd_compr_tstamp *tstamp); + int (*pointer64)(struct snd_soc_component *component, + struct snd_compr_stream *stream, + struct snd_compr_tstamp64 *tstamp); int (*copy)(struct snd_soc_component *component, struct snd_compr_stream *stream, char __user *buf, size_t count); @@ -500,6 +503,8 @@ int snd_soc_component_compr_get_codec_caps(struct snd_c= ompr_stream *cstream, int snd_soc_component_compr_ack(struct snd_compr_stream *cstream, size_t b= ytes); int snd_soc_component_compr_pointer(struct snd_compr_stream *cstream, struct snd_compr_tstamp *tstamp); +int snd_soc_component_compr_pointer64(struct snd_compr_stream *cstream, + struct snd_compr_tstamp64 *tstamp); int snd_soc_component_compr_copy(struct snd_compr_stream *cstream, char __user *buf, size_t count); int snd_soc_component_compr_set_metadata(struct snd_compr_stream *cstream, diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index d19ab5572d2b..8ecfa966e28a 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -257,6 +257,9 @@ int snd_soc_dai_compr_ack(struct snd_soc_dai *dai, int snd_soc_dai_compr_pointer(struct snd_soc_dai *dai, struct snd_compr_stream *cstream, struct snd_compr_tstamp *tstamp); +int snd_soc_dai_compr_pointer64(struct snd_soc_dai *dai, + struct snd_compr_stream *cstream, + struct snd_compr_tstamp64 *tstamp); int snd_soc_dai_compr_set_metadata(struct snd_soc_dai *dai, struct snd_compr_stream *cstream, struct snd_compr_metadata *metadata); @@ -385,6 +388,9 @@ struct snd_soc_cdai_ops { struct snd_soc_dai *); int (*pointer)(struct snd_compr_stream *, struct snd_compr_tstamp *, struct snd_soc_dai *); + int (*pointer64)(struct snd_compr_stream *cstream, + struct snd_compr_tstamp64 *tstamp, + struct snd_soc_dai *dai); int (*ack)(struct snd_compr_stream *, size_t, struct snd_soc_dai *); }; diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/com= press_offload.h index d62eb93af0ed..abd0ea3f86ee 100644 --- a/include/uapi/sound/compress_offload.h +++ b/include/uapi/sound/compress_offload.h @@ -56,6 +56,25 @@ struct snd_compr_tstamp { __u32 sampling_rate; } __attribute__((packed, aligned(4))); =20 +/** + * struct snd_compr_tstamp64 - timestamp descriptor with fields in 64 bit + * @byte_offset: Byte offset in ring buffer to DSP + * @copied_total: Total number of bytes copied from/to ring buffer to/by D= SP + * @pcm_frames: Frames decoded or encoded by DSP. This field will evolve by + * large steps and should only be used to monitor encoding/decoding + * progress. It shall not be used for timing estimates. + * @pcm_io_frames: Frames rendered or received by DSP into a mixer or an a= udio + * output/input. This field should be used for A/V sync or time estimates. + * @sampling_rate: sampling rate of audio + */ +struct snd_compr_tstamp64 { + __u32 byte_offset; + __u64 copied_total; + __u64 pcm_frames; + __u64 pcm_io_frames; + __u32 sampling_rate; +} __attribute__((packed, aligned(4))); + /** * struct snd_compr_avail - avail descriptor * @avail: Number of bytes available in ring buffer for writing/reading diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c index 840bb9cfe789..6a8873bd62ae 100644 --- a/sound/core/compress_offload.c +++ b/sound/core/compress_offload.c @@ -176,18 +176,65 @@ static int snd_compr_free(struct inode *inode, struct= file *f) return 0; } =20 +static int snd_compr_get_tstamp64(struct snd_compr_stream *stream, + struct snd_compr_tstamp64 *tstamp) +{ + struct snd_compr_tstamp64 tstamp64_zero; + + memset(tstamp, 0, sizeof(*tstamp)); + memset(&tstamp64_zero, 0, sizeof(tstamp64_zero)); + + if (!stream->ops->pointer64) + return -EOPNOTSUPP; + stream->ops->pointer64(stream, tstamp); + + /** + * Even if pointer64 is non-null, the driver call may not have been imple= mented + * (due to indirection in soc-component.c). + * Check that the call was successful by verifying that tstamp is not all= zero. + */ + if (memcmp(tstamp, &tstamp64_zero, sizeof(*tstamp)) =3D=3D 0) { + pr_debug("no tstamp returned from pointer64\n"); + return -EOPNOTSUPP; + } + return 0; +} + +static void +snd_compr_tstamp32_from_64(struct snd_compr_tstamp *tstamp32, + const struct snd_compr_tstamp64 *tstamp64) +{ + tstamp32->byte_offset =3D tstamp64->byte_offset; + tstamp32->copied_total =3D (u32)tstamp64->copied_total; + tstamp32->pcm_frames =3D (u32)tstamp64->pcm_frames; + tstamp32->pcm_io_frames =3D (u32)tstamp64->pcm_io_frames; + tstamp32->sampling_rate =3D tstamp64->sampling_rate; +} + static int snd_compr_update_tstamp(struct snd_compr_stream *stream, struct snd_compr_tstamp *tstamp) { - if (!stream->ops->pointer) - return -ENOTSUPP; - stream->ops->pointer(stream, tstamp); - pr_debug("dsp consumed till %d total %d bytes\n", - tstamp->byte_offset, tstamp->copied_total); + u64 copied_total64; + struct snd_compr_tstamp64 tstamp64; + + /* Get 64 bit values if available. Otherwise, fallback to 32 bit. */ + if (snd_compr_get_tstamp64(stream, &tstamp64) =3D=3D 0) { + copied_total64 =3D tstamp64.copied_total; + snd_compr_tstamp32_from_64(tstamp, &tstamp64); + } else { + if (!stream->ops->pointer) + return -EOPNOTSUPP; + stream->ops->pointer(stream, tstamp); + copied_total64 =3D tstamp->copied_total; + } + + pr_debug("dsp consumed till %u total %llu bytes\n", tstamp->byte_offset, + copied_total64); + if (stream->direction =3D=3D SND_COMPRESS_PLAYBACK) - stream->runtime->total_bytes_transferred =3D tstamp->copied_total; + stream->runtime->total_bytes_transferred =3D copied_total64; else - stream->runtime->total_bytes_available =3D tstamp->copied_total; + stream->runtime->total_bytes_available =3D copied_total64; return 0; } =20 @@ -204,9 +251,9 @@ static size_t snd_compr_calc_avail(struct snd_compr_str= eam *stream, pr_debug("detected init and someone forgot to do a write\n"); return stream->runtime->buffer_size; } - pr_debug("app wrote %lld, DSP consumed %lld\n", - stream->runtime->total_bytes_available, - stream->runtime->total_bytes_transferred); + pr_debug("app wrote %llu, DSP consumed %llu\n", + stream->runtime->total_bytes_available, + stream->runtime->total_bytes_transferred); if (stream->runtime->total_bytes_available =3D=3D stream->runtime->total_bytes_transferred) { if (stream->direction =3D=3D SND_COMPRESS_PLAYBACK) { @@ -223,7 +270,7 @@ static size_t snd_compr_calc_avail(struct snd_compr_str= eam *stream, if (stream->direction =3D=3D SND_COMPRESS_PLAYBACK) avail->avail =3D stream->runtime->buffer_size - avail->avail; =20 - pr_debug("ret avail as %lld\n", avail->avail); + pr_debug("ret avail as %llu\n", avail->avail); return avail->avail; } =20 @@ -274,8 +321,7 @@ static int snd_compr_write_data(struct snd_compr_stream= *stream, (app_pointer * runtime->buffer_size); =20 dstn =3D runtime->buffer + app_pointer; - pr_debug("copying %ld at %lld\n", - (unsigned long)count, app_pointer); + pr_debug("copying %lu at %llu\n", (unsigned long)count, app_pointer); if (count < runtime->buffer_size - app_pointer) { if (copy_from_user(dstn, buf, count)) return -EFAULT; @@ -318,7 +364,7 @@ static ssize_t snd_compr_write(struct file *f, const ch= ar __user *buf, } =20 avail =3D snd_compr_get_avail(stream); - pr_debug("avail returned %ld\n", (unsigned long)avail); + pr_debug("avail returned %lu\n", (unsigned long)avail); /* calculate how much we can write to buffer */ if (avail > count) avail =3D count; @@ -374,7 +420,7 @@ static ssize_t snd_compr_read(struct file *f, char __us= er *buf, } =20 avail =3D snd_compr_get_avail(stream); - pr_debug("avail returned %ld\n", (unsigned long)avail); + pr_debug("avail returned %lu\n", (unsigned long)avail); /* calculate how much we can read from buffer */ if (avail > count) avail =3D count; @@ -443,7 +489,7 @@ static __poll_t snd_compr_poll(struct file *f, poll_tab= le *wait) #endif =20 avail =3D snd_compr_get_avail(stream); - pr_debug("avail is %ld\n", (unsigned long)avail); + pr_debug("avail is %lu\n", (unsigned long)avail); /* check if we have at least one fragment to fill */ switch (runtime->state) { case SNDRV_PCM_STATE_DRAINING: diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c index 25f5e543ae8d..7751f9fc2f1d 100644 --- a/sound/soc/soc-component.c +++ b/sound/soc/soc-component.c @@ -656,6 +656,26 @@ int snd_soc_component_compr_pointer(struct snd_compr_s= tream *cstream, } EXPORT_SYMBOL_GPL(snd_soc_component_compr_pointer); =20 +int snd_soc_component_compr_pointer64(struct snd_compr_stream *cstream, + struct snd_compr_tstamp64 *tstamp) +{ + struct snd_soc_pcm_runtime *rtd =3D cstream->private_data; + struct snd_soc_component *component; + int i, ret; + + for_each_rtd_components(rtd, i, component) { + if (component->driver->compress_ops && + component->driver->compress_ops->pointer64) { + ret =3D component->driver->compress_ops->pointer64( + component, cstream, tstamp); + return soc_component_ret(component, ret); + } + } + + return 0; +} +EXPORT_SYMBOL_GPL(snd_soc_component_compr_pointer64); + int snd_soc_component_compr_copy(struct snd_compr_stream *cstream, char __user *buf, size_t count) { diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index 01d1d6bee28c..29a385d327be 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -475,6 +475,25 @@ static int soc_compr_pointer(struct snd_compr_stream *= cstream, return ret; } =20 +static int soc_compr_pointer64(struct snd_compr_stream *cstream, + struct snd_compr_tstamp64 *tstamp) +{ + struct snd_soc_pcm_runtime *rtd =3D cstream->private_data; + int ret; + struct snd_soc_dai *cpu_dai =3D snd_soc_rtd_to_cpu(rtd, 0); + + snd_soc_dpcm_mutex_lock(rtd); + + ret =3D snd_soc_dai_compr_pointer64(cpu_dai, cstream, tstamp); + if (ret < 0) + goto out; + + ret =3D snd_soc_component_compr_pointer64(cstream, tstamp); +out: + snd_soc_dpcm_mutex_unlock(rtd); + return ret; +} + static int soc_compr_set_metadata(struct snd_compr_stream *cstream, struct snd_compr_metadata *metadata) { @@ -513,6 +532,7 @@ static struct snd_compr_ops soc_compr_ops =3D { .get_params =3D soc_compr_get_params, .trigger =3D soc_compr_trigger, .pointer =3D soc_compr_pointer, + .pointer64 =3D soc_compr_pointer64, .ack =3D soc_compr_ack, .get_caps =3D snd_soc_component_compr_get_caps, .get_codec_caps =3D snd_soc_component_compr_get_codec_caps, @@ -528,6 +548,7 @@ static struct snd_compr_ops soc_compr_dyn_ops =3D { .get_metadata =3D soc_compr_get_metadata, .trigger =3D soc_compr_trigger_fe, .pointer =3D soc_compr_pointer, + .pointer64 =3D soc_compr_pointer64, .ack =3D soc_compr_ack, .get_caps =3D snd_soc_component_compr_get_caps, .get_codec_caps =3D snd_soc_component_compr_get_codec_caps, diff --git a/sound/soc/soc-dai.c b/sound/soc/soc-dai.c index a210089747d0..5aadf661b65d 100644 --- a/sound/soc/soc-dai.c +++ b/sound/soc/soc-dai.c @@ -784,6 +784,20 @@ int snd_soc_dai_compr_pointer(struct snd_soc_dai *dai, } EXPORT_SYMBOL_GPL(snd_soc_dai_compr_pointer); =20 +int snd_soc_dai_compr_pointer64(struct snd_soc_dai *dai, + struct snd_compr_stream *cstream, + struct snd_compr_tstamp64 *tstamp) +{ + int ret =3D 0; + + if (dai->driver->cops && + dai->driver->cops->pointer64) + ret =3D dai->driver->cops->pointer64(cstream, tstamp, dai); + + return soc_dai_ret(dai, ret); +} +EXPORT_SYMBOL_GPL(snd_soc_dai_compr_pointer64); + int snd_soc_dai_compr_set_metadata(struct snd_soc_dai *dai, struct snd_compr_stream *cstream, struct snd_compr_metadata *metadata) --=20 2.50.0.727.gbf7dc18ff4-goog