Re: [PATCH] ALSA: hda/ca0132: Add Sound BlasterX AE-5 external RGB LED strip support

[dchristensen8]

Thanks for the quick review!
  
    I went with sysfs mainly because of the streaming nature of WS2812 strips (using an atomic DMA frame):         
  
    -The LED subsystem has no strip or pixel-buffer concept, individual led_classdevs would hit sysfs with thousands of writes/sec at 30-60 FPS (and leds-class.rst advises against it for addressable arrays anyway).
    -An ALSA kcontrol could hold raw bytes, but pushing frames at 60 Hz spams control change events to PipeWire/PulseAudio.
  
    I initially went with text attributes  (ae5_strip_leds/ae5_strip_num_leds) so basic shell scripts could just echo colors into it, but I know comma-separated lists bend the "one value per file" rule. Would you prefer a bin_attribute for raw RGB bytes instead? or keeping both?              
  
    All your code points make total sense too. I have no objections.
  
Let me know your thoughts on the sysfs format.
  
Thanks,
Devin





On Tuesday, September 29th, 2026 at 9:10 AM, Takashi Iwai <tiwai@xxxxxxx> wrote:

> On Mon, 28 Sep 2026 19:26:05 +0200,
> dchristensen8 wrote:
> >
> >     Hi Takashi, Jaroslav,
> >
> >     Please find attached a patch adding support for the external addressable RGB LED strip header on the Creative Sound BlasterX AE-5 and AE-5 Plus.
> >
> >     The patch is attached as a .patch file.
> >
> >     The Sound BlasterX AE-5 and AE-5 Plus expose a 3-pin 5V addressable RGB header driven directly by the CA0113 PCIe-to-HD-Audio bridge. Rather than using the CA0132 DSP or MMIO GPIO, the CA0113 contains an autonomous HDA link sniffer that snoops a designated HDA DMA playback stream, extracts the 24-bit GRB color payload, and serializes it as 800 kHz single-wire NRZ pulses to the strip (WS2812B protocol).
> >
> >     Expose the strip through a sysfs interface on the codec device:
> >     ae5_strip_leds: read current colors write a comma-deparated #RRGGBB or R,G,B list (a single color replicates across all active LEDs)
> >
> > ae5_strip_num_leds: get or set the active LED count (1..100)
> >
> >     Writes allocate an idle HDA playback stream, format a 32 KB DMA buffer populated with repeating WS2812 frames (5-word zero preamble, 4 audio words per LED, and reset gap), configure the CA0113 BAR2 serializers and stream-tag pin mux, and pulse the stream long enough for the reset latch to fire. Access is serialized with a per-spec mutex, and the attributes are only registered on QUIRK_AE5 devices.
> >
> >     Document the new sysfs attributes in documentation/ABI/testing.
> >
> >     Signed-off-by: Devin Christensen <dchristensen8@xxxxxxxxx>
> >     Testing & Subsystem Context:
> >      - Verified on physical Sound BlasterX AE-5 hardware.
> >      - Tested continuous 60 FPS streaming and dynamic strip resizing (10 -> 7 -> 10 LEDs)
> >        via the OpenRGB effects engine (MR: https://gitlab.com/CalcProgrammer1/OpenRGB/-/merge_requests/3658).
> >      - Verified concurrent audio playback and LED updates without dropped samples or xruns.
> >      - Built clean against tiwai/sound for-next (commit 72107104d4f5).
> >      - Passes scripts/checkpatch.pl with 0 errors and 0 checks.
> 
> Thanks for the patch.
> 
> It's an interesting stuff (rather surprising that they (ab)use
> HD-audio for such an interface :)
> 
> Through a quick glance, most of the code looks OK.
> 
> So the question is rather the interface -- is the sysfs the best
> choice for this kind of display controls?  The implementation can be
> refactored at any time, but the interface must be set solid, so that's
> the most important question to be clarified.
> 
> About the code:
> 
> >  /*
> > - * Setup default parameters for the Sound BlasterX AE-5 DSP.
> > + * Sound BlasterX AE-5 External WS2812B RGB LED Strip Transport
> >   */
> > +static void ae5_strip_encode_24(u32 u, u32 *words)
> > +{
> > +	/* Bit-for-bit exact encoding matching CtxHda.sys FUN_00042944:
> > +	 * 24-bit pixel -> 4 x 32-bit audio words (6 bits per word).
> > +	 * Hardware audio serializer transmits MSB first (bits 31 down to 0).
> > +	 * For word i (i8 = 23, 17, 11, 5), bit_idx 0..5 (MSB to LSB within the word)
> > +	 * is placed at n = 29 - bit_idx * 4 (bits 29, 25, 21, 17, 13, 9):
> > +	 *   out |= (bit << n) | ((bit | 2) << (n + 1))
> > +	 * Bit 0 -> 0b010 (375ns high, 1042ns low)
> > +	 * Bit 1 -> 0b111 (1083ns high, 333ns low)
> > +	 */
> > +	int i8 = 23;
> > +	int i;
> > +
> > +	for (i = 0; i < 4; i++) {
> > +		u32 out = 0;
> > +		int bit_idx;
> > +
> > +		for (bit_idx = 0; bit_idx < 6; bit_idx++) {
> > +			int bit = (u >> (i8 - bit_idx)) & 1;
> > +			int n = 29 - bit_idx * 4;
> > +
> > +			out |= (u32)(bit << n) | ((u32)(bit | 2) << (n + 1));
> > +		}
> > +		words[i] = out;
> > +		i8 -= 6;
> > +	}
> 
> I guess some BIT() macro or such could help in a bit cleaner form.
> 
> 
> > +static struct hdac_stream *ae5_strip_find_stream(struct hda_codec *codec)
> > +{
> > +	struct ca0132_spec *spec = codec->spec;
> > +	struct hdac_bus *bus = &codec->bus->core;
> > +	struct hdac_stream *s, *dsp = NULL, *first = NULL;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&bus->reg_lock, flags);
> > +	list_for_each_entry(s, &bus->stream_list, list) {
> > +		if (s->direction != SNDRV_PCM_STREAM_PLAYBACK)
> > +			continue;
> > +		if (s->running || s->locked)
> > +			continue;
> > +		if (!first)
> > +			first = s;
> > +		/* The DSP DMA consumes the DSP-loader stream (tag in dsp_stream_id). */
> > +		if (spec->dsp_stream_id && s->stream_tag == spec->dsp_stream_id)
> > +			dsp = s;
> > +	}
> > +	spin_unlock_irqrestore(&bus->reg_lock, flags);
> 
> Try to use guard() instead of manual lock/unlock.
> 
> 
> > +/*
> > + * Caller must hold spec->ae5_strip_mutex.
> > + */
> 
> You can put a lockdep assert to assure that in the actual code, too.
> 
> > +static int ae5_strip_send_frame(struct hda_codec *codec, const u32 *grb_colors, int num_leds)
> > +{
> > +	struct ca0132_spec *spec = codec->spec;
> > +	struct snd_dma_buffer dmab;
> > +	struct hdac_stream *hstr = NULL;
> > +	unsigned int format, stream_tag;
> > +	u32 enc_words[4];
> > +	u32 *dst;
> > +	int total_words = 0x8000 / sizeof(u32); /* 8192 words (32KB) */
> 
> It should be defined as a constant instead?
> 
> > +	int word_idx = 0;
> > +	int frame_words;
> > +	int led, w;
> > +	int retries = 20; /* ~100ms max */
> > +	u8 orig_sd_ctl = 0;
> > +
> > +	if (!spec || !spec->mem_base)
> > +		return -ENODEV;
> > +	if (num_leds <= 0 || num_leds > AE5_STRIP_MAX_LEDS)
> > +		return -EINVAL;
> > +
> > +	snd_hda_power_up(codec);
> > +
> > +	/*
> > +	 * 44.1kHz, 24-bit, 2-channel format (0x4031) matches Windows CtxHda
> > +	 * HDAUDIO_STREAM_FORMAT.
> > +	 */
> > +	format = snd_hdac_stream_format(2, 24, 44100);
> > +
> > +	for (;;) {
> > +		hstr = ae5_strip_find_stream(codec);
> > +		if (!hstr) {
> > +			if (--retries <= 0) {
> > +				codec_warn(codec, "AE5 strip: no free azx stream after retries\n");
> > +				snd_hda_power_down(codec);
> > +				return -EBUSY;
> > +			}
> > +			usleep_range(5000, 10000);
> > +			continue;
> > +		}
> > +
> > +		stream_tag = snd_hdac_dsp_prepare(hstr, format, 0x8000, &dmab);
> > +		if ((int)stream_tag > 0)
> > +			break;
> > +
> > +		/* Prepare failed (e.g. -EBUSY due to race with playback starting); retry */
> > +		if (--retries <= 0) {
> > +			codec_warn(codec, "AE5 strip: snd_hdac_dsp_prepare failed %d\n",
> > +				   (int)stream_tag);
> > +			snd_hda_power_down(codec);
> > +			return (int)stream_tag;
> > +		}
> > +		usleep_range(5000, 10000);
> > +	}
> > +
> > +	/* Prevent interrupt storms during free-running LED stream, saving original control bits */
> > +	if (hstr->sd_addr) {
> > +		orig_sd_ctl = readb(hstr->sd_addr);
> > +		writeb(orig_sd_ctl & ~0x1c, hstr->sd_addr); /* Clear IOCE, FEIE, DEIE */
> > +	}
> > +
> > +	/* 1. Populate the 32KB DMA buffer with repeating WS2812 frames + reset gaps */
> > +	memset(dmab.area, 0, 0x8000);
> > +	dst = (u32 *)dmab.area;
> > +
> > +	/* Words per frame: 5 words zero preamble + num_leds * 4 words + 83 words reset gap */
> > +	frame_words = 5 + (num_leds * 4) + 83;
> > +
> > +	while (word_idx + frame_words <= total_words) {
> > +		/* 5-word zero preamble per Windows CtxHda FUN_00042944 */
> > +		for (w = 0; w < 5; w++) {
> > +			*dst++ = 0;
> > +			word_idx++;
> > +		}
> > +		/* LEDs */
> > +		for (led = 0; led < num_leds; led++) {
> > +			ae5_strip_encode_24(grb_colors[led], enc_words);
> > +			for (w = 0; w < 4; w++) {
> > +				*dst++ = enc_words[w];
> > +				word_idx++;
> > +			}
> > +		}
> > +		/* 83 words (> 1.8 ms) zero reset gap (WS2812 requires > 50 us low) */
> > +		for (w = 0; w < 83; w++) {
> 
> Here, too.  Let's avoid magic numbers.
> 
> > +			*dst++ = 0;
> > +			word_idx++;
> > +		}
> > +	}
> > +	while (word_idx < total_words) {
> > +		*dst++ = 0;
> > +		word_idx++;
> > +	}
> > +	/* Ensure DMA buffer writes are committed before triggering the HDA stream */
> > +	wmb();
> > +
> > +	/* 2. Configure CA0113 BAR2 hardware registers */
> > +	/* Power up clocks (CtxHdb 0x141b7 / 0x14210) */
> > +	writel(readl(spec->mem_base + 0xc04) | 0x7, spec->mem_base + 0xc04);
> > +
> > +	/* Base serializer initialization (CtxHdb 0x14ba4) */
> > +	writel(readl(spec->mem_base + 0x400) | 1, spec->mem_base + 0x400);
> > +	writel(readl(spec->mem_base + 0x42c) & ~1, spec->mem_base + 0x42c);
> > +	writel(readl(spec->mem_base + 0x46c) & ~1, spec->mem_base + 0x46c);
> > +	writel(readl(spec->mem_base + 0x4ac) & ~1, spec->mem_base + 0x4ac);
> > +	writel(readl(spec->mem_base + 0x4ec) & ~1, spec->mem_base + 0x4ec);
> > +	writel(0, spec->mem_base + 0x43c);
> > +	writel(0, spec->mem_base + 0x47c);
> > +	writel(0, spec->mem_base + 0x4bc);
> > +	writel(0, spec->mem_base + 0x4fc);
> > +	writel(readl(spec->mem_base + 0x408) | 1, spec->mem_base + 0x408);
> > +	writel(readl(spec->mem_base + 0x40c) | 1, spec->mem_base + 0x40c);
> > +	writel((readl(spec->mem_base + 0x410) & ~0xb) | 0x14, spec->mem_base + 0x410);
> > +
> > +	/* Configure serializers 0..3 clocking (CtxHdb 0x14a6c) */
> > +	writel(readl(spec->mem_base + 0x43c) | 0x33, spec->mem_base + 0x43c);
> > +	writel(readl(spec->mem_base + 0x47c) | 0x33, spec->mem_base + 0x47c);
> > +	writel(readl(spec->mem_base + 0x4bc) | 0x33, spec->mem_base + 0x4bc);
> > +	writel(readl(spec->mem_base + 0x4fc) | 0x33, spec->mem_base + 0x4fc);
> 
> Some helpers or macros look deserving.
> 
> I stop at this point, as we'd need to define the interface at first.
> 
> 
> thanks,
> 
> Takashi
>