Skip to contentSkip to conversation

ASoC: Intel: Skylake: Fix and support complex

PATCH In review Cezary Rojewski 3 participants 14 messages Aug 18, 2021 – Aug 23, 2021

Subsystems (3)
lore
Share & tools
lore

Cezary Rojewski

author maintainer
#0 lore raw

cover ASoC: Intel: Skylake: Fix and support complex

Existing skylake-driver supports very basic scenarios with limited range of modules and their control. Attached changes first fix code as several advanced configurations are 'mentioned' throughout the files but are not actually functional. Follow up are changes adding missing support for said configurations.

Cezary Rojewski (5):
ASoC: Intel: kbl_da7219_max98927: Fix format selection for max98373
ASoC: Intel: Skylake: Leave data as is when invoking TLV IPCs
ASoC: Intel: Skylake: Fix module resource and format selection
ASoC: Intel: Skylake: Fix module configuration for KPB and MIXER
ASoC: Intel: Skylake: Select first entry for singular pipe config
arrays

Gustaw Lewandowski (2):
ASoC: Intel: Skylake: Fix passing loadable flag for module
ASoC: Intel: Skylake: Simplify m_state for loadable modules

Kareem Shaik (1):
ASoC: Intel: Skylake: Support multiple format configs

Pawel Harlozinski (1):
ASoC: Intel: Skylake: Properly configure modules with generic
extension

Piotr Maziarz (1):
ASoC: Intel: Skylake: Select proper format for NHLT blob

Szymon Mielczarek (1):
ASoC: Intel: Skylake: Support modules with generic extension

 include/uapi/sound/snd_sst_tokens.h          |   6 +-
 sound/soc/intel/boards/kbl_da7219_max98927.c |  55 +------
 sound/soc/intel/skylake/skl-messages.c       | 155 ++++++++++++-------
 sound/soc/intel/skylake/skl-pcm.c            |  25 ++-
 sound/soc/intel/skylake/skl-topology.c       | 155 +++++++++++--------
 sound/soc/intel/skylake/skl-topology.h       |  26 +++-
 6 files changed, 231 insertions(+), 191 deletions(-)
Signature · 1 line
2.25.1
author maintainer
#1 lore raw

PATCH 1/11 ASoC: Intel: kbl_da7219_max98927: Fix format selection for max98373

Contrary to what is said in board's file, topology targeting kbl_da7219_max98373 expects format 16b, not 24/32b. Partially revert changes added in 'ASoC: Intel: Boards: Add Maxim98373 support' to bring old behavior back, aligning with topology expectations.

Fixes 716d53cc7837 ("ASoC: Intel: Boards: Add Maxim98373 support")
---
 sound/soc/intel/boards/kbl_da7219_max98927.c | 55 +++-----------------
 1 file changed, 7 insertions(+), 48 deletions(-)
sound/soc/intel/boards/kbl_da7219_max98927.c +7 -48
index a31a7a7bbf66..2b43459adc33 100644
@@ -199,7 +199,7 @@ static int kabylake_ssp0_hw_params(struct snd_pcm_substream *substream,
 		}
 		if (!strcmp(codec_dai->component->name, MAX98373_DEV0_NAME)) {
 			ret = snd_soc_dai_set_tdm_slot(codec_dai,
-							0x03, 3, 8, 24);
+							0x30, 3, 8, 16);
 			if (ret < 0) {
 				dev_err(runtime->dev,
 						"DEV0 TDM slot err:%d\n", ret);
@@ -208,10 +208,10 @@ static int kabylake_ssp0_hw_params(struct snd_pcm_substream *substream,
 		}
 		if (!strcmp(codec_dai->component->name, MAX98373_DEV1_NAME)) {
 			ret = snd_soc_dai_set_tdm_slot(codec_dai,
-							0x0C, 3, 8, 24);
+							0xC0, 3, 8, 16);
 			if (ret < 0) {
 				dev_err(runtime->dev,
-						"DEV0 TDM slot err:%d\n", ret);
+						"DEV1 TDM slot err:%d\n", ret);
 				return ret;
 			}
 		}
@@ -311,24 +311,6 @@ static int kabylake_ssp_fixup(struct snd_soc_pcm_runtime *rtd,
 	 * The above 2 loops are mutually exclusive based on the stream direction,
 	 * thus rtd_dpcm variable will never be overwritten
 	 */
-	/*
-	 * Topology for kblda7219m98373 & kblmax98373 supports only S24_LE,
-	 * where as kblda7219m98927 & kblmax98927 supports S16_LE by default.
-	 * Skipping the port wise FE and BE configuration for kblda7219m98373 &
-	 * kblmax98373 as the topology (FE & BE) supports S24_LE only.
-	 */
-
-	if (!strcmp(rtd->card->name, "kblda7219m98373") ||
-		!strcmp(rtd->card->name, "kblmax98373")) {
-		/* The ADSP will convert the FE rate to 48k, stereo */
-		rate->min = rate->max = 48000;
-		chan->min = chan->max = DUAL_CHANNEL;
-
-		/* set SSP to 24 bit */
-		snd_mask_none(fmt);
-		snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S24_LE);
-		return 0;
-	}
 
 	/*
 	 * The ADSP will convert the FE rate to 48k, stereo, 24 bit
@@ -479,31 +461,20 @@ static struct snd_pcm_hw_constraint_list constraints_channels_quad = {
 static int kbl_fe_startup(struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct snd_soc_pcm_runtime *soc_rt = asoc_substream_to_rtd(substream);
 
 	/*
 	 * On this platform for PCM device we support,
 	 * 48Khz
 	 * stereo
+	 * 16 bit audio
 	 */
 
 	runtime->hw.channels_max = DUAL_CHANNEL;
 	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
 					   &constraints_channels);
