[alsa-devel] [PATCH 0/3] ASoC: Intel: Skylake: Add Support for MIC select module
This series adds support in DSP to add support for MIC select module which allows user to select 1 to N channels from DMIC channel input.
Further it also adds support in bxt-rt298 machine for such a module.
Kranthikumar, GudishaX (3): ASoC: Intel: Skylake: Add mic-select module type ASoC: Intel: Skylake: Add enum control for mic selection ASoC: Intel: Boards: Add 4-channel DMIC fixup.
sound/soc/intel/boards/bxt_rt298.c | 7 +- sound/soc/intel/skylake/skl-messages.c | 2 + sound/soc/intel/skylake/skl-topology.c | 143 +++++++++++++++++++++++++++ sound/soc/intel/skylake/skl-topology.h | 20 ++++ sound/soc/intel/skylake/skl-tplg-interface.h | 2 + 5 files changed, 169 insertions(+), 5 deletions(-)
From: "Kranthikumar, GudishaX" gudishax.kranthikumar@intel.com
mic-select module is a DSP module, which is used to select one or more input channels.
This patch adds mic-select module type.
Signed-off-by: Kranthikumar, GudishaX gudishax.kranthikumar@intel.com Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com --- sound/soc/intel/skylake/skl-messages.c | 2 ++ sound/soc/intel/skylake/skl-tplg-interface.h | 1 + 2 files changed, 3 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c index ab1adc0c9cc3..5a465020ebd8 100644 --- a/sound/soc/intel/skylake/skl-messages.c +++ b/sound/soc/intel/skylake/skl-messages.c @@ -707,6 +707,7 @@ static u16 skl_get_module_param_size(struct skl_sst *ctx, return param_size;
case SKL_MODULE_TYPE_BASE_OUTFMT: + case SKL_MODULE_TYPE_MIC_SELECT: case SKL_MODULE_TYPE_KPB: return sizeof(struct skl_base_outfmt_cfg);
@@ -761,6 +762,7 @@ static int skl_set_module_format(struct skl_sst *ctx, break;
case SKL_MODULE_TYPE_BASE_OUTFMT: + case SKL_MODULE_TYPE_MIC_SELECT: case SKL_MODULE_TYPE_KPB: skl_set_base_outfmt_format(ctx, module_config, *param_data); break; diff --git a/sound/soc/intel/skylake/skl-tplg-interface.h b/sound/soc/intel/skylake/skl-tplg-interface.h index 7a2febf99019..c22517bd2161 100644 --- a/sound/soc/intel/skylake/skl-tplg-interface.h +++ b/sound/soc/intel/skylake/skl-tplg-interface.h @@ -82,6 +82,7 @@ enum skl_module_type { SKL_MODULE_TYPE_ALGO, SKL_MODULE_TYPE_BASE_OUTFMT, SKL_MODULE_TYPE_KPB, + SKL_MODULE_TYPE_MIC_SELECT, };
enum skl_core_affinity {
The patch
ASoC: Intel: Skylake: Add mic-select module type
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From db6879efb9d1d48ff9c2bd49dde05ecf757d73cf Mon Sep 17 00:00:00 2001
From: Dharageswari R dharageswari.r@intel.com Date: Wed, 31 May 2017 10:30:24 +0530 Subject: [PATCH] ASoC: Intel: Skylake: Add mic-select module type
mic-select module is a DSP module, which is used to select one or more input channels.
This patch adds mic-select module type.
Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/skylake/skl-messages.c | 2 ++ sound/soc/intel/skylake/skl-tplg-interface.h | 1 + 2 files changed, 3 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c index ab1adc0c9cc3..5a465020ebd8 100644 --- a/sound/soc/intel/skylake/skl-messages.c +++ b/sound/soc/intel/skylake/skl-messages.c @@ -707,6 +707,7 @@ static u16 skl_get_module_param_size(struct skl_sst *ctx, return param_size;
case SKL_MODULE_TYPE_BASE_OUTFMT: + case SKL_MODULE_TYPE_MIC_SELECT: case SKL_MODULE_TYPE_KPB: return sizeof(struct skl_base_outfmt_cfg);
@@ -761,6 +762,7 @@ static int skl_set_module_format(struct skl_sst *ctx, break;
case SKL_MODULE_TYPE_BASE_OUTFMT: + case SKL_MODULE_TYPE_MIC_SELECT: case SKL_MODULE_TYPE_KPB: skl_set_base_outfmt_format(ctx, module_config, *param_data); break; diff --git a/sound/soc/intel/skylake/skl-tplg-interface.h b/sound/soc/intel/skylake/skl-tplg-interface.h index 7a2febf99019..c22517bd2161 100644 --- a/sound/soc/intel/skylake/skl-tplg-interface.h +++ b/sound/soc/intel/skylake/skl-tplg-interface.h @@ -82,6 +82,7 @@ enum skl_module_type { SKL_MODULE_TYPE_ALGO, SKL_MODULE_TYPE_BASE_OUTFMT, SKL_MODULE_TYPE_KPB, + SKL_MODULE_TYPE_MIC_SELECT, };
enum skl_core_affinity {
From: "Kranthikumar, GudishaX" gudishax.kranthikumar@intel.com
User may prefer to select data from particular mics. A mic-select module in DSP allows this selection.
Create possible enum controls to allow user to select a combination of mics to capture data from. Based on the user selection, parameters are generated and passed to mic-select module during init.
Signed-off-by: Kranthikumar, GudishaX gudishax.kranthikumar@intel.com Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com --- sound/soc/intel/skylake/skl-topology.c | 143 +++++++++++++++++++++++++++ sound/soc/intel/skylake/skl-topology.h | 20 ++++ sound/soc/intel/skylake/skl-tplg-interface.h | 1 + 3 files changed, 164 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index b687ae455a61..141cfbe40c3a 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -36,6 +36,19 @@ #define SKL_IN_DIR_BIT_MASK BIT(0) #define SKL_PIN_COUNT_MASK GENMASK(7, 4)
+static const int mic_mono_list[] = { +0, 1, 2, 3, +}; +static const int mic_stereo_list[][SKL_CH_STEREO] = { +{0, 1}, {0, 2}, {0, 3}, {1, 2}, {1, 3}, {2, 3}, +}; +static const int mic_trio_list[][SKL_CH_TRIO] = { +{0, 1, 2}, {0, 1, 3}, {0, 2, 3}, {1, 2, 3}, +}; +static const int mic_quatro_list[][SKL_CH_QUATRO] = { +{0, 1, 2, 3}, +}; + void skl_tplg_d0i3_get(struct skl *skl, enum d0i3_capability caps) { struct skl_d0i3_data *d0i3 = &skl->skl_sst->d0i3; @@ -1314,6 +1327,98 @@ static int skl_tplg_tlv_control_set(struct snd_kcontrol *kcontrol, return 0; }
+static int skl_tplg_mic_control_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol); + struct skl_module_cfg *mconfig = w->priv; + struct soc_enum *ec = (struct soc_enum *)kcontrol->private_value; + u32 ch_type = *((u32 *)ec->dobj.private); + + if (mconfig->dmic_ch_type == ch_type) + ucontrol->value.integer.value[0] = mconfig->dmic_ch_combo_index; + else + ucontrol->value.integer.value[0] = 0; + + return 0; +} + +static int skl_fill_mic_sel_params(struct skl_module_cfg *mconfig, + struct skl_mic_sel_config *mic_cfg, struct device *dev) +{ + struct skl_specific_cfg *sp_cfg = &mconfig->formats_config; + + sp_cfg->caps_size = sizeof(struct skl_mic_sel_config); + sp_cfg->set_params = SKL_PARAM_SET; + sp_cfg->param_id = 0x00; + if (!sp_cfg->caps) { + sp_cfg->caps = devm_kzalloc(dev, sp_cfg->caps_size, GFP_KERNEL); + if (!sp_cfg->caps) + return -ENOMEM; + } + + mic_cfg->mic_switch = SKL_MIC_SEL_SWITCH; + mic_cfg->flags = 0; + memcpy(sp_cfg->caps, mic_cfg, sp_cfg->caps_size); + + return 0; +} + +static int skl_tplg_mic_control_set(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol); + struct skl_module_cfg *mconfig = w->priv; + struct skl_mic_sel_config mic_cfg = {0}; + struct soc_enum *ec = (struct soc_enum *)kcontrol->private_value; + u32 ch_type = *((u32 *)ec->dobj.private); + const int *list; + u8 in_ch, out_ch, index; + + mconfig->dmic_ch_type = ch_type; + mconfig->dmic_ch_combo_index = ucontrol->value.integer.value[0]; + + /* enum control index 0 is INVALID, so no channels to be set */ + if (mconfig->dmic_ch_combo_index == 0) + return 0; + + /* No valid channel selection map for index 0, so offset by 1 */ + index = mconfig->dmic_ch_combo_index - 1; + + switch (ch_type) { + case SKL_CH_MONO: + list = &mic_mono_list[index]; + break; + + case SKL_CH_STEREO: + list = mic_stereo_list[index]; + break; + + case SKL_CH_TRIO: + list = mic_trio_list[index]; + break; + + case SKL_CH_QUATRO: + list = mic_quatro_list[index]; + break; + + default: + dev_err(w->dapm->dev, + "Invalid channel %d for mic_select module\n", + ch_type); + return -EINVAL; + + } + + /* channel type enum map to number of chanels for that type */ + for (out_ch = 0; out_ch < ch_type; out_ch++) { + in_ch = list[out_ch]; + mic_cfg.blob[out_ch][in_ch] = SKL_DEFAULT_MIC_SEL_GAIN; + } + + return skl_fill_mic_sel_params(mconfig, &mic_cfg, w->dapm->dev); +} + /* * Fill the dma id for host and link. In case of passthrough * pipeline, this will both host and link in the same @@ -1666,6 +1771,11 @@ int skl_tplg_be_update_params(struct snd_soc_dai *dai, skl_tplg_tlv_control_set}, };
+static const struct snd_soc_tplg_kcontrol_ops skl_tplg_kcontrol_ops[] = { + {SKL_CONTROL_TYPE_MIC_SELECT, skl_tplg_mic_control_get, + skl_tplg_mic_control_set}, +}; + static int skl_tplg_fill_pipe_tkn(struct device *dev, struct skl_pipe *pipe, u32 tkn, u32 tkn_val) @@ -2390,14 +2500,34 @@ static int skl_init_algo_data(struct device *dev, struct soc_bytes_ext *be, return 0; }
+static int skl_init_enum_data(struct device *dev, struct soc_enum *se, + struct snd_soc_tplg_enum_control *ec) +{ + + void *data; + + if (ec->priv.size) { + data = devm_kzalloc(dev, sizeof(ec->priv.size), GFP_KERNEL); + if (!data) + return -ENOMEM; + memcpy(data, ec->priv.data, ec->priv.size); + se->dobj.private = data; + } + + return 0; + +} + static int skl_tplg_control_load(struct snd_soc_component *cmpnt, struct snd_kcontrol_new *kctl, struct snd_soc_tplg_ctl_hdr *hdr) { struct soc_bytes_ext *sb; struct snd_soc_tplg_bytes_control *tplg_bc; + struct snd_soc_tplg_enum_control *tplg_ec; struct hdac_ext_bus *ebus = snd_soc_component_get_drvdata(cmpnt); struct hdac_bus *bus = ebus_to_hbus(ebus); + struct soc_enum *se;
switch (hdr->ops.info) { case SND_SOC_TPLG_CTL_BYTES: @@ -2411,6 +2541,17 @@ static int skl_tplg_control_load(struct snd_soc_component *cmpnt, } break;
+ case SND_SOC_TPLG_CTL_ENUM: + tplg_ec = container_of(hdr, + struct snd_soc_tplg_enum_control, hdr); + if (kctl->access & SNDRV_CTL_ELEM_ACCESS_READWRITE) { + se = (struct soc_enum *)kctl->private_value; + if (tplg_ec->priv.size) + return skl_init_enum_data(bus->dev, se, + tplg_ec); + } + break; + default: dev_warn(bus->dev, "Control load not supported %d:%d:%d\n", hdr->ops.get, hdr->ops.put, hdr->ops.info); @@ -2639,6 +2780,8 @@ static int skl_manifest_load(struct snd_soc_component *cmpnt, .control_load = skl_tplg_control_load, .bytes_ext_ops = skl_tlv_ops, .bytes_ext_ops_count = ARRAY_SIZE(skl_tlv_ops), + .io_ops = skl_tplg_kcontrol_ops, + .io_ops_count = ARRAY_SIZE(skl_tplg_kcontrol_ops), .manifest = skl_manifest_load, };
diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h index cc64d6bdb4f6..3f51a0a00093 100644 --- a/sound/soc/intel/skylake/skl-topology.h +++ b/sound/soc/intel/skylake/skl-topology.h @@ -39,6 +39,11 @@ #define MODULE_MAX_IN_PINS 8 #define MODULE_MAX_OUT_PINS 8
+#define SKL_MIC_CH_SUPPORT 4 +#define SKL_MIC_MAX_CH_SUPPORT 8 +#define SKL_DEFAULT_MIC_SEL_GAIN 0x3FF +#define SKL_MIC_SEL_SWITCH 0x3 + enum skl_channel_index { SKL_CHANNEL_LEFT = 0, SKL_CHANNEL_RIGHT = 1, @@ -309,6 +314,8 @@ struct skl_module_cfg { u8 dev_type; u8 dma_id; u8 time_slot; + u8 dmic_ch_combo_index; + u32 dmic_ch_type; u32 params_fixup; u32 converter; u32 vbus_id; @@ -342,6 +349,19 @@ struct skl_module_deferred_bind { struct list_head node; };
+struct skl_mic_sel_config { + u16 mic_switch; + u16 flags; + u16 blob[SKL_MIC_MAX_CH_SUPPORT][SKL_MIC_MAX_CH_SUPPORT]; +} __packed; + +enum skl_channel { + SKL_CH_MONO = 1, + SKL_CH_STEREO = 2, + SKL_CH_TRIO = 3, + SKL_CH_QUATRO = 4, +}; + static inline struct skl *get_skl_ctx(struct device *dev) { struct hdac_ext_bus *ebus = dev_get_drvdata(dev); diff --git a/sound/soc/intel/skylake/skl-tplg-interface.h b/sound/soc/intel/skylake/skl-tplg-interface.h index c22517bd2161..f8d1749a2e0c 100644 --- a/sound/soc/intel/skylake/skl-tplg-interface.h +++ b/sound/soc/intel/skylake/skl-tplg-interface.h @@ -24,6 +24,7 @@ * SST types start at higher to avoid any overlapping in future */ #define SKL_CONTROL_TYPE_BYTE_TLV 0x100 +#define SKL_CONTROL_TYPE_MIC_SELECT 0x102
#define HDA_SST_CFG_MAX 900 /* size of copier cfg*/ #define MAX_IN_QUEUE 8
On Fri, 26 May 2017 05:20:02 +0200, Vinod Koul wrote:
From: "Kranthikumar, GudishaX" gudishax.kranthikumar@intel.com
User may prefer to select data from particular mics. A mic-select module in DSP allows this selection.
Create possible enum controls to allow user to select a combination of mics to capture data from. Based on the user selection, parameters are generated and passed to mic-select module during init.
Signed-off-by: Kranthikumar, GudishaX gudishax.kranthikumar@intel.com Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com
sound/soc/intel/skylake/skl-topology.c | 143 +++++++++++++++++++++++++++ sound/soc/intel/skylake/skl-topology.h | 20 ++++ sound/soc/intel/skylake/skl-tplg-interface.h | 1 + 3 files changed, 164 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index b687ae455a61..141cfbe40c3a 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -36,6 +36,19 @@ #define SKL_IN_DIR_BIT_MASK BIT(0) #define SKL_PIN_COUNT_MASK GENMASK(7, 4)
+static const int mic_mono_list[] = { +0, 1, 2, 3, +}; +static const int mic_stereo_list[][SKL_CH_STEREO] = { +{0, 1}, {0, 2}, {0, 3}, {1, 2}, {1, 3}, {2, 3}, +}; +static const int mic_trio_list[][SKL_CH_TRIO] = { +{0, 1, 2}, {0, 1, 3}, {0, 2, 3}, {1, 2, 3}, +}; +static const int mic_quatro_list[][SKL_CH_QUATRO] = { +{0, 1, 2, 3}, +};
void skl_tplg_d0i3_get(struct skl *skl, enum d0i3_capability caps) { struct skl_d0i3_data *d0i3 = &skl->skl_sst->d0i3; @@ -1314,6 +1327,98 @@ static int skl_tplg_tlv_control_set(struct snd_kcontrol *kcontrol, return 0; }
+static int skl_tplg_mic_control_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol);
- struct skl_module_cfg *mconfig = w->priv;
- struct soc_enum *ec = (struct soc_enum *)kcontrol->private_value;
- u32 ch_type = *((u32 *)ec->dobj.private);
- if (mconfig->dmic_ch_type == ch_type)
ucontrol->value.integer.value[0] = mconfig->dmic_ch_combo_index;- else
ucontrol->value.integer.value[0] = 0;- return 0;
+}
+static int skl_fill_mic_sel_params(struct skl_module_cfg *mconfig,
- struct skl_mic_sel_config *mic_cfg, struct device *dev)
+{
- struct skl_specific_cfg *sp_cfg = &mconfig->formats_config;
- sp_cfg->caps_size = sizeof(struct skl_mic_sel_config);
- sp_cfg->set_params = SKL_PARAM_SET;
- sp_cfg->param_id = 0x00;
- if (!sp_cfg->caps) {
sp_cfg->caps = devm_kzalloc(dev, sp_cfg->caps_size, GFP_KERNEL);if (!sp_cfg->caps)return -ENOMEM;- }
- mic_cfg->mic_switch = SKL_MIC_SEL_SWITCH;
- mic_cfg->flags = 0;
- memcpy(sp_cfg->caps, mic_cfg, sp_cfg->caps_size);
- return 0;
+}
+static int skl_tplg_mic_control_set(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol);
- struct skl_module_cfg *mconfig = w->priv;
- struct skl_mic_sel_config mic_cfg = {0};
- struct soc_enum *ec = (struct soc_enum *)kcontrol->private_value;
- u32 ch_type = *((u32 *)ec->dobj.private);
- const int *list;
- u8 in_ch, out_ch, index;
- mconfig->dmic_ch_type = ch_type;
- mconfig->dmic_ch_combo_index = ucontrol->value.integer.value[0];
- /* enum control index 0 is INVALID, so no channels to be set */
- if (mconfig->dmic_ch_combo_index == 0)
return 0;- /* No valid channel selection map for index 0, so offset by 1 */
- index = mconfig->dmic_ch_combo_index - 1;
- switch (ch_type) {
- case SKL_CH_MONO:
list = &mic_mono_list[index];break;- case SKL_CH_STEREO:
list = mic_stereo_list[index];break;- case SKL_CH_TRIO:
list = mic_trio_list[index];break;- case SKL_CH_QUATRO:
list = mic_quatro_list[index];break;
How do you guarantee that index is in the array range? It looks easy to make things vulnerable with a firmware.
thanks,
Takashi
On Fri, May 26, 2017 at 08:57:44AM +0200, Takashi Iwai wrote:
On Fri, 26 May 2017 05:20:02 +0200, Vinod Koul wrote:
From: "Kranthikumar, GudishaX" gudishax.kranthikumar@intel.com
User may prefer to select data from particular mics. A mic-select module in DSP allows this selection.
Create possible enum controls to allow user to select a combination of mics to capture data from. Based on the user selection, parameters are generated and passed to mic-select module during init.
Signed-off-by: Kranthikumar, GudishaX gudishax.kranthikumar@intel.com Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com
sound/soc/intel/skylake/skl-topology.c | 143 +++++++++++++++++++++++++++ sound/soc/intel/skylake/skl-topology.h | 20 ++++ sound/soc/intel/skylake/skl-tplg-interface.h | 1 + 3 files changed, 164 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index b687ae455a61..141cfbe40c3a 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -36,6 +36,19 @@ #define SKL_IN_DIR_BIT_MASK BIT(0) #define SKL_PIN_COUNT_MASK GENMASK(7, 4)
+static const int mic_mono_list[] = { +0, 1, 2, 3, +}; +static const int mic_stereo_list[][SKL_CH_STEREO] = { +{0, 1}, {0, 2}, {0, 3}, {1, 2}, {1, 3}, {2, 3}, +}; +static const int mic_trio_list[][SKL_CH_TRIO] = { +{0, 1, 2}, {0, 1, 3}, {0, 2, 3}, {1, 2, 3}, +}; +static const int mic_quatro_list[][SKL_CH_QUATRO] = { +{0, 1, 2, 3}, +};
void skl_tplg_d0i3_get(struct skl *skl, enum d0i3_capability caps) { struct skl_d0i3_data *d0i3 = &skl->skl_sst->d0i3; @@ -1314,6 +1327,98 @@ static int skl_tplg_tlv_control_set(struct snd_kcontrol *kcontrol, return 0; }
+static int skl_tplg_mic_control_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol);
- struct skl_module_cfg *mconfig = w->priv;
- struct soc_enum *ec = (struct soc_enum *)kcontrol->private_value;
- u32 ch_type = *((u32 *)ec->dobj.private);
- if (mconfig->dmic_ch_type == ch_type)
ucontrol->value.integer.value[0] = mconfig->dmic_ch_combo_index;- else
ucontrol->value.integer.value[0] = 0;- return 0;
+}
+static int skl_fill_mic_sel_params(struct skl_module_cfg *mconfig,
- struct skl_mic_sel_config *mic_cfg, struct device *dev)
+{
- struct skl_specific_cfg *sp_cfg = &mconfig->formats_config;
- sp_cfg->caps_size = sizeof(struct skl_mic_sel_config);
- sp_cfg->set_params = SKL_PARAM_SET;
- sp_cfg->param_id = 0x00;
- if (!sp_cfg->caps) {
sp_cfg->caps = devm_kzalloc(dev, sp_cfg->caps_size, GFP_KERNEL);if (!sp_cfg->caps)return -ENOMEM;- }
- mic_cfg->mic_switch = SKL_MIC_SEL_SWITCH;
- mic_cfg->flags = 0;
- memcpy(sp_cfg->caps, mic_cfg, sp_cfg->caps_size);
- return 0;
+}
+static int skl_tplg_mic_control_set(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol);
- struct skl_module_cfg *mconfig = w->priv;
- struct skl_mic_sel_config mic_cfg = {0};
- struct soc_enum *ec = (struct soc_enum *)kcontrol->private_value;
- u32 ch_type = *((u32 *)ec->dobj.private);
- const int *list;
- u8 in_ch, out_ch, index;
- mconfig->dmic_ch_type = ch_type;
- mconfig->dmic_ch_combo_index = ucontrol->value.integer.value[0];
- /* enum control index 0 is INVALID, so no channels to be set */
- if (mconfig->dmic_ch_combo_index == 0)
return 0;- /* No valid channel selection map for index 0, so offset by 1 */
- index = mconfig->dmic_ch_combo_index - 1;
- switch (ch_type) {
- case SKL_CH_MONO:
list = &mic_mono_list[index];break;- case SKL_CH_STEREO:
list = mic_stereo_list[index];break;- case SKL_CH_TRIO:
list = mic_trio_list[index];break;- case SKL_CH_QUATRO:
list = mic_quatro_list[index];break;How do you guarantee that index is in the array range? It looks easy to make things vulnerable with a firmware.
Yes a good catch, we should right check the index here and see if it is within bounds, will fix it and update
Thanks
On Fri, May 26, 2017 at 01:05:41PM +0530, Vinod Koul wrote:
On Fri, May 26, 2017 at 08:57:44AM +0200, Takashi Iwai wrote:
On Fri, 26 May 2017 05:20:02 +0200, Vinod Koul wrote:
From: "Kranthikumar, GudishaX" gudishax.kranthikumar@intel.com
User may prefer to select data from particular mics. A mic-select module in DSP allows this selection.
Create possible enum controls to allow user to select a combination of mics to capture data from. Based on the user selection, parameters are generated and passed to mic-select module during init.
Signed-off-by: Kranthikumar, GudishaX gudishax.kranthikumar@intel.com Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com
sound/soc/intel/skylake/skl-topology.c | 143 +++++++++++++++++++++++++++ sound/soc/intel/skylake/skl-topology.h | 20 ++++ sound/soc/intel/skylake/skl-tplg-interface.h | 1 + 3 files changed, 164 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index b687ae455a61..141cfbe40c3a 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -36,6 +36,19 @@ #define SKL_IN_DIR_BIT_MASK BIT(0) #define SKL_PIN_COUNT_MASK GENMASK(7, 4)
+static const int mic_mono_list[] = { +0, 1, 2, 3, +}; +static const int mic_stereo_list[][SKL_CH_STEREO] = { +{0, 1}, {0, 2}, {0, 3}, {1, 2}, {1, 3}, {2, 3}, +}; +static const int mic_trio_list[][SKL_CH_TRIO] = { +{0, 1, 2}, {0, 1, 3}, {0, 2, 3}, {1, 2, 3}, +}; +static const int mic_quatro_list[][SKL_CH_QUATRO] = { +{0, 1, 2, 3}, +};
void skl_tplg_d0i3_get(struct skl *skl, enum d0i3_capability caps) { struct skl_d0i3_data *d0i3 = &skl->skl_sst->d0i3; @@ -1314,6 +1327,98 @@ static int skl_tplg_tlv_control_set(struct snd_kcontrol *kcontrol, return 0; }
+static int skl_tplg_mic_control_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol);
- struct skl_module_cfg *mconfig = w->priv;
- struct soc_enum *ec = (struct soc_enum *)kcontrol->private_value;
- u32 ch_type = *((u32 *)ec->dobj.private);
- if (mconfig->dmic_ch_type == ch_type)
ucontrol->value.integer.value[0] = mconfig->dmic_ch_combo_index;- else
ucontrol->value.integer.value[0] = 0;- return 0;
+}
+static int skl_fill_mic_sel_params(struct skl_module_cfg *mconfig,
- struct skl_mic_sel_config *mic_cfg, struct device *dev)
+{
- struct skl_specific_cfg *sp_cfg = &mconfig->formats_config;
- sp_cfg->caps_size = sizeof(struct skl_mic_sel_config);
- sp_cfg->set_params = SKL_PARAM_SET;
- sp_cfg->param_id = 0x00;
- if (!sp_cfg->caps) {
sp_cfg->caps = devm_kzalloc(dev, sp_cfg->caps_size, GFP_KERNEL);if (!sp_cfg->caps)return -ENOMEM;- }
- mic_cfg->mic_switch = SKL_MIC_SEL_SWITCH;
- mic_cfg->flags = 0;
- memcpy(sp_cfg->caps, mic_cfg, sp_cfg->caps_size);
- return 0;
+}
+static int skl_tplg_mic_control_set(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol);
- struct skl_module_cfg *mconfig = w->priv;
- struct skl_mic_sel_config mic_cfg = {0};
- struct soc_enum *ec = (struct soc_enum *)kcontrol->private_value;
- u32 ch_type = *((u32 *)ec->dobj.private);
- const int *list;
- u8 in_ch, out_ch, index;
- mconfig->dmic_ch_type = ch_type;
- mconfig->dmic_ch_combo_index = ucontrol->value.integer.value[0];
- /* enum control index 0 is INVALID, so no channels to be set */
- if (mconfig->dmic_ch_combo_index == 0)
return 0;- /* No valid channel selection map for index 0, so offset by 1 */
- index = mconfig->dmic_ch_combo_index - 1;
- switch (ch_type) {
- case SKL_CH_MONO:
list = &mic_mono_list[index];break;- case SKL_CH_STEREO:
list = mic_stereo_list[index];break;- case SKL_CH_TRIO:
list = mic_trio_list[index];break;- case SKL_CH_QUATRO:
list = mic_quatro_list[index];break;How do you guarantee that index is in the array range? It looks easy to make things vulnerable with a firmware.
Yes a good catch, we should right check the index here and see if it is within bounds, will fix it and update
Hi Takashi,
Shouldn't the alsa kernel or user space check for the index of enum control to be within the allowed range? If so, the driver doesn't need to check.
Regards, Subhransu
Thanks
~Vinod
--
On Tue, 30 May 2017 10:40:27 +0200, Subhransu S. Prusty wrote:
On Fri, May 26, 2017 at 01:05:41PM +0530, Vinod Koul wrote:
On Fri, May 26, 2017 at 08:57:44AM +0200, Takashi Iwai wrote:
On Fri, 26 May 2017 05:20:02 +0200, Vinod Koul wrote:
From: "Kranthikumar, GudishaX" gudishax.kranthikumar@intel.com
User may prefer to select data from particular mics. A mic-select module in DSP allows this selection.
Create possible enum controls to allow user to select a combination of mics to capture data from. Based on the user selection, parameters are generated and passed to mic-select module during init.
Signed-off-by: Kranthikumar, GudishaX gudishax.kranthikumar@intel.com Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com
sound/soc/intel/skylake/skl-topology.c | 143 +++++++++++++++++++++++++++ sound/soc/intel/skylake/skl-topology.h | 20 ++++ sound/soc/intel/skylake/skl-tplg-interface.h | 1 + 3 files changed, 164 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index b687ae455a61..141cfbe40c3a 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -36,6 +36,19 @@ #define SKL_IN_DIR_BIT_MASK BIT(0) #define SKL_PIN_COUNT_MASK GENMASK(7, 4)
+static const int mic_mono_list[] = { +0, 1, 2, 3, +}; +static const int mic_stereo_list[][SKL_CH_STEREO] = { +{0, 1}, {0, 2}, {0, 3}, {1, 2}, {1, 3}, {2, 3}, +}; +static const int mic_trio_list[][SKL_CH_TRIO] = { +{0, 1, 2}, {0, 1, 3}, {0, 2, 3}, {1, 2, 3}, +}; +static const int mic_quatro_list[][SKL_CH_QUATRO] = { +{0, 1, 2, 3}, +};
void skl_tplg_d0i3_get(struct skl *skl, enum d0i3_capability caps) { struct skl_d0i3_data *d0i3 = &skl->skl_sst->d0i3; @@ -1314,6 +1327,98 @@ static int skl_tplg_tlv_control_set(struct snd_kcontrol *kcontrol, return 0; }
+static int skl_tplg_mic_control_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol);
- struct skl_module_cfg *mconfig = w->priv;
- struct soc_enum *ec = (struct soc_enum *)kcontrol->private_value;
- u32 ch_type = *((u32 *)ec->dobj.private);
- if (mconfig->dmic_ch_type == ch_type)
ucontrol->value.integer.value[0] = mconfig->dmic_ch_combo_index;- else
ucontrol->value.integer.value[0] = 0;- return 0;
+}
+static int skl_fill_mic_sel_params(struct skl_module_cfg *mconfig,
- struct skl_mic_sel_config *mic_cfg, struct device *dev)
+{
- struct skl_specific_cfg *sp_cfg = &mconfig->formats_config;
- sp_cfg->caps_size = sizeof(struct skl_mic_sel_config);
- sp_cfg->set_params = SKL_PARAM_SET;
- sp_cfg->param_id = 0x00;
- if (!sp_cfg->caps) {
sp_cfg->caps = devm_kzalloc(dev, sp_cfg->caps_size, GFP_KERNEL);if (!sp_cfg->caps)return -ENOMEM;- }
- mic_cfg->mic_switch = SKL_MIC_SEL_SWITCH;
- mic_cfg->flags = 0;
- memcpy(sp_cfg->caps, mic_cfg, sp_cfg->caps_size);
- return 0;
+}
+static int skl_tplg_mic_control_set(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol);
- struct skl_module_cfg *mconfig = w->priv;
- struct skl_mic_sel_config mic_cfg = {0};
- struct soc_enum *ec = (struct soc_enum *)kcontrol->private_value;
- u32 ch_type = *((u32 *)ec->dobj.private);
- const int *list;
- u8 in_ch, out_ch, index;
- mconfig->dmic_ch_type = ch_type;
- mconfig->dmic_ch_combo_index = ucontrol->value.integer.value[0];
- /* enum control index 0 is INVALID, so no channels to be set */
- if (mconfig->dmic_ch_combo_index == 0)
return 0;- /* No valid channel selection map for index 0, so offset by 1 */
- index = mconfig->dmic_ch_combo_index - 1;
- switch (ch_type) {
- case SKL_CH_MONO:
list = &mic_mono_list[index];break;- case SKL_CH_STEREO:
list = mic_stereo_list[index];break;- case SKL_CH_TRIO:
list = mic_trio_list[index];break;- case SKL_CH_QUATRO:
list = mic_quatro_list[index];break;How do you guarantee that index is in the array range? It looks easy to make things vulnerable with a firmware.
Yes a good catch, we should right check the index here and see if it is within bounds, will fix it and update
Hi Takashi,
Shouldn't the alsa kernel or user space check for the index of enum control to be within the allowed range? If so, the driver doesn't need to check.
It's not the enum index at all. Here the index is deduced in a very complex way:
struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol); struct skl_module_cfg *mconfig = w->priv; ..... index = mconfig->dmic_ch_combo_index - 1;
So I feel uneasy unless it's explicitly declared to be really safe.
Takashi
On Tue, 30 May 2017 11:02:01 +0200, Takashi Iwai wrote:
On Tue, 30 May 2017 10:40:27 +0200, Subhransu S. Prusty wrote:
On Fri, May 26, 2017 at 01:05:41PM +0530, Vinod Koul wrote:
On Fri, May 26, 2017 at 08:57:44AM +0200, Takashi Iwai wrote:
On Fri, 26 May 2017 05:20:02 +0200, Vinod Koul wrote:
From: "Kranthikumar, GudishaX" gudishax.kranthikumar@intel.com
User may prefer to select data from particular mics. A mic-select module in DSP allows this selection.
Create possible enum controls to allow user to select a combination of mics to capture data from. Based on the user selection, parameters are generated and passed to mic-select module during init.
Signed-off-by: Kranthikumar, GudishaX gudishax.kranthikumar@intel.com Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com
sound/soc/intel/skylake/skl-topology.c | 143 +++++++++++++++++++++++++++ sound/soc/intel/skylake/skl-topology.h | 20 ++++ sound/soc/intel/skylake/skl-tplg-interface.h | 1 + 3 files changed, 164 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index b687ae455a61..141cfbe40c3a 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -36,6 +36,19 @@ #define SKL_IN_DIR_BIT_MASK BIT(0) #define SKL_PIN_COUNT_MASK GENMASK(7, 4)
+static const int mic_mono_list[] = { +0, 1, 2, 3, +}; +static const int mic_stereo_list[][SKL_CH_STEREO] = { +{0, 1}, {0, 2}, {0, 3}, {1, 2}, {1, 3}, {2, 3}, +}; +static const int mic_trio_list[][SKL_CH_TRIO] = { +{0, 1, 2}, {0, 1, 3}, {0, 2, 3}, {1, 2, 3}, +}; +static const int mic_quatro_list[][SKL_CH_QUATRO] = { +{0, 1, 2, 3}, +};
void skl_tplg_d0i3_get(struct skl *skl, enum d0i3_capability caps) { struct skl_d0i3_data *d0i3 = &skl->skl_sst->d0i3; @@ -1314,6 +1327,98 @@ static int skl_tplg_tlv_control_set(struct snd_kcontrol *kcontrol, return 0; }
+static int skl_tplg_mic_control_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol);
- struct skl_module_cfg *mconfig = w->priv;
- struct soc_enum *ec = (struct soc_enum *)kcontrol->private_value;
- u32 ch_type = *((u32 *)ec->dobj.private);
- if (mconfig->dmic_ch_type == ch_type)
ucontrol->value.integer.value[0] = mconfig->dmic_ch_combo_index;- else
ucontrol->value.integer.value[0] = 0;- return 0;
+}
+static int skl_fill_mic_sel_params(struct skl_module_cfg *mconfig,
- struct skl_mic_sel_config *mic_cfg, struct device *dev)
+{
- struct skl_specific_cfg *sp_cfg = &mconfig->formats_config;
- sp_cfg->caps_size = sizeof(struct skl_mic_sel_config);
- sp_cfg->set_params = SKL_PARAM_SET;
- sp_cfg->param_id = 0x00;
- if (!sp_cfg->caps) {
sp_cfg->caps = devm_kzalloc(dev, sp_cfg->caps_size, GFP_KERNEL);if (!sp_cfg->caps)return -ENOMEM;- }
- mic_cfg->mic_switch = SKL_MIC_SEL_SWITCH;
- mic_cfg->flags = 0;
- memcpy(sp_cfg->caps, mic_cfg, sp_cfg->caps_size);
- return 0;
+}
+static int skl_tplg_mic_control_set(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol);
- struct skl_module_cfg *mconfig = w->priv;
- struct skl_mic_sel_config mic_cfg = {0};
- struct soc_enum *ec = (struct soc_enum *)kcontrol->private_value;
- u32 ch_type = *((u32 *)ec->dobj.private);
- const int *list;
- u8 in_ch, out_ch, index;
- mconfig->dmic_ch_type = ch_type;
- mconfig->dmic_ch_combo_index = ucontrol->value.integer.value[0];
- /* enum control index 0 is INVALID, so no channels to be set */
- if (mconfig->dmic_ch_combo_index == 0)
return 0;- /* No valid channel selection map for index 0, so offset by 1 */
- index = mconfig->dmic_ch_combo_index - 1;
- switch (ch_type) {
- case SKL_CH_MONO:
list = &mic_mono_list[index];break;- case SKL_CH_STEREO:
list = mic_stereo_list[index];break;- case SKL_CH_TRIO:
list = mic_trio_list[index];break;- case SKL_CH_QUATRO:
list = mic_quatro_list[index];break;How do you guarantee that index is in the array range? It looks easy to make things vulnerable with a firmware.
Yes a good catch, we should right check the index here and see if it is within bounds, will fix it and update
Hi Takashi,
Shouldn't the alsa kernel or user space check for the index of enum control to be within the allowed range? If so, the driver doesn't need to check.
It's not the enum index at all. Here the index is deduced in a very complex way:
struct snd_soc_dapm_widget *w = snd_soc_dapm_kcontrol_widget(kcontrol); struct skl_module_cfg *mconfig = w->priv; ..... index = mconfig->dmic_ch_combo_index - 1;
So I feel uneasy unless it's explicitly declared to be really safe.
Wait, further looking at it, it has the following line:
mconfig->dmic_ch_type = ch_type; mconfig->dmic_ch_combo_index = ucontrol->value.integer.value[0];
OK, so it's a *value*, not the index.
And, the value range isn't checked in the control core side. Checking it is the responsibility of the callback.
And, yet another bug in the above: you shouldn't access to ucontrol->value.integer.xxx for enum ctl. Instead, access ucontrol->value.enumerated.xxx.
Takashi
From: "Kranthikumar, GudishaX" gudishax.kranthikumar@intel.com
This patch adds a 4-channel dmic fixup so that DMIC copier will receive 4 channel data and further selection will be done by mic-select module.
Signed-off-by: Kranthikumar, GudishaX gudishax.kranthikumar@intel.com Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com --- sound/soc/intel/boards/bxt_rt298.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/sound/soc/intel/boards/bxt_rt298.c b/sound/soc/intel/boards/bxt_rt298.c index 1a68d043c803..36ee7480e9f1 100644 --- a/sound/soc/intel/boards/bxt_rt298.c +++ b/sound/soc/intel/boards/bxt_rt298.c @@ -222,16 +222,13 @@ static int broxton_dmic_fixup(struct snd_soc_pcm_runtime *rtd, { struct snd_interval *channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); - if (params_channels(params) == 2) - channels->min = channels->max = 2; - else - channels->min = channels->max = 4; + channels->min = channels->max = 4;
return 0; }
static unsigned int channels_dmic[] = { - 2, 4, + 1, 2, 3, 4, };
static struct snd_pcm_hw_constraint_list constraints_dmic_channels = {
The patch
ASoC: Intel: Boards: Add 4-channel DMIC fixup.
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From e8883cb61aa0a91980222e5e9d114100783eb7e2 Mon Sep 17 00:00:00 2001
From: Dharageswari R dharageswari.r@intel.com Date: Wed, 31 May 2017 10:30:26 +0530 Subject: [PATCH] ASoC: Intel: Boards: Add 4-channel DMIC fixup.
This patch adds a 4-channel dmic fixup so that DMIC copier will receive 4 channel data and further selection will be done by mic-select module.
Signed-off-by: Dharageswari R dharageswari.r@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/boards/bxt_rt298.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/sound/soc/intel/boards/bxt_rt298.c b/sound/soc/intel/boards/bxt_rt298.c index 1a68d043c803..36ee7480e9f1 100644 --- a/sound/soc/intel/boards/bxt_rt298.c +++ b/sound/soc/intel/boards/bxt_rt298.c @@ -222,16 +222,13 @@ static int broxton_dmic_fixup(struct snd_soc_pcm_runtime *rtd, { struct snd_interval *channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); - if (params_channels(params) == 2) - channels->min = channels->max = 2; - else - channels->min = channels->max = 4; + channels->min = channels->max = 4;
return 0; }
static unsigned int channels_dmic[] = { - 2, 4, + 1, 2, 3, 4, };
static struct snd_pcm_hw_constraint_list constraints_dmic_channels = {
participants (4)
-
Mark Brown -
Subhransu S. Prusty -
Takashi Iwai -
Vinod Koul