Re: ca0132: add generic parser fallback for broken DSP path
Takashi Iwai <[email protected]>
| Newsgroups | org.alsa-project.alsa-devel,org.kernel.vger.linux-sound |
|---|---|
| Message-ID | <[email protected]> |
On Sun, 10 May 2026 11:38:49 +0200, [email protected] wrote: > > Some CA0132 implementations (e.g. Gigabyte GA-Z170X-Gaming G1) > produce white noise when using the DSP firmware path. Add a > QUIRK_GENERIC path that uses the standard HDA generic parser > instead, with custom pin configs that disable the broken auto-mute > from HP jack detection. > > Also applies VREF_50 on the mic pin and adds an init_hook for > suspend/resume stability. > > Kernel version: 6.18.24 > The PCI SSID this was tested on: `1458:a046` > it's been tested with PulseAudio and speaker-test > That the DSP path (QUIRK_SBZ) remains unchanged for cards that work with it. > > Thanks The idea sounds reasonable, and most of the changes look OK. Could you submit a patch in a proper format (with a proper Signed-off-by tag)? About the code changes: > --- /home/ezio/tmp/kernel/linux-6.18.24/sound/hda/codecs/ca0132.c.0 2026-05-02 09:19:21.217137158 +0200 > +++ /home/ezio/tmp/kernel/linux-6.18.24/sound/hda/codecs/ca0132.c 2026-05-10 11:21:41.933355825 +0200 A patch should be applicable with -p1 option. > @@ -24,6 +24,7 @@ > #include "hda_local.h" > #include "hda_auto_parser.h" > #include "hda_jack.h" > +#include "generic.h" I guess you need also add the selection of CONFIG_SND_HDA_CODEC_GENERIC in Kconfig, too. > #include "ca0132_regs.h" > > @@ -1060,6 +1061,8 @@ > */ > > struct ca0132_spec { > + struct hda_gen_spec gen; > + > const struct snd_kcontrol_new *mixers[5]; > unsigned int num_mixers; > const struct hda_verb *base_init_verbs; > @@ -1175,6 +1178,7 @@ > QUIRK_AE5, > QUIRK_AE7, > QUIRK_NONE = HDA_FIXUP_ID_NOT_SET, > + QUIRK_GENERIC, Better to put QUIRK_GENERIC before QUIRK_NONE. > static void ca0132_codec_remove(struct hda_codec *codec) > { > struct ca0132_spec *spec = codec->spec; > > + if (ca0132_quirk(spec) == QUIRK_GENERIC) { > + snd_hda_gen_remove(codec); > + return; > + } > if (ca0132_quirk(spec) == QUIRK_ZXR_DBPRO) > return dbpro_free(codec); > else Better to replace with switch(). > + if (ca0132_quirk(spec) == QUIRK_GENERIC) { > + struct auto_pin_cfg *cfg = &spec->gen.autocfg; > + > + snd_hda_gen_spec_init(&spec->gen); > + > + snd_hda_apply_pincfgs(codec, > + (const struct hda_pintbl[]) { > + { 0x0b, 0x41014111 }, > + { 0x0c, 0x414520f0 }, /* SPDIF out */ > + { 0x0d, 0x01014010 }, /* lineout */ > + { 0x0e, 0x41c501f0 }, > + { 0x0f, 0x411111f0 }, /* disabled */ > + { 0x10, 0x411111f0 }, /* disabled */ > + { 0x11, 0x41012014 }, > + { 0x12, 0x37a790f0 }, /* mic */ > + { 0x13, 0x77a701f0 }, > + { 0x18, 0x500000f0 }, > + {} > + }); > > + ca0132_init_chip(codec); > + > + err = ca0132_prepare_verbs(codec); > + if (err < 0) > + goto error; > + > + err = snd_hda_parse_pin_def_config(codec, cfg, NULL); > + if (err < 0) > + goto error; > + err = snd_hda_gen_parse_auto_config(codec, cfg); > + if (err < 0) > + goto error; > + > + spec->gen.init_hook = ca0132_generic_init_hook; > + spec->gen.automute_speaker = 0; > + spec->gen.automute_lo = 0; > + > + snd_hda_sequence_write(codec, spec->spec_init_verbs); > + return 0; > + } It's a too big block, and better to be factored out as a function. The verb table should be also defined outside the argument, too; otherwise it looks too hackish. > @@ -10014,6 +10077,10 @@ > { > struct ca0132_spec *spec = codec->spec; > > + if (ca0132_quirk(spec) == QUIRK_GENERIC) { > + return snd_hda_gen_build_controls(codec); > + } > + > if (ca0132_quirk(spec) == QUIRK_ZXR_DBPRO) > return dbpro_build_controls(codec); > else > @@ -10024,6 +10091,10 @@ > { > struct ca0132_spec *spec = codec->spec; > > + if (ca0132_quirk(spec) == QUIRK_GENERIC) { > + return snd_hda_gen_build_pcms(codec); > + } > + > if (ca0132_quirk(spec) == QUIRK_ZXR_DBPRO) > return dbpro_build_pcms(codec); > else > @@ -10034,6 +10105,10 @@ > { > struct ca0132_spec *spec = codec->spec; > > + if (ca0132_quirk(spec) == QUIRK_GENERIC) { > + return snd_hda_gen_init(codec); > + } > + > if (ca0132_quirk(spec) == QUIRK_ZXR_DBPRO) > return dbpro_init(codec); > else All those seem better with switch(). thanks, Takashi