[RFC PATCH 0/2] acpi: video: Allow forcing native backlight on non win8 laptops

[RFC PATCH 0/2] acpi: video: Allow forcing native backlight on non win8 laptops

[Hans de Goede]

Hi All,

I've long wanted to have a way for users to force native-backlight behavior as
that makes debugging backlight problems a bit easier.

And now a laptop has shown up which needs native backlight behavior for things
to work, has a non win8 ready bios, and I cannot use a quirk in the vendor
backlight driver as it implements both the acer-wmi and ideapad-laptop vendor
interfaces and putting the same quirk in 2 places is just wrong.

So I've finally written a small patch to allow forcing native-backlight
behavior as well as a second patch using this to add a quirk for said laptop.

For now this is just a RFC series as I'm waiting for confirmation from the
user that this actually fixes things, but I wanted to get the discussion going
beforehand in case anyone has any objections against the solution I've come
up with.

Regards,

Hans

[RFC PATCH 1/2] acpi: video: Allow forcing native backlight on non win8 machines

[Hans de Goede]

The native backlight behavior (so not registering both the acpi-video and the
vendor backlight driver) can be useful on some non win8 machines too, allow
the user to force this behavior by passing video.use_native_backlight=2
on the kernel commandline.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/video.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index debd309..9817b52 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -84,7 +84,7 @@ module_param(allow_duplicates, bool, 0644);
  */
 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 = true;
+static int use_native_backlight_dmi = 1;
 
 static int register_count;
 static struct mutex video_list_lock;
@@ -235,7 +235,7 @@ static int acpi_video_get_next_level(struct acpi_video_device *device,
 				     u32 level_current, u32 event);
 static void acpi_video_switch_brightness(struct work_struct *work);
 
