linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Default to using the native backlight control on Windows 8 systems
@ 2014-04-10 20:13 Matthew Garrett
  2014-04-11  2:56 ` Aaron Lu
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Matthew Garrett @ 2014-04-10 20:13 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-acpi, Matthew Garrett

The list of machines in the "Use native backlight" table is getting longer
and longer, which is a solid indication that we're doing something wrong.
Disable the ACPI backlight interface if the system claims to be Windows 8
or later.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---

Let's at least get this into -next for 3.16 and figure out whether the
drivers actually work now.

 drivers/acpi/video.c | 139 +--------------------------------------------------
 1 file changed, 1 insertion(+), 138 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 48c7e8a..21d2fc9 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -78,14 +78,6 @@ module_param(brightness_switch_enabled, bool, 0644);
 static bool allow_duplicates;
 module_param(allow_duplicates, bool, 0644);
 
-/*
- * For Windows 8 systems: used to decide if video module
- * should skip registering backlight interface of its own.
- */
-static int use_native_backlight_param = -1;
-module_param_named(use_native_backlight, use_native_backlight_param, int, 0444);
-static bool use_native_backlight_dmi = false;
-
 static int register_count;
 static struct mutex video_list_lock;
 static struct list_head video_bus_head;
@@ -230,18 +222,9 @@ static int acpi_video_get_next_level(struct acpi_video_device *device,
 static int acpi_video_switch_brightness(struct acpi_video_device *device,
 					 int event);
 
-static bool acpi_video_use_native_backlight(void)
-{
-	if (use_native_backlight_param != -1)
-		return use_native_backlight_param;
-	else
-		return use_native_backlight_dmi;
-}
-
 static bool acpi_video_verify_backlight_support(void)
 {
-	if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
-	    backlight_device_registered(BACKLIGHT_RAW))
+	if (acpi_osi_is_win8() && backlight_device_registered(BACKLIGHT_RAW))
 		return false;
 	return acpi_video_backlight_support();
 }
@@ -405,12 +388,6 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d)
 	return 0;
 }
 
-static int __init video_set_use_native_backlight(const struct dmi_system_id *d)
-{
-	use_native_backlight_dmi = true;
-	return 0;
-}
-
 static struct dmi_system_id video_dmi_table[] __initdata = {
 	/*
 	 * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121
@@ -455,120 +432,6 @@ static struct dmi_system_id video_dmi_table[] __initdata = {
 		DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 7720"),
 		},
 	},
-	{
-	 .callback = video_set_use_native_backlight,
-	 .ident = "ThinkPad T430s",
-	 .matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
-		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"),
-		},
-	},
-	{
-	 .callback = video_set_use_native_backlight,
-	 .ident = "ThinkPad X230",
-	 .matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
-		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"),
-		},
-	},
-	{
-	.callback = video_set_use_native_backlight,
-	.ident = "ThinkPad X1 Carbon",
-	.matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
-		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X1 Carbon"),
-		},
-	},
-	{
-	 .callback = video_set_use_native_backlight,
-	 .ident = "Lenovo Yoga 13",
-	 .matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
-		DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"),
-		},
-	},
-	{
-	 .callback = video_set_use_native_backlight,
-	 .ident = "Dell Inspiron 7520",
-	 .matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
-		DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"),
-		},
-	},
-	{
-	 .callback = video_set_use_native_backlight,
-	 .ident = "Acer Aspire 5733Z",
-	 .matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
-		DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 5733Z"),
-		},
-	},
-	{
-	 .callback = video_set_use_native_backlight,
-	 .ident = "Acer Aspire V5-431",
-	 .matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
-		DMI_MATCH(DMI_PRODUCT_NAME, "Aspire V5-431"),
-		},
-	},
-	{
-	.callback = video_set_use_native_backlight,
-	.ident = "HP ProBook 4340s",
-	.matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
-		DMI_MATCH(DMI_PRODUCT_VERSION, "HP ProBook 4340s"),
-		},
-	},
-	{
-	.callback = video_set_use_native_backlight,
-	.ident = "HP ProBook 2013 models",
-	.matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
-		DMI_MATCH(DMI_PRODUCT_NAME, "HP ProBook "),
-		DMI_MATCH(DMI_PRODUCT_NAME, " G1"),
-		},
-	},
-	{
-	.callback = video_set_use_native_backlight,
-	.ident = "HP EliteBook 2013 models",
-	.matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
-		DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook "),
-		DMI_MATCH(DMI_PRODUCT_NAME, " G1"),
-		},
-	},
-	{
-	.callback = video_set_use_native_backlight,
-	.ident = "HP ZBook 14",
-	.matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
-		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 14"),
-		},
-	},
-	{
-	.callback = video_set_use_native_backlight,
-	.ident = "HP ZBook 15",
-	.matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
-		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 15"),
-		},
-	},
-	{
-	.callback = video_set_use_native_backlight,
-	.ident = "HP ZBook 17",
-	.matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
-		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 17"),
-		},
-	},
-	{
-	.callback = video_set_use_native_backlight,
-	.ident = "HP EliteBook 8780w",
-	.matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
-		DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook 8780w"),
-		},
-	},
 	{}
 };
 
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Default to using the native backlight control on Windows 8 systems
  2014-04-10 20:13 [PATCH] Default to using the native backlight control on Windows 8 systems Matthew Garrett
@ 2014-04-11  2:56 ` Aaron Lu
  2014-04-11 10:58   ` Jani Nikula
  2014-04-21 20:55 ` Rafael J. Wysocki
  2014-06-05 20:27 ` [PATCH] ACPI / video: Change the default for video.use_native_backlight to 1 Rafael J. Wysocki
  2 siblings, 1 reply; 6+ messages in thread
From: Aaron Lu @ 2014-04-11  2:56 UTC (permalink / raw)
  To: Matthew Garrett, dri-devel; +Cc: linux-acpi

On 04/11/2014 04:13 AM, Matthew Garrett wrote:
> The list of machines in the "Use native backlight" table is getting longer
> and longer, which is a solid indication that we're doing something wrong.
> Disable the ACPI backlight interface if the system claims to be Windows 8
> or later.

And has a native backlight control interface.

> 
> Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
> ---
> 
> Let's at least get this into -next for 3.16 and figure out whether the
> drivers actually work now.

I agree.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Default to using the native backlight control on Windows 8 systems
  2014-04-11  2:56 ` Aaron Lu
