[v4] PCI: Avoid putting some root ports into D3 on TUXEDO Sirius Gen1

[PATCH v4] PCI: Avoid putting some root ports into D3 on TUXEDO Sirius Gen1

Posted by Werner Sembach 12 months ago

From: Mario Limonciello <mario.limonciello@amd.com>

commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") sets the
policy that all PCIe ports are allowed to use D3.  When the system is
suspended if the port is not power manageable by the platform and won't be
used for wakeup via a PME this sets up the policy for these ports to go
into D3hot.

This policy generally makes sense from an OSPM perspective but it leads to
problems with wakeup from suspend on the TUXEDO Sirius 16 Gen 1 with a
specific old BIOS. This manifests as a system hang.

On the affected Device + BIOS combination, add a quirk for the root port of
the problematic controller to ensure that these root ports are not put into
D3hot at suspend.

This patch is based on
https://lore.kernel.org/linux-pci/20230708214457.1229-2-mario.limonciello@amd.com/
but with the added condition both in the documentation and in the code to
apply only to the TUXEDO Sirius 16 Gen 1 with a specific old BIOS and only
the affected root ports.

Co-developed-by: Georg Gottleuber <ggo@tuxedocomputers.com>
Signed-off-by: Georg Gottleuber <ggo@tuxedocomputers.com>
Co-developed-by: Werner Sembach <wse@tuxedocomputers.com>
Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
Cc: stable@vger.kernel.org # 6.1+
Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/pci/quirks.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 76f4df75b08a1..d2f45c3e24c0a 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3908,6 +3908,36 @@ static void quirk_apple_poweroff_thunderbolt(struct pci_dev *dev)
 DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL,
 			       PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C,
 			       quirk_apple_poweroff_thunderbolt);
