Skip to content

Commit

Permalink
ALSA: hda/realtek: Refactor and simplify Samsung Galaxy Book init
Browse files Browse the repository at this point in the history 
[ Upstream commit 7e4d4b3 ]

I have done a lot of analysis for these type of devices and collaborated
quite a bit with Nick Weihs (author of the first patch submitted for this
including adding samsung_helper.c). More information can be found in the
issue on Github [1] including additional rationale and testing.

The existing implementation includes a large number of equalizer coef
values that are not necessary to actually init and enable the speaker
amps, as well as create a somewhat worse sound profile. Users have
reported "muffled" or "muddy" sound; more information about this including
my analysis of the differences can be found in the linked Github issue.

This patch refactors the "v2" version of ALC298_FIXUP_SAMSUNG_AMP to a much
simpler implementation which removes the new samsung_helper.c, reuses more
of the existing patch_realtek.c, and sends significantly fewer unnecessary
coef values (including removing all of these EQ-specific coef values).

A pcm_playback_hook is used to dynamically enable and disable the speaker
amps only when there will be audio playback; this is to match the behavior
of how the driver for these devices is working in Windows, and is
suspected but not yet tested or confirmed to help with power consumption.

Support for models with 2 speaker amps vs 4 speaker amps is controlled by
a specific quirk name for both types. A new int num_speaker_amps has been
added to alc_spec so that the hooks can know how many speaker amps to
enable or disable. This design was chosen to limit the number of places
that subsystem ids will need to be maintained: like this, they can be
maintained only once in the quirk table and there will not be another
separate list of subsystem ids to maintain elsewhere in the code.

Also updated the quirk name from ALC298_FIXUP_SAMSUNG_AMP2 to
ALC298_FIXUP_SAMSUNG_AMP_V2_.. as this is not a quirk for "Amp #2" on
ALC298 but is instead a different version of how to handle it.

More devices have been added (see Github issue for testing confirmation),
as well as a small cleanup to existing names.

[1]: thesofproject#4055 (comment)

Signed-off-by: Joshua Grisham <josh@joshuagrisham.com>
Link: https://patch.msgid.link/20240909193000.838815-1-josh@joshuagrisham.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
@joshuagrisham @gregkh
joshuagrisham authored and gregkh committed Oct 10, 2024
1 parent 50f63f1 commit 7030469
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 317 deletions.
151 changes: 144 additions & 7 deletions sound/pci/hda/patch_realtek.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ struct alc_spec {
unsigned int has_hs_key:1;
unsigned int no_internal_mic_pin:1;
unsigned int en_3kpull_low:1;
int num_speaker_amps;

/* for PLL fix */
hda_nid_t pll_nid;
Expand Down Expand Up @@ -4803,7 +4804,133 @@ static void alc298_fixup_samsung_amp(struct hda_codec *codec,
}
}

#include "samsung_helper.c"
struct alc298_samsung_v2_amp_desc {
unsigned short nid;
int init_seq_size;
unsigned short init_seq[18][2];
};

