[PATCH] platform/x86: thinkpad_acpi: do not use PSC mode on Intel platforms

[PATCH] platform/x86: thinkpad_acpi: do not use PSC mode on Intel platforms

[Mark Pearson]

PSC platform profile mode is only supported on Linux for AMD platforms.

Some older Intel platforms (e.g T490) are advertising it's capability
as Windows uses it - but on Linux we should only be using MMC profile
for Intel systems.

Add a check to prevent it being enabled incorrectly.

Signed-off-by: Mark Pearson <markpearson@lenovo.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index e6cb4a14cdd4..be194be43663 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -10548,6 +10548,11 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
 				dytc_mmc_get_available = true;
 		}
 	} else if (output & BIT(DYTC_FC_PSC)) { /* PSC MODE */
+		/* Support for this only works on AMD platforms */
+		if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) {
+			dbg_printk(TPACPI_DBG_INIT, "PSC not support on Intel platforms\n");
+			return -ENODEV;
+		}
 		dytc_profile_available = DYTC_FUNCMODE_PSC;
 	} else {
 		dbg_printk(TPACPI_DBG_INIT, "No DYTC support available\n");
-- 
2.35.1

Re: [PATCH] platform/x86: thinkpad_acpi: do not use PSC mode on Intel platforms

[Hans de Goede]

Hi,

On 6/22/22 20:13, Mark Pearson wrote:
> PSC platform profile mode is only supported on Linux for AMD platforms.
> 
> Some older Intel platforms (e.g T490) are advertising it's capability
> as Windows uses it - but on Linux we should only be using MMC profile
> for Intel systems.
> 
> Add a check to prevent it being enabled incorrectly.
> 
> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index e6cb4a14cdd4..be194be43663 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -10548,6 +10548,11 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
>  				dytc_mmc_get_available = true;
>  		}
>  	} else if (output & BIT(DYTC_FC_PSC)) { /* PSC MODE */

After your recent patch series this now reads:

        } else if (dytc_capabilities & BIT(DYTC_FC_PSC)) { /* PSC MODE */

Please rebase on pdx86/for-next and send a new version.



> +		/* Support for this only works on AMD platforms */
> +		if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) {
> +			dbg_printk(TPACPI_DBG_INIT, "PSC not support on Intel platforms\n");
> +			return -ENODEV;
> +		}

So I assume that e.g. the T490 does advertise MMC capability so
this path is not actually hit there ?

IOW this is just a sanity check. Or is this path being hit on actual
hw? The reason I'm asking is because if the path is being hit on actual
hw then the patch should go to my fixes branch too.

Regards,

Hans



>  		dytc_profile_available = DYTC_FUNCMODE_PSC;
>  	} else {
>  		dbg_printk(TPACPI_DBG_INIT, "No DYTC support available\n");

Re: [External] Re: [PATCH] platform/x86: thinkpad_acpi: do not use PSC mode on Intel platforms

[Mark Pearson]

Hi Hans

On 6/27/22 03:52, Hans de Goede wrote:
> Hi,
> 
> On 6/22/22 20:13, Mark Pearson wrote:
>> PSC platform profile mode is only supported on Linux for AMD platforms.
>>
>> Some older Intel platforms (e.g T490) are advertising it's capability
>> as Windows uses it - but on Linux we should only be using MMC profile
>> for Intel systems.
>>
>> Add a check to prevent it being enabled incorrectly.
>>
>> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
>> ---
>>  drivers/platform/x86/thinkpad_acpi.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>> index e6cb4a14cdd4..be194be43663 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -10548,6 +10548,11 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
>>  				dytc_mmc_get_available = true;
>>  		}
>>  	} else if (output & BIT(DYTC_FC_PSC)) { /* PSC MODE */
> 
> After your recent patch series this now reads:
> 
>         } else if (dytc_capabilities & BIT(DYTC_FC_PSC)) { /* PSC MODE */
> 
> Please rebase on pdx86/for-next and send a new version.
Ack - will do.

> 
> 
> 
>> +		/* Support for this only works on AMD platforms */
>> +		if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) {
>> +			dbg_printk(TPACPI_DBG_INIT, "PSC not support on Intel platforms\n");
>> +			return -ENODEV;
>> +		}
> 
> So I assume that e.g. the T490 does advertise MMC capability so
> this path is not actually hit there ?
No - they don't have MMC so this path is hit.

> 
> IOW this is just a sanity check. Or is this path being hit on actual
> hw? The reason I'm asking is because if the path is being hit on actual
> hw then the patch should go to my fixes branch too.
> 
This is being hit on a few of the Intel platforms of that generation. It
seems they don't have MMC mode support, but do have PSC mode - but that
only works on Windows for Intel (needs driver changes we don't have).

Thanks
Mark

Re: [External] Re: [PATCH] platform/x86: thinkpad_acpi: do not use PSC mode on Intel platforms

[Hans de Goede]

Hi,

On 6/27/22 16:43, Mark Pearson wrote:
> 
> Hi Hans
> 
> On 6/27/22 03:52, Hans de Goede wrote:
>> Hi,
>>
>> On 6/22/22 20:13, Mark Pearson wrote:
>>> PSC platform profile mode is only supported on Linux for AMD platforms.
>>>
>>> Some older Intel platforms (e.g T490) are advertising it's capability
>>> as Windows uses it - but on Linux we should only be using MMC profile
>>> for Intel systems.
>>>
>>> Add a check to prevent it being enabled incorrectly.
>>>
>>> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
>>> ---
>>>  drivers/platform/x86/thinkpad_acpi.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>>> index e6cb4a14cdd4..be194be43663 100644
>>> --- a/drivers/platform/x86/thinkpad_acpi.c
>>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>>> @@ -10548,6 +10548,11 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
>>>  				dytc_mmc_get_available = true;
>>>  		}
>>>  	} else if (output & BIT(DYTC_FC_PSC)) { /* PSC MODE */
>>
>> After your recent patch series this now reads:
>>
>>         } else if (dytc_capabilities & BIT(DYTC_FC_PSC)) { /* PSC MODE */
>>
>> Please rebase on pdx86/for-next and send a new version.
> Ack - will do.
> 
>>
>>
>>
>>> +		/* Support for this only works on AMD platforms */
>>> +		if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) {
>>> +			dbg_printk(TPACPI_DBG_INIT, "PSC not support on Intel platforms\n");
>>> +			return -ENODEV;
>>> +		}
>>
>> So I assume that e.g. the T490 does advertise MMC capability so
>> this path is not actually hit there ?
> No - they don't have MMC so this path is hit.
> 
>>
>> IOW this is just a sanity check. Or is this path being hit on actual
>> hw? The reason I'm asking is because if the path is being hit on actual
>> hw then the patch should go to my fixes branch too.
>>
> This is being hit on a few of the Intel platforms of that generation. It
> seems they don't have MMC mode support, but do have PSC mode - but that
> only works on Windows for Intel (needs driver changes we don't have).

Ok, once I have a new version of this, I'll add it to my fixes branch then.

Regards,

Hans