Attention is currently required from: Felix Singer, Keith Hui.
Nicholas Sudsgaard has posted comments on this change by Nicholas Sudsgaard. ( https://review.coreboot.org/c/coreboot/+/80333?usp=email )
Change subject: device/azalia: Rework azalia verb tables ......................................................................
Patch Set 9:
(4 comments)
This change is ready for review.
Patchset:
PS5:
Did some quick tests on compiler explorer and it seems like using pointers can cut down the instruct […]
Instruction count is not that important. Using pointers to avoid stack overflows, as this struct is quite big.
File src/include/device/azalia_device.h:
https://review.coreboot.org/c/coreboot/+/80333/comment/63e65bae_2660ebfe?usp... : PS2, Line 21: #define HDA_MAX_CORBSIZE 256
Not sure when to use the HDA_ prefix and the AZALIA_ prefix.
Not relevant for new patchset.
File src/include/device/azalia_table_exporter.c:
https://review.coreboot.org/c/coreboot/+/80333/comment/c5eca2a3_3b945ee5?usp... : PS7, Line 20:
This file is assumed to be #included at the end of mainboard hda_verb.c. […]
Thanks for pointing that out! `mainboard_azalia_program_runtime_verbs` should work fine without any modifications in the new patchset (tested a simple function).
File src/mainboard/hp/snb_ivb_laptops/variants/2560p/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/80333/comment/812ad63a_0da9834a?usp... : PS7, Line 10: 0
How about adding another attribute for the codec number to the struct? So it's not repeated over and […]
I have experimented with that, but I'm not sure how to do that without adding some code to runtime. There are some benefits to moving all the verb initialization to runtime (similar to `init` of `chip_operations`). - No need to repeat the codec address. Though, it would look like `AZALIA_PIN_CFG(codec, 0x0a, 0x40f000f0)` instead. - `verb_count` can be set automatically - AFAIK there is no benefits using macros for `AZALIA_VERB_12B` etc... we can change those to inline functions for some type safety? - `AZALIA_SUBVENDOR` can be set automatically (`AZALIA_RESET` as well?) - A separate `mainboard_azalia_program_runtime_verbs` is not needed
You could still add the values directly into `.verbs` if you really need that performance.