[PATCH v4 0/3] ALSA: compress_offload: Add 64-bit safe timestamp API

[PATCH v4 0/3] ALSA: compress_offload: Add 64-bit safe timestamp API

[Joris Verhaegen]

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 transitions to a 64-bit safe API to solve the problem
while maintaining perfect backward compatibility with the existing UAPI.
The pointer operation is reworked to use a new timestamp struct with
64-bit fields for the cumulative counters, named snd_compr_tstamp64.
ASoC drivers are updated to use the 64-bit structures. Corresponding
ioctls are added to expose them to user-space.

The series is structured as follows:

Patch 1: Updates the pointer op, refactors the core logic and ASoC
drivers 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.

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.

Thanks,
Joris (George) Verhaegen

Signed-off-by: Joris Verhaegen <verhaegen@google.com>

---
Changes in v2:
  - Corrected author and Signed-off-by to be consistent (Mark Brown).
Changes in v3:
  - Rework pointer op to return 64-bit timestamp, rather than adding a
    parallel pointer64 op (Charles Keepax, Takashi Iwai, Vinod Koul)
  - Bump the protocol version for ABI change (Takashi Iwai)
  -Fix linker error on Intel Atom (lkp@intel.com)
  -Rebase on latest for-next sound tree (Takashi Iwai)
  -Avoid changing return error code for ioctl (internal)
  -ASoC: Replace u64 % u32 by do_div(u64, u32) (internal)
  -ASoC: sprd: change current_data_offset to u64 (internal)
Changes in v4:
  -Fix compiler error on Intel AVS (lkp@intel.com)

Joris Verhaegen (3):
  ALSA: compress_offload: Add 64-bit safe timestamp infrastructure
  ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl
  ALSA: compress_offload: Add SNDRV_COMPRESS_AVAIL64 ioctl

 include/sound/compress_driver.h               |   2 +-
 include/sound/soc-component.h                 |   4 +-
 include/sound/soc-dai.h                       |   7 +-
 include/uapi/sound/compress_offload.h         |  35 +++++-
 sound/core/compress_offload.c                 | 100 ++++++++++++------
 sound/soc/codecs/wm_adsp.c                    |   4 +-
 sound/soc/codecs/wm_adsp.h                    |   2 +-
 .../intel/atom/sst-mfld-platform-compress.c   |  12 ++-
 sound/soc/intel/atom/sst-mfld-platform.h      |   2 +-
 sound/soc/intel/atom/sst/sst_drv_interface.c  |   9 +-
 sound/soc/intel/avs/probes.c                  |   2 +-
 sound/soc/qcom/qdsp6/q6apm-dai.c              |  26 +++--
 sound/soc/qcom/qdsp6/q6asm-dai.c              |  26 +++--
 sound/soc/soc-component.c                     |   2 +-
 sound/soc/soc-compress.c                      |   2 +-
 sound/soc/soc-dai.c                           |   2 +-
 sound/soc/sof/amd/acp-probes.c                |   2 +-
 sound/soc/sof/compress.c                      |   2 +-
 sound/soc/sof/intel/hda-probes.c              |   2 +-
 sound/soc/sof/sof-client-probes.c             |   2 +-
 sound/soc/sof/sof-client-probes.h             |   4 +-
 sound/soc/sprd/sprd-pcm-compress.c            |   6 +-
 sound/soc/sprd/sprd-pcm-dma.h                 |   4 +-
 sound/soc/uniphier/aio-compress.c             |   2 +-
 24 files changed, 173 insertions(+), 88 deletions(-)

-- 
2.50.1.565.gc32cd1483b-goog

[PATCH v4 1/3] ALSA: compress_offload: Add 64-bit safe timestamp infrastructure

[Joris Verhaegen]

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.
A new UAPI struct, snd_compr_tstamp64, is added which uses 64-bit
types for byte counters. The relevant ops structures across the
ASoC and core compress code are updated to use this new struct.
ASoC drivers are updated to use u64 counters.

Internal timestamps being u64 now, a compatibility function is added
to convert the 64-bit timestamp back to the 32-bit format for legacy
ioctl callers.

Reviewed-by: Miller Liang <millerliang@google.com>
Tested-by: Joris Verhaegen <verhaegen@google.com>
Signed-off-by: Joris Verhaegen <verhaegen@google.com>
---
 include/sound/compress_driver.h               |  2 +-
 include/sound/soc-component.h                 |  4 +-
 include/sound/soc-dai.h                       |  7 +--
 include/uapi/sound/compress_offload.h         | 19 +++++++
 sound/core/compress_offload.c                 | 52 +++++++++++++------
 sound/soc/codecs/wm_adsp.c                    |  4 +-
 sound/soc/codecs/wm_adsp.h                    |  2 +-
 .../intel/atom/sst-mfld-platform-compress.c   | 12 +++--
 sound/soc/intel/atom/sst-mfld-platform.h      |  2 +-
 sound/soc/intel/atom/sst/sst_drv_interface.c  |  9 ++--
 sound/soc/intel/avs/probes.c                  |  2 +-
 sound/soc/qcom/qdsp6/q6apm-dai.c              | 26 ++++++----
 sound/soc/qcom/qdsp6/q6asm-dai.c              | 26 ++++++----
 sound/soc/soc-component.c                     |  2 +-
 sound/soc/soc-compress.c                      |  2 +-
 sound/soc/soc-dai.c                           |  2 +-
 sound/soc/sof/amd/acp-probes.c                |  2 +-
 sound/soc/sof/compress.c                      |  2 +-
 sound/soc/sof/intel/hda-probes.c              |  2 +-
 sound/soc/sof/sof-client-probes.c             |  2 +-
 sound/soc/sof/sof-client-probes.h             |  4 +-
 sound/soc/sprd/sprd-pcm-compress.c            |  6 +--
 sound/soc/sprd/sprd-pcm-dma.h                 |  4 +-
 sound/soc/uniphier/aio-compress.c             |  2 +-
 24 files changed, 125 insertions(+), 72 deletions(-)

diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
index b55c9eeb2b54..9e3d801e45ec 100644
--- a/include/sound/compress_driver.h
+++ b/include/sound/compress_driver.h
@@ -161,7 +161,7 @@ struct snd_compr_ops {
 			struct snd_compr_metadata *metadata);
 	int (*trigger)(struct snd_compr_stream *stream, int cmd);
 	int (*pointer)(struct snd_compr_stream *stream,
-			struct snd_compr_tstamp *tstamp);
+		       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..754627c6361a 100644
--- a/include/sound/soc-component.h
+++ b/include/sound/soc-component.h
@@ -47,7 +47,7 @@ struct snd_compress_ops {
 		       struct snd_compr_stream *stream, int cmd);
 	int (*pointer)(struct snd_soc_component *component,
 		       struct snd_compr_stream *stream,
-		       struct snd_compr_tstamp *tstamp);
+		       struct snd_compr_tstamp64 *tstamp);
 	int (*copy)(struct snd_soc_component *component,
 		    struct snd_compr_stream *stream, char __user *buf,
 		    size_t count);
@@ -499,7 +499,7 @@ int snd_soc_component_compr_get_codec_caps(struct snd_compr_stream *cstream,
 					   struct snd_compr_codec_caps *codec);
 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);
