* [PATCH v2 1/4] ALSA: compress_offload: Add 64-bit safe timestamp infrastructure
2025-07-11 9:36 [PATCH v2 0/4] ALSA: compress_offload: Add 64-bit safe timestamp API Joris Verhaegen
@ 2025-07-11 9:36 ` Joris Verhaegen
2025-07-11 9:36 ` [PATCH v2 2/4] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl Joris Verhaegen
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Joris Verhaegen @ 2025-07-11 9:36 UTC (permalink / raw)
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, linux-sound, linux-kernel, patches,
linux-arm-msm, sound-open-firmware, linux-arm-kernel, David Li,
Miller Liang
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 <verhaegen@google.com>
Tested-by: Joris Verhaegen <verhaegen@google.com>
Reviewed-by: David Li <dvdli@google.com>
Reviewed-by: Miller Liang <millerliang@google.com>
---
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_driver.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_compr_stream *cstream,
int snd_soc_component_compr_ack(struct snd_compr_stream *cstream, size_t bytes);
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/compress_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)));
+/**
+ * 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 DSP
+ * @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 audio
+ * 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;
}
+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 implemented
+ * (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)) == 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 = tstamp64->byte_offset;
+ tstamp32->copied_total = (u32)tstamp64->copied_total;
+ tstamp32->pcm_frames = (u32)tstamp64->pcm_frames;
+ tstamp32->pcm_io_frames = (u32)tstamp64->pcm_io_frames;
+ tstamp32->sampling_rate = 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) == 0) {
+ copied_total64 = tstamp64.copied_total;
+ snd_compr_tstamp32_from_64(tstamp, &tstamp64);
+ } else {
+ if (!stream->ops->pointer)
+ return -EOPNOTSUPP;
+ stream->ops->pointer(stream, tstamp);
+ copied_total64 = tstamp->copied_total;
+ }
+
+ pr_debug("dsp consumed till %u total %llu bytes\n", tstamp->byte_offset,
+ copied_total64);
+
if (stream->direction == SND_COMPRESS_PLAYBACK)
- stream->runtime->total_bytes_transferred = tstamp->copied_total;
+ stream->runtime->total_bytes_transferred = copied_total64;
else
- stream->runtime->total_bytes_available = tstamp->copied_total;
+ stream->runtime->total_bytes_available = copied_total64;
return 0;
}
@@ -204,9 +251,9 @@ static size_t snd_compr_calc_avail(struct snd_compr_stream *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 ==
stream->runtime->total_bytes_transferred) {
if (stream->direction == SND_COMPRESS_PLAYBACK) {
@@ -223,7 +270,7 @@ static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
if (stream->direction == SND_COMPRESS_PLAYBACK)
avail->avail = stream->runtime->buffer_size - avail->avail;
- pr_debug("ret avail as %lld\n", avail->avail);
+ pr_debug("ret avail as %llu\n", avail->avail);
return avail->avail;
}
@@ -274,8 +321,7 @@ static int snd_compr_write_data(struct snd_compr_stream *stream,
(app_pointer * runtime->buffer_size);
dstn = 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 char __user *buf,
}
avail = 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 = count;
@@ -374,7 +420,7 @@ static ssize_t snd_compr_read(struct file *f, char __user *buf,
}
avail = 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 = count;
@@ -443,7 +489,7 @@ static __poll_t snd_compr_poll(struct file *f, poll_table *wait)
#endif
avail = 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_stream *cstream,
}
EXPORT_SYMBOL_GPL(snd_soc_component_compr_pointer);
+int snd_soc_component_compr_pointer64(struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp64 *tstamp)
+{
+ struct snd_soc_pcm_runtime *rtd = 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 = 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;
}
+static int soc_compr_pointer64(struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp64 *tstamp)
+{
+ struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+ int ret;
+ struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
+
+ snd_soc_dpcm_mutex_lock(rtd);
+
+ ret = snd_soc_dai_compr_pointer64(cpu_dai, cstream, tstamp);
+ if (ret < 0)
+ goto out;
+
+ ret = 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 = {
.get_params = soc_compr_get_params,
.trigger = soc_compr_trigger,
.pointer = soc_compr_pointer,
+ .pointer64 = soc_compr_pointer64,
.ack = soc_compr_ack,
.get_caps = snd_soc_component_compr_get_caps,
.get_codec_caps = snd_soc_component_compr_get_codec_caps,
@@ -528,6 +548,7 @@ static struct snd_compr_ops soc_compr_dyn_ops = {
.get_metadata = soc_compr_get_metadata,
.trigger = soc_compr_trigger_fe,
.pointer = soc_compr_pointer,
+ .pointer64 = soc_compr_pointer64,
.ack = soc_compr_ack,
.get_caps = snd_soc_component_compr_get_caps,
.get_codec_caps = 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);
+int snd_soc_dai_compr_pointer64(struct snd_soc_dai *dai,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp64 *tstamp)
+{
+ int ret = 0;
+
+ if (dai->driver->cops &&
+ dai->driver->cops->pointer64)
+ ret = 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)
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 2/4] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl
2025-07-11 9:36 [PATCH v2 0/4] ALSA: compress_offload: Add 64-bit safe timestamp API Joris Verhaegen
2025-07-11 9:36 ` [PATCH v2 1/4] ALSA: compress_offload: Add 64-bit safe timestamp infrastructure Joris Verhaegen
@ 2025-07-11 9:36 ` Joris Verhaegen
2025-07-11 12:45 ` Takashi Iwai
2025-07-11 9:36 ` [PATCH v2 3/4] ALSA: compress_offload: Add SNDRV_COMPRESS_AVAIL64 ioctl Joris Verhaegen
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Joris Verhaegen @ 2025-07-11 9:36 UTC (permalink / raw)
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, linux-sound, linux-kernel, patches,
linux-arm-msm, sound-open-firmware, linux-arm-kernel, David Li,
Miller Liang
The previous patch introduced the internal infrastructure for handling
64-bit timestamps. This patch exposes this capability to user-space.
Define the new ioctl command SNDRV_COMPRESS_TSTAMP64, which allows
applications to fetch the overflow-safe struct snd_compr_tstamp64.
The ioctl dispatch table is updated to handle the new command by
calling a new snd_compr_tstamp64 handler, while the legacy path is
renamed to snd_compr_tstamp32 for clarity.
Signed-off-by: Joris Verhaegen <verhaegen@google.com>
Tested-by: Joris Verhaegen <verhaegen@google.com>
Reviewed-by: David Li <dvdli@google.com>
Reviewed-by: Miller Liang <millerliang@google.com>
---
include/uapi/sound/compress_offload.h | 2 ++
sound/core/compress_offload.c | 50 +++++++++++++++++++++++----
2 files changed, 45 insertions(+), 7 deletions(-)
diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
index abd0ea3f86ee..2758d9ee3d91 100644
--- a/include/uapi/sound/compress_offload.h
+++ b/include/uapi/sound/compress_offload.h
@@ -208,6 +208,7 @@ struct snd_compr_task_status {
* Note: only codec params can be changed runtime and stream params cant be
* SNDRV_COMPRESS_GET_PARAMS: Query codec params
* SNDRV_COMPRESS_TSTAMP: get the current timestamp value
+ * SNDRV_COMPRESS_TSTAMP64: get the current timestamp value in 64 bit format
* SNDRV_COMPRESS_AVAIL: get the current buffer avail value.
* This also queries the tstamp properties
* SNDRV_COMPRESS_PAUSE: Pause the running stream
@@ -230,6 +231,7 @@ struct snd_compr_task_status {
struct snd_compr_metadata)
#define SNDRV_COMPRESS_TSTAMP _IOR('C', 0x20, struct snd_compr_tstamp)
#define SNDRV_COMPRESS_AVAIL _IOR('C', 0x21, struct snd_compr_avail)
+#define SNDRV_COMPRESS_TSTAMP64 _IOR('C', 0x22, struct snd_compr_tstamp64)
#define SNDRV_COMPRESS_PAUSE _IO('C', 0x30)
#define SNDRV_COMPRESS_RESUME _IO('C', 0x31)
#define SNDRV_COMPRESS_START _IO('C', 0x32)
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index 6a8873bd62ae..40c1e69961b7 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -211,8 +211,8 @@ snd_compr_tstamp32_from_64(struct snd_compr_tstamp *tstamp32,
tstamp32->sampling_rate = tstamp64->sampling_rate;
}
-static int snd_compr_update_tstamp(struct snd_compr_stream *stream,
- struct snd_compr_tstamp *tstamp)
+static int snd_compr_update_tstamp32(struct snd_compr_stream *stream,
+ struct snd_compr_tstamp *tstamp)
{
u64 copied_total64;
struct snd_compr_tstamp64 tstamp64;
@@ -238,11 +238,30 @@ static int snd_compr_update_tstamp(struct snd_compr_stream *stream,
return 0;
}
+static int snd_compr_update_tstamp64(struct snd_compr_stream *stream,
+ struct snd_compr_tstamp64 *tstamp)
+{
+ int retval = snd_compr_get_tstamp64(stream, tstamp);
+
+ if (retval != 0)
+ return retval;
+
+ pr_debug("dsp consumed till %u total %llu bytes\n", tstamp->byte_offset,
+ tstamp->copied_total);
+
+ if (stream->direction == SND_COMPRESS_PLAYBACK)
+ stream->runtime->total_bytes_transferred = tstamp->copied_total;
+ else
+ stream->runtime->total_bytes_available = tstamp->copied_total;
+
+ return 0;
+}
+
static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
struct snd_compr_avail *avail)
{
memset(avail, 0, sizeof(*avail));
- snd_compr_update_tstamp(stream, &avail->tstamp);
+ snd_compr_update_tstamp32(stream, &avail->tstamp);
/* Still need to return avail even if tstamp can't be filled in */
if (stream->runtime->total_bytes_available == 0 &&
@@ -769,19 +788,34 @@ snd_compr_set_metadata(struct snd_compr_stream *stream, unsigned long arg)
return retval;
}
-static inline int
-snd_compr_tstamp(struct snd_compr_stream *stream, unsigned long arg)
+static inline int snd_compr_tstamp32(struct snd_compr_stream *stream,
+ unsigned long arg)
{
struct snd_compr_tstamp tstamp = {0};
int ret;
- ret = snd_compr_update_tstamp(stream, &tstamp);
+ ret = snd_compr_update_tstamp32(stream, &tstamp);
if (ret == 0)
ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
&tstamp, sizeof(tstamp)) ? -EFAULT : 0;
return ret;
}
+static inline int snd_compr_tstamp64(struct snd_compr_stream *stream,
+ unsigned long arg)
+{
+ struct snd_compr_tstamp64 tstamp = { 0 };
+ int ret;
+
+ ret = snd_compr_update_tstamp64(stream, &tstamp);
+ if (ret == 0)
+ ret = copy_to_user((struct snd_compr_tstamp64 __user *)arg,
+ &tstamp, sizeof(tstamp)) ?
+ -EFAULT :
+ 0;
+ return ret;
+}
+
static int snd_compr_pause(struct snd_compr_stream *stream)
{
int retval;
@@ -1355,7 +1389,9 @@ static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
switch (_IOC_NR(cmd)) {
case _IOC_NR(SNDRV_COMPRESS_TSTAMP):
- return snd_compr_tstamp(stream, arg);
+ return snd_compr_tstamp32(stream, arg);
+ case _IOC_NR(SNDRV_COMPRESS_TSTAMP64):
+ return snd_compr_tstamp64(stream, arg);
case _IOC_NR(SNDRV_COMPRESS_AVAIL):
return snd_compr_ioctl_avail(stream, arg);
case _IOC_NR(SNDRV_COMPRESS_PAUSE):
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 2/4] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl
2025-07-11 9:36 ` [PATCH v2 2/4] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl Joris Verhaegen
@ 2025-07-11 12:45 ` Takashi Iwai
0 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-07-11 12:45 UTC (permalink / raw)
To: Joris Verhaegen
Cc: 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, kernel-team, linux-sound,
linux-kernel, patches, linux-arm-msm, sound-open-firmware,
linux-arm-kernel, David Li, Miller Liang
On Fri, 11 Jul 2025 11:36:28 +0200,
Joris Verhaegen wrote:
> @@ -1355,7 +1389,9 @@ static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
>
> switch (_IOC_NR(cmd)) {
> case _IOC_NR(SNDRV_COMPRESS_TSTAMP):
> - return snd_compr_tstamp(stream, arg);
> + return snd_compr_tstamp32(stream, arg);
> + case _IOC_NR(SNDRV_COMPRESS_TSTAMP64):
> + return snd_compr_tstamp64(stream, arg);
> case _IOC_NR(SNDRV_COMPRESS_AVAIL):
> return snd_compr_ioctl_avail(stream, arg);
> case _IOC_NR(SNDRV_COMPRESS_PAUSE):
This function was recently updated to evaluate the ioctl cmd value as
is, as found in commit 19c4096ccdd809c6213e2e62b0d4f57c880138cd
ALSA: compress_offload: tighten ioctl command number checks
in sound git tree (likely included in 6.16-rc7).
Please adjust your patch accordingly.
thanks,
Takashi
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/4] ALSA: compress_offload: Add SNDRV_COMPRESS_AVAIL64 ioctl
2025-07-11 9:36 [PATCH v2 0/4] ALSA: compress_offload: Add 64-bit safe timestamp API Joris Verhaegen
2025-07-11 9:36 ` [PATCH v2 1/4] ALSA: compress_offload: Add 64-bit safe timestamp infrastructure Joris Verhaegen
2025-07-11 9:36 ` [PATCH v2 2/4] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl Joris Verhaegen
@ 2025-07-11 9:36 ` Joris Verhaegen
2025-07-11 9:36 ` [PATCH v2 4/4] ASoC: codecs: Implement 64-bit pointer operation Joris Verhaegen
2025-07-11 11:56 ` [PATCH v2 0/4] ALSA: compress_offload: Add 64-bit safe timestamp API Charles Keepax
4 siblings, 0 replies; 12+ messages in thread
From: Joris Verhaegen @ 2025-07-11 9:36 UTC (permalink / raw)
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, linux-sound, linux-kernel, patches,
linux-arm-msm, sound-open-firmware, linux-arm-kernel, David Li,
Miller Liang
Previous patches introduced ioctl for fetching a 64-bit timestamp via
SNDRV_COMPRESS_TSTAMP64. To provide a consistent API, a 64-bit
version of SNDRV_COMPRESS_AVAIL ioctl is required.
Define a new struct snd_compr_avail64, which embeds the 64-bit
timestamp struct, and define the corresponding SNDRV_COMPRESS_AVAIL64
ioctl command in the UAPI header.
Signed-off-by: Joris Verhaegen <verhaegen@google.com>
Tested-by: Joris Verhaegen <verhaegen@google.com>
Reviewed-by: David Li <dvdli@google.com>
Reviewed-by: Miller Liang <millerliang@google.com>
---
include/uapi/sound/compress_offload.h | 11 ++++
sound/core/compress_offload.c | 88 +++++++++++++++++++++------
2 files changed, 79 insertions(+), 20 deletions(-)
diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
index 2758d9ee3d91..48525a87d98f 100644
--- a/include/uapi/sound/compress_offload.h
+++ b/include/uapi/sound/compress_offload.h
@@ -85,6 +85,16 @@ struct snd_compr_avail {
struct snd_compr_tstamp tstamp;
} __attribute__((packed, aligned(4)));
+/**
+ * struct snd_compr_avail64 - avail descriptor with tstamp in 64 bit format
+ * @avail: Number of bytes available in ring buffer for writing/reading
+ * @tstamp: timestamp information
+ */
+struct snd_compr_avail64 {
+ __u64 avail;
+ struct snd_compr_tstamp64 tstamp;
+} __attribute__((packed, aligned(4)));
+
enum snd_compr_direction {
SND_COMPRESS_PLAYBACK = 0,
SND_COMPRESS_CAPTURE,
@@ -232,6 +242,7 @@ struct snd_compr_task_status {
#define SNDRV_COMPRESS_TSTAMP _IOR('C', 0x20, struct snd_compr_tstamp)
#define SNDRV_COMPRESS_AVAIL _IOR('C', 0x21, struct snd_compr_avail)
#define SNDRV_COMPRESS_TSTAMP64 _IOR('C', 0x22, struct snd_compr_tstamp64)
+#define SNDRV_COMPRESS_AVAIL64 _IOR('C', 0x23, struct snd_compr_avail64)
#define SNDRV_COMPRESS_PAUSE _IO('C', 0x30)
#define SNDRV_COMPRESS_RESUME _IO('C', 0x31)
#define SNDRV_COMPRESS_START _IO('C', 0x32)
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index 40c1e69961b7..78ba86e0d74f 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -257,12 +257,11 @@ static int snd_compr_update_tstamp64(struct snd_compr_stream *stream,
return 0;
}
-static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
- struct snd_compr_avail *avail)
+static size_t snd_compr_calc_avail_internal(struct snd_compr_stream *stream,
+ struct snd_compr_avail *avail32,
+ struct snd_compr_avail64 *avail64)
{
- memset(avail, 0, sizeof(*avail));
- snd_compr_update_tstamp32(stream, &avail->tstamp);
- /* Still need to return avail even if tstamp can't be filled in */
+ u64 avail;
if (stream->runtime->total_bytes_available == 0 &&
stream->runtime->state == SNDRV_PCM_STATE_SETUP &&
@@ -284,33 +283,68 @@ static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
}
}
- avail->avail = stream->runtime->total_bytes_available -
- stream->runtime->total_bytes_transferred;
+ avail = stream->runtime->total_bytes_available -
+ stream->runtime->total_bytes_transferred;
if (stream->direction == SND_COMPRESS_PLAYBACK)
- avail->avail = stream->runtime->buffer_size - avail->avail;
+ avail = stream->runtime->buffer_size - avail;
+
+ if (avail32)
+ avail32->avail = avail;
+ if (avail64)
+ avail64->avail = avail;
+
+ pr_debug("ret avail %llu as %zu\n", avail, (size_t)avail);
+ return avail;
+}
+
+static size_t snd_compr_calc_avail32(struct snd_compr_stream *stream,
+ struct snd_compr_avail *avail)
+{
+ memset(avail, 0, sizeof(*avail));
+ snd_compr_update_tstamp32(stream, &avail->tstamp);
+ /* Still need to return avail even if tstamp can't be filled in */
- pr_debug("ret avail as %llu\n", avail->avail);
- return avail->avail;
+ return snd_compr_calc_avail_internal(stream, avail, NULL);
+}
+
+static size_t snd_compr_calc_avail64(struct snd_compr_stream *stream,
+ struct snd_compr_avail64 *avail)
+{
+ memset(avail, 0, sizeof(*avail));
+ snd_compr_update_tstamp64(stream, &avail->tstamp);
+ /* Still need to return avail even if tstamp can't be filled in */
+
+ return snd_compr_calc_avail_internal(stream, NULL, avail);
}
static inline size_t snd_compr_get_avail(struct snd_compr_stream *stream)
{
struct snd_compr_avail avail;
- return snd_compr_calc_avail(stream, &avail);
+ return snd_compr_calc_avail32(stream, &avail);
}
-static int
-snd_compr_ioctl_avail(struct snd_compr_stream *stream, unsigned long arg)
+static int snd_compr_ioctl_avail(struct snd_compr_stream *stream,
+ unsigned long arg, bool is_64bit)
{
- struct snd_compr_avail ioctl_avail;
- size_t avail;
+ union {
+ struct snd_compr_avail avail32;
+ struct snd_compr_avail64 avail64;
+ } ioctrl_avail_u;
+ size_t avail, ioctrl_avail_size;
if (stream->direction == SND_COMPRESS_ACCEL)
return -EBADFD;
- avail = snd_compr_calc_avail(stream, &ioctl_avail);
- ioctl_avail.avail = avail;
+ if (is_64bit) {
+ avail = snd_compr_calc_avail64(stream, &ioctrl_avail_u.avail64);
+ ioctrl_avail_u.avail64.avail = avail;
+ ioctrl_avail_size = sizeof(ioctrl_avail_u.avail64);
+ } else {
+ avail = snd_compr_calc_avail32(stream, &ioctrl_avail_u.avail32);
+ ioctrl_avail_u.avail32.avail = avail;
+ ioctrl_avail_size = sizeof(ioctrl_avail_u.avail32);
+ }
switch (stream->runtime->state) {
case SNDRV_PCM_STATE_OPEN:
@@ -321,12 +355,24 @@ snd_compr_ioctl_avail(struct snd_compr_stream *stream, unsigned long arg)
break;
}
- if (copy_to_user((__u64 __user *)arg,
- &ioctl_avail, sizeof(ioctl_avail)))
+ if (copy_to_user((__u64 __user *)arg, &ioctrl_avail_u,
+ ioctrl_avail_size))
return -EFAULT;
return 0;
}
+static int snd_compr_ioctl_avail32(struct snd_compr_stream *stream,
+ unsigned long arg)
+{
+ return snd_compr_ioctl_avail(stream, arg, false);
+}
+
+static int snd_compr_ioctl_avail64(struct snd_compr_stream *stream,
+ unsigned long arg)
+{
+ return snd_compr_ioctl_avail(stream, arg, true);
+}
+
static int snd_compr_write_data(struct snd_compr_stream *stream,
const char __user *buf, size_t count)
{
@@ -1393,7 +1439,9 @@ static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
case _IOC_NR(SNDRV_COMPRESS_TSTAMP64):
return snd_compr_tstamp64(stream, arg);
case _IOC_NR(SNDRV_COMPRESS_AVAIL):
- return snd_compr_ioctl_avail(stream, arg);
+ return snd_compr_ioctl_avail32(stream, arg);
+ case _IOC_NR(SNDRV_COMPRESS_AVAIL64):
+ return snd_compr_ioctl_avail64(stream, arg);
case _IOC_NR(SNDRV_COMPRESS_PAUSE):
return snd_compr_pause(stream);
case _IOC_NR(SNDRV_COMPRESS_RESUME):
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 4/4] ASoC: codecs: Implement 64-bit pointer operation
2025-07-11 9:36 [PATCH v2 0/4] ALSA: compress_offload: Add 64-bit safe timestamp API Joris Verhaegen
` (2 preceding siblings ...)
2025-07-11 9:36 ` [PATCH v2 3/4] ALSA: compress_offload: Add SNDRV_COMPRESS_AVAIL64 ioctl Joris Verhaegen
@ 2025-07-11 9:36 ` Joris Verhaegen
2025-07-11 10:10 ` Richard Fitzgerald
2025-07-15 13:07 ` kernel test robot
2025-07-11 11:56 ` [PATCH v2 0/4] ALSA: compress_offload: Add 64-bit safe timestamp API Charles Keepax
4 siblings, 2 replies; 12+ messages in thread
From: Joris Verhaegen @ 2025-07-11 9:36 UTC (permalink / raw)
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, linux-sound, linux-kernel, patches,
linux-arm-msm, sound-open-firmware, linux-arm-kernel, David Li,
Miller Liang
Implement the new .pointer64 compress operation for the various ASoC
drivers that support compress offload.
For drivers with complex but similar 32-bit and 64-bit logic
(wm_adsp, sof, uniphier), a shared internal function is used to handle
both requests, minimizing code duplication and improving maintainability.
For other drivers, internal counters and related variables are updated
to u64 to ensure the source of the timestamp data is overflow-safe.
The legacy .pointer operation is retained in all drivers to ensure
full backward compatibility.
Signed-off-by: Joris Verhaegen <verhaegen@google.com>
Tested-by: Joris Verhaegen <verhaegen@google.com>
Reviewed-by: David Li <dvdli@google.com>
Reviewed-by: Miller Liang <millerliang@google.com>
---
sound/soc/codecs/cs47l15.c | 1 +
sound/soc/codecs/cs47l24.c | 1 +
sound/soc/codecs/cs47l35.c | 1 +
sound/soc/codecs/cs47l85.c | 1 +
sound/soc/codecs/cs47l90.c | 1 +
sound/soc/codecs/cs47l92.c | 1 +
sound/soc/codecs/wm5102.c | 1 +
sound/soc/codecs/wm5110.c | 1 +
sound/soc/codecs/wm_adsp.c | 53 +++++++++++++++----
sound/soc/codecs/wm_adsp.h | 3 ++
.../intel/atom/sst-mfld-platform-compress.c | 17 +++++-
sound/soc/intel/atom/sst-mfld-platform.h | 2 +
sound/soc/intel/atom/sst/sst_drv_interface.c | 43 +++++++++++++--
sound/soc/qcom/qdsp6/q6asm-dai.c | 41 ++++++++++----
sound/soc/sof/compress.c | 44 +++++++++++----
sound/soc/sprd/sprd-pcm-compress.c | 28 +++++++---
sound/soc/sprd/sprd-pcm-dma.h | 2 +-
sound/soc/uniphier/aio-compress.c | 40 +++++++++++---
18 files changed, 232 insertions(+), 49 deletions(-)
diff --git a/sound/soc/codecs/cs47l15.c b/sound/soc/codecs/cs47l15.c
index 29a2bcfb3048..40e75af3367e 100644
--- a/sound/soc/codecs/cs47l15.c
+++ b/sound/soc/codecs/cs47l15.c
@@ -1342,6 +1342,7 @@ static const struct snd_compress_ops cs47l15_compress_ops = {
.get_caps = &wm_adsp_compr_get_caps,
.trigger = &wm_adsp_compr_trigger,
.pointer = &wm_adsp_compr_pointer,
+ .pointer64 = &wm_adsp_compr_pointer64,
.copy = &wm_adsp_compr_copy,
};
diff --git a/sound/soc/codecs/cs47l24.c b/sound/soc/codecs/cs47l24.c
index e2a839fae4fc..30c434a3fa53 100644
--- a/sound/soc/codecs/cs47l24.c
+++ b/sound/soc/codecs/cs47l24.c
@@ -1189,6 +1189,7 @@ static const struct snd_compress_ops cs47l24_compress_ops = {
.get_caps = wm_adsp_compr_get_caps,
.trigger = wm_adsp_compr_trigger,
.pointer = wm_adsp_compr_pointer,
+ .pointer64 = wm_adsp_compr_pointer64,
.copy = wm_adsp_compr_copy,
};
diff --git a/sound/soc/codecs/cs47l35.c b/sound/soc/codecs/cs47l35.c
index 85555c7a2e4b..8b416f452c45 100644
--- a/sound/soc/codecs/cs47l35.c
+++ b/sound/soc/codecs/cs47l35.c
@@ -1624,6 +1624,7 @@ static const struct snd_compress_ops cs47l35_compress_ops = {
.get_caps = &wm_adsp_compr_get_caps,
.trigger = &wm_adsp_compr_trigger,
.pointer = &wm_adsp_compr_pointer,
+ .pointer64 = &wm_adsp_compr_pointer64,
.copy = &wm_adsp_compr_copy,
};
diff --git a/sound/soc/codecs/cs47l85.c b/sound/soc/codecs/cs47l85.c
index d34f4e8c26d3..b8ef0a69a2c5 100644
--- a/sound/soc/codecs/cs47l85.c
+++ b/sound/soc/codecs/cs47l85.c
@@ -2568,6 +2568,7 @@ static const struct snd_compress_ops cs47l85_compress_ops = {
.get_caps = &wm_adsp_compr_get_caps,
.trigger = &wm_adsp_compr_trigger,
.pointer = &wm_adsp_compr_pointer,
+ .pointer64 = &wm_adsp_compr_pointer64,
.copy = &wm_adsp_compr_copy,
};
diff --git a/sound/soc/codecs/cs47l90.c b/sound/soc/codecs/cs47l90.c
index a9e703981f37..1ae44155e306 100644
--- a/sound/soc/codecs/cs47l90.c
+++ b/sound/soc/codecs/cs47l90.c
@@ -2483,6 +2483,7 @@ static const struct snd_compress_ops cs47l90_compress_ops = {
.get_caps = &wm_adsp_compr_get_caps,
.trigger = &wm_adsp_compr_trigger,
.pointer = &wm_adsp_compr_pointer,
+ .pointer64 = &wm_adsp_compr_pointer64,
.copy = &wm_adsp_compr_copy,
};
diff --git a/sound/soc/codecs/cs47l92.c b/sound/soc/codecs/cs47l92.c
index 2c355c61acd8..0be338dabf25 100644
--- a/sound/soc/codecs/cs47l92.c
+++ b/sound/soc/codecs/cs47l92.c
@@ -1950,6 +1950,7 @@ static const struct snd_compress_ops cs47l92_compress_ops = {
.get_caps = &wm_adsp_compr_get_caps,
.trigger = &wm_adsp_compr_trigger,
.pointer = &wm_adsp_compr_pointer,
+ .pointer64 = &wm_adsp_compr_pointer64,
.copy = &wm_adsp_compr_copy,
};
diff --git a/sound/soc/codecs/wm5102.c b/sound/soc/codecs/wm5102.c
index 9fc7a8325724..3f691f2f3269 100644
--- a/sound/soc/codecs/wm5102.c
+++ b/sound/soc/codecs/wm5102.c
@@ -2013,6 +2013,7 @@ static const struct snd_compress_ops wm5102_compress_ops = {
.get_caps = wm_adsp_compr_get_caps,
.trigger = wm_adsp_compr_trigger,
.pointer = wm_adsp_compr_pointer,
+ .pointer64 = wm_adsp_compr_pointer64,
.copy = wm_adsp_compr_copy,
};
diff --git a/sound/soc/codecs/wm5110.c b/sound/soc/codecs/wm5110.c
index 212eca675f27..8470ade90bad 100644
--- a/sound/soc/codecs/wm5110.c
+++ b/sound/soc/codecs/wm5110.c
@@ -2370,6 +2370,7 @@ static const struct snd_compress_ops wm5110_compress_ops = {
.get_caps = wm_adsp_compr_get_caps,
.trigger = wm_adsp_compr_trigger,
.pointer = wm_adsp_compr_pointer,
+ .pointer64 = wm_adsp_compr_pointer64,
.copy = wm_adsp_compr_copy,
};
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 8a1d5cc75d6c..0c3cc08aa3a8 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -173,7 +173,7 @@ struct wm_adsp_compr {
struct snd_compressed_buffer size;
u32 *raw_buf;
- unsigned int copied_total;
+ u64 copied_total;
unsigned int sample_rate;
@@ -1858,17 +1858,16 @@ static int wm_adsp_buffer_reenable_irq(struct wm_adsp_compr_buf *buf)
buf->irq_count);
}
-int wm_adsp_compr_pointer(struct snd_soc_component *component,
- struct snd_compr_stream *stream,
- struct snd_compr_tstamp *tstamp)
+static int wm_adsp_compr_pointer_internal(struct snd_soc_component *component,
+ struct snd_compr_stream *stream,
+ struct snd_compr_tstamp *tstamp32,
+ struct snd_compr_tstamp64 *tstamp64)
{
struct wm_adsp_compr *compr = stream->runtime->private_data;
struct wm_adsp *dsp = compr->dsp;
struct wm_adsp_compr_buf *buf;
int ret = 0;
- compr_dbg(compr, "Pointer request\n");
-
mutex_lock(&dsp->cs_dsp.pwr_lock);
buf = compr->buf;
@@ -1908,17 +1907,53 @@ int wm_adsp_compr_pointer(struct snd_soc_component *component,
}
}
- tstamp->copied_total = compr->copied_total;
- tstamp->copied_total += buf->avail * CS_DSP_DATA_WORD_SIZE;
- tstamp->sampling_rate = compr->sample_rate;
+ if (tstamp32) {
+ tstamp32->copied_total = (u32)compr->copied_total;
+ tstamp32->copied_total += buf->avail * CS_DSP_DATA_WORD_SIZE;
+ tstamp32->sampling_rate = compr->sample_rate;
+ }
+ if (tstamp64) {
+ tstamp64->copied_total = compr->copied_total;
+ tstamp64->copied_total += buf->avail * CS_DSP_DATA_WORD_SIZE;
+ tstamp64->sampling_rate = compr->sample_rate;
+ }
out:
mutex_unlock(&dsp->cs_dsp.pwr_lock);
return ret;
}
+
+int wm_adsp_compr_pointer(struct snd_soc_component *component,
+ struct snd_compr_stream *stream,
+ struct snd_compr_tstamp *tstamp)
+{
+ struct wm_adsp_compr *compr = stream->runtime->private_data;
+ int ret = 0;
+
+ compr_dbg(compr, "Pointer request\n");
+
+ ret = wm_adsp_compr_pointer_internal(component, stream, tstamp, NULL);
+
+ return ret;
+}
EXPORT_SYMBOL_GPL(wm_adsp_compr_pointer);
+int wm_adsp_compr_pointer64(struct snd_soc_component *component,
+ struct snd_compr_stream *stream,
+ struct snd_compr_tstamp64 *tstamp)
+{
+ struct wm_adsp_compr *compr = stream->runtime->private_data;
+ int ret = 0;
+
+ compr_dbg(compr, "Pointer64 request\n");
+
+ ret = wm_adsp_compr_pointer_internal(component, stream, NULL, tstamp);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(wm_adsp_compr_pointer64);
+
static int wm_adsp_buffer_capture_block(struct wm_adsp_compr *compr, int target)
{
struct wm_adsp_compr_buf *buf = compr->buf;
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index 25210d404bf1..5027236bb92e 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -132,6 +132,9 @@ int wm_adsp_compr_handle_irq(struct wm_adsp *dsp);
int wm_adsp_compr_pointer(struct snd_soc_component *component,
struct snd_compr_stream *stream,
struct snd_compr_tstamp *tstamp);
+int wm_adsp_compr_pointer64(struct snd_soc_component *component,
+ struct snd_compr_stream *stream,
+ struct snd_compr_tstamp64 *tstamp);
int wm_adsp_compr_copy(struct snd_soc_component *component,
struct snd_compr_stream *stream,
char __user *buf, size_t count);
diff --git a/sound/soc/intel/atom/sst-mfld-platform-compress.c b/sound/soc/intel/atom/sst-mfld-platform-compress.c
index 89c9c5ad6b21..9fbd0d641059 100644
--- a/sound/soc/intel/atom/sst-mfld-platform-compress.c
+++ b/sound/soc/intel/atom/sst-mfld-platform-compress.c
@@ -210,7 +210,21 @@ static int sst_platform_compr_pointer(struct snd_soc_component *component,
stream->compr_ops->tstamp(sst->dev, stream->id, tstamp);
tstamp->byte_offset = tstamp->copied_total %
(u32)cstream->runtime->buffer_size;
- pr_debug("calc bytes offset/copied bytes as %d\n", tstamp->byte_offset);
+ pr_debug("calc bytes offset/copied bytes as %u\n", tstamp->byte_offset);
+ return 0;
+}
+
+static int sst_platform_compr_pointer64(struct snd_soc_component *component,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp64 *tstamp)
+{
+ struct sst_runtime_stream *stream;
+
+ stream = cstream->runtime->private_data;
+ stream->compr_ops->tstamp64(sst->dev, stream->id, tstamp);
+ tstamp->byte_offset =
+ tstamp->copied_total % cstream->runtime->buffer_size;
+ pr_debug("calc bytes offset/copied bytes as %u\n", tstamp->byte_offset);
return 0;
}
@@ -265,6 +279,7 @@ const struct snd_compress_ops sst_platform_compress_ops = {
.set_metadata = sst_platform_compr_set_metadata,
.trigger = sst_platform_compr_trigger,
.pointer = sst_platform_compr_pointer,
+ .pointer64 = sst_platform_compr_pointer64,
.ack = sst_platform_compr_ack,
.get_caps = sst_platform_compr_get_caps,
.get_codec_caps = sst_platform_compr_get_codec_caps,
diff --git a/sound/soc/intel/atom/sst-mfld-platform.h b/sound/soc/intel/atom/sst-mfld-platform.h
index 8b5777d3229a..bc21a36a4cc0 100644
--- a/sound/soc/intel/atom/sst-mfld-platform.h
+++ b/sound/soc/intel/atom/sst-mfld-platform.h
@@ -106,6 +106,8 @@ struct compress_sst_ops {
int (*tstamp)(struct device *dev, unsigned int str_id,
struct snd_compr_tstamp *tstamp);
+ int (*tstamp64)(struct device *dev, unsigned int str_id,
+ struct snd_compr_tstamp64 *tstamp);
int (*ack)(struct device *dev, unsigned int str_id,
unsigned long bytes);
int (*close)(struct device *dev, unsigned int str_id);
diff --git a/sound/soc/intel/atom/sst/sst_drv_interface.c b/sound/soc/intel/atom/sst/sst_drv_interface.c
index 8bb27f86eb65..f6472ad29e47 100644
--- a/sound/soc/intel/atom/sst/sst_drv_interface.c
+++ b/sound/soc/intel/atom/sst/sst_drv_interface.c
@@ -325,8 +325,8 @@ static int sst_cdev_stream_partial_drain(struct device *dev,
return sst_drain_stream(ctx, str_id, true);
}
-static int sst_cdev_tstamp(struct device *dev, unsigned int str_id,
- struct snd_compr_tstamp *tstamp)
+static int sst_cdev_tstamp32(struct device *dev, unsigned int str_id,
+ struct snd_compr_tstamp *tstamp)
{
struct snd_sst_tstamp fw_tstamp = {0,};
struct stream_info *stream;
@@ -350,9 +350,41 @@ static int sst_cdev_tstamp(struct device *dev, unsigned int str_id,
tstamp->sampling_rate = fw_tstamp.sampling_frequency;
dev_dbg(dev, "PCM = %u\n", tstamp->pcm_io_frames);
- dev_dbg(dev, "Ptr Query on strid = %d copied_total %d, decodec %d\n",
+ dev_dbg(dev, "Ptr Query on strid = %d copied_total %u, decodec %u\n",
str_id, tstamp->copied_total, tstamp->pcm_frames);
- dev_dbg(dev, "rendered %d\n", tstamp->pcm_io_frames);
+ dev_dbg(dev, "rendered %u\n", tstamp->pcm_io_frames);
+
+ return 0;
+}
+
+static int sst_cdev_tstamp64(struct device *dev, unsigned int str_id,
+ struct snd_compr_tstamp64 *tstamp)
+{
+ struct snd_sst_tstamp fw_tstamp = {0,};
+ struct stream_info *stream;
+ struct intel_sst_drv *ctx = dev_get_drvdata(dev);
+ void __iomem *addr;
+
+ addr = (void __iomem *)(ctx->mailbox + ctx->tstamp) +
+ (str_id * sizeof(fw_tstamp));
+
+ memcpy_fromio(&fw_tstamp, addr, sizeof(fw_tstamp));
+
+ stream = get_stream_info(ctx, str_id);
+ if (!stream)
+ return -EINVAL;
+ dev_dbg(dev, "rb_counter %llu in bytes\n", fw_tstamp.ring_buffer_counter);
+
+ tstamp->copied_total = fw_tstamp.ring_buffer_counter;
+ tstamp->pcm_frames = fw_tstamp.frames_decoded;
+ tstamp->pcm_io_frames = div_u64(fw_tstamp.hardware_counter,
+ (u64)stream->num_ch * SST_GET_BYTES_PER_SAMPLE(24));
+ tstamp->sampling_rate = fw_tstamp.sampling_frequency;
+
+ dev_dbg(dev, "PCM = %llu\n", tstamp->pcm_io_frames);
+ dev_dbg(dev, "Ptr Query on strid = %d copied_total %llu, decodec %llu\n",
+ str_id, tstamp->copied_total, tstamp->pcm_frames);
+ dev_dbg(dev, "rendered %llu\n", tstamp->pcm_io_frames);
return 0;
}
@@ -650,7 +682,8 @@ static struct compress_sst_ops compr_ops = {
.stream_drop = sst_cdev_stream_drop,
.stream_drain = sst_cdev_stream_drain,
.stream_partial_drain = sst_cdev_stream_partial_drain,
- .tstamp = sst_cdev_tstamp,
+ .tstamp = sst_cdev_tstamp32,
+ .tstamp64 = sst_cdev_tstamp64,
.ack = sst_cdev_ack,
.get_caps = sst_cdev_caps,
.get_codec_caps = sst_cdev_codec_caps,
diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c
index a400c9a31fea..d9917eca5f80 100644
--- a/sound/soc/qcom/qdsp6/q6asm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6asm-dai.c
@@ -59,9 +59,9 @@ struct q6asm_dai_rtd {
unsigned int pcm_count;
unsigned int pcm_irq_pos; /* IRQ position */
unsigned int periods;
- unsigned int bytes_sent;
- unsigned int bytes_received;
- unsigned int copied_total;
+ uint64_t bytes_sent;
+ uint64_t bytes_received;
+ uint64_t copied_total;
uint16_t bits_per_sample;
uint16_t source; /* Encoding source bit mask */
struct audio_client *audio_client;
@@ -1024,9 +1024,27 @@ static int q6asm_dai_compr_trigger(struct snd_soc_component *component,
return ret;
}
-static int q6asm_dai_compr_pointer(struct snd_soc_component *component,
- struct snd_compr_stream *stream,
- struct snd_compr_tstamp *tstamp)
+static int q6asm_dai_compr_pointer32(struct snd_soc_component *component,
+ struct snd_compr_stream *stream,
+ struct snd_compr_tstamp *tstamp)
+{
+ struct snd_compr_runtime *runtime = stream->runtime;
+ struct q6asm_dai_rtd *prtd = runtime->private_data;
+ unsigned long flags;
+
+ spin_lock_irqsave(&prtd->lock, flags);
+
+ tstamp->copied_total = (u32) prtd->copied_total;
+ tstamp->byte_offset = prtd->copied_total % prtd->pcm_size;
+
+ spin_unlock_irqrestore(&prtd->lock, flags);
+
+ return 0;
+}
+
+static int q6asm_dai_compr_pointer64(struct snd_soc_component *component,
+ struct snd_compr_stream *stream,
+ struct snd_compr_tstamp64 *tstamp)
{
struct snd_compr_runtime *runtime = stream->runtime;
struct q6asm_dai_rtd *prtd = runtime->private_data;
@@ -1050,11 +1068,12 @@ static int q6asm_compr_copy(struct snd_soc_component *component,
struct q6asm_dai_rtd *prtd = runtime->private_data;
unsigned long flags;
u32 wflags = 0;
- int avail, bytes_in_flight = 0;
+ u64 avail = 0;
+ u64 bytes_in_flight = 0;
void *dstn;
size_t copy;
u32 app_pointer;
- u32 bytes_received;
+ u64 bytes_received;
bytes_received = prtd->bytes_received;
@@ -1065,8 +1084,7 @@ static int q6asm_compr_copy(struct snd_soc_component *component,
if (prtd->next_track)
bytes_received = ALIGN(prtd->bytes_received, prtd->pcm_count);
- app_pointer = bytes_received/prtd->pcm_size;
- app_pointer = bytes_received - (app_pointer * prtd->pcm_size);
+ app_pointer = bytes_received % prtd->pcm_size;
dstn = prtd->dma_buffer.area + app_pointer;
if (count < prtd->pcm_size - app_pointer) {
@@ -1164,7 +1182,8 @@ static const struct snd_compress_ops q6asm_dai_compress_ops = {
.free = q6asm_dai_compr_free,
.set_params = q6asm_dai_compr_set_params,
.set_metadata = q6asm_dai_compr_set_metadata,
- .pointer = q6asm_dai_compr_pointer,
+ .pointer = q6asm_dai_compr_pointer32,
+ .pointer64 = q6asm_dai_compr_pointer64,
.trigger = q6asm_dai_compr_trigger,
.get_caps = q6asm_dai_compr_get_caps,
.get_codec_caps = q6asm_dai_compr_get_codec_caps,
diff --git a/sound/soc/sof/compress.c b/sound/soc/sof/compress.c
index d7b044f33d79..6eb0ceccde37 100644
--- a/sound/soc/sof/compress.c
+++ b/sound/soc/sof/compress.c
@@ -359,33 +359,59 @@ static int sof_compr_copy(struct snd_soc_component *component,
return sof_compr_copy_capture(rtd, buf, count);
}
-static int sof_compr_pointer(struct snd_soc_component *component,
- struct snd_compr_stream *cstream,
- struct snd_compr_tstamp *tstamp)
+static int sof_compr_pointer_internal(struct snd_soc_component *component,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp *tstamp32,
+ struct snd_compr_tstamp64 *tstamp64)
{
struct snd_sof_pcm *spcm;
struct snd_soc_pcm_runtime *rtd = cstream->private_data;
struct sof_compr_stream *sstream = cstream->runtime->private_data;
+ u64 pcm_io_frames;
spcm = snd_sof_find_spcm_dai(component, rtd);
if (!spcm)
return -EINVAL;
-
- tstamp->sampling_rate = sstream->sampling_rate;
- tstamp->copied_total = sstream->copied_total;
- tstamp->pcm_io_frames = div_u64(spcm->stream[cstream->direction].posn.dai_posn,
- sstream->channels * sstream->sample_container_bytes);
+ pcm_io_frames =
+ div_u64(spcm->stream[cstream->direction].posn.dai_posn,
+ sstream->channels * sstream->sample_container_bytes);
+
+ if (tstamp32) {
+ tstamp32->sampling_rate = sstream->sampling_rate;
+ tstamp32->copied_total = (u32)sstream->copied_total;
+ tstamp32->pcm_io_frames = (u32)pcm_io_frames;
+ }
+ if (tstamp64) {
+ tstamp64->sampling_rate = sstream->sampling_rate;
+ tstamp64->copied_total = sstream->copied_total;
+ tstamp64->pcm_io_frames = pcm_io_frames;
+ }
return 0;
}
+static int sof_compr_pointer32(struct snd_soc_component *component,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp *tstamp)
+{
+ return sof_compr_pointer_internal(component, cstream, tstamp, NULL);
+}
+
+static int sof_compr_pointer64(struct snd_soc_component *component,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp64 *tstamp)
+{
+ return sof_compr_pointer_internal(component, cstream, NULL, tstamp);
+}
+
struct snd_compress_ops sof_compressed_ops = {
.open = sof_compr_open,
.free = sof_compr_free,
.set_params = sof_compr_set_params,
.get_params = sof_compr_get_params,
.trigger = sof_compr_trigger,
- .pointer = sof_compr_pointer,
+ .pointer = sof_compr_pointer32,
+ .pointer64 = sof_compr_pointer64,
.copy = sof_compr_copy,
};
EXPORT_SYMBOL(sof_compressed_ops);
diff --git a/sound/soc/sprd/sprd-pcm-compress.c b/sound/soc/sprd/sprd-pcm-compress.c
index 57bd1a0728ac..3dea3b00af77 100644
--- a/sound/soc/sprd/sprd-pcm-compress.c
+++ b/sound/soc/sprd/sprd-pcm-compress.c
@@ -85,9 +85,9 @@ struct sprd_compr_stream {
int info_size;
/* Data size copied to IRAM buffer */
- int copied_total;
+ u64 copied_total;
/* Total received data size from userspace */
- int received_total;
+ u64 received_total;
/* Stage 0 IRAM buffer received data size */
int received_stage0;
/* Stage 1 DDR buffer received data size */
@@ -511,9 +511,24 @@ static int sprd_platform_compr_trigger(struct snd_soc_component *component,
return ret;
}
-static int sprd_platform_compr_pointer(struct snd_soc_component *component,
- struct snd_compr_stream *cstream,
- struct snd_compr_tstamp *tstamp)
+static int sprd_platform_compr_pointer32(struct snd_soc_component *component,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp *tstamp)
+{
+ struct snd_compr_runtime *runtime = cstream->runtime;
+ struct sprd_compr_stream *stream = runtime->private_data;
+ struct sprd_compr_playinfo *info =
+ (struct sprd_compr_playinfo *)stream->info_area;
+
+ tstamp->copied_total = (u32)stream->copied_total;
+ tstamp->pcm_io_frames = (u32)info->current_data_offset;
+
+ return 0;
+}
+
+static int sprd_platform_compr_pointer64(struct snd_soc_component *component,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp64 *tstamp)
{
struct snd_compr_runtime *runtime = cstream->runtime;
struct sprd_compr_stream *stream = runtime->private_data;
@@ -660,7 +675,8 @@ const struct snd_compress_ops sprd_platform_compress_ops = {
.free = sprd_platform_compr_free,
.set_params = sprd_platform_compr_set_params,
.trigger = sprd_platform_compr_trigger,
- .pointer = sprd_platform_compr_pointer,
+ .pointer = sprd_platform_compr_pointer32,
+ .pointer64 = sprd_platform_compr_pointer64,
.copy = sprd_platform_compr_copy,
.get_caps = sprd_platform_compr_get_caps,
.get_codec_caps = sprd_platform_compr_get_codec_caps,
diff --git a/sound/soc/sprd/sprd-pcm-dma.h b/sound/soc/sprd/sprd-pcm-dma.h
index be5e385f5e42..31866a5df84b 100644
--- a/sound/soc/sprd/sprd-pcm-dma.h
+++ b/sound/soc/sprd/sprd-pcm-dma.h
@@ -46,7 +46,7 @@ struct sprd_compr_ops {
int (*stop)(int str_id);
int (*pause)(int str_id);
int (*pause_release)(int str_id);
- int (*drain)(int received_total);
+ int (*drain)(u64 received_total);
int (*set_params)(int str_id, struct sprd_compr_params *params);
};
diff --git a/sound/soc/uniphier/aio-compress.c b/sound/soc/uniphier/aio-compress.c
index 4a19d4908ffd..d450f5432d02 100644
--- a/sound/soc/uniphier/aio-compress.c
+++ b/sound/soc/uniphier/aio-compress.c
@@ -247,9 +247,9 @@ static int uniphier_aio_compr_trigger(struct snd_soc_component *component,
return ret;
}
-static int uniphier_aio_compr_pointer(struct snd_soc_component *component,
- struct snd_compr_stream *cstream,
- struct snd_compr_tstamp *tstamp)
+static int uniphier_aio_compr_pointer_internal(
+ struct snd_soc_component *component, struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp *tstamp32, struct snd_compr_tstamp64 *tstamp64)
{
struct snd_soc_pcm_runtime *rtd = cstream->private_data;
struct snd_compr_runtime *runtime = cstream->runtime;
@@ -258,6 +258,7 @@ static int uniphier_aio_compr_pointer(struct snd_soc_component *component,
int bytes = runtime->fragment_size;
unsigned long flags;
u32 pos;
+ u64 copied_total;
spin_lock_irqsave(&sub->lock, flags);
@@ -266,18 +267,42 @@ static int uniphier_aio_compr_pointer(struct snd_soc_component *component,
if (sub->swm->dir == PORT_DIR_OUTPUT) {
pos = sub->rd_offs;
/* Size of AIO output format is double of IEC61937 */
- tstamp->copied_total = sub->rd_total / 2;
+ copied_total = sub->rd_total / 2;
} else {
pos = sub->wr_offs;
- tstamp->copied_total = sub->rd_total;
+ copied_total = sub->rd_total;
+ }
+
+ if (tstamp32) {
+ tstamp32->copied_total = (u32)copied_total;
+ tstamp32->byte_offset = pos;
+ }
+ if (tstamp64) {
+ tstamp64->copied_total = copied_total;
+ tstamp64->byte_offset = pos;
}
- tstamp->byte_offset = pos;
spin_unlock_irqrestore(&sub->lock, flags);
return 0;
}
+static int uniphier_aio_compr_pointer32(struct snd_soc_component *component,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp *tstamp)
+{
+ return uniphier_aio_compr_pointer_internal(component, cstream, tstamp,
+ NULL);
+}
+
+static int uniphier_aio_compr_pointer64(struct snd_soc_component *component,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp64 *tstamp)
+{
+ return uniphier_aio_compr_pointer_internal(component, cstream, NULL,
+ tstamp);
+}
+
static int aio_compr_send_to_hw(struct uniphier_aio_sub *sub,
char __user *buf, size_t dstsize)
{
@@ -426,7 +451,8 @@ const struct snd_compress_ops uniphier_aio_compress_ops = {
.get_params = uniphier_aio_compr_get_params,
.set_params = uniphier_aio_compr_set_params,
.trigger = uniphier_aio_compr_trigger,
- .pointer = uniphier_aio_compr_pointer,
+ .pointer = uniphier_aio_compr_pointer32,
+ .pointer64 = uniphier_aio_compr_pointer64,
.copy = uniphier_aio_compr_copy,
.get_caps = uniphier_aio_compr_get_caps,
.get_codec_caps = uniphier_aio_compr_get_codec_caps,
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 4/4] ASoC: codecs: Implement 64-bit pointer operation
2025-07-11 9:36 ` [PATCH v2 4/4] ASoC: codecs: Implement 64-bit pointer operation Joris Verhaegen
@ 2025-07-11 10:10 ` Richard Fitzgerald
2025-07-15 13:07 ` kernel test robot
1 sibling, 0 replies; 12+ messages in thread
From: Richard Fitzgerald @ 2025-07-11 10:10 UTC (permalink / raw)
To: Joris Verhaegen, Vinod Koul, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Charles Keepax, 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: kernel-team, linux-sound, linux-kernel, patches, linux-arm-msm,
sound-open-firmware, linux-arm-kernel, David Li, Miller Liang
On 11/7/25 10:36, Joris Verhaegen wrote:
> Implement the new .pointer64 compress operation for the various ASoC
> drivers that support compress offload.
>
> For drivers with complex but similar 32-bit and 64-bit logic
> (wm_adsp, sof, uniphier), a shared internal function is used to handle
> both requests, minimizing code duplication and improving maintainability.
>
> For other drivers, internal counters and related variables are updated
> to u64 to ensure the source of the timestamp data is overflow-safe.
> The legacy .pointer operation is retained in all drivers to ensure
> full backward compatibility.
>
> Signed-off-by: Joris Verhaegen <verhaegen@google.com>
> Tested-by: Joris Verhaegen <verhaegen@google.com>
> Reviewed-by: David Li <dvdli@google.com>
> Reviewed-by: Miller Liang <millerliang@google.com>
> ---
> sound/soc/codecs/cs47l15.c | 1 +
> sound/soc/codecs/cs47l24.c | 1 +
> sound/soc/codecs/cs47l35.c | 1 +
> sound/soc/codecs/cs47l85.c | 1 +
> sound/soc/codecs/cs47l90.c | 1 +
> sound/soc/codecs/cs47l92.c | 1 +
> sound/soc/codecs/wm5102.c | 1 +
> sound/soc/codecs/wm5110.c | 1 +
> sound/soc/codecs/wm_adsp.c | 53 +++++++++++++++----
> sound/soc/codecs/wm_adsp.h | 3 ++
For these 10 files:
Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] ASoC: codecs: Implement 64-bit pointer operation
2025-07-11 9:36 ` [PATCH v2 4/4] ASoC: codecs: Implement 64-bit pointer operation Joris Verhaegen
2025-07-11 10:10 ` Richard Fitzgerald
@ 2025-07-15 13:07 ` kernel test robot
1 sibling, 0 replies; 12+ messages in thread
From: kernel test robot @ 2025-07-15 13:07 UTC (permalink / raw)
To: Joris Verhaegen, 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: oe-kbuild-all, Joris Verhaegen, kernel-team, linux-sound,
linux-kernel, patches, linux-arm-msm, sound-open-firmware,
linux-arm-kernel, David Li
Hi Joris,
kernel test robot noticed the following build errors:
[auto build test ERROR on broonie-sound/for-next]
[also build test ERROR on vkoul-dmaengine/next linus/master v6.16-rc6]
[cannot apply to tiwai-sound/for-next tiwai-sound/for-linus next-20250714]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Joris-Verhaegen/ALSA-compress_offload-Add-64-bit-safe-timestamp-infrastructure/20250711-174008
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
patch link: https://lore.kernel.org/r/20250711093636.28204-5-verhaegen%40google.com
patch subject: [PATCH v2 4/4] ASoC: codecs: Implement 64-bit pointer operation
config: i386-randconfig-141-20250714 (https://download.01.org/0day-ci/archive/20250715/202507152054.EKID8Uop-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250715/202507152054.EKID8Uop-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507152054.EKID8Uop-lkp@intel.com/
All errors (new ones prefixed by >>):
ld: sound/soc/intel/atom/sst-mfld-platform-compress.o: in function `sst_platform_compr_pointer64':
>> sound/soc/intel/atom/sst-mfld-platform-compress.c:226: undefined reference to `__umoddi3'
vim +226 sound/soc/intel/atom/sst-mfld-platform-compress.c
216
217 static int sst_platform_compr_pointer64(struct snd_soc_component *component,
218 struct snd_compr_stream *cstream,
219 struct snd_compr_tstamp64 *tstamp)
220 {
221 struct sst_runtime_stream *stream;
222
223 stream = cstream->runtime->private_data;
224 stream->compr_ops->tstamp64(sst->dev, stream->id, tstamp);
225 tstamp->byte_offset =
> 226 tstamp->copied_total % cstream->runtime->buffer_size;
227 pr_debug("calc bytes offset/copied bytes as %u\n", tstamp->byte_offset);
228 return 0;
229 }
230
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/4] ALSA: compress_offload: Add 64-bit safe timestamp API
2025-07-11 9:36 [PATCH v2 0/4] ALSA: compress_offload: Add 64-bit safe timestamp API Joris Verhaegen
` (3 preceding siblings ...)
2025-07-11 9:36 ` [PATCH v2 4/4] ASoC: codecs: Implement 64-bit pointer operation Joris Verhaegen
@ 2025-07-11 11:56 ` Charles Keepax
2025-07-11 12:41 ` Takashi Iwai
4 siblings, 1 reply; 12+ messages in thread
From: Charles Keepax @ 2025-07-11 11:56 UTC (permalink / raw)
To: Joris Verhaegen
Cc: Vinod Koul, Jaroslav Kysela, Takashi Iwai, Liam Girdwood,
Mark Brown, 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, kernel-team, linux-sound, linux-kernel, patches,
linux-arm-msm, sound-open-firmware, linux-arm-kernel
On Fri, Jul 11, 2025 at 10:36:26AM +0100, Joris Verhaegen wrote:
> The current compress offload timestamping API relies on
> struct snd_compr_tstamp, whose cumulative counters like
> copied_total are defined as __u32. On long-running high-resolution
> audio streams, these 32-bit counters can overflow,
> causing incorrect availability calculations.
>
> This patch series introduces a parallel, 64-bit safe API to solve
> this problem while maintaining perfect backward compatibility with the
> existing UAPI. A new pointer64 operation and corresponding ioctls
> are added to allow the kernel to track counters using u64 and expose
> these full-width values to user-space.
>
> The series is structured as follows:
>
> Patch 1: Introduces the new internal pointer64 op, refactors the
> core logic to use it, and defines the new UAPI structs.
>
> Patch 2: Exposes the SNDRV_COMPRESS_TSTAMP64 ioctl.
>
> Patch 3: Exposes the corresponding SNDRV_COMPRESS_AVAIL64 ioctl.
>
> Patch 4: Implements the new .pointer64 operation in various ASoC
> drivers that support compress offload.
>
> This series has been tested on a Pixel 9 device. All compress offload
> use cases, including long-running playback, were verified to work
> correctly with the new 64-bit API, and no regressions were observed
> when using the legacy API.
>
> Thanks,
> Joris (George) Verhaegen
>
> Signed-off-by: Joris Verhaegen <verhaegen@google.com>
>
> ---
Would it not be slightly simpler to just update all the in kernel
bits to use 64-bit and then only convert to 32-bit for the
existing 32-bit IOCTLs? Why do we need 32-bit callbacks into the
drivers for example?
Thanks,
Charles
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 0/4] ALSA: compress_offload: Add 64-bit safe timestamp API
2025-07-11 11:56 ` [PATCH v2 0/4] ALSA: compress_offload: Add 64-bit safe timestamp API Charles Keepax
@ 2025-07-11 12:41 ` Takashi Iwai
2025-07-11 13:00 ` Takashi Iwai
2025-07-15 12:39 ` Vinod Koul
0 siblings, 2 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-07-11 12:41 UTC (permalink / raw)
To: Charles Keepax
Cc: Joris Verhaegen, Vinod Koul, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, 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, kernel-team, linux-sound,
linux-kernel, patches, linux-arm-msm, sound-open-firmware,
linux-arm-kernel
On Fri, 11 Jul 2025 13:56:47 +0200,
Charles Keepax wrote:
>
> On Fri, Jul 11, 2025 at 10:36:26AM +0100, Joris Verhaegen wrote:
> > The current compress offload timestamping API relies on
> > struct snd_compr_tstamp, whose cumulative counters like
> > copied_total are defined as __u32. On long-running high-resolution
> > audio streams, these 32-bit counters can overflow,
> > causing incorrect availability calculations.
> >
> > This patch series introduces a parallel, 64-bit safe API to solve
> > this problem while maintaining perfect backward compatibility with the
> > existing UAPI. A new pointer64 operation and corresponding ioctls
> > are added to allow the kernel to track counters using u64 and expose
> > these full-width values to user-space.
> >
> > The series is structured as follows:
> >
> > Patch 1: Introduces the new internal pointer64 op, refactors the
> > core logic to use it, and defines the new UAPI structs.
> >
> > Patch 2: Exposes the SNDRV_COMPRESS_TSTAMP64 ioctl.
> >
> > Patch 3: Exposes the corresponding SNDRV_COMPRESS_AVAIL64 ioctl.
> >
> > Patch 4: Implements the new .pointer64 operation in various ASoC
> > drivers that support compress offload.
> >
> > This series has been tested on a Pixel 9 device. All compress offload
> > use cases, including long-running playback, were verified to work
> > correctly with the new 64-bit API, and no regressions were observed
> > when using the legacy API.
> >
> > Thanks,
> > Joris (George) Verhaegen
> >
> > Signed-off-by: Joris Verhaegen <verhaegen@google.com>
> >
> > ---
>
> Would it not be slightly simpler to just update all the in kernel
> bits to use 64-bit and then only convert to 32-bit for the
> existing 32-bit IOCTLs? Why do we need 32-bit callbacks into the
> drivers for example?
Right, it's a usual pattern to have only the 64bit ops in the kernel
driver side while providing the 32bit stuff converted in the core
layer. Having two different ops are rather confusing and
superfluous after conversions.
If there are tons of users for this API, it'd be needed to convert
gradually, and eventually drop the 32bit ops at the end. But in this
case, there doesn't seem so many relevant drivers, hence the
conversion can be done in a shot as done in your patch 4.
thanks,
Takashi
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/4] ALSA: compress_offload: Add 64-bit safe timestamp API
2025-07-11 12:41 ` Takashi Iwai
@ 2025-07-11 13:00 ` Takashi Iwai
2025-07-15 12:39 ` Vinod Koul
1 sibling, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-07-11 13:00 UTC (permalink / raw)
To: Joris Verhaegen
Cc: Charles Keepax, Vinod Koul, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, 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, kernel-team, linux-sound,
linux-kernel, patches, linux-arm-msm, sound-open-firmware,
linux-arm-kernel
On Fri, 11 Jul 2025 14:41:05 +0200,
Takashi Iwai wrote:
>
> On Fri, 11 Jul 2025 13:56:47 +0200,
> Charles Keepax wrote:
> >
> > On Fri, Jul 11, 2025 at 10:36:26AM +0100, Joris Verhaegen wrote:
> > > The current compress offload timestamping API relies on
> > > struct snd_compr_tstamp, whose cumulative counters like
> > > copied_total are defined as __u32. On long-running high-resolution
> > > audio streams, these 32-bit counters can overflow,
> > > causing incorrect availability calculations.
> > >
> > > This patch series introduces a parallel, 64-bit safe API to solve
> > > this problem while maintaining perfect backward compatibility with the
> > > existing UAPI. A new pointer64 operation and corresponding ioctls
> > > are added to allow the kernel to track counters using u64 and expose
> > > these full-width values to user-space.
> > >
> > > The series is structured as follows:
> > >
> > > Patch 1: Introduces the new internal pointer64 op, refactors the
> > > core logic to use it, and defines the new UAPI structs.
> > >
> > > Patch 2: Exposes the SNDRV_COMPRESS_TSTAMP64 ioctl.
> > >
> > > Patch 3: Exposes the corresponding SNDRV_COMPRESS_AVAIL64 ioctl.
> > >
> > > Patch 4: Implements the new .pointer64 operation in various ASoC
> > > drivers that support compress offload.
> > >
> > > This series has been tested on a Pixel 9 device. All compress offload
> > > use cases, including long-running playback, were verified to work
> > > correctly with the new 64-bit API, and no regressions were observed
> > > when using the legacy API.
> > >
> > > Thanks,
> > > Joris (George) Verhaegen
> > >
> > > Signed-off-by: Joris Verhaegen <verhaegen@google.com>
> > >
> > > ---
> >
> > Would it not be slightly simpler to just update all the in kernel
> > bits to use 64-bit and then only convert to 32-bit for the
> > existing 32-bit IOCTLs? Why do we need 32-bit callbacks into the
> > drivers for example?
>
> Right, it's a usual pattern to have only the 64bit ops in the kernel
> driver side while providing the 32bit stuff converted in the core
> layer. Having two different ops are rather confusing and
> superfluous after conversions.
>
> If there are tons of users for this API, it'd be needed to convert
> gradually, and eventually drop the 32bit ops at the end. But in this
> case, there doesn't seem so many relevant drivers, hence the
> conversion can be done in a shot as done in your patch 4.
Also, don't forget to increase the protocol version if you change the
ABI.
thanks,
Takashi
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/4] ALSA: compress_offload: Add 64-bit safe timestamp API
2025-07-11 12:41 ` Takashi Iwai
2025-07-11 13:00 ` Takashi Iwai
@ 2025-07-15 12:39 ` Vinod Koul
1 sibling, 0 replies; 12+ messages in thread
From: Vinod Koul @ 2025-07-15 12:39 UTC (permalink / raw)
To: Takashi Iwai
Cc: Charles Keepax, Joris Verhaegen, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, 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, kernel-team, linux-sound,
linux-kernel, patches, linux-arm-msm, sound-open-firmware,
linux-arm-kernel
On 11-07-25, 14:41, Takashi Iwai wrote:
> On Fri, 11 Jul 2025 13:56:47 +0200,
> > Would it not be slightly simpler to just update all the in kernel
> > bits to use 64-bit and then only convert to 32-bit for the
> > existing 32-bit IOCTLs? Why do we need 32-bit callbacks into the
> > drivers for example?
>
> Right, it's a usual pattern to have only the 64bit ops in the kernel
> driver side while providing the 32bit stuff converted in the core
> layer. Having two different ops are rather confusing and
> superfluous after conversions.
>
> If there are tons of users for this API, it'd be needed to convert
> gradually, and eventually drop the 32bit ops at the end. But in this
> case, there doesn't seem so many relevant drivers, hence the
> conversion can be done in a shot as done in your patch 4.
I agree we should do that. Kernel can be 64bit only while we keep
maintaining the 32bit ioctls, cant drop that one
--
~Vinod
^ permalink raw reply [flat|nested] 12+ messages in thread