snd_hda(4): Add quirks for Lenovo ThinkBooks and ASUS TUFs
Needs ReviewPublic
Actions

Authored by wulf on Feb 14 2025, 9:57 AM.

Details

Reviewers

christos

Summary

Includes pin configuration and generic quirks for ALC255 - ALC257
codecs initialization.

Sponsored by: Future Crew, LLC
MFC after: 1 month

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 77694
Build 74577: arc lint + arc unit

Event Timeline

wulf created this revision.Feb 14 2025, 9:57 AM

Herald added a subscriber: imp. · View Herald TranscriptFeb 14 2025, 9:57 AM

wulf requested review of this revision.Feb 14 2025, 9:57 AM

Harbormaster completed remote builds in B62436: Diff 151011.Feb 14 2025, 9:57 AM

The commit message says "ALC257 codecs initialization", but in hdaa_patch_direct() and in pin_patch_realtek.h we seem to be affecting other codecs as well. Shouldn't the commit message reflect that?

My HDA card also is ALC257, so I will make sure to test the patch ASAP.

sys/dev/sound/pci/hda/hdaa_patches.c
358	According to snd_hda.4: loc Connector physical location. Can be specified as a number from 0 to 63. This is a reference only value. It is ignored by the snd_hda driver. So I suppose these are not a functional changes, right?
767	I think we could simply call `hdaa_read_coef()` here.
771	And `hdaa_write_coef()` here. Or did you intentionally duplicate them?
821	Just out of curiosity, what does this do exactly? :-)
844	Stray newline.

In D49002#1117625, @christos wrote:

The commit message says "ALC257 codecs initialization", but in hdaa_patch_direct() and in pin_patch_realtek.h we seem to be affecting other codecs as well. Shouldn't the commit message reflect that?

It is a typo. Must be ALC255 - ALC257. Fixed

Fix description and stray newline

Harbormaster completed remote builds in B62483: Diff 151106.Feb 17 2025, 7:51 AM

wulf marked an inline comment as done.Feb 17 2025, 8:04 AM

wulf added inline comments.

sys/dev/sound/pci/hda/hdaa_patches.c
358	It is a cosmetic change only
771	Yes it is duplicated intentionally to be distinct from Linux where this function originated.
821	It is required to dig through the bunch of Realtek commits to alc256_init() function in Linux kernel. The only thing I need is a patch to enable jack's mic on ASUS TUF. But I ported more tweaks while was here.

christos added inline comments.Feb 18 2025, 2:17 PM

sys/dev/sound/pci/hda/hdaa_patches.c
771	Right. Is there a reason we wouldn't want to use them here though? Maybe I'm missing something.
821	Are any of these tweaks worth noting either in a comment or in the commit message?

With the exception of the latest comments I left last time, LGTM.

This revision is now accepted and ready to land.Mar 6 2025, 1:00 PM

New ALC257 patch: D49396

We can rebase this one on top of it.

sys/dev/sound/pci/hda/hdaa_patches.c
358	On my ALC257 (Lenovo Thinkpad L480) the location of the headphone-mic jack is right. Since this is a cosmetic change, I think we can drop it.
358	Ditto. The location is on the right for me.