+
+/*
+ * Putting PCIe root ports on Ryzen SoCs with USB4 controllers into D3hot
+ * may cause problems when the system attempts wake up from s2idle.
+ *
+ * On the TUXEDO Sirius 16 Gen 1 with a specific old BIOS this manifests as
+ * a system hang.
+ */
+static const struct dmi_system_id quirk_ryzen_rp_d3_dmi_table[] = {
+	{
+		.matches = {
+			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "TUXEDO"),
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "APX958"),
+			DMI_EXACT_MATCH(DMI_BIOS_VERSION, "V1.00A00_20240108"),
+		},
+	},
+	{}
+};
+
+static void quirk_ryzen_rp_d3(struct pci_dev *pdev)
+{
+	struct pci_dev *root_pdev;
+
+	if (dmi_check_system(quirk_ryzen_rp_d3_dmi_table)) {
+		root_pdev = pcie_find_root_port(pdev);
+		if (root_pdev && !acpi_pci_power_manageable(root_pdev))
+			root_pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
+	}
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x1502, quirk_ryzen_rp_d3);
 #endif
 
 /*
-- 
2.43.0

Re: [PATCH v4] PCI: Avoid putting some root ports into D3 on TUXEDO Sirius Gen1

Posted by Mario Limonciello 11 months, 2 weeks ago

On 12/20/2024 05:35, Werner Sembach wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") sets the
> policy that all PCIe ports are allowed to use D3.  When the system is
> suspended if the port is not power manageable by the platform and won't be
> used for wakeup via a PME this sets up the policy for these ports to go
> into D3hot.
> 
> This policy generally makes sense from an OSPM perspective but it leads to
> problems with wakeup from suspend on the TUXEDO Sirius 16 Gen 1 with a
> specific old BIOS. This manifests as a system hang.
> 
> On the affected Device + BIOS combination, add a quirk for the root port of
> the problematic controller to ensure that these root ports are not put into
> D3hot at suspend.
> 
> This patch is based on
> https://lore.kernel.org/linux-pci/20230708214457.1229-2-mario.limonciello@amd.com/
> but with the added condition both in the documentation and in the code to
> apply only to the TUXEDO Sirius 16 Gen 1 with a specific old BIOS and only
> the affected root ports.
> 
> Co-developed-by: Georg Gottleuber <ggo@tuxedocomputers.com>
> Signed-off-by: Georg Gottleuber <ggo@tuxedocomputers.com>
> Co-developed-by: Werner Sembach <wse@tuxedocomputers.com>
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> Cc: stable@vger.kernel.org # 6.1+
> Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

So I don't think this should have my S-o-b.  At most it should 
Suggested-by: or Co-developed-by: since it was based on my original patch.

> ---
>   drivers/pci/quirks.c | 30 ++++++++++++++++++++++++++++++

I think a better location for this is arch/x86/pci/fixup.c, similar to 
how we have https://git.kernel.org/torvalds/c/7d08f21f8c630

thoughts?

>   1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 76f4df75b08a1..d2f45c3e24c0a 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c

> @@ -3908,6 +3908,36 @@ static void quirk_apple_poweroff_thunderbolt(struct pci_dev *dev)
>   DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL,
>   			       PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C,
>   			       quirk_apple_poweroff_thunderbolt);
> +
> +/*
> + * Putting PCIe root ports on Ryzen SoCs with USB4 controllers into D3hot
> + * may cause problems when the system attempts wake up from s2idle.
> + *
> + * On the TUXEDO Sirius 16 Gen 1 with a specific old BIOS this manifests as
> + * a system hang.
> + */
> +static const struct dmi_system_id quirk_ryzen_rp_d3_dmi_table[] = {
> +	{
> +		.matches = {
> +			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "TUXEDO"),
> +			DMI_EXACT_MATCH(DMI_BOARD_NAME, "APX958"),
> +			DMI_EXACT_MATCH(DMI_BIOS_VERSION, "V1.00A00_20240108"),
> +		},
> +	},
> +	{}
> +};
> +
> +static void quirk_ryzen_rp_d3(struct pci_dev *pdev)
> +{
> +	struct pci_dev *root_pdev;
> +
> +	if (dmi_check_system(quirk_ryzen_rp_d3_dmi_table)) {
> +		root_pdev = pcie_find_root_port(pdev);
> +		if (root_pdev && !acpi_pci_power_manageable(root_pdev))
> +			root_pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
> +	}
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x1502, quirk_ryzen_rp_d3);
>   #endif
>   
>   /*

Re: [PATCH v4] PCI: Avoid putting some root ports into D3 on TUXEDO Sirius Gen1

Posted by Werner Sembach 11 months, 1 week ago

Am 08.01.25 um 22:26 schrieb Mario Limonciello:
> On 12/20/2024 05:35, Werner Sembach wrote:
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") sets the
>> policy that all PCIe ports are allowed to use D3.  When the system is
>> suspended if the port is not power manageable by the platform and won't be
>> used for wakeup via a PME this sets up the policy for these ports to go
>> into D3hot.
>>
>> This policy generally makes sense from an OSPM perspective but it leads to
>> problems with wakeup from suspend on the TUXEDO Sirius 16 Gen 1 with a
>> specific old BIOS. This manifests as a system hang.
>>
>> On the affected Device + BIOS combination, add a quirk for the root port of
>> the problematic controller to ensure that these root ports are not put into
>> D3hot at suspend.
>>
>> This patch is based on
>> https://lore.kernel.org/linux-pci/20230708214457.1229-2-mario.limonciello@amd.com/ 
>>
>> but with the added condition both in the documentation and in the code to
>> apply only to the TUXEDO Sirius 16 Gen 1 with a specific old BIOS and only
>> the affected root ports.
>>
>> Co-developed-by: Georg Gottleuber <ggo@tuxedocomputers.com>
>> Signed-off-by: Georg Gottleuber <ggo@tuxedocomputers.com>
>> Co-developed-by: Werner Sembach <wse@tuxedocomputers.com>
>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>> Cc: stable@vger.kernel.org # 6.1+
>> Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>
> So I don't think this should have my S-o-b.  At most it should Suggested-by: 
> or Co-developed-by: since it was based on my original patch.
kk
>
>> ---
>>   drivers/pci/quirks.c | 30 ++++++++++++++++++++++++++++++
>
> I think a better location for this is arch/x86/pci/fixup.c, similar to how we 
> have https://git.kernel.org/torvalds/c/7d08f21f8c630
>
> thoughts?

Fine with me

I will make a v5

>
>>   1 file changed, 30 insertions(+)
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 76f4df75b08a1..d2f45c3e24c0a 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>
>> @@ -3908,6 +3908,36 @@ static void quirk_apple_poweroff_thunderbolt(struct 
>> pci_dev *dev)
>>   DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL,
>>                      PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C,
>>                      quirk_apple_poweroff_thunderbolt);
>> +
>> +/*
>> + * Putting PCIe root ports on Ryzen SoCs with USB4 controllers into D3hot
>> + * may cause problems when the system attempts wake up from s2idle.
>> + *
>> + * On the TUXEDO Sirius 16 Gen 1 with a specific old BIOS this manifests as
>> + * a system hang.
>> + */
>> +static const struct dmi_system_id quirk_ryzen_rp_d3_dmi_table[] = {
>> +    {
>> +        .matches = {
>> +            DMI_EXACT_MATCH(DMI_SYS_VENDOR, "TUXEDO"),
>> +            DMI_EXACT_MATCH(DMI_BOARD_NAME, "APX958"),
>> +            DMI_EXACT_MATCH(DMI_BIOS_VERSION, "V1.00A00_20240108"),
>> +        },
>> +    },
>> +    {}
>> +};
>> +
>> +static void quirk_ryzen_rp_d3(struct pci_dev *pdev)
>> +{
>> +    struct pci_dev *root_pdev;
>> +
>> +    if (dmi_check_system(quirk_ryzen_rp_d3_dmi_table)) {
>> +        root_pdev = pcie_find_root_port(pdev);
>> +        if (root_pdev && !acpi_pci_power_manageable(root_pdev))
>> +            root_pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
>> +    }
>> +}
>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x1502, quirk_ryzen_rp_d3);
>>   #endif
>>     /*
>

Re: [PATCH v4] PCI: Avoid putting some root ports into D3 on TUXEDO Sirius Gen1

Posted by Werner Sembach 11 months, 1 week ago

Am 10.01.25 um 18:15 schrieb Werner Sembach:
>
> Am 08.01.25 um 22:26 schrieb Mario Limonciello:
>> On 12/20/2024 05:35, Werner Sembach wrote:
>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>
>>> commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") sets the
>>> policy that all PCIe ports are allowed to use D3.  When the system is
>>> suspended if the port is not power manageable by the platform and won't be
>>> used for wakeup via a PME this sets up the policy for these ports to go
>>> into D3hot.
>>>
>>> This policy generally makes sense from an OSPM perspective but it leads to
>>> problems with wakeup from suspend on the TUXEDO Sirius 16 Gen 1 with a
>>> specific old BIOS. This manifests as a system hang.
>>>
>>> On the affected Device + BIOS combination, add a quirk for the root port of
>>> the problematic controller to ensure that these root ports are not put into
>>> D3hot at suspend.
>>>
>>> This patch is based on
>>> https://lore.kernel.org/linux-pci/20230708214457.1229-2-mario.limonciello@amd.com/ 
>>>
>>> but with the added condition both in the documentation and in the code to
>>> apply only to the TUXEDO Sirius 16 Gen 1 with a specific old BIOS and only
>>> the affected root ports.
>>>
>>> Co-developed-by: Georg Gottleuber <ggo@tuxedocomputers.com>
>>> Signed-off-by: Georg Gottleuber <ggo@tuxedocomputers.com>
>>> Co-developed-by: Werner Sembach <wse@tuxedocomputers.com>
>>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>>> Cc: stable@vger.kernel.org # 6.1+
>>> Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>
>> So I don't think this should have my S-o-b.  At most it should Suggested-by: 
>> or Co-developed-by: since it was based on my original patch.
> kk
>>
>>> ---
>>>   drivers/pci/quirks.c | 30 ++++++++++++++++++++++++++++++
>>
>> I think a better location for this is arch/x86/pci/fixup.c, similar to how we 
>> have https://git.kernel.org/torvalds/c/7d08f21f8c630
>>
>> thoughts?
>
> Fine with me
>
> I will make a v5
In fixup.c i don't have access to acpi_pci_power_manageable, but since 
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x1502, quirk_ryzen_rp_d3); matches 
to only one device anyways can i just skip it?
>
>>
>>>   1 file changed, 30 insertions(+)
>>>
>>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>>> index 76f4df75b08a1..d2f45c3e24c0a 100644
>>> --- a/drivers/pci/quirks.c
>>> +++ b/drivers/pci/quirks.c
>>
>>> @@ -3908,6 +3908,36 @@ static void quirk_apple_poweroff_thunderbolt(struct 
>>> pci_dev *dev)
>>>   DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL,
>>>                      PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C,
>>>                      quirk_apple_poweroff_thunderbolt);
>>> +
>>> +/*
>>> + * Putting PCIe root ports on Ryzen SoCs with USB4 controllers into D3hot
>>> + * may cause problems when the system attempts wake up from s2idle.
>>> + *
>>> + * On the TUXEDO Sirius 16 Gen 1 with a specific old BIOS this manifests as
>>> + * a system hang.
>>> + */
>>> +static const struct dmi_system_id quirk_ryzen_rp_d3_dmi_table[] = {
>>> +    {
>>> +        .matches = {
>>> +            DMI_EXACT_MATCH(DMI_SYS_VENDOR, "TUXEDO"),
>>> +            DMI_EXACT_MATCH(DMI_BOARD_NAME, "APX958"),
>>> +            DMI_EXACT_MATCH(DMI_BIOS_VERSION, "V1.00A00_20240108"),
>>> +        },
>>> +    },
>>> +    {}
>>> +};
>>> +
>>> +static void quirk_ryzen_rp_d3(struct pci_dev *pdev)
>>> +{
>>> +    struct pci_dev *root_pdev;
>>> +
>>> +    if (dmi_check_system(quirk_ryzen_rp_d3_dmi_table)) {
>>> +        root_pdev = pcie_find_root_port(pdev);
>>> +        if (root_pdev && !acpi_pci_power_manageable(root_pdev))
>>> +            root_pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
>>> +    }
>>> +}
>>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x1502, quirk_ryzen_rp_d3);
>>>   #endif
>>>     /*
>>

Re: [PATCH v4] PCI: Avoid putting some root ports into D3 on TUXEDO Sirius Gen1

Posted by Mario Limonciello 11 months, 1 week ago

On 1/10/2025 12:15, Werner Sembach wrote:
> 
> Am 10.01.25 um 18:15 schrieb Werner Sembach:
>>
>> Am 08.01.25 um 22:26 schrieb Mario Limonciello:
>>> On 12/20/2024 05:35, Werner Sembach wrote:
>>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>>
>>>> commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") 
>>>> sets the
>>>> policy that all PCIe ports are allowed to use D3.  When the system is
>>>> suspended if the port is not power manageable by the platform and 
>>>> won't be
>>>> used for wakeup via a PME this sets up the policy for these ports to go
>>>> into D3hot.
>>>>
>>>> This policy generally makes sense from an OSPM perspective but it 
>>>> leads to
>>>> problems with wakeup from suspend on the TUXEDO Sirius 16 Gen 1 with a
>>>> specific old BIOS. This manifests as a system hang.
>>>>
>>>> On the affected Device + BIOS combination, add a quirk for the root 
>>>> port of
>>>> the problematic controller to ensure that these root ports are not 
>>>> put into
>>>> D3hot at suspend.
>>>>
>>>> This patch is based on
>>>> https://lore.kernel.org/linux-pci/20230708214457.1229-2- 
>>>> mario.limonciello@amd.com/
>>>> but with the added condition both in the documentation and in the 
>>>> code to
>>>> apply only to the TUXEDO Sirius 16 Gen 1 with a specific old BIOS 
>>>> and only
>>>> the affected root ports.
>>>>
>>>> Co-developed-by: Georg Gottleuber <ggo@tuxedocomputers.com>
>>>> Signed-off-by: Georg Gottleuber <ggo@tuxedocomputers.com>
>>>> Co-developed-by: Werner Sembach <wse@tuxedocomputers.com>
>>>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>>>> Cc: stable@vger.kernel.org # 6.1+
>>>> Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>
>>> So I don't think this should have my S-o-b.  At most it should 
>>> Suggested-by: or Co-developed-by: since it was based on my original 
>>> patch.
>> kk
>>>
>>>> ---
>>>>   drivers/pci/quirks.c | 30 ++++++++++++++++++++++++++++++
>>>
>>> I think a better location for this is arch/x86/pci/fixup.c, similar 
>>> to how we have https://git.kernel.org/torvalds/c/7d08f21f8c630
>>>
>>> thoughts?
>>
>> Fine with me
>>
>> I will make a v5
> In fixup.c i don't have access to acpi_pci_power_manageable, but since 
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x1502, quirk_ryzen_rp_d3); 
> matches to only one device anyways can i just skip it?

Is it just a header problem?  Maybe you can just add the header?

I think if you want to drop it that should be ok, but as it's a problem 
in your BIOS (specifically) and only matching your platform combo I 
would suggest renaming the function and struct to quirk_tuxeo_rp_d3 and 
quirk_tuxedo_rp_d3_dmi_table.

>>
>>>
>>>>   1 file changed, 30 insertions(+)
>>>>
>>>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>>>> index 76f4df75b08a1..d2f45c3e24c0a 100644
>>>> --- a/drivers/pci/quirks.c
>>>> +++ b/drivers/pci/quirks.c
>>>
>>>> @@ -3908,6 +3908,36 @@ static void 
>>>> quirk_apple_poweroff_thunderbolt(struct pci_dev *dev)
>>>>   DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL,
>>>>                      PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C,
>>>>                      quirk_apple_poweroff_thunderbolt);
>>>> +
>>>> +/*
>>>> + * Putting PCIe root ports on Ryzen SoCs with USB4 controllers into 
>>>> D3hot
>>>> + * may cause problems when the system attempts wake up from s2idle.
>>>> + *
>>>> + * On the TUXEDO Sirius 16 Gen 1 with a specific old BIOS this 
>>>> manifests as
>>>> + * a system hang.
>>>> + */
>>>> +static const struct dmi_system_id quirk_ryzen_rp_d3_dmi_table[] = {
>>>> +    {
>>>> +        .matches = {
>>>> +            DMI_EXACT_MATCH(DMI_SYS_VENDOR, "TUXEDO"),
>>>> +            DMI_EXACT_MATCH(DMI_BOARD_NAME, "APX958"),
>>>> +            DMI_EXACT_MATCH(DMI_BIOS_VERSION, "V1.00A00_20240108"),
>>>> +        },
>>>> +    },
>>>> +    {}
>>>> +};
>>>> +
>>>> +static void quirk_ryzen_rp_d3(struct pci_dev *pdev)
>>>> +{
>>>> +    struct pci_dev *root_pdev;
>>>> +
>>>> +    if (dmi_check_system(quirk_ryzen_rp_d3_dmi_table)) {
>>>> +        root_pdev = pcie_find_root_port(pdev);
>>>> +        if (root_pdev && !acpi_pci_power_manageable(root_pdev))
>>>> +            root_pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
>>>> +    }
>>>> +}
>>>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x1502, quirk_ryzen_rp_d3);
>>>>   #endif
>>>>     /*
>>>

Re: [PATCH v4] PCI: Avoid putting some root ports into D3 on TUXEDO Sirius Gen1

Posted by Werner Sembach 11 months, 1 week ago

Am 10.01.25 um 19:20 schrieb Mario Limonciello:
> On 1/10/2025 12:15, Werner Sembach wrote:
>>
>> Am 10.01.25 um 18:15 schrieb Werner Sembach:
>>>
>>> Am 08.01.25 um 22:26 schrieb Mario Limonciello:
>>>> On 12/20/2024 05:35, Werner Sembach wrote:
>>>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>>>
>>>>> commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") sets the
>>>>> policy that all PCIe ports are allowed to use D3.  When the system is
>>>>> suspended if the port is not power manageable by the platform and won't be
>>>>> used for wakeup via a PME this sets up the policy for these ports to go
>>>>> into D3hot.
>>>>>
>>>>> This policy generally makes sense from an OSPM perspective but it leads to
>>>>> problems with wakeup from suspend on the TUXEDO Sirius 16 Gen 1 with a
>>>>> specific old BIOS. This manifests as a system hang.
>>>>>
>>>>> On the affected Device + BIOS combination, add a quirk for the root port of
>>>>> the problematic controller to ensure that these root ports are not put into
>>>>> D3hot at suspend.
>>>>>
>>>>> This patch is based on
>>>>> https://lore.kernel.org/linux-pci/20230708214457.1229-2- 
>>>>> mario.limonciello@amd.com/
>>>>> but with the added condition both in the documentation and in the code to
>>>>> apply only to the TUXEDO Sirius 16 Gen 1 with a specific old BIOS and only
>>>>> the affected root ports.
>>>>>
>>>>> Co-developed-by: Georg Gottleuber <ggo@tuxedocomputers.com>
>>>>> Signed-off-by: Georg Gottleuber <ggo@tuxedocomputers.com>
>>>>> Co-developed-by: Werner Sembach <wse@tuxedocomputers.com>
>>>>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>>>>> Cc: stable@vger.kernel.org # 6.1+
>>>>> Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
>>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>>
>>>> So I don't think this should have my S-o-b.  At most it should 
>>>> Suggested-by: or Co-developed-by: since it was based on my original patch.
>>> kk
>>>>
>>>>> ---
>>>>>   drivers/pci/quirks.c | 30 ++++++++++++++++++++++++++++++
>>>>
>>>> I think a better location for this is arch/x86/pci/fixup.c, similar to how 
>>>> we have https://git.kernel.org/torvalds/c/7d08f21f8c630
>>>>
>>>> thoughts?
>>>
>>> Fine with me
>>>
>>> I will make a v5
>> In fixup.c i don't have access to acpi_pci_power_manageable, but since 
>> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x1502, quirk_ryzen_rp_d3); 
>> matches to only one device anyways can i just skip it?
>
> Is it just a header problem?  Maybe you can just add the header?
It's a local header to the other folder. so no.
>
> I think if you want to drop it that should be ok, but as it's a problem in 
> your BIOS (specifically) and only matching your platform combo I would suggest 
> renaming the function and struct to quirk_tuxeo_rp_d3 and 
> quirk_tuxedo_rp_d3_dmi_table.
Forgot this in v5. I will send a v6.
>
>>>
>>>>
>>>>>   1 file changed, 30 insertions(+)
>>>>>
>>>>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>>>>> index 76f4df75b08a1..d2f45c3e24c0a 100644
>>>>> --- a/drivers/pci/quirks.c
>>>>> +++ b/drivers/pci/quirks.c
>>>>
>>>>> @@ -3908,6 +3908,36 @@ static void quirk_apple_poweroff_thunderbolt(struct 
>>>>> pci_dev *dev)
>>>>>   DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL,
>>>>>                      PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C,
>>>>>                      quirk_apple_poweroff_thunderbolt);
>>>>> +
>>>>> +/*
>>>>> + * Putting PCIe root ports on Ryzen SoCs with USB4 controllers into D3hot
>>>>> + * may cause problems when the system attempts wake up from s2idle.
>>>>> + *
>>>>> + * On the TUXEDO Sirius 16 Gen 1 with a specific old BIOS this manifests as
>>>>> + * a system hang.
>>>>> + */
>>>>> +static const struct dmi_system_id quirk_ryzen_rp_d3_dmi_table[] = {
>>>>> +    {
>>>>> +        .matches = {
>>>>> +            DMI_EXACT_MATCH(DMI_SYS_VENDOR, "TUXEDO"),
>>>>> +            DMI_EXACT_MATCH(DMI_BOARD_NAME, "APX958"),
>>>>> +            DMI_EXACT_MATCH(DMI_BIOS_VERSION, "V1.00A00_20240108"),
>>>>> +        },
>>>>> +    },
>>>>> +    {}
>>>>> +};
>>>>> +
>>>>> +static void quirk_ryzen_rp_d3(struct pci_dev *pdev)
>>>>> +{
>>>>> +    struct pci_dev *root_pdev;
>>>>> +
>>>>> +    if (dmi_check_system(quirk_ryzen_rp_d3_dmi_table)) {
>>>>> +        root_pdev = pcie_find_root_port(pdev);
>>>>> +        if (root_pdev && !acpi_pci_power_manageable(root_pdev))
>>>>> +            root_pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
>>>>> +    }
>>>>> +}
>>>>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x1502, quirk_ryzen_rp_d3);
>>>>>   #endif
>>>>>     /*
>>>>
>