[2/2] ACPI / osi: add DMI quirk for Dell systems

Commit Message

Alex Hung Jan. 31, 2018, 8:40 a.m. UTC

A number of Dell systems require an OEM _OSI string "Linux-Dell-Video" as
a BIOS workaround for a system hang bug caused by discrete VGA. The form of
the OEM _OSI string is discussed in Documentation/acpi/osi.txt and is
defined by each OEM.

Signed-off-by: Alex Hung <[e-mail]>
---
 drivers/acpi/osi.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

Comments

Jean Delvare Feb. 5, 2018, 1:14 p.m. UTC | #1

Hi Alex,

On Wed, 31 Jan 2018 00:40:05 -0800, Alex Hung wrote:
> A number of Dell systems require an OEM _OSI string "Linux-Dell-Video" as
> a BIOS workaround for a system hang bug caused by discrete VGA. The form of
> the OEM _OSI string is discussed in Documentation/acpi/osi.txt and is
> defined by each OEM.

I admit I don't understand how it is the operating system's job to
carry the information from the BIOS to the BIOS.

> Signed-off-by: Alex Hung <[e-mail]>
> ---
>  drivers/acpi/osi.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 107 insertions(+)
> 
> diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
> index 76998a5..43e4349 100644
> --- a/drivers/acpi/osi.c
> +++ b/drivers/acpi/osi.c
> @@ -477,6 +477,112 @@ static const struct dmi_system_id acpi_osi_dmi_table[] __initconst = {
>  	{}
>  };
>  
> +static int __init dmi_oem_osi_add(const struct dmi_system_id *d)
> +{
> +	struct acpi_osi_entry *osi;
> +	const char *str = d->driver_data;
> +	int i;
> +
> +	for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
> +		osi = &osi_setup_entries[i];
> +		if (!strcmp(osi->string, str)) {

This can only happen if the user passes acpi_osi=Linux-Dell-Video or
acpi_osi=!Linux-Dell-Video on the boot command line, right?

> +			osi->enable = true;

Does this not prevent the user from explicitly disabling it with
acpi_osi=!Linux-Dell-Video ?

> +			continue;

Are you not done at this point? I think you want to break, not
continue, else you may add a duplicate Linux-Dell-Video entry below.

> +		} else if (osi->string[0] == '\0') {
> +			osi->enable = true;
> +			strncpy(osi->string, str, OSI_STRING_LENGTH_MAX);
> +			break;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct dmi_system_id acpi_oem_osi_dmi_table[] __initconst = {
> +	{
> +	.callback = dmi_oem_osi_add,
> +	.ident = "Dell Latitude 5491",
> +	.matches = {
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0818]"),
> +		},
> +	.driver_data = "Linux-Dell-Video",
> +	},
> +	{
> +	.callback = dmi_oem_osi_add,
> +	.ident = "Dell Latitude 5591",
> +	.matches = {
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0819]"),
> +		},
> +	.driver_data = "Linux-Dell-Video",
> +	},
> +	{
> +	.callback = dmi_oem_osi_add,
> +	.ident = "Dell Precision 3530",
> +	.matches = {
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0820]"),
> +		},
> +	.driver_data = "Linux-Dell-Video",
> +	},
> +	{
> +	.callback = dmi_oem_osi_add,
> +	.ident = "Dell Inspiron 7777 AIO",
> +	.matches = {
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0850]"),
> +		},
> +	.driver_data = "Linux-Dell-Video",
> +	},
> +	{
> +	.callback = dmi_oem_osi_add,
> +	.ident = "Dell Inspiron 5477 AIO",
> +	.matches = {
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0851]"),
> +		},
> +	.driver_data = "Linux-Dell-Video",
> +	},
> +	{
> +	.callback = dmi_oem_osi_add,
> +	.ident = "Dell G5 5779",
> +	.matches = {
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0886]"),
> +		},
> +	.driver_data = "Linux-Dell-Video",
> +	},
> +	{
> +	.callback = dmi_oem_osi_add,
> +	.ident = "Dell G5 5779",
> +	.matches = {
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0870]"),
> +		},
> +	.driver_data = "Linux-Dell-Video",
> +	},
> +	{
> +	.callback = dmi_oem_osi_add,
> +	.ident = "Dell G5 5579",
> +	.matches = {
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[086F]"),
> +		},
> +	.driver_data = "Linux-Dell-Video",
> +	},
> +	{
> +	.callback = dmi_oem_osi_add,
> +	.ident = "Dell G5 5579",
> +	.matches = {
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0885]"),
> +		},
> +	.driver_data = "Linux-Dell-Video",
> +	},
> +	{}
> +};

G5 5779 and G5 5579 appear twice each, are these copy-and-paste errors?

Choosing a sort rule and sticking to it would make it easier to add
entries later with not risk of duplicates.

> +
>  static __init void acpi_osi_dmi_blacklisted(void)
>  {
>  	dmi_check_system(acpi_osi_dmi_table);
> @@ -496,6 +602,7 @@ int __init early_acpi_osi_init(void)
>  int __init acpi_osi_init(void)
>  {
>  	acpi_install_interface_handler(acpi_osi_handler);
> +	dmi_check_system(acpi_oem_osi_dmi_table);
>  	acpi_osi_setup_late();
>  
>  	return 0;

Andy Shevchenko Feb. 5, 2018, 2:15 p.m. UTC | #2

On Mon, 2018-02-05 at 14:14 +0100, Jean Delvare wrote:

> On Wed, 31 Jan 2018 00:40:05 -0800, Alex Hung wrote:
> > A number of Dell systems require an OEM _OSI string "Linux-Dell-
> > Video" as
> > a BIOS workaround for a system hang bug caused by discrete VGA. The
> > form of
> > the OEM _OSI string is discussed in Documentation/acpi/osi.txt and
> > is
> > defined by each OEM.
> 
> I admit I don't understand how it is the operating system's job to
> carry the information from the BIOS to the BIOS.

> > +	for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
> > +		osi = &osi_setup_entries[i];
> > +		if (!strcmp(osi->string, str)) {
> 
> This can only happen if the user passes acpi_osi=Linux-Dell-Video or
> acpi_osi=!Linux-Dell-Video on the boot command line, right?
> 
> > +			osi->enable = true;
> 
> Does this not prevent the user from explicitly disabling it with
> acpi_osi=!Linux-Dell-Video ?

Playing with OSI string is a bad idea. I wouldn't do anything while
Rafael, or even Len can confirm that is the right thing to do.

For me, AFAIK we need to be bug-to-bug compatible with Windows (at least
on ACPICA side), so, what Windows exactly does on such laptops?

Limonciello, Mario Feb. 5, 2018, 5:24 p.m. UTC | #3

> -----Original Message-----
> From: Jean Delvare [mailto:[e-mail]]
> Sent: Monday, February 5, 2018 7:15 AM
> To: Alex Hung <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail]; linux-
> [e-mail]; Limonciello, Mario <[e-mail]>
> Subject: Re: [PATCH 2/2] ACPI / osi: add DMI quirk for Dell systems
> 
> Hi Alex,
> 
> On Wed, 31 Jan 2018 00:40:05 -0800, Alex Hung wrote:
> > A number of Dell systems require an OEM _OSI string "Linux-Dell-Video" as
> > a BIOS workaround for a system hang bug caused by discrete VGA. The form of
> > the OEM _OSI string is discussed in Documentation/acpi/osi.txt and is
> > defined by each OEM.
> 
> I admit I don't understand how it is the operating system's job to
> carry the information from the BIOS to the BIOS.

The reason it's done this way is so that Linux kernel can revert this behavior when it's
no longer appropriate without system vendor having to issue a BIOS update to do this.

For example if underlying issue that required this BIOS ASL workaround is fixed in a
newer kernel.

You can read the documentation about this in Documentations/acpi/osi.txt.

> 
> > Signed-off-by: Alex Hung <[e-mail]>
> > ---
> >  drivers/acpi/osi.c | 107
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 107 insertions(+)
> >
> > diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
> > index 76998a5..43e4349 100644
> > --- a/drivers/acpi/osi.c
> > +++ b/drivers/acpi/osi.c
> > @@ -477,6 +477,112 @@ static const struct dmi_system_id
> acpi_osi_dmi_table[] __initconst = {
> >  	{}
> >  };
> >
> > +static int __init dmi_oem_osi_add(const struct dmi_system_id *d)
> > +{
> > +	struct acpi_osi_entry *osi;
> > +	const char *str = d->driver_data;
> > +	int i;
> > +
> > +	for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
> > +		osi = &osi_setup_entries[i];
> > +		if (!strcmp(osi->string, str)) {
> 
> This can only happen if the user passes acpi_osi=Linux-Dell-Video or
> acpi_osi=!Linux-Dell-Video on the boot command line, right?
> 
> > +			osi->enable = true;
> 
> Does this not prevent the user from explicitly disabling it with
> acpi_osi=!Linux-Dell-Video ?
> 
> > +			continue;
> 
> Are you not done at this point? I think you want to break, not
> continue, else you may add a duplicate Linux-Dell-Video entry below.

You might have two different entries that apply to the same system in the future.

At least the way that Dell is intending to use this is that "Linux-Dell-Video" would
only apply to ASL related to video configuration.

If for example there is later an issue with audio on a different platform that also
needed "Linux-Dell-Video", the ASL to do the audio configuration would be activated
by "Linux-Dell-Audio".

> 
> > +		} else if (osi->string[0] == '\0') {
> > +			osi->enable = true;
> > +			strncpy(osi->string, str, OSI_STRING_LENGTH_MAX);
> > +			break;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct dmi_system_id acpi_oem_osi_dmi_table[] __initconst = {
> > +	{
> > +	.callback = dmi_oem_osi_add,
> > +	.ident = "Dell Latitude 5491",
> > +	.matches = {
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0818]"),
> > +		},
> > +	.driver_data = "Linux-Dell-Video",
> > +	},
> > +	{
> > +	.callback = dmi_oem_osi_add,
> > +	.ident = "Dell Latitude 5591",
> > +	.matches = {
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0819]"),
> > +		},
> > +	.driver_data = "Linux-Dell-Video",
> > +	},
> > +	{
> > +	.callback = dmi_oem_osi_add,
> > +	.ident = "Dell Precision 3530",
> > +	.matches = {
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0820]"),
> > +		},
> > +	.driver_data = "Linux-Dell-Video",
> > +	},
> > +	{
> > +	.callback = dmi_oem_osi_add,
> > +	.ident = "Dell Inspiron 7777 AIO",
> > +	.matches = {
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0850]"),
> > +		},
> > +	.driver_data = "Linux-Dell-Video",
> > +	},
> > +	{
> > +	.callback = dmi_oem_osi_add,
> > +	.ident = "Dell Inspiron 5477 AIO",
> > +	.matches = {
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0851]"),
> > +		},
> > +	.driver_data = "Linux-Dell-Video",
> > +	},
> > +	{
> > +	.callback = dmi_oem_osi_add,
> > +	.ident = "Dell G5 5779",
> > +	.matches = {
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0886]"),
> > +		},
> > +	.driver_data = "Linux-Dell-Video",
> > +	},
> > +	{
> > +	.callback = dmi_oem_osi_add,
> > +	.ident = "Dell G5 5779",
> > +	.matches = {
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0870]"),
> > +		},
> > +	.driver_data = "Linux-Dell-Video",
> > +	},
> > +	{
> > +	.callback = dmi_oem_osi_add,
> > +	.ident = "Dell G5 5579",
> > +	.matches = {
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[086F]"),
> > +		},
> > +	.driver_data = "Linux-Dell-Video",
> > +	},
> > +	{
> > +	.callback = dmi_oem_osi_add,
> > +	.ident = "Dell G5 5579",
> > +	.matches = {
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
> > +		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0885]"),
> > +		},
> > +	.driver_data = "Linux-Dell-Video",
> > +	},
> > +	{}
> > +};
> 
> G5 5779 and G5 5579 appear twice each, are these copy-and-paste errors?

No, there are two different PCBs that can be used in these systems.  They share
same "Product Name".  You can only differentiate between them via OEM string.

