Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
soc/intel/common: Add PCIe Runtime D3 driver for ACPI
This driver is for devices attached to a PCIe root port that support Runtime D3. It creates the necessary PowerResource in the root port to provide _ON/_OFF methods for which will turn off power and clocks to the device when it is in the D3cold state.
The mainboard declares the driver in devicetree and provides the GPIOs that control power/reset for the device attached to the root port and the SRCCLK pin used for the PMC IPC mailbox to enable/disable the clock.
An additional device property is created for storage devices if it matches the PCI storage class which is used to indicate that the storage device should use D3 for power savings.
BUG=b:160996445 TEST=boot on volteer device with this driver enabled in the devicetree and disassemble the SSDT to ensure this code exists.
Signed-off-by: Duncan Laurie dlaurie@google.com Change-Id: I13e59c996b4f5e4c2657694bda9fad869b64ffde --- M src/soc/intel/common/block/pcie/Kconfig M src/soc/intel/common/block/pcie/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/Kconfig A src/soc/intel/common/block/pcie/rtd3/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/chip.h A src/soc/intel/common/block/pcie/rtd3/rtd3.c 6 files changed, 306 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/46260/1
diff --git a/src/soc/intel/common/block/pcie/Kconfig b/src/soc/intel/common/block/pcie/Kconfig index 9c42af6..25cde37 100644 --- a/src/soc/intel/common/block/pcie/Kconfig +++ b/src/soc/intel/common/block/pcie/Kconfig @@ -7,6 +7,8 @@
if SOC_INTEL_COMMON_BLOCK_PCIE
+source "src/soc/intel/common/block/pcie/*/Kconfig" + config PCIEXP_CLK_PM default y
diff --git a/src/soc/intel/common/block/pcie/Makefile.inc b/src/soc/intel/common/block/pcie/Makefile.inc index 0cded9d..e2ad685 100644 --- a/src/soc/intel/common/block/pcie/Makefile.inc +++ b/src/soc/intel/common/block/pcie/Makefile.inc @@ -1,2 +1,4 @@ +subdirs-y += ./* + ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_PCIE) += pcie.c ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_PCIE) += pcie_rp.c diff --git a/src/soc/intel/common/block/pcie/rtd3/Kconfig b/src/soc/intel/common/block/pcie/rtd3/Kconfig new file mode 100644 index 0000000..03e13da --- /dev/null +++ b/src/soc/intel/common/block/pcie/rtd3/Kconfig @@ -0,0 +1,9 @@ +config SOC_INTEL_COMMON_BLOCK_PCIE_RTD3 + bool + default n + depends on HAVE_ACPI_TABLES + select PMC_IPC_MAILBOX_ACPI_INTERFACE + help + When enabled, this driver will add support for ACPI controlled + Runtime D3 using GPIOs for power/reset control of the device + attached to a PCIe root port. diff --git a/src/soc/intel/common/block/pcie/rtd3/Makefile.inc b/src/soc/intel/common/block/pcie/rtd3/Makefile.inc new file mode 100644 index 0000000..7edea97 --- /dev/null +++ b/src/soc/intel/common/block/pcie/rtd3/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_PCIE_RTD3) += rtd3.c diff --git a/src/soc/intel/common/block/pcie/rtd3/chip.h b/src/soc/intel/common/block/pcie/rtd3/chip.h new file mode 100644 index 0000000..062ce30 --- /dev/null +++ b/src/soc/intel/common/block/pcie/rtd3/chip.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __SOC_INTEL_COMMON_BLOCK_PCIE_RTD3_CHIP_H__ +#define __SOC_INTEL_COMMON_BLOCK_PCIE_RTD3_CHIP_H__ + +#include <acpi/acpi_device.h> + +struct soc_intel_common_block_pcie_rtd3_config { + struct acpi_gpio power_gpio; /* Power GPIO for this device */ + struct acpi_gpio reset_gpio; /* Reset GPIO for this device (optional) */ + unsigned int clock_pin; /* SRCCLK assigned to this root port */ +}; + +#endif /* __SOC_INTEL_COMMON_BLOCK_PCIE_RTD3_CHIP_H__ */ diff --git a/src/soc/intel/common/block/pcie/rtd3/rtd3.c b/src/soc/intel/common/block/pcie/rtd3/rtd3.c new file mode 100644 index 0000000..0331e95 --- /dev/null +++ b/src/soc/intel/common/block/pcie/rtd3/rtd3.c @@ -0,0 +1,278 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <acpi/acpigen.h> +#include <acpi/acpi_device.h> +#include <console/console.h> +#include <device/device.h> +#include <device/pci_ids.h> +#include <device/pci_ops.h> +#include <device/pci.h> +#include <intelblocks/pmc.h> + +#include "chip.h" + +/* + * This UUID and the resulting ACPI Device Property is defined by the + * Power Management for Storage Hardware Devices: + * + * https://docs.microsoft.com/en-us/windows-hardware/design/component-guideline... + */ +#define PCIE_RTD3_STORAGE_UUID "5025030F-842F-4AB4-A561-99A5189762D0" + +/* Address for start of PCIe port control and status registers. */ +#define PCIE_PXCS_ADDRESS (CONFIG_MMCONF_BASE_ADDRESS + 0xe8000) + +#define PCH_PCIE_PXCS_CFG_LSTS 0x52 /* Link Status Register */ +#define PCH_PCIE_PXCS_CFG_SPR 0xe0 /* Scratchpad */ +#define PCH_PCIE_PXCS_CFG_RPPGEN 0xe2 /* Root Port Power Gating Enable */ +#define PCH_PCIE_CFG_LCAP_PN 0x4f /* Root Port Number */ + +/* + * Delay up to wait_ms until provided register matches expected value. + * + * Local7 = segments + * While (Local7 > 0) + * { + * If (name == val) { + * Break + * } + * Sleep (wait_ms_segment) + * Local7++ + * } + */ +static void pcie_rtd3_delay(uint32_t wait_ms, const char *name, uint64_t val) +{ + uint32_t wait_ms_segment = 1; + uint32_t segments = wait_ms; + + if (wait_ms > 32) { + wait_ms_segment = 16; + segments = wait_ms / 16; + } + + acpigen_write_store_int_to_op(segments, LOCAL7_OP); + acpigen_emit_byte(WHILE_OP); + acpigen_write_len_f(); + acpigen_emit_byte(LGREATER_OP); + acpigen_emit_byte(LOCAL7_OP); + acpigen_emit_byte(ZERO_OP); + if (name) { + acpigen_write_if_lequal_namestr_int(name, val); + acpigen_emit_byte(BREAK_OP); + acpigen_pop_len(); /* If */ + } + acpigen_write_sleep(wait_ms_segment); + acpigen_emit_byte(DECREMENT_OP); + acpigen_emit_byte(LOCAL7_OP); + acpigen_pop_len(); /* While */ +} + +/* Called from _ON to get PCIe link back to active state. */ +static void pcie_rtd3_l23_exit(void) +{ + /* Skip if port is not in L2/L3. */ + acpigen_write_if_lequal_namestr_int("NCB7", 0); + acpigen_write_return_op(ZERO_OP); + acpigen_pop_len(); + + /* Initiate L2/L3 Ready To Detect transition. */ + acpigen_write_store_int_to_namestr(1, "L23R"); + + /* Wait for transition to detect. */ + pcie_rtd3_delay(320, "L23R", 0); + + /* Once in detect, wait for link active. */ + pcie_rtd3_delay(128, "LASX", 1); + + /* Track state of port if link is active. */ + acpigen_write_if_lequal_namestr_int("LASX", 1); + acpigen_write_store_int_to_namestr(0, "NCB7"); + acpigen_pop_len(); +} + +/* Called from _OFF to put PCIe link into L2/L3 state. */ +static void pcie_rtd3_l23_entry(void) +{ + /* Initiate L2/L3 Entry request. */ + acpigen_write_store_int_to_namestr(1, "L23E"); + + /* Wait for L2/L3 Entry request to clear. */ + pcie_rtd3_delay(128, "L23E", 0); + + /* Track state of port if L2/L3 entry request is clear. */ + acpigen_write_if_lequal_namestr_int("L23E", 0); + acpigen_write_store_int_to_namestr(1, "NCB7"); + acpigen_pop_len(); +} + +static void pcie_rtd3_method_on(unsigned int pcie_rp, unsigned int clock_pin, + const struct acpi_gpio *power_gpio, + const struct acpi_gpio *reset_gpio) +{ + acpigen_write_method_serialized("_ON", 0); + + /* Assert power gpio. */ + acpigen_enable_tx_gpio(power_gpio); + + /* Enable CLKSRC for root port. */ + pmc_acpi_set_pci_clock(pcie_rp, clock_pin, true); + + /* De-assert reset gpio. */ + if (reset_gpio->pin_count) + acpigen_disable_tx_gpio(reset_gpio); + + /* Trigger L3/S3 ready exit flow. */ + pcie_rtd3_l23_exit(); + + acpigen_pop_len(); +} + +static void pcie_rtd3_method_off(int pcie_rp, int clock_pin, + const struct acpi_gpio *power_gpio, + const struct acpi_gpio *reset_gpio) +{ + acpigen_write_method_serialized("_OFF", 0); + + /* Trigger L2/L3 ready entry flow. */ + pcie_rtd3_l23_entry(); + + /* Assert reset gpio. */ + if (reset_gpio->pin_count) + acpigen_enable_tx_gpio(reset_gpio); + + /* Disable CLKSRC for this root port. */ + pmc_acpi_set_pci_clock(pcie_rp, clock_pin, false); + + /* De-assert power gpio. */ + acpigen_disable_tx_gpio(power_gpio); + + acpigen_pop_len(); +} + +static void pcie_rtd3_method_status(const struct acpi_gpio *power_gpio) +{ + acpigen_write_method("_STA", 0); + + /* Read power gpio value into Local0. */ + acpigen_get_tx_gpio(power_gpio); + + acpigen_write_if_lequal_op_op(LOCAL0_OP, ZERO_OP); + acpigen_write_return_op(ZERO_OP); + acpigen_pop_len(); + acpigen_write_else(); + acpigen_write_return_op(ONE_OP); + acpigen_pop_len(); + + acpigen_pop_len(); +} + +static void pcie_rtd3_fill_ssdt(const struct device *dev) +{ + const struct soc_intel_common_block_pcie_rtd3_config *config = config_of(dev); + static const char * const power_res_states[] = { "_PR0", "_PR3" }; + const char *scope = acpi_device_scope(dev); + const struct opregion opregion = OPREGION("PXCS", SYSTEMMEMORY, PCIE_PXCS_ADDRESS, 0xff); + const struct fieldlist fieldlist[] = { + FIELDLIST_OFFSET(PCH_PCIE_PXCS_CFG_LSTS), + FIELDLIST_RESERVED(13), + FIELDLIST_NAMESTR("LASX", 1), /* Link Active Status */ + FIELDLIST_OFFSET(PCH_PCIE_PXCS_CFG_SPR), + FIELDLIST_RESERVED(7), + FIELDLIST_NAMESTR("NCB7", 1), + FIELDLIST_OFFSET(PCH_PCIE_PXCS_CFG_RPPGEN), + FIELDLIST_RESERVED(2), + FIELDLIST_NAMESTR("L23E", 1), /* L23_Rdy Entry Request */ + FIELDLIST_NAMESTR("L23R", 1), /* L23_Rdy Detect Transition */ + }; + uint8_t pcie_rp; + + if (!scope) { + printk(BIOS_ERR, "%s: scope not found: %s\n", __func__, dev_path(dev)); + return; + } + if (!config->power_gpio.pin_count) { + printk(BIOS_ERR, "%s: power gpio not provided for %s.\n", __func__, scope); + return; + } + if (config->clock_pin > CONFIG_MAX_PCIE_CLOCKS) { + printk(BIOS_ERR, "%s: Invalid clock pin %u for %s.\n", __func__, + config->clock_pin, scope); + return; + } + + /* Read port number of root port that this device is attached to. */ + pcie_rp = pci_read_config8(dev->bus->dev, PCH_PCIE_CFG_LCAP_PN); + if (pcie_rp == 0 || pcie_rp > CONFIG_MAX_ROOT_PORTS) { + printk(BIOS_ERR, "%s: Invalid root port number: %u\n", __func__, pcie_rp); + return; + } + /* Port number is 1-based, PMC IPC method expects 0-based. */ + pcie_rp--; + + printk(BIOS_INFO, "%s: Enable RTD3 for %s\n", scope, dev_path(dev->bus->dev)); + + /* The RTD3 power resource is added to the root port, not the device. */ + acpigen_write_scope(scope); + + acpigen_write_opregion(&opregion); + acpigen_write_field("PXCS", fieldlist, ARRAY_SIZE(fieldlist), + FIELD_ANYACC | FIELD_NOLOCK | FIELD_PRESERVE); + + /* Power resources */ + acpigen_write_power_res("PRIC", 0, 0, power_res_states, ARRAY_SIZE(power_res_states)); + + pcie_rtd3_method_status(&config->power_gpio); + pcie_rtd3_method_on(pcie_rp, config->clock_pin, &config->power_gpio, + &config->reset_gpio); + pcie_rtd3_method_off(pcie_rp, config->clock_pin, &config->power_gpio, + &config->reset_gpio); + + acpigen_pop_len(); /* PowerResource */ + + /* Add property for OS to enable storage D3. */ + if ((dev->class >> 16) == PCI_BASE_CLASS_STORAGE) { + struct acpi_dp *dsd, *pkg; + + acpigen_write_device(acpi_device_name(dev)); + acpigen_write_ADR(0); + acpigen_write_STA(ACPI_STATUS_DEVICE_ALL_ON); + acpigen_write_name_integer("_S0W", 4); + + dsd = acpi_dp_new_table("_DSD"); + pkg = acpi_dp_new_table(PCIE_RTD3_STORAGE_UUID); + acpi_dp_add_integer(pkg, "StorageD3Enable", 1); + acpi_dp_add_package(dsd, pkg); + acpi_dp_write(dsd); + + acpigen_pop_len(); /* Device */ + + printk(BIOS_INFO, "%s: Added StorageD3Enable property\n", + acpi_device_path(dev)); + } + + acpigen_pop_len(); /* Scope */ +} + +static const char *pcie_rtd3_acpi_name(const struct device *dev) +{ + return "RTD3"; +} + +static struct device_operations pcie_rtd3_ops = { + .read_resources = pci_dev_read_resources, + .set_resources = pci_dev_set_resources, + .enable_resources = pci_dev_enable_resources, + .acpi_fill_ssdt = pcie_rtd3_fill_ssdt, + .acpi_name = pcie_rtd3_acpi_name, + .ops_pci = &pci_dev_ops_pci, +}; + +static void pcie_rtd3_enable(struct device *dev) +{ + dev->ops = &pcie_rtd3_ops; +} + +struct chip_operations soc_intel_common_block_pcie_rtd3_ops = { + CHIP_NAME("Intel PCIe Runtime D3") + .enable_dev = pcie_rtd3_enable +};
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46260/1/src/soc/intel/common/block/... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/1/src/soc/intel/common/block/... PS1, Line 174: const struct opregion opregion = OPREGION("PXCS", SYSTEMMEMORY, PCIE_PXCS_ADDRESS, 0xff); line over 96 characters
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46260
to look at the new patch set (#2).
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
soc/intel/common: Add PCIe Runtime D3 driver for ACPI
This driver is for devices attached to a PCIe root port that support Runtime D3. It creates the necessary PowerResource in the root port to provide _ON/_OFF methods for which will turn off power and clocks to the device when it is in the D3cold state.
The mainboard declares the driver in devicetree and provides the GPIOs that control power/reset for the device attached to the root port and the SRCCLK pin used for the PMC IPC mailbox to enable/disable the clock.
An additional device property is created for storage devices if it matches the PCI storage class which is used to indicate that the storage device should use D3 for power savings.
BUG=b:160996445 TEST=boot on volteer device with this driver enabled in the devicetree and disassemble the SSDT to ensure this code exists.
Signed-off-by: Duncan Laurie dlaurie@google.com Change-Id: I13e59c996b4f5e4c2657694bda9fad869b64ffde --- M src/soc/intel/common/block/pcie/Kconfig M src/soc/intel/common/block/pcie/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/Kconfig A src/soc/intel/common/block/pcie/rtd3/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/chip.h A src/soc/intel/common/block/pcie/rtd3/rtd3.c 6 files changed, 307 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/46260/2
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46260
to look at the new patch set (#4).
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
soc/intel/common: Add PCIe Runtime D3 driver for ACPI
This driver is for devices attached to a PCIe root port that support Runtime D3. It creates the necessary PowerResource in the root port to provide _ON/_OFF methods for which will turn off power and clocks to the device when it is in the D3cold state.
The mainboard declares the driver in devicetree and provides the GPIOs that control power/reset for the device attached to the root port and the SRCCLK pin used for the PMC IPC mailbox to enable/disable the clock.
An additional device property is created for storage devices if it matches the PCI storage class which is used to indicate that the storage device should use D3 for power savings.
BUG=b:160996445 TEST=boot on volteer device with this driver enabled in the devicetree and disassemble the SSDT to ensure this code exists.
Signed-off-by: Duncan Laurie dlaurie@google.com Change-Id: I13e59c996b4f5e4c2657694bda9fad869b64ffde --- M src/soc/intel/common/block/pcie/Kconfig M src/soc/intel/common/block/pcie/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/Kconfig A src/soc/intel/common/block/pcie/rtd3/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/chip.h A src/soc/intel/common/block/pcie/rtd3/rtd3.c 6 files changed, 307 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/46260/4
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46260
to look at the new patch set (#5).
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
soc/intel/common: Add PCIe Runtime D3 driver for ACPI
This driver is for devices attached to a PCIe root port that support Runtime D3. It creates the necessary PowerResource in the root port to provide _ON/_OFF methods for which will turn off power and clocks to the device when it is in the D3cold state.
The mainboard declares the driver in devicetree and provides the GPIOs that control power/reset for the device attached to the root port and the SRCCLK pin used for the PMC IPC mailbox to enable/disable the clock.
An additional device property is created for storage devices if it matches the PCI storage class which is used to indicate that the storage device should use D3 for power savings.
BUG=b:160996445 TEST=boot on volteer device with this driver enabled in the devicetree and disassemble the SSDT to ensure this code exists.
Signed-off-by: Duncan Laurie dlaurie@google.com Change-Id: I13e59c996b4f5e4c2657694bda9fad869b64ffde --- M src/soc/intel/common/block/pcie/Kconfig M src/soc/intel/common/block/pcie/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/Kconfig A src/soc/intel/common/block/pcie/rtd3/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/chip.h A src/soc/intel/common/block/pcie/rtd3/rtd3.c 6 files changed, 314 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/46260/5
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 10:
I need a property indicate the device is external here it seems. Since we might want RTD3 on the SD-Express port it would need both properties which means they can't come from different drivers since there can only be one _DSD.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 10:
(6 comments)
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/chip.h:
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 11: unsigned int clock_pin; /* SRCCLK assigned to this root port */ This is available in the SoC chip info, means one less place to forget something 😊
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 30: /* : * Delay up to wait_ms until provided register matches expected value. : * : * Local7 = segments : * While (Local7 > 0) : * { : * If (name == val) { : * Break : * } : * Sleep (wait_ms_segment) : * Local7++ : * } : */ : static void pcie_rtd3_acpi_delay(uint32_t wait_ms, const char *name, uint64_t val) : { : uint32_t wait_ms_segment = 1; : uint32_t segments = wait_ms; : : if (wait_ms > 32) { : wait_ms_segment = 16; : segments = wait_ms / 16; : } : : acpigen_write_store_int_to_op(segments, LOCAL7_OP); : acpigen_emit_byte(WHILE_OP); : acpigen_write_len_f(); : acpigen_emit_byte(LGREATER_OP); : acpigen_emit_byte(LOCAL7_OP); : acpigen_emit_byte(ZERO_OP); : if (name) { : acpigen_write_if_lequal_namestr_int(name, val); : acpigen_emit_byte(BREAK_OP); : acpigen_pop_len(); /* If */ : } : acpigen_write_sleep(wait_ms_segment); : acpigen_emit_byte(DECREMENT_OP); : acpigen_emit_byte(LOCAL7_OP); : acpigen_pop_len(); /* While */ : } Seems like this might be common enough to go in acipgen, wdyt?
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 76: ; /* If */
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 82: 320 I sure hope OSPM gets its own kthread........
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 90: ; /* If */
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 114: /* Assert power gpio. */ : acpigen_enable_tx_gpio(power_gpio); : : /* Enable CLKSRC for root port. */ : pmc_ipc_acpi_set_pci_clock(pcie_rp, clock_pin, true); : : /* De-assert reset gpio. */ : if (reset_gpio->pin_count) : acpigen_disable_tx_gpio(reset_gpio); It's surprising to me that there isn't some kind of minimum time in reset here (I see the ASL didn't do it though *shrug*)
Sukumar Ghorai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 10: #include <intelblocks/pmc.h> +#include <intelblocks/pmc_ipc.h>
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 10:
(3 comments)
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 30: /* : * Delay up to wait_ms until provided register matches expected value. : * : * Local7 = segments : * While (Local7 > 0) : * { : * If (name == val) { : * Break : * } : * Sleep (wait_ms_segment) : * Local7++ : * } : */ : static void pcie_rtd3_acpi_delay(uint32_t wait_ms, const char *name, uint64_t val) : { : uint32_t wait_ms_segment = 1; : uint32_t segments = wait_ms; : : if (wait_ms > 32) { : wait_ms_segment = 16; : segments = wait_ms / 16; : } : : acpigen_write_store_int_to_op(segments, LOCAL7_OP); : acpigen_emit_byte(WHILE_OP); : acpigen_write_len_f(); : acpigen_emit_byte(LGREATER_OP); : acpigen_emit_byte(LOCAL7_OP); : acpigen_emit_byte(ZERO_OP); : if (name) { : acpigen_write_if_lequal_namestr_int(name, val); : acpigen_emit_byte(BREAK_OP); : acpigen_pop_len(); /* If */ : } : acpigen_write_sleep(wait_ms_segment); : acpigen_emit_byte(DECREMENT_OP); : acpigen_emit_byte(LOCAL7_OP); : acpigen_pop_len(); /* While */ : }
Seems like this might be common enough to go in acipgen, wdyt?
Ya I can do that.
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 114: /* Assert power gpio. */ : acpigen_enable_tx_gpio(power_gpio); : : /* Enable CLKSRC for root port. */ : pmc_ipc_acpi_set_pci_clock(pcie_rp, clock_pin, true); : : /* De-assert reset gpio. */ : if (reset_gpio->pin_count) : acpigen_disable_tx_gpio(reset_gpio);
It's surprising to me that there isn't some kind of minimum time in reset here (I see the ASL didn't […]
It does seems like there might be some sequencing violations without any delays. I can add some config options to add delays.
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 266: RTD3 Looks like this has to be named PXSX for the kernel to recognize it. (that seems like a completely unnecessary implementation detail and there is no reason it couldn't look for any device name as long as it had _ADR=0)
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46260
to look at the new patch set (#11).
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
soc/intel/common: Add PCIe Runtime D3 driver for ACPI
This driver is for devices attached to a PCIe root port that support Runtime D3. It creates the necessary PowerResource in the root port to provide _ON/_OFF methods for which will turn off power and clocks to the device when it is in the D3cold state.
The mainboard declares the driver in devicetree and provides the GPIOs that control power/reset for the device attached to the root port and the SRCCLK pin used for the PMC IPC mailbox to enable/disable the clock.
An additional device property is created for storage devices if it matches the PCI storage class which is used to indicate that the storage device should use D3 for power savings.
BUG=b:160996445 TEST=boot on volteer device with this driver enabled in the devicetree and disassemble the SSDT to ensure this code exists.
Signed-off-by: Duncan Laurie dlaurie@google.com Change-Id: I13e59c996b4f5e4c2657694bda9fad869b64ffde --- M src/soc/intel/common/block/pcie/Kconfig M src/soc/intel/common/block/pcie/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/Kconfig A src/soc/intel/common/block/pcie/rtd3/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/chip.h A src/soc/intel/common/block/pcie/rtd3/rtd3.c 6 files changed, 319 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/46260/11
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/chip.h:
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 11: unsigned int clock_pin; /* SRCCLK assigned to this root port */
This is available in the SoC chip info, means one less place to forget something 😊
The problem I ran into is that the soc chip info headers are soc-specific (and not all implement the same srcclk option) and this is meant to be common code.
It might be nice if we had a common chip header for some things like PCI configuration which could be inherited by all the SOCs..
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 12:
(6 comments)
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 10: #include <intelblocks/pmc.h>
+#include <intelblocks/pmc_ipc. […]
Done
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 30: /* : * Delay up to wait_ms until provided register matches expected value. : * : * Local7 = segments : * While (Local7 > 0) : * { : * If (name == val) { : * Break : * } : * Sleep (wait_ms_segment) : * Local7++ : * } : */ : static void pcie_rtd3_acpi_delay(uint32_t wait_ms, const char *name, uint64_t val) : { : uint32_t wait_ms_segment = 1; : uint32_t segments = wait_ms; : : if (wait_ms > 32) { : wait_ms_segment = 16; : segments = wait_ms / 16; : } : : acpigen_write_store_int_to_op(segments, LOCAL7_OP); : acpigen_emit_byte(WHILE_OP); : acpigen_write_len_f(); : acpigen_emit_byte(LGREATER_OP); : acpigen_emit_byte(LOCAL7_OP); : acpigen_emit_byte(ZERO_OP); : if (name) { : acpigen_write_if_lequal_namestr_int(name, val); : acpigen_emit_byte(BREAK_OP); : acpigen_pop_len(); /* If */ : } : acpigen_write_sleep(wait_ms_segment); : acpigen_emit_byte(DECREMENT_OP); : acpigen_emit_byte(LOCAL7_OP); : acpigen_pop_len(); /* While */ : }
Ya I can do that.
Done
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 76: ;
/* If */
Done
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 90: ;
/* If */
Done
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 114: /* Assert power gpio. */ : acpigen_enable_tx_gpio(power_gpio); : : /* Enable CLKSRC for root port. */ : pmc_ipc_acpi_set_pci_clock(pcie_rp, clock_pin, true); : : /* De-assert reset gpio. */ : if (reset_gpio->pin_count) : acpigen_disable_tx_gpio(reset_gpio);
It does seems like there might be some sequencing violations without any delays. […]
Added some, still trying to debug the resume behavior...
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 266: RTD3
Looks like this has to be named PXSX for the kernel to recognize it. […]
Done
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 157: PCIE_PXCS_ADDRESS Obviously I was not thinking clearly when I did this, it should just be using PCI config space (or a calculated MCFG address) as this value is specific for RP09.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 13:
(7 comments)
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 266: RTD3
Looks like this has to be named PXSX for the kernel to recognize it. […]
agreed
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 70: odd alignment?
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 73: should reset be asserted here first? or do you think the default programmed state of the pin + toggling _ON / _OFF is enough?
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 140: acpigen_write_if_lequal_op_op(LOCAL0_OP, LOCAL1_OP); : acpigen_write_return_op(ZERO_OP); : acpigen_pop_len(); /* If */ : acpigen_write_else(); : acpigen_write_return_op(ONE_OP); : acpigen_pop_len(); /* Else */ so the kernel is treating the present bit as on/off here?
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 157: PCIE_PXCS_ADDRESS
Obviously I was not thinking clearly when I did this, it should just be using PCI config space (or a […]
I'd just use PCI_Config if possible, that's what it's for 😊
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 161: FIELDLIST_NAMESTR("LASX", 1), /* Link Active Status */ : FIELDLIST_OFFSET(PCH_PCIE_PXCS_CFG_SPR), : FIELDLIST_RESERVED(7), : FIELDLIST_NAMESTR("NCB7", 1), : FIELDLIST_OFFSET(PCH_PCIE_PXCS_CFG_RPPGEN), : FIELDLIST_RESERVED(2), : FIELDLIST_NAMESTR("L23E", 1), /* L23_Rdy Entry Request */ : FIELDLIST_NAMESTR("L23R", 1), /* L23_Rdy Detect Transition */ suggestion: macros for the field names so they can be reused above too
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 263: #if CONFIG(HAVE_ACPI_TABLES) you already have `depends on` this in the Kconfig
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/chip.h:
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 23: clock_pin suggestion: srcclk_pin ?
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 13:
(9 comments)
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/chip.h:
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 23: clock_pin
suggestion: srcclk_pin ?
Done
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 70:
odd alignment?
ya it went beyond 96 chars, such long names..
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 73:
should reset be asserted here first? or do you think the default programmed state of the pin + toggl […]
I think there is an implicit assumption that the pins are configured to retain their state in S0ix in order for this whole thing to work anyway.
I do wonder if the CLKSRC should be enabled before applying power, I followed the previous example code but I have found other examples where the clock was enabled first. (it seems to work either way...)
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 80: pmc_ipc_acpi_set_pci_clock(pcie_rp, config->clock_pin, true); I'm changing the config so if the clock is set to -1 it will not do this step because it doesn't seem to be needed for all devices and it makes it easier to reuse this code.
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 90: pcie_rtd3_acpi_l23_exit(); I'm adding a config option for the L23 entry/exit so it can be reused on devices where this isn't required.
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 140: acpigen_write_if_lequal_op_op(LOCAL0_OP, LOCAL1_OP); : acpigen_write_return_op(ZERO_OP); : acpigen_pop_len(); /* If */ : acpigen_write_else(); : acpigen_write_return_op(ONE_OP); : acpigen_pop_len(); /* Else */
so the kernel is treating the present bit as on/off here?
It should really just use the power enable to determine if the power is on. I had plumbed for the reset GPIO since the PEDET issue was preventing the power GPIO from being used properly but I should just remove it now and rely on power GPIO being present since it is 'required' for using this driver.
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 157: PCIE_PXCS_ADDRESS
I'd just use PCI_Config if possible, that's what it's for 😊
Ya that is what I went with since all the used registers are below 0xff anyway.
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 161: FIELDLIST_NAMESTR("LASX", 1), /* Link Active Status */ : FIELDLIST_OFFSET(PCH_PCIE_PXCS_CFG_SPR), : FIELDLIST_RESERVED(7), : FIELDLIST_NAMESTR("NCB7", 1), : FIELDLIST_OFFSET(PCH_PCIE_PXCS_CFG_RPPGEN), : FIELDLIST_RESERVED(2), : FIELDLIST_NAMESTR("L23E", 1), /* L23_Rdy Entry Request */ : FIELDLIST_NAMESTR("L23R", 1), /* L23_Rdy Detect Transition */
suggestion: macros for the field names so they can be reused above too
I can do that.
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 263: #if CONFIG(HAVE_ACPI_TABLES)
you already have `depends on` this in the Kconfig
Done
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46260
to look at the new patch set (#14).
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
soc/intel/common: Add PCIe Runtime D3 driver for ACPI
This driver is for devices attached to a PCIe root port that support Runtime D3. It creates the necessary PowerResource in the root port to provide _ON/_OFF methods for which will turn off power and clocks to the device when it is in the D3cold state.
The mainboard declares the driver in devicetree and provides the GPIOs that control power/reset for the device attached to the root port and the SRCCLK pin used for the PMC IPC mailbox to enable/disable the clock.
An additional device property is created for storage devices if it matches the PCI storage class which is used to indicate that the storage device should use D3 for power savings.
BUG=b:160996445 TEST=boot on volteer device with this driver enabled in the devicetree and disassemble the SSDT to ensure this code exists.
Signed-off-by: Duncan Laurie dlaurie@google.com Change-Id: I13e59c996b4f5e4c2657694bda9fad869b64ffde --- M src/soc/intel/common/block/pcie/Kconfig M src/soc/intel/common/block/pcie/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/Kconfig A src/soc/intel/common/block/pcie/rtd3/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/chip.h A src/soc/intel/common/block/pcie/rtd3/rtd3.c 6 files changed, 362 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/46260/14
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 14:
(4 comments)
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 70:
ya it went beyond 96 chars, such long names..
Ack
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 73:
I think there is an implicit assumption that the pins are configured to retain their state in S0ix i […]
Ack
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 140: acpigen_write_if_lequal_op_op(LOCAL0_OP, LOCAL1_OP); : acpigen_write_return_op(ZERO_OP); : acpigen_pop_len(); /* If */ : acpigen_write_else(); : acpigen_write_return_op(ONE_OP); : acpigen_pop_len(); /* Else */
It should really just use the power enable to determine if the power is on. […]
Actually going to keep this and make 'enable gpio' optional as well, as long as one of enable/reset is provided. This is because the enable pin for RTS5261 was repurposed so it only has reset control...
https://review.coreboot.org/c/coreboot/+/46260/13/src/soc/intel/common/block... PS13, Line 153: , "_PR3" removing _PR3 here because technically these don't support D3hot since it completely removes power.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/10/src/soc/intel/common/block... PS10, Line 266: RTD3
agreed
Ack
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 15: Code-Review+2
(6 comments)
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/chip.h:
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 33: OS to apply security restrictions. Are these security restrictions specific to D3?
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 44: Disable the ACPI-driven L23 Under what conditions would a mainboard want to disable this?
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 148: config->enable_gpio.pin_count Isn't enable gpio mandatory as per https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block...
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 199: !config->enable_gpio.pin_count Is it okay not to provide the enable GPIO?
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 241: PCIE_HOTPLUG_IN_D3_UUID Is this true for all PCIe devices? Or would it be only for external facing ports? Given that there is a separate property for external facing ports, I think this can be true independent of whether the port is user visible.
If we are cutting power to the device completely, do we really support Hotplug in D3? Or is this something completely different?
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 281: PXSX Is this not present in kernel v5.4 in chromium tree? I was not able to find it. Also, does this apply only to storage devices?
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 15:
(6 comments)
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/chip.h:
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 33: OS to apply security restrictions.
Are these security restrictions specific to D3?
This ties into external devices in general but there are some devices like SD-express which need both the external property and the other RTD3 properties like hotplug.
What makes it difficult to split into separate drivers is the limitation of only one _DSD in the device. Even with separate UUID for this property it needs to be under the _DSD package, and currently that means it has to be written out at the same time as any other properties.
I did write a separate 'drivers/generic/external' driver for supplying this property separately but have not uploaded it since I don't have a use case yet.
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 44: Disable the ACPI-driven L23
Under what conditions would a mainboard want to disable this?
I've been asking myself (and Intel) the same question. They have said that SD doesn't need it, but I am finding that s0ix testing is more stable if I do the full flow with L23 and clock disable. I put this in to make the driver more flexible in case we do find a situation where the GPIO controls are needed but the L23 flow is not.
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 148: config->enable_gpio.pin_count
Isn't enable gpio mandatory as per https://review.coreboot. […]
Ah I need to fix the comment in chip.h as I did end up making it optional.
On volteer the later revisions of the RTS5261 daughter board repurposed the power enable GPIO as a presence input from the SD-express device, so we don't actually have power control but we still need to be able to (pretend to?) do RTD3 on that port with just placing the device in reset.
I suspect this isn't something we would ship a product with (would probably want HW to add an enable GPIO) but it is useful for supporting the different DBs for volteer.
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 199: !config->enable_gpio.pin_count
Is it okay not to provide the enable GPIO?
Ack
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 241: PCIE_HOTPLUG_IN_D3_UUID
Is this true for all PCIe devices? Or would it be only for external facing ports? Given that there i […]
This is the property that actually tells the kernel to run the _ON/_OFF methods on a hotplug root port (or force it on the kernel command line with pcie_port_pm=force).
It seems to have originally came about with thunderbolt but seems to now extend to any RTD3 capable port.
There is the separate question of whether devices like regular (non-express) SD should be hotplug ports at all, but I found that when I did not enable the root port for hotplug with a regular GL9755 it would have issues on resume when the device lost power in D3. So the two seem to go together if we are looking to do power control for getting into deeper S0ix substates.
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 281: PXSX
Is this not present in kernel v5.4 in chromium tree? I was not able to find it. […]
It is still pending backport into 5.4, waiting for this patch to land in coreboot: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2... (not sure why though, it doesn't do anything by itself)
As for the naming, I have only seen this specific name required for NVMe but Intel does use it for devices attached to thunderbolt ports as well.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 15:
(5 comments)
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/chip.h:
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 33: OS to apply security restrictions.
This ties into external devices in general but there are some devices like SD-express which need bot […]
Ack.
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 44: Disable the ACPI-driven L23
I've been asking myself (and Intel) the same question. […]
Ack.
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 148: config->enable_gpio.pin_count
Ah I need to fix the comment in chip.h as I did end up making it optional. […]
Ack.
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 241: PCIE_HOTPLUG_IN_D3_UUID
This is the property that actually tells the kernel to run the _ON/_OFF methods on a hotplug root po […]
Ack.
https://review.coreboot.org/c/coreboot/+/46260/15/src/soc/intel/common/block... PS15, Line 281: PXSX
It is still pending backport into 5. […]
Aah I see. Sounds good.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 15: Code-Review+1
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46260
to look at the new patch set (#16).
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
soc/intel/common: Add PCIe Runtime D3 driver for ACPI
This driver is for devices attached to a PCIe root port that support Runtime D3. It creates the necessary PowerResource in the root port to provide _ON/_OFF methods for which will turn off power and clocks to the device when it is in the D3cold state.
The mainboard declares the driver in devicetree and provides the GPIOs that control power/reset for the device attached to the root port and the SRCCLK pin used for the PMC IPC mailbox to enable/disable the clock.
An additional device property is created for storage devices if it matches the PCI storage class which is used to indicate that the storage device should use D3 for power savings.
BUG=b:160996445 TEST=boot on volteer device with this driver enabled in the devicetree and disassemble the SSDT to ensure this code exists.
Signed-off-by: Duncan Laurie dlaurie@google.com Change-Id: I13e59c996b4f5e4c2657694bda9fad869b64ffde --- M src/soc/intel/common/block/pcie/Kconfig M src/soc/intel/common/block/pcie/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/Kconfig A src/soc/intel/common/block/pcie/rtd3/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/chip.h A src/soc/intel/common/block/pcie/rtd3/rtd3.c 6 files changed, 363 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/46260/16
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
Patch Set 16: Code-Review+2
Duncan Laurie has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46260 )
Change subject: soc/intel/common: Add PCIe Runtime D3 driver for ACPI ......................................................................
soc/intel/common: Add PCIe Runtime D3 driver for ACPI
This driver is for devices attached to a PCIe root port that support Runtime D3. It creates the necessary PowerResource in the root port to provide _ON/_OFF methods for which will turn off power and clocks to the device when it is in the D3cold state.
The mainboard declares the driver in devicetree and provides the GPIOs that control power/reset for the device attached to the root port and the SRCCLK pin used for the PMC IPC mailbox to enable/disable the clock.
An additional device property is created for storage devices if it matches the PCI storage class which is used to indicate that the storage device should use D3 for power savings.
BUG=b:160996445 TEST=boot on volteer device with this driver enabled in the devicetree and disassemble the SSDT to ensure this code exists.
Signed-off-by: Duncan Laurie dlaurie@google.com Change-Id: I13e59c996b4f5e4c2657694bda9fad869b64ffde Reviewed-on: https://review.coreboot.org/c/coreboot/+/46260 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/common/block/pcie/Kconfig M src/soc/intel/common/block/pcie/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/Kconfig A src/soc/intel/common/block/pcie/rtd3/Makefile.inc A src/soc/intel/common/block/pcie/rtd3/chip.h A src/soc/intel/common/block/pcie/rtd3/rtd3.c 6 files changed, 363 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/common/block/pcie/Kconfig b/src/soc/intel/common/block/pcie/Kconfig index 9c42af6..25cde37 100644 --- a/src/soc/intel/common/block/pcie/Kconfig +++ b/src/soc/intel/common/block/pcie/Kconfig @@ -7,6 +7,8 @@
if SOC_INTEL_COMMON_BLOCK_PCIE
+source "src/soc/intel/common/block/pcie/*/Kconfig" + config PCIEXP_CLK_PM default y
diff --git a/src/soc/intel/common/block/pcie/Makefile.inc b/src/soc/intel/common/block/pcie/Makefile.inc index 0cded9d..e2ad685 100644 --- a/src/soc/intel/common/block/pcie/Makefile.inc +++ b/src/soc/intel/common/block/pcie/Makefile.inc @@ -1,2 +1,4 @@ +subdirs-y += ./* + ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_PCIE) += pcie.c ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_PCIE) += pcie_rp.c diff --git a/src/soc/intel/common/block/pcie/rtd3/Kconfig b/src/soc/intel/common/block/pcie/rtd3/Kconfig new file mode 100644 index 0000000..7c21ce8 --- /dev/null +++ b/src/soc/intel/common/block/pcie/rtd3/Kconfig @@ -0,0 +1,9 @@ +config SOC_INTEL_COMMON_BLOCK_PCIE_RTD3 + bool + default n + depends on HAVE_ACPI_TABLES + select PMC_IPC_ACPI_INTERFACE + help + When enabled, this driver will add support for ACPI controlled + Runtime D3 using GPIOs for power/reset control of the device + attached to a PCIe root port. diff --git a/src/soc/intel/common/block/pcie/rtd3/Makefile.inc b/src/soc/intel/common/block/pcie/rtd3/Makefile.inc new file mode 100644 index 0000000..7edea97 --- /dev/null +++ b/src/soc/intel/common/block/pcie/rtd3/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_PCIE_RTD3) += rtd3.c diff --git a/src/soc/intel/common/block/pcie/rtd3/chip.h b/src/soc/intel/common/block/pcie/rtd3/chip.h new file mode 100644 index 0000000..15b8f64 --- /dev/null +++ b/src/soc/intel/common/block/pcie/rtd3/chip.h @@ -0,0 +1,50 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __SOC_INTEL_COMMON_BLOCK_PCIE_RTD3_CHIP_H__ +#define __SOC_INTEL_COMMON_BLOCK_PCIE_RTD3_CHIP_H__ + +#include <acpi/acpi_device.h> + +/* Device support at least one of enable/reset GPIO. */ +struct soc_intel_common_block_pcie_rtd3_config { + const char *desc; + + /* GPIO used to enable device. */ + struct acpi_gpio enable_gpio; + /* Delay to be inserted after device is enabled. */ + unsigned int enable_delay_ms; + /* Delay to be inserted after device is disabled. */ + unsigned int enable_off_delay_ms; + + /* GPIO used to take device out of reset or to put it into reset. */ + struct acpi_gpio reset_gpio; + /* Delay to be inserted after device is taken out of reset. */ + unsigned int reset_delay_ms; + /* Delay to be inserted after device is put into reset. */ + unsigned int reset_off_delay_ms; + + /* + * SRCCLK assigned to this root port which will be turned off via PMC IPC. + * If set to -1 then the clock will not be disabled in D3. + */ + int srcclk_pin; + + /* + * Add device property indicating the device provides an external PCI port + * for the OS to apply security restrictions. + */ + bool is_external; + + /* + * Allow a device to add the RuntimeD3Storage property even if the detected + * PCI device does not identify as storage class. + */ + bool is_storage; + + /* + * Disable the ACPI-driven L23 Ready-to-Detect transition for the root port. + */ + bool disable_l23; +}; + +#endif /* __SOC_INTEL_COMMON_BLOCK_PCIE_RTD3_CHIP_H__ */ diff --git a/src/soc/intel/common/block/pcie/rtd3/rtd3.c b/src/soc/intel/common/block/pcie/rtd3/rtd3.c new file mode 100644 index 0000000..be412e7 --- /dev/null +++ b/src/soc/intel/common/block/pcie/rtd3/rtd3.c @@ -0,0 +1,299 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <acpi/acpigen.h> +#include <acpi/acpi_device.h> +#include <console/console.h> +#include <device/device.h> +#include <device/pci_ids.h> +#include <device/pci_ops.h> +#include <device/pci.h> +#include <intelblocks/pmc.h> +#include <intelblocks/pmc_ipc.h> +#include "chip.h" + +/* + * The "ExternalFacingPort" and "HotPlugSupportInD3" properties are defined at + * https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-r... + */ +#define PCIE_EXTERNAL_PORT_UUID "EFCC06CC-73AC-4BC3-BFF0-76143807C389" +#define PCIE_EXTERNAL_PORT_PROPERTY "ExternalFacingPort" + +#define PCIE_HOTPLUG_IN_D3_UUID "6211E2C0-58A3-4AF3-90E1-927A4E0C55A4" +#define PCIE_HOTPLUG_IN_D3_PROPERTY "HotPlugSupportInD3" + +/* + * This UUID and the resulting ACPI Device Property is defined by the + * Power Management for Storage Hardware Devices: + * + * https://docs.microsoft.com/en-us/windows-hardware/design/component-guideline... + */ +#define PCIE_RTD3_STORAGE_UUID "5025030F-842F-4AB4-A561-99A5189762D0" +#define PCIE_RTD3_STORAGE_PROPERTY "StorageD3Enable" + +/* PCIe Root Port registers for link status and L23 control. */ +#define PCH_PCIE_CFG_LSTS 0x52 /* Link Status Register */ +#define PCH_PCIE_CFG_SPR 0xe0 /* Scratchpad */ +#define PCH_PCIE_CFG_RPPGEN 0xe2 /* Root Port Power Gating Enable */ +#define PCH_PCIE_CFG_LCAP_PN 0x4f /* Root Port Number */ + +/* ACPI register names corresponding to PCIe root port registers. */ +#define ACPI_REG_PCI_LINK_ACTIVE "LASX" /* Link active status */ +#define ACPI_REG_PCI_L23_RDY_ENTRY "L23E" /* L23_Rdy Entry Request */ +#define ACPI_REG_PCI_L23_RDY_DETECT "L23R" /* L23_Rdy Detect Transition */ +#define ACPI_REG_PCI_L23_SAVE_STATE "NCB7" /* Scratch bit to save L23 state */ + +/* Called from _ON to get PCIe link back to active state. */ +static void pcie_rtd3_acpi_l23_exit(void) +{ + /* Skip if port is not in L2/L3. */ + acpigen_write_if_lequal_namestr_int(ACPI_REG_PCI_L23_SAVE_STATE, 1); + + /* Initiate L2/L3 Ready To Detect transition. */ + acpigen_write_store_int_to_namestr(1, ACPI_REG_PCI_L23_RDY_DETECT); + + /* Wait for transition to detect. */ + acpigen_write_delay_until_namestr_int(320, ACPI_REG_PCI_L23_RDY_DETECT, 0); + + acpigen_write_store_int_to_namestr(0, ACPI_REG_PCI_L23_SAVE_STATE); + + /* Once in detect, wait for link active. */ + acpigen_write_delay_until_namestr_int(128, ACPI_REG_PCI_LINK_ACTIVE, 1); + + acpigen_pop_len(); /* If */ +} + +/* Called from _OFF to put PCIe link into L2/L3 state. */ +static void pcie_rtd3_acpi_l23_entry(void) +{ + /* Initiate L2/L3 Entry request. */ + acpigen_write_store_int_to_namestr(1, ACPI_REG_PCI_L23_RDY_ENTRY); + + /* Wait for L2/L3 Entry request to clear. */ + acpigen_write_delay_until_namestr_int(128, ACPI_REG_PCI_L23_RDY_ENTRY, 0); + + acpigen_write_store_int_to_namestr(1, ACPI_REG_PCI_L23_SAVE_STATE); +} + +static void +pcie_rtd3_acpi_method_on(unsigned int pcie_rp, + const struct soc_intel_common_block_pcie_rtd3_config *config) +{ + acpigen_write_method_serialized("_ON", 0); + + /* Assert enable GPIO to turn on device power. */ + if (config->enable_gpio.pin_count) { + acpigen_enable_tx_gpio(&config->enable_gpio); + if (config->enable_delay_ms) + acpigen_write_sleep(config->enable_delay_ms); + } + + /* Enable SRCCLK for root port if pin is defined. */ + if (config->srcclk_pin >= 0) + pmc_ipc_acpi_set_pci_clock(pcie_rp, config->srcclk_pin, true); + + /* De-assert reset GPIO to bring device out of reset. */ + if (config->reset_gpio.pin_count) { + acpigen_disable_tx_gpio(&config->reset_gpio); + if (config->reset_delay_ms) + acpigen_write_sleep(config->reset_delay_ms); + } + + /* Trigger L23 ready exit flow unless disabld by config. */ + if (!config->disable_l23) + pcie_rtd3_acpi_l23_exit(); + + acpigen_pop_len(); /* Method */ +} + +static void +pcie_rtd3_acpi_method_off(int pcie_rp, + const struct soc_intel_common_block_pcie_rtd3_config *config) +{ + acpigen_write_method_serialized("_OFF", 0); + + /* Trigger L23 ready entry flow unless disabled by config. */ + if (!config->disable_l23) + pcie_rtd3_acpi_l23_entry(); + + /* Assert reset GPIO to place device into reset. */ + if (config->reset_gpio.pin_count) { + acpigen_enable_tx_gpio(&config->reset_gpio); + if (config->reset_off_delay_ms) + acpigen_write_sleep(config->reset_off_delay_ms); + } + + /* Disable SRCCLK for this root port if pin is defined. */ + if (config->srcclk_pin >= 0) + pmc_ipc_acpi_set_pci_clock(pcie_rp, config->srcclk_pin, false); + + /* De-assert enable GPIO to turn off device power. */ + if (config->enable_gpio.pin_count) { + acpigen_disable_tx_gpio(&config->enable_gpio); + if (config->enable_off_delay_ms) + acpigen_write_sleep(config->enable_off_delay_ms); + } + + acpigen_pop_len(); /* Method */ +} + +static void +pcie_rtd3_acpi_method_status(int pcie_rp, + const struct soc_intel_common_block_pcie_rtd3_config *config) +{ + const struct acpi_gpio *gpio; + + acpigen_write_method("_STA", 0); + + /* Use enable GPIO for status if provided, otherwise use reset GPIO. */ + if (config->enable_gpio.pin_count) + gpio = &config->enable_gpio; + else + gpio = &config->reset_gpio; + + /* Read current GPIO value into Local0. */ + acpigen_get_tx_gpio(gpio); + + /* Ensure check works for both active low and active high GPIOs. */ + acpigen_write_store_int_to_op(gpio->active_low, LOCAL1_OP); + + acpigen_write_if_lequal_op_op(LOCAL0_OP, LOCAL1_OP); + acpigen_write_return_op(ZERO_OP); + acpigen_pop_len(); /* If */ + acpigen_write_else(); + acpigen_write_return_op(ONE_OP); + acpigen_pop_len(); /* Else */ + + acpigen_pop_len(); /* Method */ +} + +static void pcie_rtd3_acpi_fill_ssdt(const struct device *dev) +{ + const struct soc_intel_common_block_pcie_rtd3_config *config = config_of(dev); + static const char *const power_res_states[] = {"_PR0"}; + const struct device *parent = dev->bus->dev; + const char *scope = acpi_device_path(parent); + const struct opregion opregion = OPREGION("PXCS", PCI_CONFIG, 0, 0xff); + const struct fieldlist fieldlist[] = { + FIELDLIST_OFFSET(PCH_PCIE_CFG_LSTS), + FIELDLIST_RESERVED(13), + FIELDLIST_NAMESTR(ACPI_REG_PCI_LINK_ACTIVE, 1), + FIELDLIST_OFFSET(PCH_PCIE_CFG_SPR), + FIELDLIST_RESERVED(7), + FIELDLIST_NAMESTR(ACPI_REG_PCI_L23_SAVE_STATE, 1), + FIELDLIST_OFFSET(PCH_PCIE_CFG_RPPGEN), + FIELDLIST_RESERVED(2), + FIELDLIST_NAMESTR(ACPI_REG_PCI_L23_RDY_ENTRY, 1), + FIELDLIST_NAMESTR(ACPI_REG_PCI_L23_RDY_DETECT, 1), + }; + uint8_t pcie_rp; + struct acpi_dp *dsd, *pkg; + + if (!is_dev_enabled(parent)) { + printk(BIOS_ERR, "%s: root port not enabled\n", __func__); + return; + } + if (!scope) { + printk(BIOS_ERR, "%s: root port scope not found\n", __func__); + return; + } + if (!config->enable_gpio.pin_count && !config->reset_gpio.pin_count) { + printk(BIOS_ERR, "%s: Enable and/or Reset GPIO required for %s.\n", + __func__, scope); + return; + } + if (config->srcclk_pin > CONFIG_MAX_PCIE_CLOCKS) { + printk(BIOS_ERR, "%s: Invalid clock pin %u for %s.\n", __func__, + config->srcclk_pin, scope); + return; + } + + /* Read port number of root port that this device is attached to. */ + pcie_rp = pci_read_config8(parent, PCH_PCIE_CFG_LCAP_PN); + if (pcie_rp == 0 || pcie_rp > CONFIG_MAX_ROOT_PORTS) { + printk(BIOS_ERR, "%s: Invalid root port number: %u\n", __func__, pcie_rp); + return; + } + /* Port number is 1-based, PMC IPC method expects 0-based. */ + pcie_rp--; + + printk(BIOS_INFO, "%s: Enable RTD3 for %s (%s)\n", scope, dev_path(parent), + config->desc ?: dev->chip_ops->name); + + /* The RTD3 power resource is added to the root port, not the device. */ + acpigen_write_scope(scope); + + if (config->desc) + acpigen_write_name_string("_DDN", config->desc); + + acpigen_write_opregion(&opregion); + acpigen_write_field("PXCS", fieldlist, ARRAY_SIZE(fieldlist), + FIELD_ANYACC | FIELD_NOLOCK | FIELD_PRESERVE); + + /* ACPI Power Resource for controlling the attached device power. */ + acpigen_write_power_res("RTD3", 0, 0, power_res_states, ARRAY_SIZE(power_res_states)); + pcie_rtd3_acpi_method_status(pcie_rp, config); + pcie_rtd3_acpi_method_on(pcie_rp, config); + pcie_rtd3_acpi_method_off(pcie_rp, config); + acpigen_pop_len(); /* PowerResource */ + + /* Indicate to the OS that device supports hotplug in D3. */ + dsd = acpi_dp_new_table("_DSD"); + pkg = acpi_dp_new_table(PCIE_HOTPLUG_IN_D3_UUID); + acpi_dp_add_integer(pkg, PCIE_HOTPLUG_IN_D3_PROPERTY, 1); + acpi_dp_add_package(dsd, pkg); + + /* Indicate to the OS if the device provides an External facing port. */ + if (config->is_external) { + pkg = acpi_dp_new_table(PCIE_EXTERNAL_PORT_UUID); + acpi_dp_add_integer(pkg, PCIE_EXTERNAL_PORT_PROPERTY, 1); + acpi_dp_add_package(dsd, pkg); + } + acpi_dp_write(dsd); + + /* + * Check the sibling device on the root port to see if it is storage class and add the + * property for the OS to enable storage D3, or allow it to be enabled by config. + */ + if (config->is_storage + || (dev->sibling && (dev->sibling->class >> 16) == PCI_BASE_CLASS_STORAGE)) { + acpigen_write_device(acpi_device_name(dev)); + acpigen_write_ADR(0); + acpigen_write_STA(ACPI_STATUS_DEVICE_ALL_ON); + acpigen_write_name_integer("_S0W", 4); + + dsd = acpi_dp_new_table("_DSD"); + pkg = acpi_dp_new_table(PCIE_RTD3_STORAGE_UUID); + acpi_dp_add_integer(pkg, PCIE_RTD3_STORAGE_PROPERTY, 1); + acpi_dp_add_package(dsd, pkg); + acpi_dp_write(dsd); + + acpigen_pop_len(); /* Device */ + + printk(BIOS_INFO, "%s: Added StorageD3Enable property\n", scope); + } + + acpigen_pop_len(); /* Scope */ +} + +static const char *pcie_rtd3_acpi_name(const struct device *dev) +{ + /* Attached device name must be "PXSX" for the Linux Kernel to recognize it. */ + return "PXSX"; +} + +static struct device_operations pcie_rtd3_ops = { + .read_resources = noop_read_resources, + .set_resources = noop_set_resources, + .acpi_fill_ssdt = pcie_rtd3_acpi_fill_ssdt, + .acpi_name = pcie_rtd3_acpi_name, +}; + +static void pcie_rtd3_acpi_enable(struct device *dev) +{ + dev->ops = &pcie_rtd3_ops; +} + +struct chip_operations soc_intel_common_block_pcie_rtd3_ops = { + CHIP_NAME("Intel PCIe Runtime D3") + .enable_dev = pcie_rtd3_acpi_enable +};