Re: [PATCH v1 1/6] ASoC: qcom: qdsp6: add topology-driven Audio IF support

[Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>]

On 6/15/2026 2:58 PM, Srinivas Kandagatla wrote:
>
> On 6/10/26 4:45 PM, Prasad Kumpatla wrote:
>> Add topology parsing and media-format programming for Audio IF
>> source and sink modules.
>>
>> Add the Audio IF module IDs, the required topology tokens, and a
>> dedicated topology loader that stores the parsed interface
>> configuration in the AudioReach module state. Also add the Audio IF
>> media-format path that sends the interface configuration, hardware
>> endpoint media format, and frame-duration parameters for Audio IF
>> modules.
>>
>> This keeps the serial-interface configuration topology-driven while
>> still allowing the machine driver to provide runtime slot and media
>> format settings. The same Audio IF path can then be reused for TDM,
>> PCM, and I2S style backends.
>>
>> The new UAPI tokens (AR_TKN_U32_MODULE_SYNC_SRC=262 through
>> AR_TKN_U32_MODULE_INV_EXT_BIT_CLK=276) are added.
>>
>> MODULE_ID_AUDIO_IF_SINK (0x0700117C) and MODULE_ID_AUDIO_IF_SOURCE
>> (0x0700117D) are introduced in this patch.
>>
> Which platform is this tested on, also please send a PR to
> github.com/linux-msm/audioreach-topology to add thse new tokens.

Hi Srini,

Thanks for reviewing and comments.

this module is validated on Hawi and Shikra. Sure will raise the PR with 
these tokens usage.

