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 & Tweeterspair */
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(structsnd_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.