2020-01-03 00:23:29

by Sam McNally

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

Some members of the Google_Hatch family include a rt5682, but not a
speaker amp. When a speaker amp is also present, the first 10EC5682
entry in snd_soc_acpi_intel_cml_machines[] matches, finding the
MX98357A as well, resulting in the quirk_data field in the
snd_soc_acpi_mach being non-null. When only the rt5682 is present, the
second 10EC5682 entry is matched instead and quirk_data is left null.

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

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

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 00:42:46

by Pierre-Louis Bossart

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



On 1/2/20 6:22 PM, Sam McNally wrote:
> Some members of the Google_Hatch family include a rt5682, but not a
> speaker amp. When a speaker amp is also present, the first 10EC5682
> entry in snd_soc_acpi_intel_cml_machines[] matches, finding the
> MX98357A as well, resulting in the quirk_data field in the
> snd_soc_acpi_mach being non-null. When only the rt5682 is present, the
> second 10EC5682 entry is matched instead and quirk_data is left null.

Ah yes, I confused GLK and CML code names.

The code looks fine, but would you mind updating your commit message?
We've reshuffled the 10EC5682 entries to account for the variations with
RT1011, so it's technically the 3rd entry that does not have a speaker
amplifier. And we should also mention that for CML we use the jack codec
as the primary key, so the quirk data does really refer to the speaker
amplifier.

>
> The sof_rt5682 driver's DMI data matching identifies that a speaker amp
> is present for all Google_Hatch family devices. Detect cases where there
> is no speaker amp by checking for a null quirk_data in the
> snd_soc_acpi_mach and remove the speaker amp bit in that case.
>
> Signed-off-by: Sam McNally <[email protected]>
> ---
>
> 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,
>