* [PATCH v2 0/2] PCI/ACPI: Simplify PCIe _OSC feature negotiation
@ 2021-07-14 8:55 Joerg Roedel
2021-07-14 8:55 ` [PATCH v2 1/2] PCI/APCI: Move acpi_pci_osc_support() check to negotiation phase Joerg Roedel
2021-07-14 8:55 ` [PATCH v2 2/2] PCI/ACPI: Remove OSC_PCI_SUPPORT_MASKS and OSC_PCI_CONTROL_MASKS Joerg Roedel
0 siblings, 2 replies; 5+ messages in thread
From: Joerg Roedel @ 2021-07-14 8:55 UTC (permalink / raw)
To: Bjorn Helgaas, Rafael J . Wysocki, Len Brown
Cc: linux-pci, linux-acpi, linux-kernel, Joerg Roedel
From: Joerg Roedel <jroedel@suse.de>
Hi,
here is the second version of my patch(es) to simplify the _OSC
negotiation of PCIe features between Linux and the firmware.
The main part is in patch 1, which removes the _OSC call for supported
features by merging it with the actuall _OSC call to negotiate the
features with the firmware.
This allows some simplifications of the code, notably the removal of
the acpi_pci_osc_support() function and the control=NULL special
casing in the acpi_pci_query_osc() function.
Please review.
Thanks,
Joerg
Joerg Roedel (2):
PCI/APCI: Move acpi_pci_osc_support() check to negotiation phase
PCI/ACPI: Remove OSC_PCI_SUPPORT_MASKS and OSC_PCI_CONTROL_MASKS
drivers/acpi/pci_root.c | 116 ++++++++++++++++++----------------------
include/linux/acpi.h | 2 -
2 files changed, 52 insertions(+), 66 deletions(-)
base-commit: e73f0f0ee7541171d89f2e2491130c7771ba58d3
--
2.31.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] PCI/APCI: Move acpi_pci_osc_support() check to negotiation phase
2021-07-14 8:55 [PATCH v2 0/2] PCI/ACPI: Simplify PCIe _OSC feature negotiation Joerg Roedel
@ 2021-07-14 8:55 ` Joerg Roedel
2021-07-14 12:04 ` Rafael J. Wysocki
2021-07-14 8:55 ` [PATCH v2 2/2] PCI/ACPI: Remove OSC_PCI_SUPPORT_MASKS and OSC_PCI_CONTROL_MASKS Joerg Roedel
1 sibling, 1 reply; 5+ messages in thread
From: Joerg Roedel @ 2021-07-14 8:55 UTC (permalink / raw)
To: Bjorn Helgaas, Rafael J . Wysocki, Len Brown
Cc: linux-pci, linux-acpi, linux-kernel, Joerg Roedel
From: Joerg Roedel <jroedel@suse.de>
The acpi_pci_osc_support() does an _OSC query with _OSC supported set
to what the OS supports but a zero _OSC control value, only to later
claim the features Linux wants to control with an extra _OSC query.
Nothing between the two _OSC querys depends on the result of the first
one (if successfull), and if the supported query fails the control
query will fail too. So it is a good code simplification to combine
these two querys into one.
As a result the acpi_pci_osc_support() function can be removed and
acpi_pci_query_osc() be simplified because it no longer called with a
NULL pointer for *control. Also some code duplication in the existing
error paths was consolidated.
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
drivers/acpi/pci_root.c | 114 ++++++++++++++++++----------------------
1 file changed, 52 insertions(+), 62 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index d7deedf3548e..c703832b7f7f 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -201,31 +201,20 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
support &= OSC_PCI_SUPPORT_MASKS;
support |= root->osc_support_set;
+ *control &= OSC_PCI_CONTROL_MASKS;
capbuf[OSC_QUERY_DWORD] = OSC_QUERY_ENABLE;
capbuf[OSC_SUPPORT_DWORD] = support;
- if (control) {
- *control &= OSC_PCI_CONTROL_MASKS;
- capbuf[OSC_CONTROL_DWORD] = *control | root->osc_control_set;
- } else {
- /* Run _OSC query only with existing controls. */
- capbuf[OSC_CONTROL_DWORD] = root->osc_control_set;
- }
+ capbuf[OSC_CONTROL_DWORD] = *control | root->osc_control_set;
status = acpi_pci_run_osc(root->device->handle, capbuf, &result);
if (ACPI_SUCCESS(status)) {
root->osc_support_set = support;
- if (control)
- *control = result;
+ *control = result;
}
return status;
}
-static acpi_status acpi_pci_osc_support(struct acpi_pci_root *root, u32 flags)
-{
- return acpi_pci_query_osc(root, flags, NULL);
-}
-
struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
{
struct acpi_pci_root *root;
@@ -348,7 +337,8 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
* _OSC bits the BIOS has granted control of, but its contents are meaningless
* on failure.
**/
-static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
+static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32
+ *mask, u32 req, u32 support)
{
struct acpi_pci_root *root;
acpi_status status;
@@ -372,7 +362,7 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 r
/* Need to check the available controls bits before requesting them. */
while (*mask) {
- status = acpi_pci_query_osc(root, root->osc_support_set, mask);
+ status = acpi_pci_query_osc(root, support, mask);
if (ACPI_FAILURE(status))
return status;
if (ctrl == *mask)
@@ -402,7 +392,7 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 r
static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
bool is_pcie)
{
- u32 support, control, requested;
+ u32 support, control = 0, requested;
acpi_status status;
struct acpi_device *device = root->device;
acpi_handle handle = device->handle;
@@ -435,59 +425,49 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
support |= OSC_PCI_EDR_SUPPORT;
decode_osc_support(root, "OS supports", support);
- status = acpi_pci_osc_support(root, support);
- if (ACPI_FAILURE(status)) {
- *no_aspm = 1;
- /* _OSC is optional for PCI host bridges */
- if ((status == AE_NOT_FOUND) && !is_pcie)
+ if (!pcie_ports_disabled) {
+ if ((support & ACPI_PCIE_REQ_SUPPORT) != ACPI_PCIE_REQ_SUPPORT) {
+ decode_osc_support(root, "not requesting OS control; OS requires",
+ ACPI_PCIE_REQ_SUPPORT);
return;
+ }
- dev_info(&device->dev, "_OSC: platform retains control of PCIe features (%s)\n",
- acpi_format_exception(status));
- return;
- }
-
- if (pcie_ports_disabled) {
- dev_info(&device->dev, "PCIe port services disabled; not requesting _OSC control\n");
- return;
- }
-
- if ((support & ACPI_PCIE_REQ_SUPPORT) != ACPI_PCIE_REQ_SUPPORT) {
- decode_osc_support(root, "not requesting OS control; OS requires",
- ACPI_PCIE_REQ_SUPPORT);
- return;
- }
-
- control = OSC_PCI_EXPRESS_CAPABILITY_CONTROL
- | OSC_PCI_EXPRESS_PME_CONTROL;
+ control = OSC_PCI_EXPRESS_CAPABILITY_CONTROL
+ | OSC_PCI_EXPRESS_PME_CONTROL;
- if (IS_ENABLED(CONFIG_PCIEASPM))
- control |= OSC_PCI_EXPRESS_LTR_CONTROL;
+ if (IS_ENABLED(CONFIG_PCIEASPM))
+ control |= OSC_PCI_EXPRESS_LTR_CONTROL;
- if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
- control |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
+ if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
+ control |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
- if (IS_ENABLED(CONFIG_HOTPLUG_PCI_SHPC))
- control |= OSC_PCI_SHPC_NATIVE_HP_CONTROL;
+ if (IS_ENABLED(CONFIG_HOTPLUG_PCI_SHPC))
+ control |= OSC_PCI_SHPC_NATIVE_HP_CONTROL;
- if (pci_aer_available())
- control |= OSC_PCI_EXPRESS_AER_CONTROL;
+ if (pci_aer_available())
+ control |= OSC_PCI_EXPRESS_AER_CONTROL;
- /*
- * Per the Downstream Port Containment Related Enhancements ECN to
- * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5,
- * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC
- * and EDR.
- */
- if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
- control |= OSC_PCI_EXPRESS_DPC_CONTROL;
+ /*
+ * Per the Downstream Port Containment Related Enhancements ECN to
+ * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5,
+ * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC
+ * and EDR.
+ */
+ if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
+ control |= OSC_PCI_EXPRESS_DPC_CONTROL;
+ }
+ /* Need an _OSC call even with pcie_ports_disabled set */
requested = control;
status = acpi_pci_osc_control_set(handle, &control,
- OSC_PCI_EXPRESS_CAPABILITY_CONTROL);
+ OSC_PCI_EXPRESS_CAPABILITY_CONTROL,
+ support);
+
if (ACPI_SUCCESS(status)) {
- decode_osc_control(root, "OS now controls", control);
+ if (control)
+ decode_osc_control(root, "OS now controls", control);
+
if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
/*
* We have ASPM control, but the FADT indicates that
@@ -498,10 +478,8 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
*no_aspm = 1;
}
} else {
- decode_osc_control(root, "OS requested", requested);
- decode_osc_control(root, "platform willing to grant", control);
- dev_info(&device->dev, "_OSC: platform retains control of PCIe features (%s)\n",
- acpi_format_exception(status));
+ /* Platform wants to control PCIe features */
+ root->osc_support_set = 0;
/*
* We want to disable ASPM here, but aspm_disabled
@@ -511,6 +489,18 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
* root scan.
*/
*no_aspm = 1;
+
+ /* _OSC is optional for PCI host bridges */
+ if ((status == AE_NOT_FOUND) && !is_pcie)
+ return;
+
+ if (requested) {
+ decode_osc_control(root, "OS requested", requested);
+ decode_osc_control(root, "platform willing to grant", control);
+ }
+
+ dev_info(&device->dev, "_OSC: platform retains control of PCIe features (%s)\n",
+ acpi_format_exception(status));
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] PCI/ACPI: Remove OSC_PCI_SUPPORT_MASKS and OSC_PCI_CONTROL_MASKS
2021-07-14 8:55 [PATCH v2 0/2] PCI/ACPI: Simplify PCIe _OSC feature negotiation Joerg Roedel
2021-07-14 8:55 ` [PATCH v2 1/2] PCI/APCI: Move acpi_pci_osc_support() check to negotiation phase Joerg Roedel
@ 2021-07-14 8:55 ` Joerg Roedel
1 sibling, 0 replies; 5+ messages in thread
From: Joerg Roedel @ 2021-07-14 8:55 UTC (permalink / raw)
To: Bjorn Helgaas, Rafael J . Wysocki, Len Brown
Cc: linux-pci, linux-acpi, linux-kernel, Joerg Roedel, Bjorn Helgaas
From: Joerg Roedel <jroedel@suse.de>
These masks are only used internally in the PCI Host Bridge _OSC
negotiation code, which already makes sure nothing outside of these
masks is set. Remove the masks and simplify the code.
Suggested-by: Bjorn Helgaas <helgass@kernel.org>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
drivers/acpi/pci_root.c | 4 +---
include/linux/acpi.h | 2 --
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index c703832b7f7f..0d147c682929 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -199,9 +199,7 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
acpi_status status;
u32 result, capbuf[3];
- support &= OSC_PCI_SUPPORT_MASKS;
support |= root->osc_support_set;
- *control &= OSC_PCI_CONTROL_MASKS;
capbuf[OSC_QUERY_DWORD] = OSC_QUERY_ENABLE;
capbuf[OSC_SUPPORT_DWORD] = support;
@@ -347,7 +345,7 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32
if (!mask)
return AE_BAD_PARAMETER;
- ctrl = *mask & OSC_PCI_CONTROL_MASKS;
+ ctrl = *mask;
if ((ctrl & req) != req)
return AE_TYPE;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 72e4f7fd268c..c6dba5f21384 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -577,7 +577,6 @@ extern u32 osc_sb_native_usb4_control;
#define OSC_PCI_MSI_SUPPORT 0x00000010
#define OSC_PCI_EDR_SUPPORT 0x00000080
#define OSC_PCI_HPX_TYPE_3_SUPPORT 0x00000100
-#define OSC_PCI_SUPPORT_MASKS 0x0000019f
/* PCI Host Bridge _OSC: Capabilities DWORD 3: Control Field */
#define OSC_PCI_EXPRESS_NATIVE_HP_CONTROL 0x00000001
@@ -587,7 +586,6 @@ extern u32 osc_sb_native_usb4_control;
#define OSC_PCI_EXPRESS_CAPABILITY_CONTROL 0x00000010
#define OSC_PCI_EXPRESS_LTR_CONTROL 0x00000020
#define OSC_PCI_EXPRESS_DPC_CONTROL 0x00000080
-#define OSC_PCI_CONTROL_MASKS 0x000000bf
#define ACPI_GSB_ACCESS_ATTRIB_QUICK 0x00000002
#define ACPI_GSB_ACCESS_ATTRIB_SEND_RCV 0x00000004
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] PCI/APCI: Move acpi_pci_osc_support() check to negotiation phase
2021-07-14 8:55 ` [PATCH v2 1/2] PCI/APCI: Move acpi_pci_osc_support() check to negotiation phase Joerg Roedel
@ 2021-07-14 12:04 ` Rafael J. Wysocki
2021-07-15 7:34 ` Joerg Roedel
0 siblings, 1 reply; 5+ messages in thread
From: Rafael J. Wysocki @ 2021-07-14 12:04 UTC (permalink / raw)
To: Joerg Roedel
Cc: Bjorn Helgaas, Rafael J . Wysocki, Len Brown, Linux PCI,
ACPI Devel Maling List, Linux Kernel Mailing List, Joerg Roedel
On Wed, Jul 14, 2021 at 10:55 AM Joerg Roedel <joro@8bytes.org> wrote:
>
> From: Joerg Roedel <jroedel@suse.de>
>
> The acpi_pci_osc_support() does an _OSC query with _OSC supported set
> to what the OS supports but a zero _OSC control value, only to later
> claim the features Linux wants to control with an extra _OSC query.
>
> Nothing between the two _OSC querys depends on the result of the first
> one (if successfull), and if the supported query fails the control
> query will fail too. So it is a good code simplification to combine
> these two querys into one.
>
> As a result the acpi_pci_osc_support() function can be removed and
> acpi_pci_query_osc() be simplified because it no longer called with a
> NULL pointer for *control. Also some code duplication in the existing
> error paths was consolidated.
>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
> drivers/acpi/pci_root.c | 114 ++++++++++++++++++----------------------
> 1 file changed, 52 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index d7deedf3548e..c703832b7f7f 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -201,31 +201,20 @@ static acpi_status acpi_pci_query_osc(struct acpi_pci_root *root,
>
> support &= OSC_PCI_SUPPORT_MASKS;
> support |= root->osc_support_set;
> + *control &= OSC_PCI_CONTROL_MASKS;
>
> capbuf[OSC_QUERY_DWORD] = OSC_QUERY_ENABLE;
> capbuf[OSC_SUPPORT_DWORD] = support;
> - if (control) {
> - *control &= OSC_PCI_CONTROL_MASKS;
> - capbuf[OSC_CONTROL_DWORD] = *control | root->osc_control_set;
> - } else {
> - /* Run _OSC query only with existing controls. */
> - capbuf[OSC_CONTROL_DWORD] = root->osc_control_set;
> - }
> + capbuf[OSC_CONTROL_DWORD] = *control | root->osc_control_set;
>
> status = acpi_pci_run_osc(root->device->handle, capbuf, &result);
> if (ACPI_SUCCESS(status)) {
> root->osc_support_set = support;
> - if (control)
> - *control = result;
> + *control = result;
> }
> return status;
> }
>
> -static acpi_status acpi_pci_osc_support(struct acpi_pci_root *root, u32 flags)
> -{
> - return acpi_pci_query_osc(root, flags, NULL);
> -}
> -
> struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
> {
> struct acpi_pci_root *root;
> @@ -348,7 +337,8 @@ EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
> * _OSC bits the BIOS has granted control of, but its contents are meaningless
> * on failure.
> **/
> -static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req)
> +static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32
> + *mask, u32 req, u32 support)
> {
> struct acpi_pci_root *root;
> acpi_status status;
> @@ -372,7 +362,7 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 r
>
> /* Need to check the available controls bits before requesting them. */
> while (*mask) {
> - status = acpi_pci_query_osc(root, root->osc_support_set, mask);
> + status = acpi_pci_query_osc(root, support, mask);
> if (ACPI_FAILURE(status))
> return status;
> if (ctrl == *mask)
> @@ -402,7 +392,7 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 r
> static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
> bool is_pcie)
> {
> - u32 support, control, requested;
> + u32 support, control = 0, requested;
> acpi_status status;
> struct acpi_device *device = root->device;
> acpi_handle handle = device->handle;
> @@ -435,59 +425,49 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
> support |= OSC_PCI_EDR_SUPPORT;
>
> decode_osc_support(root, "OS supports", support);
> - status = acpi_pci_osc_support(root, support);
> - if (ACPI_FAILURE(status)) {
> - *no_aspm = 1;
>
> - /* _OSC is optional for PCI host bridges */
> - if ((status == AE_NOT_FOUND) && !is_pcie)
> + if (!pcie_ports_disabled) {
If pcie_ports_disabled is set, we don't want to request any control
from the platform firmware at all and, specifically, we don't want to
evaluate _OSC with the OSC_QUERY_ENABLE clear in
capbuf[OSC_QUERY_DWORD].
I'm not sure how this is achieved after your changes.
> + if ((support & ACPI_PCIE_REQ_SUPPORT) != ACPI_PCIE_REQ_SUPPORT) {
> + decode_osc_support(root, "not requesting OS control; OS requires",
> + ACPI_PCIE_REQ_SUPPORT);
> return;
> + }
>
> - dev_info(&device->dev, "_OSC: platform retains control of PCIe features (%s)\n",
> - acpi_format_exception(status));
> - return;
> - }
> -
> - if (pcie_ports_disabled) {
> - dev_info(&device->dev, "PCIe port services disabled; not requesting _OSC control\n");
> - return;
> - }
> -
> - if ((support & ACPI_PCIE_REQ_SUPPORT) != ACPI_PCIE_REQ_SUPPORT) {
> - decode_osc_support(root, "not requesting OS control; OS requires",
> - ACPI_PCIE_REQ_SUPPORT);
> - return;
> - }
> -
> - control = OSC_PCI_EXPRESS_CAPABILITY_CONTROL
> - | OSC_PCI_EXPRESS_PME_CONTROL;
> + control = OSC_PCI_EXPRESS_CAPABILITY_CONTROL
> + | OSC_PCI_EXPRESS_PME_CONTROL;
>
> - if (IS_ENABLED(CONFIG_PCIEASPM))
> - control |= OSC_PCI_EXPRESS_LTR_CONTROL;
> + if (IS_ENABLED(CONFIG_PCIEASPM))
> + control |= OSC_PCI_EXPRESS_LTR_CONTROL;
>
> - if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
> - control |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
> + if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
> + control |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
>
> - if (IS_ENABLED(CONFIG_HOTPLUG_PCI_SHPC))
> - control |= OSC_PCI_SHPC_NATIVE_HP_CONTROL;
> + if (IS_ENABLED(CONFIG_HOTPLUG_PCI_SHPC))
> + control |= OSC_PCI_SHPC_NATIVE_HP_CONTROL;
>
> - if (pci_aer_available())
> - control |= OSC_PCI_EXPRESS_AER_CONTROL;
> + if (pci_aer_available())
> + control |= OSC_PCI_EXPRESS_AER_CONTROL;
>
> - /*
> - * Per the Downstream Port Containment Related Enhancements ECN to
> - * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5,
> - * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC
> - * and EDR.
> - */
> - if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
> - control |= OSC_PCI_EXPRESS_DPC_CONTROL;
> + /*
> + * Per the Downstream Port Containment Related Enhancements ECN to
> + * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5,
> + * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC
> + * and EDR.
> + */
> + if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
> + control |= OSC_PCI_EXPRESS_DPC_CONTROL;
> + }
>
> + /* Need an _OSC call even with pcie_ports_disabled set */
> requested = control;
> status = acpi_pci_osc_control_set(handle, &control,
> - OSC_PCI_EXPRESS_CAPABILITY_CONTROL);
> + OSC_PCI_EXPRESS_CAPABILITY_CONTROL,
> + support);
> +
> if (ACPI_SUCCESS(status)) {
> - decode_osc_control(root, "OS now controls", control);
> + if (control)
> + decode_osc_control(root, "OS now controls", control);
> +
> if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
> /*
> * We have ASPM control, but the FADT indicates that
> @@ -498,10 +478,8 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
> *no_aspm = 1;
> }
> } else {
> - decode_osc_control(root, "OS requested", requested);
> - decode_osc_control(root, "platform willing to grant", control);
> - dev_info(&device->dev, "_OSC: platform retains control of PCIe features (%s)\n",
> - acpi_format_exception(status));
> + /* Platform wants to control PCIe features */
> + root->osc_support_set = 0;
>
> /*
> * We want to disable ASPM here, but aspm_disabled
> @@ -511,6 +489,18 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
> * root scan.
> */
> *no_aspm = 1;
> +
> + /* _OSC is optional for PCI host bridges */
> + if ((status == AE_NOT_FOUND) && !is_pcie)
> + return;
> +
> + if (requested) {
> + decode_osc_control(root, "OS requested", requested);
> + decode_osc_control(root, "platform willing to grant", control);
> + }
> +
> + dev_info(&device->dev, "_OSC: platform retains control of PCIe features (%s)\n",
> + acpi_format_exception(status));
> }
> }
>
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] PCI/APCI: Move acpi_pci_osc_support() check to negotiation phase
2021-07-14 12:04 ` Rafael J. Wysocki
@ 2021-07-15 7:34 ` Joerg Roedel
0 siblings, 0 replies; 5+ messages in thread
From: Joerg Roedel @ 2021-07-15 7:34 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Joerg Roedel, Bjorn Helgaas, Rafael J . Wysocki, Len Brown,
Linux PCI, ACPI Devel Maling List, Linux Kernel Mailing List
Hi Rafael,
On Wed, Jul 14, 2021 at 02:04:17PM +0200, Rafael J. Wysocki wrote:
> > decode_osc_support(root, "OS supports", support);
> > - status = acpi_pci_osc_support(root, support);
> > - if (ACPI_FAILURE(status)) {
> > - *no_aspm = 1;
> >
> > - /* _OSC is optional for PCI host bridges */
> > - if ((status == AE_NOT_FOUND) && !is_pcie)
> > + if (!pcie_ports_disabled) {
>
> If pcie_ports_disabled is set, we don't want to request any control
> from the platform firmware at all and, specifically, we don't want to
> evaluate _OSC with the OSC_QUERY_ENABLE clear in
> capbuf[OSC_QUERY_DWORD].
>
> I'm not sure how this is achieved after your changes.
Yeah, it isn't. The acpi_pci_osc_control_set() function will always do
an _OSC call with OSC_QUERY_ENABLE clear. I will come up with a new
approach.
Thanks,
Joerg
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-15 7:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-14 8:55 [PATCH v2 0/2] PCI/ACPI: Simplify PCIe _OSC feature negotiation Joerg Roedel
2021-07-14 8:55 ` [PATCH v2 1/2] PCI/APCI: Move acpi_pci_osc_support() check to negotiation phase Joerg Roedel
2021-07-14 12:04 ` Rafael J. Wysocki
2021-07-15 7:34 ` Joerg Roedel
2021-07-14 8:55 ` [PATCH v2 2/2] PCI/ACPI: Remove OSC_PCI_SUPPORT_MASKS and OSC_PCI_CONTROL_MASKS Joerg Roedel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).