-static bool acpi_video_use_native_backlight(void)
+static int acpi_video_use_native_backlight(void)
 {
 	if (use_native_backlight_param != -1)
 		return use_native_backlight_param;
@@ -245,7 +245,8 @@ static bool acpi_video_use_native_backlight(void)
 
 bool acpi_video_verify_backlight_support(void)
 {
-	if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
+	if (((acpi_video_use_native_backlight() == 2) ||
+	     (acpi_video_use_native_backlight() == 1 && acpi_osi_is_win8())) &&
 	    backlight_device_registered(BACKLIGHT_RAW))
 		return false;
 	return acpi_video_backlight_support();
@@ -414,7 +415,7 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d)
 
 static int __init video_disable_native_backlight(const struct dmi_system_id *d)
 {
-	use_native_backlight_dmi = false;
+	use_native_backlight_dmi = 0;
 	return 0;
 }
 
-- 
2.3.1

Re: [RFC PATCH 1/2] acpi: video: Allow forcing native backlight on non win8 machines

[Aaron Lu]

On 03/03/2015 03:39 PM, Hans de Goede wrote:
> The native backlight behavior (so not registering both the acpi-video and the
> vendor backlight driver) can be useful on some non win8 machines too, allow
> the user to force this behavior by passing video.use_native_backlight=2
> on the kernel commandline.

Just bikeshedding, what about doing it this way?

In the acpi_video_use_native_backlight function:
1 If user has set a cmdline option, use that(no matter if it is a win8
  system or not);
2 If the system is in a DMI table, use that(no matter if it is a win8
  system or not);
3 return true if this is a win8 system; false otherwise.

Something like this:

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index debd30917010..4cd0c8a4fd9d 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -84,7 +84,7 @@ module_param(allow_duplicates, bool, 0644);
  */
 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 = true;
+static int use_native_backlight_dmi = -1;
 
 static int register_count;
 static struct mutex video_list_lock;
@@ -239,13 +239,14 @@ static bool acpi_video_use_native_backlight(void)
 {
 	if (use_native_backlight_param != -1)
 		return use_native_backlight_param;
-	else
+	else if (use_native_backlight_dmi != -1)
 		return use_native_backlight_dmi;
+	return acpi_osi_is_win8();
 }
 
 bool acpi_video_verify_backlight_support(void)
 {
-	if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
+	if (acpi_video_use_native_backlight() &&
 	    backlight_device_registered(BACKLIGHT_RAW))
 		return false;
 	return acpi_video_backlight_support();
@@ -414,7 +415,7 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d)
 
 static int __init video_disable_native_backlight(const struct dmi_system_id *d)
 {
-	use_native_backlight_dmi = false;
+	use_native_backlight_dmi = 0;
 	return 0;
 }
 

Regards,
Aaron

> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/acpi/video.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index debd309..9817b52 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -84,7 +84,7 @@ module_param(allow_duplicates, bool, 0644);
>   */
>  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 = true;
> +static int use_native_backlight_dmi = 1;
>  
>  static int register_count;
>  static struct mutex video_list_lock;
> @@ -235,7 +235,7 @@ static int acpi_video_get_next_level(struct acpi_video_device *device,
>  				     u32 level_current, u32 event);
>  static void acpi_video_switch_brightness(struct work_struct *work);
>  
> -static bool acpi_video_use_native_backlight(void)
> +static int acpi_video_use_native_backlight(void)
>  {
>  	if (use_native_backlight_param != -1)
>  		return use_native_backlight_param;
> @@ -245,7 +245,8 @@ static bool acpi_video_use_native_backlight(void)
>  
>  bool acpi_video_verify_backlight_support(void)
>  {
> -	if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
> +	if (((acpi_video_use_native_backlight() == 2) ||
> +	     (acpi_video_use_native_backlight() == 1 && acpi_osi_is_win8())) &&
>  	    backlight_device_registered(BACKLIGHT_RAW))
>  		return false;
>  	return acpi_video_backlight_support();
> @@ -414,7 +415,7 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d)
>  
>  static int __init video_disable_native_backlight(const struct dmi_system_id *d)
>  {
> -	use_native_backlight_dmi = false;
> +	use_native_backlight_dmi = 0;
>  	return 0;
>  }
>  
> 

Re: [RFC PATCH 1/2] acpi: video: Allow forcing native backlight on non win8 machines

[Hans de Goede]

Hi,

On 03-03-15 09:26, Aaron Lu wrote:
> On 03/03/2015 03:39 PM, Hans de Goede wrote:
>> The native backlight behavior (so not registering both the acpi-video and the
>> vendor backlight driver) can be useful on some non win8 machines too, allow
>> the user to force this behavior by passing video.use_native_backlight=2
>> on the kernel commandline.
>
> Just bikeshedding, what about doing it this way?
>
> In the acpi_video_use_native_backlight function:
> 1 If user has set a cmdline option, use that(no matter if it is a win8
>    system or not);
> 2 If the system is in a DMI table, use that(no matter if it is a win8
>    system or not);
> 3 return true if this is a win8 system; false otherwise.
>
> Something like this:

That works for me, and has the added advantage of not changing the
cmdline syntax (but it does change the cmdline behavior ...).

So going either way is fine with me.

Aaron's version is:

Acked-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

>
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index debd30917010..4cd0c8a4fd9d 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -84,7 +84,7 @@ module_param(allow_duplicates, bool, 0644);
>    */
>   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 = true;
> +static int use_native_backlight_dmi = -1;
>
>   static int register_count;
>   static struct mutex video_list_lock;
> @@ -239,13 +239,14 @@ static bool acpi_video_use_native_backlight(void)
>   {
>   	if (use_native_backlight_param != -1)
>   		return use_native_backlight_param;
> -	else
> +	else if (use_native_backlight_dmi != -1)
>   		return use_native_backlight_dmi;
> +	return acpi_osi_is_win8();
>   }
>
>   bool acpi_video_verify_backlight_support(void)
>   {
> -	if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
> +	if (acpi_video_use_native_backlight() &&
>   	    backlight_device_registered(BACKLIGHT_RAW))
>   		return false;
>   	return acpi_video_backlight_support();
> @@ -414,7 +415,7 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d)
>
>   static int __init video_disable_native_backlight(const struct dmi_system_id *d)
>   {
> -	use_native_backlight_dmi = false;
> +	use_native_backlight_dmi = 0;
>   	return 0;
>   }
>
>
> Regards,
> Aaron
>
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/acpi/video.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
>> index debd309..9817b52 100644
>> --- a/drivers/acpi/video.c
>> +++ b/drivers/acpi/video.c
>> @@ -84,7 +84,7 @@ module_param(allow_duplicates, bool, 0644);
>>    */
>>   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 = true;
>> +static int use_native_backlight_dmi = 1;
>>
>>   static int register_count;
>>   static struct mutex video_list_lock;
>> @@ -235,7 +235,7 @@ static int acpi_video_get_next_level(struct acpi_video_device *device,
>>   				     u32 level_current, u32 event);
>>   static void acpi_video_switch_brightness(struct work_struct *work);
>>
>> -static bool acpi_video_use_native_backlight(void)
>> +static int acpi_video_use_native_backlight(void)
>>   {
>>   	if (use_native_backlight_param != -1)
>>   		return use_native_backlight_param;
>> @@ -245,7 +245,8 @@ static bool acpi_video_use_native_backlight(void)
>>
>>   bool acpi_video_verify_backlight_support(void)
>>   {
>> -	if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
>> +	if (((acpi_video_use_native_backlight() == 2) ||
>> +	     (acpi_video_use_native_backlight() == 1 && acpi_osi_is_win8())) &&
>>   	    backlight_device_registered(BACKLIGHT_RAW))
>>   		return false;
>>   	return acpi_video_backlight_support();
>> @@ -414,7 +415,7 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d)
>>
>>   static int __init video_disable_native_backlight(const struct dmi_system_id *d)
>>   {
>> -	use_native_backlight_dmi = false;
>> +	use_native_backlight_dmi = 0;
>>   	return 0;
>>   }
>>
>>
>

Re: [RFC PATCH 1/2] acpi: video: Allow forcing native backlight on non win8 machines

[Aaron Lu]

On 03/03/2015 09:11 PM, Hans de Goede wrote:
> Hi,
> 
> On 03-03-15 09:26, Aaron Lu wrote:
>> On 03/03/2015 03:39 PM, Hans de Goede wrote:
>>> The native backlight behavior (so not registering both the acpi-video and the
>>> vendor backlight driver) can be useful on some non win8 machines too, allow
>>> the user to force this behavior by passing video.use_native_backlight=2
>>> on the kernel commandline.
>>
>> Just bikeshedding, what about doing it this way?
>>
>> In the acpi_video_use_native_backlight function:
>> 1 If user has set a cmdline option, use that(no matter if it is a win8
>>    system or not);
>> 2 If the system is in a DMI table, use that(no matter if it is a win8
>>    system or not);
>> 3 return true if this is a win8 system; false otherwise.
>>
>> Something like this:
> 
> That works for me, and has the added advantage of not changing the
> cmdline syntax (but it does change the cmdline behavior ...).

Oh yes, I didn't consider this. Not sure how much impact this will have,
but I don't see an immediate problem with it, so let's see.

> 
> So going either way is fine with me.
> 
> Aaron's version is:
> 
> Acked-by: Hans de Goede <hdegoede@redhat.com>

Thanks. Here is the full patch:

From: Aaron Lu <aaron.lu@intel.com>
Date: Wed, 4 Mar 2015 10:24:58 +0800
Subject: [PATCH] acpi: video: Allow forcing native backlight on non win8
 machines

The native backlight behavior (so not registering both the acpi-video
and the vendor backlight driver) can be useful on some non win8 machines
too, so change the behavior of the video.use_native_backlight=1 or 0
kernel cmdline option to be: if user has set video.use_native_backlight=1
or 0, use that no matter if it is a win8 system or not. Also, we will
put some known systems into the DMI table to make them either use native
backlight interface or not, so the use_native_backlight_dmi is used to
reflect that.

Original-by: Hans de Goede <hdegoede@redhat.com>
Signed-offby: Aaron Lu <aaron.lu@intel.com>
Acked-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/video.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index debd30917010..4cd0c8a4fd9d 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -84,7 +84,7 @@ module_param(allow_duplicates, bool, 0644);
  */
 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 = true;
+static int use_native_backlight_dmi = -1;
 
 static int register_count;
 static struct mutex video_list_lock;
@@ -239,13 +239,14 @@ static bool acpi_video_use_native_backlight(void)
 {
 	if (use_native_backlight_param != -1)
 		return use_native_backlight_param;
-	else
+	else if (use_native_backlight_dmi != -1)
 		return use_native_backlight_dmi;
+	return acpi_osi_is_win8();
 }
 
 bool acpi_video_verify_backlight_support(void)
 {
-	if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
+	if (acpi_video_use_native_backlight() &&
 	    backlight_device_registered(BACKLIGHT_RAW))
 		return false;
 	return acpi_video_backlight_support();
@@ -414,7 +415,7 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d)
 
 static int __init video_disable_native_backlight(const struct dmi_system_id *d)
 {
-	use_native_backlight_dmi = false;
+	use_native_backlight_dmi = 0;
 	return 0;
 }
 
-- 
2.1.0

Re: [RFC PATCH 1/2] acpi: video: Allow forcing native backlight on non win8 machines

[Rafael J. Wysocki]

On Wednesday, March 04, 2015 10:50:20 AM Aaron Lu wrote:
> On 03/03/2015 09:11 PM, Hans de Goede wrote:
> > Hi,
> > 
> > On 03-03-15 09:26, Aaron Lu wrote:
> >> On 03/03/2015 03:39 PM, Hans de Goede wrote:
> >>> The native backlight behavior (so not registering both the acpi-video and the
> >>> vendor backlight driver) can be useful on some non win8 machines too, allow
> >>> the user to force this behavior by passing video.use_native_backlight=2
> >>> on the kernel commandline.
> >>
> >> Just bikeshedding, what about doing it this way?
> >>
> >> In the acpi_video_use_native_backlight function:
> >> 1 If user has set a cmdline option, use that(no matter if it is a win8
> >>    system or not);
> >> 2 If the system is in a DMI table, use that(no matter if it is a win8
> >>    system or not);
> >> 3 return true if this is a win8 system; false otherwise.
> >>
> >> Something like this:
> > 
> > That works for me, and has the added advantage of not changing the
> > cmdline syntax (but it does change the cmdline behavior ...).
> 
> Oh yes, I didn't consider this. Not sure how much impact this will have,
> but I don't see an immediate problem with it, so let's see.
> 
> > 
> > So going either way is fine with me.
> > 
> > Aaron's version is:
> > 
> > Acked-by: Hans de Goede <hdegoede@redhat.com>
> 
> Thanks. Here is the full patch:
> 
> From: Aaron Lu <aaron.lu@intel.com>
> Date: Wed, 4 Mar 2015 10:24:58 +0800
> Subject: [PATCH] acpi: video: Allow forcing native backlight on non win8
>  machines
> 
> The native backlight behavior (so not registering both the acpi-video
> and the vendor backlight driver) can be useful on some non win8 machines
> too, so change the behavior of the video.use_native_backlight=1 or 0
> kernel cmdline option to be: if user has set video.use_native_backlight=1
> or 0, use that no matter if it is a win8 system or not. Also, we will
> put some known systems into the DMI table to make them either use native
> backlight interface or not, so the use_native_backlight_dmi is used to
> reflect that.
> 
> Original-by: Hans de Goede <hdegoede@redhat.com>
> Signed-offby: Aaron Lu <aaron.lu@intel.com>
> Acked-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/acpi/video.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index debd30917010..4cd0c8a4fd9d 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -84,7 +84,7 @@ module_param(allow_duplicates, bool, 0644);
>   */
>  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 = true;
> +static int use_native_backlight_dmi = -1;

So can we have proper symbols instead of these numbers, please?

>  
>  static int register_count;
>  static struct mutex video_list_lock;
> @@ -239,13 +239,14 @@ static bool acpi_video_use_native_backlight(void)
>  {
>  	if (use_native_backlight_param != -1)
>  		return use_native_backlight_param;
> -	else
> +	else if (use_native_backlight_dmi != -1)
>  		return use_native_backlight_dmi;
> +	return acpi_osi_is_win8();
>  }
>  
>  bool acpi_video_verify_backlight_support(void)
>  {
> -	if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
> +	if (acpi_video_use_native_backlight() &&
>  	    backlight_device_registered(BACKLIGHT_RAW))
>  		return false;
>  	return acpi_video_backlight_support();
> @@ -414,7 +415,7 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d)
>  
>  static int __init video_disable_native_backlight(const struct dmi_system_id *d)
>  {
> -	use_native_backlight_dmi = false;
> +	use_native_backlight_dmi = 0;
>  	return 0;
>  }
>  
> 

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

Re: [RFC PATCH 1/2] acpi: video: Allow forcing native backlight on non win8 machines

[Hans de Goede]

Hi,

On 03/10/2015 11:25 PM, Rafael J. Wysocki wrote:
> On Wednesday, March 04, 2015 10:50:20 AM Aaron Lu wrote:
>> On 03/03/2015 09:11 PM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 03-03-15 09:26, Aaron Lu wrote:
>>>> On 03/03/2015 03:39 PM, Hans de Goede wrote:
>>>>> The native backlight behavior (so not registering both the acpi-video and the
>>>>> vendor backlight driver) can be useful on some non win8 machines too, allow
>>>>> the user to force this behavior by passing video.use_native_backlight=2
>>>>> on the kernel commandline.
>>>>
>>>> Just bikeshedding, what about doing it this way?
>>>>
>>>> In the acpi_video_use_native_backlight function:
>>>> 1 If user has set a cmdline option, use that(no matter if it is a win8
>>>>     system or not);
>>>> 2 If the system is in a DMI table, use that(no matter if it is a win8
>>>>     system or not);
>>>> 3 return true if this is a win8 system; false otherwise.
>>>>
>>>> Something like this:
>>>
>>> That works for me, and has the added advantage of not changing the
>>> cmdline syntax (but it does change the cmdline behavior ...).
>>
>> Oh yes, I didn't consider this. Not sure how much impact this will have,
>> but I don't see an immediate problem with it, so let's see.
>>
>>>
>>> So going either way is fine with me.
>>>
>>> Aaron's version is:
>>>
>>> Acked-by: Hans de Goede <hdegoede@redhat.com>
>>
>> Thanks. Here is the full patch:
>>
>> From: Aaron Lu <aaron.lu@intel.com>
>> Date: Wed, 4 Mar 2015 10:24:58 +0800
>> Subject: [PATCH] acpi: video: Allow forcing native backlight on non win8
>>   machines
>>
>> The native backlight behavior (so not registering both the acpi-video
>> and the vendor backlight driver) can be useful on some non win8 machines
>> too, so change the behavior of the video.use_native_backlight=1 or 0
>> kernel cmdline option to be: if user has set video.use_native_backlight=1
>> or 0, use that no matter if it is a win8 system or not. Also, we will
>> put some known systems into the DMI table to make them either use native
>> backlight interface or not, so the use_native_backlight_dmi is used to
>> reflect that.
>>
>> Original-by: Hans de Goede <hdegoede@redhat.com>
>> Signed-offby: Aaron Lu <aaron.lu@intel.com>
>> Acked-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/acpi/video.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
>> index debd30917010..4cd0c8a4fd9d 100644
>> --- a/drivers/acpi/video.c
>> +++ b/drivers/acpi/video.c
>> @@ -84,7 +84,7 @@ module_param(allow_duplicates, bool, 0644);
>>    */
>>   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 = true;
>> +static int use_native_backlight_dmi = -1;
>
> So can we have proper symbols instead of these numbers, please?

So you want to have a native_backlight enum or some such ? with -1 being not_set ?

Regards,

Hans

>
>>
>>   static int register_count;
>>   static struct mutex video_list_lock;
>> @@ -239,13 +239,14 @@ static bool acpi_video_use_native_backlight(void)
>>   {
>>   	if (use_native_backlight_param != -1)
>>   		return use_native_backlight_param;
>> -	else
>> +	else if (use_native_backlight_dmi != -1)
>>   		return use_native_backlight_dmi;
>> +	return acpi_osi_is_win8();
>>   }
>>
>>   bool acpi_video_verify_backlight_support(void)
>>   {
>> -	if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
>> +	if (acpi_video_use_native_backlight() &&
>>   	    backlight_device_registered(BACKLIGHT_RAW))
>>   		return false;
>>   	return acpi_video_backlight_support();
>> @@ -414,7 +415,7 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d)
>>
>>   static int __init video_disable_native_backlight(const struct dmi_system_id *d)
>>   {
>> -	use_native_backlight_dmi = false;
>> +	use_native_backlight_dmi = 0;
>>   	return 0;
>>   }
>>
>>
>

Re: [RFC PATCH 1/2] acpi: video: Allow forcing native backlight on non win8 machines

[Rafael J. Wysocki]

On Tuesday, March 10, 2015 11:42:21 PM Hans de Goede wrote:
> Hi,
> 
> On 03/10/2015 11:25 PM, Rafael J. Wysocki wrote:
> > On Wednesday, March 04, 2015 10:50:20 AM Aaron Lu wrote:
> >> On 03/03/2015 09:11 PM, Hans de Goede wrote:
> >>> Hi,
> >>>
> >>> On 03-03-15 09:26, Aaron Lu wrote:
> >>>> On 03/03/2015 03:39 PM, Hans de Goede wrote:
> >>>>> The native backlight behavior (so not registering both the acpi-video and the
> >>>>> vendor backlight driver) can be useful on some non win8 machines too, allow
> >>>>> the user to force this behavior by passing video.use_native_backlight=2
> >>>>> on the kernel commandline.
> >>>>
> >>>> Just bikeshedding, what about doing it this way?
> >>>>
> >>>> In the acpi_video_use_native_backlight function:
> >>>> 1 If user has set a cmdline option, use that(no matter if it is a win8
> >>>>     system or not);
> >>>> 2 If the system is in a DMI table, use that(no matter if it is a win8
> >>>>     system or not);
> >>>> 3 return true if this is a win8 system; false otherwise.
> >>>>
> >>>> Something like this:
> >>>
> >>> That works for me, and has the added advantage of not changing the
> >>> cmdline syntax (but it does change the cmdline behavior ...).
> >>
> >> Oh yes, I didn't consider this. Not sure how much impact this will have,
> >> but I don't see an immediate problem with it, so let's see.
> >>
> >>>
> >>> So going either way is fine with me.
> >>>
> >>> Aaron's version is:
> >>>
> >>> Acked-by: Hans de Goede <hdegoede@redhat.com>
> >>
> >> Thanks. Here is the full patch:
> >>
> >> From: Aaron Lu <aaron.lu@intel.com>
> >> Date: Wed, 4 Mar 2015 10:24:58 +0800
> >> Subject: [PATCH] acpi: video: Allow forcing native backlight on non win8
> >>   machines
> >>
> >> The native backlight behavior (so not registering both the acpi-video
> >> and the vendor backlight driver) can be useful on some non win8 machines
> >> too, so change the behavior of the video.use_native_backlight=1 or 0
> >> kernel cmdline option to be: if user has set video.use_native_backlight=1
> >> or 0, use that no matter if it is a win8 system or not. Also, we will
> >> put some known systems into the DMI table to make them either use native
> >> backlight interface or not, so the use_native_backlight_dmi is used to
> >> reflect that.
> >>
> >> Original-by: Hans de Goede <hdegoede@redhat.com>
> >> Signed-offby: Aaron Lu <aaron.lu@intel.com>
> >> Acked-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >>   drivers/acpi/video.c | 9 +++++----
> >>   1 file changed, 5 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> >> index debd30917010..4cd0c8a4fd9d 100644
> >> --- a/drivers/acpi/video.c
> >> +++ b/drivers/acpi/video.c
> >> @@ -84,7 +84,7 @@ module_param(allow_duplicates, bool, 0644);
> >>    */
> >>   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 = true;
> >> +static int use_native_backlight_dmi = -1;
> >
> > So can we have proper symbols instead of these numbers, please?
> 
> So you want to have a native_backlight enum or some such ? with -1 being not_set ?

Yes, something like that.


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

[PATCH update] acpi: video: Allow forcing native backlight on non win8, machines

[Aaron Lu]

On 03/11/2015 07:10 AM, Rafael J. Wysocki wrote:
> On Tuesday, March 10, 2015 11:42:21 PM Hans de Goede wrote:
>> So you want to have a native_backlight enum or some such ? with -1 being not_set ?
> 
> Yes, something like that.

OK, here is an updated one:

>From db4eb1928219e47fcfbffb913f08b1e1b432c28a Mon Sep 17 00:00:00 2001
From: Aaron Lu <aaron.lu@intel.com>
Date: Wed, 4 Mar 2015 10:24:58 +0800
Subject: [PATCH] acpi: video: Allow forcing native backlight on non win8
 machines

The native backlight behavior (so not registering both the acpi-video
and the vendor backlight driver) can be useful on some non win8 machines
too, so change the behavior of the video.use_native_backlight=1 or 0
kernel cmdline option to be: if user has set video.use_native_backlight=1
or 0, use that no matter if it is a win8 system or not. Also, we will
put some known systems into the DMI table to make them either use native
backlight interface or not, and the use_native_backlight_dmi is used to
reflect that.

Original-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Aaron Lu <aaron.lu@intel.com>
Acked-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/video.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 26eb70c8f518..2f45dca31724 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -82,9 +82,15 @@ 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;
+enum {
+	NATIVE_BACKLIGHT_NOT_SET = -1,
+	NATIVE_BACKLIGHT_OFF,
+	NATIVE_BACKLIGHT_ON,
+};
+
+static int use_native_backlight_param = NATIVE_BACKLIGHT_NOT_SET;
 module_param_named(use_native_backlight, use_native_backlight_param, int, 0444);
-static bool use_native_backlight_dmi = true;
+static int use_native_backlight_dmi = NATIVE_BACKLIGHT_NOT_SET;
 
 static int register_count;
 static struct mutex video_list_lock;
@@ -237,15 +243,16 @@ static void acpi_video_switch_brightness(struct work_struct *work);
 
 static bool acpi_video_use_native_backlight(void)
 {
-	if (use_native_backlight_param != -1)
+	if (use_native_backlight_param != NATIVE_BACKLIGHT_NOT_SET)
 		return use_native_backlight_param;
-	else
+	else if (use_native_backlight_dmi != NATIVE_BACKLIGHT_NOT_SET)
 		return use_native_backlight_dmi;
+	return acpi_osi_is_win8();
 }
 
 bool acpi_video_verify_backlight_support(void)
 {
-	if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
+	if (acpi_video_use_native_backlight() &&
 	    backlight_device_registered(BACKLIGHT_RAW))
 		return false;
 	return acpi_video_backlight_support();
@@ -414,7 +421,7 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d)
 
 static int __init video_disable_native_backlight(const struct dmi_system_id *d)
 {
-	use_native_backlight_dmi = false;
+	use_native_backlight_dmi = NATIVE_BACKLIGHT_OFF;
 	return 0;
 }
 
-- 
2.1.0

Re: [PATCH update] acpi: video: Allow forcing native backlight on non win8, machines

[Hans de Goede]

Hi,

On 11-03-15 07:14, Aaron Lu wrote:
> On 03/11/2015 07:10 AM, Rafael J. Wysocki wrote:
>> On Tuesday, March 10, 2015 11:42:21 PM Hans de Goede wrote:
>>> So you want to have a native_backlight enum or some such ? with -1 being not_set ?
>>
>> Yes, something like that.
>
> OK, here is an updated one:

Looks good to me, my ack still stands. I need to send a bunch of
mails back / forth to tackle the issue the ideapad which caused me to
work on this in the first case. So lets merge this one first, and
then I'll get back with a patch on top of this when I know how to
actually best deal with the problem on that machine.

Regards,

Hans


>
>  From db4eb1928219e47fcfbffb913f08b1e1b432c28a Mon Sep 17 00:00:00 2001
> From: Aaron Lu <aaron.lu@intel.com>
> Date: Wed, 4 Mar 2015 10:24:58 +0800
> Subject: [PATCH] acpi: video: Allow forcing native backlight on non win8
>   machines
>
> The native backlight behavior (so not registering both the acpi-video
> and the vendor backlight driver) can be useful on some non win8 machines
> too, so change the behavior of the video.use_native_backlight=1 or 0
> kernel cmdline option to be: if user has set video.use_native_backlight=1
> or 0, use that no matter if it is a win8 system or not. Also, we will
> put some known systems into the DMI table to make them either use native
> backlight interface or not, and the use_native_backlight_dmi is used to
> reflect that.
>
> Original-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> Acked-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   drivers/acpi/video.c | 19 +++++++++++++------
>   1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 26eb70c8f518..2f45dca31724 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -82,9 +82,15 @@ 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;
> +enum {
> +	NATIVE_BACKLIGHT_NOT_SET = -1,
> +	NATIVE_BACKLIGHT_OFF,
> +	NATIVE_BACKLIGHT_ON,
> +};
> +
> +static int use_native_backlight_param = NATIVE_BACKLIGHT_NOT_SET;
>   module_param_named(use_native_backlight, use_native_backlight_param, int, 0444);
> -static bool use_native_backlight_dmi = true;
> +static int use_native_backlight_dmi = NATIVE_BACKLIGHT_NOT_SET;
>
>   static int register_count;
>   static struct mutex video_list_lock;
> @@ -237,15 +243,16 @@ static void acpi_video_switch_brightness(struct work_struct *work);
>
>   static bool acpi_video_use_native_backlight(void)
>   {
> -	if (use_native_backlight_param != -1)
> +	if (use_native_backlight_param != NATIVE_BACKLIGHT_NOT_SET)
>   		return use_native_backlight_param;
> -	else
> +	else if (use_native_backlight_dmi != NATIVE_BACKLIGHT_NOT_SET)
>   		return use_native_backlight_dmi;
> +	return acpi_osi_is_win8();
>   }
>
>   bool acpi_video_verify_backlight_support(void)
>   {
> -	if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
> +	if (acpi_video_use_native_backlight() &&
>   	    backlight_device_registered(BACKLIGHT_RAW))
>   		return false;
>   	return acpi_video_backlight_support();
> @@ -414,7 +421,7 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d)
>
>   static int __init video_disable_native_backlight(const struct dmi_system_id *d)
>   {
> -	use_native_backlight_dmi = false;
> +	use_native_backlight_dmi = NATIVE_BACKLIGHT_OFF;
>   	return 0;
>   }
>
>

Re: [PATCH update] acpi: video: Allow forcing native backlight on non win8, machines

[Aaron Lu]

On Wed, Mar 11, 2015 at 12:14:41PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 11-03-15 07:14, Aaron Lu wrote:
> >On 03/11/2015 07:10 AM, Rafael J. Wysocki wrote:
> >>On Tuesday, March 10, 2015 11:42:21 PM Hans de Goede wrote:
> >>>So you want to have a native_backlight enum or some such ? with -1 being not_set ?
> >>
> >>Yes, something like that.
> >
> >OK, here is an updated one:
> 
> Looks good to me, my ack still stands. I need to send a bunch of
> mails back / forth to tackle the issue the ideapad which caused me to
> work on this in the first case. So lets merge this one first, and
> then I'll get back with a patch on top of this when I know how to
> actually best deal with the problem on that machine.

Thanks for the ack and taking care of the problem.

Regards,
Aaron

> 
> Regards,
> 
> Hans
> 
> 
> >
> > From db4eb1928219e47fcfbffb913f08b1e1b432c28a Mon Sep 17 00:00:00 2001
> >From: Aaron Lu <aaron.lu@intel.com>
> >Date: Wed, 4 Mar 2015 10:24:58 +0800
> >Subject: [PATCH] acpi: video: Allow forcing native backlight on non win8
> >  machines
> >
> >The native backlight behavior (so not registering both the acpi-video
> >and the vendor backlight driver) can be useful on some non win8 machines
> >too, so change the behavior of the video.use_native_backlight=1 or 0
> >kernel cmdline option to be: if user has set video.use_native_backlight=1
> >or 0, use that no matter if it is a win8 system or not. Also, we will
> >put some known systems into the DMI table to make them either use native
> >backlight interface or not, and the use_native_backlight_dmi is used to
> >reflect that.
> >
> >Original-by: Hans de Goede <hdegoede@redhat.com>
> >Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> >Acked-by: Hans de Goede <hdegoede@redhat.com>
> >---
> >  drivers/acpi/video.c | 19 +++++++++++++------
> >  1 file changed, 13 insertions(+), 6 deletions(-)
> >
> >diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> >index 26eb70c8f518..2f45dca31724 100644
> >--- a/drivers/acpi/video.c
> >+++ b/drivers/acpi/video.c
> >@@ -82,9 +82,15 @@ 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;
> >+enum {
> >+	NATIVE_BACKLIGHT_NOT_SET = -1,
> >+	NATIVE_BACKLIGHT_OFF,
> >+	NATIVE_BACKLIGHT_ON,
> >+};
> >+
> >+static int use_native_backlight_param = NATIVE_BACKLIGHT_NOT_SET;
> >  module_param_named(use_native_backlight, use_native_backlight_param, int, 0444);
> >-static bool use_native_backlight_dmi = true;
> >+static int use_native_backlight_dmi = NATIVE_BACKLIGHT_NOT_SET;
> >
> >  static int register_count;
> >  static struct mutex video_list_lock;
> >@@ -237,15 +243,16 @@ static void acpi_video_switch_brightness(struct work_struct *work);
> >
> >  static bool acpi_video_use_native_backlight(void)
> >  {
> >-	if (use_native_backlight_param != -1)
> >+	if (use_native_backlight_param != NATIVE_BACKLIGHT_NOT_SET)
> >  		return use_native_backlight_param;
> >-	else
> >+	else if (use_native_backlight_dmi != NATIVE_BACKLIGHT_NOT_SET)
> >  		return use_native_backlight_dmi;
> >+	return acpi_osi_is_win8();
> >  }
> >
> >  bool acpi_video_verify_backlight_support(void)
> >  {
> >-	if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
> >+	if (acpi_video_use_native_backlight() &&
> >  	    backlight_device_registered(BACKLIGHT_RAW))
> >  		return false;
> >  	return acpi_video_backlight_support();
> >@@ -414,7 +421,7 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d)
> >
> >  static int __init video_disable_native_backlight(const struct dmi_system_id *d)
> >  {
> >-	use_native_backlight_dmi = false;
> >+	use_native_backlight_dmi = NATIVE_BACKLIGHT_OFF;
> >  	return 0;
> >  }
> >
> >

Re: [PATCH update] acpi: video: Allow forcing native backlight on non win8, machines

[Rafael J. Wysocki]

On Wednesday, March 11, 2015 09:05:03 PM Aaron Lu wrote:
> On Wed, Mar 11, 2015 at 12:14:41PM +0100, Hans de Goede wrote:
> > Hi,
> > 
> > On 11-03-15 07:14, Aaron Lu wrote:
> > >On 03/11/2015 07:10 AM, Rafael J. Wysocki wrote:
> > >>On Tuesday, March 10, 2015 11:42:21 PM Hans de Goede wrote:
> > >>>So you want to have a native_backlight enum or some such ? with -1 being not_set ?
> > >>
> > >>Yes, something like that.
> > >
> > >OK, here is an updated one:
> > 
> > Looks good to me, my ack still stands. I need to send a bunch of
> > mails back / forth to tackle the issue the ideapad which caused me to
> > work on this in the first case. So lets merge this one first, and
> > then I'll get back with a patch on top of this when I know how to
> > actually best deal with the problem on that machine.
> 
> Thanks for the ack and taking care of the problem.
> 
> Regards,
> Aaron
> 
> > 
> > Regards,
> > 
> > Hans
> > 
> > 
> > >
> > > From db4eb1928219e47fcfbffb913f08b1e1b432c28a Mon Sep 17 00:00:00 2001
> > >From: Aaron Lu <aaron.lu@intel.com>
> > >Date: Wed, 4 Mar 2015 10:24:58 +0800
> > >Subject: [PATCH] acpi: video: Allow forcing native backlight on non win8
> > >  machines
> > >
> > >The native backlight behavior (so not registering both the acpi-video
> > >and the vendor backlight driver) can be useful on some non win8 machines
> > >too, so change the behavior of the video.use_native_backlight=1 or 0
> > >kernel cmdline option to be: if user has set video.use_native_backlight=1
> > >or 0, use that no matter if it is a win8 system or not. Also, we will
> > >put some known systems into the DMI table to make them either use native
> > >backlight interface or not, and the use_native_backlight_dmi is used to
> > >reflect that.
> > >
> > >Original-by: Hans de Goede <hdegoede@redhat.com>
> > >Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> > >Acked-by: Hans de Goede <hdegoede@redhat.com>

Queued up for 4.1, thanks!

> > >---
> > >  drivers/acpi/video.c | 19 +++++++++++++------
> > >  1 file changed, 13 insertions(+), 6 deletions(-)
> > >
> > >diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> > >index 26eb70c8f518..2f45dca31724 100644
> > >--- a/drivers/acpi/video.c
> > >+++ b/drivers/acpi/video.c
> > >@@ -82,9 +82,15 @@ 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;
> > >+enum {
> > >+	NATIVE_BACKLIGHT_NOT_SET = -1,
> > >+	NATIVE_BACKLIGHT_OFF,
> > >+	NATIVE_BACKLIGHT_ON,
> > >+};
> > >+
> > >+static int use_native_backlight_param = NATIVE_BACKLIGHT_NOT_SET;
> > >  module_param_named(use_native_backlight, use_native_backlight_param, int, 0444);
> > >-static bool use_native_backlight_dmi = true;
> > >+static int use_native_backlight_dmi = NATIVE_BACKLIGHT_NOT_SET;
> > >
> > >  static int register_count;
> > >  static struct mutex video_list_lock;
> > >@@ -237,15 +243,16 @@ static void acpi_video_switch_brightness(struct work_struct *work);
> > >
> > >  static bool acpi_video_use_native_backlight(void)
> > >  {
> > >-	if (use_native_backlight_param != -1)
> > >+	if (use_native_backlight_param != NATIVE_BACKLIGHT_NOT_SET)
> > >  		return use_native_backlight_param;
> > >-	else
> > >+	else if (use_native_backlight_dmi != NATIVE_BACKLIGHT_NOT_SET)
> > >  		return use_native_backlight_dmi;
> > >+	return acpi_osi_is_win8();
> > >  }
> > >
> > >  bool acpi_video_verify_backlight_support(void)
> > >  {
> > >-	if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
> > >+	if (acpi_video_use_native_backlight() &&
> > >  	    backlight_device_registered(BACKLIGHT_RAW))
> > >  		return false;
> > >  	return acpi_video_backlight_support();
> > >@@ -414,7 +421,7 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d)
> > >
> > >  static int __init video_disable_native_backlight(const struct dmi_system_id *d)
> > >  {
> > >-	use_native_backlight_dmi = false;
> > >+	use_native_backlight_dmi = NATIVE_BACKLIGHT_OFF;
> > >  	return 0;
> > >  }
> > >
> > >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[RFC PATCH 2/2] acpi: video: Add force native backlight quirk for Lenovo Ideapad Z570

[Hans de Goede]

The Lenovo Ideapad Z570 (which is an Acer in disguise like some other Ideapads)
has a broken acpi_video interface and is too old for the acpi-video code to
automatically prefer the native backlight interface, so add a quirk for it.

Note that this cannot be done with a quirk in the vendor backlight driver as
is the normal way to fix this as the Z570 offers both acer-wmi and
ideapad-laptop vendor backlight interfaces, so this would require the same
quirk in 2 places which is the wrong thing to do.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1187004
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/video.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 9817b52..70a7ec3 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -419,6 +419,12 @@ static int __init video_disable_native_backlight(const struct dmi_system_id *d)
 	return 0;
 }
 
+static int __init video_force_native_backlight(const struct dmi_system_id *d)
+{
+	use_native_backlight_dmi = 2;
+	return 0;
+}
+
 static struct dmi_system_id video_dmi_table[] __initdata = {
 	/*
 	 * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121
@@ -560,6 +566,17 @@ static struct dmi_system_id video_dmi_table[] __initdata = {
 		DMI_MATCH(DMI_PRODUCT_NAME, "XPS L521X"),
 		},
 	},
+
+	/* Non win8 machines which need native backlight nevertheless */
+	{
+	 /* https://bugzilla.redhat.com/show_bug.cgi?id=1187004 */
+	 .callback = video_force_native_backlight,
+	 .ident = "Lenovo Ideapad Z570",
+	 .matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+		DMI_MATCH(DMI_PRODUCT_NAME, "102434U"),
+		},
+	},
 	{}
 };
 
-- 
2.3.1

Re: [RFC PATCH 2/2] acpi: video: Add force native backlight quirk for Lenovo Ideapad Z570

[Rafael J. Wysocki]

On Tuesday, March 03, 2015 08:39:10 AM Hans de Goede wrote:
> The Lenovo Ideapad Z570 (which is an Acer in disguise like some other Ideapads)
> has a broken acpi_video interface and is too old for the acpi-video code to
> automatically prefer the native backlight interface, so add a quirk for it.
> 
> Note that this cannot be done with a quirk in the vendor backlight driver as
> is the normal way to fix this as the Z570 offers both acer-wmi and
> ideapad-laptop vendor backlight interfaces, so this would require the same
> quirk in 2 places which is the wrong thing to do.
> 
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1187004
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

This one would need to be updated on top of the Aaron's patch, right?

> ---
>  drivers/acpi/video.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 9817b52..70a7ec3 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -419,6 +419,12 @@ static int __init video_disable_native_backlight(const struct dmi_system_id *d)
>  	return 0;
>  }
>  
> +static int __init video_force_native_backlight(const struct dmi_system_id *d)
> +{
> +	use_native_backlight_dmi = 2;
> +	return 0;
> +}
> +
>  static struct dmi_system_id video_dmi_table[] __initdata = {
>  	/*
>  	 * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121
> @@ -560,6 +566,17 @@ static struct dmi_system_id video_dmi_table[] __initdata = {
>  		DMI_MATCH(DMI_PRODUCT_NAME, "XPS L521X"),
>  		},
>  	},
> +
> +	/* Non win8 machines which need native backlight nevertheless */
> +	{
> +	 /* https://bugzilla.redhat.com/show_bug.cgi?id=1187004 */
> +	 .callback = video_force_native_backlight,
> +	 .ident = "Lenovo Ideapad Z570",
> +	 .matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +		DMI_MATCH(DMI_PRODUCT_NAME, "102434U"),
> +		},
> +	},
>  	{}
>  };
>  
> 

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