static const struct alc298_samsung_v2_amp_desc
alc298_samsung_v2_amp_desc_tbl[] = {
{ 0x38, 18, {
{ 0x23e1, 0x0000 }, { 0x2012, 0x006f }, { 0x2014, 0x0000 },
{ 0x201b, 0x0001 }, { 0x201d, 0x0001 }, { 0x201f, 0x00fe },
{ 0x2021, 0x0000 }, { 0x2022, 0x0010 }, { 0x203d, 0x0005 },
{ 0x203f, 0x0003 }, { 0x2050, 0x002c }, { 0x2076, 0x000e },
{ 0x207c, 0x004a }, { 0x2081, 0x0003 }, { 0x2399, 0x0003 },
{ 0x23a4, 0x00b5 }, { 0x23a5, 0x0001 }, { 0x23ba, 0x0094 }
}},
{ 0x39, 18, {
{ 0x23e1, 0x0000 }, { 0x2012, 0x006f }, { 0x2014, 0x0000 },
{ 0x201b, 0x0002 }, { 0x201d, 0x0002 }, { 0x201f, 0x00fd },
{ 0x2021, 0x0001 }, { 0x2022, 0x0010 }, { 0x203d, 0x0005 },
{ 0x203f, 0x0003 }, { 0x2050, 0x002c }, { 0x2076, 0x000e },
{ 0x207c, 0x004a }, { 0x2081, 0x0003 }, { 0x2399, 0x0003 },
{ 0x23a4, 0x00b5 }, { 0x23a5, 0x0001 }, { 0x23ba, 0x0094 }
}},
{ 0x3c, 15, {
{ 0x23e1, 0x0000 }, { 0x2012, 0x006f }, { 0x2014, 0x0000 },
{ 0x201b, 0x0001 }, { 0x201d, 0x0001 }, { 0x201f, 0x00fe },
{ 0x2021, 0x0000 }, { 0x2022, 0x0010 }, { 0x203d, 0x0005 },
{ 0x203f, 0x0003 }, { 0x2050, 0x002c }, { 0x2076, 0x000e },
{ 0x207c, 0x004a }, { 0x2081, 0x0003 }, { 0x23ba, 0x008d }
}},
{ 0x3d, 15, {
{ 0x23e1, 0x0000 }, { 0x2012, 0x006f }, { 0x2014, 0x0000 },
{ 0x201b, 0x0002 }, { 0x201d, 0x0002 }, { 0x201f, 0x00fd },
{ 0x2021, 0x0001 }, { 0x2022, 0x0010 }, { 0x203d, 0x0005 },
{ 0x203f, 0x0003 }, { 0x2050, 0x002c }, { 0x2076, 0x000e },
{ 0x207c, 0x004a }, { 0x2081, 0x0003 }, { 0x23ba, 0x008d }
}}
};

static void alc298_samsung_v2_enable_amps(struct hda_codec *codec)
{
struct alc_spec *spec = codec->spec;
static const unsigned short enable_seq[][2] = {
{ 0x203a, 0x0081 }, { 0x23ff, 0x0001 },
};
int i, j;

for (i = 0; i < spec->num_speaker_amps; i++) {
alc_write_coef_idx(codec, 0x22, alc298_samsung_v2_amp_desc_tbl[i].nid);
for (j = 0; j < ARRAY_SIZE(enable_seq); j++)
alc298_samsung_write_coef_pack(codec, enable_seq[j]);
codec_dbg(codec, "alc298_samsung_v2: Enabled speaker amp 0x%02x\n",
alc298_samsung_v2_amp_desc_tbl[i].nid);
}
}

static void alc298_samsung_v2_disable_amps(struct hda_codec *codec)
{
struct alc_spec *spec = codec->spec;
static const unsigned short disable_seq[][2] = {
{ 0x23ff, 0x0000 }, { 0x203a, 0x0080 },
};
int i, j;

for (i = 0; i < spec->num_speaker_amps; i++) {
alc_write_coef_idx(codec, 0x22, alc298_samsung_v2_amp_desc_tbl[i].nid);
for (j = 0; j < ARRAY_SIZE(disable_seq); j++)
alc298_samsung_write_coef_pack(codec, disable_seq[j]);
codec_dbg(codec, "alc298_samsung_v2: Disabled speaker amp 0x%02x\n",
alc298_samsung_v2_amp_desc_tbl[i].nid);
}
}

static void alc298_samsung_v2_playback_hook(struct hda_pcm_stream *hinfo,
struct hda_codec *codec,
struct snd_pcm_substream *substream,
int action)
{
/* Dynamically enable/disable speaker amps before and after playback */
if (action == HDA_GEN_PCM_ACT_OPEN)
alc298_samsung_v2_enable_amps(codec);
if (action == HDA_GEN_PCM_ACT_CLOSE)
alc298_samsung_v2_disable_amps(codec);
}

