Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36041 )
Change subject: drivers/gfx: Add generic graphics with SSDT generator ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36041/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36041/7//COMMIT_MSG@21 PS7, Line 21: register "device[0].privacy.detect_function" = ""\_SB.PCI0.PVSC.GPVD""
You can give the functions the same name and it will work across platforms, regardless of the implem […]
I have 2 thoughts.
1. I feel that the explicit method names gives a better understanding of what the code is doing with more flexibility. With the method names the next person to add this feature to a new platform will know that those methods must be defined. Without method names it is not obvious that some exact method under some exact scope must be defined for this to work. Documentation can help with that but I usually error on the side of making things easier for the next developer that comes along.
2. I'm not sure the best scope to put these new methods. I could have them under _SB.PCI0.GFX0 but that would limit us to one privacy screen per device, admittedly that is probably fine for now but it may cause some headaches if we ever needed more than one screen. I could do it under the device created by this driver, _SB.PCI0.GFX0.LCD for this example, and for that to work the functions would need to be defined in the DSDT for a device that will be defined in the SSDT. Is that allowed in ACPI?
With that said I am not opposed to changing it, I just wanted to share my thought I why I did things this way.