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

[Prasad Kumpatla]

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@xxxxxxxxxxxxxxxx>
> > ---
> >   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;
> > +}