[PATCH v5 0/4] Fix Win8 backlight issue

2013-10-30 Thread Rafael J. Wysocki 
On Wednesday, October 30, 2013 12:40:13 AM Matthew Garrett wrote:
> On Wed, 2013-10-16 at 01:33 +0200, Rafael J. Wysocki wrote:
> 
> > Since the next step will be to introduce a list of systems that need
> > video.use_native_backlight=1 *and* don't break in that configuration, I 
> > don't
> > see much point adding another Kconfig option for the default.
> 
> I'd still really prefer not to add such a list, because it almost
> inevitably means that we'll never fix this problem properly.

We have this list in blacklist.c anyway.

Thanks!

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

[PATCH v5 0/4] Fix Win8 backlight issue

2013-10-30 Thread Matthew Garrett 
On Wed, 2013-10-16 at 01:33 +0200, Rafael J. Wysocki wrote:

> Since the next step will be to introduce a list of systems that need
> video.use_native_backlight=1 *and* don't break in that configuration, I don't
> see much point adding another Kconfig option for the default.

I'd still really prefer not to add such a list, because it almost
inevitably means that we'll never fix this problem properly.

-- 
Matthew Garrett

[PATCH v5 0/4] Fix Win8 backlight issue

2013-10-29 Thread Aaron Lu 
On 10/28/2013 04:09 PM, Jani Nikula wrote:
> On Mon, 28 Oct 2013, Aaron Lu  wrote:
>> +static int __init video_set_use_native_backlight(const struct dmi_system_id 
>> *d)
>> +{
>> +use_native_backlight = true;
>> +return 0;
>> +}
> 
> Hi Aaron, it might be beneficial to make use_native_backlight support
> three values: force true, force false, and use platform default based on
> DMI.

Makes sense. I modified the patch a little bit so that if user has
specified the cmdline option use_native_backlight=0/1, it will always
take effect no matter if the system is in DMI table or not.

From: Aaron Lu 
Subject: [PATCH] ACPI / video: Add systems that should favor native backlight
 interface

Some system's ACPI video backlight control interface is broken and the
native backlight control interface should be used by default. This patch
sets the use_native_backlight parameter to true for those systems so
that video backlight control interface will not be created. To be
specific, the ThinkPad T430s/X230, Lenovo Yoga 13 and Dell Inspiron 7520
are added here.

Note that the user specified kernel cmdline option will always have the
highest priority, i.e. if use_native_backlight=0 is specified and the
system is in the DMI table, the video module will not skip register
backlight interface for it.

Thinkpad T430s:
Reported-by: Theodore Tso 
Reported-and-tested-by: Peter Weber 
Thinkpad X230:
Reported-and-tested-by: Igor Gnatenko 
Lenovo Yoga 13:
Reported-by: Lennart Poettering 
Reported-and-tested-by: Kevin Smith 
Dell Inspiron 7520:
Reported-by: Rinat Ibragimov 

Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231
Signed-off-by: Aaron Lu 
---
 drivers/acpi/video.c | 57 +++-
 1 file changed, 52 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 38c3a28..41bd4b4 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -89,11 +89,12 @@ static bool use_bios_initial_backlight = 1;
 module_param(use_bios_initial_backlight, bool, 0644);

 /*
- * For Windows 8 systems: if set ture and the GPU driver has
- * registered a backlight interface, skip registering ACPI video's.
+ * For Windows 8 systems: used to decide if video module
+ * should skip registering backlight interface of its own.
  */
-static bool use_native_backlight = false;
-module_param(use_native_backlight, 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 = false;

 static int register_count;
 static struct mutex video_list_lock;
@@ -239,9 +240,17 @@ 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() && use_native_backlight &&
+   if (acpi_osi_is_win8() && acpi_video_use_native_backlight() &&
backlight_device_registered(BACKLIGHT_RAW))
return false;
return acpi_video_backlight_support();
@@ -412,6 +421,12 @@ static int video_ignore_initial_backlight(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
@@ -512,6 +527,38 @@ static struct dmi_system_id video_dmi_table[] __initdata = 
{
DMI_MATCH(DMI_PRODUCT_NAME, "HP 250 G1 Notebook PC"),
},
},
+   {
+.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 = "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 = {
+

[PATCH v5 0/4] Fix Win8 backlight issue

2013-10-28 Thread Aaron Lu 
On 10/25/2013 02:35 PM, Igor Gnatenko wrote:
> Aaron, add this notebook to list. I've CC'ed owner.
> And I've tested this patch on my TP X230 (add as Reported-and-Tested me
> please)
> + {
> +  .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"),
> + },
> + },

Thanks Igor, updated patch follows:

From: Aaron Lu 
Subject: [PATCH] ACPI / video: Add systems that should favour native backlight
 interface

Some system's ACPI video backlight control interface is broken and the
native backlight control interface should be used by default. This patch
sets the use_native_backlight parameter to true for those systems so
that video backlight control interface will not be created. To be
specific, the ThinkPad T430s/X230, Lenovo Yoga 13 and Dell Inspiron 7520
are added here.

