Re: [PATCH 1/4] ASoC: qcom: audioreach: compute active channel maps from channel_map

[Srinivas Kandagatla <srini@kernel.org>]

On 6/15/26 2:54 PM, Neil Armstrong wrote:
> On 6/15/26 11:36, Srinivas Kandagatla wrote:
>> On 6/15/26 10:31 AM, Neil Armstrong wrote:
>>> On 6/15/26 10:38, Srinivas Kandagatla wrote:
>>>>
>>>>
>>>> On 6/10/26 8:41 AM, Neil Armstrong wrote:
>>>>> The Qualcom SM8650 based Ayaneo Pocket S2 gaming device has a set
>>>>> of 2 WSA speakers connected on the WSA2 lines.
>>>>>
>>>>> But the Audioreach DSP only handles WSA2 in pair with the WSA
>>>>> interface by using the upper bits of the active_channels_mask
>>>>> for WSA2 and the lower bits for WSA:
>>>>>
>>>>> /-------------------------------------------------\
>>>>> | Bits  |     3    |     2    |   1     |     0   |
>>>>> |-------------------------------------------------|
>>>>> | Line  | WSA2 Ch2 | WSA2 Ch1 | WSA Ch2 | WSA Ch1 |
>>>>> \-------------------------------------------------/
>>>>>
>>>> No, this is not totally correct, if the setup only has WSA2, then
>>>> channel 0 and 1 should be WSA2 channels.
>>>>
>>>> What is the backend dai id that is in DT, it should be
>>>>
>>>>      sound-dai = <&q6apmbedai WSA2_CODEC_DMA_RX_0>;
>>>>
>>>> I also noticed that you are using
>>>> https://github.com/linux-msm/audioreach-topology/blob/main/SM8550-
>>>> HDK.m4
>>>> which has WSA as backend dai, that is not correct, you should have
>>>> WSA2.
>>>
>>> So I did try that, and DSP would error out when using the
>>> LPAIF_INTF_TYPE_WSA2,
>>> but I'm retrying from scratch right now.
>>
>> Please share the failure logs, we need to change
>> 1. dt : bedai id, codec dais with correct soundwire wsa2 instance, the
>> routes.
>> 2. tplg
>>
> 
> So I did all the changes as you suggested:
> 
> Resurected Krzk's serie: https://
> patch.msgid.link/20231019153541.49753-1-krzysztof.kozlowski@linaro.org
> 
> Adapted/Fixes it to apply on v7.1:
> https://gitlab.com/superna9999/linux/-/commit/
> fd8cf1922d10175c5bcd8cf2a444c5825392d994
> https://gitlab.com/superna9999/linux/-/
> commit/0c4e89e167b9ca9c7b500577c030e550ec2a6e73
> https://gitlab.com/superna9999/linux/-/
> commit/6364a0a45a3f0985b872d9f504e9ea1d1f3f2a35
> 
> ```
> +#define WSA2_CODEC_DMA_RX_0    147
> +#define WSA2_CODEC_DMA_TX_0    148
> +#define WSA2_CODEC_DMA_RX_1    149
> +#define WSA2_CODEC_DMA_TX_1    150
> +#define WSA2_CODEC_DMA_TX_2    151
> ```
> 
> https://gitlab.com/superna9999/linux/-/
> commit/9bd0ce21f73df92fb35e3db7ef570f561a106478
> 
> DT:
> https://gitlab.com/superna9999/linux/-/
> commit/2fc270860e3b77ccae28e0c38228cba3e39ea78a
> 
> ```
> -                               sound-dai = <&q6apmbedai
> WSA_CODEC_DMA_RX_0>;
> +                               sound-dai = <&q6apmbedai
> WSA2_CODEC_DMA_RX_0>;
>                         };
> ```
> 
> Topology, copied the SM8550-HDK into a new one, dropped I2S and changed
> all WSA to WSA
> and added the WSA defines:
> https://github.com/superna9999/audioreach-topology/
> commit/12adc76859cde606c67e5a95df204b8d407038df
> 
> 
> ```
> +define(`WSA2_CODEC_DMA_RX_0', `147') dnl
> +define(`WSA2_CODEC_DMA_TX_0', `148') dnl
> +define(`WSA2_CODEC_DMA_RX_1', `149') dnl
> +define(`WSA2_CODEC_DMA_TX_1', `150') dnl
> +define(`WSA2_CODEC_DMA_TX_2', `151') dnl
> ```
> 
> Extract of the SM8650-APS2.m4 concerning WSA2:
> ```
> ...
> +dnl WSA Playback
> +DEVICE_SG_ADD(audioreach/subgraph-device-codec-dma-playback.m4,
> `WSA2_CODEC_DMA_RX_0', WSA2_CODEC_DMA_RX_0,
> +       `S16_LE', 48000, 48000, 2, 2,
> +       LPAIF_INTF_TYPE_WSA2, CODEC_INTF_IDX_RX0, 0,
> DATA_FORMAT_FIXED_POINT,
> +       0x00004006, 0x00004006, 0x00006050)
> +dnl
> ...
> +STREAM_DEVICE_PLAYBACK_MIXER(WSA2_CODEC_DMA_RX_0,
> ``WSA2_CODEC_DMA_RX_0'', ``MultiMedia1'', ``MultiMedia2'', ``MultiMedia5'')
> ...
> +STREAM_DEVICE_PLAYBACK_ROUTE(WSA2_CODEC_DMA_RX_0, ``WSA2_CODEC_DMA_RX_0
> Audio Mixer'', ``MultiMedia1, stream0.logger1'', ``MultiMedia2,
> stream1.logger1'', ``MultiMedia5, stream4.logger1'')
> ...
> ```
> 
> On device, all sets up without errors:
> ```
> [   20.710228] qcom-apm gprsvc:service:2:1: CMD timeout for [1001021]
> opcode
> [   20.720234] platform 6800000.remoteproc:glink-
> edge:gpr:service@1:dais: Adding to iommu group 30
> [   20.763797] va_macro 6d44000.codec: qcom,dmic-sample-rate dt entry
> missing
> [   20.791279] wsa_macro 6aa0000.codec: using zero-initialized flat
> cache, this may cause unexpected behavior
> [   20.912445] wcd939x_codec audio-codec: bound sdw:2:0:0217:010e:00:4
> (ops wcd_sdw_component_ops [snd_soc_wcd_common])
> [   20.923343] wcd939x_codec audio-codec: bound sdw:3:0:0217:010e:00:3
> (ops wcd_sdw_component_ops [snd_soc_wcd_common])
> [   20.960741] snd-sc8280xp sound: ASoC: Parent card not yet available,
> widget card binding deferred
> [   20.972182] va_macro 6d44000.codec: supply vdd-micb not found, using
> dummy regulator
> [   20.985751] ALSA: Control name 'stream0.vol_ctrl0 MultiMedia1
> Playback Volume' truncated to 'stream0.vol_ctrl0 MultiMedia1 Playback Volu'
> [   20.998589] ALSA: Control name 'stream1.vol_ctrl1 MultiMedia2
> Playback Volume' truncated to 'stream1.vol_ctrl1 MultiMedia2 Playback Volu'
> [   21.011536] ALSA: Control name 'stream4.vol_ctrl4 MultiMedia5
> Playback Volume' truncated to 'stream4.vol_ctrl4 MultiMedia5 Playback Volu'
> [   21.026510] input: SM8650-APS2 Headset Jack as /devices/platform/
> sound/sound/card0/input7
> [   21.035151] input: SM8650-APS2 DP0 Jack as /devices/platform/sound/
> sound/card0/input8
> ```
> 
> Available mixer elements:
> ```
> # amixer | grep WSA
> Simple mixer control 'SpkrLeft WSA MODE',0
> Simple mixer control 'SpkrRight WSA MODE',0
> Simple mixer control 'WSA RX0 MUX',0
> Simple mixer control 'WSA RX1 MUX',0
> Simple mixer control 'WSA RX_MIX EC0_MUX',0
> Simple mixer control 'WSA RX_MIX EC1_MUX',0
> Simple mixer control 'WSA RX_MIX0 MUX',0
> Simple mixer control 'WSA RX_MIX1 MUX',0
> Simple mixer control 'WSA2_CODEC_DMA_RX_0 Audio Mixer MultiMedia1',0
> Simple mixer control 'WSA2_CODEC_DMA_RX_0 Audio Mixer MultiMedia2',0
> Simple mixer control 'WSA2_CODEC_DMA_RX_0 Audio Mixer MultiMedia5',0
> Simple mixer control 'WSA_AIF_VI Mixer WSA_SPKR_VI_1',0
> Simple mixer control 'WSA_AIF_VI Mixer WSA_SPKR_VI_2',0
> Simple mixer control 'WSA_COMP1',0
> Simple mixer control 'WSA_COMP2',0
> Simple mixer control 'WSA_RX0 Digital',0
> Simple mixer control 'WSA_RX0 Digital Mute',0
> Simple mixer control 'WSA_RX0 EC_HQ',0
> Simple mixer control 'WSA_RX0 INP0',0
> Simple mixer control 'WSA_RX0 INP1',0
> Simple mixer control 'WSA_RX0 INP2',0
> Simple mixer control 'WSA_RX0 INT0 SIDETONE MIX',0
> Simple mixer control 'WSA_RX0 MIX INP',0
> Simple mixer control 'WSA_RX0_MIX Digital',0
> Simple mixer control 'WSA_RX0_MIX Digital Mute',0
> Simple mixer control 'WSA_RX1 Digital',0
> Simple mixer control 'WSA_RX1 Digital Mute',0
> Simple mixer control 'WSA_RX1 EC_HQ',0
> Simple mixer control 'WSA_RX1 INP0',0
> Simple mixer control 'WSA_RX1 INP1',0
> Simple mixer control 'WSA_RX1 INP2',0
> Simple mixer control 'WSA_RX1 MIX INP',0
> Simple mixer control 'WSA_RX1_MIX Digital',0
> Simple mixer control 'WSA_RX1_MIX Digital Mute',0
> Simple mixer control 'WSA_Softclip0 Enable',0
> Simple mixer control 'WSA_Softclip1 Enable',0
> ```
> 
> I setup the speaker with (no errors):
> ```
> amixer -c 0 cset name='SpkrLeft PA Volume' 20
> amixer -c 0 cset name='SpkrRight PA Volume' 20
> amixer -c 0 cset name='WSA RX0 MUX' AIF1_PB
> amixer -c 0 cset name='WSA RX1 MUX' AIF1_PB
> amixer -c 0 cset name='WSA_RX0 INP0' RX0
> amixer -c 0 cset name='WSA_RX1 INP0' RX1
> amixer -c 0 cset name='SpkrLeft DAC Switch' 1
> amixer -c 0 cset name='SpkrRight DAC Switch' 1
> amixer -c 0 cset name='WSA_RX0 Digital Volume' 85
> amixer -c 0 cset name='WSA_RX1 Digital Volume' 85
> ```
> 
> and finally:
> ```
> amixer -c 0 cset name='WSA2_CODEC_DMA_RX_0 Audio Mixer MultiMedia1' 1
> numid=216,iface=MIXER,name='WSA2_CODEC_DMA_RX_0 Audio Mixer MultiMedia1'
>   ; type=BOOLEAN,access=rw------,values=2
>   : values=on,off
> 
> ```
> 
> When playing sound, it just timeouts, no printed errors:
> ```
> # speaker-test -D plughw:0,0 -c 2
> 
> speaker-test 1.2.14
> 
> Playback device is plughw:0,0
> Stream parameters are 48000Hz, S16_LE, 2 channels
> Using 16 octaves of pink noise
> Rate set to 48000Hz (requested 48000Hz)
> Buffer size range from 960 to 130560
> Period size range from 480 to 16320
> Periods = 4
> was set period_size = 12000
> was set buffer_size = 48000
>  0 - Front Left
> Write error: -5,Input/output error
> xrun_recovery failed: -5,Input/output error
> Transfer failed: Input/output error
> ```

Thanks Neil for trying this out, I had pretty much simillar behavoir on
Glymur device too, My understanding was that there are some codec DMA
instances where we can use 2 channels as default for WSA2.

But on Glymur all the DMA instances are 8 channel wide. Am trying to
check internally to see if its actually doable to get this working with
the ch1 and ch2 mask.

Am working on this to get more details on what it takes to get this working.


Only concern with this patch is that the channel mapping, which does not
reflect the channel mask. can you confirm that in your working setup you
have chmask as 0xC0 and channel_map setup for ch1 and ch2?

From DSP docs:
https://github.com/AudioReach/audioreach-engine/blob/master/fwk/api/modules/media_fmt_api_basic.h#L780

Channel[i] mapping describes channel i. Each element i of the array
describes channel i inside the buffer where i is less than num_channels.
An unused channel is set to 0.


So unused channels should be set to zero, but the patch padding the
channels starting form zero.

-srini


> 
> Neil
> 
>>
>> --srini
>>>
>>> Thanks,
>>> Neil
>>>
>>>>
>>>>
>>>>> Setting only the WSA2 upper bits is perfectly valid and
>>>>> functional but the current Audioreach code builds the bitmask
>>>>> from the channels count with:
>>>>>      active_channels_mask = (1 << num_channels) - 1;
>>>>>
>>>>> In order to enable the WSA2 bits the channel count should be 4,
>>>>> but the lower WSA bits are then also enabled and the DSP errors
>>>>> out when trying to play on the disabled WSA interface.
>>>>>
>>>>> A solution would've been to add a fake WSA2 topology element which
>>>>> would be translated into the top bits only, but it's not clean and
>>>>> add some special exceptions in the generic Audioreach code.
>>>>>
>>>>> The solution suggested by Srinivas is to use the channel mapping to
>>>>> set this bitmask.
>>>>>
>>>>> This works but makes all the other calls using the channel mapping
>>>>> fail
>>>>> because the DSP requires the channel_mapping table to start from
>>>>> index 0
>>>>> and using num_channel length in order to apply the mapping on the
>>>>> active_channels_mask bits in order.
>>>>>
>>>>> So we need to skip the empty channel mapping entries in all other
>>>>> users of the channel_map to build valid channel_mapping tables.
>>>>>
>>>>> This should not break any other usecases since the default channel
>>>>> mapping always start from index 0, and will add flexibilty to allow
>>>>> some special non linear mapping for other interfaces as well.
>>>>>
>>>>> Suggested-by: Srinivas Kandagatla
>>>>> <srinivas.kandagatla@oss.qualcomm.com>
>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>>> ---
>>>>>    sound/soc/qcom/qdsp6/audioreach.c | 47 ++++++++++++++++++++++++++++
>>>>> ++---------
>>>>>    1 file changed, 37 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/
>>>>> qdsp6/audioreach.c
>>>>> index a13f753eff98..9b80cfa56e8a 100644
>>>>> --- a/sound/soc/qcom/qdsp6/audioreach.c
>>>>> +++ b/sound/soc/qcom/qdsp6/audioreach.c
>>>>> @@ -703,6 +703,7 @@ static int
>>>>> audioreach_codec_dma_set_media_format(struct q6apm_graph *graph,
>>>>>        int pm_sz = APM_HW_EP_PMODE_CFG_PSIZE;
>>>>>        int size = ic_sz + ep_sz + fs_sz + pm_sz;
>>>>>        void *p;
>>>>> +    int i;
>>>>>          struct gpr_pkt *pkt __free(kfree) =
>>>>> audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0);
>>>>>        if (IS_ERR(pkt))
>>>>> @@ -741,7 +742,12 @@ static int
>>>>> audioreach_codec_dma_set_media_format(struct q6apm_graph *graph,
>>>>>          intf_cfg->cfg.lpaif_type = module->hw_interface_type;
>>>>>        intf_cfg->cfg.intf_index = module->hw_interface_idx;
>>>>> -    intf_cfg->cfg.active_channels_mask = (1 << cfg->num_channels)
>>>>> - 1;
>>>>> +    intf_cfg->cfg.active_channels_mask = 0;
>>>>> +    /* Convert the physical channel mapping into a bit field */
>>>>> +    for (i = 0; i < AR_PCM_MAX_NUM_CHANNEL; i++)
>>>>> +        if (cfg->channel_map[i])
>>>>> +            intf_cfg->cfg.active_channels_mask |= BIT(i);
>>>>> +
>>>>
>>>> This one looks good, this should be a bug fix patch.
>>>>
>>>>>        p += ic_sz;
>>>>>          pm_cfg = p;
>>>>> @@ -840,7 +846,7 @@ static int audioreach_mfc_set_media_format(struct
>>>>> q6apm_graph *graph,
>>>>>        uint32_t num_channels = cfg->num_channels;
>>>>>        int payload_size = APM_MFC_CFG_PSIZE(media_format,
>>>>> num_channels) +
>>>>>                    APM_MODULE_PARAM_DATA_SIZE;
>>>>> -    int i;
>>>>> +    int i, j;
>>>>>        void *p;
>>>>>          struct gpr_pkt *pkt __free(kfree) =
>>>>> audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0);
>>>>> @@ -860,8 +866,12 @@ static int
>>>>> audioreach_mfc_set_media_format(struct q6apm_graph *graph,
>>>>>        media_format->sample_rate = cfg->sample_rate;
>>>>>        media_format->bit_width = cfg->bit_width;
>>>>>        media_format->num_channels = cfg->num_channels;
>>>>> -    for (i = 0; i < num_channels; i++)
>>>>> -        media_format->channel_mapping[i] = cfg->channel_map[i];
>>>>> +    /* Convert the physical mapping to a logical mapping of the
>>>>> channels */
>>>>> +    for (i = 0, j = 0; i < AR_PCM_MAX_NUM_CHANNEL && j < cfg-
>>>>>> num_channels; i++) {
>>>>> +        if (!cfg->channel_map[i])
>>>>> +            continue;
>>>>> +        media_format->channel_mapping[j++] = cfg->channel_map[i];
>>>> Each element i of the channel_mapping[i] array, describes the channel i
>>>> inside the buffer where i is less than num_channels.  An unused channel
>>>> is set to 0.
>>>>
>>>> For some reason I get impression that user is trying to set a 4
>>>> channels
>>>> instead of 2 channel.
>>>>
>>>> Can you fix the backend-dai id and play it directly on WSA2 instead of
>>>> WSA.
>>>> Or was there a reason for not doing it otherwise?
>>>>
>>>> --srini
>>>>
>>>>> +    }
>>>>>          return q6apm_send_cmd_sync(graph->apm, pkt, 0);
>>>>>    }
>>>>> @@ -1080,6 +1090,7 @@ static int
>>>>> audioreach_pcm_set_media_format(struct q6apm_graph *graph,
>>>>>        struct apm_pcm_module_media_fmt_cmd *cfg;
>>>>>        struct apm_module_param_data *param_data;
>>>>>        int payload_size;
>>>>> +    int i, j;
>>>>>          if (num_channels > 4) {
>>>>>            dev_err(graph->dev, "Error: Invalid channels (%d)!\n",
>>>>> num_channels);
>>>>> @@ -1113,7 +1124,12 @@ static int
>>>>> audioreach_pcm_set_media_format(struct q6apm_graph *graph,
>>>>>        media_cfg->num_channels = mcfg->num_channels;
>>>>>        media_cfg->q_factor = mcfg->bit_width - 1;
>>>>>        media_cfg->bits_per_sample = mcfg->bit_width;
>>>>> -    memcpy(media_cfg->channel_mapping, mcfg->channel_map, mcfg-
>>>>>> num_channels);
>>>>> +    /* Convert the physical mapping to a logical mapping of the
>>>>> channels */
>>>>> +    for (i = 0, j = 0; i < AR_PCM_MAX_NUM_CHANNEL && j < mcfg-
>>>>>> num_channels; i++) {
>>>>> +        if (!mcfg->channel_map[i])
>>>>> +            continue;
>>>>> +        media_cfg->channel_mapping[j++] = mcfg->channel_map[i];
>>>>> +    }
>>>>>          return q6apm_send_cmd_sync(graph->apm, pkt, 0);
>>>>>    }
>>>>> @@ -1127,6 +1143,7 @@ static int
>>>>> audioreach_shmem_set_media_format(struct q6apm_graph *graph,
>>>>>        struct payload_media_fmt_pcm *cfg;
>>>>>        struct media_format *header;
>>>>>        int rc, payload_size;
>>>>> +    int i, j;
>>>>>        void *p;
>>>>>          if (num_channels > 4) {
>>>>> @@ -1166,7 +1183,12 @@ static int
>>>>> audioreach_shmem_set_media_format(struct q6apm_graph *graph,
>>>>>            cfg->q_factor = mcfg->bit_width - 1;
>>>>>            cfg->endianness = PCM_LITTLE_ENDIAN;
>>>>>            cfg->num_channels = mcfg->num_channels;
>>>>> -        memcpy(cfg->channel_mapping, mcfg->channel_map, mcfg-
>>>>>> num_channels);
>>>>> +        /* Convert the physical mapping to a logical mapping of the
>>>>> channels */
>>>>> +        for (i = 0, j = 0; i < AR_PCM_MAX_NUM_CHANNEL && j < cfg-
>>>>>> num_channels; i++) {
>>>>> +            if (!mcfg->channel_map[i])
>>>>> +                continue;
>>>>> +            cfg->channel_mapping[j++] = mcfg->channel_map[i];
>>>>> +        }
>>>>>        } else {
>>>>>            rc = audioreach_set_compr_media_format(header, p, mcfg);
>>>>>            if (rc)
>>>>> @@ -1243,7 +1265,7 @@ static int
>>>>> audioreach_speaker_protection_vi(struct q6apm_graph *graph,
>>>>>        struct apm_module_sp_vi_ex_mode_cfg *ex_cfg;
>>>>>        int op_sz, cm_sz, ex_sz;
>>>>>        struct apm_module_param_data *param_data;
>>>>> -    int rc, i, payload_size;
>>>>> +    int rc, i, payload_size, j;
>>>>>        struct gpr_pkt *pkt;
>>>>>        void *p;
>>>>>    @@ -1284,14 +1306,19 @@ static int
>>>>> audioreach_speaker_protection_vi(struct q6apm_graph *graph,
>>>>>        param_data->param_size = cm_sz - APM_MODULE_PARAM_DATA_SIZE;
>>>>>          cm_cfg->cfg.num_channels = num_channels * 2;
>>>>> -    for (i = 0; i < num_channels; i++) {
>>>>> +    /* Convert the physical mapping to a logical mapping of the
>>>>> channels */
>>>>> +    for (i = 0, j = 0; i < AR_PCM_MAX_NUM_CHANNEL && j <
>>>>> num_channels; i++) {
>>>>> +        if (!mcfg->channel_map[i])
>>>>> +            continue;
>>>>>            /*
>>>>>             * Map speakers into Vsense and then Isense of each
>>>>> channel.
>>>>>             * E.g. for PCM_CHANNEL_FL and PCM_CHANNEL_FR to:
>>>>>             * [1, 2, 3, 4]
>>>>>             */
>>>>> -        cm_cfg->cfg.channel_mapping[2 * i] = (mcfg->channel_map[i] -
>>>>> 1) * 2 + 1;
>>>>> -        cm_cfg->cfg.channel_mapping[2 * i + 1] = (mcfg-
>>>>>> channel_map[i] - 1) * 2 + 2;
>>>>> +        cm_cfg->cfg.channel_mapping[2 * j] = (mcfg->channel_map[i] -
>>>>> 1) * 2 + 1;
>>>>> +        cm_cfg->cfg.channel_mapping[2 * j + 1] = (mcfg-
>>>>>> channel_map[i] - 1) * 2 + 2;
>>>>> +
>>>>> +        ++j;
>>>>>        }
>>>>>          p += cm_sz;
>>>>>
>>>>
>>>
>>
>