ASoC: qcom: q6apm: Fix shared memory unmap

Shawn Guo <shengchao.guo@oss.qualcomm.com> says:

Since the early buffer mapping, q6apm maps a front-end's DSP buffers
once in pcm_new() and unmaps them in pcm_free(). The unmap side has a
few problems, which show up when the sound card is unbound and bound
again, e.g. by unbinding and binding any codec on the card:

- The card core removes the components before it frees the PCMs, so
  the topology and its graph info are gone by the time pcm_free()
  tries to unmap. The unmap is silently skipped, the DSP keeps the old
  mapping, and the next bind fails to create the PCM:

    qcom-apm gprsvc:service:2:1: Error (1) Processing 0x0100100c cmd
    snd-sc8280xp sound: ASoC: can't create pcm MultiMedia1 Playback :-22

- Once unmap is actually sent, every command stalls for five seconds,
  because the response handler matches on the wrong opcode:

    qcom-apm gprsvc:service:2:1: CMD timeout for [100100d] opcode

- Unmapping the position buffer clears the circular buffer's handle
  instead of its own.

Patch 1 fixes the unmap response handling, patch 2 makes the position
buffer unmap clear its own handle, and patch 3 moves the unmap to the
APM component's remove callback, before the topology is unloaded.
Patches 1 and 2 go first so that patch 3 does not introduce the five
second stalls.

Tested on Qualcomm Ride board by unbinding and binding the PCM1681
codec.

  echo 0-004c > /sys/bus/i2c/drivers/pcm1681/unbind
  echo 0-004c > /sys/bus/i2c/drivers/pcm1681/bind

Link: https://patch.msgid.link/20261021041044.2080369-1-shengchao.guo@oss.qualcomm.com
This commit is contained in:
Mark Brown 2026-10-26 19:24:16 +00:00
commit 6977822f9d

View File

@ -322,15 +322,15 @@ int q6apm_alloc_fragments(struct q6apm_graph *graph, unsigned int dir, phys_addr
}
EXPORT_SYMBOL_GPL(q6apm_alloc_fragments);
static int __q6apm_unmap_memory_fixed_region(struct device *dev, unsigned int graph_id,
static int __q6apm_unmap_memory_fixed_region(struct q6apm *apm, unsigned int graph_id,
bool is_pos_buf)
{
struct apm_cmd_shared_mem_unmap_regions *cmd;
struct q6apm *apm = dev_get_drvdata(dev->parent);
struct audioreach_graph_info *info;
uint32_t mem_map_handle;
uint32_t pos_mask = is_pos_buf ? APM_MMAP_TOKEN_MAP_TYPE_POS_BUF : 0;
struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(sizeof(*cmd),
APM_CMD_SHARED_MEM_UNMAP_REGIONS, graph_id);
APM_CMD_SHARED_MEM_UNMAP_REGIONS, (graph_id | pos_mask));
if (IS_ERR(pkt))
return PTR_ERR(pkt);
@ -357,13 +357,15 @@ static int __q6apm_unmap_memory_fixed_region(struct device *dev, unsigned int gr
int q6apm_unmap_memory_fixed_region(struct device *dev, unsigned int graph_id)
{
return __q6apm_unmap_memory_fixed_region(dev, graph_id, false);
return __q6apm_unmap_memory_fixed_region(dev_get_drvdata(dev->parent),
graph_id, false);
}
EXPORT_SYMBOL_GPL(q6apm_unmap_memory_fixed_region);
int q6apm_unmap_pos_buffer(struct device *dev, unsigned int graph_id)
{
return __q6apm_unmap_memory_fixed_region(dev, graph_id, true);
return __q6apm_unmap_memory_fixed_region(dev_get_drvdata(dev->parent),
graph_id, true);
}
EXPORT_SYMBOL_GPL(q6apm_unmap_pos_buffer);
@ -850,6 +852,21 @@ static int q6apm_audio_probe(struct snd_soc_component *component)
static void q6apm_audio_remove(struct snd_soc_component *component)
{
struct q6apm *apm = dev_get_drvdata(component->dev);
struct audioreach_graph_info *info;
int id;
/*
* The card core removes components before freeing the PCMs, so
* q6apm-dai's pcm_free() runs after the topology (and graph_info)
* is gone and can no longer unmap. Release the DSP mappings here,
* otherwise the next pcm_new() fails to map the same buffer again.
*/
idr_for_each_entry(&apm->graph_info_idr, info, id) {
__q6apm_unmap_memory_fixed_region(apm, id, false);
__q6apm_unmap_memory_fixed_region(apm, id, true);
}
/* remove topology */
snd_soc_tplg_component_remove(component);
}
@ -960,17 +977,23 @@ static int apm_callback(const struct gpr_resp_pkt *data, void *priv, int op)
wake_up(&apm->wait);
break;
case APM_CMD_SHARED_MEM_UNMAP_REGIONS:
apm->result.opcode = hdr->opcode;
apm->result.status = 0;
rsp = data->payload;
info = idr_find(&apm->graph_info_idr, hdr->token);
if (info)
info->mem_map_handle = 0;
else
apm->result.opcode = result->opcode;
apm->result.status = result->status;
if (result->status)
dev_err(dev, "Error (%d) Processing 0x%08x cmd\n", result->status,
result->opcode);
graph_id = hdr->token & APM_MMAP_TOKEN_GID_MASK;
is_pos_buf = hdr->token & APM_MMAP_TOKEN_MAP_TYPE_POS_BUF;
info = idr_find(&apm->graph_info_idr, graph_id);
if (info && !result->status) {
if (is_pos_buf)
info->pos_buf_mem_map_handle = 0;
else
info->mem_map_handle = 0;
}
wake_up(&apm->wait);
break;
default: