Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update alc-verb param passing & layout of verb log #762

Merged
merged 1 commit into from
Feb 7, 2022

Conversation

mikebeaton
Copy link
Contributor

@mikebeaton mikebeaton commented Feb 4, 2022

So I got the the bottom of the alc-verb issue.

The Intel HDA spec, and Apple's IOHDACodecDevice::executeVerb (and AudioDxe) all pass 4 bit verbs (0x4, 0x5, 0xC, 0xD, etc.) as 4 bits, and 12 bit verbs (0x7nn, 0xFnn) as 12 bits.

The Linux sound drivers, and Linux alsa-tools' hda-verb all represent 4 bit verbs as a 12 bit verb with 00 (0x400, 0x500, 0xC00, 0xD00, etc.).

The back end of alc-verb is using the Apple method, but the front end follows the Linux convention. This is not as bad as it sounds, because of the way codec verbs are eventually packed.

If we imagine using alc-verb or hda-verb to write to a 16bit processing coefficient on node 0x20 (which coefficient to use is set previously using verb 0x5, and the set value can be read afterwards using verb 0xC) with the following parameters, these are the results:

command hda-verb alc-verb modified alc-verb
0x20 SET_PROC_COEF* 0x1234 0x1234 0x34 0x1234
0x20 0x400 0x1234 0x1234 0x34 0x1234
0x20 0x412 0x34 (acidanthera/bugtracker#1644) 0x1234 0x1234 0x1234
0x20 0x4 0x1234 undefined result (illegal verb) 0x1234 0x1234

*SET_PROC_COEF is 0x400, or 0x4 depending on the convention

@mikebeaton
Copy link
Contributor Author

mikebeaton commented Feb 4, 2022

By the way, it is probably worth noting here or somewhere that this current change will behave differently from hda-verb for broken Linux examples which specify different values for the top 8 bits of the param in the bottom 8 bits of the verb value and in the top 8 bits of the param value. It would be possible to add a small hack (i.e. for 12 bit verbs only, add verb |= param >> 8) specifically to match the exact behaviour of broken Linux examples, and this would not affect non-broken examples, but I think it is probably better not to?

To be clear, Linux ORs the overlapping bits together, whereas if Apple detects non-zero in the bottom 8 bits of the verb, it masks off and ignores the top 8 bits of the param (I have not reversed, but that is what it is doing).

https://www.linux.org/threads/solved-asus-zenbook-15-ux534f-realtek-hd-audio-problem.27384/#post-94194

command hda-verb alc-verb modified alc-verb modified alc-verb if extra hack added
0x20 0x477 0x4a4b 0x7f4b* 0x774b 0x774b 0x7f4b

*0x7f = 0x77 | 0x4a

No changes to all other examples given above.

@mikebeaton
Copy link
Contributor Author

Oh, btw, the motivation for going this way - making alc-verb more compatible with hda-verb - rather than making alc-verb just use a different syntax - the Apple/Intel syntax - is that almost all the examples for how to fix not-quite-working sound on the web are from Linux, so I feel it will be much more convenient to make/keep alc-verb compatible with the Linux examples.

@vit9696 vit9696 merged commit dc4896d into acidanthera:master Feb 7, 2022
@mikebeaton
Copy link
Contributor Author

mikebeaton commented Feb 8, 2022

I have not reversed, but that is what it is doing

I have now quickly reversed __ZN16IOHDACodecDevice11executeVerbEtttPjb, and it is doing what I thought it was doing, though I described it a bit wrongly above. Described more correctly, if the incoming (Apple/Intel-style) verb is greater than or equal to 0x10 (i.e. if it is a 12 bit verb, effectively), it uses only the bottom 8 bits of the param.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants