Currently we are performing an extra layer of calculation on the hw_ptr,
which is always prone to errors.
Move this to common dsp layer for better accuracy.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
---
 sound/soc/qcom/qdsp6/q6asm.c | 12 ++++++++++++
 sound/soc/qcom/qdsp6/q6asm.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/sound/soc/qcom/qdsp6/q6asm.c b/sound/soc/qcom/qdsp6/q6asm.c
index 371389c8fa7a..643ca944b1b5 100644
--- a/sound/soc/qcom/qdsp6/q6asm.c
+++ b/sound/soc/qcom/qdsp6/q6asm.c
@@ -6,6 +6,7 @@
 #include <linux/mutex.h>
 #include <linux/wait.h>
 #include <linux/module.h>
+#include <linux/atomic.h>
 #include <linux/soc/qcom/apr.h>
 #include <linux/device.h>
 #include <linux/of_platform.h>
@@ -248,6 +249,7 @@ struct audio_port_data {
 	uint32_t num_periods;
 	uint32_t dsp_buf;
 	uint32_t mem_map_handle;
+	atomic_t hw_ptr;
 };
 
 struct q6asm {
@@ -598,6 +600,14 @@ static struct audio_client *q6asm_get_audio_client(struct q6asm *a,
 	return ac;
 }
 
+int q6asm_get_hw_pointer(struct audio_client *ac, unsigned int dir)
+{
+	struct audio_port_data *data = &ac->port[dir];
+
+	return (int)atomic_read(&data->hw_ptr);
+}
+EXPORT_SYMBOL_GPL(q6asm_get_hw_pointer);
+
 static int32_t q6asm_stream_callback(struct apr_device *adev,
 				     struct apr_resp_pkt *data,
 				     int session_id)
@@ -703,6 +713,7 @@ static int32_t q6asm_stream_callback(struct apr_device *adev,
 				goto done;
 			}
 			spin_unlock_irqrestore(&ac->lock, flags);
+			atomic_set(&port->hw_ptr, token + 1);
 		}
 		break;
 	case ASM_DATA_EVENT_READ_DONE_V2:
@@ -721,6 +732,7 @@ static int32_t q6asm_stream_callback(struct apr_device *adev,
 			}
 
 			phys = port->buf[hdr->token].phys;
+			atomic_set(&port->hw_ptr, hdr->token + 1);
 
 			if (upper_32_bits(phys) != done->buf_addr_msw ||
 			    lower_32_bits(phys) != done->buf_addr_lsw) {
diff --git a/sound/soc/qcom/qdsp6/q6asm.h b/sound/soc/qcom/qdsp6/q6asm.h
index 519e1b3a3f7c..6fafda5bd849 100644
--- a/sound/soc/qcom/qdsp6/q6asm.h
+++ b/sound/soc/qcom/qdsp6/q6asm.h
@@ -148,4 +148,5 @@ int q6asm_map_memory_regions(unsigned int dir,
 			     phys_addr_t phys,
 			     size_t period_sz, unsigned int periods);
 int q6asm_unmap_memory_regions(unsigned int dir, struct audio_client *ac);
+int q6asm_get_hw_pointer(struct audio_client *ac, unsigned int dir);
 #endif /* __Q6_ASM_H__ */
-- 
2.51.0

On Wed Oct 15, 2025 at 2:17 PM BST, Srinivas Kandagatla wrote:
> Currently we are performing an extra layer of calculation on the hw_ptr,
> which is always prone to errors.
> Move this to common dsp layer for better accuracy.

The subject says that the change adds q6asm_get_hw_ptr but here it says
that calculation of hw_ptr is moved. Where is it moved out of or from?

Currently the commit message is confusing.

It seems to be potential confusing split with commit.
("ASoC: qcom: q6asm-dai: use q6asm_get_hw_pointer") where calculation
of hw_ptr was implemented in q6asm-dai.c.

> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
> ---
>  sound/soc/qcom/qdsp6/q6asm.c | 12 ++++++++++++
>  sound/soc/qcom/qdsp6/q6asm.h |  1 +
>  2 files changed, 13 insertions(+)
>
> diff --git a/sound/soc/qcom/qdsp6/q6asm.c b/sound/soc/qcom/qdsp6/q6asm.c
> index 371389c8fa7a..643ca944b1b5 100644
> --- a/sound/soc/qcom/qdsp6/q6asm.c
> +++ b/sound/soc/qcom/qdsp6/q6asm.c
> @@ -6,6 +6,7 @@
>  #include <linux/mutex.h>
>  #include <linux/wait.h>
>  #include <linux/module.h>
> +#include <linux/atomic.h>

Ideally this should be sorted but it seems it was not initially.

>  #include <linux/soc/qcom/apr.h>
>  #include <linux/device.h>
>  #include <linux/of_platform.h>
> @@ -248,6 +249,7 @@ struct audio_port_data {
>  	uint32_t num_periods;
>  	uint32_t dsp_buf;
>  	uint32_t mem_map_handle;
> +	atomic_t hw_ptr;
>  };

Thanks,
Alexey

On 10/20/25 4:04 PM, Alexey Klimov wrote:
> On Wed Oct 15, 2025 at 2:17 PM BST, Srinivas Kandagatla wrote:
>> Currently we are performing an extra layer of calculation on the hw_ptr,
>> which is always prone to errors.
>> Move this to common dsp layer for better accuracy.
> 
> The subject says that the change adds q6asm_get_hw_ptr but here it says
> that calculation of hw_ptr is moved. Where is it moved out of or from?
> 
> Currently the commit message is confusing.

Sure, Will rephrase this.

Currently q6asm-dai.c implement tracking the dsp hardware pointer based
on callbacks from q6asm, this is really an overhead, prone to errors and
redundant.
We already have buffers and tokens which can be used to get hw_ptr
location, use this instead.

--srini>
> It seems to be potential confusing split with commit.
> ("ASoC: qcom: q6asm-dai: use q6asm_get_hw_pointer") where calculation
> of hw_ptr was implemented in q6asm-dai.c.
> 
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
>> ---
>>  sound/soc/qcom/qdsp6/q6asm.c | 12 ++++++++++++
>>  sound/soc/qcom/qdsp6/q6asm.h |  1 +
>>  2 files changed, 13 insertions(+)
>>
>> diff --git a/sound/soc/qcom/qdsp6/q6asm.c b/sound/soc/qcom/qdsp6/q6asm.c
>> index 371389c8fa7a..643ca944b1b5 100644
>> --- a/sound/soc/qcom/qdsp6/q6asm.c
>> +++ b/sound/soc/qcom/qdsp6/q6asm.c
>> @@ -6,6 +6,7 @@
>>  #include <linux/mutex.h>
>>  #include <linux/wait.h>
>>  #include <linux/module.h>
>> +#include <linux/atomic.h>
> 
> Ideally this should be sorted but it seems it was not initially.
> 
>>  #include <linux/soc/qcom/apr.h>
>>  #include <linux/device.h>
>>  #include <linux/of_platform.h>
>> @@ -248,6 +249,7 @@ struct audio_port_data {
>>  	uint32_t num_periods;
>>  	uint32_t dsp_buf;
>>  	uint32_t mem_map_handle;
>> +	atomic_t hw_ptr;
>>  };
> 
> Thanks,
> Alexey