@ 2014-04-11 10:58   ` Jani Nikula
  2014-04-11 11:31     ` Aaron Lu
  0 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2014-04-11 10:58 UTC (permalink / raw)
  To: Aaron Lu, Matthew Garrett, dri-devel; +Cc: linux-acpi

On Fri, 11 Apr 2014, Aaron Lu <aaron.lu@intel.com> wrote:
> On 04/11/2014 04:13 AM, Matthew Garrett wrote:
>> The list of machines in the "Use native backlight" table is getting longer
>> and longer, which is a solid indication that we're doing something wrong.
>> Disable the ACPI backlight interface if the system claims to be Windows 8
>> or later.
>
> And has a native backlight control interface.

This. One thing we were doing wrong in i915 was unconditionally
registering a native backlight interface even when it was not
functional. We can't really know whether that is the case except by
trusting the VBT, and although we've been let down in the past, trusting
the VBT on this one is probably a better approximation of the reality
than assuming it always works. Thus we've queued [1] and [2] for 3.15
which should help.

[1] http://cgit.freedesktop.org/drm-intel/commit/?id=39fbc9c8f6765959b55e0b127dd5c57df5a47d67
[2] http://cgit.freedesktop.org/drm-intel/commit/?id=c675949ec58ca50d5a3ae3c757892f1560f6e896

>> Let's at least get this into -next for 3.16 and figure out whether the
>> drivers actually work now.
>
> I agree.

Agreed.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Default to using the native backlight control on Windows 8 systems
  2014-04-11 10:58   ` Jani Nikula
@ 2014-04-11 11:31     ` Aaron Lu
  0 siblings, 0 replies; 6+ messages in thread
From: Aaron Lu @ 2014-04-11 11:31 UTC (permalink / raw)
  To: Jani Nikula, Matthew Garrett, dri-devel; +Cc: linux-acpi

On 04/11/2014 06:58 PM, Jani Nikula wrote:
> On Fri, 11 Apr 2014, Aaron Lu <aaron.lu@intel.com> wrote:
>> On 04/11/2014 04:13 AM, Matthew Garrett wrote:
>>> The list of machines in the "Use native backlight" table is getting longer
>>> and longer, which is a solid indication that we're doing something wrong.
>>> Disable the ACPI backlight interface if the system claims to be Windows 8
>>> or later.
>>
>> And has a native backlight control interface.
> 
> This. One thing we were doing wrong in i915 was unconditionally
> registering a native backlight interface even when it was not
> functional. We can't really know whether that is the case except by
> trusting the VBT, and although we've been let down in the past, trusting
> the VBT on this one is probably a better approximation of the reality
> than assuming it always works. Thus we've queued [1] and [2] for 3.15
> which should help.
> 
> [1] http://cgit.freedesktop.org/drm-intel/commit/?id=39fbc9c8f6765959b55e0b127dd5c57df5a47d67
> [2] http://cgit.freedesktop.org/drm-intel/commit/?id=c675949ec58ca50d5a3ae3c757892f1560f6e896