Thinkpad T430s:
Reported-by: Theodore Tso 
Reported-and-tested-by: Peter Weber 
Thinkpad X230:
Reported-and-tested-by: Igor Gnatenko 
Lenovo Yoga 13:
Reported-by: Lennart Poettering 
Reported-and-tested-by: Kevin Smith 
Dell Inspiron 7520:
Reported-by: Renat Ibragimov 

Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231
Signed-off-by: Aaron Lu 
---
 drivers/acpi/video.c | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index d020df5..0a1b030 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -412,6 +412,12 @@ static int video_ignore_initial_backlight(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 = true;
+   return 0;
+}
+
 static struct dmi_system_id video_dmi_table[] __initdata = {
/*
 * Broken _BQC workaround 
http://bugzilla.kernel.org/show_bug.cgi?id=13121
@@ -504,6 +510,38 @@ static struct dmi_system_id video_dmi_table[] __initdata = 
{
DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion m4 Notebook PC"),
},
},
+   {
+.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 = "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"),
+   },
+   },
{}
 };

-- 
1.8.4.39.ga0d3f10

[PATCH v5 0/4] Fix Win8 backlight issue

2013-10-28 Thread Jani Nikula 
On Mon, 28 Oct 2013, Aaron Lu  wrote:
> On 10/25/2013 02:35 PM, Igor Gnatenko wrote:
>> Aaron, add this notebook to list. I've CC'ed owner.
>> And I've tested this patch on my TP X230 (add as Reported-and-Tested me
>> please)
>> +{
>> + .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"),
>> +},
>> +},
>
> Thanks Igor, updated patch follows:
>
> From: Aaron Lu 
> Subject: [PATCH] ACPI / video: Add systems that should favour native backlight
>  interface
>
> Some system's ACPI video backlight control interface is broken and the
> native backlight control interface should be used by default. This patch
> sets the use_native_backlight parameter to true for those systems so
> that video backlight control interface will not be created. To be
> specific, the ThinkPad T430s/X230, Lenovo Yoga 13 and Dell Inspiron 7520
> are added here.
>
> Thinkpad T430s:
> Reported-by: Theodore Tso 
> Reported-and-tested-by: Peter Weber 
> Thinkpad X230:
> Reported-and-tested-by: Igor Gnatenko 
> Lenovo Yoga 13:
> Reported-by: Lennart Poettering 
> Reported-and-tested-by: Kevin Smith 
> Dell Inspiron 7520:
> Reported-by: Renat Ibragimov 
>
> Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231
> Signed-off-by: Aaron Lu 
> ---
>  drivers/acpi/video.c | 38 ++
>  1 file changed, 38 insertions(+)
>
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index d020df5..0a1b030 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -412,6 +412,12 @@ static int video_ignore_initial_backlight(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 = true;
> + return 0;
> +}

Hi Aaron, it might be beneficial to make use_native_backlight support
three values: force true, force false, and use platform default based on
DMI.

BR,
Jani.


> +
>  static struct dmi_system_id video_dmi_table[] __initdata = {
>   /*
>* Broken _BQC workaround 
> http://bugzilla.kernel.org/show_bug.cgi?id=13121
> @@ -504,6 +510,38 @@ static struct dmi_system_id video_dmi_table[] __initdata 
> = {
>   DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion m4 Notebook PC"),
>   },
>   },
> + {
> +  .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 = "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"),
> + },
> + },
>   {}
>  };
>  
> -- 
> 1.8.4.39.ga0d3f10
>

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH v5 0/4] Fix Win8 backlight issue

2013-10-25 Thread Igor Gnatenko 
On Thu, 2013-10-24 at 16:13 +0800, Aaron Lu wrote:
> On 10/16/2013 07:33 AM, Rafael J. Wysocki wrote:
> > On Friday, October 11, 2013 09:27:42 PM Aaron Lu wrote:
> >> v5:
> >> 1 Introduce video.use_native_backlight module parameter and set its
> >>   value to false by default as suggested by Rafael. For Win8 systems
> >>   which have broken ACPI video backlight control, the parameter can be
> >>   set to 1 in kernel cmdline to skip registering ACPI video's backlight
> >>   interface. Due to this change, the acpi_video_verify_backlight_support
> >>   is moved from video_detect.c to video.c - patch 3/4;
> >> 2 Rename bd_list_head and bd_list_mutex in backlight.c to
> >>   backlight_dev_list and backlight_dev_list_mutex as suggested by Rafael
> >>   - patch 1/4.
> >>
> >> v4:
> >> Remove decleration and stub for acpi_video_unregister_backlight in
> >> video.h of patch 2/4 since that function doesn't exist anymore in v3.
> >>
> >> v3:
> >> 1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module;
> >> 2 Remove unnecessary function acpi_video_unregister_backlight introduced
> >>   in patch 2/4 as pointed out by Jani Nikula.
> >>
> >> v2:
> >> v1 has the subject of "Rework ACPI video driver" and is posted here:
> >> http://lkml.org/lkml/2013/9/9/74
> >> Since the objective is really to fix Win8 backlight issues, I changed
> >> the subject in this version, sorry about that.
> >>
> >> This patchset has four patches, the first introduced a new API named
> >> backlight_device_registered in backlight layer that can be used for
> >> backlight interface provider module to check if a specific type backlight
> >> interface has been registered, see changelog for patch 1/4 for details.
> >> Then patch 2/4 does the cleanup to sepeate the backlight control and
> >> event delivery functionality in the ACPI video module and patch 3/4
> >> solves some Win8 backlight control problems by avoiding register ACPI
> >> video's backlight interface if:
> >> 1 Kernel cmdline option acpi_backlight=video is not given;
> >> 2 This is a Win8 system;
> >> 3 Native backlight control interface exists.
> >> Patch 4/4 fixes some problems in thinkpad-acpi module.
> >>
> >> Technically, patch 2/4 is not required to fix the issue here. So if you
> >> think it is not necessary, I can remove it from the series.
> >>
> >> Aaron Lu (4):
> >>   backlight: introduce backlight_device_registered
> >>   ACPI / video: seperate backlight control and event interface
> >>   ACPI / video: Do not register backlight if win8 and native interface
> >> exists
> >>   thinkpad-acpi: fix handle locate for video and query of _BCL
> >>
> >>  drivers/acpi/internal.h  |   4 +-
> >>  drivers/acpi/video.c | 457 
> >> ---
> >>  drivers/acpi/video_detect.c  |   4 +-
> >>  drivers/platform/x86/thinkpad_acpi.c |  31 ++-
> >>  drivers/video/backlight/backlight.c  |  31 +++
> >>  include/linux/backlight.h|   4 +
> >>  6 files changed, 326 insertions(+), 205 deletions(-)
> > 
> > I've added this series to my queue for 3.13.
> > 
> > Since the next step will be to introduce a list of systems that need
> > video.use_native_backlight=1 *and* don't break in that configuration, I 
> > don't
> > see much point adding another Kconfig option for the default.
> > 
> > Hopefully, in the future we'll be able to fix the problems causing
> > video.use_native_backlight=1 to fail of the systems where it fails and then
> > we'll be able to make that the default behavior and drop the option 
> > altogether.
> 
> I've prepared a patch(at the end of the mail) to set use_native_backlight
> by default for some systems. There are 3 systems currently that I'm
> kind of sure that should be added:
> 
> The ThinkPad T430s and X230 is:
> Reported-by: Theodore Tso 
> Reported-and-tested-by: Peter Weber 
> Reported-by: Igor Gnatenko 
> Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231
> 
> The Lenovo Yoga is:
> Reported-by: Lennart Poettering 
> Reference: https://lkml.org/lkml/2013/10/13/178
> 
> From: Aaron Lu 
> Subject: [PATCH] ACPI / video: Add systems that should favor native backlight
>  interface
> 
> Some system's ACPI video backlight control interface is broken and the
> native backlight control interface should be used by default. This patch
> sets the use_native_backlight parameter to true for those systems so
> that video backlight control interface will not be created. To be
> clear, the ThinkPad T430s/X230 and Lenovo Yoga 13 are added here.
> 
> Reported-by: Theodore Tso 
> Reported-and-tested-by: Peter Weber 
> Reported-by: Lennart Poettering 
> Reported-by: Igor Gnatenko 
> Signed-off-by: Aaron Lu 
> ---
>  drivers/acpi/video.c | 30 ++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index d020df5..9a80a94 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -412,6 +412,12 @@ static int

[PATCH v5 0/4] Fix Win8 backlight issue

2013-10-24 Thread Aaron Lu 
On 10/16/2013 07:33 AM, Rafael J. Wysocki wrote:
> On Friday, October 11, 2013 09:27:42 PM Aaron Lu wrote:
>> v5:
>> 1 Introduce video.use_native_backlight module parameter and set its
>>   value to false by default as suggested by Rafael. For Win8 systems
>>   which have broken ACPI video backlight control, the parameter can be
>>   set to 1 in kernel cmdline to skip registering ACPI video's backlight
>>   interface. Due to this change, the acpi_video_verify_backlight_support
>>   is moved from video_detect.c to video.c - patch 3/4;
>> 2 Rename bd_list_head and bd_list_mutex in backlight.c to
>>   backlight_dev_list and backlight_dev_list_mutex as suggested by Rafael
>>   - patch 1/4.
>>
>> v4:
>> Remove decleration and stub for acpi_video_unregister_backlight in
>> video.h of patch 2/4 since that function doesn't exist anymore in v3.
>>
>> v3:
>> 1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module;
>> 2 Remove unnecessary function acpi_video_unregister_backlight introduced
>>   in patch 2/4 as pointed out by Jani Nikula.
>>
>> v2:
>> v1 has the subject of "Rework ACPI video driver" and is posted here:
>> http://lkml.org/lkml/2013/9/9/74
>> Since the objective is really to fix Win8 backlight issues, I changed
>> the subject in this version, sorry about that.
>>
>> This patchset has four patches, the first introduced a new API named
>> backlight_device_registered in backlight layer that can be used for
>> backlight interface provider module to check if a specific type backlight
>> interface has been registered, see changelog for patch 1/4 for details.
>> Then patch 2/4 does the cleanup to sepeate the backlight control and
>> event delivery functionality in the ACPI video module and patch 3/4
>> solves some Win8 backlight control problems by avoiding register ACPI
>> video's backlight interface if:
>> 1 Kernel cmdline option acpi_backlight=video is not given;
>> 2 This is a Win8 system;
>> 3 Native backlight control interface exists.
>> Patch 4/4 fixes some problems in thinkpad-acpi module.
>>
>> Technically, patch 2/4 is not required to fix the issue here. So if you
>> think it is not necessary, I can remove it from the series.
>>
>> Aaron Lu (4):
>>   backlight: introduce backlight_device_registered
>>   ACPI / video: seperate backlight control and event interface
>>   ACPI / video: Do not register backlight if win8 and native interface
>> exists
>>   thinkpad-acpi: fix handle locate for video and query of _BCL
>>
>>  drivers/acpi/internal.h  |   4 +-
>>  drivers/acpi/video.c | 457 
>> ---
>>  drivers/acpi/video_detect.c  |   4 +-
>>  drivers/platform/x86/thinkpad_acpi.c |  31 ++-
>>  drivers/video/backlight/backlight.c  |  31 +++
>>  include/linux/backlight.h|   4 +
>>  6 files changed, 326 insertions(+), 205 deletions(-)
> 
> I've added this series to my queue for 3.13.
> 
> Since the next step will be to introduce a list of systems that need
> video.use_native_backlight=1 *and* don't break in that configuration, I don't
> see much point adding another Kconfig option for the default.
> 
> Hopefully, in the future we'll be able to fix the problems causing
> video.use_native_backlight=1 to fail of the systems where it fails and then
> we'll be able to make that the default behavior and drop the option 
> altogether.

I've prepared a patch(at the end of the mail) to set use_native_backlight
by default for some systems. There are 3 systems currently that I'm
kind of sure that should be added:

The ThinkPad T430s and X230 is:
Reported-by: Theodore Tso 
Reported-and-tested-by: Peter Weber 
Reported-by: Igor Gnatenko 
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231

The Lenovo Yoga is:
Reported-by: Lennart Poettering 
Reference: https://lkml.org/lkml/2013/10/13/178

From: Aaron Lu 
Subject: [PATCH] ACPI / video: Add systems that should favor native backlight
 interface

Some system's ACPI video backlight control interface is broken and the
native backlight control interface should be used by default. This patch
sets the use_native_backlight parameter to true for those systems so
that video backlight control interface will not be created. To be
clear, the ThinkPad T430s/X230 and Lenovo Yoga 13 are added here.

Reported-by: Theodore Tso 
Reported-and-tested-by: Peter Weber 
Reported-by: Lennart Poettering 
Reported-by: Igor Gnatenko 
Signed-off-by: Aaron Lu 
---
 drivers/acpi/video.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index d020df5..9a80a94 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -412,6 +412,12 @@ static int video_ignore_initial_backlight(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 = true;
+   return 0;
+}
+
 static struct dmi_system_id

[PATCH v5 0/4] Fix Win8 backlight issue

2013-10-16 Thread Rafael J. Wysocki 
On Friday, October 11, 2013 09:27:42 PM Aaron Lu wrote:
> v5:
> 1 Introduce video.use_native_backlight module parameter and set its
>   value to false by default as suggested by Rafael. For Win8 systems
>   which have broken ACPI video backlight control, the parameter can be
>   set to 1 in kernel cmdline to skip registering ACPI video's backlight
>   interface. Due to this change, the acpi_video_verify_backlight_support
>   is moved from video_detect.c to video.c - patch 3/4;
> 2 Rename bd_list_head and bd_list_mutex in backlight.c to
>   backlight_dev_list and backlight_dev_list_mutex as suggested by Rafael
>   - patch 1/4.
> 
> v4:
> Remove decleration and stub for acpi_video_unregister_backlight in
> video.h of patch 2/4 since that function doesn't exist anymore in v3.
> 
> v3:
> 1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module;
> 2 Remove unnecessary function acpi_video_unregister_backlight introduced
>   in patch 2/4 as pointed out by Jani Nikula.
> 
> v2:
> v1 has the subject of "Rework ACPI video driver" and is posted here:
> http://lkml.org/lkml/2013/9/9/74
> Since the objective is really to fix Win8 backlight issues, I changed
> the subject in this version, sorry about that.
> 
> This patchset has four patches, the first introduced a new API named
> backlight_device_registered in backlight layer that can be used for
> backlight interface provider module to check if a specific type backlight
> interface has been registered, see changelog for patch 1/4 for details.
> Then patch 2/4 does the cleanup to sepeate the backlight control and
> event delivery functionality in the ACPI video module and patch 3/4
> solves some Win8 backlight control problems by avoiding register ACPI
> video's backlight interface if:
> 1 Kernel cmdline option acpi_backlight=video is not given;
> 2 This is a Win8 system;
> 3 Native backlight control interface exists.
> Patch 4/4 fixes some problems in thinkpad-acpi module.
> 
> Technically, patch 2/4 is not required to fix the issue here. So if you
> think it is not necessary, I can remove it from the series.
> 
> Aaron Lu (4):
>   backlight: introduce backlight_device_registered
>   ACPI / video: seperate backlight control and event interface
>   ACPI / video: Do not register backlight if win8 and native interface
> exists
>   thinkpad-acpi: fix handle locate for video and query of _BCL
> 
>  drivers/acpi/internal.h  |   4 +-
>  drivers/acpi/video.c | 457 
> ---
>  drivers/acpi/video_detect.c  |   4 +-
>  drivers/platform/x86/thinkpad_acpi.c |  31 ++-
>  drivers/video/backlight/backlight.c  |  31 +++
>  include/linux/backlight.h|   4 +
>  6 files changed, 326 insertions(+), 205 deletions(-)

I've added this series to my queue for 3.13.

Since the next step will be to introduce a list of systems that need
video.use_native_backlight=1 *and* don't break in that configuration, I don't
see much point adding another Kconfig option for the default.

Hopefully, in the future we'll be able to fix the problems causing
video.use_native_backlight=1 to fail of the systems where it fails and then
we'll be able to make that the default behavior and drop the option altogether.

Thanks!

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

[PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Aaron Lu 
v5:
1 Introduce video.use_native_backlight module parameter and set its
  value to false by default as suggested by Rafael. For Win8 systems
  which have broken ACPI video backlight control, the parameter can be
  set to 1 in kernel cmdline to skip registering ACPI video's backlight
  interface. Due to this change, the acpi_video_verify_backlight_support
  is moved from video_detect.c to video.c - patch 3/4;
2 Rename bd_list_head and bd_list_mutex in backlight.c to
  backlight_dev_list and backlight_dev_list_mutex as suggested by Rafael
  - patch 1/4.

v4:
Remove decleration and stub for acpi_video_unregister_backlight in
video.h of patch 2/4 since that function doesn't exist anymore in v3.

v3:
1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module;
2 Remove unnecessary function acpi_video_unregister_backlight introduced
  in patch 2/4 as pointed out by Jani Nikula.

v2:
v1 has the subject of Rework ACPI video driver and is posted here:
http://lkml.org/lkml/2013/9/9/74
Since the objective is really to fix Win8 backlight issues, I changed
the subject in this version, sorry about that.

This patchset has four patches, the first introduced a new API named
backlight_device_registered in backlight layer that can be used for
backlight interface provider module to check if a specific type backlight
interface has been registered, see changelog for patch 1/4 for details.
Then patch 2/4 does the cleanup to sepeate the backlight control and
event delivery functionality in the ACPI video module and patch 3/4
solves some Win8 backlight control problems by avoiding register ACPI
video's backlight interface if:
1 Kernel cmdline option acpi_backlight=video is not given;
2 This is a Win8 system;
3 Native backlight control interface exists.
Patch 4/4 fixes some problems in thinkpad-acpi module.

Technically, patch 2/4 is not required to fix the issue here. So if you
think it is not necessary, I can remove it from the series.

Aaron Lu (4):
  backlight: introduce backlight_device_registered
  ACPI / video: seperate backlight control and event interface
  ACPI / video: Do not register backlight if win8 and native interface
exists
  thinkpad-acpi: fix handle locate for video and query of _BCL

 drivers/acpi/internal.h  |   4 +-
 drivers/acpi/video.c | 457 ---
 drivers/acpi/video_detect.c  |   4 +-
 drivers/platform/x86/thinkpad_acpi.c |  31 ++-
 drivers/video/backlight/backlight.c  |  31 +++
 include/linux/backlight.h|   4 +
 6 files changed, 326 insertions(+), 205 deletions(-)

-- 
1.8.4.12.g2ea3df6

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Josh Boyer 
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote:
 v5:
 1 Introduce video.use_native_backlight module parameter and set its
   value to false by default as suggested by Rafael. For Win8 systems
   which have broken ACPI video backlight control, the parameter can be
   set to 1 in kernel cmdline to skip registering ACPI video's backlight
   interface. Due to this change, the acpi_video_verify_backlight_support
   is moved from video_detect.c to video.c - patch 3/4;

That's a fairly untenable position for distro kernels to be in.  They
now have to ask every user that reports an issue with the backlight to
try setting that option on the command line.  While I appreciate the
setting breaks things for some people, doesn't the Win8 issue impact
far more people?  Shouldn't it be defaulted to true?

If nothing else, can you add a config option for the default so
distros can use that to decide which way to default it and then work
on fixing the remaining users that have troubles?

josh
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Igor Gnatenko 
On Fri, 2013-10-11 at 12:42 -0400, Josh Boyer wrote:
 On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote:
  v5:
  1 Introduce video.use_native_backlight module parameter and set its
value to false by default as suggested by Rafael. For Win8 systems
which have broken ACPI video backlight control, the parameter can be
set to 1 in kernel cmdline to skip registering ACPI video's backlight
interface. Due to this change, the acpi_video_verify_backlight_support
is moved from video_detect.c to video.c - patch 3/4;
 
 That's a fairly untenable position for distro kernels to be in.  They
 now have to ask every user that reports an issue with the backlight to
 try setting that option on the command line.  While I appreciate the
 setting breaks things for some people, doesn't the Win8 issue impact
 far more people?  Shouldn't it be defaulted to true?
 
 If nothing else, can you add a config option for the default so
 distros can use that to decide which way to default it and then work
 on fixing the remaining users that have troubles?
 
 josh

I think more better to use this unregister as default and give option to
disable it.

-- 
Igor Gnatenko
Fedora release 20 (Heisenbug)
Linux 3.11.4-301.fc20.x86_64

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Rafael J. Wysocki 
On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
 On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote:
  v5:
  1 Introduce video.use_native_backlight module parameter and set its
value to false by default as suggested by Rafael. For Win8 systems
which have broken ACPI video backlight control, the parameter can be
set to 1 in kernel cmdline to skip registering ACPI video's backlight
interface. Due to this change, the acpi_video_verify_backlight_support
is moved from video_detect.c to video.c - patch 3/4;
 
 That's a fairly untenable position for distro kernels to be in.  They
 now have to ask every user that reports an issue with the backlight to
 try setting that option on the command line.  While I appreciate the
 setting breaks things for some people, doesn't the Win8 issue impact
 far more people?  Shouldn't it be defaulted to true?

Well, we have a rule in the kernel not to introduce regressions for users even
if they are minority.

 If nothing else, can you add a config option for the default so
 distros can use that to decide which way to default it and then work
 on fixing the remaining users that have troubles?

The current plan is to create a blacklist of systems where that option should
be set.  We actually already have one, but it is at the _OSI() level, which
is overkill in my view and may affect things beyond backlight.  Along with that
we will debug systems where setting that option (to true) causes problems to
happen, so that we'll be able to drop it going forward (hopefully).

Of course, distro kernels may always change the default to true if they want.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Josh Boyer 
On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki r...@rjwysocki.net wrote:
 On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
 On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote:
  v5:
  1 Introduce video.use_native_backlight module parameter and set its
value to false by default as suggested by Rafael. For Win8 systems
which have broken ACPI video backlight control, the parameter can be
set to 1 in kernel cmdline to skip registering ACPI video's backlight
interface. Due to this change, the acpi_video_verify_backlight_support
is moved from video_detect.c to video.c - patch 3/4;

 That's a fairly untenable position for distro kernels to be in.  They
 now have to ask every user that reports an issue with the backlight to
 try setting that option on the command line.  While I appreciate the
 setting breaks things for some people, doesn't the Win8 issue impact
 far more people?  Shouldn't it be defaulted to true?

 Well, we have a rule in the kernel not to introduce regressions for users even
 if they are minority.

 If nothing else, can you add a config option for the default so
 distros can use that to decide which way to default it and then work
 on fixing the remaining users that have troubles?

 The current plan is to create a blacklist of systems where that option should
 be set.  We actually already have one, but it is at the _OSI() level, which
 is overkill in my view and may affect things beyond backlight.  Along with 
 that
 we will debug systems where setting that option (to true) causes problems to
 happen, so that we'll be able to drop it going forward (hopefully).

 Of course, distro kernels may always change the default to true if they want.

They can, but they'd need to either patch the kernel to do so, or code
it in userspace bootloader configs.  Having a config option they can
set to change the default makes it reasonable and contained within the
kernel.

josh
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Rafael J. Wysocki 
On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote:
 On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki r...@rjwysocki.net wrote:
  On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
  On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote:
   v5:
   1 Introduce video.use_native_backlight module parameter and set its
 value to false by default as suggested by Rafael. For Win8 systems
 which have broken ACPI video backlight control, the parameter can be
 set to 1 in kernel cmdline to skip registering ACPI video's backlight
 interface. Due to this change, the acpi_video_verify_backlight_support
 is moved from video_detect.c to video.c - patch 3/4;
 
  That's a fairly untenable position for distro kernels to be in.  They
  now have to ask every user that reports an issue with the backlight to
  try setting that option on the command line.  While I appreciate the
  setting breaks things for some people, doesn't the Win8 issue impact
  far more people?  Shouldn't it be defaulted to true?
 
  Well, we have a rule in the kernel not to introduce regressions for users 
  even
  if they are minority.
 
  If nothing else, can you add a config option for the default so
  distros can use that to decide which way to default it and then work
  on fixing the remaining users that have troubles?
 
  The current plan is to create a blacklist of systems where that option 
  should
  be set.  We actually already have one, but it is at the _OSI() level, which
  is overkill in my view and may affect things beyond backlight.  Along with 
  that
  we will debug systems where setting that option (to true) causes problems to
  happen, so that we'll be able to drop it going forward (hopefully).
 
  Of course, distro kernels may always change the default to true if they 
  want.
 
 They can, but they'd need to either patch the kernel to do so, or code
 it in userspace bootloader configs.  Having a config option they can
 set to change the default makes it reasonable and contained within the
 kernel.

Well, adding a Kconfig option should be simple enough, but I'm not sure
I understand the point.  You'll still need to rebuild the kernel after changing
that option.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Igor Gnatenko 
On Sat, 2013-10-12 at 00:45 +0200, Rafael J. Wysocki wrote:
 On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote:
  On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki r...@rjwysocki.net 
  wrote:
   On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
   On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote:
v5:
1 Introduce video.use_native_backlight module parameter and set its
  value to false by default as suggested by Rafael. For Win8 systems
  which have broken ACPI video backlight control, the parameter can be
  set to 1 in kernel cmdline to skip registering ACPI video's backlight
  interface. Due to this change, the 
acpi_video_verify_backlight_support
  is moved from video_detect.c to video.c - patch 3/4;
  
   That's a fairly untenable position for distro kernels to be in.  They
   now have to ask every user that reports an issue with the backlight to
   try setting that option on the command line.  While I appreciate the
   setting breaks things for some people, doesn't the Win8 issue impact
   far more people?  Shouldn't it be defaulted to true?
  
   Well, we have a rule in the kernel not to introduce regressions for users 
   even
   if they are minority.
  
   If nothing else, can you add a config option for the default so
   distros can use that to decide which way to default it and then work
   on fixing the remaining users that have troubles?
  
   The current plan is to create a blacklist of systems where that option 
   should
   be set.  We actually already have one, but it is at the _OSI() level, 
   which
   is overkill in my view and may affect things beyond backlight.  Along 
   with that
   we will debug systems where setting that option (to true) causes problems 
   to
   happen, so that we'll be able to drop it going forward (hopefully).
  
   Of course, distro kernels may always change the default to true if they 
   want.
  
  They can, but they'd need to either patch the kernel to do so, or code
  it in userspace bootloader configs.  Having a config option they can
  set to change the default makes it reasonable and contained within the
  kernel.
 
 Well, adding a Kconfig option should be simple enough, but I'm not sure
 I understand the point.  You'll still need to rebuild the kernel after 
 changing
 that option.
 

I think he meant that we should have Kconfig option alike
CONFIG_I915_BACKLIGHT_UNREGISTER which have true or false.
If it true - unregister acpi backlight as default.
I think so.

-- 
Igor Gnatenko
Fedora release 20 (Heisenbug)
Linux 3.11.4-301.fc20.x86_64

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Rafael J. Wysocki 
On Saturday, October 12, 2013 02:53:38 AM Igor Gnatenko wrote:
 On Sat, 2013-10-12 at 00:45 +0200, Rafael J. Wysocki wrote:
  On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote:
   On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki r...@rjwysocki.net 
   wrote:
On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote:
 v5:
 1 Introduce video.use_native_backlight module parameter and set its
   value to false by default as suggested by Rafael. For Win8 systems
   which have broken ACPI video backlight control, the parameter can 
 be
   set to 1 in kernel cmdline to skip registering ACPI video's 
 backlight
   interface. Due to this change, the 
 acpi_video_verify_backlight_support
   is moved from video_detect.c to video.c - patch 3/4;
   
That's a fairly untenable position for distro kernels to be in.  They
now have to ask every user that reports an issue with the backlight to
try setting that option on the command line.  While I appreciate the
setting breaks things for some people, doesn't the Win8 issue impact
far more people?  Shouldn't it be defaulted to true?
   
Well, we have a rule in the kernel not to introduce regressions for 
users even
if they are minority.
   
If nothing else, can you add a config option for the default so
distros can use that to decide which way to default it and then work
on fixing the remaining users that have troubles?
   
The current plan is to create a blacklist of systems where that option 
should
be set.  We actually already have one, but it is at the _OSI() level, 
which
is overkill in my view and may affect things beyond backlight.  Along 
with that
we will debug systems where setting that option (to true) causes 
problems to
happen, so that we'll be able to drop it going forward (hopefully).
   
Of course, distro kernels may always change the default to true if they 
want.
   
   They can, but they'd need to either patch the kernel to do so, or code
   it in userspace bootloader configs.  Having a config option they can
   set to change the default makes it reasonable and contained within the
   kernel.
  
  Well, adding a Kconfig option should be simple enough, but I'm not sure
  I understand the point.  You'll still need to rebuild the kernel after 
  changing
  that option.
  
 
 I think he meant that we should have Kconfig option alike
 CONFIG_I915_BACKLIGHT_UNREGISTER which have true or false.
 If it true - unregister acpi backlight as default.
 I think so.

That's my understanding too, but I'm not sure about the benefit.  The only one
seems to be that distros wanting to change the default won't have to carry a
patch for that.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Rafael J. Wysocki 
On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote:
 On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki r...@rjwysocki.net wrote:
  On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
  On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote:
   v5:
   1 Introduce video.use_native_backlight module parameter and set its
 value to false by default as suggested by Rafael. For Win8 systems
 which have broken ACPI video backlight control, the parameter can be
 set to 1 in kernel cmdline to skip registering ACPI video's backlight
 interface. Due to this change, the acpi_video_verify_backlight_support
 is moved from video_detect.c to video.c - patch 3/4;
 
  That's a fairly untenable position for distro kernels to be in.  They
  now have to ask every user that reports an issue with the backlight to
  try setting that option on the command line.  While I appreciate the
  setting breaks things for some people, doesn't the Win8 issue impact
  far more people?  Shouldn't it be defaulted to true?
 
  Well, we have a rule in the kernel not to introduce regressions for users 
  even
  if they are minority.
 
  If nothing else, can you add a config option for the default so
  distros can use that to decide which way to default it and then work
  on fixing the remaining users that have troubles?
 
  The current plan is to create a blacklist of systems where that option 
  should
  be set.  We actually already have one, but it is at the _OSI() level, which
  is overkill in my view and may affect things beyond backlight.  Along with 
  that
  we will debug systems where setting that option (to true) causes problems to
  happen, so that we'll be able to drop it going forward (hopefully).
 
  Of course, distro kernels may always change the default to true if they 
  want.
 
 They can, but they'd need to either patch the kernel to do so, or code
 it in userspace bootloader configs.  Having a config option they can
 set to change the default makes it reasonable and contained within the
 kernel.

If we are to use a Kconfig option, why don't we use one instead of rather than
in addition to a command line option?  Say, we have
CONFIG_ACPI_VIDEO_WIN8_WORKAROUND and if that is set, the code will work like
the previous version of the Aaron's patchset (the one without
video.use_native_backlight)?

Opinions?

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Yves-Alexis Perez 
On sam., 2013-10-12 at 01:27 +0200, Rafael J. Wysocki wrote:
 If we are to use a Kconfig option, why don't we use one instead of rather than
 in addition to a command line option?  Say, we have
 CONFIG_ACPI_VIDEO_WIN8_WORKAROUND and if that is set, the code will work like
 the previous version of the Aaron's patchset (the one without
 video.use_native_backlight)?

Doesn't this mean user will have to rebuild distro kernel in order to
test behavior?

Regards,
-- 
Yves-Alexis


signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Yves-Alexis Perez 
On sam., 2013-10-12 at 00:10 +0200, Rafael J. Wysocki wrote:
 On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
  On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote:
   v5:
   1 Introduce video.use_native_backlight module parameter and set its
 value to false by default as suggested by Rafael. For Win8 systems
 which have broken ACPI video backlight control, the parameter can be
 set to 1 in kernel cmdline to skip registering ACPI video's backlight
 interface. Due to this change, the acpi_video_verify_backlight_support
 is moved from video_detect.c to video.c - patch 3/4;
  
  That's a fairly untenable position for distro kernels to be in.  They
  now have to ask every user that reports an issue with the backlight to
  try setting that option on the command line.  While I appreciate the
  setting breaks things for some people, doesn't the Win8 issue impact
  far more people?  Shouldn't it be defaulted to true?
 
 Well, we have a rule in the kernel not to introduce regressions for users even
 if they are minority.

Well, for some users, the regression actually happened when support for
Win8 OSI call was introduced.

Regards,
-- 
Yves-Alexis


signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Josh Boyer 
On Fri, Oct 11, 2013 at 4:27 PM, Rafael J. Wysocki r...@rjwysocki.net wrote:
 On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote:
 On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki r...@rjwysocki.net 
 wrote:
  On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
  On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote:
   v5:
   1 Introduce video.use_native_backlight module parameter and set its
 value to false by default as suggested by Rafael. For Win8 systems
 which have broken ACPI video backlight control, the parameter can be
 set to 1 in kernel cmdline to skip registering ACPI video's backlight
 interface. Due to this change, the acpi_video_verify_backlight_support
 is moved from video_detect.c to video.c - patch 3/4;
 
  That's a fairly untenable position for distro kernels to be in.  They
  now have to ask every user that reports an issue with the backlight to
  try setting that option on the command line.  While I appreciate the
  setting breaks things for some people, doesn't the Win8 issue impact
  far more people?  Shouldn't it be defaulted to true?
 
  Well, we have a rule in the kernel not to introduce regressions for users 
  even
  if they are minority.
 
  If nothing else, can you add a config option for the default so
  distros can use that to decide which way to default it and then work
  on fixing the remaining users that have troubles?
 
  The current plan is to create a blacklist of systems where that option 
  should
  be set.  We actually already have one, but it is at the _OSI() level, which
  is overkill in my view and may affect things beyond backlight.  Along with 
  that
  we will debug systems where setting that option (to true) causes problems 
  to
  happen, so that we'll be able to drop it going forward (hopefully).
 
  Of course, distro kernels may always change the default to true if they 
  want.

 They can, but they'd need to either patch the kernel to do so, or code
 it in userspace bootloader configs.  Having a config option they can
 set to change the default makes it reasonable and contained within the
 kernel.

 If we are to use a Kconfig option, why don't we use one instead of rather than
 in addition to a command line option?  Say, we have
 CONFIG_ACPI_VIDEO_WIN8_WORKAROUND and if that is set, the code will work like
 the previous version of the Aaron's patchset (the one without
 video.use_native_backlight)?

 Opinions?

If you only have a config option, users can't override the distro
settings.  If you simply have a config option for the default value,
the distros can set it without having to carry a patch (the primary
benefit), but users can still override that without having to rebuild
a kernel.

josh
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Josh Boyer 
On Fri, Oct 11, 2013 at 11:00 PM, Yves-Alexis Perez cor...@debian.org wrote:
 On sam., 2013-10-12 at 00:10 +0200, Rafael J. Wysocki wrote:
 On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
  On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote:
   v5:
   1 Introduce video.use_native_backlight module parameter and set its
 value to false by default as suggested by Rafael. For Win8 systems
 which have broken ACPI video backlight control, the parameter can be
 set to 1 in kernel cmdline to skip registering ACPI video's backlight
 interface. Due to this change, the acpi_video_verify_backlight_support
 is moved from video_detect.c to video.c - patch 3/4;
 
  That's a fairly untenable position for distro kernels to be in.  They
  now have to ask every user that reports an issue with the backlight to
  try setting that option on the command line.  While I appreciate the
  setting breaks things for some people, doesn't the Win8 issue impact
  far more people?  Shouldn't it be defaulted to true?

 Well, we have a rule in the kernel not to introduce regressions for users 
 even
 if they are minority.

 Well, for some users, the regression actually happened when support for
 Win8 OSI call was introduced.

Yes, this is true.  It's probably one of the more common bug reports
we get in this area.  Kernels prior to that have working backlight,
kernels after that don't.

josh
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Seth Forshee 
On Sat, Oct 12, 2013 at 04:44:30AM -0700, Josh Boyer wrote:
 On Fri, Oct 11, 2013 at 4:27 PM, Rafael J. Wysocki r...@rjwysocki.net wrote:
  On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote:
  On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki r...@rjwysocki.net 
  wrote:
   On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
   On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote:
v5:
1 Introduce video.use_native_backlight module parameter and set its
  value to false by default as suggested by Rafael. For Win8 systems
  which have broken ACPI video backlight control, the parameter can be
  set to 1 in kernel cmdline to skip registering ACPI video's 
backlight
  interface. Due to this change, the 
acpi_video_verify_backlight_support
  is moved from video_detect.c to video.c - patch 3/4;
  
   That's a fairly untenable position for distro kernels to be in.  They
   now have to ask every user that reports an issue with the backlight to
   try setting that option on the command line.  While I appreciate the
   setting breaks things for some people, doesn't the Win8 issue impact
   far more people?  Shouldn't it be defaulted to true?
  
   Well, we have a rule in the kernel not to introduce regressions for 
   users even
   if they are minority.
  
   If nothing else, can you add a config option for the default so
   distros can use that to decide which way to default it and then work
   on fixing the remaining users that have troubles?
  
   The current plan is to create a blacklist of systems where that option 
   should
   be set.  We actually already have one, but it is at the _OSI() level, 
   which
   is overkill in my view and may affect things beyond backlight.  Along 
   with that
   we will debug systems where setting that option (to true) causes 
   problems to
   happen, so that we'll be able to drop it going forward (hopefully).
  
   Of course, distro kernels may always change the default to true if they 
   want.
 
  They can, but they'd need to either patch the kernel to do so, or code
  it in userspace bootloader configs.  Having a config option they can
  set to change the default makes it reasonable and contained within the
  kernel.
 
  If we are to use a Kconfig option, why don't we use one instead of rather 
  than
  in addition to a command line option?  Say, we have
  CONFIG_ACPI_VIDEO_WIN8_WORKAROUND and if that is set, the code will work 
  like
  the previous version of the Aaron's patchset (the one without
  video.use_native_backlight)?
 
  Opinions?
 
 If you only have a config option, users can't override the distro
 settings.  If you simply have a config option for the default value,
 the distros can set it without having to carry a patch (the primary
 benefit), but users can still override that without having to rebuild
 a kernel.

It sounds like the blacklist and the default value of the parameter
would be inherently tied together, i.e. the blacklist essentially
overrides the default value for specific machines. So when the config
option were flipped from its default the blacklist wouldn't work
anymore, and you'd need a second blacklist for machines which require
video.use_native_backlight=n. I doubt anyone wants to see that happen,
so I think we have to pick one value or the other for the default and
make it configurable only via the command line.

Seth

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Rafael J. Wysocki 
On Saturday, October 12, 2013 08:34:32 AM Seth Forshee wrote:
 On Sat, Oct 12, 2013 at 04:44:30AM -0700, Josh Boyer wrote:
  On Fri, Oct 11, 2013 at 4:27 PM, Rafael J. Wysocki r...@rjwysocki.net 
  wrote:
   On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote:
   On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki r...@rjwysocki.net 
   wrote:
On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron...@intel.com wrote:
 v5:
 1 Introduce video.use_native_backlight module parameter and set its
   value to false by default as suggested by Rafael. For Win8 systems
   which have broken ACPI video backlight control, the parameter can 
 be
   set to 1 in kernel cmdline to skip registering ACPI video's 
 backlight
   interface. Due to this change, the 
 acpi_video_verify_backlight_support
   is moved from video_detect.c to video.c - patch 3/4;
   
That's a fairly untenable position for distro kernels to be in.  They
now have to ask every user that reports an issue with the backlight to
try setting that option on the command line.  While I appreciate the
setting breaks things for some people, doesn't the Win8 issue impact
far more people?  Shouldn't it be defaulted to true?
   
Well, we have a rule in the kernel not to introduce regressions for 
users even
if they are minority.
   
If nothing else, can you add a config option for the default so
distros can use that to decide which way to default it and then work
on fixing the remaining users that have troubles?
   
The current plan is to create a blacklist of systems where that option 
should
be set.  We actually already have one, but it is at the _OSI() level, 
which
is overkill in my view and may affect things beyond backlight.  Along 
with that
we will debug systems where setting that option (to true) causes 
problems to
happen, so that we'll be able to drop it going forward (hopefully).
   
Of course, distro kernels may always change the default to true if 
they want.
  
   They can, but they'd need to either patch the kernel to do so, or code
   it in userspace bootloader configs.  Having a config option they can
   set to change the default makes it reasonable and contained within the
   kernel.
  
   If we are to use a Kconfig option, why don't we use one instead of rather 
   than
   in addition to a command line option?  Say, we have
   CONFIG_ACPI_VIDEO_WIN8_WORKAROUND and if that is set, the code will work 
   like
   the previous version of the Aaron's patchset (the one without
   video.use_native_backlight)?
  
   Opinions?
  
  If you only have a config option, users can't override the distro
  settings.  If you simply have a config option for the default value,
  the distros can set it without having to carry a patch (the primary
  benefit), but users can still override that without having to rebuild
  a kernel.
 
 It sounds like the blacklist and the default value of the parameter
 would be inherently tied together, i.e. the blacklist essentially
 overrides the default value for specific machines. So when the config
 option were flipped from its default the blacklist wouldn't work
 anymore, and you'd need a second blacklist for machines which require
 video.use_native_backlight=n. I doubt anyone wants to see that happen,
 so I think we have to pick one value or the other for the default and
 make it configurable only via the command line.

Good point.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] Fix Win8 backlight issue

2013-10-12 Thread Rafael J. Wysocki 
On Saturday, October 12, 2013 07:54:06 AM Yves-Alexis Perez wrote:
 On sam., 2013-10-12 at 01:27 +0200, Rafael J. Wysocki wrote:
  If we are to use a Kconfig option, why don't we use one instead of rather 
  than
  in addition to a command line option?  Say, we have
  CONFIG_ACPI_VIDEO_WIN8_WORKAROUND and if that is set, the code will work 
  like
  the previous version of the Aaron's patchset (the one without
  video.use_native_backlight)?
 
 Doesn't this mean user will have to rebuild distro kernel in order to
 test behavior?

Yes, it does, which is a good point too.

Thanks!

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

signature.asc
Description: This is a digitally signed message part.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel