2020-01-03 01:52:37

by Sam McNally

[permalink] [raw]
Subject: [PATCH v3] ASoC: Intel: sof_rt5682: Ignore the speaker amp when there isn't one.

Some members of the Google_Hatch family include a rt5682 jack codec, but
no speaker amplifier. This uses the same driver (sof_rt5682) as a
combination of rt5682 jack codec and max98357a speaker amplifier. Within
the sof_rt5682 driver, these cases are not currently distinguishable,
relying on a DMI quirk to decide the configuration. This causes an
incorrect configuration when only the rt5682 is present on a
Google_Hatch device.

For CML, the jack codec is used as the primary key when matching,
with a possible speaker amplifier described in quirk_data. The two cases
of interest are the second and third 10EC5682 entries in
snd_soc_acpi_intel_cml_machines[]. The second entry matches the
combination of rt5682 and max98357a, resulting in the quirk_data field
in the snd_soc_acpi_mach being non-null, pointing at
max98357a_spk_codecs, the snd_soc_acpi_codecs for the matched speaker
amplifier. The third entry matches just the rt5682, resulting in a null
quirk_data.

The sof_rt5682 driver's DMI data matching identifies that a speaker
amplifier is present for all Google_Hatch family devices. Detect cases
where there is no speaker amplifier by checking for a null quirk_data in
the snd_soc_acpi_mach and remove the speaker amplifier bit in that case.

Signed-off-by: Sam McNally <[email protected]>
---

Changes in v3:
- Rewrote commit message to refer to correct
snd_soc_acpi_intel_cml_machines[] entries and better describe the
change

Changes in v2:
- Added details about the relevant ACPI matches to the description

sound/soc/intel/boards/sof_rt5682.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/sof_rt5682.c b/sound/soc/intel/boards/sof_rt5682.c
index ad8a2b4bc709..8a13231dee15 100644
--- a/sound/soc/intel/boards/sof_rt5682.c
+++ b/sound/soc/intel/boards/sof_rt5682.c
@@ -603,6 +603,14 @@ static int sof_audio_probe(struct platform_device *pdev)

dmi_check_system(sof_rt5682_quirk_table);

+ mach = (&pdev->dev)->platform_data;
+
+ /* A speaker amp might not be present when the quirk claims one is.
+ * Detect this via whether the machine driver match includes quirk_data.
+ */
+ if ((sof_rt5682_quirk & SOF_SPEAKER_AMP_PRESENT) && !mach->quirk_data)
+ sof_rt5682_quirk &= ~SOF_SPEAKER_AMP_PRESENT;
+
if (soc_intel_is_byt() || soc_intel_is_cht()) {
is_legacy_cpu = 1;
dmic_be_num = 0;
@@ -663,7 +671,6 @@ static int sof_audio_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&ctx->hdmi_pcm_list);

sof_audio_card_rt5682.dev = &pdev->dev;
- mach = (&pdev->dev)->platform_data;

/* set platform name for each dailink */
ret = snd_soc_fixup_dai_links_platform_name(&sof_audio_card_rt5682,
--
2.24.1.735.g03f4e72817-goog


2020-01-03 02:24:51

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [PATCH v3] ASoC: Intel: sof_rt5682: Ignore the speaker amp when there isn't one.



On 1/2/20 7:50 PM, Sam McNally wrote:
> Some members of the Google_Hatch family include a rt5682 jack codec, but
> no speaker amplifier. This uses the same driver (sof_rt5682) as a
> combination of rt5682 jack codec and max98357a speaker amplifier. Within
> the sof_rt5682 driver, these cases are not currently distinguishable,
> relying on a DMI quirk to decide the configuration. This causes an
> incorrect configuration when only the rt5682 is present on a
> Google_Hatch device.
>
> For CML, the jack codec is used as the primary key when matching,
> with a possible speaker amplifier described in quirk_data. The two cases
> of interest are the second and third 10EC5682 entries in
> snd_soc_acpi_intel_cml_machines[]. The second entry matches the
> combination of rt5682 and max98357a, resulting in the quirk_data field
> in the snd_soc_acpi_mach being non-null, pointing at
> max98357a_spk_codecs, the snd_soc_acpi_codecs for the matched speaker
> amplifier. The third entry matches just the rt5682, resulting in a null
> quirk_data.
>
> The sof_rt5682 driver's DMI data matching identifies that a speaker
> amplifier is present for all Google_Hatch family devices. Detect cases
> where there is no speaker amplifier by checking for a null quirk_data in
> the snd_soc_acpi_mach and remove the speaker amplifier bit in that case.
>
> Signed-off-by: Sam McNally <[email protected]>

thanks, this is a great explanation

Acked-by: Pierre-Louis Bossart <[email protected]>


> ---
>
> Changes in v3:
> - Rewrote commit message to refer to correct
> snd_soc_acpi_intel_cml_machines[] entries and better describe the
> change
>
> Changes in v2:
> - Added details about the relevant ACPI matches to the description
>
> sound/soc/intel/boards/sof_rt5682.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/intel/boards/sof_rt5682.c b/sound/soc/intel/boards/sof_rt5682.c
> index ad8a2b4bc709..8a13231dee15 100644
> --- a/sound/soc/intel/boards/sof_rt5682.c
> +++ b/sound/soc/intel/boards/sof_rt5682.c
> @@ -603,6 +603,14 @@ static int sof_audio_probe(struct platform_device *pdev)
>
> dmi_check_system(sof_rt5682_quirk_table);
>
> + mach = (&pdev->dev)->platform_data;
> +
> + /* A speaker amp might not be present when the quirk claims one is.
> + * Detect this via whether the machine driver match includes quirk_data.
> + */
> + if ((sof_rt5682_quirk & SOF_SPEAKER_AMP_PRESENT) && !mach->quirk_data)
> + sof_rt5682_quirk &= ~SOF_SPEAKER_AMP_PRESENT;
> +
> if (soc_intel_is_byt() || soc_intel_is_cht()) {
> is_legacy_cpu = 1;
> dmic_be_num = 0;
> @@ -663,7 +671,6 @@ static int sof_audio_probe(struct platform_device *pdev)
> INIT_LIST_HEAD(&ctx->hdmi_pcm_list);
>
> sof_audio_card_rt5682.dev = &pdev->dev;
> - mach = (&pdev->dev)->platform_data;
>
> /* set platform name for each dailink */
> ret = snd_soc_fixup_dai_links_platform_name(&sof_audio_card_rt5682,
>

2020-01-06 20:59:45

by Mark Brown

[permalink] [raw]
Subject: Applied "ASoC: Intel: sof_rt5682: Ignore the speaker amp when there isn't one." to the asoc tree

The patch

ASoC: Intel: sof_rt5682: Ignore the speaker amp when there isn't one.

has been applied to the asoc tree at

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

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 d4b74e218a8d0d6cf58e546627ab9d4d4f2645ab Mon Sep 17 00:00:00 2001
From: Sam McNally <[email protected]>
Date: Fri, 3 Jan 2020 12:50:19 +1100
Subject: [PATCH] ASoC: Intel: sof_rt5682: Ignore the speaker amp when there
isn't one.

Some members of the Google_Hatch family include a rt5682 jack codec, but
no speaker amplifier. This uses the same driver (sof_rt5682) as a
combination of rt5682 jack codec and max98357a speaker amplifier. Within
the sof_rt5682 driver, these cases are not currently distinguishable,
relying on a DMI quirk to decide the configuration. This causes an
incorrect configuration when only the rt5682 is present on a
Google_Hatch device.

For CML, the jack codec is used as the primary key when matching,
with a possible speaker amplifier described in quirk_data. The two cases
of interest are the second and third 10EC5682 entries in
snd_soc_acpi_intel_cml_machines[]. The second entry matches the
combination of rt5682 and max98357a, resulting in the quirk_data field
in the snd_soc_acpi_mach being non-null, pointing at
max98357a_spk_codecs, the snd_soc_acpi_codecs for the matched speaker
amplifier. The third entry matches just the rt5682, resulting in a null
quirk_data.

The sof_rt5682 driver's DMI data matching identifies that a speaker
amplifier is present for all Google_Hatch family devices. Detect cases
where there is no speaker amplifier by checking for a null quirk_data in
the snd_soc_acpi_mach and remove the speaker amplifier bit in that case.

Signed-off-by: Sam McNally <[email protected]>
Acked-by: Pierre-Louis Bossart <[email protected]>
Link: https://lore.kernel.org/r/20200103124921.v3.1.Ib87c4a7fbb3fc818ea12198e291b87dc2d5bc8c2@changeid
Signed-off-by: Mark Brown <[email protected]>
---
sound/soc/intel/boards/sof_rt5682.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/sof_rt5682.c b/sound/soc/intel/boards/sof_rt5682.c
index ad8a2b4bc709..8a13231dee15 100644
--- a/sound/soc/intel/boards/sof_rt5682.c
+++ b/sound/soc/intel/boards/sof_rt5682.c
@@ -603,6 +603,14 @@ static int sof_audio_probe(struct platform_device *pdev)

dmi_check_system(sof_rt5682_quirk_table);

+ mach = (&pdev->dev)->platform_data;
+
+ /* A speaker amp might not be present when the quirk claims one is.
+ * Detect this via whether the machine driver match includes quirk_data.
+ */
+ if ((sof_rt5682_quirk & SOF_SPEAKER_AMP_PRESENT) && !mach->quirk_data)
+ sof_rt5682_quirk &= ~SOF_SPEAKER_AMP_PRESENT;
+
if (soc_intel_is_byt() || soc_intel_is_cht()) {
is_legacy_cpu = 1;
dmic_be_num = 0;
@@ -663,7 +671,6 @@ static int sof_audio_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&ctx->hdmi_pcm_list);

sof_audio_card_rt5682.dev = &pdev->dev;
- mach = (&pdev->dev)->platform_data;

/* set platform name for each dailink */
ret = snd_soc_fixup_dai_links_platform_name(&sof_audio_card_rt5682,
--
2.20.1