Page MenuHomeFreeBSD

hdaa: update pin patch configurations
ClosedPublic

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

Details

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.

Pull Request: https://github.com/freebsd/freebsd-src/pull/139
Pull Request: https://github.com/freebsd/freebsd-src/pull/140
Pull Request: https://github.com/freebsd/freebsd-src/pull/141
Pull Request: https://github.com/freebsd/freebsd-src/pull/142
Pull Request: https://github.com/freebsd/freebsd-src/pull/143
Pull Request: https://github.com/freebsd/freebsd-src/pull/144
Pull Request: https://github.com/freebsd/freebsd-src/pull/145
Pull Request: https://github.com/freebsd/freebsd-src/pull/146
Pull Request: https://github.com/freebsd/freebsd-src/pull/147
Pull Request: https://github.com/freebsd/freebsd-src/pull/148
Pull Request: https://github.com/freebsd/freebsd-src/pull/149
Pull Request: https://github.com/freebsd/freebsd-src/pull/150

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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

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.

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.

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
155

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)

158
sys/dev/sound/pci/hda/pin_patch.h
38–45

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, \
}
46

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
46

Maybe? This sort of cuddle brace isn't too common in the tree today I don't think.

51

Similar suggestion to above.

imp marked 4 inline comments as done.Jul 3 2021, 5:57 AM

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
46

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
This revision was automatically updated to reflect the committed changes.
imp marked an inline comment as done.
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).

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[]){
sys/dev/sound/pci/hda/hdaa_patches.c
392–394