Re: [RFC PATCH 2/2] acpi: video: Add force native backlight quirk for Lenovo Ideapad Z570

[Hans de Goede]

Hi,

On 03/10/2015 11:26 PM, Rafael J. Wysocki wrote:
> On Tuesday, March 03, 2015 08:39:10 AM Hans de Goede wrote:
>> The Lenovo Ideapad Z570 (which is an Acer in disguise like some other Ideapads)
>> has a broken acpi_video interface and is too old for the acpi-video code to
>> automatically prefer the native backlight interface, so add a quirk for it.
>>
>> Note that this cannot be done with a quirk in the vendor backlight driver as
>> is the normal way to fix this as the Z570 offers both acer-wmi and
>> ideapad-laptop vendor backlight interfaces, so this would require the same
>> quirk in 2 places which is the wrong thing to do.
>>
>> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1187004
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> This one would need to be updated on top of the Aaron's patch, right?

Correct, and it seems to need some more work in general as it conflicts
with a patch to video-detect.c which is already upstream for this laptop model,
but does not seem to 100% fix the problem.

Regards,

Hans

>
>> ---
>>   drivers/acpi/video.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
>> index 9817b52..70a7ec3 100644
>> --- a/drivers/acpi/video.c
>> +++ b/drivers/acpi/video.c
>> @@ -419,6 +419,12 @@ static int __init video_disable_native_backlight(const struct dmi_system_id *d)
>>   	return 0;
>>   }
>>
>> +static int __init video_force_native_backlight(const struct dmi_system_id *d)
>> +{
>> +	use_native_backlight_dmi = 2;
>> +	return 0;
>> +}
>> +
>>   static struct dmi_system_id video_dmi_table[] __initdata = {
>>   	/*
>>   	 * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121
>> @@ -560,6 +566,17 @@ static struct dmi_system_id video_dmi_table[] __initdata = {
>>   		DMI_MATCH(DMI_PRODUCT_NAME, "XPS L521X"),
>>   		},
>>   	},
>> +
>> +	/* Non win8 machines which need native backlight nevertheless */
>> +	{
>> +	 /* https://bugzilla.redhat.com/show_bug.cgi?id=1187004 */
>> +	 .callback = video_force_native_backlight,
>> +	 .ident = "Lenovo Ideapad Z570",
>> +	 .matches = {
>> +		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> +		DMI_MATCH(DMI_PRODUCT_NAME, "102434U"),
>> +		},
>> +	},
>>   	{}
>>   };
>>
>>
>

Re: [RFC PATCH 0/2] acpi: video: Allow forcing native backlight on non win8 laptops

[Aaron Lu]

On 03/03/2015 03:39 PM, Hans de Goede wrote:
> Hi All,
> 
> I've long wanted to have a way for users to force native-backlight behavior as
> that makes debugging backlight problems a bit easier.

I agree.

> 
> And now a laptop has shown up which needs native backlight behavior for things
> to work, has a non win8 ready bios, and I cannot use a quirk in the vendor
> backlight driver as it implements both the acer-wmi and ideapad-laptop vendor
> interfaces and putting the same quirk in 2 places is just wrong.
> 
> So I've finally written a small patch to allow forcing native-backlight
> behavior as well as a second patch using this to add a quirk for said laptop.
> 
> For now this is just a RFC series as I'm waiting for confirmation from the
> user that this actually fixes things, but I wanted to get the discussion going
> beforehand in case anyone has any objections against the solution I've come
> up with.

See my comments for patch 1.

Thanks,
Aaron

> 
> Regards,
> 
> Hans
>