Attention is currently required from: Sean Rhodes, Matt DeVillier, Angel Pons. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59263 )
Change subject: Documentation/drivers: Add notes on Realtek HW EQ ......................................................................
Patch Set 4: Code-Review+1
(10 comments)
Patchset:
PS4: Thank you so much!
File Documentation/drivers/realtek.md:
https://review.coreboot.org/c/coreboot/+/59263/comment/92f02438_ac10b5ef PS4, Line 4: The datasheet has extensive documentation on this, as after : it says that it supports it, it provides all the below information on : how to configure it: : : ``` : : ``` : As this information is extemely helpful, we wrote up the notes below on : how to configure it. While I appreciate the sense of humour, I'm not sure if everyone would understand the sarcasm here. Maybe dilute it a bit?
The Realtek ALC256 and several other codecs support a 7-band hardware equalizer. The datasheet clearly and earnestly encourages its readers to harness the entire potential of this hardware feature by providing absolutely no information on how to configure it.
https://review.coreboot.org/c/coreboot/+/59263/comment/8134c5e8_1cce98d1 PS4, Line 12: mamed typo? ma*i*med
https://review.coreboot.org/c/coreboot/+/59263/comment/3294e7a9_b6559f65 PS4, Line 13: killed Rest in pieces ðŸ˜
https://review.coreboot.org/c/coreboot/+/59263/comment/54ad4616_ef0b0b5a PS4, Line 19: 53 0x53, actually. Same for all other values.
https://review.coreboot.org/c/coreboot/+/59263/comment/c4f93e4f_0c11eb1b PS4, Line 28: 0x053404DA nit: Hex values in coreboot are typically written in lowercase
https://review.coreboot.org/c/coreboot/+/59263/comment/a9f37dda_60333638 PS4, Line 41: Butterworth HPF Butterworth High-Pass Filter? Maybe link to Wikipedia?
https://en.wikipedia.org/wiki/Butterworth_filter
https://review.coreboot.org/c/coreboot/+/59263/comment/00227485_dc298c33 PS4, Line 136: trailing space
https://review.coreboot.org/c/coreboot/+/59263/comment/631b9228_15f056b8 PS4, Line 136: /* : * EQ Mode = Boost Gain : * : * High Pass Filter 1: : * Band Width = 1000Hz : * Gain (dB) = 0dB : * : * High Pass Filter 2: : * Band Width = 1000Hz : * Gain (dB) = 0dB : * : * Band Pass Filter 1 : * Frequency = 3000Hz : * Band Width = 1000Hz : * Gain = 0dB : * : * Band Pass Filter 2 : * Frequency = 6000Hz : * Band Width = 1000Hz : * Gain = 0dB : * : * Band Pass Filter 3 : * Frequency = 9000Hz : * Band Width = 1000Hz : * Gain = 0dB : * : * Band Pass Filter 4 : * Frequency = 9000Hz : * Band Width = 1000Hz : * Gain = 0dB : * : * Low Pass Filter 1 : * Band Width = 1000Hz : * Gain (dB) = 0dB : * : * AGC Threshold = -6dB : * AGC Front Boost Gain = 6dB : * AGC Post Boost Gain = 6dB : * : * Power Output = 2.5W : */ I'd appreciate if you could split this comment and interleave the parts with the verb list, so that it's easy to see what does each group of verbs do.
https://review.coreboot.org/c/coreboot/+/59263/comment/1057f65e_2c976022 PS4, Line 306: Missing closing ``` ?
-
Angel Pons (Code Review)