> 
> Choosing a sort rule and sticking to it would make it easier to add
> entries later with not risk of duplicates.
> 
> > +
> >  static __init void acpi_osi_dmi_blacklisted(void)
> >  {
> >  	dmi_check_system(acpi_osi_dmi_table);
> > @@ -496,6 +602,7 @@ int __init early_acpi_osi_init(void)
> >  int __init acpi_osi_init(void)
> >  {
> >  	acpi_install_interface_handler(acpi_osi_handler);
> > +	dmi_check_system(acpi_oem_osi_dmi_table);
> >  	acpi_osi_setup_late();
> >
> >  	return 0;
> 
> 
> --
> Jean Delvare
> SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Limonciello, Mario Feb. 5, 2018, 5:36 p.m. UTC | #4

> -----Original Message-----

> From: Andy Shevchenko [mailto:[e-mail]]

> Sent: Monday, February 5, 2018 8:15 AM

> To: Jean Delvare <[e-mail]>; Alex Hung <[e-mail]>

> Cc: [e-mail]; [e-mail]; [e-mail];

> [e-mail]; [e-mail]; [e-mail];

> [e-mail]; [e-mail]; [e-mail]; linux-

> [e-mail]; Limonciello, Mario <[e-mail]>

> Subject: Re: [PATCH 2/2] ACPI / osi: add DMI quirk for Dell systems

> 

> On Mon, 2018-02-05 at 14:14 +0100, Jean Delvare wrote:

> 

> > On Wed, 31 Jan 2018 00:40:05 -0800, Alex Hung wrote:

> > > A number of Dell systems require an OEM _OSI string "Linux-Dell-

> > > Video" as

> > > a BIOS workaround for a system hang bug caused by discrete VGA. The

> > > form of

> > > the OEM _OSI string is discussed in Documentation/acpi/osi.txt and

> > > is

> > > defined by each OEM.

> >

> > I admit I don't understand how it is the operating system's job to

> > carry the information from the BIOS to the BIOS.

> 

> > > +	for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {

> > > +		osi = &osi_setup_entries[i];

> > > +		if (!strcmp(osi->string, str)) {

> >

> > This can only happen if the user passes acpi_osi=Linux-Dell-Video or

> > acpi_osi=!Linux-Dell-Video on the boot command line, right?

> >

> > > +			osi->enable = true;

> >

> > Does this not prevent the user from explicitly disabling it with

> > acpi_osi=!Linux-Dell-Video ?

> 

> Playing with OSI string is a bad idea. I wouldn't do anything while

> Rafael, or even Len can confirm that is the right thing to do.

> 

> For me, AFAIK we need to be bug-to-bug compatible with Windows (at least

> on ACPICA side), so, what Windows exactly does on such laptops?

> 


The issue that's being worked around isn't an ACPICA interpreter issue, but it's 
a graphics device configuration issue.

Windows expects to use RTD3 on the NVIDIA GPU but Linux drivers
don't.  It leads to system hangs on the Linux side.

Jean Delvare Feb. 5, 2018, 9:55 p.m. UTC | #5

On Mon, 5 Feb 2018 17:24:43 +0000, [e-mail] wrote:
> > > +static int __init dmi_oem_osi_add(const struct dmi_system_id *d)
> > > +{
> > > +	struct acpi_osi_entry *osi;
> > > +	const char *str = d->driver_data;
> > > +	int i;
> > > +
> > > +	for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
> > > +		osi = &osi_setup_entries[i];
> > > +		if (!strcmp(osi->string, str)) {  
> > 
> > This can only happen if the user passes acpi_osi=Linux-Dell-Video or
> > acpi_osi=!Linux-Dell-Video on the boot command line, right?
> >   
> > > +			osi->enable = true;  
> > 
> > Does this not prevent the user from explicitly disabling it with
> > acpi_osi=!Linux-Dell-Video ?
> >   
> > > +			continue;  
> > 
> > Are you not done at this point? I think you want to break, not
> > continue, else you may add a duplicate Linux-Dell-Video entry below.  
> 
> You might have two different entries that apply to the same system in the future.
> 
> At least the way that Dell is intending to use this is that "Linux-Dell-Video" would
> only apply to ASL related to video configuration.
> 
> If for example there is later an issue with audio on a different platform that also
> needed "Linux-Dell-Video", the ASL to do the audio configuration would be activated
> by "Linux-Dell-Audio".

I understand what you say, but can't see how this relates to my
concerns above.

Dmitry Torokhov Feb. 5, 2018, 10:45 p.m. UTC | #6

On Mon, Feb 05, 2018 at 05:36:18PM +0000, [e-mail] wrote:
> > -----Original Message-----
> > From: Andy Shevchenko [mailto:[e-mail]]
> > Sent: Monday, February 5, 2018 8:15 AM
> > To: Jean Delvare <[e-mail]>; Alex Hung <[e-mail]>
> > Cc: [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail]; linux-
> > [e-mail]; Limonciello, Mario <[e-mail]>
> > Subject: Re: [PATCH 2/2] ACPI / osi: add DMI quirk for Dell systems
> > 
> > On Mon, 2018-02-05 at 14:14 +0100, Jean Delvare wrote:
> > 
> > > On Wed, 31 Jan 2018 00:40:05 -0800, Alex Hung wrote:
> > > > A number of Dell systems require an OEM _OSI string "Linux-Dell-
> > > > Video" as
> > > > a BIOS workaround for a system hang bug caused by discrete VGA. The
> > > > form of
> > > > the OEM _OSI string is discussed in Documentation/acpi/osi.txt and
> > > > is
> > > > defined by each OEM.
> > >
> > > I admit I don't understand how it is the operating system's job to
> > > carry the information from the BIOS to the BIOS.
> > 
> > > > +	for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
> > > > +		osi = &osi_setup_entries[i];
> > > > +		if (!strcmp(osi->string, str)) {
> > >
> > > This can only happen if the user passes acpi_osi=Linux-Dell-Video or
> > > acpi_osi=!Linux-Dell-Video on the boot command line, right?
> > >
> > > > +			osi->enable = true;
> > >
> > > Does this not prevent the user from explicitly disabling it with
> > > acpi_osi=!Linux-Dell-Video ?
> > 
> > Playing with OSI string is a bad idea. I wouldn't do anything while
> > Rafael, or even Len can confirm that is the right thing to do.
> > 
> > For me, AFAIK we need to be bug-to-bug compatible with Windows (at least
> > on ACPICA side), so, what Windows exactly does on such laptops?
> > 
> 
> The issue that's being worked around isn't an ACPICA interpreter issue, but it's 
> a graphics device configuration issue.
> 
> Windows expects to use RTD3 on the NVIDIA GPU but Linux drivers
> don't.  It leads to system hangs on the Linux side.

Can we adjust Linux drivers to do the right thing? Or is it regarding
the binary NVIDIA blob?

Thanks.

Limonciello, Mario Feb. 6, 2018, 12:45 a.m. UTC | #7

> -----Original Message-----
> From: Dmitry Torokhov [mailto:[e-mail]]
> Sent: Monday, February 5, 2018 4:46 PM
> To: Limonciello, Mario <[e-mail]>
> Cc: [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]
> Subject: Re: [PATCH 2/2] ACPI / osi: add DMI quirk for Dell systems
> 
> On Mon, Feb 05, 2018 at 05:36:18PM +0000, [e-mail] wrote:
> > > -----Original Message-----
> > > From: Andy Shevchenko [mailto:[e-mail]]
> > > Sent: Monday, February 5, 2018 8:15 AM
> > > To: Jean Delvare <[e-mail]>; Alex Hung <[e-mail]>
> > > Cc: [e-mail]; [e-mail]; [e-mail];
> > > [e-mail]; [e-mail];
> [e-mail];
> > > [e-mail]; [e-mail]; [e-mail];
> linux-
> > > [e-mail]; Limonciello, Mario <[e-mail]>
> > > Subject: Re: [PATCH 2/2] ACPI / osi: add DMI quirk for Dell systems
> > >
> > > On Mon, 2018-02-05 at 14:14 +0100, Jean Delvare wrote:
> > >
> > > > On Wed, 31 Jan 2018 00:40:05 -0800, Alex Hung wrote:
> > > > > A number of Dell systems require an OEM _OSI string "Linux-Dell-
> > > > > Video" as
> > > > > a BIOS workaround for a system hang bug caused by discrete VGA. The
> > > > > form of
> > > > > the OEM _OSI string is discussed in Documentation/acpi/osi.txt and
> > > > > is
> > > > > defined by each OEM.
> > > >
> > > > I admit I don't understand how it is the operating system's job to
> > > > carry the information from the BIOS to the BIOS.
> > >
> > > > > +	for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
> > > > > +		osi = &osi_setup_entries[i];
> > > > > +		if (!strcmp(osi->string, str)) {
> > > >
> > > > This can only happen if the user passes acpi_osi=Linux-Dell-Video or
> > > > acpi_osi=!Linux-Dell-Video on the boot command line, right?
> > > >
> > > > > +			osi->enable = true;
> > > >
> > > > Does this not prevent the user from explicitly disabling it with
> > > > acpi_osi=!Linux-Dell-Video ?
> > >
> > > Playing with OSI string is a bad idea. I wouldn't do anything while
> > > Rafael, or even Len can confirm that is the right thing to do.
> > >
> > > For me, AFAIK we need to be bug-to-bug compatible with Windows (at least
> > > on ACPICA side), so, what Windows exactly does on such laptops?
> > >
> >
> > The issue that's being worked around isn't an ACPICA interpreter issue, but it's
> > a graphics device configuration issue.
> >
> > Windows expects to use RTD3 on the NVIDIA GPU but Linux drivers
> > don't.  It leads to system hangs on the Linux side.
> 
> Can we adjust Linux drivers to do the right thing? Or is it regarding
> the binary NVIDIA blob?
> 

Neither Nouveau nor the NVIDIA blob have support for RTD3.

Last I heard it's waiting on NVIDIA releasing something Nouveau
needs.  So.. Eventually?  For now it's better to not hang though.

That's part of why we wanted to enable this via a transient OSI string,
to let this be removed by Linux whenever the driver does grow support.

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Andy Shevchenko Feb. 6, 2018, 1:45 p.m. UTC | #8

On Tue, 2018-02-06 at 00:45 +0000, [e-mail] wrote:

> > > > Playing with OSI string is a bad idea. I wouldn't do anything
> > > > while
> > > > Rafael, or even Len can confirm that is the right thing to do.
> > > > 
> > > > For me, AFAIK we need to be bug-to-bug compatible with Windows
> > > > (at least
> > > > on ACPICA side), so, what Windows exactly does on such laptops?
> > > > 
> > > 
> > > The issue that's being worked around isn't an ACPICA interpreter
> > > issue, but it's
> > > a graphics device configuration issue.

Then clearly nothing to do with OSI strings here, right?

> > > Windows expects to use RTD3 on the NVIDIA GPU but Linux drivers
> > > don't.  It leads to system hangs on the Linux side.
> > 
> > Can we adjust Linux drivers to do the right thing?

+100

> >  Or is it regarding
> > the binary NVIDIA blob?

nVidia vs. Linux again? :-)

> 
> Neither Nouveau nor the NVIDIA blob have support for RTD3.
> 
> Last I heard it's waiting on NVIDIA releasing something Nouveau
> needs.  So.. Eventually?  For now it's better to not hang though.

Hmm... While you are talking sense, the patch itself looks like an ugly
hack.

> That's part of why we wanted to enable this via a transient OSI
> string,
> to let this be removed by Linux whenever the driver does grow support.

So, means "never" then? (Assume a bit of irony here)

I don't know how it feels for maintainers, for me it's quite unlikely to
go (at least in this shape).

I'm sorry I can't be much constructive here, I heard Len once about OSI
huge abuse by almost every party. I would rather let him speak on the
matter.

Limonciello, Mario Feb. 6, 2018, 4:24 p.m. UTC | #9

> -----Original Message-----

> From: Andy Shevchenko [mailto:[e-mail]]

> Sent: Tuesday, February 6, 2018 7:46 AM

> To: Limonciello, Mario <[e-mail]>;

> [e-mail]

> Cc: [e-mail]; [e-mail]; [e-mail];

> [e-mail]; [e-mail]; [e-mail];

> [e-mail]; [e-mail]; [e-mail];

> [e-mail]; [e-mail]

> Subject: Re: [PATCH 2/2] ACPI / osi: add DMI quirk for Dell systems

> 

> On Tue, 2018-02-06 at 00:45 +0000, [e-mail] wrote:

> 

> > > > > Playing with OSI string is a bad idea. I wouldn't do anything

> > > > > while

> > > > > Rafael, or even Len can confirm that is the right thing to do.

> > > > >

> > > > > For me, AFAIK we need to be bug-to-bug compatible with Windows

> > > > > (at least

> > > > > on ACPICA side), so, what Windows exactly does on such laptops?

> > > > >

> > > >

> > > > The issue that's being worked around isn't an ACPICA interpreter

> > > > issue, but it's

> > > > a graphics device configuration issue.

> 

> Then clearly nothing to do with OSI strings here, right?


The usage of OSI here is: do you support the feature "Linux-Dell-Video"
to Linux kernel.  If Linux kernel responds yes then firmware will turn off 
RTD3 functionality for the discrete GPU.

> 

> > > > Windows expects to use RTD3 on the NVIDIA GPU but Linux drivers

> > > > don't.  It leads to system hangs on the Linux side.

> > >

> > > Can we adjust Linux drivers to do the right thing?

> 

> +100

> 

> > >  Or is it regarding

> > > the binary NVIDIA blob?

> 

> nVidia vs. Linux again? :-)

> 

> >

> > Neither Nouveau nor the NVIDIA blob have support for RTD3.

> >

> > Last I heard it's waiting on NVIDIA releasing something Nouveau

> > needs.  So.. Eventually?  For now it's better to not hang though.

> 

> Hmm... While you are talking sense, the patch itself looks like an ugly

> hack.


I don't disagree it's a workaround for a driver deficiency.

> 

> > That's part of why we wanted to enable this via a transient OSI

> > string,

> > to let this be removed by Linux whenever the driver does grow support.

> 

> So, means "never" then? (Assume a bit of irony here)

> 

> I don't know how it feels for maintainers, for me it's quite unlikely to

> go (at least in this shape).


I'm not going to be able to force NVIDIA to fix the blob or to give what's
necessary to enable Nouvaeu for this function.  What we can do is prevent
someone's system from hanging because of this situation.

> 

> I'm sorry I can't be much constructive here, I heard Len once about OSI

> huge abuse by almost every party. I would rather let him speak on the

> matter.


I know that OSI has been abused in the past, but that's exactly why this
has been implemented as seen in this patch series.

It's done specifically to fit within exactly what the Linux kernel ideally
looks for in a custom OSI feature string.

* It has an OS Prefix (Linux-)
* It's an OEM specific string (-Dell-)
* It's a feature specific string (-Video)
*It's specific to a single platform (not a blanket match to Dell Inc)
*It only modifies one feature in ASL

Alex Hung Feb. 7, 2018, 8:05 p.m. UTC | #10

On Mon, Feb 5, 2018 at 5:14 AM, Jean Delvare <[e-mail]> wrote:
> Hi Alex,
>
> On Wed, 31 Jan 2018 00:40:05 -0800, Alex Hung wrote:
>> A number of Dell systems require an OEM _OSI string "Linux-Dell-Video" as
>> a BIOS workaround for a system hang bug caused by discrete VGA. The form of
>> the OEM _OSI string is discussed in Documentation/acpi/osi.txt and is
>> defined by each OEM.
>
> I admit I don't understand how it is the operating system's job to
> carry the information from the BIOS to the BIOS.
>
>> Signed-off-by: Alex Hung <[e-mail]>
>> ---
>>  drivers/acpi/osi.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 107 insertions(+)
>>
>> diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
>> index 76998a5..43e4349 100644
>> --- a/drivers/acpi/osi.c
>> +++ b/drivers/acpi/osi.c
>> @@ -477,6 +477,112 @@ static const struct dmi_system_id acpi_osi_dmi_table[] __initconst = {
>>       {}
>>  };
>>
>> +static int __init dmi_oem_osi_add(const struct dmi_system_id *d)
>> +{
>> +     struct acpi_osi_entry *osi;
>> +     const char *str = d->driver_data;
>> +     int i;
>> +
>> +     for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
>> +             osi = &osi_setup_entries[i];
>> +             if (!strcmp(osi->string, str)) {
>
> This can only happen if the user passes acpi_osi=Linux-Dell-Video or
> acpi_osi=!Linux-Dell-Video on the boot command line, right?
>
>> +                     osi->enable = true;
>
> Does this not prevent the user from explicitly disabling it with
> acpi_osi=!Linux-Dell-Video ?

Thanks for pointing this out.

This can be fixed by placing
"dmi_check_system(acpi_oem_osi_dmi_table)" call in
"early_acpi_osi_init" to fix this, as "osi_setup" used by the kernel
parameter acpi_osi is executed after early_acpi_osi_init. It is also
where other quirks are initialised too.

>
>> +                     continue;
>
> Are you not done at this point? I think you want to break, not
> continue, else you may add a duplicate Linux-Dell-Video entry below.

Thanks. "break" should be used here.

>
>> +             } else if (osi->string[0] == '\0') {
>> +                     osi->enable = true;
>> +                     strncpy(osi->string, str, OSI_STRING_LENGTH_MAX);
>> +                     break;
>> +             }
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct dmi_system_id acpi_oem_osi_dmi_table[] __initconst = {
>> +     {
>> +     .callback = dmi_oem_osi_add,
>> +     .ident = "Dell Latitude 5491",
>> +     .matches = {
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0818]"),
>> +             },
>> +     .driver_data = "Linux-Dell-Video",
>> +     },
>> +     {
>> +     .callback = dmi_oem_osi_add,
>> +     .ident = "Dell Latitude 5591",
>> +     .matches = {
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0819]"),
>> +             },
>> +     .driver_data = "Linux-Dell-Video",
>> +     },
>> +     {
>> +     .callback = dmi_oem_osi_add,
>> +     .ident = "Dell Precision 3530",
>> +     .matches = {
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0820]"),
>> +             },
>> +     .driver_data = "Linux-Dell-Video",
>> +     },
>> +     {
>> +     .callback = dmi_oem_osi_add,
>> +     .ident = "Dell Inspiron 7777 AIO",
>> +     .matches = {
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0850]"),
>> +             },
>> +     .driver_data = "Linux-Dell-Video",
>> +     },
>> +     {
>> +     .callback = dmi_oem_osi_add,
>> +     .ident = "Dell Inspiron 5477 AIO",
>> +     .matches = {
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0851]"),
>> +             },
>> +     .driver_data = "Linux-Dell-Video",
>> +     },
>> +     {
>> +     .callback = dmi_oem_osi_add,
>> +     .ident = "Dell G5 5779",
>> +     .matches = {
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0886]"),
>> +             },
>> +     .driver_data = "Linux-Dell-Video",
>> +     },
>> +     {
>> +     .callback = dmi_oem_osi_add,
>> +     .ident = "Dell G5 5779",
>> +     .matches = {
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0870]"),
>> +             },
>> +     .driver_data = "Linux-Dell-Video",
>> +     },
>> +     {
>> +     .callback = dmi_oem_osi_add,
>> +     .ident = "Dell G5 5579",
>> +     .matches = {
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "1[086F]"),
>> +             },
>> +     .driver_data = "Linux-Dell-Video",
>> +     },
>> +     {
>> +     .callback = dmi_oem_osi_add,
>> +     .ident = "Dell G5 5579",
>> +     .matches = {
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
>> +                  DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0885]"),
>> +             },
>> +     .driver_data = "Linux-Dell-Video",
>> +     },
>> +     {}
>> +};
>
> G5 5779 and G5 5579 appear twice each, are these copy-and-paste errors?
>
> Choosing a sort rule and sticking to it would make it easier to add
> entries later with not risk of duplicates.

