Differential D30619

hdaa: update pin patch configurations
ClosedPublic
Actions

Authored by imp on Jun 2 2021, 8:51 PM.

Details

Reviewers

None

Group Reviewers

multimedia

Commits

rG6228b19e0ef0: hdaa: update pin patch configurations
rGef790cc7407e: hdaa: update pin patch configurations

Summary

A number of structural changes:

Use decimal nid numbers instead of hex
updated the branch to incoorporate the suggestions made in the ALC280 pull request github thread
Convert magic pin values into strings.
Also update hdaa_patches to use clearer enums..
made pin patch type enum clearer, add macro for 'string' type patches
Added pin_patch structures to separate data from logic.
Integrated Realtek patches into new structure.

These incorporate fixes for ALC255, ALC256, ALC260, ALC262, ALC268,
ALC269, ALC280, ALC282, ALC283, ALC286, ALC290, ALC293, ALC296, ALC2880

And have definitions for a number of Dell and HP laptops.

imp squashed these into one commit because the changes from the github
pull requests no longer cleanly apply.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

imp requested review of this revision.Jun 2 2021, 8:51 PM

imp created this revision.

Harbormaster completed remote builds in B39672: Diff 90341.Jun 2 2021, 8:51 PM

While you are at it, maybe you can also resolve https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256169 ? It seems highly related and also trivial.

rhurlin added a subscriber: rhurlin.Jun 3 2021, 5:49 PM

I will try to take a look in a week or two (after the DevSummit) if nobody else does.

So this would be proper fix for dev.hdaa.%d.init_clear from snd_hda(4) ? Although I dont't seem to need that hack on my

hdaa0: <Realtek ALC255 Audio Function Group> at nid 1 on hdacc0

right now.

linimon added a reviewer: multimedia.Jun 11 2021, 1:54 PM

FWIW, on my X1 6th gen this didn't change anything (but perhaps that is expected, it is an ALC285 and the headphone jack already works as-is). Only some style suggestions, but in general it looks ok to me.

sys/dev/sound/pci/hda/hdaa_patches.c
156	You can reduce the indentation a bit by making use of continue: if (vendor_id != p->id) continue; for (struct model_pin_patch_t *pp = p->patches; pp->models != NULL; pp++) { ... While here, good to use explicit comparisons against NULL (e.g. pp->models)
159
sys/dev/sound/pci/hda/pin_patch.h
39–46	The typical style in FreeBSD probably wouldn't indent these quite so far over but probably use something like: #define <mumble> { \ .foo = x, \ .bar = y, \ }
47	Might have a better chance of fitting, though they will still end up long. It is probably not worth splitting up the strings though.
sys/dev/sound/pci/hda/pin_patch_realtek.h
47	Maybe? This sort of cuddle brace isn't too common in the tree today I don't think.
52	Similar suggestion to above.

In D30619#687767, @arrowd wrote:

While you are at it, maybe you can also resolve https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256169 ? It seems highly related and also trivial.

while related, it's not quite the same thing this patches.

sys/dev/sound/pci/hda/pin_patch_realtek.h
47	While that's true, this isn't normal code so some deviation to get something more compact isn't terrible. Since the style is consistent, I'm going to leave it as-is.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 3 2021, 6:17 AM

Closed by commit rGef790cc7407e: hdaa: update pin patch configurations (authored by K Staring <qdk@quickdekay.net>, committed by imp). · Explain Why

This revision was automatically updated to reflect the committed changes.

imp marked an inline comment as done.

imp added a commit: rGef790cc7407e: hdaa: update pin patch configurations.

emaste added inline comments.Jan 14 2022, 7:17 PM

sys/dev/sound/pci/hda/hdaa_patches.c
392–394	After updating to -current which includes this change headphones no longer work on my X1 Carbon 7th Generation (20QD).

emaste added inline comments.Jan 14 2022, 7:44 PM

sys/dev/sound/pci/hda/hdaa_patches.c

392–394

diff --git a/sys/dev/sound/pci/hda/pin_patch_realtek.h b/sys/dev/sound/pci/hda/pin_patch_realtek.h
index dfa262e3610a..e59cd6b78b8f 100644
--- a/sys/dev/sound/pci/hda/pin_patch_realtek.h
+++ b/sys/dev/sound/pci/hda/pin_patch_realtek.h
@@ -554,6 +554,21 @@ static struct hdaa_model_pin_patch_t realtek_model_pin_patches[] = {
                                }
                        }, { }
                }
+       }, { /**** CODEC: HDA_CODEC_ALC285 ****/
+               .id = HDA_CODEC_ALC285,
+               .patches = (struct model_pin_patch_t[]){
+                       {
+                               .models = (struct pin_machine_model_t[]){
+                                       PIN_SUBVENDOR(LENOVO_X120KH_SUBVENDOR),
+                                       PIN_SUBVENDOR(LENOVO_X120QD_SUBVENDOR),
+                                       { }
+                               },
+                               .pin_patches = (struct pin_patch_t[]){
+                                       PIN_PATCH_STRING(33, "seq=15 as=1 color=Black ctype=1/8 device=Headphones loc=Left"),
+                                       { }
+                               }
+                       }, { }
+               }
        }, { /**** CODEC: HDA_CODEC_ALC286 ****/
                .id = HDA_CODEC_ALC286,
                .patches = (struct model_pin_patch_t[]){