[PATCH] platform/x86: thinkpad_acpi: Add missing T14s Gen1 type to s2idle quirk list

[PATCH] platform/x86: thinkpad_acpi: Add missing T14s Gen1 type to s2idle quirk list

[Benjamin Asbach]

> Lenovo laptops that contain NVME SSDs across a variety of generations have
> trouble resuming from suspend to idle when the IOMMU translation layer is
> active for the NVME storage device.
>
> This generally manifests as a large resume delay or page faults. These
> delays and page faults occur as a result of a Lenovo BIOS specific SMI
> that runs during the D3->D0 transition on NVME devices.

Link: https://lore.kernel.org/all/20220503183420.348-1-mario.limonciello@amd.com/

As Lenovo distributes T14s Gen1 laptops with different product names
a missing one is added by this patch.

Note: Based on lenovo support page there might be some more variants which
are not represented in s2idle quirk list.

Signed-off-by: Benjamin Asbach <asbachb.kernel@impl.it>
Tested-by: Benjamin Asbach <asbachb.kernel@impl.it>

Link: https://bbs.archlinux.org/viewtopic.php?pid=2084655#p2084655
---
 drivers/platform/x86/thinkpad_acpi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 32c10457399e..57428d7a392d 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -4478,6 +4478,15 @@ static const struct dmi_system_id fwbug_list[] __initconst = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "20UH"),
 		}
 	},