I was just as surprised, but they are indeed two different systems, as
pointed out by Mario.
>
>> +
>>  static __init void acpi_osi_dmi_blacklisted(void)
>>  {
>>       dmi_check_system(acpi_osi_dmi_table);
>> @@ -496,6 +602,7 @@ int __init early_acpi_osi_init(void)
>>  int __init acpi_osi_init(void)
>>  {
>>       acpi_install_interface_handler(acpi_osi_handler);
>> +     dmi_check_system(acpi_oem_osi_dmi_table);
>>       acpi_osi_setup_late();
>>
>>       return 0;
>
>
> --
> Jean Delvare
> SUSE L3 Support

Alex Hung Feb. 7, 2018, 8:38 p.m. UTC | #11

On Tue, Feb 6, 2018 at 8:24 AM,  <[e-mail]> wrote:
>> -----Original Message-----
>> From: Andy Shevchenko [mailto:[e-mail]]
>> Sent: Tuesday, February 6, 2018 7:46 AM
>> To: Limonciello, Mario <[e-mail]>;
>> [e-mail]
>> Cc: [e-mail]; [e-mail]; [e-mail];
>> [e-mail]; [e-mail]; [e-mail];
>> [e-mail]; [e-mail]; [e-mail];
>> [e-mail]; [e-mail]
>> Subject: Re: [PATCH 2/2] ACPI / osi: add DMI quirk for Dell systems
>>
>> On Tue, 2018-02-06 at 00:45 +0000, [e-mail] wrote:
>>
>> > > > > Playing with OSI string is a bad idea. I wouldn't do anything
>> > > > > while
>> > > > > Rafael, or even Len can confirm that is the right thing to do.
>> > > > >
>> > > > > For me, AFAIK we need to be bug-to-bug compatible with Windows
>> > > > > (at least
>> > > > > on ACPICA side), so, what Windows exactly does on such laptops?
>> > > > >
>> > > >
>> > > > The issue that's being worked around isn't an ACPICA interpreter
>> > > > issue, but it's
>> > > > a graphics device configuration issue.
>>
>> Then clearly nothing to do with OSI strings here, right?
>
> The usage of OSI here is: do you support the feature "Linux-Dell-Video"
> to Linux kernel.  If Linux kernel responds yes then firmware will turn off
> RTD3 functionality for the discrete GPU.
>
>>
>> > > > Windows expects to use RTD3 on the NVIDIA GPU but Linux drivers
>> > > > don't.  It leads to system hangs on the Linux side.
>> > >
>> > > Can we adjust Linux drivers to do the right thing?
>>
>> +100
>>
>> > >  Or is it regarding
>> > > the binary NVIDIA blob?
>>
>> nVidia vs. Linux again? :-)
>>
>> >
>> > Neither Nouveau nor the NVIDIA blob have support for RTD3.
>> >
>> > Last I heard it's waiting on NVIDIA releasing something Nouveau
>> > needs.  So.. Eventually?  For now it's better to not hang though.
>>
>> Hmm... While you are talking sense, the patch itself looks like an ugly
>> hack.
>
> I don't disagree it's a workaround for a driver deficiency.
>
>>
>> > That's part of why we wanted to enable this via a transient OSI
>> > string,
>> > to let this be removed by Linux whenever the driver does grow support.
>>
>> So, means "never" then? (Assume a bit of irony here)
>>
>> I don't know how it feels for maintainers, for me it's quite unlikely to
>> go (at least in this shape).

Workarounds are never preferred, but they are needed from time to time.

There are other alternatives such as 1) _REV & acpi_rev_dmi_table, 2)
windows & linux osi strings & acpi_osi_dmi_table.

However, Linux-OEM-my_interface_name strings + OEM strings in DMI are
better choices for couple reasons:

1. Linux-OEM-my_interface_name are more informative than current
available mechanisms.

2. Linux-OEM-my_interface_name is defined in
Documentation/acpi/osi.txt as below:

_OSI("Linux-OEM-my_interface_name")
where 'OEM' is needed if this is an OEM-specific hook,
and 'my_interface_name' describes the hook, which could be a
quirk, a bug, or a bug-fix.

3. DMI OEM strings used in this patch are System IDs and they are
unlikely to repeat for years. System IDs are also more precise than
Product names.

4. The workaround is controlled and limited to specific systems as
Mario commented previously.

>
> I'm not going to be able to force NVIDIA to fix the blob or to give what's
> necessary to enable Nouvaeu for this function.  What we can do is prevent
> someone's system from hanging because of this situation.

and we never stopped poking NVIDIA to fix drivers and will never stop
trying. If NVIDIA fixes RTD3, a clean-up patch will be submit to
remove the quirks.

>
>>
>> I'm sorry I can't be much constructive here, I heard Len once about OSI
>> huge abuse by almost every party. I would rather let him speak on the
>> matter.
>
> I know that OSI has been abused in the past, but that's exactly why this
> has been implemented as seen in this patch series.
>
> It's done specifically to fit within exactly what the Linux kernel ideally
> looks for in a custom OSI feature string.
>
> * It has an OS Prefix (Linux-)
> * It's an OEM specific string (-Dell-)
> * It's a feature specific string (-Video)
> *It's specific to a single platform (not a blanket match to Dell Inc)
> *It only modifies one feature in ASL
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Limonciello, Mario Feb. 7, 2018, 8:49 p.m. UTC | #12

> -----Original Message-----

> From: Alex Hung [mailto:[e-mail]]

> Sent: Wednesday, February 7, 2018 2:39 PM

> To: Limonciello, Mario <[e-mail]>

> Cc: Andy Shevchenko <[e-mail]>; Dmitry Torokhov

> <[e-mail]>; Jean Delvare <[e-mail]>; Rafael J.

> Wysocki <[e-mail]>; Len Brown <[e-mail]>;

> [e-mail]; [e-mail];

> [e-mail]; Florian Fainelli <[e-mail]>;

> [e-mail]; sayli karnik <[e-mail]>; Linux ACPI Mailing List

> <[e-mail]>

> Subject: Re: [PATCH 2/2] ACPI / osi: add DMI quirk for Dell systems

> 

> On Tue, Feb 6, 2018 at 8:24 AM,  <[e-mail]> wrote:

> >> -----Original Message-----

> >> From: Andy Shevchenko [mailto:[e-mail]]

> >> Sent: Tuesday, February 6, 2018 7:46 AM

> >> To: Limonciello, Mario <[e-mail]>;

> >> [e-mail]

> >> Cc: [e-mail]; [e-mail]; [e-mail];

> >> [e-mail]; [e-mail]; [e-mail];

> >> [e-mail]; [e-mail]; [e-mail];

> >> [e-mail]; [e-mail]

> >> Subject: Re: [PATCH 2/2] ACPI / osi: add DMI quirk for Dell systems

> >>

> >> On Tue, 2018-02-06 at 00:45 +0000, [e-mail] wrote:

> >>

> >> > > > > Playing with OSI string is a bad idea. I wouldn't do anything

> >> > > > > while

> >> > > > > Rafael, or even Len can confirm that is the right thing to do.

> >> > > > >

> >> > > > > For me, AFAIK we need to be bug-to-bug compatible with Windows

> >> > > > > (at least

> >> > > > > on ACPICA side), so, what Windows exactly does on such laptops?

> >> > > > >

> >> > > >

> >> > > > The issue that's being worked around isn't an ACPICA interpreter

> >> > > > issue, but it's

> >> > > > a graphics device configuration issue.

> >>

> >> Then clearly nothing to do with OSI strings here, right?

> >

> > The usage of OSI here is: do you support the feature "Linux-Dell-Video"

> > to Linux kernel.  If Linux kernel responds yes then firmware will turn off

> > RTD3 functionality for the discrete GPU.

> >

> >>

> >> > > > Windows expects to use RTD3 on the NVIDIA GPU but Linux drivers

> >> > > > don't.  It leads to system hangs on the Linux side.

> >> > >

> >> > > Can we adjust Linux drivers to do the right thing?

> >>

> >> +100

> >>

> >> > >  Or is it regarding

> >> > > the binary NVIDIA blob?

> >>

> >> nVidia vs. Linux again? :-)

> >>

> >> >

> >> > Neither Nouveau nor the NVIDIA blob have support for RTD3.

> >> >

> >> > Last I heard it's waiting on NVIDIA releasing something Nouveau

> >> > needs.  So.. Eventually?  For now it's better to not hang though.

> >>

> >> Hmm... While you are talking sense, the patch itself looks like an ugly

> >> hack.

> >

> > I don't disagree it's a workaround for a driver deficiency.

> >

> >>

> >> > That's part of why we wanted to enable this via a transient OSI

> >> > string,

