[PATCH] ALSA: hda/realtek: Add quirk for CSL Unity BF24B

[PATCH] ALSA: hda/realtek: Add quirk for CSL Unity BF24B

[Zhang Heng]

The CSL Unity BF24B all-in-one PC uses a Realtek ALC662 rev3 audio
codec and requires the correct GPIO configuration to enable sound
output from both the speakers and the headphone.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=221258
Signed-off-by: Zhang Heng <zhangheng@kylinos.cn>
---
 sound/hda/codecs/realtek/alc662.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/sound/hda/codecs/realtek/alc662.c b/sound/hda/codecs/realtek/alc662.c
index 5073165d1f3c..3abe41c7315c 100644
--- a/sound/hda/codecs/realtek/alc662.c
+++ b/sound/hda/codecs/realtek/alc662.c
@@ -255,6 +255,25 @@ static void alc_fixup_headset_mode_alc668(struct hda_codec *codec,
 	alc_fixup_headset_mode(codec, fix, action);
 }
 
+static void alc662_fixup_csl_amp(struct hda_codec *codec,
+				 const struct hda_fixup *fix, int action)
+{
+	struct alc_spec *spec = codec->spec;
+
+	switch (action) {
+	case HDA_FIXUP_ACT_PRE_PROBE:
+		spec->gpio_mask |= 0x03;
+		spec->gpio_dir |= 0x03;
+		break;
+	case HDA_FIXUP_ACT_INIT:
+		/* need to toggle GPIO to enable the amp */
+		alc_update_gpio_data(codec, 0x03, true);
+		msleep(100);
+		alc_update_gpio_data(codec, 0x03, false);
+		break;
+	}
+}
+
 enum {
 	ALC662_FIXUP_ASPIRE,
 	ALC662_FIXUP_LED_GPIO1,
@@ -313,6 +332,7 @@ enum {
 	ALC897_FIXUP_HEADSET_MIC_PIN2,
 	ALC897_FIXUP_UNIS_H3C_X500S,
 	ALC897_FIXUP_HEADSET_MIC_PIN3,
+	ALC662_FIXUP_CSL_GPIO,
 };
 
 static const struct hda_fixup alc662_fixups[] = {
@@ -766,11 +786,16 @@ static const struct hda_fixup alc662_fixups[] = {
 			{ }
 		},
 	},
+	[ALC662_FIXUP_CSL_GPIO] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc662_fixup_csl_amp,
+	},
 };
 
 static const struct hda_quirk alc662_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x1019, 0x9087, "ECS", ALC662_FIXUP_ASUS_MODE2),
 	SND_PCI_QUIRK(0x1019, 0x9859, "JP-IK LEAP W502", ALC897_FIXUP_HEADSET_MIC_PIN3),
+	SND_PCI_QUIRK(0x1022, 0xc950, "CSL Unity BF24B", ALC662_FIXUP_CSL_GPIO),
 	SND_PCI_QUIRK(0x1025, 0x022f, "Acer Aspire One", ALC662_FIXUP_INV_DMIC),
 	SND_PCI_QUIRK(0x1025, 0x0241, "Packard Bell DOTS", ALC662_FIXUP_INV_DMIC),
 	SND_PCI_QUIRK(0x1025, 0x0308, "Acer Aspire 8942G", ALC662_FIXUP_ASPIRE),
-- 
2.34.1

Re: [PATCH] ALSA: hda/realtek: Add quirk for CSL Unity BF24B

[Takashi Iwai]

