Varshit B Pandya has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in waddledee ......................................................................
mb/google/dedede: Add ACPI support for camera in waddledee
1. Add support as per the schematics 2. Add 2 Ports and 2 Endpoints 3. Add support for OTVI9734 and OTVI5676 4. Add ON and OFF logic as Power Rails are same for both sensor
BUG=None BRANCH=None TEST=Build and Boot waddledee board and able to capture image using world facing camera and user facing camera.
Change-Id: I3f6b490832d2c34c86512384eb7470f13a212a66 Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com --- M src/mainboard/google/dedede/Kconfig.name A src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/cam0.asl A src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/cam1.asl A src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/camera.asl A src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/ipu_endpoints.asl A src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/ipu_mainboard.asl 6 files changed, 559 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/42205/1
diff --git a/src/mainboard/google/dedede/Kconfig.name b/src/mainboard/google/dedede/Kconfig.name index 25ad61f..b6e1d64 100644 --- a/src/mainboard/google/dedede/Kconfig.name +++ b/src/mainboard/google/dedede/Kconfig.name @@ -18,6 +18,7 @@ select BOARD_GOOGLE_BASEBOARD_DEDEDE select BASEBOARD_DEDEDE_LAPTOP select BOARD_ROMSIZE_KB_32768 + select VARIANT_HAS_CAMERA_ACPI
config BOARD_GOOGLE_WHEELIE bool "Wheelie" diff --git a/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/cam0.asl b/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/cam0.asl new file mode 100644 index 0000000..90c7d58 --- /dev/null +++ b/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/cam0.asl @@ -0,0 +1,238 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Scope (_SB.PCI0.I2C3) +{ + Name (STA0, Zero) + + /* Method to turn off Power Rails */ + Method (POFF, 0) + { + /* Disable PP1200 lane */ + CTXS(GPP_D14) + /* Disable PP2800 lane */ + CTXS(GPP_D13) + } + + Method (PON, 0) + { + /* Enable PP2800 lane */ + STXS(GPP_D13) + /* Enable PP1200 lane */ + STXS(GPP_D14) + } + + PowerResource (FCPR, 0x00, 0x0000) + { + Method (_ON, 0, Serialized) /* _ON_: Power On */ + { + MCON(1, 1) /* Clock 1, 19.2MHz */ + IF(!STA1) + { + /* Other sensor is OFF, so turn on power signals. */ + PON() + } + /* Assert Reset */ + CTXS(GPP_D12) + Sleep(5) /* 5 us */ + /* Deassert Reset */ + STXS(GPP_D12) + Sleep(5) /* 5 us */ + STA0 = 1 + } + Method (_OFF, 0, Serialized) /* _OFF_: Power Off */ + { + MCOF(1) /* Clock 1 */ + /* Assert Reset */ + CTXS(GPP_D12) + IF(!STA1) + { + /* Other sensor is OFF, so turn off power signals. */ + POFF() + } + STA0 = 0 + } + Method (_STA, 0, NotSerialized) /* _STA: Status */ + { + Return (STA0) + } + } + + Device (CAM0) + { + Name (_HID, "OVTI5675") /* _HID: Hardware ID */ + + Name (_UID, Zero) /* _UID: Unique ID */ + + Name (_DDN, "Ov 5675 Camera") /* _DDN: DOS Device Name */ + + Method (_STA, 0, NotSerialized) /* _STA: Status */ + { + Return (0x0F) + } + + Name (_CRS, ResourceTemplate () /* _CRS: Current Resource Settings */ + { + I2cSerialBus (0x0036, ControllerInitiated, 0x00061A80, + AddressingMode7Bit, "\_SB.PCI0.I2C3", + 0x00, ResourceConsumer, , + ) + }) + + Name (_PR0, Package (0x01) /* _PR0: Power Resources for D0 */ + { + FCPR + }) + Name (_PR3, Package (0x01) /* _PR3: Power Resources for D3hot */ + { + FCPR + }) + + Name (_DSD, Package (0x04) /* _DSD: Device-Specific Data */ + { + ToUUID ("dbb8e3e6-5886-4ba6-8795-1319f52a966b"), + Package (0x01) + { + Package (0x02) + { + "port0", + "PRT0" + } + }, + + ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package (0x02) + { + Package (0x02) + { + "clock-frequency", + 0x0124F800 + }, + + Package (0x02) + { + "lens-focus", + Package (0x01) + { + VCM0 + } + } + } + }) + + Name (PRT0, Package (0x04) + { + ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package (0x01) + { + Package (0x02) + { + "port", + Zero + } + }, + + ToUUID ("dbb8e3e6-5886-4ba6-8795-1319f52a966b"), + Package (0x01) + { + Package (0x02) + { + "endpoint0", + "EP00" + } + } + }) + + Name (EP00, Package (0x02) + { + ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package (0x04) + { + Package (0x02) + { + "endpoint", + Zero + }, + + Package (0x02) + { + "data-lanes", + Package (0x02) + { + One, + 0x02 + } + }, + + Package (0x02) + { + "link-frequencies", + Package (0x01) + { + 0x1AD27480 + } + }, + + Package (0x02) + { + "remote-endpoint", + Package (0x03) + { + IPU0, + One, + Zero + } + } + } + }) + } + + Device (VCM0) + { + Name (_HID, "PRP0001") /* _HID: Hadware ID */ + + Name (_UID, 0x03) /* _UID: Unique ID */ + + Name (_DDN, "DW9714 VCM") /* _DDN: DOS Device Name */ + + Method (_STA, 0, NotSerialized) /* _STA: Status */ + { + Return (0x0F) + } + + Name (_CRS, ResourceTemplate () /* _CRS: Current Resource Setting */ + { + I2cSerialBusV2 (0x000C, ControllerInitiated, 0x00061A80, + AddressingMode7Bit, "\_SB.PCI0.I2C5", + 0x00, ResourceConsumer, , Exclusive, + ) + }) + + Name (_DEP, Package (0x01) /* _DEP: Dependencies */ + { + CAM0 + }) + + Name (_PR0, Package (0x01) /* _PR0: Power Resources for D0 */ + { + FCPR + }) + + Name (_PR3, Package (0x01) /* _PR3: Power Resources for D3Hot */ + { + FCPR + }) + + Name (_DSD, Package (0x02) /* _DSD: Device-Specific Data */ + { + ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), /* Device Properties for _DSD */ + Package(0x01) + { + Package (0x02) + { + "compatible", + "dongwoon,dw9714" + } + } + }) + } +} diff --git a/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/cam1.asl b/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/cam1.asl new file mode 100644 index 0000000..3e76197 --- /dev/null +++ b/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/cam1.asl @@ -0,0 +1,152 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Scope (_SB.PCI0.I2C3) +{ + Name (STA1, Zero) + + PowerResource (RCPR, 0x00, 0x0000) + { + Method (_ON, 0, Serialized) /* _ON_: Power On */ + { + MCON(0, 1) /* Clock 0, 19.2MHz */ + /* Check if another sensor is ON */ + IF(!STA0) + { + /* Other sensor is OFF, so turn on power signals. */ + PON() + } + /* Assert Reset */ + CTXS(GPP_D15) + Sleep(5) /* 5 us */ + /* DeAssert Reset */ + STXS(GPP_D15) + Sleep(5) /* 5 us */ + STA1 = 1 + } + + Method (_OFF, 0, Serialized) /* _OFF_: Power Off */ + { + MCOF(0) /* Clock 0 */ + /* Assert Reset */ + CTXS(GPP_D15) + IF(!STA0) + { + /* Other sensor is OFF, so turn off power signals. */ + POFF() + } + STA1 = 0 + } + + Method (_STA, 0, NotSerialized) /* _STA: Status */ + { + Return (STA1) + } + } + + Device(CAM1) + { + Name (_HID, "OVTI9734") /* _HID: Hardware ID */ + + Name (_UID, Zero) /* _UID: Unique ID */ + + Name (_DDN, "Ov 9734 Camera") /* _DDN: DOS Device Name */ + + Method (_STA, 0, NotSerialized) /* _STA: Status */ + { + Return (0x0F) + } + + Name (_CRS, ResourceTemplate () /* _CRS: Current Resource Settings */ + { + I2cSerialBus (0x0036, ControllerInitiated, 0x00061A80, + AddressingMode7Bit, "\_SB.PCI0.I2C3", + 0x00, ResourceConsumer, , + ) + }) + + Name (_PR0, Package (0x01) /* _PR0: Power Resources for D0 */ + { + RCPR + }) + + Name (_PR3, Package (0x01) /* _PR3: Power Resources for D3hot */ + { + RCPR + }) + + Name (_DSD, Package (0x04) /* _DSD: Device-Specific Data */ + { + ToUUID ("dbb8e3e6-5886-4ba6-8795-1319f52a966b"), + Package (0x01) + { + Package (0x02) + { + "port0", + "PRT0" + } + }, + ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package (0x01) + { + Package (0x02) + { + "clock-frequency", + 0x0124F800 + }, + } + }) + + Name (PRT0, Package (0x04) + { + ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package (0x01) + { + Package (0x02) + { + "port", + Zero + } + }, + ToUUID ("dbb8e3e6-5886-4ba6-8795-1319f52a966b"), + Package (0x01) + { + Package (0x02) + { + "endpoint0", + "EP00" + } + } + }) + + Name (EP00, Package (0x02) + { + ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package (0x03) + { + Package (0x02) + { + "endpoint", + Zero + }, + Package (0x02) + { + "link-frequencies", + Package (0x01) + { + 0x0ABA9500 + } + }, + Package (0x02) + { + "remote-endpoint", + Package (0x03) + { + IPU0, + Zero, + Zero + } + } + } + }) + } +} diff --git a/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/camera.asl b/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/camera.asl new file mode 100644 index 0000000..51b4ebc --- /dev/null +++ b/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/camera.asl @@ -0,0 +1,6 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include "ipu_mainboard.asl" +#include "ipu_endpoints.asl" +#include "cam0.asl" +#include "cam1.asl" diff --git a/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/ipu_endpoints.asl b/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/ipu_endpoints.asl new file mode 100644 index 0000000..89ccb48 --- /dev/null +++ b/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/ipu_endpoints.asl @@ -0,0 +1,83 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Scope (_SB.PCI0.IPU0) +{ + Name (EP00, Package (0x02) + { + ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package (0x04) + { + Package (0x02) + { + "endpoint", + Zero + }, + + Package (0x02) + { + "clock-lanes", + Zero + }, + + Package (0x02) + { + "data-lanes", + Package (0x02) + { + One, + 0x02, + } + }, + + Package (0x02) + { + "remote-endpoint", + Package (0x03) + { + ^I2C3.CAM0, + Zero, + Zero + } + } + } + }) + + Name (EP10, Package (0x02) + { + ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package (0x04) + { + Package (0x02) + { + "endpoint", + Zero + }, + + Package (0x02) + { + "clock-lanes", + Zero + }, + + Package (0x02) + { + "data-lanes", + Package (0x01) + { + One, + } + }, + + Package (0x02) + { + "remote-endpoint", + Package (0x03) + { + ^I2C3.CAM1, + Zero, + Zero + } + } + } + }) +} diff --git a/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/ipu_mainboard.asl b/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/ipu_mainboard.asl new file mode 100644 index 0000000..8d37674 --- /dev/null +++ b/src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/ipu_mainboard.asl @@ -0,0 +1,79 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Scope (_SB.PCI0) +{ + Device (IPU0) + { + Name (_ADR, 0x00050000) /* _ADR: Address */ + + Name (_DDN, "Camera and Imaging Subsystem") /* _DDN: DOS Device Name */ + } +} + +Scope (_SB.PCI0.IPU0) +{ + Name (_DSD, Package (0x02) /* _DSD: Device-Specific Data */ + { + ToUUID ("dbb8e3e6-5886-4ba6-8795-1319f52a966b"), + Package (0x02) + { + Package (0x02) + { + "port0", + "PRT0" + }, + + Package (0x02) + { + "port1", + "PRT1" + } + } + }) + + Name (PRT0, Package (0x04) + { + ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package (0x01) + { + Package (0x02) + { + "port", + Zero + } + }, + + ToUUID ("dbb8e3e6-5886-4ba6-8795-1319f52a966b"), + Package (0x01) + { + Package (0x02) + { + "endpoint0", + "EP00" + } + } + }) + + Name (PRT1, Package (0x04) + { + ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package (0x01) + { + Package (0x02) + { + "port", + 2 + } + }, + + ToUUID ("dbb8e3e6-5886-4ba6-8795-1319f52a966b"), + Package (0x01) + { + Package (0x02) + { + "endpoint0", + "EP10" + } + } + }) +}
Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in waddledee ......................................................................
Patch Set 9:
This change is ready for review.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in waddledee ......................................................................
Patch Set 9:
FYI, +Sugnan, Matt Delco and I are going through a patchset (almost merged!) that is using the SSDT runtime generation mechanism to generate these camera devices. Let me take a look at this and that patch train, and see if/how we can use those to generate this ASL/AML.
See the patch train starting at CB:41607
Matt Delco has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in waddledee ......................................................................
Patch Set 9:
see if/how we can use those to generate this ASL/AML.
See the patch train starting at CB:41607
I skipped over the PowerResource sequence, but otherwise the only inadequacy of the patch train I noticed is that the mipi driver assumes the IPU ports are used sequentially (e.g., port 0 and 1) but ipu_mainboard.asl is using port=0 in PRT1 and port=2 in PRT1.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in waddledee ......................................................................
Patch Set 9:
Varshit, are you up for converting this to use the runtime generation? Or would you like me to take a look?
Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in waddledee ......................................................................
Patch Set 9:
Patch Set 9:
Varshit, are you up for converting this to use the runtime generation? Or would you like me to take a look?
Hello Tim, yes, the patch train you have shared has one known limitation it cannot support the ASL generation in a case where we have shared power resources across two camera (sugnan is working on it), till the time new patches are reviewed and merged we will need this patch.
Matt Delco has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in waddledee ......................................................................
Patch Set 9:
Patch Set 9:
Hello Tim, yes, the patch train you have shared has one known limitation it cannot support the ASL generation in a case where we have shared power resources across two camera (sugnan is working on it), till the time new patches are reviewed and merged we will need this patch.
A potential hybrid approach (ideally for the intermediate term) is to keep the PowerResources in ASL files but use the mipi_camera driver for everything else. That's basically what I was doing before the mipi_camera driver was enhanced to handle power, e.g.:
https://review.coreboot.org/c/coreboot/+/31166 https://review.coreboot.org/c/coreboot/+/31164
Sugnan Prabhu S has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in waddledee ......................................................................
Patch Set 9:
@Matt,@Tim
Please check the following patch, which handles the shared resources with a guarded variable. https://review.coreboot.org/c/coreboot/+/43003
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in waddledee ......................................................................
Patch Set 9:
Patch Set 9:
@Matt,@Tim
Please check the following patch, which handles the shared resources with a guarded variable. https://review.coreboot.org/c/coreboot/+/43003
Varshit, how urgent is this camera support? Can it wait until Sugnan's patch train (including the guarding) is ready? It is very nearly done. If not, then I will take a look at this, otherwise we can put a hold on this and convert it
Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in waddledee ......................................................................
Patch Set 9:
Patch Set 9:
Patch Set 9:
@Matt,@Tim
Please check the following patch, which handles the shared resources with a guarded variable. https://review.coreboot.org/c/coreboot/+/43003
Varshit, how urgent is this camera support? Can it wait until Sugnan's patch train (including the guarding) is ready? It is very nearly done. If not, then I will take a look at this, otherwise we can put a hold on this and convert it
Hello Tim,
We can put this on hold till the other patches are merged.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in waddledee ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42205/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/cam0.asl:
https://review.coreboot.org/c/coreboot/+/42205/9/src/mainboard/google/dedede... PS9, Line 36: Sleep(5) /* 5 us */ So all of the comments in here for the Sleeps say `us` implying microseconds, but the ASL Sleep op sleeps for *milliseconds*. So are the comments wrong, or do you actually only need to sleep for 5 us? Because the finest granularity is the 1ms of Sleep, so a Sleep(1) would be the best you could do.
Hello build bot (Jenkins), Furquan Shaikh, Maulik V Vaghela, Tim Wawrzynczak, Rizwan Qureshi, Krishna P Bhat D, Aamir Bohra, Ronak Kanabar, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42205
to look at the new patch set (#10).
Change subject: mb/google/dedede: Add ACPI support for camera in waddledee ......................................................................
mb/google/dedede: Add ACPI support for camera in waddledee
1. Add support as per the schematics 2. Add 2 Ports and 2 Endpoints 3. Add support for OTVI9734 and OTVI5676 4. Add ON and OFF logic as Power Rails are same for both sensor
BUG=None BRANCH=None TEST=Build and Boot waddledee board and able to capture image using world facing camera.
Change-Id: I3f6b490832d2c34c86512384eb7470f13a212a66 Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com --- M src/mainboard/google/dedede/variants/waddledee/overridetree.cb 1 file changed, 92 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/42205/10
Andy Yeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in WDee ......................................................................
Patch Set 16:
This change is ready for review.
Andy Yeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in WDee ......................................................................
Patch Set 17: Code-Review-1
(1 comment)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/42205/17/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledee/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42205/17/src/mainboard/google/deded... PS17, Line 118: device pci 15.3 on # I2C 3 Why there is no NVM i2c device defined?
Tomasz Figa has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in WDee ......................................................................
Patch Set 19:
(1 comment)
Could someone also share a dump of the final DSDT table that is generated from this?
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledee/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... PS19, Line 197: register "pr0" = ""\_SB.PCI0.I2C3.CAM1.PRIC"" In general we shouldn't need to power on the sensor to access the EEPROM. Might not be a problem for this one, since this is rear-facing and might not have a privacy LED (for someone to check), but in general it would cause the LED to blink which isn't acceptable because of user concerns.
Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in WDee ......................................................................
Patch Set 19:
(5 comments)
https://review.coreboot.org/c/coreboot/+/42205/15//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42205/15//COMMIT_MSG@2 PS15, Line 2: Pandya, Varshit B
I’d be great if you put your first name first, so no comma is used.
Done
https://review.coreboot.org/c/coreboot/+/42205/15//COMMIT_MSG@9 PS15, Line 9: overridedevice
overridetree
Done
https://review.coreboot.org/c/coreboot/+/42205/15//COMMIT_MSG@14 PS15, Line 14:
Tested how?
Still testing this patch, will update the details.
https://review.coreboot.org/c/coreboot/+/42205/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/waddledee/include/variant/acpi/cam0.asl:
https://review.coreboot.org/c/coreboot/+/42205/9/src/mainboard/google/dedede... PS9, Line 36: Sleep(5) /* 5 us */
So all of the comments in here for the Sleeps say `us` implying microseconds, but the ASL Sleep op s […]
Ack
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledee/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... PS19, Line 197: register "pr0" = ""\_SB.PCI0.I2C3.CAM1.PRIC""
In general we shouldn't need to power on the sensor to access the EEPROM. […]
Attaching the SSDT CAM related entries from WDoo, I donot have the WDee board right now
Scope (_SB.PCI0.I2C3) { Device (CAM1) { PowerResource (PRIC, 0x00, 0x0000) { Name (STA, Zero) Method (_STA, 0, NotSerialized) // _STA: Status { Return (STA) /* _SB_.PCI0.I2C3.CAM1.PRIC.STA_ */ }
Method (_ON, 0, Serialized) // _ON_: Power On { If ((STA == Zero)) { ENB0 () ENB1 () Sleep (0x05) ENB2 () Sleep (0x05) DSB3 () Sleep (0x05) ENB3 () Sleep (0x05) STA = One } }
Method (_OFF, 0, Serialized) // _OFF: Power Off { If ((STA == One)) { DSB0 () DSB3 () DSB2 () DSB1 () STA = Zero } } }
Name (_ADR, Zero) // _ADR: Address Name (_HID, "OVTI8856") // _HID: Hardware ID Name (_UID, Zero) // _UID: Unique ID Name (_DDN, "Ov 8856 Camera") // _DDN: DOS Device Name Method (_STA, 0, NotSerialized) // _STA: Status { Return (0x0F) }
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings { I2cSerialBusV2 (0x0010, ControllerInitiated, 0x00061A80, AddressingMode7Bit, "\_SB.PCI0.I2C3", 0x00, ResourceConsumer, , Exclusive, ) }) Name (CAMD, 0x02) Name (_PR0, Package (0x01) // _PR0: Power Resources for D0 { PRIC }) Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method { ToBuffer (Arg0, Local0) If ((Local0 == ToUUID ("822ace8f-2814-4174-a56b-5f029fe079ee"))) { Return ("UNKNOWN") }
If ((Local0 == ToUUID ("26257549-9271-4ca4-bb43-c4899d5a4881"))) { ToInteger (Arg2, Local1) If ((Local1 == One)) { Return (0x02) }
If ((Local1 == 0x02)) { Return (0x05001000) }
If ((Local1 == 0x03)) { Return (0x05000001) } }
Return (Buffer (One) { 0x00 // . }) }
Name (_DSD, Package (0x04) // _DSD: Device-Specific Data { ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */, Package (0x02) { Package (0x02) { "clock-frequency", 0x0124F800 },
Package (0x02) { "lens-focus", Package (0x01) { VCM0 } } },
ToUUID ("dbb8e3e6-5886-4ba6-8795-1319f52a966b"), Package (0x01) { Package (0x02) { "port0", "PRT0" } } }) Name (PRT0, Package (0x04) { ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */, Package (0x01) { Package (0x02) { "port", Zero } },
ToUUID ("dbb8e3e6-5886-4ba6-8795-1319f52a966b"), Package (0x01) { Package (0x02) { "endpoint0", "EP00" } } }) Name (EP00, Package (0x02) { ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */, Package (0x05) { Package (0x02) { "endpoint", Zero },
Package (0x02) { "clock-lanes", Zero },
Package (0x02) { "data-lanes", Package (0x04) { One, 0x02, 0x03, 0x04 } },
Package (0x02) { "link-frequencies", Package (0x02) { 0x15752A00, 0x0ABA9500 } },
Package (0x02) { "remote-endpoint", Package (0x03) { IPU0, One, Zero } } } }) Method (SSDB, 0, Serialized) { Return (Buffer (0x70) { /* 0000 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // ........ /* 0008 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // ........ /* 0010 */ 0x00, 0x00, 0xA3, 0x00, 0x00, 0x00, 0x00, 0x00, // ........ /* 0018 */ 0x00, 0x00, 0x00, 0x00, 0x01, 0x04, 0x00, 0x00, // ........ /* 0020 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // ........ /* 0028 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // ........ /* 0030 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // ........ /* 0038 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // ........ /* 0040 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // ........ /* 0048 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0C, // ........ /* 0050 */ 0x09, 0x00, 0x02, 0x01, 0x00, 0x01, 0x00, 0xF8, // ........ /* 0058 */ 0x24, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // $....... /* 0060 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // ........ /* 0068 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 // ........ }) } } }
Scope (_SB.PCI0.I2C3) { Device (VCM0) { Name (_HID, "PRP0001") // _HID: Hardware ID Name (_UID, 0x02) // _UID: Unique ID Name (_DDN, "DW9768 VCM") // _DDN: DOS Device Name Method (_STA, 0, NotSerialized) // _STA: Status { Return (0x0F) }
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings { I2cSerialBusV2 (0x000C, ControllerInitiated, 0x00061A80, AddressingMode7Bit, "\_SB.PCI0.I2C3", 0x00, ResourceConsumer, , Exclusive, ) }) Name (CAMD, 0x03) Name (_PR0, Package (0x01) // _PR0: Power Resources for D0 { _SB.PCI0.I2C3.CAM1.PRIC }) Name (_DSD, Package (0x02) // _DSD: Device-Specific Data { ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */, Package (0x01) { Package (0x02) { "compatible", "dongwoon,dw9768" } } }) } }
Scope (_SB.PCI0.I2C3) { Device (NVM0) { Name (_HID, "PRP0001") // _HID: Hardware ID Name (_UID, One) // _UID: Unique ID Name (_DDN, "AT24 EEPROM") // _DDN: DOS Device Name Method (_STA, 0, NotSerialized) // _STA: Status { Return (0x0F) }
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings { I2cSerialBusV2 (0x0058, ControllerInitiated, 0x00061A80, AddressingMode7Bit, "\_SB.PCI0.I2C3", 0x00, ResourceConsumer, , Exclusive, ) }) Name (_PR0, Package (0x01) // _PR0: Power Resources for D0 { _SB.PCI0.I2C3.CAM1.PRIC }) Name (_DSD, Package (0x02) // _DSD: Device-Specific Data { ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */, Package (0x05) { Package (0x02) { "size", 0x2800 },
Package (0x02) { "pagesize", One },
Package (0x02) { "read-only", One },
Package (0x02) { "address-width", 0x0E },
Package (0x02) { "compatible", "atmel,24c1024" } } }) } }
Evan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in WDee ......................................................................
Patch Set 19:
(1 comment)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledee/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... PS19, Line 136: 15.3 on # I2C 3 As ren points out in https://buganizer.corp.google.com/issues/162024182#comment51 , isn't this backwards? This is WFC, which moved to I2C5. This CL has UFC on I2C5 instead.
Evan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in WDee ......................................................................
Patch Set 19:
(1 comment)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledee/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... PS19, Line 136: 15.3 on # I2C 3
As ren points out in https://buganizer.corp.google. […]
I notice Ren also had a hunk to light up i2c5 in general:
diff --git a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb index 363307b0db..0eb77ffd8f 100644 --- a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb @@ -45,7 +45,7 @@ chip soc/intel/jasperlake [PchSerialIoIndexI2C2] = PchSerialIoPci, [PchSerialIoIndexI2C3] = PchSerialIoPci, [PchSerialIoIndexI2C4] = PchSerialIoPci, - [PchSerialIoIndexI2C5] = PchSerialIoDisabled, + [PchSerialIoIndexI2C5] = PchSerialIoPci, }"
register "SerialIoGSpiMode" = "{
Evan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in WDee ......................................................................
Patch Set 19:
(1 comment)
Patch Set 19:
(1 comment)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledee/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... PS19, Line 136: 15.3 on # I2C 3
I notice Ren also had a hunk to light up i2c5 in general: […]
For reference, see: https://buganizer.corp.google.com/issues/162024182#comment53
Tomasz Figa has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in WDee ......................................................................
Patch Set 19:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledee/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... PS19, Line 197: register "pr0" = ""\_SB.PCI0.I2C3.CAM1.PRIC""
Attaching the SSDT CAM related entries from WDoo, I donot have the WDee board right now […]
Thanks. So it looks like both the VCM and EEPROM are tied to the sensor power sequence, but that shouldn't be the case. It should be possible to turn them on and off separately without fully powering on the others.
Also, what are the _DSM and SSDB methods for?
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in WDee ......................................................................
Patch Set 19:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledee/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... PS19, Line 184: ""\_SB.PCI0.I2C5.CAM1.PRIC"" Why VCM for UFC is using Power Resource for WFC? Is that expected or am I understanding it incorrectly? ASL pasted below does not match with this config.
Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in WDee ......................................................................
Patch Set 20:
(3 comments)
Patch Set 19:
(1 comment)
This change is ready for review.
My WDee with rework has went down, will verify this patch with Evan's patch and update the commit message with the test results soon.
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledee/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... PS19, Line 136: 15.3 on # I2C 3
For reference, see: https://buganizer.corp.google. […]
Ack
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... PS19, Line 184: ""\_SB.PCI0.I2C5.CAM1.PRIC""
Why VCM for UFC is using Power Resource for WFC? Is that expected or am I understanding it incorrect […]
Updated
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... PS19, Line 197: register "pr0" = ""\_SB.PCI0.I2C3.CAM1.PRIC""
Thanks. […]
if physically they share the same power source how can they be powered on/off separately? can you please elaborate. I think _DSM and SSDB method are used by the windows driver, They are generated by the SSDT logic, I'll confirm it.
Karthik Ramasubramanian has uploaded a new patch set (#21) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in WDee ......................................................................
mb/google/dedede: Add ACPI support for camera in WDee
Update overridetree of Waddledee to generate SSDT entries for Camera.
1. Add support as per the schematics 2. Add 2 Ports and 2 Endpoints 3. Add support for OVTI5675 and OVTI9734
BUG=None TEST=Build and boot waddledee to OS. Ensure that the ACPI identifiers for mipi cameras are added under the appropriate scope. Here is the output SSDT: Scope (_SB.PCI0.I2C3.MUX0.MXA1) { Device (CAM1) { PowerResource (PRIC, 0x00, 0x0000) { ... } } }
Scope (_SB.PCI0.I2C3.MUX0.MXA0) { Device (CAM0) { PowerResource (PRIC, 0x00, 0x0000) { ... } } }
Change-Id: I3f6b490832d2c34c86512384eb7470f13a212a66 Signed-off-by: Varshit B Pandya varshit.b.pandya@intel.com --- M src/mainboard/google/dedede/Kconfig.name M src/mainboard/google/dedede/variants/waddledee/overridetree.cb 2 files changed, 128 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/42205/21
Tomasz Figa has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in WDee ......................................................................
Patch Set 22:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledee/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42205/19/src/mainboard/google/deded... PS19, Line 197: register "pr0" = ""\_SB.PCI0.I2C3.CAM1.PRIC""
if physically they share the same power source how can they be powered on/off separately? can you pl […]
I believe they don't share all of the power sequencing. An EEPROM usually requires only a single voltage rail and no poweron/reset signals. Please consult with respective schematics or hw engineers.
Karthik Ramasubramanian has uploaded a new patch set (#24) to the change originally created by Varshit B Pandya. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in WDee ......................................................................
mb/google/dedede: Add ACPI support for camera in WDee
Update overridetree of Waddledee to generate SSDT entries for Camera.
1. Add support as per the schematics 2. Add 2 Ports and 2 Endpoints 3. Add support for OVTI5675 and OVTI9734
BUG=None TEST=Build and boot waddledee to OS. Ensure that the ACPI identifiers for mipi cameras are added under the appropriate scope. Here is the output SSDT: Scope (_SB.PCI0.I2C3.MUX0.MXA1) { Device (CAM1) { PowerResource (PRIC, 0x00, 0x0000) { ... } } }
Scope (_SB.PCI0.I2C3.MUX0.MXA0) { Device (CAM0) { PowerResource (PRIC, 0x00, 0x0000) { ... } } }
Change-Id: I3f6b490832d2c34c86512384eb7470f13a212a66 Signed-off-by: Varshit B Pandya varshit.b.pandya@intel.com --- M src/mainboard/google/dedede/Kconfig.name M src/mainboard/google/dedede/variants/waddledee/overridetree.cb 2 files changed, 128 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/42205/24
Varshit B Pandya has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/42205 )
Change subject: mb/google/dedede: Add ACPI support for camera in WDee ......................................................................
Abandoned
-
Andy Yeh (Code Review) -
Evan Green (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Matt Delco (Code Review)
-
Sugnan Prabhu S (Code Review)
-
Tim Wawrzynczak (Code Review)
-
Tomasz Figa (Code Review)
-
Varshit B Pandya (Code Review)