All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: "eziogale@gmail.com" <eziogale@gmail.com>
Cc: Takashi Iwai ` <tiwai@suse.com>,
	Jaroslav Kysela ` <perex@perex.cz>,
	linux-sound@vger.kernel.org, alsa-devel@alsa-project.org
Subject: Re: ca0132: add generic parser fallback for broken DSP path
Date: Sat, 16 May 2026 15:41:02 +0200	[thread overview]
Message-ID: <87tss7xyj5.wl-tiwai@suse.de> (raw)
In-Reply-To: <a1dd5dfd-8678-4dd9-b6af-68a1e078168a@gmail.com>

On Sun, 10 May 2026 11:38:49 +0200,
eziogale@gmail.com 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

      reply	other threads:[~2026-05-16 13:41 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-10  9:38 ca0132: add generic parser fallback for broken DSP path eziogale
2026-05-16 13:41 ` Takashi Iwai [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87tss7xyj5.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=eziogale@gmail.com \
    --cc=linux-sound@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.