* [PATCH] Add support for Sabre HiFi on System76 and Clevo laptops
@ 2017-02-01 20:57 Jeremy Soller
2017-02-08 13:11 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Jeremy Soller @ 2017-02-01 20:57 UTC (permalink / raw)
To: alsa-devel
This patch adds support for the ESS Sabre HiFi DAC found on some
System76 and Clevo laptops. These devices can be identified using the
Clevo subvendor id, and the Realtek ALC898 codec vendor ID. When a
connection is detected on the ESS DAC, identified by GPIO pin 2 being
set low, the 0x1b pin VREF should be set to 80%. Otherwise, it should be
set to HIZ. Testing has been performed on the current System76 line of
laptops, with and without DACs, no regressions have been found.
---
sound/pci/hda/patch_realtek.c | 53
++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 52 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_realtek.c
b/sound/pci/hda/patch_realtek.c
index 7d660ee..ed15317 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -1797,6 +1797,7 @@ enum {
ALC882_FIXUP_NO_PRIMARY_HP,
ALC887_FIXUP_ASUS_BASS,
ALC887_FIXUP_BASS_CHMAP,
+ ALC898_FIXUP_CLEVO_SPDIF,
};
static void alc889_fixup_coef(struct hda_codec *codec,
@@ -1956,6 +1957,43 @@ static void alc882_fixup_no_primary_hp(struct
hda_codec *codec,
}
}
+static void alc898_clevo_dac_hook(struct hda_codec *codec,
+ struct hda_jack_callback *jack)
+{
+ int val;
+
+ // Read ESS DAC status
+ snd_hda_codec_write(codec, codec->core.afg, 0,
AC_VERB_SET_GPIO_MASK, 4);
+ snd_hda_codec_write(codec, codec->core.afg, 0,
AC_VERB_SET_GPIO_DIRECTION, 0);
+ val = snd_hda_codec_read(codec, codec->core.afg, 0,
AC_VERB_GET_GPIO_DATA, 0);
+ val &= 0x04;
+
+ if (val == 0) {
+ // Set VREF on headphone pin to 80%
+ val = snd_hda_codec_get_pin_target(codec, 0x1b);
+ val |= AC_PINCTL_VREF_80;
+ snd_hda_set_pin_ctl(codec, 0x1b, val);
+ } else {
+ // Set VREF on headphone pin to HIZ
+ val = snd_hda_codec_get_pin_target(codec, 0x1b);
+ val = val & 0xfff8;
+ snd_hda_set_pin_ctl(codec, 0x1b, val);
+ }
+}
+
+static void alc898_fixup_clevo(struct hda_codec *codec,
+ const struct hda_fixup *fix, int
action)
+{
+ switch (action) {
+ case HDA_FIXUP_ACT_PRE_PROBE:
+ snd_hda_jack_detect_enable_callback(codec, 0x1b,
alc898_clevo_dac_hook);
+ break;
+ case HDA_FIXUP_ACT_INIT:
+ snd_hda_codec_read(codec, 0x1b, 0,
AC_VERB_SET_PIN_WIDGET_CONTROL, 4);
+ break;
+ }
+}
+
static void alc_fixup_bass_chmap(struct hda_codec *codec,
const struct hda_fixup *fix, int
action);
@@ -2195,6 +2233,10 @@ static const struct hda_fixup alc882_fixups[] = {
.type = HDA_FIXUP_FUNC,
.v.func = alc_fixup_bass_chmap,
},
+ [ALC898_FIXUP_CLEVO_SPDIF] = {
+ .type = HDA_FIXUP_FUNC,
+ .v.func = alc898_fixup_clevo,
+ },
};
static const struct snd_pci_quirk alc882_fixup_tbl[] = {
@@ -2271,6 +2313,10 @@ static const struct snd_pci_quirk
alc882_fixup_tbl[] = {
SND_PCI_QUIRK(0x8086, 0x0022, "DX58SO", ALC889_FIXUP_COEF),
{}
};
+static const struct snd_pci_quirk alc898_fixup_tbl[] = {
+ SND_PCI_QUIRK_VENDOR(0x1558, "Clevo laptop",
ALC898_FIXUP_CLEVO_SPDIF),
+ {}
+};
static const struct hda_model_fixup alc882_fixup_models[] = {
{.id = ALC882_FIXUP_ACER_ASPIRE_4930G, .name =
"acer-aspire-4930g"},
@@ -2310,6 +2356,11 @@ static int patch_alc882(struct hda_codec *codec)
case 0x10ec0885:
case 0x10ec0900:
break;
+ case 0x10ec0898:
+ case 0x10ec0899:
+ snd_hda_pick_fixup(codec, NULL, alc898_fixup_tbl,
+ alc882_fixups);
+ break;
default:
/* ALC883 and variants */
alc_fix_pll_init(codec, 0x20, 0x0a, 10);
@@ -4357,7 +4408,7 @@ static void
alc_fixup_headset_mode_alc255_no_hp_mic(struct hda_codec *codec,
struct alc_spec *spec = codec->spec;
spec->parse_flags |= HDA_PINCFG_HEADSET_MIC;
alc255_set_default_jack_type(codec);
- }
+ }
else
alc_fixup_headset_mode(codec, fix, action);
}
--
2.9.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] Add support for Sabre HiFi on System76 and Clevo laptops
2017-02-01 20:57 [PATCH] Add support for Sabre HiFi on System76 and Clevo laptops Jeremy Soller
@ 2017-02-08 13:11 ` Takashi Iwai
0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2017-02-08 13:11 UTC (permalink / raw)
To: Jeremy Soller; +Cc: alsa-devel
On Wed, 01 Feb 2017 21:57:13 +0100,
Jeremy Soller wrote:
>
> This patch adds support for the ESS Sabre HiFi DAC found on some
> System76 and Clevo laptops. These devices can be identified using the
> Clevo subvendor id, and the Realtek ALC898 codec vendor ID. When a
> connection is detected on the ESS DAC, identified by GPIO pin 2 being
> set low, the 0x1b pin VREF should be set to 80%. Otherwise, it should be
> set to HIZ. Testing has been performed on the current System76 line of
> laptops, with and without DACs, no regressions have been found.
>
Please give your sign off. Otherwise it can't be merged at all.
Looking at the changes, the changes themselves look almost good.
But the patch isn't applicable as is because of spacing or line
break. Please fix your MUA setup or use an attachment if it really
doesn't work.
Also, last but not least, try to run checkpatch.pl and fix the
reported issues there.
thanks,
Takashi
> ---
> sound/pci/hda/patch_realtek.c | 53
> ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/sound/pci/hda/patch_realtek.c
> b/sound/pci/hda/patch_realtek.c
> index 7d660ee..ed15317 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -1797,6 +1797,7 @@ enum {
> ALC882_FIXUP_NO_PRIMARY_HP,
> ALC887_FIXUP_ASUS_BASS,
> ALC887_FIXUP_BASS_CHMAP,
> + ALC898_FIXUP_CLEVO_SPDIF,
> };
>
> static void alc889_fixup_coef(struct hda_codec *codec,
> @@ -1956,6 +1957,43 @@ static void alc882_fixup_no_primary_hp(struct
> hda_codec *codec,
> }
> }
>
> +static void alc898_clevo_dac_hook(struct hda_codec *codec,
> + struct hda_jack_callback *jack)
> +{
> + int val;
> +
> + // Read ESS DAC status
> + snd_hda_codec_write(codec, codec->core.afg, 0,
> AC_VERB_SET_GPIO_MASK, 4);
> + snd_hda_codec_write(codec, codec->core.afg, 0,
> AC_VERB_SET_GPIO_DIRECTION, 0);
> + val = snd_hda_codec_read(codec, codec->core.afg, 0,
> AC_VERB_GET_GPIO_DATA, 0);
> + val &= 0x04;
> +
> + if (val == 0) {
> + // Set VREF on headphone pin to 80%
> + val = snd_hda_codec_get_pin_target(codec, 0x1b);
> + val |= AC_PINCTL_VREF_80;
> + snd_hda_set_pin_ctl(codec, 0x1b, val);
> + } else {
> + // Set VREF on headphone pin to HIZ
> + val = snd_hda_codec_get_pin_target(codec, 0x1b);
> + val = val & 0xfff8;
> + snd_hda_set_pin_ctl(codec, 0x1b, val);
> + }
> +}
> +
> +static void alc898_fixup_clevo(struct hda_codec *codec,
> + const struct hda_fixup *fix, int
> action)
> +{
> + switch (action) {
> + case HDA_FIXUP_ACT_PRE_PROBE:
> + snd_hda_jack_detect_enable_callback(codec, 0x1b,
> alc898_clevo_dac_hook);
> + break;
> + case HDA_FIXUP_ACT_INIT:
> + snd_hda_codec_read(codec, 0x1b, 0,
> AC_VERB_SET_PIN_WIDGET_CONTROL, 4);
> + break;
> + }
> +}
> +
> static void alc_fixup_bass_chmap(struct hda_codec *codec,
> const struct hda_fixup *fix, int
> action);
>
> @@ -2195,6 +2233,10 @@ static const struct hda_fixup alc882_fixups[] = {
> .type = HDA_FIXUP_FUNC,
> .v.func = alc_fixup_bass_chmap,
> },
> + [ALC898_FIXUP_CLEVO_SPDIF] = {
> + .type = HDA_FIXUP_FUNC,
> + .v.func = alc898_fixup_clevo,
> + },
> };
>
> static const struct snd_pci_quirk alc882_fixup_tbl[] = {
> @@ -2271,6 +2313,10 @@ static const struct snd_pci_quirk
> alc882_fixup_tbl[] = {
> SND_PCI_QUIRK(0x8086, 0x0022, "DX58SO", ALC889_FIXUP_COEF),
> {}
> };
> +static const struct snd_pci_quirk alc898_fixup_tbl[] = {
> + SND_PCI_QUIRK_VENDOR(0x1558, "Clevo laptop",
> ALC898_FIXUP_CLEVO_SPDIF),
> + {}
> +};
>
> static const struct hda_model_fixup alc882_fixup_models[] = {
> {.id = ALC882_FIXUP_ACER_ASPIRE_4930G, .name =
> "acer-aspire-4930g"},
> @@ -2310,6 +2356,11 @@ static int patch_alc882(struct hda_codec *codec)
> case 0x10ec0885:
> case 0x10ec0900:
> break;
> + case 0x10ec0898:
> + case 0x10ec0899:
> + snd_hda_pick_fixup(codec, NULL, alc898_fixup_tbl,
> + alc882_fixups);
> + break;
> default:
> /* ALC883 and variants */
> alc_fix_pll_init(codec, 0x20, 0x0a, 10);
> @@ -4357,7 +4408,7 @@ static void
> alc_fixup_headset_mode_alc255_no_hp_mic(struct hda_codec *codec,
> struct alc_spec *spec = codec->spec;
> spec->parse_flags |= HDA_PINCFG_HEADSET_MIC;
> alc255_set_default_jack_type(codec);
> - }
> + }
> else
> alc_fixup_headset_mode(codec, fix, action);
> }
> --
> 2.9.3
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] Add support for Sabre HiFi on System76 and Clevo laptops
@ 2019-08-11 22:19 Dennis Mungai
2019-08-14 6:46 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Dennis Mungai @ 2019-08-11 22:19 UTC (permalink / raw)
To: alsa-devel; +Cc: Takashi Iwai, jackpot51
[-- Attachment #1: Type: text/plain, Size: 333 bytes --]
Hello there,
The patch attached below is based on Jeremy Soller's prior work on:
https://patchwork.kernel.org/patch/9552671/ , copied herein.
Cleaned up to pass checkpatch.pl tests.
I can confirm that this patch does indeed fix the audio DAC init on
the Clevo based P775TM1-R chassis with no issues.
Warm regards,
Dennis Mungai
[-- Attachment #2: 0001-Fix-ESS-Sabre-DAC-init-for-Clevo-laptops.patch --]
[-- Type: text/x-patch, Size: 3495 bytes --]
From 9f6b7f51a8be738bb588e8d6b0f4d9fb8ac5a0ce Mon Sep 17 00:00:00 2001
From: brainiarc7 <dmngaie@gmail.com>
Date: Mon, 12 Aug 2019 00:43:55 +0300
Subject: [PATCH 1/1] Fix ESS Sabre DAC init for Clevo laptops
Signed-off-by: brainiarc7 <dmngaie@gmail.com>
---
sound/pci/hda/patch_realtek.c | 54 ++++++++++++++++++++++++++++++++++-
1 file changed, 53 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index de224cbea..d8f9862cb 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -1899,6 +1899,7 @@ enum {
ALC882_FIXUP_NO_PRIMARY_HP,
ALC887_FIXUP_ASUS_BASS,
ALC887_FIXUP_BASS_CHMAP,
+ ALC898_FIXUP_CLEVO_SPDIF,
ALC1220_FIXUP_GB_DUAL_CODECS,
ALC1220_FIXUP_CLEVO_P950,
ALC1220_FIXUP_CLEVO_PB51ED,
@@ -2029,6 +2030,43 @@ static void alc882_fixup_no_primary_hp(struct hda_codec *codec,
}
}
+static void alc898_clevo_dac_hook(struct hda_codec *codec,
+ struct hda_jack_callback *jack)
+{
+ int val;
+
+ // Read ESS DAC status
+ snd_hda_codec_write(codec, codec->core.afg, 0, AC_VERB_SET_GPIO_MASK, 4);
+ snd_hda_codec_write(codec, codec->core.afg, 0, AC_VERB_SET_GPIO_DIRECTION, 0);
+ val = snd_hda_codec_read(codec, codec->core.afg, 0, AC_VERB_GET_GPIO_DATA, 0);
+ val &= 0x04;
+
+ if (val == 0) {
+ // Set VREF on headphone pin to 80%
+ val = snd_hda_codec_get_pin_target(codec, 0x1b);
+ val |= AC_PINCTL_VREF_80;
+ snd_hda_set_pin_ctl(codec, 0x1b, val);
+ } else {
+ // Set VREF on headphone pin to HIZ
+ val = snd_hda_codec_get_pin_target(codec, 0x1b);
+ val = val & 0xfff8;
+ snd_hda_set_pin_ctl(codec, 0x1b, val);
+ }
+}
+
+static void alc898_fixup_clevo(struct hda_codec *codec,
+ const struct hda_fixup *fix, int action)
+{
+ switch (action) {
+ case HDA_FIXUP_ACT_PRE_PROBE:
+ snd_hda_jack_detect_enable_callback(codec, 0x1b, alc898_clevo_dac_hook);
+ break;
+ case HDA_FIXUP_ACT_INIT:
+ snd_hda_codec_read(codec, 0x1b, 0, AC_VERB_SET_PIN_WIDGET_CONTROL, 4);
+ break;
+ }
+}
+
static void alc_fixup_bass_chmap(struct hda_codec *codec,
const struct hda_fixup *fix, int action);
@@ -2350,7 +2388,11 @@ static const struct hda_fixup alc882_fixups[] = {
[ALC887_FIXUP_BASS_CHMAP] = {
.type = HDA_FIXUP_FUNC,
.v.func = alc_fixup_bass_chmap,
- },
+ }, [ALC898_FIXUP_CLEVO_SPDIF] = {
+ .type = HDA_FIXUP_FUNC,
+ .v.func = alc898_fixup_clevo,
+ },
+
[ALC1220_FIXUP_GB_DUAL_CODECS] = {
.type = HDA_FIXUP_FUNC,
.v.func = alc1220_fixup_gb_dual_codecs,
@@ -2459,6 +2501,11 @@ static const struct snd_pci_quirk alc882_fixup_tbl[] = {
{}
};
+static const struct snd_pci_quirk alc898_fixup_tbl[] = {
+ SND_PCI_QUIRK_VENDOR(0x1558, "Clevo laptop", ALC898_FIXUP_CLEVO_SPDIF),
+ {}
+};
+
static const struct hda_model_fixup alc882_fixup_models[] = {
{.id = ALC882_FIXUP_ABIT_AW9D_MAX, .name = "abit-aw9d"},
{.id = ALC882_FIXUP_LENOVO_Y530, .name = "lenovo-y530"},
@@ -2524,6 +2571,11 @@ static int patch_alc882(struct hda_codec *codec)
case 0x10ec0900:
case 0x10ec1220:
break;
+ case 0x10ec0898:
+ case 0x10ec0899:
+ snd_hda_pick_fixup(codec, NULL, alc898_fixup_tbl,
+ alc882_fixups);
+ break;
default:
/* ALC883 and variants */
alc_fix_pll_init(codec, 0x20, 0x0a, 10);
--
2.17.1
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] Add support for Sabre HiFi on System76 and Clevo laptops
2019-08-11 22:19 Dennis Mungai
@ 2019-08-14 6:46 ` Takashi Iwai
0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2019-08-14 6:46 UTC (permalink / raw)
To: Dennis Mungai; +Cc: jackpot51, alsa-devel
On Mon, 12 Aug 2019 00:19:17 +0200,
Dennis Mungai wrote:
>
> Hello there,
>
> The patch attached below is based on Jeremy Soller's prior work on:
> https://patchwork.kernel.org/patch/9552671/ , copied herein.
>
> Cleaned up to pass checkpatch.pl tests.
>
> I can confirm that this patch does indeed fix the audio DAC init on
> the Clevo based P775TM1-R chassis with no issues.
Thanks for the patch. But this needs more consideration,
unfortunately.
> From 9f6b7f51a8be738bb588e8d6b0f4d9fb8ac5a0ce Mon Sep 17 00:00:00 2001
> From: brainiarc7 <dmngaie@gmail.com>
> Date: Mon, 12 Aug 2019 00:43:55 +0300
> Subject: [PATCH 1/1] Fix ESS Sabre DAC init for Clevo laptops
>
> Signed-off-by: brainiarc7 <dmngaie@gmail.com>
First off, the sign-off needs to have the real name. It's a mandatory
legal matter.
And we need a proper description of the patch in the changelog.
About the code change:
> +static void alc898_clevo_dac_hook(struct hda_codec *codec,
> + struct hda_jack_callback *jack)
> +{
> + int val;
> +
> + // Read ESS DAC status
> + snd_hda_codec_write(codec, codec->core.afg, 0, AC_VERB_SET_GPIO_MASK, 4);
> + snd_hda_codec_write(codec, codec->core.afg, 0, AC_VERB_SET_GPIO_DIRECTION, 0);
Usually the mask and the direction bits are set once at initialization.
> + val = snd_hda_codec_read(codec, codec->core.afg, 0, AC_VERB_GET_GPIO_DATA, 0);
> + val &= 0x04;
> +
> + if (val == 0) {
> + // Set VREF on headphone pin to 80%
> + val = snd_hda_codec_get_pin_target(codec, 0x1b);
> + val |= AC_PINCTL_VREF_80;
> + snd_hda_set_pin_ctl(codec, 0x1b, val);
> + } else {
> + // Set VREF on headphone pin to HIZ
> + val = snd_hda_codec_get_pin_target(codec, 0x1b);
> + val = val & 0xfff8;
> + snd_hda_set_pin_ctl(codec, 0x1b, val);
> + }
... and this whole function looks strange. Why it's a jack detection
callback although the jack state isn't checked at all?
> +}
> +
> +static void alc898_fixup_clevo(struct hda_codec *codec,
> + const struct hda_fixup *fix, int action)
> +{
> + switch (action) {
> + case HDA_FIXUP_ACT_PRE_PROBE:
> + snd_hda_jack_detect_enable_callback(codec, 0x1b, alc898_clevo_dac_hook);
Is it really the jack detection on NID 0x1b? This is no pin widget
but a mixer widget which has no such capability.
> + break;
> + case HDA_FIXUP_ACT_INIT:
> + snd_hda_codec_read(codec, 0x1b, 0, AC_VERB_SET_PIN_WIDGET_CONTROL, 4);
Use the proper constant.
> + break;
> + }
> +}
> +
> static void alc_fixup_bass_chmap(struct hda_codec *codec,
> const struct hda_fixup *fix, int action);
>
> @@ -2350,7 +2388,11 @@ static const struct hda_fixup alc882_fixups[] = {
> [ALC887_FIXUP_BASS_CHMAP] = {
> .type = HDA_FIXUP_FUNC,
> .v.func = alc_fixup_bass_chmap,
> - },
> + }, [ALC898_FIXUP_CLEVO_SPDIF] = {
> + .type = HDA_FIXUP_FUNC,
> + .v.func = alc898_fixup_clevo,
> + },
> +
> [ALC1220_FIXUP_GB_DUAL_CODECS] = {
> .type = HDA_FIXUP_FUNC,
> .v.func = alc1220_fixup_gb_dual_codecs,
> @@ -2459,6 +2501,11 @@ static const struct snd_pci_quirk alc882_fixup_tbl[] = {
> {}
> };
>
> +static const struct snd_pci_quirk alc898_fixup_tbl[] = {
> + SND_PCI_QUIRK_VENDOR(0x1558, "Clevo laptop", ALC898_FIXUP_CLEVO_SPDIF),
> + {}
> +};
> +
> static const struct hda_model_fixup alc882_fixup_models[] = {
> {.id = ALC882_FIXUP_ABIT_AW9D_MAX, .name = "abit-aw9d"},
> {.id = ALC882_FIXUP_LENOVO_Y530, .name = "lenovo-y530"},
> @@ -2524,6 +2571,11 @@ static int patch_alc882(struct hda_codec *codec)
> case 0x10ec0900:
> case 0x10ec1220:
> break;
> + case 0x10ec0898:
> + case 0x10ec0899:
> + snd_hda_pick_fixup(codec, NULL, alc898_fixup_tbl,
> + alc882_fixups);
> + break;
This is no-go, will break other machines.
You can put the vendor catch-all entry in the existing table, and
check the codec IDs in the fixup function instead.
thanks,
Takashi
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-08-14 6:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-01 20:57 [PATCH] Add support for Sabre HiFi on System76 and Clevo laptops Jeremy Soller
2017-02-08 13:11 ` Takashi Iwai
-- strict thread matches above, loose matches on Subject: below --
2019-08-11 22:19 Dennis Mungai
2019-08-14 6:46 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).