Varshit B Pandya has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/intel/dedede: Add camera support for WDoo ......................................................................
mb/intel/dedede: Add camera support for WDoo
Add support as per the schmatics Add 2 Ports and 2 Endpoints Add support for OTVI8856 and OTVI5676 Add ON and OFF logic as Power Rails are same for both sensor
Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com Change-Id: Ic8687bce4896d9fc17b2190b8d11618af3515cc1 --- M src/mainboard/google/dedede/Kconfig M src/mainboard/google/dedede/dsdt.asl M src/mainboard/google/dedede/variants/baseboard/gpio.c A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl A src/mainboard/google/dedede/variants/baseboard/include/waddledoo/camera.asl 9 files changed, 601 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/39360/1
diff --git a/src/mainboard/google/dedede/Kconfig b/src/mainboard/google/dedede/Kconfig index 068650b..880fe6a 100644 --- a/src/mainboard/google/dedede/Kconfig +++ b/src/mainboard/google/dedede/Kconfig @@ -62,6 +62,12 @@ string default "variants/$(CONFIG_VARIANT_DIR)/overridetree.cb" if !BOARD_GOOGLE_DEDEDE
+# Select this option to enable camera ACPI support on the variant. +config VARIANT_HAS_CAMERA_ACPI + bool + default n if !BOARD_GOOGLE_WADDLEDOO + default y if BOARD_GOOGLE_WADDLEDOO + config TPM_TIS_ACPI_INTERRUPT int default 4 # GPE0_DW0_4 (GPP_B4) diff --git a/src/mainboard/google/dedede/dsdt.asl b/src/mainboard/google/dedede/dsdt.asl index 45a1486..3281cad 100644 --- a/src/mainboard/google/dedede/dsdt.asl +++ b/src/mainboard/google/dedede/dsdt.asl @@ -35,6 +35,11 @@ } }
+ #if CONFIG(VARIANT_HAS_CAMERA_ACPI) + /* Camera */ + #include <variants/acpi/camera.asl> + #endif + /* Chrome OS specific */ #include <vendorcode/google/chromeos/acpi/chromeos.asl>
diff --git a/src/mainboard/google/dedede/variants/baseboard/gpio.c b/src/mainboard/google/dedede/variants/baseboard/gpio.c index 0831649..13b6c0c 100644 --- a/src/mainboard/google/dedede/variants/baseboard/gpio.c +++ b/src/mainboard/google/dedede/variants/baseboard/gpio.c @@ -95,6 +95,14 @@ /* C23 : UART2_CTS_N */ PAD_NC(GPP_C23, DN_20K),
+ /* D12 : WCAM_RST_L */ + PAD_CFG_GPO(GPP_D12, 0, PLTRST), + /* D13 : EN_PP2800_CAMERA */ + PAD_CFG_GPO(GPP_D13, 0, PLTRST), + /* D14 : EN_PP1200_CAMERA */ + PAD_CFG_GPO(GPP_D14, 0, PLTRST), + /* D15 : UCAM_RST_L */ + PAD_CFG_GPO(GPP_D15, 0, PLTRST), /* D16 : HP_INT_ODL*/ PAD_CFG_GPI_INT(GPP_D16, NONE, PLTRST, EDGE_BOTH), /* D17 : EN_SPK */ @@ -102,6 +110,11 @@ /* D18 : I2S_MCLK */ PAD_CFG_NF(GPP_D18, NONE, DEEP, NF1),
+ /* E0 : CLK_24M_UCAM */ + PAD_CFG_NF(GPP_E0, NONE, PLTRST, NF2), + /* E2 : CLK_24M_WCAM */ + PAD_CFG_NF(GPP_E2, NONE, PLTRST, NF1), + /* F7 : EMMC_CMD */ PAD_CFG_NF(GPP_F7, NONE, DEEP, NF1), /* F8 : EMMC_DATA0 */ @@ -149,9 +162,9 @@ /* H5 : AP_I2C_TS_SCL */ PAD_CFG_NF(GPP_H5, NONE, DEEP, NF1), /* H6 : AP_I2C_CAM_SDA */ - PAD_CFG_NF(GPP_H6, NONE, DEEP, NF1), + PAD_CFG_NF(GPP_H6, NONE, PLTRST, NF1), /* H7 : AP_I2C_CAM_SCL */ - PAD_CFG_NF(GPP_H7, NONE, DEEP, NF1), + PAD_CFG_NF(GPP_H7, NONE, PLTRST, NF1), /* H8 : AP_I2C_AUDIO_SDA */ PAD_CFG_NF(GPP_H8, NONE, DEEP, NF1), /* H9 : AP_I2C_AUDIO_SCL */ diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl new file mode 100644 index 0000000..aa0d0d5 --- /dev/null +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl @@ -0,0 +1,179 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2020 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +scope (_SB.PCI0.I2C3) +{ + Name (STA0, Zero) + //Method to turn of Power Rails + Method (POFF, 0) + { + //Disable PP1200 lane + CTXS(GPP_D13) + //Disable PP2800 lane + CTXS(GPP_D14) + } + Method (PON, 0) + { + //Enable PP1200 lane + STXS(GPP_D13) + //Enable PP2800 lane + STXS(GPP_D14) + } + PowerResource (FCPR, 0x00, 0x0000) + { + Method (_ON, 0, Serialized) // _ON_: Power On + { + //DeAssert Reset + STXS(GPP_D15) + //Enable CLK0 + MCCT(0,1,1) // Clock 0, enable, 19.2MHz + Store(1,STA0) + //Check if another sensor is ON + IF((STA0 + STA1 > 0)) + { + //Do nothing since the other sensor is ON + } + ELSE + { + PON() + } + } + Method (_OFF, 0, Serialized) // _OFF_: Power Off + { + //Assert Reset + CTXS(GPP_D15) + Store(0,STA0) + IF((STA0 + STA1 > 0)) + { + //Do nothing since the other sensor is ON + } + ELSE + { + POFF() + } + } + Method (_STA, 0, NotSerialized) // _STA: Status + { + Return (STA0) + } + } + + Device (CAM0) + { + 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 + { + 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 + }, + } + }) + + 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) + { + 0x325AA000 + } + }, + Package (0x02) + { + "remote-endpoint", + Package (0x03) + { + IPU0, + Zero, + Zero + } + } + } + }) + } +} diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl new file mode 100644 index 0000000..22670b4 --- /dev/null +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl @@ -0,0 +1,182 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2020 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +Scope (_SB.PCI0.I2C3) +{ + Name (STA1, Zero) + PowerResource (RCPR, 0x00, 0x0000) + { + Method (_ON, 0, Serialized) // _ON_: Power On + { + //DeAssert Reset + STXS(GPP_D12) + //Enable CLK1 + MCCT(1,1,1) // Clock 0, enable, 19.2MHz + Store(1,STA1) + //Check if another sensor is ON + IF((STA0 + STA1 > 0)) + { + //Do nothing since the other sensor is ON + } + ELSE + { + PON() + } + } + Method (_OFF, 0, Serialized) // _OFF_: Power Off + { + //Assert Reset + CTXS(GPP_D12) + Store(0,STA1) + IF((STA0 + STA1 > 0)) + { + //Do nothing since the other sensor is ON + } + ELSE + { + POFF() + } + } + Method (_STA, 0, NotSerialized) // _STA: Status + { + Return (STA1) + } + } + Device(CAM1) + { + 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 + { + I2cSerialBus (0x0010, 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 (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, + 0xABA9500 + } + }, + Package (0x02) + { + "remote-endpoint", + Package (0x03) + { + IPU0, + One, + Zero + } + } + } + }) + } +} diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl new file mode 100644 index 0000000..542a274 --- /dev/null +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl @@ -0,0 +1,19 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2020 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include "ipu_mainboard.asl" +#include "ipu_endpoints.asl" +#include "cam0.asl" +#include "cam1.asl" diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl new file mode 100644 index 0000000..b30615e --- /dev/null +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl @@ -0,0 +1,97 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2020 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +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 (0x01) + { + One, + } + }, + + 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 (0x04) + { + One, + 0x02, + 0x03, + 0x04, + } + }, + + Package (0x02) + { + "remote-endpoint", + Package (0x03) + { + ^I2C3.CAM1, + Zero, + Zero + } + } + } + }) +} diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl new file mode 100644 index 0000000..da85e26 --- /dev/null +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl @@ -0,0 +1,82 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2020 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +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" + } + } + }) +} diff --git a/src/mainboard/google/dedede/variants/baseboard/include/waddledoo/camera.asl b/src/mainboard/google/dedede/variants/baseboard/include/waddledoo/camera.asl new file mode 100644 index 0000000..66a20ec --- /dev/null +++ b/src/mainboard/google/dedede/variants/baseboard/include/waddledoo/camera.asl @@ -0,0 +1,16 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2020 Intel Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <baseboard/acpi/camera.asl>
Hello Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Aamir Bohra, Divagar Mohandass,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39360
to look at the new patch set (#2).
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
mb/google/dedede: Add camera support for WDoo
Add support as per the schmatics Add 2 Ports and 2 Endpoints Add support for OTVI8856 and OTVI5676 Add ON and OFF logic as Power Rails are same for both sensor
Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com Change-Id: Ic8687bce4896d9fc17b2190b8d11618af3515cc1 --- M src/mainboard/google/dedede/Kconfig M src/mainboard/google/dedede/dsdt.asl M src/mainboard/google/dedede/variants/baseboard/gpio.c A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl A src/mainboard/google/dedede/variants/baseboard/include/waddledoo/camera.asl 9 files changed, 601 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/39360/2
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 9:
This change is ready for review.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 9:
I asked this question for volteer as well. Can we please move to SSDT generation for camera ACPI tables? I think a lot of ACPI duplication can be removed and it will make it easier to add support for new variants as well. Reference: b/146512346
Adding Shaunak as well.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 9:
(21 comments)
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/Kconfig:
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 78: # Select this option to enable camera ACPI support on the variant. Put this comment as a help string.
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 81: default n Keep it default n here. select VARIANT_HAS_ACPI_CAMERA under BOARD_GOOGLE_WADDLEDOO in Kconfig.name file.
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 173: Nit: Remove the extra space. Here and lines 175, 177, 179.
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 180: 0 Why PLTRST?
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl:
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 16: scope scope -> Scope
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 19: Method Always use a space after //. Here and everywhere. Always use period at the end of the comment.
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 19: of Nit: off
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 22: PP1200 Nit: PP2800. Here and PON
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 24: PP2800 Nit: PP1200. Here and PON
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 43: GPP_D12 Why assert WCAM RESET line for Front Camera? Shouldn't it be UCAM RESET line(GPP_D15) - here and rest of the file?
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 51: //Enable PP1200 lane : STXS(GPP_D13) : //Enable PP2800 lane : STXS(GPP_D14) Can PON method be invoked instead of duplication?
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 55: Sleep(5) Is that extra 5 ms sleep intentional?
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 72: MCOF Why can't you turn off clock 0 outside the if block? Disable the power lanes if the other sensor is already off.
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 76: //Disable PP1200 lane : CTXS(GPP_D13) : //Disable PP2800 lane : CTXS(GPP_D14) POFF Method?
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 82: } Shouldn't the UCAM reset line(GPP_D15) be deasserted after Powering off?
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 132: 0x02 0x01
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl:
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 37: //Enable PP1200 lane : STXS(GPP_D13) : //Enable PP2800 lane : STXS(GPP_D14) PON Method?
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 41: Sleep(5) Is this sleep intentional and required?
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 58: MCOF(1) // Clock 1 Same comment as in the previous file.
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 63: CTXS(GPP_D13) : //Disable PP2800 lane : CTXS(GPP_D14) POFF Method?
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 67: } No need to de-assert WCAM RESET Line (GPP_D12)?
Hello Shaunak Saha, build bot (Jenkins), Wonkyu Kim, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Aamir Bohra, Divagar Mohandass, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39360
to look at the new patch set (#10).
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
mb/google/dedede: Add camera support for WDoo
Add support as per the schmatics Add 2 Ports and 2 Endpoints Add support for OTVI8856 and OTVI5676 Add ON and OFF logic as Power Rails are same for both sensor
Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com Change-Id: Ic8687bce4896d9fc17b2190b8d11618af3515cc1 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/mainboard/google/dedede/Kconfig M src/mainboard/google/dedede/Kconfig.name M src/mainboard/google/dedede/dsdt.asl M src/mainboard/google/dedede/variants/baseboard/gpio.c A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl A src/mainboard/google/dedede/variants/waddledoo/include/variant/acpi/camera.asl 10 files changed, 623 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/39360/10
Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 10:
(21 comments)
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/Kconfig:
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 78: # Select this option to enable camera ACPI support on the variant.
Put this comment as a help string.
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 81: default n
Keep it default n here. select VARIANT_HAS_ACPI_CAMERA under BOARD_GOOGLE_WADDLEDOO in Kconfig. […]
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 173:
Nit: Remove the extra space. Here and lines 175, 177, 179.
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 180: 0
Why PLTRST?
This GPIOs are controlled by the Kernel, so kept PLTRST
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl:
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 16: scope
scope -> Scope
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 19: Method
Always use a space after //. Here and everywhere. Always use period at the end of the comment.
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 19: of
Nit: off
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 22: PP1200
Nit: PP2800. […]
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 24: PP2800
Nit: PP1200. […]
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 43: GPP_D12
Why assert WCAM RESET line for Front Camera? Shouldn't it be UCAM RESET line(GPP_D15) - here and res […]
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 51: //Enable PP1200 lane : STXS(GPP_D13) : //Enable PP2800 lane : STXS(GPP_D14)
Can PON method be invoked instead of duplication?
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 55: Sleep(5)
Is that extra 5 ms sleep intentional?
Removed
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 72: MCOF
Why can't you turn off clock 0 outside the if block? Disable the power lanes if the other sensor is […]
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 76: //Disable PP1200 lane : CTXS(GPP_D13) : //Disable PP2800 lane : CTXS(GPP_D14)
POFF Method?
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 82: }
Shouldn't the UCAM reset line(GPP_D15) be deasserted after Powering off?
Kept this outside the IF/ELSE block
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 132: 0x02
0x01
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl:
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 37: //Enable PP1200 lane : STXS(GPP_D13) : //Enable PP2800 lane : STXS(GPP_D14)
PON Method?
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 41: Sleep(5)
Is this sleep intentional and required?
Removed
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 58: MCOF(1) // Clock 1
Same comment as in the previous file.
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 63: CTXS(GPP_D13) : //Disable PP2800 lane : CTXS(GPP_D14)
POFF Method?
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 67: }
No need to de-assert WCAM RESET Line (GPP_D12)?
Done
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 10:
Add topic jsl_port just so that this CL is not forgotten.
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 10:
Please move the cam APCI devices into variants, since camera sensors used by variants could be different.
Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 10:
Patch Set 10:
Please move the cam APCI devices into variants, since camera sensors used by variants could be different.
Hello Aamir,
If the camera sensor is different in other variant then we can include the delta files in the include/variants/acpi/ folder
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 10:
Varshit, can you please rebase you changes over soc/intel/jasperlake
Hello build bot (Jenkins), Shaunak Saha, Wonkyu Kim, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Aamir Bohra, Divagar Mohandass, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39360
to look at the new patch set (#11).
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
mb/google/dedede: Add camera support for WDoo
Add support as per the schmatics Add 2 Ports and 2 Endpoints Add support for OTVI8856 and OTVI5676 Add ON and OFF logic as Power Rails are same for both sensor
Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com Change-Id: Ic8687bce4896d9fc17b2190b8d11618af3515cc1 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/mainboard/google/dedede/Kconfig M src/mainboard/google/dedede/Kconfig.name M src/mainboard/google/dedede/dsdt.asl M src/mainboard/google/dedede/variants/baseboard/gpio.c A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl A src/mainboard/google/dedede/variants/waddledoo/include/variant/acpi/camera.asl 10 files changed, 623 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/39360/11
Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 11:
Patch Set 10:
Varshit, can you please rebase you changes over soc/intel/jasperlake
done
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 11:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... File src/mainboard/google/dedede/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... PS11, Line 37: #if please indent it to left
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... PS11, Line 304: PLTRST DEEP
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... PS11, Line 306: PLTRST DEEP
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl:
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... PS11, Line 19: // Method to turn off Power Rails /* Method to turn off Power Rails */
use coreboot comment style, I see it on many places in this file as well as other files in the CL
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... PS11, Line 29: // Enable PP2800 lane. /* Enable PP2800 lane */
Hello build bot (Jenkins), Shaunak Saha, Wonkyu Kim, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Aamir Bohra, Divagar Mohandass, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39360
to look at the new patch set (#12).
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
mb/google/dedede: Add camera support for WDoo
Add support as per the schmatics Add 2 Ports and 2 Endpoints Add support for OTVI8856 and OTVI5676 Add ON and OFF logic as Power Rails are same for both sensor
Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com Change-Id: Ic8687bce4896d9fc17b2190b8d11618af3515cc1 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/mainboard/google/dedede/Kconfig M src/mainboard/google/dedede/Kconfig.name M src/mainboard/google/dedede/dsdt.asl M src/mainboard/google/dedede/variants/baseboard/gpio.c A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl A src/mainboard/google/dedede/variants/waddledoo/include/variant/acpi/camera.asl 10 files changed, 623 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/39360/12
Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 12:
Patch Set 11:
(5 comments)
Done
Hello build bot (Jenkins), Shaunak Saha, Wonkyu Kim, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Aamir Bohra, Divagar Mohandass, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39360
to look at the new patch set (#13).
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
mb/google/dedede: Add camera support for WDoo
Add support as per the schmatics Add 2 Ports and 2 Endpoints Add support for OTVI8856 and OTVI5676 Add ON and OFF logic as Power Rails are same for both sensor
Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com Change-Id: Ic8687bce4896d9fc17b2190b8d11618af3515cc1 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/mainboard/google/dedede/Kconfig M src/mainboard/google/dedede/Kconfig.name M src/mainboard/google/dedede/dsdt.asl M src/mainboard/google/dedede/variants/baseboard/gpio.c A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl A src/mainboard/google/dedede/variants/waddledoo/include/variant/acpi/camera.asl 10 files changed, 621 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/39360/13
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 13: Code-Review-1
(8 comments)
Thank you for the patch, but I am a little upset, that after two people signed off this commit, there are still coding style issues. The reviewers time is also valuable, so they should not have to be distracted with these simple issue.
https://review.coreboot.org/c/coreboot/+/39360/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39360/13//COMMIT_MSG@12 PS13, Line 12: Add ON and OFF logic as Power Rails are same for both sensor Please format it as a list.
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/Kconfig:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 85: Select this option to enable camera ACPI support on the variant Please add a dot/period at the end.
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 4: * Copyright (C) 2020 Intel Corporation. No dot/period at the end.
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 46: STXS(GPP_D15) Please use tabs. Where did you copy this from?
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 54: Sleep(5) What is the unit?
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 4: * Copyright (C) 2020 Intel Corporation. Ditto.
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 4: * Copyright (C) 2020 Intel Corporation. Ditto.
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 18: Name (EP00, Package (0x02) Tabs.
Hello build bot (Jenkins), Shaunak Saha, Wonkyu Kim, Maulik V Vaghela, Paul Menzel, Rizwan Qureshi, Subrata Banik, Aamir Bohra, Divagar Mohandass, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39360
to look at the new patch set (#14).
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
mb/google/dedede: Add camera support for WDoo
1. Add support as per the schmatics 2. Add 2 Ports and 2 Endpoints 3. Add support for OTVI8856 and OTVI5676 4. Add ON and OFF logic as Power Rails are same for both sensor
Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com Change-Id: Ic8687bce4896d9fc17b2190b8d11618af3515cc1 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/mainboard/google/dedede/Kconfig M src/mainboard/google/dedede/Kconfig.name M src/mainboard/google/dedede/dsdt.asl M src/mainboard/google/dedede/variants/baseboard/gpio.c A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl A src/mainboard/google/dedede/variants/waddledoo/include/variant/acpi/camera.asl 10 files changed, 621 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/39360/14
Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 14:
(8 comments)
https://review.coreboot.org/c/coreboot/+/39360/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39360/13//COMMIT_MSG@12 PS13, Line 12: Add ON and OFF logic as Power Rails are same for both sensor
Please format it as a list.
Updated
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/Kconfig:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 85: Select this option to enable camera ACPI support on the variant
Please add a dot/period at the end.
Updated
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 4: * Copyright (C) 2020 Intel Corporation.
No dot/period at the end.
Updated
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 46: STXS(GPP_D15)
Please use tabs. […]
Updated
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 54: Sleep(5)
What is the unit?
Updated
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 4: * Copyright (C) 2020 Intel Corporation.
Ditto.
Updated
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 4: * Copyright (C) 2020 Intel Corporation.
Ditto.
Updated
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 18: Name (EP00, Package (0x02)
Tabs.
Updated
Hello build bot (Jenkins), Shaunak Saha, Wonkyu Kim, Maulik V Vaghela, Paul Menzel, Rizwan Qureshi, Subrata Banik, Aamir Bohra, Divagar Mohandass, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39360
to look at the new patch set (#15).
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
mb/google/dedede: Add camera support for WDoo
1. Add support as per the schmatics 2. Add 2 Ports and 2 Endpoints 3. Add support for OTVI8856 and OTVI5676 4. Add ON and OFF logic as Power Rails are same for both sensor
Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com Change-Id: Ic8687bce4896d9fc17b2190b8d11618af3515cc1 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/mainboard/google/dedede/Kconfig M src/mainboard/google/dedede/Kconfig.name M src/mainboard/google/dedede/dsdt.asl M src/mainboard/google/dedede/variants/baseboard/gpio.c A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl A src/mainboard/google/dedede/variants/waddledoo/include/variant/acpi/camera.asl 10 files changed, 621 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/39360/15
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 15:
(10 comments)
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 209: /* E6 : GPP_E6/IMGCLKOUT_3 */ Configure this strap?
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 221: /* E12 : GPP_E12/IMGCLKOUT_4 */ Configure this strap?
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl:
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 1: /* : * This file is part of the coreboot project. : * : * Copyright (C) 2020 Intel Corporation : * : * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */ : There is some general clean-up of license headers going on. Please look at other files and use similar license header here and in the rest of the CL.
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 18: Name (STA0, Zero) Nit: Add an empty line between 2 entities(Name, Method, Device etc.). Here and the rest of the CL.
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 38: 0,1 Nit: space between ',' and 1.
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 39: IF(STA1) : { : /* Power signal are already ON */ : /* Assert Reset */ : CTXS(GPP_D15) : Sleep(5) /* 5 us */ : /* Deassert Reset */ : STXS(GPP_D15) : Sleep(5) /* 5 us */ : } : ELSE : { : PON() : /* Assert Reset */ : CTXS(GPP_D15) : Sleep(5) /* 5 us */ : /* Deassert Reset */ : STXS(GPP_D15) : Sleep(5) /* 5 us */ : } This can be re-flowed as
If (!STA1) { /* 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 */
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 59: Store(1,STA0) Can we not use ASL2.0? i.e. STA0 = 1
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 66: IF(STA1) : { : /* Do nothing since the other sensor is ON */ : } : ELSE : { : POFF() : } Same comment like in line 58.
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 183: } : } : } : }) : } Nit: Extra Indentation on all these lines.
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl:
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 15: Please apply all the comments as in the cam0.asl
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 15:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 209: /* E6 : GPP_E6/IMGCLKOUT_3 */
Configure this strap?
No need to configure the straps. I learnt that the hardware straps are sampled before x86 is taken out of reset. So leave it as NC.
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 221: /* E12 : GPP_E12/IMGCLKOUT_4 */
Configure this strap?
Same response as above. No need to configure the straps.
Hello build bot (Jenkins), Shaunak Saha, Wonkyu Kim, Maulik V Vaghela, Paul Menzel, Rizwan Qureshi, Subrata Banik, Aamir Bohra, Divagar Mohandass, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39360
to look at the new patch set (#16).
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
mb/google/dedede: Add camera support for WDoo
1. Add support as per the schmatics 2. Add 2 Ports and 2 Endpoints 3. Add support for OTVI8856 and OTVI5676 4. Add ON and OFF logic as Power Rails are same for both sensor
Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com Change-Id: Ic8687bce4896d9fc17b2190b8d11618af3515cc1 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/mainboard/google/dedede/Kconfig M src/mainboard/google/dedede/Kconfig.name M src/mainboard/google/dedede/dsdt.asl M src/mainboard/google/dedede/variants/baseboard/gpio.c A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl A src/mainboard/google/dedede/variants/waddledoo/include/variant/acpi/camera.asl 10 files changed, 543 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/39360/16
Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 16:
(8 comments)
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl:
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 1: /* : * This file is part of the coreboot project. : * : * Copyright (C) 2020 Intel Corporation : * : * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */ :
There is some general clean-up of license headers going on. […]
Updated.
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 18: Name (STA0, Zero)
Nit: Add an empty line between 2 entities(Name, Method, Device etc.). Here and the rest of the CL.
Updated.
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 38: 0,1
Nit: space between ',' and 1.
Updated.
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 39: IF(STA1) : { : /* Power signal are already ON */ : /* Assert Reset */ : CTXS(GPP_D15) : Sleep(5) /* 5 us */ : /* Deassert Reset */ : STXS(GPP_D15) : Sleep(5) /* 5 us */ : } : ELSE : { : PON() : /* Assert Reset */ : CTXS(GPP_D15) : Sleep(5) /* 5 us */ : /* Deassert Reset */ : STXS(GPP_D15) : Sleep(5) /* 5 us */ : }
This can be re-flowed as […]
Updated.
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 59: Store(1,STA0)
Can we not use ASL2.0? i.e. […]
Updated
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 66: IF(STA1) : { : /* Do nothing since the other sensor is ON */ : } : ELSE : { : POFF() : }
Same comment like in line 58.
Updated
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 183: } : } : } : }) : }
Nit: Extra Indentation on all these lines.
Updated.
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl:
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 15:
Please apply all the comments as in the cam0. […]
Updated
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add camera support for WDoo ......................................................................
Patch Set 16:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39360/16//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39360/16//COMMIT_MSG@7 PS16, Line 7: WDoo Please mention the full name Waddledoo somewhere.
https://review.coreboot.org/c/coreboot/+/39360/16//COMMIT_MSG@9 PS16, Line 9: schmatics schematics
https://review.coreboot.org/c/coreboot/+/39360/16//COMMIT_MSG@13 PS16, Line 13: I’d prefer if the baseboard and variant change were in separate commits. Something like:
- mb/google/dedede: Add camera support for variants
- mb/google/dedede/variants/waddledoo: Select camera support
Hello build bot (Jenkins), Shaunak Saha, Wonkyu Kim, Maulik V Vaghela, Paul Menzel, Rizwan Qureshi, Subrata Banik, Ronak Kanabar, Aamir Bohra, Divagar Mohandass, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39360
to look at the new patch set (#21).
Change subject: mb/google/dedede: Add ACPI support for camera in waddledoo ......................................................................
mb/google/dedede: Add ACPI support for camera in waddledoo
1. Add support as per the schematics 2. Add 2 Ports and 2 Endpoints 3. Add support for OTVI8856 and OTVI5676 4. Add ON and OFF logic as Power Rails are same for both sensor
Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Change-Id: Ic8687bce4896d9fc17b2190b8d11618af3515cc1 --- A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl 5 files changed, 521 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/39360/21
Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add ACPI support for camera in waddledoo ......................................................................
Patch Set 21:
(4 comments)
https://review.coreboot.org/c/coreboot/+/39360/16//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39360/16//COMMIT_MSG@7 PS16, Line 7: WDoo
Please mention the full name Waddledoo somewhere.
Updated
https://review.coreboot.org/c/coreboot/+/39360/16//COMMIT_MSG@9 PS16, Line 9: schmatics
schematics
Updated
https://review.coreboot.org/c/coreboot/+/39360/16//COMMIT_MSG@13 PS16, Line 13:
I’d prefer if the baseboard and variant change were in separate commits. Something like: […]
Updated
https://review.coreboot.org/c/coreboot/+/39360/20/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledoo/include/variant/acpi/camera.asl:
PS20:
Please add that to the variant commit.
Updated
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add ACPI support for camera in waddledoo ......................................................................
Patch Set 21:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39360/21//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39360/21//COMMIT_MSG@7 PS21, Line 7: waddledoo Now that this is enabled for waddledoo in the follow-up CL, remove reference to Waddledoo. Just mention as Add ACPI support for camera.
https://review.coreboot.org/c/coreboot/+/39360/21//COMMIT_MSG@13 PS21, Line 13: Include Bug & Test information.
https://review.coreboot.org/c/coreboot/+/39360/21//COMMIT_MSG@13 PS21, Line 13: : Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com : Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Nit: Move this line below Change-Id line.
Hello build bot (Jenkins), Shaunak Saha, Wonkyu Kim, Maulik V Vaghela, Paul Menzel, Rizwan Qureshi, Subrata Banik, Ronak Kanabar, Aamir Bohra, Divagar Mohandass, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39360
to look at the new patch set (#22).
Change subject: mb/google/dedede: Add ACPI support for camera ......................................................................
mb/google/dedede: Add ACPI support for camera
1. Add support as per the schematics 2. Add 2 Ports and 2 Endpoints 3. Add support for OTVI8856 and OTVI5676 4. Add ON and OFF logic as Power Rails are same for both sensor
BUG=None BRANCH=None TEST=Build and Boot waddledoo board and able to capture image using world facing camera.
Change-Id: Ic8687bce4896d9fc17b2190b8d11618af3515cc1 Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl 5 files changed, 521 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/39360/22
Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add ACPI support for camera ......................................................................
Patch Set 22:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39360/21//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39360/21//COMMIT_MSG@7 PS21, Line 7: waddledoo
Now that this is enabled for waddledoo in the follow-up CL, remove reference to Waddledoo. […]
Updated.
https://review.coreboot.org/c/coreboot/+/39360/21//COMMIT_MSG@13 PS21, Line 13:
Include Bug & Test information.
Updated.
https://review.coreboot.org/c/coreboot/+/39360/21//COMMIT_MSG@13 PS21, Line 13: : Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com : Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com
Nit: Move this line below Change-Id line.
Updated.
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add ACPI support for camera ......................................................................
Patch Set 22: Code-Review+1
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add ACPI support for camera ......................................................................
Patch Set 22:
Varshit, can you please mark the resolved comments.
Varshit B Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add ACPI support for camera ......................................................................
Patch Set 22:
(51 comments)
Marked the resolved comments as resolved.
https://review.coreboot.org/c/coreboot/+/39360/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39360/13//COMMIT_MSG@12 PS13, Line 12: Add ON and OFF logic as Power Rails are same for both sensor
Updated
Done
https://review.coreboot.org/c/coreboot/+/39360/16//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39360/16//COMMIT_MSG@7 PS16, Line 7: WDoo
Updated
Done
https://review.coreboot.org/c/coreboot/+/39360/16//COMMIT_MSG@9 PS16, Line 9: schmatics
Updated
Done
https://review.coreboot.org/c/coreboot/+/39360/16//COMMIT_MSG@13 PS16, Line 13:
Updated
Done
https://review.coreboot.org/c/coreboot/+/39360/21//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39360/21//COMMIT_MSG@7 PS21, Line 7: waddledoo
Updated.
Done
https://review.coreboot.org/c/coreboot/+/39360/21//COMMIT_MSG@13 PS21, Line 13:
Updated.
Done
https://review.coreboot.org/c/coreboot/+/39360/21//COMMIT_MSG@13 PS21, Line 13: : Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com : Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com
Updated.
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/Kconfig:
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 78: # Select this option to enable camera ACPI support on the variant.
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 81: default n
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/Kconfig:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 85: Select this option to enable camera ACPI support on the variant
Updated
Done
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... File src/mainboard/google/dedede/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... PS11, Line 37: #if
please indent it to left
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 173:
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 180: 0
This GPIOs are controlled by the Kernel, so kept PLTRST
Done
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... PS11, Line 304: PLTRST
DEEP
Done
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... PS11, Line 306: PLTRST
DEEP
Done
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 209: /* E6 : GPP_E6/IMGCLKOUT_3 */
No need to configure the straps. […]
Done
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 221: /* E12 : GPP_E12/IMGCLKOUT_4 */
Same response as above. No need to configure the straps.
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl:
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 16: scope
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 19: Method
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 19: of
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 22: PP1200
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 24: PP2800
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 43: GPP_D12
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 51: //Enable PP1200 lane : STXS(GPP_D13) : //Enable PP2800 lane : STXS(GPP_D14)
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 55: Sleep(5)
Removed
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 72: MCOF
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 76: //Disable PP1200 lane : CTXS(GPP_D13) : //Disable PP2800 lane : CTXS(GPP_D14)
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 82: }
Kept this outside the IF/ELSE block
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 132: 0x02
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl:
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... PS11, Line 19: // Method to turn off Power Rails
/* Method to turn off Power Rails */ […]
Done
https://review.coreboot.org/c/coreboot/+/39360/11/src/mainboard/google/deded... PS11, Line 29: // Enable PP2800 lane.
/* Enable PP2800 lane */
Done
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 4: * Copyright (C) 2020 Intel Corporation.
Updated
Done
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 46: STXS(GPP_D15)
Updated
Done
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 54: Sleep(5)
Updated
Done
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl:
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 1: /* : * This file is part of the coreboot project. : * : * Copyright (C) 2020 Intel Corporation : * : * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */ :
Updated.
Done
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 18: Name (STA0, Zero)
Updated.
Done
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 38: 0,1
Updated.
Done
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 39: IF(STA1) : { : /* Power signal are already ON */ : /* Assert Reset */ : CTXS(GPP_D15) : Sleep(5) /* 5 us */ : /* Deassert Reset */ : STXS(GPP_D15) : Sleep(5) /* 5 us */ : } : ELSE : { : PON() : /* Assert Reset */ : CTXS(GPP_D15) : Sleep(5) /* 5 us */ : /* Deassert Reset */ : STXS(GPP_D15) : Sleep(5) /* 5 us */ : }
Updated.
Done
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 59: Store(1,STA0)
Updated
Done
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 66: IF(STA1) : { : /* Do nothing since the other sensor is ON */ : } : ELSE : { : POFF() : }
Updated
Done
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 183: } : } : } : }) : }
Updated.
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl:
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 37: //Enable PP1200 lane : STXS(GPP_D13) : //Enable PP2800 lane : STXS(GPP_D14)
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 41: Sleep(5)
Removed
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 58: MCOF(1) // Clock 1
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 63: CTXS(GPP_D13) : //Disable PP2800 lane : CTXS(GPP_D14)
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/9/src/mainboard/google/dedede... PS9, Line 67: }
Done
Done
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 4: * Copyright (C) 2020 Intel Corporation.
Updated
Done
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl:
https://review.coreboot.org/c/coreboot/+/39360/15/src/mainboard/google/deded... PS15, Line 15:
Updated
Done
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl:
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 4: * Copyright (C) 2020 Intel Corporation.
Updated
Done
https://review.coreboot.org/c/coreboot/+/39360/13/src/mainboard/google/deded... PS13, Line 18: Name (EP00, Package (0x02)
Updated
Done
https://review.coreboot.org/c/coreboot/+/39360/20/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledoo/include/variant/acpi/camera.asl:
PS20:
Updated
Done
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add ACPI support for camera ......................................................................
Patch Set 22: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39360 )
Change subject: mb/google/dedede: Add ACPI support for camera ......................................................................
mb/google/dedede: Add ACPI support for camera
1. Add support as per the schematics 2. Add 2 Ports and 2 Endpoints 3. Add support for OTVI8856 and OTVI5676 4. Add ON and OFF logic as Power Rails are same for both sensor
BUG=None BRANCH=None TEST=Build and Boot waddledoo board and able to capture image using world facing camera.
Change-Id: Ic8687bce4896d9fc17b2190b8d11618af3515cc1 Signed-off-by: Pandya, Varshit B varshit.b.pandya@intel.com Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39360 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aamir Bohra aamir.bohra@intel.com Reviewed-by: Karthik Ramasubramanian kramasub@google.com --- A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl A src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl 5 files changed, 521 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Aamir Bohra: Looks good to me, but someone else must approve Karthik Ramasubramanian: Looks good to me, approved
diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl new file mode 100644 index 0000000..ca40e91 --- /dev/null +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl @@ -0,0 +1,173 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* This file is part of the coreboot project. */ + +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(0, 1) /* Clock 0, 19.2MHz */ + IF(!STA1) + { + /* 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 */ + STA0 = 1 + } + + Method (_OFF, 0, Serialized) /* _OFF_: Power Off */ + { + MCOF(0) /* Clock 0 */ + /* Assert Reset */ + CTXS(GPP_D15) + 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, "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 */ + { + 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 (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) + { + 0x325AA000 + } + }, + + Package (0x02) + { + "remote-endpoint", + Package (0x03) + { + IPU0, + Zero, + Zero + } + } + } + }) + } +} diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl new file mode 100644 index 0000000..7cc9034 --- /dev/null +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl @@ -0,0 +1,176 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* This file is part of the coreboot project. */ + +Scope (_SB.PCI0.I2C3) +{ + Name (STA1, Zero) + + PowerResource (RCPR, 0x00, 0x0000) + { + Method (_ON, 0, Serialized) /* _ON_: Power On */ + { + MCON(1, 1) /* Clock 1, 19.2MHz */ + /* Check if another sensor is ON */ + IF(!STA0) + { + /* 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 */ + STA1 = 1 + } + + Method (_OFF, 0, Serialized) /* _OFF_: Power Off */ + { + MCOF(1) /* Clock 1 */ + /* Assert Reset */ + CTXS(GPP_D12) + 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, "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 */ + { + I2cSerialBus (0x0010, 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 (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, + 0xABA9500 + } + }, + + Package (0x02) + { + "remote-endpoint", + Package (0x03) + { + IPU0, + One, + Zero + } + } + } + }) + } +} diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl new file mode 100644 index 0000000..fae8e5d --- /dev/null +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/camera.asl @@ -0,0 +1,7 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* This file is part of the coreboot project. */ + +#include "ipu_mainboard.asl" +#include "ipu_endpoints.asl" +#include "cam0.asl" +#include "cam1.asl" diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl new file mode 100644 index 0000000..cff20e4 --- /dev/null +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_endpoints.asl @@ -0,0 +1,85 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* This file is part of the coreboot project. */ + +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 (0x01) + { + One, + } + }, + + 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 (0x04) + { + One, + 0x02, + 0x03, + 0x04, + } + }, + + Package (0x02) + { + "remote-endpoint", + Package (0x03) + { + ^I2C3.CAM1, + Zero, + Zero + } + } + } + }) +} diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl new file mode 100644 index 0000000..9d29499 --- /dev/null +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/ipu_mainboard.asl @@ -0,0 +1,80 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* This file is part of the coreboot project. */ + +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" + } + } + }) +}
-
Aamir Bohra (Code Review) -
Furquan Shaikh (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Rizwan Qureshi (Code Review)
-
Varshit B Pandya (Code Review)