-	/*
-	 * Setup S24_LE (32 bit container and 24 bit valid data) for
-	 * kblda7219m98373 & kblmax98373. For kblda7219m98927 &
-	 * kblmax98927 keeping it as 16/16 due to topology FW dependency.
-	 */
-	if (!strcmp(soc_rt->card->name, "kblda7219m98373") ||
-		!strcmp(soc_rt->card->name, "kblmax98373")) {
-		runtime->hw.formats = SNDRV_PCM_FMTBIT_S24_LE;
-		snd_pcm_hw_constraint_msbits(runtime, 0, 32, 24);
-
-	} else {
-		runtime->hw.formats = SNDRV_PCM_FMTBIT_S16_LE;
-		snd_pcm_hw_constraint_msbits(runtime, 0, 16, 16);
-	}
+
+	runtime->hw.formats = SNDRV_PCM_FMTBIT_S16_LE;
+	snd_pcm_hw_constraint_msbits(runtime, 0, 16, 16);
 
 	snd_pcm_hw_constraint_list(runtime, 0,
 				SNDRV_PCM_HW_PARAM_RATE, &constraints_rates);
@@ -536,23 +507,11 @@ static int kabylake_dmic_fixup(struct snd_soc_pcm_runtime *rtd,
 static int kabylake_dmic_startup(struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct snd_soc_pcm_runtime *soc_rt = asoc_substream_to_rtd(substream);
 
 	runtime->hw.channels_min = runtime->hw.channels_max = QUAD_CHANNEL;
 	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
 			&constraints_channels_quad);
 
-	/*
-	 * Topology for kblda7219m98373 & kblmax98373 supports only S24_LE.
-	 * The DMIC also configured for S24_LE. Forcing the DMIC format to
-	 * S24_LE due to the topology FW dependency.
-	 */
-	if (!strcmp(soc_rt->card->name, "kblda7219m98373") ||
-		!strcmp(soc_rt->card->name, "kblmax98373")) {
-		runtime->hw.formats = SNDRV_PCM_FMTBIT_S24_LE;
-		snd_pcm_hw_constraint_msbits(runtime, 0, 32, 24);
-	}
-
 	return snd_pcm_hw_constraint_list(substream->runtime, 0,
 			SNDRV_PCM_HW_PARAM_RATE, &constraints_rates);
 }
Signature · 1 line
2.25.1
author maintainer
#2 lore raw

PATCH 2/11 ASoC: Intel: Skylake: Leave data as is when invoking TLV IPCs

In reply to Cezary Rojewski

Advancing pointer initially fixed issue for some users but caused regression for others. Leave data as it to make it easier for end users to adjust their topology files if needed.

Fixes a8cd7066f042 ("ASoC: Intel: Skylake: Strip T and L from TLV IPCs")
---
 sound/soc/intel/skylake/skl-topology.c | 6 ------
 1 file changed, 6 deletions(-)