On Thu, 09 Apr 2026 04:40:28 +0200,
Zhang Heng wrote:
> 
> The CSL Unity BF24B all-in-one PC uses a Realtek ALC662 rev3 audio
> codec and requires the correct GPIO configuration to enable sound
> output from both the speakers and the headphone.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=221258
> Signed-off-by: Zhang Heng <zhangheng@kylinos.cn>
> ---
>  sound/hda/codecs/realtek/alc662.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/sound/hda/codecs/realtek/alc662.c b/sound/hda/codecs/realtek/alc662.c
> index 5073165d1f3c..3abe41c7315c 100644
> --- a/sound/hda/codecs/realtek/alc662.c
> +++ b/sound/hda/codecs/realtek/alc662.c
> @@ -255,6 +255,25 @@ static void alc_fixup_headset_mode_alc668(struct hda_codec *codec,
>  	alc_fixup_headset_mode(codec, fix, action);
>  }
>  
> +static void alc662_fixup_csl_amp(struct hda_codec *codec,
> +				 const struct hda_fixup *fix, int action)
> +{
> +	struct alc_spec *spec = codec->spec;
> +
> +	switch (action) {
> +	case HDA_FIXUP_ACT_PRE_PROBE:
> +		spec->gpio_mask |= 0x03;
> +		spec->gpio_dir |= 0x03;
> +		break;

Do we need to set gpio_mask and gpio_dir explicitly for this bit?
Looking at the rest of the patch...

> +	case HDA_FIXUP_ACT_INIT:
> +		/* need to toggle GPIO to enable the amp */
> +		alc_update_gpio_data(codec, 0x03, true);
> +		msleep(100);
> +		alc_update_gpio_data(codec, 0x03, false);
> +		break;

... here is the only place to fiddle with this GPIO bit, and it's used
for gating.  OTOH, spec->gpio_mask, gpio_dir and gpio_data are used at
alc_write_gpio() that is called from alc_init(), and IIUC, this bit
doesn't have to be initialized there.


thanks,

Takashi

Re: [PATCH] ALSA: hda/realtek: Add quirk for CSL Unity BF24B

[Zhang Heng]

hda-verb /dev/snd/hwC1D0 0x14 0xB 0x40
  I think this command is meaningless, so it was not written.
hda-verb [ALC662_device] 0x01 SET_GPIO_MASK 0x03
hda-verb [ALC662_device] 0x01 SET_GPIO_DIRECTION 0x03
hda-verb [ALC662_device] 0x01 SET_GPIO_DATA 0x03
sleep 1
hda-verb [ALC662_device] 0x01 SET_GPIO_DATA 0x01
sleep 1
hda-verb [ALC662_device] 0x01 SET_GPIO_DATA 0x02


This user actually used these commands and the audio output was normal.

The last two commands clearly trigger the falling edge of gpio_data 0x03,
Referring to these commands, my patch writing should be normal.

Re: [PATCH] ALSA: hda/realtek: Add quirk for CSL Unity BF24B

[Takashi Iwai]

On Thu, 09 Apr 2026 07:58:21 +0200,
Zhang Heng wrote:
> 
> hda-verb /dev/snd/hwC1D0 0x14 0xB 0x40
>  I think this command is meaningless, so it was not written.
> hda-verb [ALC662_device] 0x01 SET_GPIO_MASK 0x03
> hda-verb [ALC662_device] 0x01 SET_GPIO_DIRECTION 0x03
> hda-verb [ALC662_device] 0x01 SET_GPIO_DATA 0x03
> sleep 1
> hda-verb [ALC662_device] 0x01 SET_GPIO_DATA 0x01
> sleep 1
> hda-verb [ALC662_device] 0x01 SET_GPIO_DATA 0x02
> 
> 
> This user actually used these commands and the audio output was normal.
> 
> The last two commands clearly trigger the falling edge of gpio_data 0x03,
> Referring to these commands, my patch writing should be normal.

With your code, you're writing GPIO bits 0x03 3 times at each init or
resume call; at first in alc_auto_init_amp(), then toggling high and
low in the fixup.  So the first write is obviously superfluous.

You should be aware of this behavior although the impact is rather
small.


Takashi

Re: [PATCH] ALSA: hda/realtek: Add quirk for CSL Unity BF24B

[Takashi Iwai]

On Thu, 09 Apr 2026 08:28:42 +0200,
Takashi Iwai wrote:
> 
> On Thu, 09 Apr 2026 07:58:21 +0200,
> Zhang Heng wrote:
> > 
> > hda-verb /dev/snd/hwC1D0 0x14 0xB 0x40
> >  I think this command is meaningless, so it was not written.
> > hda-verb [ALC662_device] 0x01 SET_GPIO_MASK 0x03
> > hda-verb [ALC662_device] 0x01 SET_GPIO_DIRECTION 0x03
> > hda-verb [ALC662_device] 0x01 SET_GPIO_DATA 0x03
> > sleep 1
> > hda-verb [ALC662_device] 0x01 SET_GPIO_DATA 0x01
> > sleep 1
> > hda-verb [ALC662_device] 0x01 SET_GPIO_DATA 0x02
> > 
> > 
> > This user actually used these commands and the audio output was normal.
> > 
> > The last two commands clearly trigger the falling edge of gpio_data 0x03,
> > Referring to these commands, my patch writing should be normal.
> 
> With your code, you're writing GPIO bits 0x03 3 times at each init or
> resume call; at first in alc_auto_init_amp(), then toggling high and
> low in the fixup.  So the first write is obviously superfluous.
> 
> You should be aware of this behavior although the impact is rather
> small.

That said, I'm going to take your patch for now, but we might need to
reconsider the implementation of Realtek codec helpers about GPIO
handling.


thanks,

Takashi

Re: [PATCH] ALSA: hda/realtek: Add quirk for CSL Unity BF24B

[Zhang Heng]

Some machines also need to write GPIO again after waking up from sleep,

please consider this.

在 2026/4/9 14:30, Takashi Iwai 写道:
> That said, I'm going to take your patch for now, but we might need to
> reconsider the implementation of Realtek codec helpers about GPIO
> handling.
>

Re: [PATCH] ALSA: hda/realtek: Add quirk for CSL Unity BF24B

[Takashi Iwai]

On Thu, 09 Apr 2026 08:33:19 +0200,
Zhang Heng wrote:
> 
> Some machines also need to write GPIO again after waking up from sleep,
> 
> please consider this.

Sure, but your patch puts one extra unnecessary GPIO operation than
needed, and it can be optimized later -- that's my point.  The fixup
could deal directly with only GPIO high/low only once.  With the
current alc_*_gpio() helpers, the values are cached and it's restored
already once before the fixup at each resume call.  So we'd need some
tune up in the helper code for this optimization.


Takashi

> 
> 在 2026/4/9 14:30, Takashi Iwai 写道:
> > That said, I'm going to take your patch for now, but we might need to
> > reconsider the implementation of Realtek codec helpers about GPIO
> > handling.
> > 

Re: [PATCH] ALSA: hda/realtek: Add quirk for CSL Unity BF24B

[Zhang Heng]

>> +		alc_update_gpio_data(codec, 0x03, true);
>> +		msleep(100);
>> +		alc_update_gpio_data(codec, 0x03, false);

Is it possible to make these lines into another auxiliary function? I think so.

Re: [PATCH] ALSA: hda/realtek: Add quirk for CSL Unity BF24B

[Takashi Iwai]

On Thu, 09 Apr 2026 09:00:59 +0200,
Zhang Heng wrote:
> 
> 
> >> +		alc_update_gpio_data(codec, 0x03, true);
> >> +		msleep(100);
> >> +		alc_update_gpio_data(codec, 0x03, false);
> 
> Is it possible to make these lines into another auxiliary function? I think so.

I had something like below in mind.  This can be applied to clean up
many existing code.

With that change, the fixup becomes

	snd_hda_codec_set_gpio(codec, 0x03, 0x03, 0x03, 0);
	msleep(100);
	snd_hda_codec_set_gpio(codec, 0x03, 0x03, 0x00, 0);

without setting spec->gpio_* fields.


Takashi

-- 8< --
diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
index 5d9f0ef228af..cb1755d535f9 100644
--- a/include/sound/hda_codec.h
+++ b/include/sound/hda_codec.h
@@ -470,6 +470,10 @@ void snd_hda_unlock_devices(struct hda_bus *bus);
 void snd_hda_bus_reset(struct hda_bus *bus);
 void snd_hda_bus_reset_codecs(struct hda_bus *bus);
 
+void snd_hda_codec_set_gpio(struct hda_codec *codec, unsigned int mask,
+			    unsigned int dir, unsigned int data,
+			    unsigned int delay);
+
 int snd_hda_codec_set_name(struct hda_codec *codec, const char *name);
 
 /*
diff --git a/sound/hda/common/codec.c b/sound/hda/common/codec.c
index 5123df32ad89..6eb11e933606 100644
--- a/sound/hda/common/codec.c
+++ b/sound/hda/common/codec.c
@@ -4053,6 +4053,29 @@ void snd_hda_bus_reset_codecs(struct hda_bus *bus)
 	}
 }
 
+/**
+ * snd_hda_codec_set_gpio - Set up GPIO bits for AFG
+ * @codec: the HDA codec
+ * @mask: GPIO bitmask
+ * @dir: GPIO direction bits
+ * @data: GPIO data bits
+ * @delay: the delay in msec before writing GPIO data bits
+ */
+void snd_hda_codec_set_gpio(struct hda_codec *codec, unsigned int mask,
+			    unsigned int dir, unsigned int data,
+			    unsigned int delay)
+{
+	snd_hda_codec_write(codec, codec->core.afg, 0,
+			    AC_VERB_SET_GPIO_MASK, mask);
+	snd_hda_codec_write(codec, codec->core.afg, 0,
+			    AC_VERB_SET_GPIO_DIRECTION, dir);
+	if (delay)
+		msleep(delay);
+	snd_hda_codec_write(codec, codec->core.afg, 0,
+			    AC_VERB_SET_GPIO_DATA, data);
+}
+EXPORT_SYMBOL_GPL(snd_hda_codec_set_gpio);
+
 /**
  * snd_print_pcm_bits - Print the supported PCM fmt bits to the string buffer
  * @pcm: PCM caps bits

Re: [PATCH] ALSA: hda/realtek: Add quirk for CSL Unity BF24B

[Zhang Heng]

Do we still need to consider HDA_FIXUP_ACT_PRE_PROBE

and HDA_FIXUP_ACT_INIT?

在 2026/4/9 15:24, Takashi Iwai 写道:
> 	snd_hda_codec_set_gpio(codec, 0x03, 0x03, 0x03, 0);
> 	msleep(100);
> 	snd_hda_codec_set_gpio(codec, 0x03, 0x03, 0x00, 0);

Re: [PATCH] ALSA: hda/realtek: Add quirk for CSL Unity BF24B

[Takashi Iwai]

On Thu, 09 Apr 2026 09:51:44 +0200,
Zhang Heng wrote:
> 
> Do we still need to consider HDA_FIXUP_ACT_PRE_PROBE
> 
> and HDA_FIXUP_ACT_INIT?

With my proposed change, only HDA_FIXUP_ACT_INIT is needed.

Of course, it pretty much depends on how the GPIO bits are handled.
For a volatile toggle like your case, there is no need to cache the
value, hence the direct call to handle GPIO bits is better.  OTOH,
for static GPIO setups, the caching can work better as is now.

In anyway, I already applied your original patch.  After cooking up
the whole, I'll submit the cleanups.


thanks,

Takashi