+				    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..38ee6e158102 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -256,7 +256,7 @@ int snd_soc_dai_compr_ack(struct snd_soc_dai *dai,
 			  size_t bytes);
 int snd_soc_dai_compr_pointer(struct snd_soc_dai *dai,
 			      struct snd_compr_stream *cstream,
-			      struct snd_compr_tstamp *tstamp);
+			      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);
@@ -383,8 +383,9 @@ struct snd_soc_cdai_ops {
 			struct snd_compr_metadata *, struct snd_soc_dai *);
 	int (*trigger)(struct snd_compr_stream *, int,
 			struct snd_soc_dai *);
-	int (*pointer)(struct snd_compr_stream *,
-			struct snd_compr_tstamp *, struct snd_soc_dai *);
+	int (*pointer)(struct snd_compr_stream *stream,
+		       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 a66f258cafaa..d3164aa07158 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -176,14 +176,25 @@ static int snd_compr_free(struct inode *inode, struct file *f)
 	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)
+				   struct snd_compr_tstamp64 *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);
+	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
@@ -194,8 +205,11 @@ static int snd_compr_update_tstamp(struct snd_compr_stream *stream,
 static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
 		struct snd_compr_avail *avail)
 {
+	struct snd_compr_tstamp64 tstamp64 = { 0 };
+
 	memset(avail, 0, sizeof(*avail));
-	snd_compr_update_tstamp(stream, &avail->tstamp);
+	snd_compr_update_tstamp(stream, &tstamp64);
+	snd_compr_tstamp32_from_64(&avail->tstamp, &tstamp64);
 	/* Still need to return avail even if tstamp can't be filled in */
 
 	if (stream->runtime->total_bytes_available == 0 &&
@@ -204,9 +218,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 +237,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 +288,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 +331,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 +387,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 +456,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:
@@ -726,13 +739,18 @@ snd_compr_set_metadata(struct snd_compr_stream *stream, unsigned long arg)
 static inline int
 snd_compr_tstamp(struct snd_compr_stream *stream, unsigned long arg)
 {
-	struct snd_compr_tstamp tstamp = {0};
+	struct snd_compr_tstamp64 tstamp64 = { 0 };
+	struct snd_compr_tstamp tstamp32 = { 0 };
 	int ret;
 
-	ret = snd_compr_update_tstamp(stream, &tstamp);
-	if (ret == 0)
+	ret = snd_compr_update_tstamp(stream, &tstamp64);
+	if (ret == 0) {
+		snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
 		ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
-			&tstamp, sizeof(tstamp)) ? -EFAULT : 0;
+				   &tstamp32, sizeof(tstamp32)) ?
+			      -EFAULT :
+			      0;
+	}
 	return ret;
 }
 
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 8a1d5cc75d6c..f197034fd594 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;
 
@@ -1860,7 +1860,7 @@ static int wm_adsp_buffer_reenable_irq(struct wm_adsp_compr_buf *buf)
 
 int wm_adsp_compr_pointer(struct snd_soc_component *component,
 			  struct snd_compr_stream *stream,
-			  struct snd_compr_tstamp *tstamp)
+			  struct snd_compr_tstamp64 *tstamp)
 {
 	struct wm_adsp_compr *compr = stream->runtime->private_data;
 	struct wm_adsp *dsp = compr->dsp;
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index 25210d404bf1..8035fda71f8d 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -131,7 +131,7 @@ int wm_adsp_compr_trigger(struct snd_soc_component *component,
 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);
+			  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..9dfb0a814b94 100644
--- a/sound/soc/intel/atom/sst-mfld-platform-compress.c
+++ b/sound/soc/intel/atom/sst-mfld-platform-compress.c
@@ -18,6 +18,7 @@
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include <sound/compress_driver.h>
+#include <asm/div64.h>
 #include "sst-mfld-platform.h"
 
 /* compress stream operations */
@@ -202,15 +203,16 @@ static int sst_platform_compr_trigger(struct snd_soc_component *component,
 
 static int sst_platform_compr_pointer(struct snd_soc_component *component,
 				      struct snd_compr_stream *cstream,
-				      struct snd_compr_tstamp *tstamp)
+				      struct snd_compr_tstamp64 *tstamp)
 {
 	struct sst_runtime_stream *stream;
+	u64 temp_copied_total = tstamp->copied_total;
 
-	stream  = cstream->runtime->private_data;
+	stream = cstream->runtime->private_data;
 	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);
+	tstamp->byte_offset =
+		do_div(temp_copied_total, cstream->runtime->buffer_size);
+	pr_debug("calc bytes offset/copied bytes as %u\n", tstamp->byte_offset);
 	return 0;
 }
 
diff --git a/sound/soc/intel/atom/sst-mfld-platform.h b/sound/soc/intel/atom/sst-mfld-platform.h
index 8b5777d3229a..a0e33f7f01c5 100644
--- a/sound/soc/intel/atom/sst-mfld-platform.h
+++ b/sound/soc/intel/atom/sst-mfld-platform.h
@@ -105,7 +105,7 @@ struct compress_sst_ops {
 	int (*stream_pause_release)(struct device *dev,	unsigned int str_id);
 
 	int (*tstamp)(struct device *dev, unsigned int str_id,
-			struct snd_compr_tstamp *tstamp);
+		      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..2646c4632ca1 100644
--- a/sound/soc/intel/atom/sst/sst_drv_interface.c
+++ b/sound/soc/intel/atom/sst/sst_drv_interface.c
@@ -326,7 +326,7 @@ static int sst_cdev_stream_partial_drain(struct device *dev,
 }
 
 static int sst_cdev_tstamp(struct device *dev, unsigned int str_id,
-		struct snd_compr_tstamp *tstamp)
+			   struct snd_compr_tstamp64 *tstamp)
 {
 	struct snd_sst_tstamp fw_tstamp = {0,};
 	struct stream_info *stream;
@@ -349,10 +349,11 @@ static int sst_cdev_tstamp(struct device *dev, unsigned int str_id,
 			(u64)stream->num_ch * SST_GET_BYTES_PER_SAMPLE(24));
 	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, "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 %d\n", tstamp->pcm_io_frames);
+	dev_dbg(dev, "rendered %llu\n", tstamp->pcm_io_frames);
 
 	return 0;
 }
diff --git a/sound/soc/intel/avs/probes.c b/sound/soc/intel/avs/probes.c
index a42736b9aa55..b5b4b0754b71 100644
--- a/sound/soc/intel/avs/probes.c
+++ b/sound/soc/intel/avs/probes.c
@@ -213,7 +213,7 @@ static int avs_probe_compr_trigger(struct snd_compr_stream *cstream, int cmd,
 }
 
 static int avs_probe_compr_pointer(struct snd_compr_stream *cstream,
-				   struct snd_compr_tstamp *tstamp, struct snd_soc_dai *dai)
+				   struct snd_compr_tstamp64 *tstamp, struct snd_soc_dai *dai)
 {
 	struct hdac_ext_stream *host_stream = avs_compr_get_host_stream(cstream);
 	struct snd_soc_pcm_stream *pstream;
diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
index 2cd522108221..09da26f712a6 100644
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -11,6 +11,7 @@
 #include <sound/soc-dapm.h>
 #include <linux/spinlock.h>
 #include <sound/pcm.h>
+#include <asm/div64.h>
 #include <asm/dma.h>
 #include <linux/dma-mapping.h>
 #include <sound/pcm_params.h>
@@ -65,9 +66,9 @@ struct q6apm_dai_rtd {
 	unsigned int pcm_size;
 	unsigned int pcm_count;
 	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;
 	snd_pcm_uframes_t queue_ptr;
 	bool next_track;
@@ -575,15 +576,17 @@ static int q6apm_dai_compr_get_codec_caps(struct snd_soc_component *component,
 
 static int q6apm_dai_compr_pointer(struct snd_soc_component *component,
 				   struct snd_compr_stream *stream,
-				   struct snd_compr_tstamp *tstamp)
+				   struct snd_compr_tstamp64 *tstamp)
 {
 	struct snd_compr_runtime *runtime = stream->runtime;
 	struct q6apm_dai_rtd *prtd = runtime->private_data;
 	unsigned long flags;
+	uint64_t temp_copied_total;
 
 	spin_lock_irqsave(&prtd->lock, flags);
 	tstamp->copied_total = prtd->copied_total;
-	tstamp->byte_offset = prtd->copied_total % prtd->pcm_size;
+	temp_copied_total = tstamp->copied_total;
+	tstamp->byte_offset = do_div(temp_copied_total, prtd->pcm_size);
 	spin_unlock_irqrestore(&prtd->lock, flags);
 
 	return 0;
@@ -760,21 +763,24 @@ static int q6apm_compr_copy(struct snd_soc_component *component,
 	size_t copy;
 	u32 wflags = 0;
 	u32 app_pointer;
-	u32 bytes_received;
+	uint64_t bytes_received;
+	uint64_t temp_bytes_received;
 	uint32_t bytes_to_write;
-	int avail, bytes_in_flight = 0;
+	uint64_t avail, bytes_in_flight = 0;
 
 	bytes_received = prtd->bytes_received;
+	temp_bytes_received = bytes_received;
 
 	/**
 	 * Make sure that next track data pointer is aligned at 32 bit boundary
 	 * This is a Mandatory requirement from DSP data buffers alignment
 	 */
-	if (prtd->next_track)
+	if (prtd->next_track) {
 		bytes_received = ALIGN(prtd->bytes_received, prtd->pcm_count);
+		temp_bytes_received = bytes_received;
+	}
 
-	app_pointer = bytes_received/prtd->pcm_size;
-	app_pointer = bytes_received -  (app_pointer * prtd->pcm_size);
+	app_pointer = do_div(temp_bytes_received, prtd->pcm_size);
 	dstn = prtd->dma_buffer.area + app_pointer;
 
 	if (count < prtd->pcm_size - app_pointer) {
diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c
index a400c9a31fea..b616ce316d2f 100644
--- a/sound/soc/qcom/qdsp6/q6asm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6asm-dai.c
@@ -14,6 +14,7 @@
 #include <sound/pcm.h>
 #include <linux/spinlock.h>
 #include <sound/compress_driver.h>
+#include <asm/div64.h>
 #include <asm/dma.h>
 #include <linux/dma-mapping.h>
 #include <sound/pcm_params.h>
@@ -59,9 +60,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;
@@ -1026,16 +1027,18 @@ static int q6asm_dai_compr_trigger(struct snd_soc_component *component,
 
 static int q6asm_dai_compr_pointer(struct snd_soc_component *component,
 				   struct snd_compr_stream *stream,
-				   struct snd_compr_tstamp *tstamp)
+				   struct snd_compr_tstamp64 *tstamp)
 {
 	struct snd_compr_runtime *runtime = stream->runtime;
 	struct q6asm_dai_rtd *prtd = runtime->private_data;
 	unsigned long flags;
+	uint64_t temp_copied_total;
 
 	spin_lock_irqsave(&prtd->lock, flags);
 
 	tstamp->copied_total = prtd->copied_total;
-	tstamp->byte_offset = prtd->copied_total % prtd->pcm_size;
+	temp_copied_total = tstamp->copied_total;
+	tstamp->byte_offset = do_div(temp_copied_total, prtd->pcm_size);
 
 	spin_unlock_irqrestore(&prtd->lock, flags);
 
@@ -1050,23 +1053,26 @@ 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;
+	uint64_t avail, bytes_in_flight = 0;
 	void *dstn;
 	size_t copy;
 	u32 app_pointer;
-	u32 bytes_received;
+	uint64_t bytes_received;
+	uint64_t temp_bytes_received;
 
 	bytes_received = prtd->bytes_received;
+	temp_bytes_received = bytes_received;
 
 	/**
 	 * Make sure that next track data pointer is aligned at 32 bit boundary
 	 * This is a Mandatory requirement from DSP data buffers alignment
 	 */
-	if (prtd->next_track)
+	if (prtd->next_track) {
 		bytes_received = ALIGN(prtd->bytes_received, prtd->pcm_count);
+		temp_bytes_received = bytes_received;
+	}
 
-	app_pointer = bytes_received/prtd->pcm_size;
-	app_pointer = bytes_received -  (app_pointer * prtd->pcm_size);
+	app_pointer = do_div(temp_bytes_received, prtd->pcm_size);
 	dstn = prtd->dma_buffer.area + app_pointer;
 
 	if (count < prtd->pcm_size - app_pointer) {
diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
index 25f5e543ae8d..7976953b20f0 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -637,7 +637,7 @@ int snd_soc_component_compr_ack(struct snd_compr_stream *cstream, size_t bytes)
 EXPORT_SYMBOL_GPL(snd_soc_component_compr_ack);
 
 int snd_soc_component_compr_pointer(struct snd_compr_stream *cstream,
-				    struct snd_compr_tstamp *tstamp)
+				    struct snd_compr_tstamp64 *tstamp)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 01d1d6bee28c..7b81dffc6a93 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -457,7 +457,7 @@ static int soc_compr_ack(struct snd_compr_stream *cstream, size_t bytes)
 }
 
 static int soc_compr_pointer(struct snd_compr_stream *cstream,
-			     struct snd_compr_tstamp *tstamp)
+			     struct snd_compr_tstamp64 *tstamp)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	int ret;
diff --git a/sound/soc/soc-dai.c b/sound/soc/soc-dai.c
index a210089747d0..ff82effa48e5 100644
--- a/sound/soc/soc-dai.c
+++ b/sound/soc/soc-dai.c
@@ -772,7 +772,7 @@ EXPORT_SYMBOL_GPL(snd_soc_dai_compr_ack);
 
 int snd_soc_dai_compr_pointer(struct snd_soc_dai *dai,
 			      struct snd_compr_stream *cstream,
-			      struct snd_compr_tstamp *tstamp)
+			      struct snd_compr_tstamp64 *tstamp)
 {
 	int ret = 0;
 
diff --git a/sound/soc/sof/amd/acp-probes.c b/sound/soc/sof/amd/acp-probes.c
index 0d0f8ec4aed8..ce51ed108a47 100644
--- a/sound/soc/sof/amd/acp-probes.c
+++ b/sound/soc/sof/amd/acp-probes.c
@@ -108,7 +108,7 @@ static int acp_probes_compr_trigger(struct sof_client_dev *cdev,
 
 static int acp_probes_compr_pointer(struct sof_client_dev *cdev,
 				    struct snd_compr_stream *cstream,
-				    struct snd_compr_tstamp *tstamp,
+				    struct snd_compr_tstamp64 *tstamp,
 				    struct snd_soc_dai *dai)
 {
 	struct acp_dsp_stream *stream = cstream->runtime->private_data;
diff --git a/sound/soc/sof/compress.c b/sound/soc/sof/compress.c
index d7b044f33d79..90b932ae3bab 100644
--- a/sound/soc/sof/compress.c
+++ b/sound/soc/sof/compress.c
@@ -361,7 +361,7 @@ static int sof_compr_copy(struct snd_soc_component *component,
 
 static int sof_compr_pointer(struct snd_soc_component *component,
 			     struct snd_compr_stream *cstream,
-			     struct snd_compr_tstamp *tstamp)
+			     struct snd_compr_tstamp64 *tstamp)
 {
 	struct snd_sof_pcm *spcm;
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
diff --git a/sound/soc/sof/intel/hda-probes.c b/sound/soc/sof/intel/hda-probes.c
index c645346c2c84..b06933cebc45 100644
--- a/sound/soc/sof/intel/hda-probes.c
+++ b/sound/soc/sof/intel/hda-probes.c
@@ -112,7 +112,7 @@ static int hda_probes_compr_trigger(struct sof_client_dev *cdev,
 
 static int hda_probes_compr_pointer(struct sof_client_dev *cdev,
 				    struct snd_compr_stream *cstream,
-				    struct snd_compr_tstamp *tstamp,
+				    struct snd_compr_tstamp64 *tstamp,
 				    struct snd_soc_dai *dai)
 {
 	struct hdac_ext_stream *hext_stream = hda_compr_get_stream(cstream);
diff --git a/sound/soc/sof/sof-client-probes.c b/sound/soc/sof/sof-client-probes.c
index aff9ce980429..71930a922c29 100644
--- a/sound/soc/sof/sof-client-probes.c
+++ b/sound/soc/sof/sof-client-probes.c
@@ -137,7 +137,7 @@ static int sof_probes_compr_trigger(struct snd_compr_stream *cstream, int cmd,
 }
 
 static int sof_probes_compr_pointer(struct snd_compr_stream *cstream,
-				    struct snd_compr_tstamp *tstamp,
+				    struct snd_compr_tstamp64 *tstamp,
 				    struct snd_soc_dai *dai)
 {
 	struct snd_soc_card *card = snd_soc_component_get_drvdata(dai->component);
diff --git a/sound/soc/sof/sof-client-probes.h b/sound/soc/sof/sof-client-probes.h
index da04d65b8d99..8713b69cda4b 100644
--- a/sound/soc/sof/sof-client-probes.h
+++ b/sound/soc/sof/sof-client-probes.h
@@ -4,7 +4,7 @@
 #define __SOF_CLIENT_PROBES_H
 
 struct snd_compr_stream;
-struct snd_compr_tstamp;
+struct snd_compr_tstamp64;
 struct snd_compr_params;
 struct sof_client_dev;
 struct snd_soc_dai;
@@ -24,7 +24,7 @@ struct sof_probes_host_ops {
 	int (*trigger)(struct sof_client_dev *cdev, struct snd_compr_stream *cstream,
 		       int cmd, struct snd_soc_dai *dai);
 	int (*pointer)(struct sof_client_dev *cdev, struct snd_compr_stream *cstream,
-		       struct snd_compr_tstamp *tstamp,
+		       struct snd_compr_tstamp64 *tstamp,
 		       struct snd_soc_dai *dai);
 };
 
diff --git a/sound/soc/sprd/sprd-pcm-compress.c b/sound/soc/sprd/sprd-pcm-compress.c
index 57bd1a0728ac..4b6ebfa5b033 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 */
@@ -513,7 +513,7 @@ static int sprd_platform_compr_trigger(struct snd_soc_component *component,
 
 static int sprd_platform_compr_pointer(struct snd_soc_component *component,
 				       struct snd_compr_stream *cstream,
-				       struct snd_compr_tstamp *tstamp)
+				       struct snd_compr_tstamp64 *tstamp)
 {
 	struct snd_compr_runtime *runtime = cstream->runtime;
 	struct sprd_compr_stream *stream = runtime->private_data;
diff --git a/sound/soc/sprd/sprd-pcm-dma.h b/sound/soc/sprd/sprd-pcm-dma.h
index be5e385f5e42..c5935a1367e6 100644
--- a/sound/soc/sprd/sprd-pcm-dma.h
+++ b/sound/soc/sprd/sprd-pcm-dma.h
@@ -19,7 +19,7 @@ struct sprd_compr_playinfo {
 	int total_time;
 	int current_time;
 	int total_data_length;
-	int current_data_offset;
+	u64 current_data_offset;
 };
 
 struct sprd_compr_params {
@@ -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..b18af98a552b 100644
--- a/sound/soc/uniphier/aio-compress.c
+++ b/sound/soc/uniphier/aio-compress.c
@@ -249,7 +249,7 @@ static int uniphier_aio_compr_trigger(struct snd_soc_component *component,
 
 static int uniphier_aio_compr_pointer(struct snd_soc_component *component,
 				      struct snd_compr_stream *cstream,
-				      struct snd_compr_tstamp *tstamp)
+				      struct snd_compr_tstamp64 *tstamp)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_compr_runtime *runtime = cstream->runtime;
-- 
2.50.1.565.gc32cd1483b-goog

[PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl

[Joris Verhaegen]

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.

This patch bumps the SNDRV_COMPRESS_VERSION to 0.4.0.

Reviewed-by: Miller Liang <millerliang@google.com>
Tested-by: Joris Verhaegen <verhaegen@google.com>
Signed-off-by: Joris Verhaegen <verhaegen@google.com>
---
 include/uapi/sound/compress_offload.h |  5 +++--
 sound/core/compress_offload.c         | 19 +++++++++++++------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
index abd0ea3f86ee..70b8921601f9 100644
--- a/include/uapi/sound/compress_offload.h
+++ b/include/uapi/sound/compress_offload.h
@@ -13,8 +13,7 @@
 #include <sound/asound.h>
 #include <sound/compress_params.h>
 
-
-#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 3, 0)
+#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 4, 0)
 /**
  * struct snd_compressed_buffer - compressed buffer
  * @fragment_size: size of buffer fragment in bytes
@@ -208,6 +207,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 +230,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 d3164aa07158..445220fdb6a0 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -736,18 +736,23 @@ 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_tstamp(struct snd_compr_stream *stream,
+				   unsigned long arg, bool is_32bit)
 {
 	struct snd_compr_tstamp64 tstamp64 = { 0 };
 	struct snd_compr_tstamp tstamp32 = { 0 };
+	const void *copy_from = &tstamp64;
+	size_t copy_size = sizeof(tstamp64);
 	int ret;
 
 	ret = snd_compr_update_tstamp(stream, &tstamp64);
 	if (ret == 0) {
-		snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
-		ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
-				   &tstamp32, sizeof(tstamp32)) ?
+		if (is_32bit) {
+			snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
+			copy_from = &tstamp32;
+			copy_size = sizeof(tstamp32);
+		}
+		ret = copy_to_user((void __user *)arg, copy_from, copy_size) ?
 			      -EFAULT :
 			      0;
 	}
@@ -1327,7 +1332,9 @@ static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
 
 	switch (cmd) {
 	case SNDRV_COMPRESS_TSTAMP:
-		return snd_compr_tstamp(stream, arg);
+		return snd_compr_tstamp(stream, arg, true);
+	case SNDRV_COMPRESS_TSTAMP64:
+		return snd_compr_tstamp(stream, arg, false);
 	case SNDRV_COMPRESS_AVAIL:
 		return snd_compr_ioctl_avail(stream, arg);
 	case SNDRV_COMPRESS_PAUSE:
-- 
2.50.1.565.gc32cd1483b-goog

[PATCH v4 3/3] ALSA: compress_offload: Add SNDRV_COMPRESS_AVAIL64 ioctl

[Joris Verhaegen]

The previous patch introduced a 64-bit timestamp ioctl
(SNDRV_COMPRESS_TSTAMP64). To provide a consistent API, this patch
adds a corresponding 64-bit version of the SNDRV_COMPRESS_AVAIL ioctl.

A new struct snd_compr_avail64 is added to the UAPI, which includes
the 64-bit timestamp. The existing ioctl implementation is refactored
to handle both the 32-bit and 64-bit variants.

Reviewed-by: Miller Liang <millerliang@google.com>
Tested-by: Joris Verhaegen <verhaegen@google.com>
Signed-off-by: Joris Verhaegen <verhaegen@google.com>
---
 include/uapi/sound/compress_offload.h | 11 +++++++
 sound/core/compress_offload.c         | 45 +++++++++++++++++----------
 2 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
index 70b8921601f9..26f756cc2e62 100644
--- a/include/uapi/sound/compress_offload.h
+++ b/include/uapi/sound/compress_offload.h
@@ -84,6 +84,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,
@@ -231,6 +241,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 445220fdb6a0..4d3cf49c0c47 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -203,13 +203,10 @@ static int snd_compr_update_tstamp(struct snd_compr_stream *stream,
 }
 
 static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
-		struct snd_compr_avail *avail)
+				   struct snd_compr_avail64 *avail)
 {
-	struct snd_compr_tstamp64 tstamp64 = { 0 };
-
 	memset(avail, 0, sizeof(*avail));
-	snd_compr_update_tstamp(stream, &tstamp64);
-	snd_compr_tstamp32_from_64(&avail->tstamp, &tstamp64);
+	snd_compr_update_tstamp(stream, &avail->tstamp);
 	/* Still need to return avail even if tstamp can't be filled in */
 
 	if (stream->runtime->total_bytes_available == 0 &&
@@ -233,32 +230,47 @@ static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
 	}
 
 	avail->avail = stream->runtime->total_bytes_available -
-			stream->runtime->total_bytes_transferred;
+		       stream->runtime->total_bytes_transferred;
 	if (stream->direction == SND_COMPRESS_PLAYBACK)
 		avail->avail = stream->runtime->buffer_size - avail->avail;
 
-	pr_debug("ret avail as %llu\n", avail->avail);
+	pr_debug("ret avail as %zu\n", (size_t)avail->avail);
 	return avail->avail;
 }
 
 static inline size_t snd_compr_get_avail(struct snd_compr_stream *stream)
 {
-	struct snd_compr_avail avail;
+	struct snd_compr_avail64 avail;
 
 	return snd_compr_calc_avail(stream, &avail);
 }
 
-static int
-snd_compr_ioctl_avail(struct snd_compr_stream *stream, unsigned long arg)
+static void snd_compr_avail32_from_64(struct snd_compr_avail *avail32,
+				      const struct snd_compr_avail64 *avail64)
 {
-	struct snd_compr_avail ioctl_avail;
+	avail32->avail = avail64->avail;
+	snd_compr_tstamp32_from_64(&avail32->tstamp, &avail64->tstamp);
+}
+
+static int snd_compr_ioctl_avail(struct snd_compr_stream *stream,
+				 unsigned long arg, bool is_32bit)
+{
+	struct snd_compr_avail64 ioctl_avail64;
+	struct snd_compr_avail ioctl_avail32;
 	size_t avail;
+	const void *copy_from = &ioctl_avail64;
+	size_t copy_size = sizeof(ioctl_avail64);
 
 	if (stream->direction == SND_COMPRESS_ACCEL)
 		return -EBADFD;
 
-	avail = snd_compr_calc_avail(stream, &ioctl_avail);
-	ioctl_avail.avail = avail;
+	avail = snd_compr_calc_avail(stream, &ioctl_avail64);
+	ioctl_avail64.avail = avail;
+	if (is_32bit) {
+		snd_compr_avail32_from_64(&ioctl_avail32, &ioctl_avail64);
+		copy_from = &ioctl_avail32;
+		copy_size = sizeof(ioctl_avail32);
+	}
 
 	switch (stream->runtime->state) {
 	case SNDRV_PCM_STATE_OPEN:
@@ -269,8 +281,7 @@ 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, copy_from, copy_size))
 		return -EFAULT;
 	return 0;
 }
@@ -1336,7 +1347,9 @@ static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
 	case SNDRV_COMPRESS_TSTAMP64:
 		return snd_compr_tstamp(stream, arg, false);
 	case SNDRV_COMPRESS_AVAIL:
-		return snd_compr_ioctl_avail(stream, arg);
+		return snd_compr_ioctl_avail(stream, arg, true);
+	case SNDRV_COMPRESS_AVAIL64:
+		return snd_compr_ioctl_avail(stream, arg, false);
 	case SNDRV_COMPRESS_PAUSE:
 		return snd_compr_pause(stream);
 	case SNDRV_COMPRESS_RESUME:
-- 
2.50.1.565.gc32cd1483b-goog

Re: [PATCH v4 0/3] ALSA: compress_offload: Add 64-bit safe timestamp API

[Charles Keepax]

On Fri, Aug 01, 2025 at 10:27:13AM +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 transitions to a 64-bit safe API to solve the problem
> while maintaining perfect backward compatibility with the existing UAPI.
> The pointer operation is reworked to use a new timestamp struct with
> 64-bit fields for the cumulative counters, named snd_compr_tstamp64.
> ASoC drivers are updated to use the 64-bit structures. Corresponding
> ioctls are added to expose them to user-space.
> 
> The series is structured as follows:
> 
> Patch 1: Updates the pointer op, refactors the core logic and ASoC
> drivers 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.
> 
> 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.
> 
> Thanks,
> Joris (George) Verhaegen
> 
> Signed-off-by: Joris Verhaegen <verhaegen@google.com>
> 
> ---

Think it all looks good to me:

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

Re: [PATCH v4 1/3] ALSA: compress_offload: Add 64-bit safe timestamp infrastructure

[Mark Brown]

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

On Fri, Aug 01, 2025 at 10:27:14AM +0100, Joris Verhaegen wrote:
> 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.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl

[Vinod Koul]

On 01-08-25, 10:27, Joris Verhaegen wrote:
> 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.
> 
> This patch bumps the SNDRV_COMPRESS_VERSION to 0.4.0.
> 
> Reviewed-by: Miller Liang <millerliang@google.com>
> Tested-by: Joris Verhaegen <verhaegen@google.com>
> Signed-off-by: Joris Verhaegen <verhaegen@google.com>
> ---
>  include/uapi/sound/compress_offload.h |  5 +++--
>  sound/core/compress_offload.c         | 19 +++++++++++++------
>  2 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
> index abd0ea3f86ee..70b8921601f9 100644
> --- a/include/uapi/sound/compress_offload.h
> +++ b/include/uapi/sound/compress_offload.h
> @@ -13,8 +13,7 @@
>  #include <sound/asound.h>
>  #include <sound/compress_params.h>
>  
> -
> -#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 3, 0)
> +#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 4, 0)
>  /**
>   * struct snd_compressed_buffer - compressed buffer
>   * @fragment_size: size of buffer fragment in bytes
> @@ -208,6 +207,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 +230,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 d3164aa07158..445220fdb6a0 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -736,18 +736,23 @@ 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_tstamp(struct snd_compr_stream *stream,
> +				   unsigned long arg, bool is_32bit)
>  {
>  	struct snd_compr_tstamp64 tstamp64 = { 0 };
>  	struct snd_compr_tstamp tstamp32 = { 0 };
> +	const void *copy_from = &tstamp64;
> +	size_t copy_size = sizeof(tstamp64);
>  	int ret;
>  
>  	ret = snd_compr_update_tstamp(stream, &tstamp64);
>  	if (ret == 0) {
> -		snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> -		ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
> -				   &tstamp32, sizeof(tstamp32)) ?
> +		if (is_32bit) {
> +			snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> +			copy_from = &tstamp32;
> +			copy_size = sizeof(tstamp32);
> +		}

Most of the applications and people would be 32bit right now and we
expect this to progressively change, but then this imposes a penalty as
default path is 64 bit, since we expect this ioctl to be called very
frequently, should we do this optimization for 64bit here?

> +		ret = copy_to_user((void __user *)arg, copy_from, copy_size) ?
>  			      -EFAULT :
>  			      0;
>  	}
> @@ -1327,7 +1332,9 @@ static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
>  
>  	switch (cmd) {
>  	case SNDRV_COMPRESS_TSTAMP:
> -		return snd_compr_tstamp(stream, arg);
> +		return snd_compr_tstamp(stream, arg, true);
> +	case SNDRV_COMPRESS_TSTAMP64:
> +		return snd_compr_tstamp(stream, arg, false);
>  	case SNDRV_COMPRESS_AVAIL:
>  		return snd_compr_ioctl_avail(stream, arg);
>  	case SNDRV_COMPRESS_PAUSE:
> -- 
> 2.50.1.565.gc32cd1483b-goog

-- 
~Vinod

Re: [PATCH v4 3/3] ALSA: compress_offload: Add SNDRV_COMPRESS_AVAIL64 ioctl

[Vinod Koul]

On 01-08-25, 10:27, Joris Verhaegen wrote:
> The previous patch introduced a 64-bit timestamp ioctl
> (SNDRV_COMPRESS_TSTAMP64). To provide a consistent API, this patch
> adds a corresponding 64-bit version of the SNDRV_COMPRESS_AVAIL ioctl.
> 
> A new struct snd_compr_avail64 is added to the UAPI, which includes
> the 64-bit timestamp. The existing ioctl implementation is refactored
> to handle both the 32-bit and 64-bit variants.
> 
> Reviewed-by: Miller Liang <millerliang@google.com>
> Tested-by: Joris Verhaegen <verhaegen@google.com>
> Signed-off-by: Joris Verhaegen <verhaegen@google.com>
> ---
>  include/uapi/sound/compress_offload.h | 11 +++++++
>  sound/core/compress_offload.c         | 45 +++++++++++++++++----------
>  2 files changed, 40 insertions(+), 16 deletions(-)
> 
> diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
> index 70b8921601f9..26f756cc2e62 100644
> --- a/include/uapi/sound/compress_offload.h
> +++ b/include/uapi/sound/compress_offload.h
> @@ -84,6 +84,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,
> @@ -231,6 +241,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 445220fdb6a0..4d3cf49c0c47 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -203,13 +203,10 @@ static int snd_compr_update_tstamp(struct snd_compr_stream *stream,
>  }
>  
>  static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
> -		struct snd_compr_avail *avail)
> +				   struct snd_compr_avail64 *avail)
>  {
> -	struct snd_compr_tstamp64 tstamp64 = { 0 };
> -
>  	memset(avail, 0, sizeof(*avail));
> -	snd_compr_update_tstamp(stream, &tstamp64);
> -	snd_compr_tstamp32_from_64(&avail->tstamp, &tstamp64);
> +	snd_compr_update_tstamp(stream, &avail->tstamp);
>  	/* Still need to return avail even if tstamp can't be filled in */
>  
>  	if (stream->runtime->total_bytes_available == 0 &&
> @@ -233,32 +230,47 @@ static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
>  	}
>  
>  	avail->avail = stream->runtime->total_bytes_available -
> -			stream->runtime->total_bytes_transferred;
> +		       stream->runtime->total_bytes_transferred;

Lets not do formatting changes in current patch please

>  	if (stream->direction == SND_COMPRESS_PLAYBACK)
>  		avail->avail = stream->runtime->buffer_size - avail->avail;
>  
> -	pr_debug("ret avail as %llu\n", avail->avail);
> +	pr_debug("ret avail as %zu\n", (size_t)avail->avail);
>  	return avail->avail;
>  }
>  
>  static inline size_t snd_compr_get_avail(struct snd_compr_stream *stream)
>  {
> -	struct snd_compr_avail avail;
> +	struct snd_compr_avail64 avail;
>  
>  	return snd_compr_calc_avail(stream, &avail);
>  }
>  
> -static int
> -snd_compr_ioctl_avail(struct snd_compr_stream *stream, unsigned long arg)
> +static void snd_compr_avail32_from_64(struct snd_compr_avail *avail32,
> +				      const struct snd_compr_avail64 *avail64)
>  {
> -	struct snd_compr_avail ioctl_avail;
> +	avail32->avail = avail64->avail;
> +	snd_compr_tstamp32_from_64(&avail32->tstamp, &avail64->tstamp);
> +}
> +
> +static int snd_compr_ioctl_avail(struct snd_compr_stream *stream,
> +				 unsigned long arg, bool is_32bit)
> +{
> +	struct snd_compr_avail64 ioctl_avail64;
> +	struct snd_compr_avail ioctl_avail32;
>  	size_t avail;
> +	const void *copy_from = &ioctl_avail64;
> +	size_t copy_size = sizeof(ioctl_avail64);
>  
>  	if (stream->direction == SND_COMPRESS_ACCEL)
>  		return -EBADFD;
>  
> -	avail = snd_compr_calc_avail(stream, &ioctl_avail);
> -	ioctl_avail.avail = avail;
> +	avail = snd_compr_calc_avail(stream, &ioctl_avail64);
> +	ioctl_avail64.avail = avail;
> +	if (is_32bit) {
> +		snd_compr_avail32_from_64(&ioctl_avail32, &ioctl_avail64);
> +		copy_from = &ioctl_avail32;
> +		copy_size = sizeof(ioctl_avail32);
> +	}

Same comment as previous patch

>  
>  	switch (stream->runtime->state) {
>  	case SNDRV_PCM_STATE_OPEN:
> @@ -269,8 +281,7 @@ 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, copy_from, copy_size))
>  		return -EFAULT;
>  	return 0;
>  }
> @@ -1336,7 +1347,9 @@ static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
>  	case SNDRV_COMPRESS_TSTAMP64:
>  		return snd_compr_tstamp(stream, arg, false);
>  	case SNDRV_COMPRESS_AVAIL:
> -		return snd_compr_ioctl_avail(stream, arg);
> +		return snd_compr_ioctl_avail(stream, arg, true);
> +	case SNDRV_COMPRESS_AVAIL64:
> +		return snd_compr_ioctl_avail(stream, arg, false);
>  	case SNDRV_COMPRESS_PAUSE:
>  		return snd_compr_pause(stream);
>  	case SNDRV_COMPRESS_RESUME:
> -- 
> 2.50.1.565.gc32cd1483b-goog

-- 
~Vinod

Re: [PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl

[Takashi Iwai]

On Tue, 05 Aug 2025 06:47:59 +0200,
Vinod Koul wrote:
> 
> On 01-08-25, 10:27, Joris Verhaegen wrote:
> > 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.
> > 
> > This patch bumps the SNDRV_COMPRESS_VERSION to 0.4.0.
> > 
> > Reviewed-by: Miller Liang <millerliang@google.com>
> > Tested-by: Joris Verhaegen <verhaegen@google.com>
> > Signed-off-by: Joris Verhaegen <verhaegen@google.com>
> > ---
> >  include/uapi/sound/compress_offload.h |  5 +++--
> >  sound/core/compress_offload.c         | 19 +++++++++++++------
> >  2 files changed, 16 insertions(+), 8 deletions(-)
> > 
> > diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
> > index abd0ea3f86ee..70b8921601f9 100644
> > --- a/include/uapi/sound/compress_offload.h
> > +++ b/include/uapi/sound/compress_offload.h
> > @@ -13,8 +13,7 @@
> >  #include <sound/asound.h>
> >  #include <sound/compress_params.h>
> >  
> > -
> > -#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 3, 0)
> > +#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 4, 0)
> >  /**
> >   * struct snd_compressed_buffer - compressed buffer
> >   * @fragment_size: size of buffer fragment in bytes
> > @@ -208,6 +207,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 +230,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 d3164aa07158..445220fdb6a0 100644
> > --- a/sound/core/compress_offload.c
> > +++ b/sound/core/compress_offload.c
> > @@ -736,18 +736,23 @@ 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_tstamp(struct snd_compr_stream *stream,
> > +				   unsigned long arg, bool is_32bit)
> >  {
> >  	struct snd_compr_tstamp64 tstamp64 = { 0 };
> >  	struct snd_compr_tstamp tstamp32 = { 0 };
> > +	const void *copy_from = &tstamp64;
> > +	size_t copy_size = sizeof(tstamp64);
> >  	int ret;
> >  
> >  	ret = snd_compr_update_tstamp(stream, &tstamp64);
> >  	if (ret == 0) {
> > -		snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > -		ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
> > -				   &tstamp32, sizeof(tstamp32)) ?
> > +		if (is_32bit) {
> > +			snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > +			copy_from = &tstamp32;
> > +			copy_size = sizeof(tstamp32);
> > +		}
> 
> Most of the applications and people would be 32bit right now and we
> expect this to progressively change, but then this imposes a penalty as
> default path is 64 bit, since we expect this ioctl to be called very
> frequently, should we do this optimization for 64bit here?

Through a quick glance over the patch, I don't think you'll hit the
significant performance loss.  It's merely a few bytes of extra copies
before copy_to_user(), after all.  But, of course, it'd be more
convincing if anyone can test and give the actual numbers.


thanks,

Takashi

Re: [PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl

[George Verhaegen]

On Tue, 05 Aug 2025 06:47:59 +0200,
Vinod Koul wrote:
>
> On 01-08-25, 10:27, Joris Verhaegen wrote:
> > 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.
> >
> > This patch bumps the SNDRV_COMPRESS_VERSION to 0.4.0.
> >
> > Reviewed-by: Miller Liang <millerliang@google.com>
> > Tested-by: Joris Verhaegen <verhaegen@google.com>
> > Signed-off-by: Joris Verhaegen <verhaegen@google.com>
> > ---
> >  include/uapi/sound/compress_offload.h |  5 +++--
> >  sound/core/compress_offload.c         | 19 +++++++++++++------
> >  2 files changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
> > index abd0ea3f86ee..70b8921601f9 100644
> > --- a/include/uapi/sound/compress_offload.h
> > +++ b/include/uapi/sound/compress_offload.h
> > @@ -13,8 +13,7 @@
> >  #include <sound/asound.h>
> >  #include <sound/compress_params.h>
> >
> > -
> > -#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 3, 0)
> > +#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 4, 0)
> >  /**
> >   * struct snd_compressed_buffer - compressed buffer
> >   * @fragment_size: size of buffer fragment in bytes
> > @@ -208,6 +207,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 +230,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 d3164aa07158..445220fdb6a0 100644
> > --- a/sound/core/compress_offload.c
> > +++ b/sound/core/compress_offload.c
> > @@ -736,18 +736,23 @@ 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_tstamp(struct snd_compr_stream *stream,
> > +                              unsigned long arg, bool is_32bit)
> >  {
> >     struct snd_compr_tstamp64 tstamp64 = { 0 };
> >     struct snd_compr_tstamp tstamp32 = { 0 };
> > +   const void *copy_from = &tstamp64;
> > +   size_t copy_size = sizeof(tstamp64);
> >     int ret;
> >
> >     ret = snd_compr_update_tstamp(stream, &tstamp64);
> >     if (ret == 0) {
> > -           snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > -           ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
> > -                              &tstamp32, sizeof(tstamp32)) ?
> > +           if (is_32bit) {
> > +                   snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > +                   copy_from = &tstamp32;
> > +                   copy_size = sizeof(tstamp32);
> > +           }
>
> Most of the applications and people would be 32bit right now and we
> expect this to progressively change, but then this imposes a penalty as
> default path is 64 bit, since we expect this ioctl to be called very
> frequently, should we do this optimization for 64bit here?

Valid point about optimizing the common path. But since the underlying
.pointer op was changed in the patch V3 to always return a 64-bit
snd_compr_tstamp64, there is no longer a "native" 32-bit path to fetch.

Any call to the legacy SNDRV_COMPRESS_TSTAMP ioctl must fetch the 64-bit
value and then truncate it down. The current patch reflects this by sharing the
common 64-bit fetch and then conditionally performing the truncation to 32-bit.
Splitting the function would result in two functions that both call
snd_compr_update_tstamp() to get the 64-bit data, so there would be code
duplication with no performance gain.

Thanks,
George

Re: [PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl

[Charles Keepax]

On Tue, Aug 05, 2025 at 07:59:42AM +0200, Takashi Iwai wrote:
> On Tue, 05 Aug 2025 06:47:59 +0200,
> Vinod Koul wrote:
> > On 01-08-25, 10:27, Joris Verhaegen wrote:
> > >  	ret = snd_compr_update_tstamp(stream, &tstamp64);
> > >  	if (ret == 0) {
> > > -		snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > > -		ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
> > > -				   &tstamp32, sizeof(tstamp32)) ?
> > > +		if (is_32bit) {
> > > +			snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > > +			copy_from = &tstamp32;
> > > +			copy_size = sizeof(tstamp32);
> > > +		}
> > 
> > Most of the applications and people would be 32bit right now and we
> > expect this to progressively change, but then this imposes a penalty as
> > default path is 64 bit, since we expect this ioctl to be called very
> > frequently, should we do this optimization for 64bit here?
> 
> Through a quick glance over the patch, I don't think you'll hit the
> significant performance loss.  It's merely a few bytes of extra copies
> before copy_to_user(), after all.  But, of course, it'd be more
> convincing if anyone can test and give the actual numbers.
> 

I am inclined to agree the impact should be very minimal and the
only alternative is a more complex implementation. I would vote
for leaving this as is.

Thanks,
Charles

Re: [PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl

[Vinod Koul]

On 18-08-25, 15:05, Charles Keepax wrote:
> On Tue, Aug 05, 2025 at 07:59:42AM +0200, Takashi Iwai wrote:
> > On Tue, 05 Aug 2025 06:47:59 +0200,
> > Vinod Koul wrote:
> > > On 01-08-25, 10:27, Joris Verhaegen wrote:
> > > >  	ret = snd_compr_update_tstamp(stream, &tstamp64);
> > > >  	if (ret == 0) {
> > > > -		snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > > > -		ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
> > > > -				   &tstamp32, sizeof(tstamp32)) ?
> > > > +		if (is_32bit) {
> > > > +			snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > > > +			copy_from = &tstamp32;
> > > > +			copy_size = sizeof(tstamp32);
> > > > +		}
> > > 
> > > Most of the applications and people would be 32bit right now and we
> > > expect this to progressively change, but then this imposes a penalty as
> > > default path is 64 bit, since we expect this ioctl to be called very
> > > frequently, should we do this optimization for 64bit here?
> > 
> > Through a quick glance over the patch, I don't think you'll hit the
> > significant performance loss.  It's merely a few bytes of extra copies
> > before copy_to_user(), after all.  But, of course, it'd be more
> > convincing if anyone can test and give the actual numbers.

That would be better

> I am inclined to agree the impact should be very minimal and the
> only alternative is a more complex implementation. I would vote
> for leaving this as is.

But yes, we can for now, go ahead. It is internal kernel flow can be
adapted anytime :-)

-- 
~Vinod

Re: [PATCH v4 2/3] ALSA: compress_offload: Add SNDRV_COMPRESS_TSTAMP64 ioctl

[Takashi Iwai]

On Tue, 19 Aug 2025 07:08:08 +0200,
Vinod Koul wrote:
> 
> On 18-08-25, 15:05, Charles Keepax wrote:
> > On Tue, Aug 05, 2025 at 07:59:42AM +0200, Takashi Iwai wrote:
> > > On Tue, 05 Aug 2025 06:47:59 +0200,
> > > Vinod Koul wrote:
> > > > On 01-08-25, 10:27, Joris Verhaegen wrote:
> > > > >  	ret = snd_compr_update_tstamp(stream, &tstamp64);
> > > > >  	if (ret == 0) {
> > > > > -		snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > > > > -		ret = copy_to_user((struct snd_compr_tstamp __user *)arg,
> > > > > -				   &tstamp32, sizeof(tstamp32)) ?
> > > > > +		if (is_32bit) {
> > > > > +			snd_compr_tstamp32_from_64(&tstamp32, &tstamp64);
> > > > > +			copy_from = &tstamp32;
> > > > > +			copy_size = sizeof(tstamp32);
> > > > > +		}
> > > > 
> > > > Most of the applications and people would be 32bit right now and we
> > > > expect this to progressively change, but then this imposes a penalty as
> > > > default path is 64 bit, since we expect this ioctl to be called very
> > > > frequently, should we do this optimization for 64bit here?
> > > 
> > > Through a quick glance over the patch, I don't think you'll hit the
> > > significant performance loss.  It's merely a few bytes of extra copies
> > > before copy_to_user(), after all.  But, of course, it'd be more
> > > convincing if anyone can test and give the actual numbers.
> 
> That would be better
> 
> > I am inclined to agree the impact should be very minimal and the
> > only alternative is a more complex implementation. I would vote
> > for leaving this as is.
> 
> But yes, we can for now, go ahead. It is internal kernel flow can be
> adapted anytime :-)

OK, Joris, could you submit v5 with correction of what Vinod
suggested in reviews?


thanks,

Takashi

Re: [PATCH v4 3/3] ALSA: compress_offload: Add SNDRV_COMPRESS_AVAIL64 ioctl

[George Verhaegen]

On Tue, 5 Aug 2025 at 06:50, Vinod Koul <vkoul@kernel.org> wrote:
> >  static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
> > -             struct snd_compr_avail *avail)
> > +                                struct snd_compr_avail64 *avail)
> >  {
> > -     struct snd_compr_tstamp64 tstamp64 = { 0 };
> > -
> >       memset(avail, 0, sizeof(*avail));
> > -     snd_compr_update_tstamp(stream, &tstamp64);
> > -     snd_compr_tstamp32_from_64(&avail->tstamp, &tstamp64);
> > +     snd_compr_update_tstamp(stream, &avail->tstamp);
> >       /* Still need to return avail even if tstamp can't be filled in */
> >
> >       if (stream->runtime->total_bytes_available == 0 &&
> > @@ -233,32 +230,47 @@ static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
> >       }
> >
> >       avail->avail = stream->runtime->total_bytes_available -
> > -                     stream->runtime->total_bytes_transferred;
> > +                    stream->runtime->total_bytes_transferred;
>
> Lets not do formatting changes in current patch please

I'm happy to revert the formatting changes. Are you only referring to
the alignment of the subtraction operation (avail->avail), or
something else too? Note that the second line of the function
signature of snd_compr_calc_avail got re-aligned by clang-format due
to an actual change (type change to struct snd_compr_avail64) so I
assume the re-alignment here is OK, but please confirm.

> >       if (stream->direction == SND_COMPRESS_PLAYBACK)
> >               avail->avail = stream->runtime->buffer_size - avail->avail;
> >
> > -     pr_debug("ret avail as %llu\n", avail->avail);
> > +     pr_debug("ret avail as %zu\n", (size_t)avail->avail);
> >       return avail->avail;
> >  }
> >
> >  static inline size_t snd_compr_get_avail(struct snd_compr_stream *stream)
> >  {
> > -     struct snd_compr_avail avail;
> > +     struct snd_compr_avail64 avail;
> >
> >       return snd_compr_calc_avail(stream, &avail);
> >  }
> >
> > -static int
> > -snd_compr_ioctl_avail(struct snd_compr_stream *stream, unsigned long arg)
> > +static void snd_compr_avail32_from_64(struct snd_compr_avail *avail32,
> > +                                   const struct snd_compr_avail64 *avail64)
> >  {
> > -     struct snd_compr_avail ioctl_avail;
> > +     avail32->avail = avail64->avail;
> > +     snd_compr_tstamp32_from_64(&avail32->tstamp, &avail64->tstamp);
> > +}
> > +
> > +static int snd_compr_ioctl_avail(struct snd_compr_stream *stream,
> > +                              unsigned long arg, bool is_32bit)
> > +{
> > +     struct snd_compr_avail64 ioctl_avail64;
> > +     struct snd_compr_avail ioctl_avail32;
> >       size_t avail;
> > +     const void *copy_from = &ioctl_avail64;
> > +     size_t copy_size = sizeof(ioctl_avail64);
> >
> >       if (stream->direction == SND_COMPRESS_ACCEL)
> >               return -EBADFD;
> >
> > -     avail = snd_compr_calc_avail(stream, &ioctl_avail);
> > -     ioctl_avail.avail = avail;
> > +     avail = snd_compr_calc_avail(stream, &ioctl_avail64);
> > +     ioctl_avail64.avail = avail;
> > +     if (is_32bit) {
> > +             snd_compr_avail32_from_64(&ioctl_avail32, &ioctl_avail64);
> > +             copy_from = &ioctl_avail32;
> > +             copy_size = sizeof(ioctl_avail32);
> > +     }
>
> Same comment as previous patch

As agreed in the previous patch, will leave as is.

Thanks,
George