sound/soc/intel/skylake/skl-topology.c +0 -6
index c0fdab39e7c2..45b1521e6189 100644
@@ -1463,12 +1463,6 @@ static int skl_tplg_tlv_control_set(struct snd_kcontrol *kcontrol,
 	struct skl_dev *skl = get_skl_ctx(w->dapm->dev);
 
 	if (ac->params) {
-		/*
-		 * Widget data is expected to be stripped of T and L
-		 */
-		size -= 2 * sizeof(unsigned int);
-		data += 2;
-
 		if (size > ac->max)
 			return -EINVAL;
 		ac->size = size;
Signature · 1 line
2.25.1
author maintainer
#3 lore raw

PATCH 3/11 ASoC: Intel: Skylake: Select proper format for NHLT blob

In reply to Cezary Rojewski

From: Piotr Maziarz <[email protected]>

Use actual pipeline format, not PCM format for blob selection. Otherwise selected blobs are not correct in pipelines with format conversion e.g.: SRC module.

---
 sound/soc/intel/skylake/skl-topology.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
sound/soc/intel/skylake/skl-topology.c +13 -8
index 45b1521e6189..054c080b7289 100644
@@ -1821,7 +1821,7 @@ static u8 skl_tplg_be_link_type(int dev_type)
  * Fill the BE gateway parameters
  * The BE gateway expects a blob of parameters which are kept in the ACPI
  * NHLT blob, so query the blob for interface type (i2s/pdm) and instance.
- * The port can have multiple settings so pick based on the PCM
+ * The port can have multiple settings so pick based on the pipeline
  * parameters
  */
 static int skl_tplg_be_fill_pipe_params(struct snd_soc_dai *dai,
@@ -1829,6 +1829,8 @@ static int skl_tplg_be_fill_pipe_params(struct snd_soc_dai *dai,
 				struct skl_pipe_params *params)
 {
 	struct nhlt_specific_cfg *cfg;
+	struct skl_pipe *pipe = mconfig->pipe;
+	struct skl_pipe_fmt *pipe_fmt;
 	struct skl_dev *skl = get_skl_ctx(dai->dev);
 	int link_type = skl_tplg_be_link_type(mconfig->dev_type);
 	u8 dev_type = skl_tplg_be_dev_type(mconfig->dev_type);
@@ -1838,20 +1840,23 @@ static int skl_tplg_be_fill_pipe_params(struct snd_soc_dai *dai,
 	if (link_type == NHLT_LINK_HDA)
 		return 0;
 
+	if (pipe->direction == SNDRV_PCM_STREAM_PLAYBACK)
+		pipe_fmt = &pipe->configs[pipe->pipe_config_idx].out_fmt;
+	else
+		pipe_fmt = &pipe->configs[pipe->pipe_config_idx].in_fmt;
+
 	/* update the blob based on virtual bus_id*/
 	cfg = skl_get_ep_blob(skl, mconfig->vbus_id, link_type,
-					params->s_fmt, params->ch,
-					params->s_freq, params->stream,
+					pipe_fmt->bps, pipe_fmt->channels,
+					pipe_fmt->freq, pipe->direction,
 					dev_type);
 	if (cfg) {
 		mconfig->formats_config.caps_size = cfg->size;
 		mconfig->formats_config.caps = (u32 *) &cfg->caps;
 	} else {
-		dev_err(dai->dev, "Blob NULL for id %x type %d dirn %d\n",
-					mconfig->vbus_id, link_type,
-					params->stream);
-		dev_err(dai->dev, "PCM: ch %d, freq %d, fmt %d\n",
-				 params->ch, params->s_freq, params->s_fmt);
+		dev_err(dai->dev, "Blob NULL for id:%d type:%d dirn:%d ch:%d, freq:%d, fmt:%d\n",
+			mconfig->vbus_id, link_type, params->stream,
+			params->ch, params->s_freq, params->s_fmt);
 		return -EINVAL;
 	}
 
Signature · 1 line
2.25.1
author maintainer
#4 lore raw

PATCH 4/11 ASoC: Intel: Skylake: Fix module resource and format selection

In reply to Cezary Rojewski

Module configuration may differ between its instances depending on resources required and input and output audio format. Available parameters to select from are stored in module resource and interface (format) lists. These come from topology, together with description of each of pipe's modules.

Ignoring index value provided by topology and relying always on 0th entry leads to unexpected module behavior due to under/overbudged resources assigned or impropper format selection. Fix by taking entry at index specified by topology.

Fixes f6fa56e22559 ("ASoC: Intel: Skylake: Parse and update module config structure")
---
 sound/soc/intel/skylake/skl-topology.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)
sound/soc/intel/skylake/skl-topology.c +10 -9
index 054c080b7289..a35ee9d47d6d 100644
@@ -113,7 +113,7 @@ static int is_skl_dsp_widget_type(struct snd_soc_dapm_widget *w,
 
 static void skl_dump_mconfig(struct skl_dev *skl, struct skl_module_cfg *mcfg)
 {
-	struct skl_module_iface *iface = &mcfg->module->formats[0];
+	struct skl_module_iface *iface = &mcfg->module->formats[mcfg->fmt_idx];
 
 	dev_dbg(skl->dev, "Dumping config\n");
 	dev_dbg(skl->dev, "Input Format:\n");
@@ -195,8 +195,8 @@ static void skl_tplg_update_params_fixup(struct skl_module_cfg *m_cfg,
 	struct skl_module_fmt *in_fmt, *out_fmt;
 
 	/* Fixups will be applied to pin 0 only */
-	in_fmt = &m_cfg->module->formats[0].inputs[0].fmt;
-	out_fmt = &m_cfg->module->formats[0].outputs[0].fmt;
+	in_fmt = &m_cfg->module->formats[m_cfg->fmt_idx].inputs[0].fmt;
+	out_fmt = &m_cfg->module->formats[m_cfg->fmt_idx].outputs[0].fmt;
 
 	if (params->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		if (is_fe) {
@@ -239,9 +239,9 @@ static void skl_tplg_update_buffer_size(struct skl_dev *skl,
 	/* Since fixups is applied to pin 0 only, ibs, obs needs
 	 * change for pin 0 only
 	 */
-	res = &mcfg->module->resources[0];
-	in_fmt = &mcfg->module->formats[0].inputs[0].fmt;
-	out_fmt = &mcfg->module->formats[0].outputs[0].fmt;
+	res = &mcfg->module->resources[mcfg->res_idx];
+	in_fmt = &mcfg->module->formats[mcfg->fmt_idx].inputs[0].fmt;
+	out_fmt = &mcfg->module->formats[mcfg->fmt_idx].outputs[0].fmt;
 
 	if (mcfg->m_type == SKL_MODULE_TYPE_SRCINT)
 		multiplier = 5;
@@ -1631,11 +1631,12 @@ int skl_tplg_update_pipe_params(struct device *dev,
 			struct skl_module_cfg *mconfig,
 			struct skl_pipe_params *params)
 {
-	struct skl_module_res *res = &mconfig->module->resources[0];
+	struct skl_module_res *res;
 	struct skl_dev *skl = get_skl_ctx(dev);
 	struct skl_module_fmt *format = NULL;
 	u8 cfg_idx = mconfig->pipe->cur_config_idx;
 
+	res = &mconfig->module->resources[mconfig->res_idx];
 	skl_tplg_fill_dma_id(mconfig, params);
 	mconfig->fmt_idx = mconfig->mod_cfg[cfg_idx].fmt_idx;
 	mconfig->res_idx = mconfig->mod_cfg[cfg_idx].res_idx;
@@ -1644,9 +1645,9 @@ int skl_tplg_update_pipe_params(struct device *dev,
 		return 0;
 
 	if (params->stream == SNDRV_PCM_STREAM_PLAYBACK)
-		format = &mconfig->module->formats[0].inputs[0].fmt;
+		format = &mconfig->module->formats[mconfig->fmt_idx].inputs[0].fmt;
 	else
-		format = &mconfig->module->formats[0].outputs[0].fmt;
+		format = &mconfig->module->formats[mconfig->fmt_idx].outputs[0].fmt;
 
 	/* set the hw_params */
 	format->s_freq = params->s_freq;
Signature · 1 line
2.25.1
author maintainer
#5 lore raw

PATCH 5/11 ASoC: Intel: Skylake: Fix module configuration for KPB and MIXER

In reply to Cezary Rojewski

KeyPhrasebuffer, Mixin and Mixout modules configuration is described by firmware's basic module configuration structure. There are no extended parameters required. Update functions taking part in building INIT_INSTANCE IPC payload to reflect that.

---
 sound/soc/intel/skylake/skl-messages.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
sound/soc/intel/skylake/skl-messages.c +9 -2
index 476ef1897961..79c6cf2c14bf 100644
@@ -802,9 +802,12 @@ static u16 skl_get_module_param_size(struct skl_dev *skl,
 
 	case SKL_MODULE_TYPE_BASE_OUTFMT:
 	case SKL_MODULE_TYPE_MIC_SELECT:
-	case SKL_MODULE_TYPE_KPB:
 		return sizeof(struct skl_base_outfmt_cfg);
 
+	case SKL_MODULE_TYPE_MIXER:
+	case SKL_MODULE_TYPE_KPB:
+		return sizeof(struct skl_base_cfg);
+
 	default:
 		/*
 		 * return only base cfg when no specific module type is
@@ -857,10 +860,14 @@ static int skl_set_module_format(struct skl_dev *skl,
 
 	case SKL_MODULE_TYPE_BASE_OUTFMT:
 	case SKL_MODULE_TYPE_MIC_SELECT:
-	case SKL_MODULE_TYPE_KPB:
 		skl_set_base_outfmt_format(skl, module_config, *param_data);
 		break;
 
+	case SKL_MODULE_TYPE_MIXER:
+	case SKL_MODULE_TYPE_KPB:
+		skl_set_base_module_format(skl, module_config, *param_data);
+		break;
+
 	default:
 		skl_set_base_module_format(skl, module_config, *param_data);
 		break;
Signature · 1 line
2.25.1
author maintainer
#6 lore raw

PATCH 6/11 ASoC: Intel: Skylake: Fix passing loadable flag for module

In reply to Cezary Rojewski

From: Gustaw Lewandowski <[email protected]>

skl_get_module_info() tries to set mconfig->module->loadable before mconfig->module has been assigned thus flag was always set to false and driver did not try to load module binaries.

---
 sound/soc/intel/skylake/skl-pcm.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)
sound/soc/intel/skylake/skl-pcm.c +9 -16
index c604200de80e..9ecaf6a1e847 100644
@@ -1310,21 +1310,6 @@ static int skl_get_module_info(struct skl_dev *skl,
 		return -EIO;
 	}
 
-	list_for_each_entry(module, &skl->uuid_list, list) {
-		if (guid_equal(uuid_mod, &module->uuid)) {
-			mconfig->id.module_id = module->id;
-			if (mconfig->module)
-				mconfig->module->loadable = module->is_loadable;
-			ret = 0;
-			break;
-		}
-	}
-
-	if (ret)
-		return ret;
-
-	uuid_mod = &module->uuid;
-	ret = -EIO;
 	for (i = 0; i < skl->nr_modules; i++) {
 		skl_module = skl->modules[i];
 		uuid_tplg = &skl_module->uuid;
@@ -1334,10 +1319,18 @@ static int skl_get_module_info(struct skl_dev *skl,
 			break;
 		}
 	}
+
 	if (skl->nr_modules && ret)
 		return ret;
 
+	ret = -EIO;
 	list_for_each_entry(module, &skl->uuid_list, list) {
+		if (guid_equal(uuid_mod, &module->uuid)) {
+			mconfig->id.module_id = module->id;
+			mconfig->module->loadable = module->is_loadable;
+			ret = 0;
+		}
+
 		for (i = 0; i < MAX_IN_QUEUE; i++) {
 			pin_id = &mconfig->m_in_pin[i].id;
 			if (guid_equal(&pin_id->mod_uuid, &module->uuid))
@@ -1351,7 +1344,7 @@ static int skl_get_module_info(struct skl_dev *skl,
 		}
 	}
 
-	return 0;
+	return ret;
 }
 
 static int skl_populate_modules(struct skl_dev *skl)
Signature · 1 line
2.25.1
author maintainer
#7 lore raw

PATCH 7/11 ASoC: Intel: Skylake: Simplify m_state for loadable modules

In reply to Cezary Rojewski

From: Gustaw Lewandowski <[email protected]>

States SKL_MODULE_LOADED and SKL_MODULE_UNLOADED are redundant with 'loadable' flag in struct skl_module. Additionally skl_tplg_mixer_dapm_post_pmd_event() sets m_state always to SKL_MODULE_UNINIT so next unload function isn't called for such modules.

---
 sound/soc/intel/skylake/skl-topology.c | 5 +----
 sound/soc/intel/skylake/skl-topology.h | 6 ++----
 2 files changed, 3 insertions(+), 8 deletions(-)
sound/soc/intel/skylake/skl-topology.c +1 -4
index a35ee9d47d6d..51fb594bb172 100644
@@ -498,8 +498,6 @@ skl_tplg_init_pipe_modules(struct skl_dev *skl, struct skl_pipe *pipe)
 				mconfig->id.module_id, mconfig->guid);
 			if (ret < 0)
 				return ret;
-
-			mconfig->m_state = SKL_MODULE_LOADED;
 		}
 
 		/* prepare the DMA if the module is gateway cpr */
@@ -558,8 +556,7 @@ static int skl_tplg_unload_pipe_modules(struct skl_dev *skl,
 		mconfig  = w_module->w->priv;
 		uuid_mod = (guid_t *)mconfig->guid;
 
-		if (mconfig->module->loadable && skl->dsp->fw_ops.unload_mod &&
-			mconfig->m_state > SKL_MODULE_UNINIT) {
+		if (mconfig->module->loadable && skl->dsp->fw_ops.unload_mod) {
 			ret = skl->dsp->fw_ops.unload_mod(skl->dsp,
 						mconfig->id.module_id);
 			if (ret < 0)
sound/soc/intel/skylake/skl-topology.h +2 -4
index fb011862fb24..ad230fd64758 100644
@@ -311,10 +311,8 @@ struct skl_pipe {
 
 enum skl_module_state {
 	SKL_MODULE_UNINIT = 0,
-	SKL_MODULE_LOADED = 1,
-	SKL_MODULE_INIT_DONE = 2,
-	SKL_MODULE_BIND_DONE = 3,
-	SKL_MODULE_UNLOADED = 4,
+	SKL_MODULE_INIT_DONE = 1,
+	SKL_MODULE_BIND_DONE = 2,
 };
 
 enum d0i3_capability {
Signature · 1 line
2.25.1
author maintainer
#8 lore raw

PATCH 8/11 ASoC: Intel: Skylake: Support multiple format configs

In reply to Cezary Rojewski

From: Kareem Shaik <[email protected]>

A module can have two kinds of set params, as per topology requirements. For example, one pre-init and one post-init. But currently, there is support for just one type, as the format_config.

This patch extends the format_configs to 4, so as to be able to support pre-init, post-init and post-bind type of set params, for the same module, simultaneously.

---
 include/uapi/sound/snd_sst_tokens.h    |  6 +-
 sound/soc/intel/skylake/skl-messages.c | 19 +++--
 sound/soc/intel/skylake/skl-topology.c | 99 +++++++++++++++++---------
 sound/soc/intel/skylake/skl-topology.h |  5 +-
 4 files changed, 82 insertions(+), 47 deletions(-)
include/uapi/sound/snd_sst_tokens.h +5 -1
index 8ba0112e5336..ff3748e9308a 100644
@@ -233,6 +233,8 @@
  *
  * %SKL_TKN_U32_ASTATE_CLK_SRC: Clock source for A-State entry
  *
+ * %SKL_TKN_U32_FMT_CFG_IDX:    Format config index
+ *
  * module_id and loadable flags dont have tokens as these values will be
  * read from the DSP FW manifest
  *
@@ -324,7 +326,9 @@ enum SKL_TKNS {
 	SKL_TKN_U32_ASTATE_COUNT,
 	SKL_TKN_U32_ASTATE_KCPS,
 	SKL_TKN_U32_ASTATE_CLK_SRC,
-	SKL_TKN_MAX = SKL_TKN_U32_ASTATE_CLK_SRC,
+
+	SKL_TKN_U32_FMT_CFG_IDX = 96,
+	SKL_TKN_MAX = SKL_TKN_U32_FMT_CFG_IDX,
 };
 
 #endif
sound/soc/intel/skylake/skl-messages.c +9 -10
index 79c6cf2c14bf..7257acf0b6ee 100644
@@ -479,15 +479,15 @@ static void skl_set_base_module_format(struct skl_dev *skl,
 static void skl_copy_copier_caps(struct skl_module_cfg *mconfig,
 				struct skl_cpr_cfg *cpr_mconfig)
 {
-	if (mconfig->formats_config.caps_size == 0)
+	if (mconfig->formats_config[SKL_PARAM_INIT].caps_size == 0)
 		return;
 
 	memcpy(cpr_mconfig->gtw_cfg.config_data,
-			mconfig->formats_config.caps,
-			mconfig->formats_config.caps_size);
+			mconfig->formats_config[SKL_PARAM_INIT].caps,
+			mconfig->formats_config[SKL_PARAM_INIT].caps_size);
 
 	cpr_mconfig->gtw_cfg.config_length =
-			(mconfig->formats_config.caps_size) / 4;
+			(mconfig->formats_config[SKL_PARAM_INIT].caps_size) / 4;
 }
 
 #define SKL_NON_GATEWAY_CPR_NODE_ID 0xFFFFFFFF
@@ -749,13 +749,12 @@ static void skl_set_algo_format(struct skl_dev *skl,
 	struct skl_base_cfg *base_cfg = (struct skl_base_cfg *)algo_mcfg;
 
 	skl_set_base_module_format(skl, mconfig, base_cfg);
-
-	if (mconfig->formats_config.caps_size == 0)
+	if (mconfig->formats_config[SKL_PARAM_INIT].caps_size == 0)
 		return;
 
 	memcpy(algo_mcfg->params,
-			mconfig->formats_config.caps,
-			mconfig->formats_config.caps_size);
+			mconfig->formats_config[SKL_PARAM_INIT].caps,
+			mconfig->formats_config[SKL_PARAM_INIT].caps_size);
 
 }
 
@@ -786,7 +785,7 @@ static u16 skl_get_module_param_size(struct skl_dev *skl,
 	switch (mconfig->m_type) {
 	case SKL_MODULE_TYPE_COPIER:
 		param_size = sizeof(struct skl_cpr_cfg);
-		param_size += mconfig->formats_config.caps_size;
+		param_size += mconfig->formats_config[SKL_PARAM_INIT].caps_size;
 		return param_size;
 
 	case SKL_MODULE_TYPE_SRCINT:
@@ -797,7 +796,7 @@ static u16 skl_get_module_param_size(struct skl_dev *skl,
 
 	case SKL_MODULE_TYPE_ALGO:
 		param_size = sizeof(struct skl_base_cfg);
-		param_size += mconfig->formats_config.caps_size;
+		param_size += mconfig->formats_config[SKL_PARAM_INIT].caps_size;
 		return param_size;
 
 	case SKL_MODULE_TYPE_BASE_OUTFMT:
sound/soc/intel/skylake/skl-topology.c +64 -35

197 lines not on this page · raw message

sound/soc/intel/skylake/skl-topology.h +4 -1
index ad230fd64758..ef332c83d5c0 100644
@@ -81,6 +81,8 @@ enum skl_s_freq {
 	SKL_FS_INVALID
 };
 
+#define SKL_MAX_PARAMS_TYPES	4
+
 enum skl_widget_type {
 	SKL_WIDGET_VMIXER = 1,
 	SKL_WIDGET_MIXER = 2,
@@ -371,6 +373,7 @@ struct skl_module_cfg {
 	struct skl_module *module;
 	int res_idx;
 	int fmt_idx;
+	int fmt_cfg_idx;
 	u8 domain;
 	bool homogenous_inputs;
 	bool homogenous_outputs;
@@ -401,7 +404,7 @@ struct skl_module_cfg {
 	enum skl_hw_conn_type  hw_conn_type;
 	enum skl_module_state m_state;
 	struct skl_pipe *pipe;
-	struct skl_specific_cfg formats_config;
+	struct skl_specific_cfg formats_config[SKL_MAX_PARAMS_TYPES];
 	struct skl_pipe_mcfg mod_cfg[SKL_MAX_MODULES_IN_PIPE];
 };
 
Signature · 1 line
2.25.1
author maintainer
#9 lore raw

PATCH 9/11 ASoC: Intel: Skylake: Support modules with generic extension

In reply to Cezary Rojewski

From: Szymon Mielczarek <[email protected]>

Some DSP modules require, besides the module base configuration, a generic extension containing audio format for all module's pins that are in use.

---
 sound/soc/intel/skylake/skl-messages.c | 82 ++++++++++++++++++++++----
 sound/soc/intel/skylake/skl-topology.h | 15 +++++
 2 files changed, 84 insertions(+), 13 deletions(-)
sound/soc/intel/skylake/skl-messages.c +69 -13
index 7257acf0b6ee..8886f984ae26 100644
@@ -472,6 +472,75 @@ static void skl_set_base_module_format(struct skl_dev *skl,
 	base_cfg->is_pages = res->is_pages;
 }
 
+static void fill_pin_params(struct skl_audio_data_format *pin_fmt,
+			    struct skl_module_fmt *format)
+{
+	pin_fmt->number_of_channels = format->channels;
+	pin_fmt->s_freq = format->s_freq;
+	pin_fmt->bit_depth = format->bit_depth;
+	pin_fmt->valid_bit_depth = format->valid_bit_depth;
+	pin_fmt->ch_cfg = format->ch_cfg;
+	pin_fmt->sample_type = format->sample_type;
+	pin_fmt->channel_map = format->ch_map;
+	pin_fmt->interleaving = format->interleaving_style;
+}
+
+/*
+ * Any module configuration begins with a base module configuration but
+ * can be followed by a generic extension containing audio format for all
+ * module's pins that are in use.
+ */
+static void skl_set_base_ext_module_format(struct skl_dev *skl,
+					   struct skl_module_cfg *mconfig,
+					   struct skl_base_cfg_ext *base_cfg_ext)
+{
+	struct skl_module *module = mconfig->module;
+	struct skl_module_pin_resources *pin_res;
+	struct skl_module_iface *fmt = &module->formats[mconfig->fmt_idx];
+	struct skl_module_res *res = &module->resources[mconfig->res_idx];
+	struct skl_module_fmt *format;
+	struct skl_pin_format *pin_fmt;
+	char *params;
+	int i;
+
+	base_cfg_ext->nr_input_pins = res->nr_input_pins;
+	base_cfg_ext->nr_output_pins = res->nr_output_pins;
+	base_cfg_ext->priv_param_length =
+		mconfig->formats_config[SKL_PARAM_INIT].caps_size;
+
+	for (i = 0; i < res->nr_input_pins; i++) {
+		pin_res = &res->input[i];
+		pin_fmt = &base_cfg_ext->pins_fmt[i];
+
+		pin_fmt->pin_idx = pin_res->pin_index;
+		pin_fmt->buf_size = pin_res->buf_size;
+
+		format = &fmt->inputs[pin_res->pin_index].fmt;
+		fill_pin_params(&pin_fmt->audio_fmt, format);
+	}
+
+	for (i = 0; i < res->nr_output_pins; i++) {
+		pin_res = &res->output[i];
+		pin_fmt = &base_cfg_ext->pins_fmt[res->nr_input_pins + i];
+
+		pin_fmt->pin_idx = pin_res->pin_index;
+		pin_fmt->buf_size = pin_res->buf_size;
+
+		format = &fmt->outputs[pin_res->pin_index].fmt;
+		fill_pin_params(&pin_fmt->audio_fmt, format);
+	}
+
+	if (!base_cfg_ext->priv_param_length)
+		return;
+
+	params = (char *)base_cfg_ext + sizeof(struct skl_base_cfg_ext);
+	params += (base_cfg_ext->nr_input_pins + base_cfg_ext->nr_output_pins) *
+		  sizeof(struct skl_pin_format);
+
+	memcpy(params, mconfig->formats_config[SKL_PARAM_INIT].caps,
+	       mconfig->formats_config[SKL_PARAM_INIT].caps_size);
+}
+
 /*
  * Copies copier capabilities into copier module and updates copier module
  * config size.
@@ -1091,19 +1160,6 @@ int skl_unbind_modules(struct skl_dev *skl,
 	return ret;
 }
 
-static void fill_pin_params(struct skl_audio_data_format *pin_fmt,
-				struct skl_module_fmt *format)
-{
-	pin_fmt->number_of_channels = format->channels;
-	pin_fmt->s_freq = format->s_freq;
-	pin_fmt->bit_depth = format->bit_depth;
-	pin_fmt->valid_bit_depth = format->valid_bit_depth;
-	pin_fmt->ch_cfg = format->ch_cfg;
-	pin_fmt->sample_type = format->sample_type;
-	pin_fmt->channel_map = format->ch_map;
-	pin_fmt->interleaving = format->interleaving_style;
-}
-
 #define CPR_SINK_FMT_PARAM_ID 2
 
 /*
sound/soc/intel/skylake/skl-topology.h +15 -0
index ef332c83d5c0..f0695b2ac5dd 100644
@@ -152,6 +152,21 @@ struct skl_up_down_mixer_cfg {
 	u32 ch_map;
 } __packed;
 
+struct skl_pin_format {
+	u32 pin_idx;
+	u32 buf_size;
+	struct skl_audio_data_format audio_fmt;
+} __packed;
+
+struct skl_base_cfg_ext {
+	u16 nr_input_pins;
+	u16 nr_output_pins;
+	u8 reserved[8];
+	u32 priv_param_length;
+	/* Input pin formats followed by output ones. */
+	struct skl_pin_format pins_fmt[0];
+} __packed;
+
 struct skl_algo_cfg {
 	struct skl_base_cfg  base_cfg;
 	char params[];
Signature · 1 line
2.25.1
author maintainer
#10 lore raw

PATCH 10/11 ASoC: Intel: Skylake: Properly configure modules with generic extension

In reply to Cezary Rojewski

From: Pawel Harlozinski <[email protected]>

Make use of struct skl_base_cfg_ext and its format setter to configure modules which are described with said structure.

---
 sound/soc/intel/skylake/skl-messages.c | 51 ++++++++------------------
 1 file changed, 15 insertions(+), 36 deletions(-)
sound/soc/intel/skylake/skl-messages.c +15 -36
index 8886f984ae26..eaad180af42e 100644
@@ -806,27 +806,6 @@ static void skl_set_copier_format(struct skl_dev *skl,
 	skl_setup_cpr_gateway_cfg(skl, mconfig, cpr_mconfig);
 }
 
-/*
- * Algo module are DSP pre processing modules. Algo module take base module
- * configuration and params
- */
-
-static void skl_set_algo_format(struct skl_dev *skl,
-			struct skl_module_cfg *mconfig,
-			struct skl_algo_cfg *algo_mcfg)
-{
-	struct skl_base_cfg *base_cfg = (struct skl_base_cfg *)algo_mcfg;
-
-	skl_set_base_module_format(skl, mconfig, base_cfg);
-	if (mconfig->formats_config[SKL_PARAM_INIT].caps_size == 0)
-		return;
-
-	memcpy(algo_mcfg->params,
-			mconfig->formats_config[SKL_PARAM_INIT].caps,
-			mconfig->formats_config[SKL_PARAM_INIT].caps_size);
-
-}
-
 /*
  * Mic select module allows selecting one or many input channels, thus
  * acting as a demux.
@@ -849,6 +828,8 @@ static void skl_set_base_outfmt_format(struct skl_dev *skl,
 static u16 skl_get_module_param_size(struct skl_dev *skl,
 			struct skl_module_cfg *mconfig)
 {
+	struct skl_module_res *res;
+	struct skl_module *module = mconfig->module;
 	u16 param_size;
 
 	switch (mconfig->m_type) {
@@ -863,11 +844,6 @@ static u16 skl_get_module_param_size(struct skl_dev *skl,
 	case SKL_MODULE_TYPE_UPDWMIX:
 		return sizeof(struct skl_up_down_mixer_cfg);
 
-	case SKL_MODULE_TYPE_ALGO:
-		param_size = sizeof(struct skl_base_cfg);
-		param_size += mconfig->formats_config[SKL_PARAM_INIT].caps_size;
-		return param_size;
-
 	case SKL_MODULE_TYPE_BASE_OUTFMT:
 	case SKL_MODULE_TYPE_MIC_SELECT:
 		return sizeof(struct skl_base_outfmt_cfg);
@@ -876,12 +852,16 @@ static u16 skl_get_module_param_size(struct skl_dev *skl,
 	case SKL_MODULE_TYPE_KPB:
 		return sizeof(struct skl_base_cfg);
 
+	case SKL_MODULE_TYPE_ALGO:
 	default:
-		/*
-		 * return only base cfg when no specific module type is
-		 * specified
-		 */
-		return sizeof(struct skl_base_cfg);
+		res = &module->resources[mconfig->res_idx];
+
+		param_size = sizeof(struct skl_base_cfg) + sizeof(struct skl_base_cfg_ext);
+		param_size += (res->nr_input_pins + res->nr_output_pins) *
+			      sizeof(struct skl_pin_format);
+		param_size += mconfig->formats_config[SKL_PARAM_INIT].caps_size;
+
+		return param_size;
 	}
 
 	return 0;
@@ -922,10 +902,6 @@ static int skl_set_module_format(struct skl_dev *skl,
 		skl_set_updown_mixer_format(skl, module_config, *param_data);
 		break;
 
-	case SKL_MODULE_TYPE_ALGO:
-		skl_set_algo_format(skl, module_config, *param_data);
-		break;
-
 	case SKL_MODULE_TYPE_BASE_OUTFMT:
 	case SKL_MODULE_TYPE_MIC_SELECT:
 		skl_set_base_outfmt_format(skl, module_config, *param_data);
@@ -936,10 +912,13 @@ static int skl_set_module_format(struct skl_dev *skl,
 		skl_set_base_module_format(skl, module_config, *param_data);
 		break;
 
+	case SKL_MODULE_TYPE_ALGO:
 	default:
 		skl_set_base_module_format(skl, module_config, *param_data);
+		skl_set_base_ext_module_format(skl, module_config,
+					       *param_data +
+					       sizeof(struct skl_base_cfg));
 		break;
-
 	}
 
 	dev_dbg(skl->dev, "Module type=%d id=%d config size: %d bytes\n",
Signature · 1 line
2.25.1
author maintainer
#11 lore raw

PATCH 11/11 ASoC: Intel: Skylake: Select first entry for singular pipe config arrays

In reply to Cezary Rojewski

When pipe does not expose multiple configuration options, always select the first entry without searching for matching one.

---
 sound/soc/intel/skylake/skl-topology.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
sound/soc/intel/skylake/skl-topology.c +3 -2
index 04fbf83f3835..b036852d6889 100644
@@ -640,8 +640,9 @@ skl_tplg_get_pipe_config(struct skl_dev *skl, struct skl_module_cfg *mconfig)
 		return 0;
 	}
 
-	if (pipe->conn_type == SKL_PIPE_CONN_TYPE_NONE) {
-		dev_dbg(skl->dev, "No conn_type detected, take 0th config\n");
+	if (pipe->conn_type == SKL_PIPE_CONN_TYPE_NONE || pipe->nr_cfgs == 1) {
+		dev_dbg(skl->dev, "No conn_type or just 1 pathcfg, taking 0th for %d\n",
+			pipe->ppl_id);
 		pipe->cur_config_idx = 0;
 		pipe->memory_pages = pconfig->mem_pages;
 
Signature · 1 line
2.25.1
Tested-by on the v1 series
#12 lore raw
In reply to Cezary Rojewski

śr., 18 sie 2021 o 09:54 Cezary Rojewski <[email protected]> napisał(a):

Quoted 40 lines

Existing skylake-driver supports very basic scenarios with limited range of modules and their control. Attached changes first fix code as several advanced configurations are 'mentioned' throughout the files but are not actually functional. Follow up are changes adding missing support for said configurations.

Cezary Rojewski (5):
ASoC: Intel: kbl_da7219_max98927: Fix format selection for max98373
ASoC: Intel: Skylake: Leave data as is when invoking TLV IPCs
ASoC: Intel: Skylake: Fix module resource and format selection
ASoC: Intel: Skylake: Fix module configuration for KPB and MIXER
ASoC: Intel: Skylake: Select first entry for singular pipe config
arrays

Gustaw Lewandowski (2):
ASoC: Intel: Skylake: Fix passing loadable flag for module
ASoC: Intel: Skylake: Simplify m_state for loadable modules

Kareem Shaik (1):
ASoC: Intel: Skylake: Support multiple format configs

Pawel Harlozinski (1):
ASoC: Intel: Skylake: Properly configure modules with generic
extension

Piotr Maziarz (1):
ASoC: Intel: Skylake: Select proper format for NHLT blob

Szymon Mielczarek (1):
ASoC: Intel: Skylake: Support modules with generic extension

 include/uapi/sound/snd_sst_tokens.h          |   6 +-
 sound/soc/intel/boards/kbl_da7219_max98927.c |  55 +------
 sound/soc/intel/skylake/skl-messages.c       | 155 ++++++++++++-------
 sound/soc/intel/skylake/skl-pcm.c            |  25 ++-
 sound/soc/intel/skylake/skl-topology.c       | 155 +++++++++++--------
 sound/soc/intel/skylake/skl-topology.h       |  26 +++-
 6 files changed, 231 insertions(+), 191 deletions(-)
Quoted 3 lines

--
2.25.1

I have checked playback/record with speakers/dmic/headset on the Atlas chromebook - everything seems to work fine, so:

Best regards,
Lukasz

maintainer
#13 lore raw
In reply to Cezary Rojewski
Quoted 14 lines · Cezary Rojewski

Existing skylake-driver supports very basic scenarios with limited range of modules and their control. Attached changes first fix code as several advanced configurations are 'mentioned' throughout the files but are not actually functional. Follow up are changes adding missing support for said configurations.

Cezary Rojewski (5):
ASoC: Intel: kbl_da7219_max98927: Fix format selection for max98373
ASoC: Intel: Skylake: Leave data as is when invoking TLV IPCs
ASoC: Intel: Skylake: Fix module resource and format selection
ASoC: Intel: Skylake: Fix module configuration for KPB and MIXER
ASoC: Intel: Skylake: Select first entry for singular pipe config
arrays

Quoted 1 line

[...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[01/11] ASoC: Intel: kbl_da7219_max98927: Fix format selection for max98373

        commit: 6d41bbf2fd3615c56dbf2b67f6cbf9e83d14a2e2
[02/11] ASoC: Intel: Skylake: Leave data as is when invoking TLV IPCs
        commit: 126b3422adc80f29d2129db7f61e0113a8a526c6
[03/11] ASoC: Intel: Skylake: Select proper format for NHLT blob
        commit: 87b265260046d4e0ed3f150dac2184b9dbd4bf7c
[04/11] ASoC: Intel: Skylake: Fix module resource and format selection
        commit: e8b374b649afe756c2470e0e6668022e90bf8518
[05/11] ASoC: Intel: Skylake: Fix module configuration for KPB and MIXER
        commit: e4e0633bcadc950b4b4af06c7f1bb7f7e3e86321
[06/11] ASoC: Intel: Skylake: Fix passing loadable flag for module
        commit: c5ed9c547cba1dc1238c6e8a0c290fd62ee6e127
[07/11] ASoC: Intel: Skylake: Simplify m_state for loadable modules
        commit: e4e95d8291831fa63c32e00b59f544f0812dc0b6
[08/11] ASoC: Intel: Skylake: Support multiple format configs
        commit: a4ad42d28618eef83bee02e0a19af0d467bd9722
[09/11] ASoC: Intel: Skylake: Support modules with generic extension
        commit: db5a3f83a241a98d855e2cecc67869d06cb3e135
[10/11] ASoC: Intel: Skylake: Properly configure modules with generic extension
        commit: 5b27a71cbbfe6ffb554c87fde2ad9b71841478c2
[11/11] ASoC: Intel: Skylake: Select first entry for singular pipe config arrays
        commit: b947d2b467c0ea275ac3b468974ebe0dd0c0c3bd

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark