From 997e94ed949736d0ab011e8004f58cd40b122cc1 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 16 Mar 2021 16:51:40 +0100 Subject: [PATCH] PCI: PM: Do not read power state in pci_enable_device_flags() It should not be necessary to update the current_state field of struct pci_dev in pci_enable_device_flags() before calling do_pci_enable_device() for the device, because none of the code between that point and the pci_set_power_state() call in do_pci_enable_device() invoked later depends on it. Moreover, doing that is actively harmful in some cases. For example, if the given PCI device depends on an ACPI power resource whose _STA method initially returns 0 ("off"), but the config space of the PCI device is accessible and the power state retrieved from the PCI_PM_CTRL register is D0, the current_state field in the struct pci_dev representing that device will get out of sync with the power.state of its ACPI companion object and that will lead to power management issues going forward. To avoid such issues it is better to leave the current_state value as is until it is changed to PCI_D0 by do_pci_enable_device() as appropriate. However, the power state of the device is not changed to PCI_D0 if it is already enabled when pci_enable_device_flags() gets called for it, so update its current_state in that case, but use pci_update_current_state() covering platform PM too for that. Link: https://lore.kernel.org/lkml/20210314000439.3138941-1-luzmaximilian@gmail.com/ Reported-by: Maximilian Luz Tested-by: Maximilian Luz Signed-off-by: Rafael J. Wysocki Reviewed-by: Mika Westerberg Patchset: surface-hotplug --- drivers/pci/pci.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 9449dfde2841..5ddc27d9a275 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1870,20 +1870,10 @@ static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags) int err; int i, bars = 0; - /* - * Power state could be unknown at this point, either due to a fresh - * boot or a device removal call. So get the current power state - * so that things like MSI message writing will behave as expected - * (e.g. if the device really is in D0 at enable time). - */ - if (dev->pm_cap) { - u16 pmcsr; - pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); - dev->current_state = (pmcsr & PCI_PM_CTRL_STATE_MASK); - } - - if (atomic_inc_return(&dev->enable_cnt) > 1) + if (atomic_inc_return(&dev->enable_cnt) > 1) { + pci_update_current_state(dev, dev->current_state); return 0; /* already enabled */ + } bridge = pci_upstream_bridge(dev); if (bridge) -- 2.31.1 From 50ccd79055ad31b5edbac55a4be3b784eec0d3b8 Mon Sep 17 00:00:00 2001 From: Maximilian Luz Date: Fri, 5 Feb 2021 02:26:57 +0100 Subject: [PATCH] platform/surface: Add Surface Hot-Plug driver Some Surface Book 2 and 3 models have a discrete GPU (dGPU) that is hot-pluggable. On those devices, the dGPU is contained in the base, which can be separated from the tablet part (containing CPU and touchscreen) while the device is running. It (in general) is presented as/behaves like a standard PCIe hot-plug capable device, however, this device can also be put into D3cold. In D3cold, the device itself is turned off and can thus not submit any standard PCIe hot-plug events. To properly detect hot-(un)plugging while the dGPU is in D3cold, out-of-band signaling is required. Without this, the device state will only get updated during the next bus-check, eg. via a manually issued lspci call. This commit adds a driver to handle out-of-band PCIe hot-(un)plug events on Microsoft Surface devices. On those devices, said events can be detected via GPIO interrupts, which are then forwarded to the corresponding ACPI DSM calls by this driver. The DSM then takes care of issuing the appropriate bus-/device-check, causing the PCI core to properly pick up the device change. Signed-off-by: Maximilian Luz Link: https://lore.kernel.org/r/20210205012657.1951753-1-luzmaximilian@gmail.com Signed-off-by: Hans de Goede Patchset: surface-hotplug --- MAINTAINERS | 6 + drivers/platform/surface/Kconfig | 19 ++ drivers/platform/surface/Makefile | 1 + drivers/platform/surface/surface_hotplug.c | 282 +++++++++++++++++++++ 4 files changed, 308 insertions(+) create mode 100644 drivers/platform/surface/surface_hotplug.c diff --git a/MAINTAINERS b/MAINTAINERS index 3d57740df499..d1b36e222cd1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11823,6 +11823,12 @@ L: platform-driver-x86@vger.kernel.org S: Maintained F: drivers/hid/surface-hid/ +MICROSOFT SURFACE HOT-PLUG DRIVER +M: Maximilian Luz +L: platform-driver-x86@vger.kernel.org +S: Maintained +F: drivers/platform/surface/surface_hotplug.c + MICROSOFT SURFACE PRO 3 BUTTON DRIVER M: Chen Yu L: platform-driver-x86@vger.kernel.org diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig index 3ceeb316d56e..2784a480f310 100644 --- a/drivers/platform/surface/Kconfig +++ b/drivers/platform/surface/Kconfig @@ -140,6 +140,25 @@ config SURFACE_GPE accordingly. It is required on those devices to allow wake-ups from suspend by opening the lid. +config SURFACE_HOTPLUG + tristate "Surface Hot-Plug Driver" + depends on GPIOLIB + help + Driver for out-of-band hot-plug event signaling on Microsoft Surface + devices with hot-pluggable PCIe cards. + + This driver is used on Surface Book (2 and 3) devices with a + hot-pluggable discrete GPU (dGPU). When not in use, the dGPU on those + devices can enter D3cold, which prevents in-band (standard) PCIe + hot-plug signaling. Thus, without this driver, detaching the base + containing the dGPU will not correctly update the state of the + corresponding PCIe device if it is in D3cold. This driver adds support + for out-of-band hot-plug notifications, ensuring that the device state + is properly updated even when the device in question is in D3cold. + + Select M or Y here, if you want to (fully) support hot-plugging of + dGPU devices on the Surface Book 2 and/or 3 during D3cold. + config SURFACE_PERFMODE tristate "Surface Performance-Mode Driver" depends on SURFACE_AGGREGATOR_BUS diff --git a/drivers/platform/surface/Makefile b/drivers/platform/surface/Makefile index 31098983decc..a103fdb3ef1a 100644 --- a/drivers/platform/surface/Makefile +++ b/drivers/platform/surface/Makefile @@ -14,5 +14,6 @@ obj-$(CONFIG_SURFACE_AGGREGATOR_REGISTRY) += surface_aggregator_registry.o obj-$(CONFIG_SURFACE_BOOK1_DGPU_SWITCH) += surfacebook1_dgpu_switch.o obj-$(CONFIG_SURFACE_DTX) += surface_dtx.o obj-$(CONFIG_SURFACE_GPE) += surface_gpe.o +obj-$(CONFIG_SURFACE_HOTPLUG) += surface_hotplug.o obj-$(CONFIG_SURFACE_PERFMODE) += surface_perfmode.o obj-$(CONFIG_SURFACE_PRO3_BUTTON) += surfacepro3_button.o diff --git a/drivers/platform/surface/surface_hotplug.c b/drivers/platform/surface/surface_hotplug.c new file mode 100644 index 000000000000..cfcc15cfbacb --- /dev/null +++ b/drivers/platform/surface/surface_hotplug.c @@ -0,0 +1,282 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Surface Book (2 and later) hot-plug driver. + * + * Surface Book devices (can) have a hot-pluggable discrete GPU (dGPU). This + * driver is responsible for out-of-band hot-plug event signaling on these + * devices. It is specifically required when the hot-plug device is in D3cold + * and can thus not generate PCIe hot-plug events itself. + * + * Event signaling is handled via ACPI, which will generate the appropriate + * device-check notifications to be picked up by the PCIe hot-plug driver. + * + * Copyright (C) 2019-2021 Maximilian Luz + */ + +#include +#include +#include +#include +#include +#include +#include + +static const struct acpi_gpio_params shps_base_presence_int = { 0, 0, false }; +static const struct acpi_gpio_params shps_base_presence = { 1, 0, false }; +static const struct acpi_gpio_params shps_device_power_int = { 2, 0, false }; +static const struct acpi_gpio_params shps_device_power = { 3, 0, false }; +static const struct acpi_gpio_params shps_device_presence_int = { 4, 0, false }; +static const struct acpi_gpio_params shps_device_presence = { 5, 0, false }; + +static const struct acpi_gpio_mapping shps_acpi_gpios[] = { + { "base_presence-int-gpio", &shps_base_presence_int, 1 }, + { "base_presence-gpio", &shps_base_presence, 1 }, + { "device_power-int-gpio", &shps_device_power_int, 1 }, + { "device_power-gpio", &shps_device_power, 1 }, + { "device_presence-int-gpio", &shps_device_presence_int, 1 }, + { "device_presence-gpio", &shps_device_presence, 1 }, + { }, +}; + +/* 5515a847-ed55-4b27-8352-cd320e10360a */ +static const guid_t shps_dsm_guid = + GUID_INIT(0x5515a847, 0xed55, 0x4b27, 0x83, 0x52, 0xcd, 0x32, 0x0e, 0x10, 0x36, 0x0a); + +#define SHPS_DSM_REVISION 1 + +enum shps_dsm_fn { + SHPS_DSM_FN_PCI_NUM_ENTRIES = 0x01, + SHPS_DSM_FN_PCI_GET_ENTRIES = 0x02, + SHPS_DSM_FN_IRQ_BASE_PRESENCE = 0x03, + SHPS_DSM_FN_IRQ_DEVICE_POWER = 0x04, + SHPS_DSM_FN_IRQ_DEVICE_PRESENCE = 0x05, +}; + +enum shps_irq_type { + /* NOTE: Must be in order of enum shps_dsm_fn above. */ + SHPS_IRQ_TYPE_BASE_PRESENCE = 0, + SHPS_IRQ_TYPE_DEVICE_POWER = 1, + SHPS_IRQ_TYPE_DEVICE_PRESENCE = 2, + SHPS_NUM_IRQS, +}; + +static const char *const shps_gpio_names[] = { + [SHPS_IRQ_TYPE_BASE_PRESENCE] = "base_presence", + [SHPS_IRQ_TYPE_DEVICE_POWER] = "device_power", + [SHPS_IRQ_TYPE_DEVICE_PRESENCE] = "device_presence", +}; + +struct shps_device { + struct mutex lock[SHPS_NUM_IRQS]; /* Protects update in shps_dsm_notify_irq() */ + struct gpio_desc *gpio[SHPS_NUM_IRQS]; + unsigned int irq[SHPS_NUM_IRQS]; +}; + +#define SHPS_IRQ_NOT_PRESENT ((unsigned int)-1) + +static enum shps_dsm_fn shps_dsm_fn_for_irq(enum shps_irq_type type) +{ + return SHPS_DSM_FN_IRQ_BASE_PRESENCE + type; +} + +static void shps_dsm_notify_irq(struct platform_device *pdev, enum shps_irq_type type) +{ + struct shps_device *sdev = platform_get_drvdata(pdev); + acpi_handle handle = ACPI_HANDLE(&pdev->dev); + union acpi_object *result; + union acpi_object param; + int value; + + mutex_lock(&sdev->lock[type]); + + value = gpiod_get_value_cansleep(sdev->gpio[type]); + if (value < 0) { + mutex_unlock(&sdev->lock[type]); + dev_err(&pdev->dev, "failed to get gpio: %d (irq=%d)\n", type, value); + return; + } + + dev_dbg(&pdev->dev, "IRQ notification via DSM (irq=%d, value=%d)\n", type, value); + + param.type = ACPI_TYPE_INTEGER; + param.integer.value = value; + + result = acpi_evaluate_dsm(handle, &shps_dsm_guid, SHPS_DSM_REVISION, + shps_dsm_fn_for_irq(type), ¶m); + + if (!result) { + dev_err(&pdev->dev, "IRQ notification via DSM failed (irq=%d, gpio=%d)\n", + type, value); + + } else if (result->type != ACPI_TYPE_BUFFER) { + dev_err(&pdev->dev, + "IRQ notification via DSM failed: unexpected result type (irq=%d, gpio=%d)\n", + type, value); + + } else if (result->buffer.length != 1 || result->buffer.pointer[0] != 0) { + dev_err(&pdev->dev, + "IRQ notification via DSM failed: unexpected result value (irq=%d, gpio=%d)\n", + type, value); + } + + mutex_unlock(&sdev->lock[type]); + + if (result) + ACPI_FREE(result); +} + +static irqreturn_t shps_handle_irq(int irq, void *data) +{ + struct platform_device *pdev = data; + struct shps_device *sdev = platform_get_drvdata(pdev); + int type; + + /* Figure out which IRQ we're handling. */ + for (type = 0; type < SHPS_NUM_IRQS; type++) + if (irq == sdev->irq[type]) + break; + + /* We should have found our interrupt, if not: this is a bug. */ + if (WARN(type >= SHPS_NUM_IRQS, "invalid IRQ number: %d\n", irq)) + return IRQ_HANDLED; + + /* Forward interrupt to ACPI via DSM. */ + shps_dsm_notify_irq(pdev, type); + return IRQ_HANDLED; +} + +static int shps_setup_irq(struct platform_device *pdev, enum shps_irq_type type) +{ + unsigned long flags = IRQF_ONESHOT | IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING; + struct shps_device *sdev = platform_get_drvdata(pdev); + struct gpio_desc *gpiod; + acpi_handle handle = ACPI_HANDLE(&pdev->dev); + const char *irq_name; + const int dsm = shps_dsm_fn_for_irq(type); + int status, irq; + + /* + * Only set up interrupts that we actually need: The Surface Book 3 + * does not have a DSM for base presence, so don't set up an interrupt + * for that. + */ + if (!acpi_check_dsm(handle, &shps_dsm_guid, SHPS_DSM_REVISION, BIT(dsm))) { + dev_dbg(&pdev->dev, "IRQ notification via DSM not present (irq=%d)\n", type); + return 0; + } + + gpiod = devm_gpiod_get(&pdev->dev, shps_gpio_names[type], GPIOD_ASIS); + if (IS_ERR(gpiod)) + return PTR_ERR(gpiod); + + irq = gpiod_to_irq(gpiod); + if (irq < 0) + return irq; + + irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "shps-irq-%d", type); + if (!irq_name) + return -ENOMEM; + + status = devm_request_threaded_irq(&pdev->dev, irq, NULL, shps_handle_irq, + flags, irq_name, pdev); + if (status) + return status; + + dev_dbg(&pdev->dev, "set up irq %d as type %d\n", irq, type); + + sdev->gpio[type] = gpiod; + sdev->irq[type] = irq; + + return 0; +} + +static int surface_hotplug_remove(struct platform_device *pdev) +{ + struct shps_device *sdev = platform_get_drvdata(pdev); + int i; + + /* Ensure that IRQs have been fully handled and won't trigger any more. */ + for (i = 0; i < SHPS_NUM_IRQS; i++) { + if (sdev->irq[i] != SHPS_IRQ_NOT_PRESENT) + disable_irq(sdev->irq[i]); + + mutex_destroy(&sdev->lock[i]); + } + + return 0; +} + +static int surface_hotplug_probe(struct platform_device *pdev) +{ + struct shps_device *sdev; + int status, i; + + /* + * The MSHW0153 device is also present on the Surface Laptop 3, + * however that doesn't have a hot-pluggable PCIe device. It also + * doesn't have any GPIO interrupts/pins under the MSHW0153, so filter + * it out here. + */ + if (gpiod_count(&pdev->dev, NULL) < 0) + return -ENODEV; + + status = devm_acpi_dev_add_driver_gpios(&pdev->dev, shps_acpi_gpios); + if (status) + return status; + + sdev = devm_kzalloc(&pdev->dev, sizeof(*sdev), GFP_KERNEL); + if (!sdev) + return -ENOMEM; + + platform_set_drvdata(pdev, sdev); + + /* + * Initialize IRQs so that we can safely call surface_hotplug_remove() + * on errors. + */ + for (i = 0; i < SHPS_NUM_IRQS; i++) + sdev->irq[i] = SHPS_IRQ_NOT_PRESENT; + + /* Set up IRQs. */ + for (i = 0; i < SHPS_NUM_IRQS; i++) { + mutex_init(&sdev->lock[i]); + + status = shps_setup_irq(pdev, i); + if (status) { + dev_err(&pdev->dev, "failed to set up IRQ %d: %d\n", i, status); + goto err; + } + } + + /* Ensure everything is up-to-date. */ + for (i = 0; i < SHPS_NUM_IRQS; i++) + if (sdev->irq[i] != SHPS_IRQ_NOT_PRESENT) + shps_dsm_notify_irq(pdev, i); + + return 0; + +err: + surface_hotplug_remove(pdev); + return status; +} + +static const struct acpi_device_id surface_hotplug_acpi_match[] = { + { "MSHW0153", 0 }, + { }, +}; +MODULE_DEVICE_TABLE(acpi, surface_hotplug_acpi_match); + +static struct platform_driver surface_hotplug_driver = { + .probe = surface_hotplug_probe, + .remove = surface_hotplug_remove, + .driver = { + .name = "surface_hotplug", + .acpi_match_table = surface_hotplug_acpi_match, + .probe_type = PROBE_PREFER_ASYNCHRONOUS, + }, +}; +module_platform_driver(surface_hotplug_driver); + +MODULE_AUTHOR("Maximilian Luz "); +MODULE_DESCRIPTION("Surface Hot-Plug Signaling Driver for Surface Book Devices"); +MODULE_LICENSE("GPL"); -- 2.31.1