>
>> Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
>> ---
>>   include/uapi/sound/snd_ar_tokens.h |  58 ++++++++++++++++
>>   sound/soc/qcom/qdsp6/audioreach.c  |  97 ++++++++++++++++++++++++++
>>   sound/soc/qcom/qdsp6/audioreach.h  |  62 +++++++++++++++++
>>   sound/soc/qcom/qdsp6/topology.c    | 108 +++++++++++++++++++++++++++++
>>   4 files changed, 325 insertions(+)
>>
>> diff --git a/include/uapi/sound/snd_ar_tokens.h b/include/uapi/sound/snd_ar_tokens.h
>> index 6b8102eaa..355a1e629 100644
>> --- a/include/uapi/sound/snd_ar_tokens.h
>> +++ b/include/uapi/sound/snd_ar_tokens.h
>> @@ -168,6 +168,48 @@ enum ar_event_types {
>>    *						LOG_WAIT = 0,
>>    *						LOG_IMMEDIATELY = 1
>>    *
>> + * %AR_TKN_U32_MODULE_SYNC_SRC:			Frame sync source
>> + *						0 = external, 1 = internal
>> + *
>> + * %AR_TKN_U32_MODULE_CTRL_DATA_OUT_ENABLE:	Enable data-out tri-state control
>> + *						0 = disable, 1 = enable
>> + *
>> + * %AR_TKN_U32_MODULE_SLOT_MASK:			Active TDM slot bitmask
>> + *
>> + * %AR_TKN_U32_MODULE_NSLOTS_PER_FRAME:		Number of slots per TDM frame
>> + *
>> + * %AR_TKN_U32_MODULE_SLOT_WIDTH:			Slot width in bits (16 or 32)
>> + *
>> + * %AR_TKN_U32_MODULE_SYNC_MODE:			Frame sync mode
>> + *						0 = short pulse, 1 = long pulse
> We have 3 possible values, please correct this, also you could add
> defines for these values.
Ack, will update
>> + *
>> + * %AR_TKN_U32_MODULE_CTRL_INVERT_SYNC_PULSE:	Invert frame sync pulse polarity
>> + *						0 = normal, 1 = inverted
>> + *
>> + * %AR_TKN_U32_MODULE_CTRL_SYNC_DATA_DELAY:	Data delay relative to frame sync
>> + *						0 = no delay, 1 = one cycle delay
> Exactly same here, we have 2 cyle delay too.
Ack, will update
>> + *
>> + * %AR_TKN_U32_MODULE_INTF_MODE:			Audio IF interface mode
>> + *						AUDIO_IF_INTF_MODE_TDM = 0,
>> + *						AUDIO_IF_INTF_MODE_PCM = 1,
>> + *						AUDIO_IF_INTF_MODE_I2S = 2
> Same here, defines for these.
Ack, will update
>
>> + *
>> + * %AR_TKN_U32_MODULE_QAIF_TYPE:			QAIF hardware port type index
>> + *
>> + * %AR_TKN_U32_MODULE_ACTIVE_LANE_MASK:		Active lane bitmask for multi-lane
>> + *
>> + * %AR_TKN_U32_MODULE_FRAME_SYNC_RATE:		Frame sync rate in Hz
>> + *
>> + * %AR_TKN_U32_MODULE_BIT_CLK_TYPE:			Bit clock type
>> + *						0 = internal, 1 = external,
>> + *						2 = skip (bypass bit clock enable)
>> + *
>> + * %AR_TKN_U32_MODULE_INV_INT_BIT_CLK:		Invert internal bit clock
>> + *						0 = normal, 1 = inverted
>> + *
>> + * %AR_TKN_U32_MODULE_INV_EXT_BIT_CLK:		Invert external bit clock
>> + *						0 = normal, 1 = inverted
>> + *
>>    * %AR_TKN_DAI_INDEX:				dai index
>>    *
>>    */
>> @@ -240,6 +282,22 @@ enum ar_event_types {
>>   #define AR_TKN_U32_MODULE_LOG_TAP_POINT_ID	260
>>   #define AR_TKN_U32_MODULE_LOG_MODE		261
>>   
>> +#define AR_TKN_U32_MODULE_SYNC_SRC		262
>> +#define AR_TKN_U32_MODULE_CTRL_DATA_OUT_ENABLE	263
>> +#define AR_TKN_U32_MODULE_SLOT_MASK		264
>> +#define AR_TKN_U32_MODULE_NSLOTS_PER_FRAME	265
>> +#define AR_TKN_U32_MODULE_SLOT_WIDTH		266
>> +#define AR_TKN_U32_MODULE_SYNC_MODE		267
>> +#define AR_TKN_U32_MODULE_CTRL_INVERT_SYNC_PULSE	268
>> +#define AR_TKN_U32_MODULE_CTRL_SYNC_DATA_DELAY	269
>> +#define AR_TKN_U32_MODULE_INTF_MODE		270
>> +#define AR_TKN_U32_MODULE_QAIF_TYPE		271
>> +#define AR_TKN_U32_MODULE_ACTIVE_LANE_MASK	272
>> +#define AR_TKN_U32_MODULE_FRAME_SYNC_RATE	273
>> +#define AR_TKN_U32_MODULE_BIT_CLK_TYPE		274
>> +#define AR_TKN_U32_MODULE_INV_INT_BIT_CLK	275
>> +#define AR_TKN_U32_MODULE_INV_EXT_BIT_CLK	276
>> +
> Here you prefix the tokens with U32, however in dirver this values are
> validated against U8 and U16, So please fix the prefixes to reflect the
> range.
>
>
> ...
Yes that's correct, will update them properly.
>
>>   
>>   	default:
>>   		rc = 0;
>> diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
>> index 62a2fd79b..1dc29ddfd 100644
>> --- a/sound/soc/qcom/qdsp6/audioreach.h
>> +++ b/sound/soc/qcom/qdsp6/audioreach.h
>> @@ -22,6 +22,8 @@ struct q6apm_graph;
>>   #define MODULE_ID_PLACEHOLDER_DECODER	0x07001009
>>   #define MODULE_ID_I2S_SINK		0x0700100A
>>   #define MODULE_ID_I2S_SOURCE		0x0700100B
>> +#define MODULE_ID_AUDIO_IF_SINK		0x0700117C
>> +#define MODULE_ID_AUDIO_IF_SOURCE	0x0700117D
> Please place it in the assending order.
Ack, will update.
>
>>   #define MODULE_ID_SAL			0x07001010
>>   #define MODULE_ID_MFC			0x07001015
>>   #define MODULE_ID_DATA_LOGGING		0x0700101A
>> @@ -544,6 +546,41 @@ struct param_id_i2s_intf_cfg {
>>   #define PORT_ID_I2S_OUPUT		1
>>   #define I2S_STACK_SIZE			2048
>>   
>> +#define PARAM_ID_AUDIO_IF_INTF_CFG	0x08001B11
>> +
>> +#define AUDIO_IF_INTF_MODE_TDM		0x0
>> +#define AUDIO_IF_INTF_MODE_PCM		0x1
>> +#define AUDIO_IF_INTF_MODE_I2S		0x2
>> +
>> +struct param_id_audio_if_intf_cfg {
> I know that we have not added documentation for all the structures, but
> Am in process of adding them. Can you add kernel doc to these structs.
Ack, will update
>> +	uint16_t qaif_type;
>> +	uint16_t intf_idx;
>> +	uint16_t intf_mode;
>> +	uint16_t ctrl_data_out_enable;
>> +	uint32_t active_slot_mask;
>> +	uint16_t nslots_per_frame;
>> +	uint16_t slot_width;
>> +	uint32_t active_lane_mask;
>> +	uint32_t frame_sync_rate;
>> +	uint16_t frame_sync_src;
>> +	uint16_t frame_sync_mode;
>> +	uint16_t invert_frame_sync_pulse;
>> +	uint16_t frame_sync_data_delay;
>> +	uint16_t bit_clk_type;
>> +	uint8_t inv_int_bit_clk;
>> +	uint8_t inv_ext_bit_clk;
>> +} __packed;
>> +
>> +#define PARAM_ID_HW_EP_FRAME_DURATION		0x08001B2F
>> +#define AUDIO_IF_FRAME_DURATION_US		1000
> Why is this hardcoded?

As this module End point operate with a 1 ms processing interval.

>> +
>> +struct param_id_hw_ep_frame_duration {
>> +	uint32_t frame_duration_in_us;
>> +	uint32_t allow_frame_duration_normalization;
>> +	uint32_t min_normalized_frame_dur_us;
>> +	uint32_t max_normalized_frame_dur_us;
>> +} __packed;
>> +
>>   #define PARAM_ID_DISPLAY_PORT_INTF_CFG		0x08001154
>>   
>>   struct param_id_display_port_intf_cfg {
>> @@ -877,6 +914,28 @@ struct audioreach_module {
>>   	uint32_t data_format;
>>   	uint32_t hw_interface_type;
>>   
>> +	/* Audio IF module (TDM/PCM/I2S) */
>> +	/*
>> +	 * uint32_t fields first to minimise intra-block padding;
> Why do we need this comments does not add a real value here?
No much, will remove them.
>> +	 * 2 bytes of trailing padding remain after inv_ext_bit_clk
>> +	 * before the next uint32_t field (interleave_type).
>> +	 */
>> +	uint32_t slot_mask;
>> +	uint32_t active_lane_mask;
>> +	uint32_t frame_sync_rate;
>> +	uint16_t qaif_type;
>> +	uint16_t sync_src;
>> +	uint16_t ctrl_data_out_enable;
>> +	uint16_t nslots_per_frame;
>> +	uint16_t slot_width;
>> +	uint16_t intf_mode;
>> +	uint16_t sync_mode;
>> +	uint16_t ctrl_invert_sync_pulse;
>> +	uint16_t ctrl_sync_data_delay;
>> +	uint16_t bit_clk_type;
>> +	uint8_t inv_int_bit_clk;
>> +	uint8_t inv_ext_bit_clk;
>> +
>>   	/* PCM module specific */
>>   	uint32_t interleave_type;
>>   
>> @@ -907,6 +966,9 @@ struct audioreach_module_config {
>>   	u32	channel_allocation;
>>   	u32	sd_line_mask;
>>   	int	fmt;
>> +	u32	slot_mask;
>> +	u16	nslots_per_frame;
>> +	u16	slot_width;
>>   	struct snd_codec codec;
>>   	u8 channel_map[AR_PCM_MAX_NUM_CHANNEL];
>>   };
>> diff --git a/sound/soc/qcom/qdsp6/topology.c b/sound/soc/qcom/qdsp6/topology.c
>> index 1f69fba6d..2ae7ac3d2 100644
>> --- a/sound/soc/qcom/qdsp6/topology.c
>> +++ b/sound/soc/qcom/qdsp6/topology.c
>> @@ -753,6 +753,108 @@ static int audioreach_widget_i2s_module_load(struct audioreach_module *mod,
>>   	return 0;
>>   }
>>   
>> +static int audioreach_widget_audio_if_module_load(struct audioreach_module *mod,
>> +						  const struct snd_soc_tplg_vendor_array *mod_array)
>> +{
>> +	const struct snd_soc_tplg_vendor_value_elem *mod_elem;
>> +	int tkn_count = 0;
>> +	u32 val;
>> +
>> +	mod_elem = mod_array->value;
>> +
>> +	while (tkn_count < le32_to_cpu(mod_array->num_elems)) {
>> +		val = le32_to_cpu(mod_elem->value);
>> +		switch (le32_to_cpu(mod_elem->token)) {
>> +		case AR_TKN_U32_MODULE_HW_IF_IDX:
>> +			if (val > U16_MAX)
>> +				return -EINVAL;
> Plese fix such instances as suggested at the top.
Ack, will update
>
>> +			mod->hw_interface_idx = val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_FMT_DATA:
>> +			mod->data_format = val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_HW_IF_TYPE:
>> +			mod->hw_interface_type = val;
> where are we using this?

Its not being used any where, i will remove this.

Thanks,
Prasad

>
>> +			break;
>> +		case AR_TKN_U32_MODULE_SYNC_SRC:
>> +			if (val > U16_MAX)
>> +				return -EINVAL;
>> +			mod->sync_src = (u16)val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_CTRL_DATA_OUT_ENABLE:
>> +			if (val > U16_MAX)
>> +				return -EINVAL;
>> +			mod->ctrl_data_out_enable = (u16)val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_SLOT_MASK:
>> +			mod->slot_mask = val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_NSLOTS_PER_FRAME:
>> +			if (val > U16_MAX)
>> +				return -EINVAL;
>> +			mod->nslots_per_frame = (u16)val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_SLOT_WIDTH:
>> +			if (val > U16_MAX)
>> +				return -EINVAL;
>> +			mod->slot_width = (u16)val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_INTF_MODE:
>> +			if (val > U16_MAX)
>> +				return -EINVAL;
>> +			mod->intf_mode = (u16)val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_SYNC_MODE:
>> +			if (val > U16_MAX)
>> +				return -EINVAL;
>> +			mod->sync_mode = (u16)val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_CTRL_INVERT_SYNC_PULSE:
>> +			if (val > U16_MAX)
>> +				return -EINVAL;
>> +			mod->ctrl_invert_sync_pulse = (u16)val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_CTRL_SYNC_DATA_DELAY:
>> +			if (val > U16_MAX)
>> +				return -EINVAL;
>> +			mod->ctrl_sync_data_delay = (u16)val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_QAIF_TYPE:
>> +			if (val > U16_MAX)
>> +				return -EINVAL;
>> +			mod->qaif_type = (u16)val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_ACTIVE_LANE_MASK:
>> +			mod->active_lane_mask = val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_FRAME_SYNC_RATE:
>> +			mod->frame_sync_rate = val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_BIT_CLK_TYPE:
>> +			if (val > U16_MAX)
>> +				return -EINVAL;
>> +			mod->bit_clk_type = (u16)val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_INV_INT_BIT_CLK:
>> +			if (val > U8_MAX)
>> +				return -EINVAL;
>> +			mod->inv_int_bit_clk = (u8)val;
>> +			break;
>> +		case AR_TKN_U32_MODULE_INV_EXT_BIT_CLK:
>> +			if (val > U8_MAX)
>> +				return -EINVAL;
>> +			mod->inv_ext_bit_clk = (u8)val;
>> +			break;
>> +		default:
>> +			break;
>> +		}
>> +		tkn_count++;
>> +		mod_elem++;
>> +	}
>> +
>> +	return 0;
>> +}