That's pretty cool :-)

Regards,
Aaron

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Default to using the native backlight control on Windows 8 systems
  2014-04-10 20:13 [PATCH] Default to using the native backlight control on Windows 8 systems Matthew Garrett
  2014-04-11  2:56 ` Aaron Lu
@ 2014-04-21 20:55 ` Rafael J. Wysocki
  2014-06-05 20:27 ` [PATCH] ACPI / video: Change the default for video.use_native_backlight to 1 Rafael J. Wysocki
  2 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2014-04-21 20:55 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: dri-devel, linux-acpi, Aaron Lu

On Thursday, April 10, 2014 04:13:23 PM Matthew Garrett wrote:
> The list of machines in the "Use native backlight" table is getting longer
> and longer, which is a solid indication that we're doing something wrong.
> Disable the ACPI backlight interface if the system claims to be Windows 8
> or later.
> 
> Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
> ---
> 
> Let's at least get this into -next for 3.16 and figure out whether the
> drivers actually work now.

Does this still apply to 3.15-rc2 or does it need to be refreshed?

>  drivers/acpi/video.c | 139 +--------------------------------------------------
>  1 file changed, 1 insertion(+), 138 deletions(-)
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 48c7e8a..21d2fc9 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -78,14 +78,6 @@ module_param(brightness_switch_enabled, bool, 0644);
>  static bool allow_duplicates;
>  module_param(allow_duplicates, bool, 0644);
>  
> -/*
> - * For Windows 8 systems: used to decide if video module
> - * should skip registering backlight interface of its own.
> - */
> -static int use_native_backlight_param = -1;
> -module_param_named(use_native_backlight, use_native_backlight_param, int, 0444);
> -static bool use_native_backlight_dmi = false;
> -
>  static int register_count;
>  static struct mutex video_list_lock;
>  static struct list_head video_bus_head;
> @@ -230,18 +222,9 @@ static int acpi_video_get_next_level(struct acpi_video_device *device,
>  static int acpi_video_switch_brightness(struct acpi_video_device *device,
>  					 int event);
>  
> -static bool acpi_video_use_native_backlight(void)
> -{
> -	if (use_native_backlight_param != -1)
> -		return use_native_backlight_param;
> -	else
> -		return use_native_backlight_dmi;
> -}
> -
>  static bool acpi_video_verify_backlight_support(void)
>  {
> -	if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
> -	    backlight_device_registered(BACKLIGHT_RAW))
> +	if (acpi_osi_is_win8() && backlight_device_registered(BACKLIGHT_RAW))
>  		return false;
>  	return acpi_video_backlight_support();
>  }
> @@ -405,12 +388,6 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d)
>  	return 0;
>  }
>  
> -static int __init video_set_use_native_backlight(const struct dmi_system_id *d)
> -{
> -	use_native_backlight_dmi = true;
> -	return 0;
> -}
> -
>  static struct dmi_system_id video_dmi_table[] __initdata = {
>  	/*
>  	 * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121
> @@ -455,120 +432,6 @@ static struct dmi_system_id video_dmi_table[] __initdata = {
>  		DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 7720"),
>  		},
>  	},
> -	{
> -	 .callback = video_set_use_native_backlight,
> -	 .ident = "ThinkPad T430s",
> -	 .matches = {
> -		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> -		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"),
> -		},
> -	},
> -	{
> -	 .callback = video_set_use_native_backlight,
> -	 .ident = "ThinkPad X230",
> -	 .matches = {
> -		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> -		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"),
> -		},
> -	},
> -	{
> -	.callback = video_set_use_native_backlight,
> -	.ident = "ThinkPad X1 Carbon",
> -	.matches = {
> -		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> -		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X1 Carbon"),
> -		},
> -	},
> -	{
> -	 .callback = video_set_use_native_backlight,
> -	 .ident = "Lenovo Yoga 13",
> -	 .matches = {
> -		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> -		DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"),
> -		},
> -	},
> -	{
> -	 .callback = video_set_use_native_backlight,
> -	 .ident = "Dell Inspiron 7520",
> -	 .matches = {
> -		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> -		DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"),
> -		},
> -	},
> -	{
> -	 .callback = video_set_use_native_backlight,
> -	 .ident = "Acer Aspire 5733Z",
> -	 .matches = {
> -		DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
> -		DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 5733Z"),
> -		},
> -	},
> -	{
> -	 .callback = video_set_use_native_backlight,
> -	 .ident = "Acer Aspire V5-431",
> -	 .matches = {
> -		DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
> -		DMI_MATCH(DMI_PRODUCT_NAME, "Aspire V5-431"),
> -		},
> -	},
> -	{
> -	.callback = video_set_use_native_backlight,
> -	.ident = "HP ProBook 4340s",
> -	.matches = {
> -		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> -		DMI_MATCH(DMI_PRODUCT_VERSION, "HP ProBook 4340s"),
> -		},
> -	},
> -	{
> -	.callback = video_set_use_native_backlight,
> -	.ident = "HP ProBook 2013 models",
> -	.matches = {
> -		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> -		DMI_MATCH(DMI_PRODUCT_NAME, "HP ProBook "),
> -		DMI_MATCH(DMI_PRODUCT_NAME, " G1"),
> -		},
> -	},
> -	{
> -	.callback = video_set_use_native_backlight,
> -	.ident = "HP EliteBook 2013 models",
> -	.matches = {
> -		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> -		DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook "),
> -		DMI_MATCH(DMI_PRODUCT_NAME, " G1"),
> -		},
> -	},
> -	{
> -	.callback = video_set_use_native_backlight,
> -	.ident = "HP ZBook 14",
> -	.matches = {
> -		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> -		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 14"),
> -		},
> -	},
> -	{
> -	.callback = video_set_use_native_backlight,
> -	.ident = "HP ZBook 15",
> -	.matches = {
> -		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> -		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 15"),
> -		},
> -	},
> -	{
> -	.callback = video_set_use_native_backlight,
> -	.ident = "HP ZBook 17",
> -	.matches = {
> -		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> -		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 17"),
> -		},
> -	},
> -	{
> -	.callback = video_set_use_native_backlight,
> -	.ident = "HP EliteBook 8780w",
> -	.matches = {
> -		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> -		DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook 8780w"),
> -		},
> -	},
>  	{}
>  };
>  
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ACPI / video: Change the default for video.use_native_backlight to 1
  2014-04-10 20:13 [PATCH] Default to using the native backlight control on Windows 8 systems Matthew Garrett
  2014-04-11  2:56 ` Aaron Lu
  2014-04-21 20:55 ` Rafael J. Wysocki
@ 2014-06-05 20:27 ` Rafael J. Wysocki
  2 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2014-06-05 20:27 UTC (permalink / raw)
  To: linux-acpi
  Cc: Matthew Garrett, dri-devel, Aaron Lu, Linux Kernel Mailing List,
	Jani Nikula, Hans de Goede

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Now that we're hoping to have resolved all of the problems with
video.use_native_backlight=1 make that the default at last.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Having forgotten that the Hans' patches didn't actually flip the
default for this I kind of pre-anounced it in my last pull request,
which was a mistake that I apologize for.  That said, since everyone
involved seemed to be supportive, I'm going to apply this one and
push it to Linus next week.

Please let me know if there are any objections.

Rafael


---
 drivers/acpi/video.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-pm/drivers/acpi/video.c
===================================================================
--- linux-pm.orig/drivers/acpi/video.c
+++ linux-pm/drivers/acpi/video.c
@@ -82,7 +82,7 @@ module_param(allow_duplicates, bool, 064
  * For Windows 8 systems: used to decide if video module
  * should skip registering backlight interface of its own.
  */
-static int use_native_backlight_param = -1;
+static int use_native_backlight_param = 1;
 module_param_named(use_native_backlight, use_native_backlight_param, int, 0444);
 static bool use_native_backlight_dmi = false;
 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-06-05 20:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-10 20:13 [PATCH] Default to using the native backlight control on Windows 8 systems Matthew Garrett
2014-04-11  2:56 ` Aaron Lu
2014-04-11 10:58   ` Jani Nikula
2014-04-11 11:31     ` Aaron Lu
2014-04-21 20:55 ` Rafael J. Wysocki
2014-06-05 20:27 ` [PATCH] ACPI / video: Change the default for video.use_native_backlight to 1 Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).