+	{
+		.ident = "T14s Gen1 AMD",
+		.driver_data = &quirk_s2idle_bug,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "20UJ"),
+		}
+	},
+
 	{
 		.ident = "P14s Gen1 AMD",
 		.driver_data = &quirk_s2idle_bug,
-- 
2.40.0

Re: [PATCH] platform/x86: thinkpad_acpi: Add missing T14s Gen1 type to s2idle quirk list

[Hans de Goede]

Hi,

On 4/1/23 01:24, Benjamin Asbach wrote:
>> Lenovo laptops that contain NVME SSDs across a variety of generations have
>> trouble resuming from suspend to idle when the IOMMU translation layer is
>> active for the NVME storage device.
>>
>> This generally manifests as a large resume delay or page faults. These
>> delays and page faults occur as a result of a Lenovo BIOS specific SMI
>> that runs during the D3->D0 transition on NVME devices.
> 
> Link: https://lore.kernel.org/all/20220503183420.348-1-mario.limonciello@amd.com/
> 
> As Lenovo distributes T14s Gen1 laptops with different product names
> a missing one is added by this patch.
> 
> Note: Based on lenovo support page there might be some more variants which
> are not represented in s2idle quirk list.

Can you provide some more in info on this? Then Mark can maybe check
if we need to add more models ?

Mark, generally speaking it may help to do a DMI_EXACT_MATCH on
DMI_PRODUCT_VERSION with ThinkPads ? That contains the human
readable model string instead of things like "20UJ", and I guess
that we want to e.g. apply the s2idle quirk to all "T14s Gen1 AMD"
ThinkPads.

> 
> Signed-off-by: Benjamin Asbach <asbachb.kernel@impl.it>
> Tested-by: Benjamin Asbach <asbachb.kernel@impl.it>

Thank you for your patch, I've applied this patch to my fixes
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes

Note it will show up in my fixes branch once I've pushed my
local branch there, which might take a while.

I will include this patch in my next fixes pull-req to Linus
for the current kernel development cycle.

Regards,

Hans





> 
> Link: https://bbs.archlinux.org/viewtopic.php?pid=2084655#p2084655
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 32c10457399e..57428d7a392d 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -4478,6 +4478,15 @@ static const struct dmi_system_id fwbug_list[] __initconst = {
>  			DMI_MATCH(DMI_PRODUCT_NAME, "20UH"),
>  		}
>  	},
> +	{
> +		.ident = "T14s Gen1 AMD",
> +		.driver_data = &quirk_s2idle_bug,
> +		.matches = {
> +			DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "20UJ"),
> +		}
> +	},
> +
>  	{
>  		.ident = "P14s Gen1 AMD",
>  		.driver_data = &quirk_s2idle_bug,

Re: [PATCH] platform/x86: thinkpad_acpi: Add missing T14s Gen1 type to s2idle quirk list

[Hans de Goede]

Hi,

On 4/1/23 01:24, Benjamin Asbach wrote:
>> Lenovo laptops that contain NVME SSDs across a variety of generations have
>> trouble resuming from suspend to idle when the IOMMU translation layer is
>> active for the NVME storage device.
>>
>> This generally manifests as a large resume delay or page faults. These
>> delays and page faults occur as a result of a Lenovo BIOS specific SMI
>> that runs during the D3->D0 transition on NVME devices.
> 
> Link: https://lore.kernel.org/all/20220503183420.348-1-mario.limonciello@amd.com/
> 
> As Lenovo distributes T14s Gen1 laptops with different product names
> a missing one is added by this patch.
> 
> Note: Based on lenovo support page there might be some more variants which
> are not represented in s2idle quirk list.

Can you provide some more in info on this? Then Mark can maybe check
if we need to add more models ?

Mark, generally speaking it may help to do a DMI_EXACT_MATCH on
DMI_PRODUCT_VERSION with ThinkPads ? That contains the human
readable model string instead of things like "20UJ", and I guess
that we want to e.g. apply the s2idle quirk to all "T14s Gen1 AMD"
ThinkPads.

> 
> Signed-off-by: Benjamin Asbach <asbachb.kernel@impl.it>
> Tested-by: Benjamin Asbach <asbachb.kernel@impl.it>

Thank you for your patch, I've applied this patch to my fixes
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes

I will include this patch in my next fixes pull-req to Linus
for the current kernel development cycle.

Regards,

Hans





> 
> Link: https://bbs.archlinux.org/viewtopic.php?pid=2084655#p2084655
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 32c10457399e..57428d7a392d 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -4478,6 +4478,15 @@ static const struct dmi_system_id fwbug_list[] __initconst = {
>  			DMI_MATCH(DMI_PRODUCT_NAME, "20UH"),
>  		}
>  	},
> +	{
> +		.ident = "T14s Gen1 AMD",
> +		.driver_data = &quirk_s2idle_bug,
> +		.matches = {
> +			DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "20UJ"),
> +		}
> +	},
> +
>  	{
>  		.ident = "P14s Gen1 AMD",
>  		.driver_data = &quirk_s2idle_bug,

Re: [PATCH] platform/x86: thinkpad_acpi: Add missing T14s Gen1 type to s2idle quirk list

[Benjamin Asbach]

On 2023-04-03 12:03, Hans de Goede wrote:
> Hi,
> 
> On 4/1/23 01:24, Benjamin Asbach wrote:
>>> Lenovo laptops that contain NVME SSDs across a variety of generations 
>>> have
>>> trouble resuming from suspend to idle when the IOMMU translation 
>>> layer is
>>> active for the NVME storage device.
>>> 
>>> This generally manifests as a large resume delay or page faults. 
>>> These
>>> delays and page faults occur as a result of a Lenovo BIOS specific 
>>> SMI
>>> that runs during the D3->D0 transition on NVME devices.
>> 
>> Link: 
>> https://lore.kernel.org/all/20220503183420.348-1-mario.limonciello@amd.com/
>> 
>> As Lenovo distributes T14s Gen1 laptops with different product names
>> a missing one is added by this patch.
>> 
>> Note: Based on lenovo support page there might be some more variants 
>> which
>> are not represented in s2idle quirk list.
> 
> Can you provide some more in info on this? Then Mark can maybe check
> if we need to add more models ?

Sure. When you go to https://pcsupport.lenovo.com/us/en/ and type in any 
of the
models in that quirk list - for example `T14 Gen2` beside `20XK` there's 
also `20XL`.
I assume - but cannot confirm - that the other types suffer the same 
firmware issue.

For my model the issue only arised after I upgraded the original NVMe 
with a bigger one.

Greetings,
Benjamin

> Mark, generally speaking it may help to do a DMI_EXACT_MATCH on
> DMI_PRODUCT_VERSION with ThinkPads ? That contains the human
> readable model string instead of things like "20UJ", and I guess
> that we want to e.g. apply the s2idle quirk to all "T14s Gen1 AMD"
> ThinkPads.
> 
>> 
>> Signed-off-by: Benjamin Asbach <asbachb.kernel@impl.it>
>> Tested-by: Benjamin Asbach <asbachb.kernel@impl.it>
> 
> Thank you for your patch, I've applied this patch to my fixes
> branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes
> 
> I will include this patch in my next fixes pull-req to Linus
> for the current kernel development cycle.
> 
> Regards,
> 
> Hans
> 
> 
> 
> 
> 
>> 
>> Link: https://bbs.archlinux.org/viewtopic.php?pid=2084655#p2084655
>> ---
>>  drivers/platform/x86/thinkpad_acpi.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>> 
>> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
>> b/drivers/platform/x86/thinkpad_acpi.c
>> index 32c10457399e..57428d7a392d 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -4478,6 +4478,15 @@ static const struct dmi_system_id fwbug_list[] 
>> __initconst = {
>>  			DMI_MATCH(DMI_PRODUCT_NAME, "20UH"),
>>  		}
>>  	},
>> +	{
>> +		.ident = "T14s Gen1 AMD",
>> +		.driver_data = &quirk_s2idle_bug,
>> +		.matches = {
>> +			DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
>> +			DMI_MATCH(DMI_PRODUCT_NAME, "20UJ"),
>> +		}
>> +	},
>> +
>>  	{
>>  		.ident = "P14s Gen1 AMD",
>>  		.driver_data = &quirk_s2idle_bug,

Re: [PATCH] platform/x86: thinkpad_acpi: Add missing T14s Gen1 type to s2idle quirk list

[Mark Pearson]

Hi Hans

On Mon, Apr 3, 2023, at 6:03 AM, Hans de Goede wrote:
> Hi,
>
> On 4/1/23 01:24, Benjamin Asbach wrote:
>>> Lenovo laptops that contain NVME SSDs across a variety of generations have
>>> trouble resuming from suspend to idle when the IOMMU translation layer is
>>> active for the NVME storage device.
>>>
>>> This generally manifests as a large resume delay or page faults. These
>>> delays and page faults occur as a result of a Lenovo BIOS specific SMI
>>> that runs during the D3->D0 transition on NVME devices.
>> 
>> Link: https://lore.kernel.org/all/20220503183420.348-1-mario.limonciello@amd.com/
>> 
>> As Lenovo distributes T14s Gen1 laptops with different product names
>> a missing one is added by this patch.
>> 
>> Note: Based on lenovo support page there might be some more variants which
>> are not represented in s2idle quirk list.
>
> Can you provide some more in info on this? Then Mark can maybe check
> if we need to add more models ?
>
> Mark, generally speaking it may help to do a DMI_EXACT_MATCH on
> DMI_PRODUCT_VERSION with ThinkPads ? That contains the human
> readable model string instead of things like "20UJ", and I guess
> that we want to e.g. apply the s2idle quirk to all "T14s Gen1 AMD"
> ThinkPads.

Sadly that won't work :(
 - The same ID is used for multiple platform names and those can change by geography (for instance China often calls things differently) or if WWAN supported, etc. 
 - They use the same platform name for Intel and AMD in a few cases (not all). And this match should only be done for the AMD platforms.

For every platform there are two IDs. In this case the T14s G1 has 20UH and 20UJ. I need to figure out when each is used - I thought only the first one was in released platforms but it seems that's not the case from this patch. I need to understand how/why.

For models impacted - there are a couple missing from the list that I would expect to see there as they're the same generation: X13 G1 and L15 G2 (and a possible ? against L14/L15 G1). I'm also a bit cautious as the E-series might need to show up here - but I don't know those platforms as well..
And depending on the two IDs...some of the platforms may need doubling up. Urgh.

Mark

Re: [PATCH] platform/x86: thinkpad_acpi: Add missing T14s Gen1 type to s2idle quirk list

[Hans de Goede]

Hi Mark,

On 4/3/23 16:41, Mark Pearson wrote:
> Hi Hans
> 
> On Mon, Apr 3, 2023, at 6:03 AM, Hans de Goede wrote:
>> Hi,
>>
>> On 4/1/23 01:24, Benjamin Asbach wrote:
>>>> Lenovo laptops that contain NVME SSDs across a variety of generations have
>>>> trouble resuming from suspend to idle when the IOMMU translation layer is
>>>> active for the NVME storage device.
>>>>
>>>> This generally manifests as a large resume delay or page faults. These
>>>> delays and page faults occur as a result of a Lenovo BIOS specific SMI
>>>> that runs during the D3->D0 transition on NVME devices.
>>>
>>> Link: https://lore.kernel.org/all/20220503183420.348-1-mario.limonciello@amd.com/
>>>
>>> As Lenovo distributes T14s Gen1 laptops with different product names
>>> a missing one is added by this patch.
>>>
>>> Note: Based on lenovo support page there might be some more variants which
>>> are not represented in s2idle quirk list.
>>
>> Can you provide some more in info on this? Then Mark can maybe check
>> if we need to add more models ?
>>
>> Mark, generally speaking it may help to do a DMI_EXACT_MATCH on
>> DMI_PRODUCT_VERSION with ThinkPads ? That contains the human
>> readable model string instead of things like "20UJ", and I guess
>> that we want to e.g. apply the s2idle quirk to all "T14s Gen1 AMD"
>> ThinkPads.
> 
> Sadly that won't work :(
>  - The same ID is used for multiple platform names and those can change by geography (for instance China often calls things differently) or if WWAN supported, etc. 
>  - They use the same platform name for Intel and AMD in a few cases (not all). And this match should only be done for the AMD platforms.
> 
> For every platform there are two IDs. In this case the T14s G1 has 20UH and 20UJ. I need to figure out when each is used - I thought only the first one was in released platforms but it seems that's not the case from this patch. I need to understand how/why.
> 
> For models impacted - there are a couple missing from the list that I would expect to see there as they're the same generation: X13 G1 and L15 G2 (and a possible ? against L14/L15 G1). I'm also a bit cautious as the E-series might need to show up here - but I don't know those platforms as well..
> And depending on the two IDs...some of the platforms may need doubling up. Urgh.

Ok.

Stating the obvious here: Please send a patch adding
the necessary extra IDs once you know which ids to add.

Regards,

Hans