Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29651 )
Change subject: soc/intel/cannonlake: Program HD Audio SVID/SSID ......................................................................
Patch Set 5:
(1 comment)
Is this just setting subsystem ids of a PCI device? These should be set in the devicetree and no blob needed.
If yes, this seems backwards. Shouldn't we set the id of the PCI device first and when configuring the codec take the id of the PCI device?
From https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/soun..., line 651, we can see kernel driver is comparing the ssid with pci and verb.
ass = codec->core.subsystem_id & 0xffff; if (codec->bus->pci && ass != codec->bus->pci->subsystem_device && (ass & 1)) goto do_sku;
No need to quote the code, I already believed the commit message.
I believe the ssid of detail codec is fixed
It's been some years since I worked with audio codecs, IIRC, this was writeable. Datasheet?
https://review.coreboot.org/#/c/29651/5/src/soc/intel/cannonlake/fsp_params.... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/#/c/29651/5/src/soc/intel/cannonlake/fsp_params.... PS5, Line 132: params->SiSsidTablePtr = (uintptr_t)ssidtblptr; If we are passing this to FSP, the struct has to be defined as part of FSP's UPD headers, not by coreboot.