static void alc298_samsung_v2_init_amps(struct hda_codec *codec,
int num_speaker_amps)
{
struct alc_spec *spec = codec->spec;
int i, j;

/* Set spec's num_speaker_amps before doing anything else */
spec->num_speaker_amps = num_speaker_amps;

/* Disable speaker amps before init to prevent any physical damage */
alc298_samsung_v2_disable_amps(codec);

/* Initialize the speaker amps */
for (i = 0; i < spec->num_speaker_amps; i++) {
alc_write_coef_idx(codec, 0x22, alc298_samsung_v2_amp_desc_tbl[i].nid);
for (j = 0; j < alc298_samsung_v2_amp_desc_tbl[i].init_seq_size; j++) {
alc298_samsung_write_coef_pack(codec,
alc298_samsung_v2_amp_desc_tbl[i].init_seq[j]);
}
alc_write_coef_idx(codec, 0x89, 0x0);
codec_dbg(codec, "alc298_samsung_v2: Initialized speaker amp 0x%02x\n",
alc298_samsung_v2_amp_desc_tbl[i].nid);
}

/* register hook to enable speaker amps only when they are needed */
spec->gen.pcm_playback_hook = alc298_samsung_v2_playback_hook;
}

static void alc298_fixup_samsung_amp_v2_2_amps(struct hda_codec *codec,
const struct hda_fixup *fix, int action)
{
if (action == HDA_FIXUP_ACT_PROBE)
alc298_samsung_v2_init_amps(codec, 2);
}

static void alc298_fixup_samsung_amp_v2_4_amps(struct hda_codec *codec,
const struct hda_fixup *fix, int action)
{
if (action == HDA_FIXUP_ACT_PROBE)
alc298_samsung_v2_init_amps(codec, 4);
}