> >> > to let this be removed by Linux whenever the driver does grow support.

> >>

> >> So, means "never" then? (Assume a bit of irony here)

> >>

> >> I don't know how it feels for maintainers, for me it's quite unlikely to

> >> go (at least in this shape).

> 

> Workarounds are never preferred, but they are needed from time to time.

> 

> There are other alternatives such as 1) _REV & acpi_rev_dmi_table, 2)

> windows & linux osi strings & acpi_osi_dmi_table.

> 

> However, Linux-OEM-my_interface_name strings + OEM strings in DMI are

> better choices for couple reasons:

> 

> 1. Linux-OEM-my_interface_name are more informative than current

> available mechanisms.

> 

> 2. Linux-OEM-my_interface_name is defined in

> Documentation/acpi/osi.txt as below:

> 

> _OSI("Linux-OEM-my_interface_name")

> where 'OEM' is needed if this is an OEM-specific hook,

> and 'my_interface_name' describes the hook, which could be a

> quirk, a bug, or a bug-fix.

> 

> 3. DMI OEM strings used in this patch are System IDs and they are

> unlikely to repeat for years. System IDs are also more precise than

> Product names.


Actually for Dell they're guaranteed to always be unique and will 
never repeat on another platform.

> 

> 4. The workaround is controlled and limited to specific systems as

> Mario commented previously.

> 

> >

> > I'm not going to be able to force NVIDIA to fix the blob or to give what's

> > necessary to enable Nouvaeu for this function.  What we can do is prevent

> > someone's system from hanging because of this situation.

> 

> and we never stopped poking NVIDIA to fix drivers and will never stop

> trying. If NVIDIA fixes RTD3, a clean-up patch will be submit to

> remove the quirks.

> 

> >

> >>

> >> I'm sorry I can't be much constructive here, I heard Len once about OSI

> >> huge abuse by almost every party. I would rather let him speak on the

> >> matter.

> >

> > I know that OSI has been abused in the past, but that's exactly why this

> > has been implemented as seen in this patch series.

> >

> > It's done specifically to fit within exactly what the Linux kernel ideally

> > looks for in a custom OSI feature string.

> >

> > * It has an OS Prefix (Linux-)

> > * It's an OEM specific string (-Dell-)

> > * It's a feature specific string (-Video)

> > *It's specific to a single platform (not a blanket match to Dell Inc)

> > *It only modifies one feature in ASL

> >

> >

Rafael J. Wysocki Feb. 11, 2018, 9:29 a.m. UTC | #13

On Tue, Feb 6, 2018 at 2:45 PM, Andy Shevchenko
<[e-mail]> wrote:
> On Tue, 2018-02-06 at 00:45 +0000, [e-mail] wrote:
>
>> > > > Playing with OSI string is a bad idea. I wouldn't do anything
>> > > > while
>> > > > Rafael, or even Len can confirm that is the right thing to do.
>> > > >
>> > > > For me, AFAIK we need to be bug-to-bug compatible with Windows
>> > > > (at least
>> > > > on ACPICA side), so, what Windows exactly does on such laptops?
>> > > >
>> > >
>> > > The issue that's being worked around isn't an ACPICA interpreter
>> > > issue, but it's
>> > > a graphics device configuration issue.
>
> Then clearly nothing to do with OSI strings here, right?
>
>> > > Windows expects to use RTD3 on the NVIDIA GPU but Linux drivers
>> > > don't.  It leads to system hangs on the Linux side.
>> >
>> > Can we adjust Linux drivers to do the right thing?
>
> +100

IMO you should only say so if *you* are willing to do the work and
have all of the requisite means for that.

>> >  Or is it regarding
>> > the binary NVIDIA blob?
>
> nVidia vs. Linux again? :-)
>
>>
>> Neither Nouveau nor the NVIDIA blob have support for RTD3.
>>
>> Last I heard it's waiting on NVIDIA releasing something Nouveau
>> needs.  So.. Eventually?  For now it's better to not hang though.
>
> Hmm... While you are talking sense, the patch itself looks like an ugly
> hack.
>
>> That's part of why we wanted to enable this via a transient OSI
>> string,
>> to let this be removed by Linux whenever the driver does grow support.
>
> So, means "never" then? (Assume a bit of irony here)
>
> I don't know how it feels for maintainers, for me it's quite unlikely to
> go (at least in this shape).
>
> I'm sorry I can't be much constructive here, I heard Len once about OSI
> huge abuse by almost every party. I would rather let him speak on the
> matter.

One thing is the merit and another thing is the patch itself.

On the merit side, with all due respect, this isn't about fixing the
drivers in question.  They aren't broken.

RTD3 (or generally runtime PM) support in Linux has been regarded as
optional so far and it is not even realistic to change this policy
ATM.  The lack of it is a matter of energy-efficiency, but not a
matter of basic correctness, as far as Linux is concerned.

If some platform firmware doesn't work correctly with an OS that
doesn't do RTD3 all over and crashes when Linux runs on it for this
reason, than that platform is simply not compatible with Linux as it
stands.

Granted, we can say "Too bad, Linux is not going to run on this
platform any time soon" (but frankly I don't see how that would
benefit anyone) or we can make the kernel give a little hint to the
platform firmware to make it behave in a Linux-compatible way.

So if we can agree that this has merit (and I personally think that it
does), let's try to make the implementation cleaner, if possible.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Lukas Wunner Feb. 11, 2018, 1:45 p.m. UTC | #14

On Tue, 2018-02-06 at 00:45 +0000, [e-mail] wrote:
> Windows expects to use RTD3 on the NVIDIA GPU but Linux drivers
> don't.  It leads to system hangs on the Linux side.

So I'm on a Mac and thus indifferent to this issue, but I happen
to know a thing or two about hybrid graphics and I'm wondering
what the claim above is supposed to mean.

RTD3, that's runtime D3, right?  Because nouveau does runtime suspend
to D3cold and has been doing so since 2013.

