[PATCH v7 04/13] ASoC: qcom: q6apm-lpass-dai: Fix multiple graph opens

[PATCH v7 04/13] ASoC: qcom: q6apm-lpass-dai: Fix multiple graph opens

[Srinivas Kandagatla]

As prepare can be called mulitple times, this can result in multiple
graph opens for playback path.

This will result in a memory leaks, fix this by adding a check before
opening.

Fixes: be1fae62cf25 ("ASoC: q6apm-lpass-dai: close graph on prepare errors")
Cc: Stable@vger.kernel.org
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
---
 sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
index 5be37eeea329..ba64117b8cfe 100644
--- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
+++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
@@ -181,7 +181,7 @@ static int q6apm_lpass_dai_prepare(struct snd_pcm_substream *substream, struct s
 	 * It is recommend to load DSP with source graph first and then sink
 	 * graph, so sequence for playback and capture will be different
 	 */
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && dai_data->graph[dai->id] == NULL) {
 		graph = q6apm_graph_open(dai->dev, NULL, dai->dev, graph_id);
 		if (IS_ERR(graph)) {
 			dev_err(dai->dev, "Failed to open graph (%d)\n", graph_id);
-- 
2.47.3

Re: [PATCH v7 04/13] ASoC: qcom: q6apm-lpass-dai: Fix multiple graph opens

[Mark Brown]

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

On Mon, Mar 23, 2026 at 10:38:36PM +0000, Srinivas Kandagatla wrote:
> As prepare can be called mulitple times, this can result in multiple
> graph opens for playback path.

>  	 */
> -	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && dai_data->graph[dai->id] == NULL) {

This is an array of APM_PORT_MAX elements but we have DAI IDs in the DT
bindings over that and now we're using the DAI ID to index into the
array (I didn't check for existing instances...).  This might be
impossible due to system design though.

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

Re: [PATCH v7 04/13] ASoC: qcom: q6apm-lpass-dai: Fix multiple graph opens

[Mark Brown]

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

On Tue, Mar 24, 2026 at 06:25:42PM +0000, Mark Brown wrote:
> On Mon, Mar 23, 2026 at 10:38:36PM +0000, Srinivas Kandagatla wrote:
> > As prepare can be called mulitple times, this can result in multiple
> > graph opens for playback path.

> >  	 */
> > -	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> > +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && dai_data->graph[dai->id] == NULL) {

> This is an array of APM_PORT_MAX elements but we have DAI IDs in the DT
> bindings over that and now we're using the DAI ID to index into the
> array (I didn't check for existing instances...).  This might be
> impossible due to system design though.

Ah, found an update later in the series so it's OK in the end.  A
potential bisection issue but hopefully not the end of the world.

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

Re: [PATCH v7 04/13] ASoC: qcom: q6apm-lpass-dai: Fix multiple graph opens

[Srinivas Kandagatla]

On 3/24/26 6:25 PM, Mark Brown wrote:
> On Mon, Mar 23, 2026 at 10:38:36PM +0000, Srinivas Kandagatla wrote:
>> As prepare can be called mulitple times, this can result in multiple
>> graph opens for playback path.
> 
>>  	 */
>> -	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && dai_data->graph[dai->id] == NULL) {
> 
> This is an array of APM_PORT_MAX elements but we have DAI IDs in the DT
> bindings over that and now we're using the DAI ID to index into the

The driver has dai->id indexing the array in most places, and that is
how it has been for a while. This is one of the problem which last patch
is trying to address doing a check on the range. At somepoint we need to
move to dynamic allocation tbh.

--srini
> array (I didn't check for existing instances...).  This might be
> impossible due to system design though.

Re: [PATCH v7 04/13] ASoC: qcom: q6apm-lpass-dai: Fix multiple graph opens

[Mark Brown]

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

On Wed, Mar 25, 2026 at 11:33:45AM +0000, Srinivas Kandagatla wrote:
> On 3/24/26 6:25 PM, Mark Brown wrote:
> > On Mon, Mar 23, 2026 at 10:38:36PM +0000, Srinivas Kandagatla wrote:

> >> -	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> >> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && dai_data->graph[dai->id] == NULL) {

> > This is an array of APM_PORT_MAX elements but we have DAI IDs in the DT
> > bindings over that and now we're using the DAI ID to index into the

> The driver has dai->id indexing the array in most places, and that is
> how it has been for a while. This is one of the problem which last patch
> is trying to address doing a check on the range. At somepoint we need to
> move to dynamic allocation tbh.

Yeah, I saw it was a bit shaky all over.  I think having the array size
bumps earlier might help at least make it clearer things are OK, but
dynamic structures of some kind would indeed be ideal.

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

[PATCH v7 05/13] ASoC: qcom: q6apm-dai: reset queue ptr on trigger stop

[Srinivas Kandagatla]

Reset queue pointer on SNDRV_PCM_TRIGGER_STOP event to be inline
with resetting appl_ptr. Without this we will end up with a queue_ptr
out of sync and driver could try to send data that is not ready yet.

Fix this by resetting the queue_ptr.

Fixes: 3d4a4411aa8bb ("ASoC: q6apm-dai: schedule all available frames to avoid dsp under-runs")
Cc: Stable@vger.kernel.org
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
---
 sound/soc/qcom/qdsp6/q6apm-dai.c | 1 +
 sound/soc/qcom/qdsp6/q6apm.c     | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
index 306e928e7b49..292be457764f 100644
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -317,6 +317,7 @@ static int q6apm_dai_trigger(struct snd_soc_component *component,
 	case SNDRV_PCM_TRIGGER_STOP:
 		/* TODO support be handled via SoftPause Module */
 		prtd->state = Q6APM_STREAM_STOPPED;
+		prtd->queue_ptr = 0;
 		break;
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c
index 1fbcbbf3123d..9d4cbe29cf94 100644
--- a/sound/soc/qcom/qdsp6/q6apm.c
+++ b/sound/soc/qcom/qdsp6/q6apm.c
@@ -215,6 +215,8 @@ int q6apm_map_memory_regions(struct q6apm_graph *graph, unsigned int dir, phys_a
 
 	mutex_lock(&graph->lock);
 
+	data->dsp_buf = 0;
+
 	if (data->buf) {
 		mutex_unlock(&graph->lock);
 		return 0;
-- 
2.47.3