#if IS_REACHABLE(CONFIG_INPUT)
static void gpio2_mic_hotkey_event(struct hda_codec *codec,
Expand Down Expand Up @@ -7541,7 +7668,8 @@ enum {
ALC236_FIXUP_HP_MUTE_LED_MICMUTE_VREF,
ALC236_FIXUP_LENOVO_INV_DMIC,
ALC298_FIXUP_SAMSUNG_AMP,
ALC298_FIXUP_SAMSUNG_AMP2,
ALC298_FIXUP_SAMSUNG_AMP_V2_2_AMPS,
ALC298_FIXUP_SAMSUNG_AMP_V2_4_AMPS,
ALC298_FIXUP_SAMSUNG_HEADPHONE_VERY_QUIET,
ALC256_FIXUP_SAMSUNG_HEADPHONE_VERY_QUIET,
ALC295_FIXUP_ASUS_MIC_NO_PRESENCE,
Expand Down Expand Up @@ -9176,9 +9304,13 @@ static const struct hda_fixup alc269_fixups[] = {
.chained = true,
.chain_id = ALC298_FIXUP_SAMSUNG_HEADPHONE_VERY_QUIET
},
[ALC298_FIXUP_SAMSUNG_AMP2] = {
[ALC298_FIXUP_SAMSUNG_AMP_V2_2_AMPS] = {
.type = HDA_FIXUP_FUNC,
.v.func = alc298_fixup_samsung_amp_v2_2_amps
},
[ALC298_FIXUP_SAMSUNG_AMP_V2_4_AMPS] = {
.type = HDA_FIXUP_FUNC,
.v.func = alc298_fixup_samsung_amp2
.v.func = alc298_fixup_samsung_amp_v2_4_amps
},
[ALC298_FIXUP_SAMSUNG_HEADPHONE_VERY_QUIET] = {
.type = HDA_FIXUP_VERBS,
Expand Down Expand Up @@ -10558,8 +10690,10 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
SND_PCI_QUIRK(0x144d, 0xc832, "Samsung Galaxy Book Flex Alpha (NP730QCJ)", ALC256_FIXUP_SAMSUNG_HEADPHONE_VERY_QUIET),
SND_PCI_QUIRK(0x144d, 0xca03, "Samsung Galaxy Book2 Pro 360 (NP930QED)", ALC298_FIXUP_SAMSUNG_AMP),
SND_PCI_QUIRK(0x144d, 0xc868, "Samsung Galaxy Book2 Pro (NP930XED)", ALC298_FIXUP_SAMSUNG_AMP),
SND_PCI_QUIRK(0x144d, 0xc1ca, "Samsung Galaxy Book3 Pro 360 (NP960QFG-KB1US)", ALC298_FIXUP_SAMSUNG_AMP2),
SND_PCI_QUIRK(0x144d, 0xc1cc, "Samsung Galaxy Book3 Ultra (NT960XFH-XD92G))", ALC298_FIXUP_SAMSUNG_AMP2),
SND_PCI_QUIRK(0x144d, 0xc870, "Samsung Galaxy Book2 Pro (NP950XED)", ALC298_FIXUP_SAMSUNG_AMP_V2_2_AMPS),
SND_PCI_QUIRK(0x144d, 0xc886, "Samsung Galaxy Book3 Pro (NP964XFG)", ALC298_FIXUP_SAMSUNG_AMP_V2_4_AMPS),
SND_PCI_QUIRK(0x144d, 0xc1ca, "Samsung Galaxy Book3 Pro 360 (NP960QFG)", ALC298_FIXUP_SAMSUNG_AMP_V2_4_AMPS),
SND_PCI_QUIRK(0x144d, 0xc1cc, "Samsung Galaxy Book3 Ultra (NT960XFH)", ALC298_FIXUP_SAMSUNG_AMP_V2_4_AMPS),
SND_PCI_QUIRK(0x1458, 0xfa53, "Gigabyte BXBT-2807", ALC283_FIXUP_HEADSET_MIC),
SND_PCI_QUIRK(0x1462, 0xb120, "MSI Cubi MS-B120", ALC283_FIXUP_HEADSET_MIC),
SND_PCI_QUIRK(0x1462, 0xb171, "Cubi N 8GL (MS-B171)", ALC283_FIXUP_HEADSET_MIC),
Expand Down Expand Up @@ -10790,6 +10924,8 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
SND_PCI_QUIRK(0x1849, 0xa233, "Positivo Master C6300", ALC269_FIXUP_HEADSET_MIC),
SND_PCI_QUIRK(0x1854, 0x0440, "LG CQ6", ALC256_FIXUP_HEADPHONE_AMP_VOL),
SND_PCI_QUIRK(0x1854, 0x0441, "LG CQ6 AIO", ALC256_FIXUP_HEADPHONE_AMP_VOL),
SND_PCI_QUIRK(0x1854, 0x0488, "LG gram 16 (16Z90R)", ALC298_FIXUP_SAMSUNG_AMP_V2_4_AMPS),
SND_PCI_QUIRK(0x1854, 0x048a, "LG gram 17 (17ZD90R)", ALC298_FIXUP_SAMSUNG_AMP_V2_4_AMPS),
SND_PCI_QUIRK(0x19e5, 0x3204, "Huawei MACH-WX9", ALC256_FIXUP_HUAWEI_MACH_WX9_PINS),
SND_PCI_QUIRK(0x19e5, 0x320f, "Huawei WRT-WX9 ", ALC256_FIXUP_ASUS_MIC_NO_PRESENCE),
SND_PCI_QUIRK(0x1b35, 0x1235, "CZC B20", ALC269_FIXUP_CZC_B20),
Expand Down Expand Up @@ -11000,7 +11136,8 @@ static const struct hda_model_fixup alc269_fixup_models[] = {
{.id = ALC298_FIXUP_HUAWEI_MBX_STEREO, .name = "huawei-mbx-stereo"},
{.id = ALC256_FIXUP_MEDION_HEADSET_NO_PRESENCE, .name = "alc256-medion-headset"},
{.id = ALC298_FIXUP_SAMSUNG_AMP, .name = "alc298-samsung-amp"},
{.id = ALC298_FIXUP_SAMSUNG_AMP2, .name = "alc298-samsung-amp2"},
{.id = ALC298_FIXUP_SAMSUNG_AMP_V2_2_AMPS, .name = "alc298-samsung-amp-v2-2-amps"},
{.id = ALC298_FIXUP_SAMSUNG_AMP_V2_4_AMPS, .name = "alc298-samsung-amp-v2-4-amps"},
{.id = ALC256_FIXUP_SAMSUNG_HEADPHONE_VERY_QUIET, .name = "alc256-samsung-headphone"},
{.id = ALC255_FIXUP_XIAOMI_HEADSET_MIC, .name = "alc255-xiaomi-headset"},
{.id = ALC274_FIXUP_HP_MIC, .name = "alc274-hp-mic-detect"},
Expand Down
Loading

0 comments on commit 7030469

Please sign in to comment.