The commit message is extremely terse as to what exact problem
the commit is trying to solve, and I haven't seen anything more
specific in this thread other than handwaving. ("waiting on NVIDIA
releasing something Nouveau needs" -- what exactly?)

So please state clearly what the problem is and why solving it
this way is the best or only solution.

Thanks,

Lukas
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Rafael J. Wysocki Feb. 12, 2018, 9:49 a.m. UTC | #15

On Sun, Feb 11, 2018 at 2:45 PM, Lukas Wunner <[e-mail]> wrote:
> On Tue, 2018-02-06 at 00:45 +0000, [e-mail] wrote:
>> Windows expects to use RTD3 on the NVIDIA GPU but Linux drivers
>> don't.  It leads to system hangs on the Linux side.
>
> So I'm on a Mac and thus indifferent to this issue, but I happen
> to know a thing or two about hybrid graphics and I'm wondering
> what the claim above is supposed to mean.
>
> RTD3, that's runtime D3, right?  Because nouveau does runtime suspend
> to D3cold and has been doing so since 2013.
>
> The commit message is extremely terse as to what exact problem
> the commit is trying to solve, and I haven't seen anything more
> specific in this thread other than handwaving. ("waiting on NVIDIA
> releasing something Nouveau needs" -- what exactly?)
>
> So please state clearly what the problem is and why solving it
> this way is the best or only solution.

If that's not clear, I also would like to see a response to this
request before making any decisions here.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Limonciello, Mario Feb. 12, 2018, 8:29 p.m. UTC | #16

> -----Original Message-----

> From: [e-mail] [mailto:[e-mail]] On Behalf Of Rafael J.

> Wysocki

> Sent: Monday, February 12, 2018 3:50 AM

> To: Lukas Wunner <[e-mail]>; Alex Hung <[e-mail]>

> Cc: Limonciello, Mario <[e-mail]>; Rafael J. Wysocki

> <[e-mail]>; Andy Shevchenko <[e-mail]>;

> Dmitry Torokhov <[e-mail]>; Jean Delvare

> <[e-mail]>; Len Brown <[e-mail]>; Greg Kroah-Hartman

> <[e-mail]>; David Miller <[e-mail]>; Mika

> Westerberg <[e-mail]>; Florian Fainelli

> <[e-mail]>; Kishon Vijay Abraham I <[e-mail]>;

> [e-mail]; ACPI Devel Maling List <[e-mail]>

> Subject: Re: [PATCH 2/2] ACPI / osi: add DMI quirk for Dell systems

> 

> On Sun, Feb 11, 2018 at 2:45 PM, Lukas Wunner <[e-mail]> wrote:

> > On Tue, 2018-02-06 at 00:45 +0000, [e-mail] wrote:

> >> Windows expects to use RTD3 on the NVIDIA GPU but Linux drivers

> >> don't.  It leads to system hangs on the Linux side.

> >

> > So I'm on a Mac and thus indifferent to this issue, but I happen

> > to know a thing or two about hybrid graphics and I'm wondering

> > what the claim above is supposed to mean.

> >

> > RTD3, that's runtime D3, right?  Because nouveau does runtime suspend

> > to D3cold and has been doing so since 2013.

> >

> > The commit message is extremely terse as to what exact problem

> > the commit is trying to solve, and I haven't seen anything more

> > specific in this thread other than handwaving. ("waiting on NVIDIA

> > releasing something Nouveau needs" -- what exactly?)

> >

> > So please state clearly what the problem is and why solving it

> > this way is the best or only solution.

> 

> If that's not clear, I also would like to see a response to this

> request before making any decisions here.


It's a lack of proper D3hot support and GC6 support in Nouveau.

As for why this is the best way to solve it?
This has been a problem for many generations and Dell has had
various different heuristics for detecting to turn off RTD3 on NV GPU
to avoid exercising it.

The patches submitted reflect a sustainable way to resolve the
problem rather than the OEM and Linux kernel playing hide and seek
to make the hardware work well in the general purpose scenario.

Rafael J. Wysocki Feb. 12, 2018, 10:57 p.m. UTC | #17

On Mon, Feb 12, 2018 at 9:29 PM,  <[e-mail]> wrote:
>> -----Original Message-----
>> From: [e-mail] [mailto:[e-mail]] On Behalf Of Rafael J.
>> Wysocki
>> Sent: Monday, February 12, 2018 3:50 AM
>> To: Lukas Wunner <[e-mail]>; Alex Hung <[e-mail]>
>> Cc: Limonciello, Mario <[e-mail]>; Rafael J. Wysocki
>> <[e-mail]>; Andy Shevchenko <[e-mail]>;
>> Dmitry Torokhov <[e-mail]>; Jean Delvare
>> <[e-mail]>; Len Brown <[e-mail]>; Greg Kroah-Hartman
>> <[e-mail]>; David Miller <[e-mail]>; Mika
>> Westerberg <[e-mail]>; Florian Fainelli
>> <[e-mail]>; Kishon Vijay Abraham I <[e-mail]>;
>> [e-mail]; ACPI Devel Maling List <[e-mail]>
>> Subject: Re: [PATCH 2/2] ACPI / osi: add DMI quirk for Dell systems
>>
>> On Sun, Feb 11, 2018 at 2:45 PM, Lukas Wunner <[e-mail]> wrote:
>> > On Tue, 2018-02-06 at 00:45 +0000, [e-mail] wrote:
>> >> Windows expects to use RTD3 on the NVIDIA GPU but Linux drivers
>> >> don't.  It leads to system hangs on the Linux side.
>> >
>> > So I'm on a Mac and thus indifferent to this issue, but I happen
>> > to know a thing or two about hybrid graphics and I'm wondering
>> > what the claim above is supposed to mean.
>> >
>> > RTD3, that's runtime D3, right?  Because nouveau does runtime suspend
>> > to D3cold and has been doing so since 2013.
>> >
>> > The commit message is extremely terse as to what exact problem
>> > the commit is trying to solve, and I haven't seen anything more
>> > specific in this thread other than handwaving. ("waiting on NVIDIA
>> > releasing something Nouveau needs" -- what exactly?)
>> >
>> > So please state clearly what the problem is and why solving it
>> > this way is the best or only solution.
>>
>> If that's not clear, I also would like to see a response to this
>> request before making any decisions here.
>
> It's a lack of proper D3hot support and GC6 support in Nouveau.

Thanks, but that is still a bit enigmatic.

What exactly do you mean by "proper D3hot support", in particular?

> As for why this is the best way to solve it?
> This has been a problem for many generations and Dell has had
> various different heuristics for detecting to turn off RTD3 on NV GPU
> to avoid exercising it.
>
> The patches submitted reflect a sustainable way to resolve the
> problem rather than the OEM and Linux kernel playing hide and seek
> to make the hardware work well in the general purpose scenario.

Well, I have some reservations.

While the idea of using _OSI to let the platform firmware find out
what the kernel can do is generally fine by me, the implementation
here isn't really.

In the first place, the _OSI "feature" has to be defined clearly and
in such a way that the kernel can say right away whether or not it is
"supported".  That doesn't seem to be the case here.

Second (and what follows from the above), the kernel should not need
any quirks on its side at all to give an answer.  It should be able to
say "yes" or "no" regardless of the platform it runs on if the
firmware asks the question.

So something like "Do you support native PCI power management?" would
be fine, because native PCI power management is defined by the PCI
standard and it should be clear what supporting it means and it
doesn't depend on what platform the kernel runs on.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Limonciello, Mario Feb. 12, 2018, 11:14 p.m. UTC | #18

> >> request before making any decisions here.

> >

> > It's a lack of proper D3hot support and GC6 support in Nouveau.

> 

> Thanks, but that is still a bit enigmatic.

> 

> What exactly do you mean by "proper D3hot support", in particular?

> 


I don't believe Nouveau or NVIDIA has support for D3hot at all from 
what I've heard.  I've not dug further into this.
Maybe Canonical guys will have some more detail.

> Well, I have some reservations.

> 

> While the idea of using _OSI to let the platform firmware find out

> what the kernel can do is generally fine by me, the implementation

> here isn't really.

> 

> In the first place, the _OSI "feature" has to be defined clearly and

> in such a way that the kernel can say right away whether or not it is

> "supported".  That doesn't seem to be the case here.

> 

> Second (and what follows from the above), the kernel should not need

> any quirks on its side at all to give an answer.  It should be able to

> say "yes" or "no" regardless of the platform it runs on if the

> firmware asks the question.

> 

> So something like "Do you support native PCI power management?" would

> be fine, because native PCI power management is defined by the PCI

> standard and it should be clear what supporting it means and it

> doesn't depend on what platform the kernel runs on.

> 


The problem is this is in conflict with the documentation.
As quoted:

```
_OSI("Linux-OEM-my_interface_name")
where 'OEM' is needed if this is an OEM-specific hook,
and 'my_interface_name' describes the hook, which could be a
quirk, a bug, or a bug-fix.
```

Quite literally from an OEM perspective this is a quirk.  The affected
platforms as configured by default won't work properly with Linux.

We apply a code deviation if the OSPM responds yes to that OSI string.

It's entirely possible that the Linux kernel could not store a quirk table
to match the affected platforms and instead give a blanket simple fast 
"Yes" answer to "Linux-Dell-Video", but I think that is no better than
_OSI("Linux").  This is at least an attestation from the OEM perspective
that we have only changed one thing by this string.

The alternative (and what has been done in the past) was the ACPI
_OSI rev hack marking those platforms for quirks and other things
before that and.  I really don't want to go down that road again.

Alex Hung Feb. 13, 2018, 5:25 a.m. UTC | #19

On Mon, Feb 12, 2018 at 3:14 PM,  <[e-mail]> wrote:
>> >> request before making any decisions here.
>> >
>> > It's a lack of proper D3hot support and GC6 support in Nouveau.
>>
>> Thanks, but that is still a bit enigmatic.
>>
>> What exactly do you mean by "proper D3hot support", in particular?
>>
>
> I don't believe Nouveau or NVIDIA has support for D3hot at all from
> what I've heard.  I've not dug further into this.
> Maybe Canonical guys will have some more detail.

I do not have more information than current NV GPU driver doesn't
support RTD3 now but NV would support RTD3 from next new GPU chip.

If this is true, we may not need this _OSI or any workaround in next generation.

>
>> Well, I have some reservations.
>>
>> While the idea of using _OSI to let the platform firmware find out
>> what the kernel can do is generally fine by me, the implementation
>> here isn't really.
>>
>> In the first place, the _OSI "feature" has to be defined clearly and
>> in such a way that the kernel can say right away whether or not it is
>> "supported".  That doesn't seem to be the case here.
>>
>> Second (and what follows from the above), the kernel should not need
>> any quirks on its side at all to give an answer.  It should be able to
>> say "yes" or "no" regardless of the platform it runs on if the
>> firmware asks the question.
>>
>> So something like "Do you support native PCI power management?" would
>> be fine, because native PCI power management is defined by the PCI
>> standard and it should be clear what supporting it means and it
>> doesn't depend on what platform the kernel runs on.
>>
>
> The problem is this is in conflict with the documentation.
> As quoted:
>
> ```
> _OSI("Linux-OEM-my_interface_name")
> where 'OEM' is needed if this is an OEM-specific hook,
> and 'my_interface_name' describes the hook, which could be a
> quirk, a bug, or a bug-fix.
> ```
>
> Quite literally from an OEM perspective this is a quirk.  The affected
> platforms as configured by default won't work properly with Linux.
>
> We apply a code deviation if the OSPM responds yes to that OSI string.
>
> It's entirely possible that the Linux kernel could not store a quirk table
> to match the affected platforms and instead give a blanket simple fast
> "Yes" answer to "Linux-Dell-Video", but I think that is no better than
> _OSI("Linux").  This is at least an attestation from the OEM perspective
> that we have only changed one thing by this string.
>
> The alternative (and what has been done in the past) was the ACPI
> _OSI rev hack marking those platforms for quirks and other things
> before that and.  I really don't want to go down that road again.

Mika Westerberg Feb. 13, 2018, 7:32 a.m. UTC | #20

On Mon, Feb 12, 2018 at 11:14:42PM +0000, [e-mail] wrote:
> > So something like "Do you support native PCI power management?" would
> > be fine, because native PCI power management is defined by the PCI
> > standard and it should be clear what supporting it means and it
> > doesn't depend on what platform the kernel runs on.
> > 
> 
> The problem is this is in conflict with the documentation.
> As quoted:
> 
> ```
> _OSI("Linux-OEM-my_interface_name")
> where 'OEM' is needed if this is an OEM-specific hook,
> and 'my_interface_name' describes the hook, which could be a
> quirk, a bug, or a bug-fix.
> ```
> 
> Quite literally from an OEM perspective this is a quirk.  The affected
> platforms as configured by default won't work properly with Linux.
> 
> We apply a code deviation if the OSPM responds yes to that OSI string.
> 
> It's entirely possible that the Linux kernel could not store a quirk table
> to match the affected platforms and instead give a blanket simple fast 
> "Yes" answer to "Linux-Dell-Video", but I think that is no better than
> _OSI("Linux").  This is at least an attestation from the OEM perspective
> that we have only changed one thing by this string.
> 
> The alternative (and what has been done in the past) was the ACPI
> _OSI rev hack marking those platforms for quirks and other things
> before that and.  I really don't want to go down that road again.

I have not followed this thread too closely so I might be missing
something but why do we need to do this in the first place? Does this
work in Windows and if yes, why we can't do the the same in Linux
without any sort of hacks and/or quirks?
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Rafael J. Wysocki Feb. 13, 2018, 9:18 a.m. UTC | #21

On Tue, Feb 13, 2018 at 12:14 AM,  <[e-mail]> wrote:
>> >> request before making any decisions here.
>> >
>> > It's a lack of proper D3hot support and GC6 support in Nouveau.
>>
>> Thanks, but that is still a bit enigmatic.
>>
>> What exactly do you mean by "proper D3hot support", in particular?
>>
>
> I don't believe Nouveau or NVIDIA has support for D3hot at all from
> what I've heard.  I've not dug further into this.

But Lukas is saying that Nouveau actually supports runtime PM, so I'm
not sure what exactly the problem is with respect to this particular
driver.

> Maybe Canonical guys will have some more detail.

OK

>> Well, I have some reservations.
>>
>> While the idea of using _OSI to let the platform firmware find out
>> what the kernel can do is generally fine by me, the implementation
>> here isn't really.
>>
>> In the first place, the _OSI "feature" has to be defined clearly and
>> in such a way that the kernel can say right away whether or not it is
>> "supported".  That doesn't seem to be the case here.
>>
>> Second (and what follows from the above), the kernel should not need
>> any quirks on its side at all to give an answer.  It should be able to
>> say "yes" or "no" regardless of the platform it runs on if the
>> firmware asks the question.
>>
>> So something like "Do you support native PCI power management?" would
>> be fine, because native PCI power management is defined by the PCI
>> standard and it should be clear what supporting it means and it
>> doesn't depend on what platform the kernel runs on.
>>
>
> The problem is this is in conflict with the documentation.
> As quoted:
>
> ```
> _OSI("Linux-OEM-my_interface_name")
> where 'OEM' is needed if this is an OEM-specific hook,
> and 'my_interface_name' describes the hook, which could be a
> quirk, a bug, or a bug-fix.
> ```
>

I don't see the conflict, sorry.

The "OEM-specific hook" is the "feature" here.  It is OEM-specific,
but the kernel still should be able to say "Yes, do that" or "No,
don't do that" without looking at the platform it is running on.

> Quite literally from an OEM perspective this is a quirk.  The affected
> platforms as configured by default won't work properly with Linux.
>
> We apply a code deviation if the OSPM responds yes to that OSI string.

Right.

> It's entirely possible that the Linux kernel could not store a quirk table
> to match the affected platforms and instead give a blanket simple fast
> "Yes" answer to "Linux-Dell-Video", but I think that is no better than
> _OSI("Linux").

First of all, _OSI("Linux") is not well-defined, because it
potentially covers everything ever done or not done by Linux,
regardless of the kernel version.  This just plain doesn't work.

"Linux-Dell-Video" could be defined precisely enough to actually work,
but IMO "Video" is too generic for an _OSI "feature" name.

Something like "Linux-Dell-quirk-this-specific-video-issue" can be made work.

> This is at least an attestation from the OEM perspective
> that we have only changed one thing by this string.

I'm not sure what you mean.

The firmware should only do
_OSI("Linux-Dell-quirk-this-specific-video-issue") if the response to
that makes a difference to it.  It knows what the platform is and it
does the _OSI() thing if the platform may be affected by the quirk
(that is, when necessary).  If the kernel sees that, it has to assume
that the platform may be affected.  Double checking in DMI is
pointless unless there are platforms abusing the _OSI(), but then the
abuse is not the kernel's problem really.

The whole point is that at one point the kernel can stop saying "yes"
to "Linux-Dell-quirk-this-specific-video-issue" if the quirk is not
necessary any more in this particular kernel version.

> The alternative (and what has been done in the past) was the ACPI
> _OSI rev hack marking those platforms for quirks and other things
> before that and.  I really don't want to go down that road again.

Right, that doesn't work just like _OSI("Linux").
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Lukas Wunner Feb. 13, 2018, 9:55 a.m. UTC | #22

On Tue, Feb 13, 2018 at 10:18:01AM +0100, Rafael J. Wysocki wrote:
> On Tue, Feb 13, 2018 at 12:14 AM,  <[e-mail]> wrote:
> >> >> request before making any decisions here.
> >> >
> >> > It's a lack of proper D3hot support and GC6 support in Nouveau.
> >>
> >> Thanks, but that is still a bit enigmatic.
> >>
> >> What exactly do you mean by "proper D3hot support", in particular?
> >>
> >
> > I don't believe Nouveau or NVIDIA has support for D3hot at all from
> > what I've heard.  I've not dug further into this.
> 
> But Lukas is saying that Nouveau actually supports runtime PM, so I'm
> not sure what exactly the problem is with respect to this particular
> driver.

Just providing some background info in the hope that it might be useful:

nouveau runtime suspends to D3cold on Nvidia Optimus hybrid graphics
laptops, not on any other machines (e.g. desktop).  The code is in
drivers/gpu/drm/nouveau/nouveau_acpi.c.  It detects presence of Optimus
by looking for a specific _DSM.

On older machines, power was cut using that _DSM.  Newer machines instead
specify _PR3 resources for the root port above the GPU, so power is cut
once the GPU and the root port above it have runtime suspended.  nouveau
detects and supports both mechanisms, as well as the ancient "HybridPower"
machines that preceded Optimus.  Adding Peter Wu to cc, who is the most
knowledgable person I know for this.

Mario mentions GC6, doing a bit of googling reveals this has to do with
power sequencing:
https://www.manualslib.com/manual/1273261/Clevo-N850hk1.html?page=67#manual

It looks like a number of GPIO pins need to be toggled in just the right
order with just the right timing for the GPU to suspend and resume
properly.  Now, normally this should all be handled by the firmware,
but apparently there's some OS cooperation necessary but people may
not be at liberty to talk about it or haven't been told everything
by Nvidia.

AFAIK the proprietary Nvidia driver doesn't support Optimus at all.
That might be the real motivation for the patch.

Thanks,

Lukas
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Rafael J. Wysocki Feb. 14, 2018, 9:06 a.m. UTC | #23

On Tue, Feb 13, 2018 at 8:32 AM, Mika Westerberg
<[e-mail]> wrote:
> On Mon, Feb 12, 2018 at 11:14:42PM +0000, [e-mail] wrote:
>> > So something like "Do you support native PCI power management?" would
>> > be fine, because native PCI power management is defined by the PCI
>> > standard and it should be clear what supporting it means and it
>> > doesn't depend on what platform the kernel runs on.
>> >
>>
>> The problem is this is in conflict with the documentation.
>> As quoted:
>>
>> ```
>> _OSI("Linux-OEM-my_interface_name")
>> where 'OEM' is needed if this is an OEM-specific hook,
>> and 'my_interface_name' describes the hook, which could be a
>> quirk, a bug, or a bug-fix.
>> ```
>>
>> Quite literally from an OEM perspective this is a quirk.  The affected
>> platforms as configured by default won't work properly with Linux.
>>
>> We apply a code deviation if the OSPM responds yes to that OSI string.
>>
>> It's entirely possible that the Linux kernel could not store a quirk table
>> to match the affected platforms and instead give a blanket simple fast
>> "Yes" answer to "Linux-Dell-Video", but I think that is no better than
>> _OSI("Linux").  This is at least an attestation from the OEM perspective
>> that we have only changed one thing by this string.
>>
>> The alternative (and what has been done in the past) was the ACPI
>> _OSI rev hack marking those platforms for quirks and other things
>> before that and.  I really don't want to go down that road again.
>
> I have not followed this thread too closely so I might be missing
> something but why do we need to do this in the first place?

As a temporary (presumably) workaround, basically. :-)

> Does this work in Windows and if yes, why we can't do the the same in Linux
> without any sort of hacks and/or quirks?

Of course it works on Windows.

The underlying issue is that the platform firmware expects Linux to
behave like Windows, presumably because Linux says "yes" to
_OSI("Windows <something>"), and it fails to work, because Linux
doesn't behave as expected by it.

Theoretically, it should be possible to make Linux behave like Windows
in that particular respect, but (a) there may be missing pieces that
we have no access to (like some secret documentation or similar) and
(b) it is unclear how much time that would take even if everything was
known.

However, the platforms in question are (or shortly will be) shipping
and Linux quite promptly doesn't work with them.

So the idea is to make a the firmware ask the kernel for a hint on
whether or not it should adjust its behavior for a particular
difference in behavior between Linux and Windows.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Rafael J. Wysocki Feb. 14, 2018, 9:10 a.m. UTC | #24

On Tue, Feb 13, 2018 at 10:55 AM, Lukas Wunner <[e-mail]> wrote:
> On Tue, Feb 13, 2018 at 10:18:01AM +0100, Rafael J. Wysocki wrote:
>> On Tue, Feb 13, 2018 at 12:14 AM,  <[e-mail]> wrote:
>> >> >> request before making any decisions here.
>> >> >
>> >> > It's a lack of proper D3hot support and GC6 support in Nouveau.
>> >>
>> >> Thanks, but that is still a bit enigmatic.
>> >>
>> >> What exactly do you mean by "proper D3hot support", in particular?
>> >>
>> >
>> > I don't believe Nouveau or NVIDIA has support for D3hot at all from
>> > what I've heard.  I've not dug further into this.
>>
>> But Lukas is saying that Nouveau actually supports runtime PM, so I'm
>> not sure what exactly the problem is with respect to this particular
>> driver.
>
> Just providing some background info in the hope that it might be useful:
>
> nouveau runtime suspends to D3cold on Nvidia Optimus hybrid graphics
> laptops, not on any other machines (e.g. desktop).  The code is in
> drivers/gpu/drm/nouveau/nouveau_acpi.c.  It detects presence of Optimus
> by looking for a specific _DSM.
>
> On older machines, power was cut using that _DSM.  Newer machines instead
> specify _PR3 resources for the root port above the GPU, so power is cut
> once the GPU and the root port above it have runtime suspended.  nouveau
> detects and supports both mechanisms, as well as the ancient "HybridPower"
> machines that preceded Optimus.  Adding Peter Wu to cc, who is the most
> knowledgable person I know for this.
>
> Mario mentions GC6, doing a bit of googling reveals this has to do with
> power sequencing:
> https://www.manualslib.com/manual/1273261/Clevo-N850hk1.html?page=67#manual
>
> It looks like a number of GPIO pins need to be toggled in just the right
> order with just the right timing for the GPU to suspend and resume
> properly.  Now, normally this should all be handled by the firmware,
> but apparently there's some OS cooperation necessary but people may
> not be at liberty to talk about it or haven't been told everything
> by Nvidia.
>
> AFAIK the proprietary Nvidia driver doesn't support Optimus at all.
> That might be the real motivation for the patch.

OK, that helps, thanks!

It actually agrees with my last reply to Mika.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mika Westerberg Feb. 14, 2018, 9:50 a.m. UTC | #25

On Wed, Feb 14, 2018 at 10:06:22AM +0100, Rafael J. Wysocki wrote:
> > Does this work in Windows and if yes, why we can't do the the same in Linux
> > without any sort of hacks and/or quirks?
> 
> Of course it works on Windows.
>
> The underlying issue is that the platform firmware expects Linux to
> behave like Windows, presumably because Linux says "yes" to
> _OSI("Windows <something>"), and it fails to work, because Linux
> doesn't behave as expected by it.
> 
> Theoretically, it should be possible to make Linux behave like Windows
> in that particular respect, but (a) there may be missing pieces that
> we have no access to (like some secret documentation or similar) and
> (b) it is unclear how much time that would take even if everything was
> known.
> 
> However, the platforms in question are (or shortly will be) shipping
> and Linux quite promptly doesn't work with them.
> 
> So the idea is to make a the firmware ask the kernel for a hint on
> whether or not it should adjust its behavior for a particular
> difference in behavior between Linux and Windows.

Makes sense. Thanks for the explanation!
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Limonciello, Mario Feb. 14, 2018, 6:47 p.m. UTC | #26

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiByand5c29ja2lAZ21haWwuY29t
IFttYWlsdG86cmp3eXNvY2tpQGdtYWlsLmNvbV0gT24gQmVoYWxmIE9mIFJhZmFlbCBKLg0KPiBX
eXNvY2tpDQo+IFNlbnQ6IFR1ZXNkYXksIEZlYnJ1YXJ5IDEzLCAyMDE4IDM6MTggQU0NCj4gVG86
IExpbW9uY2llbGxvLCBNYXJpbyA8TWFyaW9fTGltb25jaWVsbG9ARGVsbC5jb20+DQo+IENjOiBS
YWZhZWwgSi4gV3lzb2NraSA8cmFmYWVsQGtlcm5lbC5vcmc+OyBMdWthcyBXdW5uZXIgPGx1a2Fz
QHd1bm5lci5kZT47DQo+IEFsZXggSHVuZyA8YWxleC5odW5nQGNhbm9uaWNhbC5jb20+OyBBbmR5
IFNoZXZjaGVua28NCj4gPGFuZHJpeS5zaGV2Y2hlbmtvQGxpbnV4LmludGVsLmNvbT47IERtaXRy
eSBUb3Jva2hvdg0KPiA8ZG1pdHJ5LnRvcm9raG92QGdtYWlsLmNvbT47IEplYW4gRGVsdmFyZSA8
amRlbHZhcmVAc3VzZS5kZT47IExlbiBCcm93bg0KPiA8bGVuYkBrZXJuZWwub3JnPjsgR3JlZyBL
cm9haC1IYXJ0bWFuIDxncmVna2hAbGludXhmb3VuZGF0aW9uLm9yZz47IERhdmlkDQo+IE1pbGxl
ciA8ZGF2ZW1AZGF2ZW1sb2Z0Lm5ldD47IE1pa2EgV2VzdGVyYmVyZw0KPiA8bWlrYS53ZXN0ZXJi
ZXJnQGxpbnV4LmludGVsLmNvbT47IEZsb3JpYW4gRmFpbmVsbGkgPGYuZmFpbmVsbGlAZ21haWwu
Y29tPjsNCj4gS2lzaG9uIFZpamF5IEFicmFoYW0gSSA8a2lzaG9uQHRpLmNvbT47IHNheWxpIGth
cm5paw0KPiA8a2Fybmlrc2F5bGkxOTk1QGdtYWlsLmNvbT47IEFDUEkgRGV2ZWwgTWFsaW5nIExp
c3QgPGxpbnV4LQ0KPiBhY3BpQHZnZXIua2VybmVsLm9yZz4NCj4gU3ViamVjdDogUmU6IFtQQVRD
SCAyLzJdIEFDUEkgLyBvc2k6IGFkZCBETUkgcXVpcmsgZm9yIERlbGwgc3lzdGVtcw0KPiANCj4g
T24gVHVlLCBGZWIgMTMsIDIwMTggYXQgMTI6MTQgQU0sICA8TWFyaW8uTGltb25jaWVsbG9AZGVs
bC5jb20+IHdyb3RlOg0KPiA+PiA+PiByZXF1ZXN0IGJlZm9yZSBtYWtpbmcgYW55IGRlY2lzaW9u
cyBoZXJlLg0KPiA+PiA+DQo+ID4+ID4gSXQncyBhIGxhY2sgb2YgcHJvcGVyIEQzaG90IHN1cHBv
cnQgYW5kIEdDNiBzdXBwb3J0IGluIE5vdXZlYXUuDQo+ID4+DQo+ID4+IFRoYW5rcywgYnV0IHRo
YXQgaXMgc3RpbGwgYSBiaXQgZW5pZ21hdGljLg0KPiA+Pg0KPiA+PiBXaGF0IGV4YWN0bHkgZG8g
eW91IG1lYW4gYnkgInByb3BlciBEM2hvdCBzdXBwb3J0IiwgaW4gcGFydGljdWxhcj8NCj4gPj4N
Cj4gPg0KPiA+IEkgZG9uJ3QgYmVsaWV2ZSBOb3V2ZWF1IG9yIE5WSURJQSBoYXMgc3VwcG9ydCBm
b3IgRDNob3QgYXQgYWxsIGZyb20NCj4gPiB3aGF0IEkndmUgaGVhcmQuICBJJ3ZlIG5vdCBkdWcg
ZnVydGhlciBpbnRvIHRoaXMuDQo+IA0KPiBCdXQgTHVrYXMgaXMgc2F5aW5nIHRoYXQgTm91dmVh
dSBhY3R1YWxseSBzdXBwb3J0cyBydW50aW1lIFBNLCBzbyBJJ20NCj4gbm90IHN1cmUgd2hhdCBl
eGFjdGx5IHRoZSBwcm9ibGVtIGlzIHdpdGggcmVzcGVjdCB0byB0aGlzIHBhcnRpY3VsYXINCj4g
ZHJpdmVyLg0KPiANCj4gPiBNYXliZSBDYW5vbmljYWwgZ3V5cyB3aWxsIGhhdmUgc29tZSBtb3Jl
IGRldGFpbC4NCj4gDQo+IE9LDQo+IA0KPiA+PiBXZWxsLCBJIGhhdmUgc29tZSByZXNlcnZhdGlv
bnMuDQo+ID4+DQo+ID4+IFdoaWxlIHRoZSBpZGVhIG9mIHVzaW5nIF9PU0kgdG8gbGV0IHRoZSBw
bGF0Zm9ybSBmaXJtd2FyZSBmaW5kIG91dA0KPiA+PiB3aGF0IHRoZSBrZXJuZWwgY2FuIGRvIGlz
IGdlbmVyYWxseSBmaW5lIGJ5IG1lLCB0aGUgaW1wbGVtZW50YXRpb24NCj4gPj4gaGVyZSBpc24n
dCByZWFsbHkuDQo+ID4+DQo+ID4+IEluIHRoZSBmaXJzdCBwbGFjZSwgdGhlIF9PU0kgImZlYXR1
cmUiIGhhcyB0byBiZSBkZWZpbmVkIGNsZWFybHkgYW5kDQo+ID4+IGluIHN1Y2ggYSB3YXkgdGhh
dCB0aGUga2VybmVsIGNhbiBzYXkgcmlnaHQgYXdheSB3aGV0aGVyIG9yIG5vdCBpdCBpcw0KPiA+
PiAic3VwcG9ydGVkIi4gIFRoYXQgZG9lc24ndCBzZWVtIHRvIGJlIHRoZSBjYXNlIGhlcmUuDQo+
ID4+DQo+ID4+IFNlY29uZCAoYW5kIHdoYXQgZm9sbG93cyBmcm9tIHRoZSBhYm92ZSksIHRoZSBr
ZXJuZWwgc2hvdWxkIG5vdCBuZWVkDQo+ID4+IGFueSBxdWlya3Mgb24gaXRzIHNpZGUgYXQgYWxs
IHRvIGdpdmUgYW4gYW5zd2VyLiAgSXQgc2hvdWxkIGJlIGFibGUgdG8NCj4gPj4gc2F5ICJ5ZXMi
IG9yICJubyIgcmVnYXJkbGVzcyBvZiB0aGUgcGxhdGZvcm0gaXQgcnVucyBvbiBpZiB0aGUNCj4g
Pj4gZmlybXdhcmUgYXNrcyB0aGUgcXVlc3Rpb24uDQo+ID4+DQo+ID4+IFNvIHNvbWV0aGluZyBs
aWtlICJEbyB5b3Ugc3VwcG9ydCBuYXRpdmUgUENJIHBvd2VyIG1hbmFnZW1lbnQ/IiB3b3VsZA0K
PiA+PiBiZSBmaW5lLCBiZWNhdXNlIG5hdGl2ZSBQQ0kgcG93ZXIgbWFuYWdlbWVudCBpcyBkZWZp
bmVkIGJ5IHRoZSBQQ0kNCj4gPj4gc3RhbmRhcmQgYW5kIGl0IHNob3VsZCBiZSBjbGVhciB3aGF0
IHN1cHBvcnRpbmcgaXQgbWVhbnMgYW5kIGl0DQo+ID4+IGRvZXNuJ3QgZGVwZW5kIG9uIHdoYXQg
cGxhdGZvcm0gdGhlIGtlcm5lbCBydW5zIG9uLg0KPiA+Pg0KPiA+DQo+ID4gVGhlIHByb2JsZW0g
aXMgdGhpcyBpcyBpbiBjb25mbGljdCB3aXRoIHRoZSBkb2N1bWVudGF0aW9uLg0KPiA+IEFzIHF1
b3RlZDoNCj4gPg0KPiA+IGBgYA0KPiA+IF9PU0koIkxpbnV4LU9FTS1teV9pbnRlcmZhY2VfbmFt
ZSIpDQo+ID4gd2hlcmUgJ09FTScgaXMgbmVlZGVkIGlmIHRoaXMgaXMgYW4gT0VNLXNwZWNpZmlj
IGhvb2ssDQo+ID4gYW5kICdteV9pbnRlcmZhY2VfbmFtZScgZGVzY3JpYmVzIHRoZSBob29rLCB3
aGljaCBjb3VsZCBiZSBhDQo+ID4gcXVpcmssIGEgYnVnLCBvciBhIGJ1Zy1maXguDQo+ID4gYGBg
DQo+ID4NCj4gDQo+IEkgZG9uJ3Qgc2VlIHRoZSBjb25mbGljdCwgc29ycnkuDQo+IA0KPiBUaGUg
Ik9FTS1zcGVjaWZpYyBob29rIiBpcyB0aGUgImZlYXR1cmUiIGhlcmUuICBJdCBpcyBPRU0tc3Bl
Y2lmaWMsDQo+IGJ1dCB0aGUga2VybmVsIHN0aWxsIHNob3VsZCBiZSBhYmxlIHRvIHNheSAiWWVz
LCBkbyB0aGF0IiBvciAiTm8sDQo+IGRvbid0IGRvIHRoYXQiIHdpdGhvdXQgbG9va2luZyBhdCB0
aGUgcGxhdGZvcm0gaXQgaXMgcnVubmluZyBvbi4NCj4gDQo+ID4gUXVpdGUgbGl0ZXJhbGx5IGZy
b20gYW4gT0VNIHBlcnNwZWN0aXZlIHRoaXMgaXMgYSBxdWlyay4gIFRoZSBhZmZlY3RlZA0KPiA+
IHBsYXRmb3JtcyBhcyBjb25maWd1cmVkIGJ5IGRlZmF1bHQgd29uJ3Qgd29yayBwcm9wZXJseSB3
aXRoIExpbnV4Lg0KPiA+DQo+ID4gV2UgYXBwbHkgYSBjb2RlIGRldmlhdGlvbiBpZiB0aGUgT1NQ
TSByZXNwb25kcyB5ZXMgdG8gdGhhdCBPU0kgc3RyaW5nLg0KPiANCj4gUmlnaHQuDQo+IA0KPiA+
IEl0J3MgZW50aXJlbHkgcG9zc2libGUgdGhhdCB0aGUgTGludXgga2VybmVsIGNvdWxkIG5vdCBz
dG9yZSBhIHF1aXJrIHRhYmxlDQo+ID4gdG8gbWF0Y2ggdGhlIGFmZmVjdGVkIHBsYXRmb3JtcyBh
bmQgaW5zdGVhZCBnaXZlIGEgYmxhbmtldCBzaW1wbGUgZmFzdA0KPiA+ICJZZXMiIGFuc3dlciB0
byAiTGludXgtRGVsbC1WaWRlbyIsIGJ1dCBJIHRoaW5rIHRoYXQgaXMgbm8gYmV0dGVyIHRoYW4N
Cj4gPiBfT1NJKCJMaW51eCIpLg0KPiANCj4gRmlyc3Qgb2YgYWxsLCBfT1NJKCJMaW51eCIpIGlz
IG5vdCB3ZWxsLWRlZmluZWQsIGJlY2F1c2UgaXQNCj4gcG90ZW50aWFsbHkgY292ZXJzIGV2ZXJ5
dGhpbmcgZXZlciBkb25lIG9yIG5vdCBkb25lIGJ5IExpbnV4LA0KPiByZWdhcmRsZXNzIG9mIHRo
ZSBrZXJuZWwgdmVyc2lvbi4gIFRoaXMganVzdCBwbGFpbiBkb2Vzbid0IHdvcmsuDQo+IA0KPiAi
TGludXgtRGVsbC1WaWRlbyIgY291bGQgYmUgZGVmaW5lZCBwcmVjaXNlbHkgZW5vdWdoIHRvIGFj
dHVhbGx5IHdvcmssDQo+IGJ1dCBJTU8gIlZpZGVvIiBpcyB0b28gZ2VuZXJpYyBmb3IgYW4gX09T
SSAiZmVhdHVyZSIgbmFtZS4NCj4gDQo+IFNvbWV0aGluZyBsaWtlICJMaW51eC1EZWxsLXF1aXJr
LXRoaXMtc3BlY2lmaWMtdmlkZW8taXNzdWUiIGNhbiBiZSBtYWRlIHdvcmsuDQoNCldvdWxkOg0K
IkxpbnV4LURlbGwtTlZJRElBLVBvd2VyTWFuYWdlbWVudCINCkJlIG1vcmUgc3VmZmljaWVudD8N
Cg0KSWYgbm90LCBjYW4geW91IHBsZWFzZSBhZHZpc2Ugd2hhdCB3b3VsZCBiZSBtb3JlIGFjY2Vw
dGFibGU/DQoNCldlJ2xsIGhhdmUgdG8gc2VlIGlmIHRoZXJlIGlzIHN0aWxsIHRpbWUgdG8gY3V0
IHRoaXMgaW4gaW5zdGVhZCBvZiBMaW51eC1EZWxsLVZpZGVvLg0Kb3IgIkxpbnV4LURlbGwtVmlk
ZW8iIGlzIGFscmVhZHkgaW4gc3RhYmxlIEJJT1MgZm9yIGFueSBvZiB0aGVzZSBwbGF0Zm9ybXMu
DQoNCj4gDQo+ID4gVGhpcyBpcyBhdCBsZWFzdCBhbiBhdHRlc3RhdGlvbiBmcm9tIHRoZSBPRU0g
cGVyc3BlY3RpdmUNCj4gPiB0aGF0IHdlIGhhdmUgb25seSBjaGFuZ2VkIG9uZSB0aGluZyBieSB0
aGlzIHN0cmluZy4NCj4gDQo+IEknbSBub3Qgc3VyZSB3aGF0IHlvdSBtZWFuLg0KPiANCj4gVGhl
IGZpcm13YXJlIHNob3VsZCBvbmx5IGRvDQo+IF9PU0koIkxpbnV4LURlbGwtcXVpcmstdGhpcy1z
cGVjaWZpYy12aWRlby1pc3N1ZSIpIGlmIHRoZSByZXNwb25zZSB0bw0KPiB0aGF0IG1ha2VzIGEg
ZGlmZmVyZW5jZSB0byBpdC4gIEl0IGtub3dzIHdoYXQgdGhlIHBsYXRmb3JtIGlzIGFuZCBpdA0K
PiBkb2VzIHRoZSBfT1NJKCkgdGhpbmcgaWYgdGhlIHBsYXRmb3JtIG1heSBiZSBhZmZlY3RlZCBi
eSB0aGUgcXVpcmsNCj4gKHRoYXQgaXMsIHdoZW4gbmVjZXNzYXJ5KS4gIElmIHRoZSBrZXJuZWwg
c2VlcyB0aGF0LCBpdCBoYXMgdG8gYXNzdW1lDQo+IHRoYXQgdGhlIHBsYXRmb3JtIG1heSBiZSBh
ZmZlY3RlZC4gIERvdWJsZSBjaGVja2luZyBpbiBETUkgaXMNCj4gcG9pbnRsZXNzIHVubGVzcyB0
aGVyZSBhcmUgcGxhdGZvcm1zIGFidXNpbmcgdGhlIF9PU0koKSwgYnV0IHRoZW4gdGhlDQo+IGFi
dXNlIGlzIG5vdCB0aGUga2VybmVsJ3MgcHJvYmxlbSByZWFsbHkuDQoNClRoZSBpbnRlbnQgZnJv
bSB0aGUgYXBwcm9hY2ggQWxleCBzdWJtaXR0ZWQgd2FzIHNwZWNpZmljYWxseSB0byBhdm9pZA0K
dW5pbnRlbnRpb25hbCAoYnV0IHBvdGVudGlhbCkgYWJ1c2UuICBVbmNoZWNrZWQgdGhlIG5ldyBP
U0kgc3RyaW5ncyBjYW4NCmVhc2lseSB0dXJuIGludG8gY29weSBhbmQgcGFzdGUgZm9yIGFsbCB0
aGUgcHJvYmxlbXMgdGhhdCBjb21lIHVwIGFuZA0Kc3RhcnQgdG8gYmVoYXZlIGxpa2UgX09TSShM
aW51eCkuICBXZSBjYW4gZG8gb3VyIGJlc3QgdG8gdHJhaW4gZXZlcnlvbmUNCnRoYXQgdG91Y2hl
cyBCSU9TIGNvZGUgYnV0IGl0IGNvbWVzIGZyb20gbWFueSBwYXJ0bmVycyBhbmQgbWlzdGFrZXMN
CmNhbiBhbmQgd2lsbCBoYXBwZW4uDQoNCkJ5IG5lZWRpbmcgdG8gYWRkIHRoZSBPRU0gc3RyaW5n
IHRvIHRoZSB0YWJsZSB3aGVuIHRoZSBzeXN0ZW1zIGFyZQ0KdGVzdGVkIHRoZXkgYXJlIHRlc3Rl
ZCB3aXRoIExpbnV4IHRoaXMgZW5zdXJlcyB0aGF0IHdoZW4gYSBkZXZpYXRlZA0KT1NJIHN0cmlu
ZyBpcyBhZGRlZCBjcmVhdGVzIGEgY2hlY2twb2ludCBmb3IgdGhlIGNoYW5nZXMgdG8gYmUgDQph
dWRpdGVkIHRvIG1ha2Ugc3VyZSB0aGV5J3JlIG9ubHkgY2hhbmdpbmcgd2hhdCB0aGV5IHNob3Vs
ZCBhbmQNCnVuZGVyc3Rvb2QuDQoNCklmIHlvdSBzdGlsbCBmZWVsIHRoYXQgaXMgYWRkaW5nIHVu
ZHVlL3VubmVjZXNzYXJ5IHByb2Nlc3MsIEFsZXggY2FuIA0KZHJvcCB0aGF0IHBhcnQgb2YgdGhp
cy4NCg0KSSdkIHN0aWxsIGxpa2Ugc2VlIHBhdGNoIDEvMiByZXN1Ym1pdHRlZCB3aXRoIHRoZSBw
cm9wb3NlZCBmaXhlcyANCmhvd2V2ZXIgZm9yIHVzZSBlbHNld2hlcmUgaW4gdGhlIGtlcm5lbCB3
aGVuIHN5c3RlbXMgbmVlZCB0bw0KYmUgZGV0ZWN0ZWQgcmVsaWFibHkuDQoNCj4gDQo+IFRoZSB3
aG9sZSBwb2ludCBpcyB0aGF0IGF0IG9uZSBwb2ludCB0aGUga2VybmVsIGNhbiBzdG9wIHNheWlu
ZyAieWVzIg0KPiB0byAiTGludXgtRGVsbC1xdWlyay10aGlzLXNwZWNpZmljLXZpZGVvLWlzc3Vl
IiBpZiB0aGUgcXVpcmsgaXMgbm90DQo+IG5lY2Vzc2FyeSBhbnkgbW9yZSBpbiB0aGlzIHBhcnRp
Y3VsYXIga2VybmVsIHZlcnNpb24uDQoNCkJ1dCB0aGVuIGl0IGRlcGVuZHMgb24gaG93IHNwZWNp
ZmljIHRoZSBxdWlyayBhY3R1YWxseSBpcy4NCg0KV2UgY2FuJ3QgcG9zc2libHkgcHJlZGljdCBh
bGwgdGhlIHByb2JsZW1zIHRoYXQgd2lsbCBjb21lIHVwIHNvIGVhY2gNCnF1aXJrIE9TSSBzdHJp
bmcgaXMgZ29pbmcgdG8gYmUgY3JlYXRlZCBhcyB0aGV5IGNvbWUgdXAuDQoNCldoYXQgaWYgb25l
IGRheSBMaW51eCBrZXJuZWwgc3VwcG9ydHMgRDNob3QgYW5kIEdDNiBOViB0aGVyZSBpcyANCnNv
bWUgbmV3ICB0ZWNobm9sb2d5IHRoYXQgcmVwbGFjZXMgR0M2IHRoYXQgd291bGQgb3RoZXJ3aXNl
IA0KZmFsbCB1bmRlciB0aGUgc2FtZSAqLVBvd2VyTWFuYWdlbWVudCBwdXJ2aWV3IGFuZCBPU0kg
c3RyaW5nIA0KcHJvcG9zZWQ/ICBOb3cgeW91IG5lZWQgdG8gZWl0aGVyIGZpbmQgYWxsIHRoZSBz
eXN0ZW1zIHRoYXQgdXNlZA0KdGhlIHN0cmluZyBwcmV2aW91c2x5IGFuZCBxdWlyayB0aGVtIG9y
IHF1aXJrIHRoZSBuZXcgc3lzdGVtLg0KDQpTbyB0aGF0J3MgdGhlIG90aGVyIHJlYXNvbiB3aHkg
dG8gbWUgaXQgaXMgc2FmZXIgdG8gYnVpbGQgdGhlIGxpc3Qgb2YNCmV2ZXJ5IHN5c3RlbSB0aGF0
IG5lZWRzIGl0LiAgRG9jdW1lbnQgaXQgaW4gdGhlIGdpdCBjb21taXQgb3IgYQ0KY29tbWVudCwg
YW5kIGlmIG9uZSBhc3BlY3Qgb2YgdGhlIHdvcmthcm91bmQgY2FuIGJlIGRyb3BwZWQNCmluIHRo
ZSBmdXR1cmUgdGFrZSB0aG9zZSBzeXN0ZW1zIG91dCBvZiB0aGUgbGlzdC4NCg0KSSdkIGxpa2Ug
dG8gaWxsdXN0cmF0ZSBhbm90aGVyIGh5cG90aGV0aWNhbCBleGFtcGxlOg0KQSBsb3Qgb2Ygb3Vy
IG1hY2hpbmVzIGhhdmUgYSBEU1AgdGhhdCBnb2VzIHVudXNlZCBpbiBMaW51eC4NCldoYXQgaWYg
d2UgY29uY2x1ZGVkIHRoYXQncyBhY3R1YWxseSBhIHNpZ25pZmljYW50IHBvd2VyIGRyYWluIHRv
IA0KaGF2ZSBpdCBzcHVuIHVwIGJ1dCB1bnVzZWQgIGluIExpbnV4IGFuZCBkZWNpZGUgdG8gbWFr
ZSBhOg0KIkxpbnV4LURlbGwtRGlzYWJsZURTUCINCg0KTGF0ZXIgYSBkcml2ZXIgY29tZXMgYWxv
bmcgdGhhdCBjYW4gc3VwcG9ydCB0aGUgRFNQIG9uIHNvbWUNCm5ld2VyIHN5c3RlbXMgYnV0IG5v
dCB0aGUgb2xkZXIgb25lcy4gIFNvIHRoZSBuZXdlciBvbmVzIHByb2JhYmx5DQpzaG91bGQgZHJv
cCB0aGUgcmVzcG9uc2Ugb2YgeWVzIHRvIHRoYXQgT1NJIHN0cmluZyBpZiBpdCdzIHN0aWxsIGlu
IEJJT1MsDQpidXQgaG93IGVsc2Ugd291bGQgeW91IGF2b2lkIHJlZ3Jlc3Npbmcgb2xkZXIgc3lz
dGVtcz8NCg0KSSBndWVzcyB0aGUgcXVpcmsgT1NJIHN0cmluZyBjb3VsZCBiZQ0KIkxpbnV4LURl
bGwtRGlzYWJsZURTUC1LQkwtR2VuZXJhdGlvbiINCg0KQnV0IHlvdSBzZWUgd2hhdCBJIG1lYW4g
YWJvdXQgaG93IGl0J3MgaGFyZCB0byBwcmVkaWN0IHRoaXMgaW4NCmFkdmFuY2UgYW5kIGJldHRl
ciB0byB3aGl0ZWxpc3QgYnkgc3lzdGVtPw0K
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Rafael J. Wysocki Feb. 15, 2018, 9:44 a.m. UTC | #27

On Wed, Feb 14, 2018 at 7:47 PM,  <[e-mail]> wrote:
>> -----Original Message-----
>> From: [e-mail] [mailto:[e-mail]] On Behalf Of Rafael J.
>> Wysocki
>> Sent: Tuesday, February 13, 2018 3:18 AM
>> To: Limonciello, Mario <[e-mail]>
>> Cc: Rafael J. Wysocki <[e-mail]>; Lukas Wunner <[e-mail]>;
>> Alex Hung <[e-mail]>; Andy Shevchenko
>> <[e-mail]>; Dmitry Torokhov
>> <[e-mail]>; Jean Delvare <[e-mail]>; Len Brown
>> <[e-mail]>; Greg Kroah-Hartman <[e-mail]>; David
>> Miller <[e-mail]>; Mika Westerberg
>> <[e-mail]>; Florian Fainelli <[e-mail]>;
>> Kishon Vijay Abraham I <[e-mail]>; sayli karnik
>> <[e-mail]>; ACPI Devel Maling List <linux-
>> [e-mail]>
>> Subject: Re: [PATCH 2/2] ACPI / osi: add DMI quirk for Dell systems
>>
>> On Tue, Feb 13, 2018 at 12:14 AM,  <[e-mail]> wrote:
>> >> >> request before making any decisions here.
>> >> >
>> >> > It's a lack of proper D3hot support and GC6 support in Nouveau.
>> >>
>> >> Thanks, but that is still a bit enigmatic.
>> >>
>> >> What exactly do you mean by "proper D3hot support", in particular?
>> >>
>> >
>> > I don't believe Nouveau or NVIDIA has support for D3hot at all from
>> > what I've heard.  I've not dug further into this.
>>
>> But Lukas is saying that Nouveau actually supports runtime PM, so I'm
>> not sure what exactly the problem is with respect to this particular
>> driver.
>>
>> > Maybe Canonical guys will have some more detail.
>>
>> OK
>>
>> >> Well, I have some reservations.
>> >>
>> >> While the idea of using _OSI to let the platform firmware find out
>> >> what the kernel can do is generally fine by me, the implementation
>> >> here isn't really.
>> >>
>> >> In the first place, the _OSI "feature" has to be defined clearly and
>> >> in such a way that the kernel can say right away whether or not it is
>> >> "supported".  That doesn't seem to be the case here.
>> >>
>> >> Second (and what follows from the above), the kernel should not need
>> >> any quirks on its side at all to give an answer.  It should be able to
>> >> say "yes" or "no" regardless of the platform it runs on if the
>> >> firmware asks the question.
>> >>
>> >> So something like "Do you support native PCI power management?" would
>> >> be fine, because native PCI power management is defined by the PCI
>> >> standard and it should be clear what supporting it means and it
>> >> doesn't depend on what platform the kernel runs on.
>> >>
>> >
>> > The problem is this is in conflict with the documentation.
>> > As quoted:
>> >
>> > ```
>> > _OSI("Linux-OEM-my_interface_name")
>> > where 'OEM' is needed if this is an OEM-specific hook,
>> > and 'my_interface_name' describes the hook, which could be a
>> > quirk, a bug, or a bug-fix.
>> > ```
>> >
>>
>> I don't see the conflict, sorry.
>>
>> The "OEM-specific hook" is the "feature" here.  It is OEM-specific,
>> but the kernel still should be able to say "Yes, do that" or "No,
>> don't do that" without looking at the platform it is running on.
>>
>> > Quite literally from an OEM perspective this is a quirk.  The affected
>> > platforms as configured by default won't work properly with Linux.
>> >
>> > We apply a code deviation if the OSPM responds yes to that OSI string.
>>
>> Right.
>>
>> > It's entirely possible that the Linux kernel could not store a quirk table
>> > to match the affected platforms and instead give a blanket simple fast
>> > "Yes" answer to "Linux-Dell-Video", but I think that is no better than
>> > _OSI("Linux").
>>
>> First of all, _OSI("Linux") is not well-defined, because it
>> potentially covers everything ever done or not done by Linux,
>> regardless of the kernel version.  This just plain doesn't work.
>>
>> "Linux-Dell-Video" could be defined precisely enough to actually work,
>> but IMO "Video" is too generic for an _OSI "feature" name.
>>
>> Something like "Linux-Dell-quirk-this-specific-video-issue" can be made work.
>
> Would:
> "Linux-Dell-NVIDIA-PowerManagement"
> Be more sufficient?

Yes, it would be better IMO.

> If not, can you please advise what would be more acceptable?
>
> We'll have to see if there is still time to cut this in instead of Linux-Dell-Video.
> or "Linux-Dell-Video" is already in stable BIOS for any of these platforms.

As I said, you can define "Linux-Dell-Video" to mean "the PM issue
with NVidia graphics", but overall the name is confusingly generic.

In any case the meaning of the string has to be precise enough for
kernel developers to know when and why it is still necessary to give a
positive response to it.

>>
>> > This is at least an attestation from the OEM perspective
>> > that we have only changed one thing by this string.
>>
>> I'm not sure what you mean.
>>
>> The firmware should only do
>> _OSI("Linux-Dell-quirk-this-specific-video-issue") if the response to
>> that makes a difference to it.  It knows what the platform is and it
>> does the _OSI() thing if the platform may be affected by the quirk
>> (that is, when necessary).  If the kernel sees that, it has to assume
>> that the platform may be affected.  Double checking in DMI is
>> pointless unless there are platforms abusing the _OSI(), but then the
>> abuse is not the kernel's problem really.
>
> The intent from the approach Alex submitted was specifically to avoid
> unintentional (but potential) abuse.  Unchecked the new OSI strings can
> easily turn into copy and paste for all the problems that come up and
> start to behave like _OSI(Linux).  We can do our best to train everyone
> that touches BIOS code but it comes from many partners and mistakes
> can and will happen.
>
> By needing to add the OEM string to the table when the systems are
> tested they are tested with Linux this ensures that when a deviated
> OSI string is added creates a checkpoint for the changes to be
> audited to make sure they're only changing what they should and
> understood.
>
> If you still feel that is adding undue/unnecessary process, Alex can
> drop that part of this.

Yes, please.  At least make it a separate patch with clearly specified
purpose.  The way it was posted caused people to think that this was
necessary part of the approach.  Also, as it stands it very much looks
like an attempt to avoid having to actually define the meaning of the
_OSI() string.

In any case, the meaning of the _OSI() string must be well-defined in
the first place.  I won't let it in otherwise.  Then, if anyone abuses
it in a way that will lead to problems when the kernel stops
responding to it positively, we can blacklist the affected platforms
at that time.

> I'd still like see patch 1/2 resubmitted with the proposed fixes
> however for use elsewhere in the kernel when systems need to
> be detected reliably.

Not without a user, though.

>>
>> The whole point is that at one point the kernel can stop saying "yes"
>> to "Linux-Dell-quirk-this-specific-video-issue" if the quirk is not
>> necessary any more in this particular kernel version.
>
> But then it depends on how specific the quirk actually is.

So that's why I'm saying that it must be well-defined.

> We can't possibly predict all the problems that will come up so each
> quirk OSI string is going to be created as they come up.
>
> What if one day Linux kernel supports D3hot and GC6 NV there is
> some new  technology that replaces GC6 that would otherwise
> fall under the same *-PowerManagement purview and OSI string
> proposed?  Now you need to either find all the systems that used
> the string previously and quirk them or quirk the new system.

If that's a new issue, technically different from the previous one, a
prudent thing to do would be to define a new _OSI() string and use
that on the new system.

> So that's the other reason why to me it is safer to build the list of
> every system that needs it.  Document it in the git commit or a
> comment, and if one aspect of the workaround can be dropped
> in the future take those systems out of the list.
>
> I'd like to illustrate another hypothetical example:
> A lot of our machines have a DSP that goes unused in Linux.
> What if we concluded that's actually a significant power drain to
> have it spun up but unused  in Linux and decide to make a:
> "Linux-Dell-DisableDSP"
>
> Later a driver comes along that can support the DSP on some
> newer systems but not the older ones.  So the newer ones probably
> should drop the response of yes to that OSI string if it's still in BIOS,
> but how else would you avoid regressing older systems?

I would define an new _OSI() string to use instead of the previous one
for the new generation.

> I guess the quirk OSI string could be
> "Linux-Dell-DisableDSP-KBL-Generation"
>
> But you see what I mean about how it's hard to predict this in
> advance and better to whitelist by system?

Yes, it is hard to predict things in general, but if your _OSI()
strings are each defined to cover a single specific issue, that
shouldn't be a problem.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
index 76998a5..43e4349 100644
--- a/drivers/acpi/osi.c
+++ b/drivers/acpi/osi.c
@@ -477,6 +477,112 @@  static const struct dmi_system_id acpi_osi_dmi_table[] __initconst = {
 	{}
 };
 
+static int __init dmi_oem_osi_add(const struct dmi_system_id *d)
+{
+	struct acpi_osi_entry *osi;
+	const char *str = d->driver_data;
+	int i;
+
+	for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
+		osi = &osi_setup_entries[i];
+		if (!strcmp(osi->string, str)) {
+			osi->enable = true;
+			continue;
+		} else if (osi->string[0] == '\0') {
+			osi->enable = true;
+			strncpy(osi->string, str, OSI_STRING_LENGTH_MAX);
+			break;
+		}
+	}
+
+	return 0;
+}
+
+static const struct dmi_system_id acpi_oem_osi_dmi_table[] __initconst = {
+	{
+	.callback = dmi_oem_osi_add,
+	.ident = "Dell Latitude 5491",
+	.matches = {
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0818]"),
+		},
+	.driver_data = "Linux-Dell-Video",
+	},
+	{
+	.callback = dmi_oem_osi_add,
+	.ident = "Dell Latitude 5591",
+	.matches = {
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0819]"),
+		},
+	.driver_data = "Linux-Dell-Video",
+	},
+	{
+	.callback = dmi_oem_osi_add,
+	.ident = "Dell Precision 3530",
+	.matches = {
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0820]"),
+		},
+	.driver_data = "Linux-Dell-Video",
+	},
+	{
+	.callback = dmi_oem_osi_add,
+	.ident = "Dell Inspiron 7777 AIO",
+	.matches = {
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0850]"),
+		},
+	.driver_data = "Linux-Dell-Video",
+	},
+	{
+	.callback = dmi_oem_osi_add,
+	.ident = "Dell Inspiron 5477 AIO",
+	.matches = {
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0851]"),
+		},
+	.driver_data = "Linux-Dell-Video",
+	},
+	{
+	.callback = dmi_oem_osi_add,
+	.ident = "Dell G5 5779",
+	.matches = {
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0886]"),
+		},
+	.driver_data = "Linux-Dell-Video",
+	},
+	{
+	.callback = dmi_oem_osi_add,
+	.ident = "Dell G5 5779",
+	.matches = {
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0870]"),
+		},
+	.driver_data = "Linux-Dell-Video",
+	},
+	{
+	.callback = dmi_oem_osi_add,
+	.ident = "Dell G5 5579",
+	.matches = {
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[086F]"),
+		},
+	.driver_data = "Linux-Dell-Video",
+	},
+	{
+	.callback = dmi_oem_osi_add,
+	.ident = "Dell G5 5579",
+	.matches = {
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "Dell System"),
+		     DMI_EXACT_MATCH(DMI_OEM_STRING, "1[0885]"),
+		},
+	.driver_data = "Linux-Dell-Video",
+	},
+	{}
+};
+
 static __init void acpi_osi_dmi_blacklisted(void)
 {
 	dmi_check_system(acpi_osi_dmi_table);
@@ -496,6 +602,7 @@  int __init early_acpi_osi_init(void)
 int __init acpi_osi_init(void)
 {
 	acpi_install_interface_handler(acpi_osi_handler);
+	dmi_check_system(acpi_oem_osi_dmi_table);
 	acpi_osi_setup_late();
 
 	return 0;