[PATCH] ASoC: Intel: boards: add stereo playback by woofer speaker
From: Mac Chiang mac.chiang@intel.com
Separate woofer stereo speakers settings.
Signed-off-by: Mac Chiang mac.chiang@intel.com --- sound/soc/intel/boards/cml_rt1011_rt5682.c | 221 +++++++++++++++++++++++------ 1 file changed, 180 insertions(+), 41 deletions(-)
diff --git a/sound/soc/intel/boards/cml_rt1011_rt5682.c b/sound/soc/intel/boards/cml_rt1011_rt5682.c index dd80d01..fc1bd94 100644 --- a/sound/soc/intel/boards/cml_rt1011_rt5682.c +++ b/sound/soc/intel/boards/cml_rt1011_rt5682.c @@ -30,6 +30,36 @@ #define CML_RT5682_CODEC_DAI "rt5682-aif1" #define NAME_SIZE 32
+#define SOF_RT1011_SPEAKER_WL BIT(0) +#define SOF_RT1011_SPEAKER_WR BIT(1) +#define SOF_RT1011_SPEAKER_TL BIT(2) +#define SOF_RT1011_SPEAKER_TR BIT(3) +#define SPK_CH 4 + +/* Default: Woofer+Tweeter speakers */ +static unsigned long sof_rt1011_quirk = SOF_RT1011_SPEAKER_WL | SOF_RT1011_SPEAKER_WR | + SOF_RT1011_SPEAKER_TL | SOF_RT1011_SPEAKER_TR; + +static int sof_rt1011_quirk_cb(const struct dmi_system_id *id) +{ + sof_rt1011_quirk = (unsigned long)id->driver_data; + return 1; +} + +static const struct dmi_system_id sof_rt1011_quirk_table[] = { + { + .callback = sof_rt1011_quirk_cb, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Google"), + DMI_MATCH(DMI_PRODUCT_NAME, "Palkia"), + }, + .driver_data = (void *)(SOF_RT1011_SPEAKER_WL | + SOF_RT1011_SPEAKER_WR), + }, + { + } +}; + static struct snd_soc_jack hdmi_jack[3];
struct hdmi_pcm { @@ -48,15 +78,16 @@ struct card_private { static const struct snd_kcontrol_new cml_controls[] = { SOC_DAPM_PIN_SWITCH("Headphone Jack"), SOC_DAPM_PIN_SWITCH("Headset Mic"), - SOC_DAPM_PIN_SWITCH("TL Ext Spk"), - SOC_DAPM_PIN_SWITCH("TR Ext Spk"), SOC_DAPM_PIN_SWITCH("WL Ext Spk"), SOC_DAPM_PIN_SWITCH("WR Ext Spk"), };
+static const struct snd_kcontrol_new cml_rt1011_tt_controls[] = { + SOC_DAPM_PIN_SWITCH("TL Ext Spk"), + SOC_DAPM_PIN_SWITCH("TR Ext Spk"), +}; + static const struct snd_soc_dapm_widget cml_rt1011_rt5682_widgets[] = { - SND_SOC_DAPM_SPK("TL Ext Spk", NULL), - SND_SOC_DAPM_SPK("TR Ext Spk", NULL), SND_SOC_DAPM_SPK("WL Ext Spk", NULL), SND_SOC_DAPM_SPK("WR Ext Spk", NULL), SND_SOC_DAPM_HP("Headphone Jack", NULL), @@ -64,6 +95,11 @@ static const struct snd_soc_dapm_widget cml_rt1011_rt5682_widgets[] = { SND_SOC_DAPM_MIC("SoC DMIC", NULL), };
+static const struct snd_soc_dapm_widget cml_rt1011_tt_widgets[] = { + SND_SOC_DAPM_SPK("TL Ext Spk", NULL), + SND_SOC_DAPM_SPK("TR Ext Spk", NULL), +}; + static const struct snd_soc_dapm_route cml_rt1011_rt5682_map[] = { /*speaker*/ {"TL Ext Spk", NULL, "TL SPO"}, @@ -82,6 +118,12 @@ static const struct snd_soc_dapm_route cml_rt1011_rt5682_map[] = { {"DMic", NULL, "SoC DMIC"}, };
+static const struct snd_soc_dapm_route cml_rt1011_tt_map[] = { + /*TL/TR speaker*/ + {"TL Ext Spk", NULL, "TL SPO" }, + {"TR Ext Spk", NULL, "TR SPO" }, +}; + static int cml_rt5682_codec_init(struct snd_soc_pcm_runtime *rtd) { struct card_private *ctx = snd_soc_card_get_drvdata(rtd->card); @@ -121,6 +163,35 @@ static int cml_rt5682_codec_init(struct snd_soc_pcm_runtime *rtd) return ret; };
+static int cml_rt1011_spk_init(struct snd_soc_pcm_runtime *rtd) +{ + int ret = 0; + struct snd_soc_card *card = rtd->card; + + if (sof_rt1011_quirk & (SOF_RT1011_SPEAKER_TL | + SOF_RT1011_SPEAKER_TR)) { + + ret = snd_soc_add_card_controls(card, cml_rt1011_tt_controls, + ARRAY_SIZE(cml_rt1011_tt_controls)); + if (ret) + return ret; + + ret = snd_soc_dapm_new_controls(&card->dapm, + cml_rt1011_tt_widgets, + ARRAY_SIZE(cml_rt1011_tt_widgets)); + if (ret) + return ret; + + ret = snd_soc_dapm_add_routes(&card->dapm, cml_rt1011_tt_map, + ARRAY_SIZE(cml_rt1011_tt_map)); + + if (ret) + return ret; + } + + return ret; +} + static int cml_rt5682_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -192,31 +263,52 @@ static int cml_rt1011_hw_params(struct snd_pcm_substream *substream, * The feedback is captured for each codec individually. * Hence all 4 codecs use 1 Tx slot each for feedback. */ - if (!strcmp(codec_dai->component->name, "i2c-10EC1011:00")) { - ret = snd_soc_dai_set_tdm_slot(codec_dai, - 0x4, 0x1, 4, 24); - if (ret < 0) - break; - } - if (!strcmp(codec_dai->component->name, "i2c-10EC1011:02")) { - ret = snd_soc_dai_set_tdm_slot(codec_dai, - 0x1, 0x1, 4, 24); - if (ret < 0) - break; - } - /* TDM Rx slot 2 is used for Right Woofer & Tweeters pair */ - if (!strcmp(codec_dai->component->name, "i2c-10EC1011:01")) { - ret = snd_soc_dai_set_tdm_slot(codec_dai, - 0x8, 0x2, 4, 24); - if (ret < 0) - break; - } - if (!strcmp(codec_dai->component->name, "i2c-10EC1011:03")) { - ret = snd_soc_dai_set_tdm_slot(codec_dai, - 0x2, 0x2, 4, 24); - if (ret < 0) - break; + if (sof_rt1011_quirk & (SOF_RT1011_SPEAKER_TL | + SOF_RT1011_SPEAKER_TR)) { + if (!strcmp(codec_dai->component->name, "i2c-10EC1011:00")) { + ret = snd_soc_dai_set_tdm_slot(codec_dai, + 0x4, 0x1, 4, 24); + if (ret < 0) + break; + } + + if (!strcmp(codec_dai->component->name, "i2c-10EC1011:02")) { + ret = snd_soc_dai_set_tdm_slot(codec_dai, + 0x1, 0x1, 4, 24); + if (ret < 0) + break; + } + + /* TDM Rx slot 2 is used for Right Woofer & Tweeters pair */ + if (!strcmp(codec_dai->component->name, "i2c-10EC1011:01")) { + ret = snd_soc_dai_set_tdm_slot(codec_dai, + 0x8, 0x2, 4, 24); + if (ret < 0) + break; + } + + if (!strcmp(codec_dai->component->name, "i2c-10EC1011:03")) { + ret = snd_soc_dai_set_tdm_slot(codec_dai, + 0x2, 0x2, 4, 24); + if (ret < 0) + break; + } + } else { + if (!strcmp(codec_dai->component->name, "i2c-10EC1011:00")) { + ret = snd_soc_dai_set_tdm_slot(codec_dai, + 0x4, 0x1, 4, 24); + if (ret < 0) + break; + } + + if (!strcmp(codec_dai->component->name, "i2c-10EC1011:01")) { + ret = snd_soc_dai_set_tdm_slot(codec_dai, + 0x8, 0x2, 4, 24); + if (ret < 0) + break; + } } + } if (ret < 0) dev_err(rtd->dev, @@ -302,10 +394,8 @@ SND_SOC_DAILINK_DEF(ssp1_pin, DAILINK_COMP_ARRAY(COMP_CPU("SSP1 Pin"))); SND_SOC_DAILINK_DEF(ssp1_codec, DAILINK_COMP_ARRAY( - /* WL */ COMP_CODEC("i2c-10EC1011:00", CML_RT1011_CODEC_DAI), - /* WR */ COMP_CODEC("i2c-10EC1011:01", CML_RT1011_CODEC_DAI), - /* TL */ COMP_CODEC("i2c-10EC1011:02", CML_RT1011_CODEC_DAI), - /* TR */ COMP_CODEC("i2c-10EC1011:03", CML_RT1011_CODEC_DAI))); + /* WL */ COMP_CODEC("i2c-10EC1011:00", CML_RT1011_CODEC_DAI), + /* WR */ COMP_CODEC("i2c-10EC1011:01", CML_RT1011_CODEC_DAI)));
SND_SOC_DAILINK_DEF(dmic_pin, DAILINK_COMP_ARRAY(COMP_CPU("DMIC01 Pin"))); @@ -399,6 +489,7 @@ static struct snd_soc_dai_link cml_rt1011_rt5682_dailink[] = { .dpcm_playback = 1, .dpcm_capture = 1, /* Capture stream provides Feedback */ .no_pcm = 1, + .init = cml_rt1011_spk_init, .ops = &cml_rt1011_ops, SND_SOC_DAILINK_REG(ssp1_pin, ssp1_codec, platform), }, @@ -413,14 +504,6 @@ static struct snd_soc_codec_conf rt1011_conf[] = { .dlc = COMP_CODEC_CONF("i2c-10EC1011:01"), .name_prefix = "WR", }, - { - .dlc = COMP_CODEC_CONF("i2c-10EC1011:02"), - .name_prefix = "TL", - }, - { - .dlc = COMP_CODEC_CONF("i2c-10EC1011:03"), - .name_prefix = "TR", - }, };
/* Cometlake audio machine driver for RT1011 and RT5682 */ @@ -442,10 +525,12 @@ static struct snd_soc_card snd_soc_card_cml = {
static int snd_cml_rt1011_probe(struct platform_device *pdev) { + struct snd_soc_dai_link_component *rt1011_dais_components; + struct snd_soc_codec_conf *rt1011_dais_confs; struct card_private *ctx; struct snd_soc_acpi_mach *mach; const char *platform_name; - int ret; + int ret, i;
ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC); if (!ctx) @@ -456,6 +541,59 @@ static int snd_cml_rt1011_probe(struct platform_device *pdev) snd_soc_card_cml.dev = &pdev->dev; platform_name = mach->mach_params.platform;
+ dmi_check_system(sof_rt1011_quirk_table); + + dev_info(&pdev->dev, "sof_rt1011_quirk = %lx\n", sof_rt1011_quirk); + + if (sof_rt1011_quirk & (SOF_RT1011_SPEAKER_TL | + SOF_RT1011_SPEAKER_TR)) { + rt1011_dais_confs = devm_kzalloc(&pdev->dev, + sizeof(struct snd_soc_codec_conf) * + SPK_CH, GFP_KERNEL); + + rt1011_dais_components = devm_kzalloc(&pdev->dev, + sizeof(struct snd_soc_dai_link_component) * + SPK_CH, GFP_KERNEL); + + for (i = 0; i < SPK_CH; i++) { + rt1011_dais_confs[i].dlc.name = devm_kasprintf(&pdev->dev, + GFP_KERNEL, + "i2c-10EC1011:0%d", + i); + switch (i) { + case 0: + rt1011_dais_confs[i].name_prefix = "WL"; + break; + + case 1: + rt1011_dais_confs[i].name_prefix = "WR"; + break; + case 2: + rt1011_dais_confs[i].name_prefix = "TL"; + break; + case 3: + rt1011_dais_confs[i].name_prefix = "TR"; + break; + } + rt1011_dais_components[i].name = devm_kasprintf(&pdev->dev, + GFP_KERNEL, + "i2c-10EC1011:0%d", + i); + rt1011_dais_components[i].dai_name = CML_RT1011_CODEC_DAI; + } + + snd_soc_card_cml.codec_conf = rt1011_dais_confs; + snd_soc_card_cml.num_configs = SPK_CH; + + for (i = 0; i < ARRAY_SIZE(cml_rt1011_rt5682_dailink); i++) { + if (!strcmp(cml_rt1011_rt5682_dailink[i].codecs->dai_name, + CML_RT1011_CODEC_DAI)) { + cml_rt1011_rt5682_dailink[i].codecs = rt1011_dais_components; + cml_rt1011_rt5682_dailink[i].num_codecs = SPK_CH; + } + } + } + /* set platform name for each dailink */ ret = snd_soc_fixup_dai_links_platform_name(&snd_soc_card_cml, platform_name); @@ -483,5 +621,6 @@ MODULE_DESCRIPTION("Cometlake Audio Machine driver - RT1011 and RT5682 in I2S mo MODULE_AUTHOR("Naveen Manohar naveen.m@intel.com"); MODULE_AUTHOR("Sathya Prakash M R sathya.prakash.m.r@intel.com"); MODULE_AUTHOR("Shuming Fan shumingf@realtek.com"); +MODULE_AUTHOR("Mac Chiang mac.chiang@intel.com"); MODULE_LICENSE("GPL v2"); MODULE_ALIAS("platform:cml_rt1011_rt5682");
Hi Mac,
+#define SOF_RT1011_SPEAKER_WL BIT(0) +#define SOF_RT1011_SPEAKER_WR BIT(1) +#define SOF_RT1011_SPEAKER_TL BIT(2) +#define SOF_RT1011_SPEAKER_TR BIT(3) +#define SPK_CH 4
use a prefix maybe for consistency? It's also unclear why this is needed when you can have 2 or more channels, and looking below
+/* Default: Woofer+Tweeter speakers */
It's more like ALL devices have Woofers. Some devices don't have tweeters.
the WL and WR quirks are always on apparently.
+static unsigned long sof_rt1011_quirk = SOF_RT1011_SPEAKER_WL | SOF_RT1011_SPEAKER_WR |
SOF_RT1011_SPEAKER_TL | SOF_RT1011_SPEAKER_TR;
+static int sof_rt1011_quirk_cb(const struct dmi_system_id *id) +{
- sof_rt1011_quirk = (unsigned long)id->driver_data;
- return 1;
+}
+static const struct dmi_system_id sof_rt1011_quirk_table[] = {
- {
.callback = sof_rt1011_quirk_cb,
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Google"),
DMI_MATCH(DMI_PRODUCT_NAME, "Palkia"),
- },
.driver_data = (void *)(SOF_RT1011_SPEAKER_WL |
SOF_RT1011_SPEAKER_WR),
- },
- {
- }
+};
+static const struct snd_soc_dapm_widget cml_rt1011_tt_widgets[] = {
- SND_SOC_DAPM_SPK("TL Ext Spk", NULL),
- SND_SOC_DAPM_SPK("TR Ext Spk", NULL),
+};
- static const struct snd_soc_dapm_route cml_rt1011_rt5682_map[] = { /*speaker*/ {"TL Ext Spk", NULL, "TL SPO"},
Something's not right, if I look at the code after applying this patch I get:
static const struct snd_soc_dapm_route cml_rt1011_rt5682_map[] = { /*speaker*/ {"TL Ext Spk", NULL, "TL SPO"}, {"TR Ext Spk", NULL, "TR SPO"},
That's duplicaged from [1]
@@ -82,6 +118,12 @@ static const struct snd_soc_dapm_route cml_rt1011_rt5682_map[] = { {"DMic", NULL, "SoC DMIC"}, };
+static const struct snd_soc_dapm_route cml_rt1011_tt_map[] = {
- /*TL/TR speaker*/
- {"TL Ext Spk", NULL, "TL SPO" },
- {"TR Ext Spk", NULL, "TR SPO" },
+};
[1] we should remove the tweeeter maps in cml_rt1011_rt5682_map, no?
static int cml_rt5682_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -192,31 +263,52 @@ static int cml_rt1011_hw_params(struct snd_pcm_substream *substream, * The feedback is captured for each codec individually. * Hence all 4 codecs use 1 Tx slot each for feedback. */
if (!strcmp(codec_dai->component->name, "i2c-10EC1011:00")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x4, 0x1, 4, 24);
if (ret < 0)
break;
}
if (!strcmp(codec_dai->component->name, "i2c-10EC1011:02")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x1, 0x1, 4, 24);
if (ret < 0)
break;
}
/* TDM Rx slot 2 is used for Right Woofer & Tweeters pair */
if (!strcmp(codec_dai->component->name, "i2c-10EC1011:01")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x8, 0x2, 4, 24);
if (ret < 0)
break;
}
if (!strcmp(codec_dai->component->name, "i2c-10EC1011:03")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x2, 0x2, 4, 24);
if (ret < 0)
break;
if (sof_rt1011_quirk & (SOF_RT1011_SPEAKER_TL |
SOF_RT1011_SPEAKER_TR)) {
if (!strcmp(codec_dai->component->name, "i2c-10EC1011:00")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x4, 0x1, 4, 24);
if (ret < 0)
break;
}
if (!strcmp(codec_dai->component->name, "i2c-10EC1011:02")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x1, 0x1, 4, 24);
if (ret < 0)
break;
}
/* TDM Rx slot 2 is used for Right Woofer & Tweeters pair */
if (!strcmp(codec_dai->component->name, "i2c-10EC1011:01")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x8, 0x2, 4, 24);
if (ret < 0)
break;
}
if (!strcmp(codec_dai->component->name, "i2c-10EC1011:03")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x2, 0x2, 4, 24);
if (ret < 0)
break;
}
} else {
if (!strcmp(codec_dai->component->name, "i2c-10EC1011:00")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x4, 0x1, 4, 24);
if (ret < 0)
break;
}
if (!strcmp(codec_dai->component->name, "i2c-10EC1011:01")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x8, 0x2, 4, 24);
if (ret < 0)
break;
}}
the if/else case can be simplified. The baseline is two woofers, so they can be added unconditionally, and then you can add what's missing for the tweeters. That way you have a consistent way of handling the TL/TR quirk.
static int snd_cml_rt1011_probe(struct platform_device *pdev) {
- struct snd_soc_dai_link_component *rt1011_dais_components;
- struct snd_soc_codec_conf *rt1011_dais_confs; struct card_private *ctx; struct snd_soc_acpi_mach *mach; const char *platform_name;
- int ret;
int ret, i;
ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
D'oh! Did we again let this slip in?
cml_rt1011_rt5682.c: ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC); sof_da7219_max98373.c: ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
This should be fixed in a separate patch, we don't need th ATOMIC attribute in any machine drivers - copy-paste!
if (!ctx) @@ -456,6 +541,59 @@ static int snd_cml_rt1011_probe(struct platform_device *pdev) snd_soc_card_cml.dev = &pdev->dev; platform_name = mach->mach_params.platform;
- dmi_check_system(sof_rt1011_quirk_table);
- dev_info(&pdev->dev, "sof_rt1011_quirk = %lx\n", sof_rt1011_quirk);
- if (sof_rt1011_quirk & (SOF_RT1011_SPEAKER_TL |
SOF_RT1011_SPEAKER_TR)) {
rt1011_dais_confs = devm_kzalloc(&pdev->dev,
sizeof(struct snd_soc_codec_conf) *
SPK_CH, GFP_KERNEL);
rt1011_dais_components = devm_kzalloc(&pdev->dev,
sizeof(struct snd_soc_dai_link_component) *
SPK_CH, GFP_KERNEL);
for (i = 0; i < SPK_CH; i++) {
rt1011_dais_confs[i].dlc.name = devm_kasprintf(&pdev->dev,
GFP_KERNEL,
"i2c-10EC1011:0%d",
i);
Check for NULL and return -ENOMEM for all 3 devm_ calls above?
Hi Mac,
+#define SOF_RT1011_SPEAKER_WL BIT(0) +#define SOF_RT1011_SPEAKER_WR BIT(1) +#define SOF_RT1011_SPEAKER_TL BIT(2) +#define SOF_RT1011_SPEAKER_TR BIT(3) +#define SPK_CH 4
use a prefix maybe for consistency?
Yes. Considering that we have Woofers and Tweeters speakers consistencies.
It's also unclear why this is needed when you can have 2 or more channels, and looking below
+/* Default: Woofer+Tweeter speakers */
/* Default: Woofer speakers */
It's more like ALL devices have Woofers. Some devices don't have tweeters.
the WL and WR quirks are always on apparently.
Yes, you are right. The purpose to do is Woofers as default and Tweeters+Woofers as specific project. I revised them below. Or any advices?
+static unsigned long sof_rt1011_quirk = SOF_RT1011_SPEAKER_WL |
SOF_RT1011_SPEAKER_WR |
SOF_RT1011_SPEAKER_TL |
SOF_RT1011_SPEAKER_TR;
static unsigned long sof_rt1011_quirk = SOF_RT1011_SPEAKER_WL | SOF_RT1011_SPEAKER_WR;
+static int sof_rt1011_quirk_cb(const struct dmi_system_id *id) {
- sof_rt1011_quirk = (unsigned long)id->driver_data;
- return 1;
+}
+static const struct dmi_system_id sof_rt1011_quirk_table[] = {
- {
.callback = sof_rt1011_quirk_cb,
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Google"),
DMI_MATCH(DMI_PRODUCT_NAME, "Palkia"),
DMI_MATCH(DMI_PRODUCT_NAME, "Helios"),
- },
.driver_data = (void *)(SOF_RT1011_SPEAKER_WL |
SOF_RT1011_SPEAKER_WR),
.driver_data = (void *)(SOF_RT1011_SPEAKER_WL | SOF_RT1011_SPEAKER_WR | SOF_RT1011_SPEAKER_TL | SOF_RT1011_SPEAKER_TR),
- },
- {
- }
+};
+static const struct snd_soc_dapm_widget cml_rt1011_tt_widgets[] = {
- SND_SOC_DAPM_SPK("TL Ext Spk", NULL),
- SND_SOC_DAPM_SPK("TR Ext Spk", NULL), };
- static const struct snd_soc_dapm_route cml_rt1011_rt5682_map[] = { /*speaker*/ {"TL Ext Spk", NULL, "TL SPO"},
Something's not right, if I look at the code after applying this patch I get:
static const struct snd_soc_dapm_route cml_rt1011_rt5682_map[] = { /*speaker*/ {"TL Ext Spk", NULL, "TL SPO"}, {"TR Ext Spk", NULL, "TR SPO"},
That's duplicaged from [1]
True. My mistake, just remain one(apart from rt1011_rt5682_map).
@@ -82,6 +118,12 @@ static const struct snd_soc_dapm_route
cml_rt1011_rt5682_map[] = {
{"DMic", NULL, "SoC DMIC"}, };
+static const struct snd_soc_dapm_route cml_rt1011_tt_map[] = {
- /*TL/TR speaker*/
- {"TL Ext Spk", NULL, "TL SPO" },
- {"TR Ext Spk", NULL, "TR SPO" },
+};
[1] we should remove the tweeeter maps in cml_rt1011_rt5682_map, no?
Yes. Remove tweeters from rt1011_rt5682_map.
static int cml_rt5682_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -192,31 +263,52 @@ static int cml_rt1011_hw_params(struct
snd_pcm_substream *substream,
* The feedback is captured for each codec individually. * Hence all 4 codecs use 1 Tx slot each for feedback. */
if (!strcmp(codec_dai->component->name, "i2c-
10EC1011:00")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x4, 0x1, 4, 24);
if (ret < 0)
break;
}
if (!strcmp(codec_dai->component->name, "i2c-
10EC1011:02")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x1, 0x1, 4, 24);
if (ret < 0)
break;
}
/* TDM Rx slot 2 is used for Right Woofer & Tweeters pair */
if (!strcmp(codec_dai->component->name, "i2c-
10EC1011:01")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x8, 0x2, 4, 24);
if (ret < 0)
break;
}
if (!strcmp(codec_dai->component->name, "i2c-
10EC1011:03")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x2, 0x2, 4, 24);
if (ret < 0)
break;
if (sof_rt1011_quirk & (SOF_RT1011_SPEAKER_TL |
SOF_RT1011_SPEAKER_TR)) {
if (!strcmp(codec_dai->component->name, "i2c-
10EC1011:00")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x4, 0x1, 4, 24);
if (ret < 0)
break;
}
if (!strcmp(codec_dai->component->name, "i2c-
10EC1011:02")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x1, 0x1, 4, 24);
if (ret < 0)
break;
}
/* TDM Rx slot 2 is used for Right Woofer & Tweeters
pair */
if (!strcmp(codec_dai->component->name, "i2c-
10EC1011:01")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x8, 0x2, 4, 24);
if (ret < 0)
break;
}
if (!strcmp(codec_dai->component->name, "i2c-
10EC1011:03")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x2, 0x2, 4, 24);
if (ret < 0)
break;
}
} else {
if (!strcmp(codec_dai->component->name, "i2c-
10EC1011:00")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x4, 0x1, 4, 24);
if (ret < 0)
break;
}
if (!strcmp(codec_dai->component->name, "i2c-
10EC1011:01")) {
ret = snd_soc_dai_set_tdm_slot(codec_dai,
0x8, 0x2, 4, 24);
if (ret < 0)
break;
}}
the if/else case can be simplified. The baseline is two woofers, so they can be added unconditionally, and then you can add what's missing for the tweeters. That way you have a consistent way of handling the TL/TR quirk.
Thanks for addressing. I revised the if/else logic below if (sof_rt1011_quirk & (SOF_RT1011_SPEAKER_WL | SOF_RT1011_SPEAKER_WR)) { /* TDM slots for WL/WR */ .... } if (sof_rt1011_quirk & (SOF_RT1011_SPEAKER_TL | SOF_RT1011_SPEAKER_TR)) { /* TDM slots for TL/TR */ ... }
static int snd_cml_rt1011_probe(struct platform_device *pdev) {
- struct snd_soc_dai_link_component *rt1011_dais_components;
- struct snd_soc_codec_conf *rt1011_dais_confs; struct card_private *ctx; struct snd_soc_acpi_mach *mach; const char *platform_name;
- int ret;
int ret, i;
ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
D'oh! Did we again let this slip in?
cml_rt1011_rt5682.c: ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC); sof_da7219_max98373.c: ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
This should be fixed in a separate patch, we don't need th ATOMIC attribute in any machine drivers - copy-paste!
Copy that.
if (!ctx) @@ -456,6 +541,59 @@ static int snd_cml_rt1011_probe(struct
platform_device *pdev)
snd_soc_card_cml.dev = &pdev->dev; platform_name = mach->mach_params.platform;
- dmi_check_system(sof_rt1011_quirk_table);
- dev_info(&pdev->dev, "sof_rt1011_quirk = %lx\n", sof_rt1011_quirk);
- if (sof_rt1011_quirk & (SOF_RT1011_SPEAKER_TL |
SOF_RT1011_SPEAKER_TR)) {
rt1011_dais_confs = devm_kzalloc(&pdev->dev,
sizeof(struct snd_soc_codec_conf) *
SPK_CH, GFP_KERNEL);
rt1011_dais_components = devm_kzalloc(&pdev->dev,
sizeof(struct
snd_soc_dai_link_component) *
SPK_CH, GFP_KERNEL);
for (i = 0; i < SPK_CH; i++) {
rt1011_dais_confs[i].dlc.name =
devm_kasprintf(&pdev->dev,
GFP_KERNEL,
"i2c-
10EC1011:0%d",
i);
Check for NULL and return -ENOMEM for all 3 devm_ calls above?
Yes, I will add the check conditions separately. Afterwards I will revise and submit the entirely new one.
participants (3)
-
Chiang, Mac
-
mac.chiang@intel.com
-
Pierre-Louis Bossart