Mathew King has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36045 )
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
mb/g/drallion: Enable privacy screen on Drallion variant
Enable ACPI methods to control privacy screen on Drallion devices. Drallion devices may not have a privacy screen and it is up to the EC to determine if the privacy screen is present on the system.
BUG=b:142656363 TEST=emerge-drallion coreboot chromeos-bootimage
Change-Id: I79d02bb1b25f0deb49ae4bb852b7ed8c21fd31c7 Signed-off-by: Mathew King mathewk@chromium.org --- M src/mainboard/google/drallion/Kconfig M src/mainboard/google/drallion/dsdt.asl M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/drallion/variants/drallion/include/variant/ec.h 4 files changed, 18 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/36045/1
diff --git a/src/mainboard/google/drallion/Kconfig b/src/mainboard/google/drallion/Kconfig index 256f8cb..1de5ea5 100644 --- a/src/mainboard/google/drallion/Kconfig +++ b/src/mainboard/google/drallion/Kconfig @@ -2,6 +2,7 @@ config BOARD_GOOGLE_BASEBOARD_DRALLION def_bool n select BOARD_ROMSIZE_KB_32768 + select DRIVERS_GENERIC_GFX if BOARD_GOOGLE_DRALLION select DRIVERS_I2C_GENERIC select DRIVERS_I2C_HID select DRIVERS_INTEL_ISH diff --git a/src/mainboard/google/drallion/dsdt.asl b/src/mainboard/google/drallion/dsdt.asl index 2568800..5c2a5c9 100644 --- a/src/mainboard/google/drallion/dsdt.asl +++ b/src/mainboard/google/drallion/dsdt.asl @@ -39,6 +39,7 @@ { #include <soc/intel/cannonlake/acpi/northbridge.asl> #include <soc/intel/cannonlake/acpi/southbridge.asl> + #include <soc/intel/cannonlake/acpi/gfx.asl> } /* Per board variant mainboard hooks. */ #include <variant/acpi/mainboard.asl> diff --git a/src/mainboard/google/drallion/variants/drallion/devicetree.cb b/src/mainboard/google/drallion/variants/drallion/devicetree.cb index 1f628d6..184ab63 100644 --- a/src/mainboard/google/drallion/variants/drallion/devicetree.cb +++ b/src/mainboard/google/drallion/variants/drallion/devicetree.cb @@ -222,7 +222,19 @@ end device domain 0 on device pci 00.0 on end # Host Bridge - device pci 02.0 on end # Integrated Graphics Device + device pci 02.0 on + chip drivers/generic/gfx + register "device_count" = "1" + register "device[0].name" = ""LCD"" + register "device[0].addr" = "0x0400" + register "device[0].privacy.enabled" = "1" + register "device[0].privacy.detect_function" = ""\_SB.PCI0.LPCB.EC0.GPVD"" + register "device[0].privacy.status_function" = ""\_SB.PCI0.LPCB.EC0.GPVX"" + register "device[0].privacy.enable_function" = ""\_SB.PCI0.LPCB.EC0.EPVX"" + register "device[0].privacy.disable_function" = ""\_SB.PCI0.LPCB.EC0.DPVX"" + device generic 0 on end + end + end # Integrated Graphics Device device pci 04.0 on end # SA Thermal device device pci 12.0 on end # Thermal Subsystem device pci 12.5 off end # UFS SCS diff --git a/src/mainboard/google/drallion/variants/drallion/include/variant/ec.h b/src/mainboard/google/drallion/variants/drallion/include/variant/ec.h index 01a17b5..9cb8ed0 100644 --- a/src/mainboard/google/drallion/variants/drallion/include/variant/ec.h +++ b/src/mainboard/google/drallion/variants/drallion/include/variant/ec.h @@ -31,4 +31,7 @@ /* Enable DPTF */ #define EC_ENABLE_DPTF
+/* Enable Privacy */ +#define EC_ENABLE_PRIVACY + #endif
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36045
to look at the new patch set (#3).
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
mb/g/drallion: Enable privacy screen on Drallion variant
Enable ACPI methods to control privacy screen on Drallion devices. Drallion devices may not have a privacy screen and it is up to the EC to determine if the privacy screen is present on the system.
BUG=b:142656363 TEST=emerge-drallion coreboot chromeos-bootimage
Change-Id: I79d02bb1b25f0deb49ae4bb852b7ed8c21fd31c7 Signed-off-by: Mathew King mathewk@chromium.org --- M src/mainboard/google/drallion/Kconfig M src/mainboard/google/drallion/dsdt.asl M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/drallion/variants/drallion/include/variant/ec.h 4 files changed, 18 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/36045/3
Rajat Jain has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36045 )
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36045/7/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/36045/7/src/mainboard/google/dralli... PS7, Line 229: 0x0400 I actually think that the _ADR shoud be 0x80000400. Looked at the ACPI spec pointed at by Duncan, https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf pages 1119 - 1123. We need the bit 31 set ("Device ID Scheme" bit)
Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36045 )
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36045/7/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/36045/7/src/mainboard/google/dralli... PS7, Line 229: 0x0400
I actually think that the _ADR shoud be 0x80000400. […]
Yeah I agree that bit 31 should be set and I believe that bit 16 should be set as well. My reading of the spec is that the values returned from _DOD should be exactly the _ADR of each device. In src/drivers/intel/gma/acpi.c bits 31 and 16 are always set in _DOD and always cleared in _ADR and I think that is wrong according to the spec.
Rajat Jain has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36045 )
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36045/7/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/36045/7/src/mainboard/google/dralli... PS7, Line 229: 0x0400
Yeah I agree that bit 31 should be set and I believe that bit 16 should be set as well. […]
Another thing, I currently see kernel complains with this when booting up:
[Firmware Bug]: ACPI(GFX0) defines _DOD but not _DOS
Do we need _DOS as well?
Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36045 )
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36045/7/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/36045/7/src/mainboard/google/dralli... PS7, Line 229: 0x0400
Another thing, I currently see kernel complains with this when booting up: […]
According to the ACPI spec A.3.1 _DOS should only exist if "the platform runtime firmware is capable of automatically switching the active display output". So I'm not really sure why the kernel considers this a firmware bug.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36045
to look at the new patch set (#9).
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
mb/g/drallion: Enable privacy screen on Drallion variant
Enable ACPI methods to control privacy screen on Drallion devices. Drallion devices may not have a privacy screen and it is up to the EC to determine if the privacy screen is present on the system.
BUG=b:142656363 TEST=emerge-drallion coreboot chromeos-bootimage
Change-Id: I79d02bb1b25f0deb49ae4bb852b7ed8c21fd31c7 Signed-off-by: Mathew King mathewk@chromium.org --- M src/mainboard/google/drallion/Kconfig M src/mainboard/google/drallion/dsdt.asl M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/drallion/variants/drallion/include/variant/ec.h 4 files changed, 18 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/36045/9
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36045
to look at the new patch set (#10).
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
mb/g/drallion: Enable privacy screen on Drallion variant
Enable ACPI methods to control privacy screen on Drallion devices. Drallion devices may not have a privacy screen and it is up to the EC to determine if the privacy screen is present on the system.
BUG=b:142656363 TEST=emerge-drallion coreboot chromeos-bootimage
Change-Id: I79d02bb1b25f0deb49ae4bb852b7ed8c21fd31c7 Signed-off-by: Mathew King mathewk@chromium.org --- M src/mainboard/google/drallion/Kconfig M src/mainboard/google/drallion/dsdt.asl M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/drallion/variants/drallion/include/variant/ec.h 4 files changed, 18 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/36045/10
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36045 )
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36045/10/src/mainboard/google/drall... File src/mainboard/google/drallion/Kconfig:
https://review.coreboot.org/c/coreboot/+/36045/10/src/mainboard/google/drall... PS10, Line 5: if BOARD_GOOGLE_DRALLION probably doesn't need the if clause, if the device isn't in devicetree it should get removed by LTO.
https://review.coreboot.org/c/coreboot/+/36045/10/src/mainboard/google/drall... File src/mainboard/google/drallion/variants/drallion/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/36045/10/src/mainboard/google/drall... PS10, Line 229: 0x80010400 add a comment describing the format?
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36045
to look at the new patch set (#12).
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
mb/g/drallion: Enable privacy screen on Drallion variant
Enable ACPI methods to control privacy screen on Drallion devices. Drallion devices may not have a privacy screen and it is up to the EC to determine if the privacy screen is present on the system.
BUG=b:142656363 TEST=emerge-drallion coreboot chromeos-bootimage
Change-Id: I79d02bb1b25f0deb49ae4bb852b7ed8c21fd31c7 Signed-off-by: Mathew King mathewk@chromium.org --- M src/mainboard/google/drallion/Kconfig M src/mainboard/google/drallion/dsdt.asl M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/drallion/variants/drallion/include/variant/ec.h 4 files changed, 20 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/36045/12
Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36045 )
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36045/10/src/mainboard/google/drall... File src/mainboard/google/drallion/Kconfig:
https://review.coreboot.org/c/coreboot/+/36045/10/src/mainboard/google/drall... PS10, Line 5: if BOARD_GOOGLE_DRALLION
probably doesn't need the if clause, if the device isn't in devicetree it should get removed by LTO.
Done
https://review.coreboot.org/c/coreboot/+/36045/10/src/mainboard/google/drall... File src/mainboard/google/drallion/variants/drallion/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/36045/10/src/mainboard/google/drall... PS10, Line 229: 0x80010400
add a comment describing the format?
Done
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36045
to look at the new patch set (#13).
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
mb/g/drallion: Enable privacy screen on Drallion variant
Enable ACPI methods to control privacy screen on Drallion devices. Drallion devices may not have a privacy screen and it is up to the EC to determine if the privacy screen is present on the system.
BUG=b:142656363 TEST=emerge-drallion coreboot chromeos-bootimage
Change-Id: I79d02bb1b25f0deb49ae4bb852b7ed8c21fd31c7 Signed-off-by: Mathew King mathewk@chromium.org --- M src/mainboard/google/drallion/Kconfig M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/drallion/variants/drallion/include/variant/ec.h 3 files changed, 19 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/36045/13
Simon Glass has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36045 )
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
Patch Set 14: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/36045/14/src/mainboard/google/drall... File src/mainboard/google/drallion/variants/drallion/include/variant/ec.h:
https://review.coreboot.org/c/coreboot/+/36045/14/src/mainboard/google/drall... PS14, Line 34: Privacy Privacy screen functionality (does not mean the hardware actually has one)
Hello Simon Glass, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36045
to look at the new patch set (#15).
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
mb/g/drallion: Enable privacy screen on Drallion variant
Enable ACPI methods to control privacy screen on Drallion devices. Drallion devices may not have a privacy screen and it is up to the EC to determine if the privacy screen is present on the system.
BUG=b:142656363 TEST=emerge-drallion coreboot chromeos-bootimage
Change-Id: I79d02bb1b25f0deb49ae4bb852b7ed8c21fd31c7 Signed-off-by: Mathew King mathewk@chromium.org --- M src/mainboard/google/drallion/Kconfig M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/drallion/variants/drallion/include/variant/ec.h 3 files changed, 19 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/36045/15
Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36045 )
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36045/14/src/mainboard/google/drall... File src/mainboard/google/drallion/variants/drallion/include/variant/ec.h:
https://review.coreboot.org/c/coreboot/+/36045/14/src/mainboard/google/drall... PS14, Line 34: Privacy
Privacy screen functionality (does not mean the hardware actually has one)
Done
Simon Glass has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36045 )
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
Patch Set 15: Code-Review+2
Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36045 )
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36045/7/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/36045/7/src/mainboard/google/dralli... PS7, Line 229: 0x0400
According to the ACPI spec A.3. […]
Ack
Duncan Laurie has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36045 )
Change subject: mb/g/drallion: Enable privacy screen on Drallion variant ......................................................................
mb/g/drallion: Enable privacy screen on Drallion variant
Enable ACPI methods to control privacy screen on Drallion devices. Drallion devices may not have a privacy screen and it is up to the EC to determine if the privacy screen is present on the system.
BUG=b:142656363 TEST=emerge-drallion coreboot chromeos-bootimage
Change-Id: I79d02bb1b25f0deb49ae4bb852b7ed8c21fd31c7 Signed-off-by: Mathew King mathewk@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/36045 Reviewed-by: Simon Glass sjg@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/drallion/Kconfig M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/drallion/variants/drallion/include/variant/ec.h 3 files changed, 19 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Simon Glass: Looks good to me, approved
diff --git a/src/mainboard/google/drallion/Kconfig b/src/mainboard/google/drallion/Kconfig index accb9c8..27818c0 100644 --- a/src/mainboard/google/drallion/Kconfig +++ b/src/mainboard/google/drallion/Kconfig @@ -2,6 +2,7 @@ config BOARD_GOOGLE_BASEBOARD_DRALLION def_bool n select BOARD_ROMSIZE_KB_32768 + select DRIVERS_GENERIC_GFX select DRIVERS_I2C_GENERIC select DRIVERS_I2C_HID select DRIVERS_INTEL_ISH diff --git a/src/mainboard/google/drallion/variants/drallion/devicetree.cb b/src/mainboard/google/drallion/variants/drallion/devicetree.cb index 6ecb689..4c5cff2 100644 --- a/src/mainboard/google/drallion/variants/drallion/devicetree.cb +++ b/src/mainboard/google/drallion/variants/drallion/devicetree.cb @@ -224,7 +224,21 @@ end device domain 0 on device pci 00.0 on end # Host Bridge - device pci 02.0 on end # Integrated Graphics Device + device pci 02.0 on + chip drivers/generic/gfx + register "device_count" = "1" + register "device[0].name" = ""LCD"" + # Address is set following the ACPI spec section A.3.2 + # for an internal panel on the first port of the graphics chip + register "device[0].addr" = "0x80010400" + register "device[0].privacy.enabled" = "1" + register "device[0].privacy.detect_function" = ""\_SB.PCI0.LPCB.EC0.GPVD"" + register "device[0].privacy.status_function" = ""\_SB.PCI0.LPCB.EC0.GPVX"" + register "device[0].privacy.enable_function" = ""\_SB.PCI0.LPCB.EC0.EPVX"" + register "device[0].privacy.disable_function" = ""\_SB.PCI0.LPCB.EC0.DPVX"" + device generic 0 on end + end + end # Integrated Graphics Device device pci 04.0 on end # SA Thermal device device pci 12.0 on end # Thermal Subsystem device pci 12.5 off end # UFS SCS diff --git a/src/mainboard/google/drallion/variants/drallion/include/variant/ec.h b/src/mainboard/google/drallion/variants/drallion/include/variant/ec.h index 01a17b5..11e3be8 100644 --- a/src/mainboard/google/drallion/variants/drallion/include/variant/ec.h +++ b/src/mainboard/google/drallion/variants/drallion/include/variant/ec.h @@ -31,4 +31,7 @@ /* Enable DPTF */ #define EC_ENABLE_DPTF
+/* Enable privacy screen functionality */ +#define EC_ENABLE_PRIVACY + #endif
-
Duncan Laurie (Code Review) -
Mathew King (Code Review)
-
Rajat Jain (Code Review)
-
Simon Glass (Code Review)