Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-08 Thread Aaron Lu 
On 07/07/2013 09:19 PM, Rafael J. Wysocki wrote:
> OK, the patch is appended.  Please have a look and tell me what you think.
> 
> Thanks,
> Rafael
> 
> 
> ---
> From: Rafael J. Wysocki 
> Subject: ACPI / video / i915: Remove ACPI backlight if firmware expects 
> Windows 8
> 
> According to Matthew Garrett, "Windows 8 leaves backlight control up
> to individual graphics drivers rather than making ACPI calls itself.
> There's plenty of evidence to suggest that the Intel driver for
> Windows [8] doesn't use the ACPI interface, including the fact that
> it's broken on a bunch of machines when the OS claims to support
> Windows 8.  The simplest thing to do appears to be to disable the
> ACPI backlight interface on these systems".
> 
> There's a problem with that approach, however, because simply
> avoiding to register the ACPI backlight interface if the firmware
> calls _OSI for Windows 8 may not work in the following situations:
>  (1) The ACPI backlight interface actually works on the given system
>  and the i915 driver is not loaded (e.g. another graphics driver
>  is used).
>  (2) The ACPI backlight interface doesn't work on the given system,
>  but there is a vendor platform driver that will register its
>  own, equally broken, backlight interface if the ACPI one is not
>  registered in advance.

The condition thinkpad_acpi checks to decide if it wants to create
backlight control interface is acpi_video_backlight_support function,
not that if ACPI video driver has registered previously. I'm sorry if my
previous words gave you such a conclusion.

> Therefore we need to keep the ACPI backlight interface registered
> until the i915 driver is loaded which then will unregister it if
> the firmware has called _OSI for Windows 8.
> 
> For this reason, introduce an alternative function for registering
> ACPI video, acpi_video_register_with_quirks(), that will check
> whether or not the ACPI video driver has already been registered
> and whether or not the backlight Windows 8 quirk has to be applied.
> If the quirk has to be applied, it will block the ACPI backlight
> support and either unregister the backlight interface if the ACPI
> video driver has already been registered, or register the ACPI
> video driver without the backlight interface otherwise.  Make
> the i915 driver use acpi_video_register_with_quirks() instead of
> acpi_video_register() in i915_driver_load().
> 
> This change is based on earlier patches from Matthew Garrett,
> Chun-Yi Lee and Seth Forshee.
> 
> Signed-off-by: Rafael J. Wysocki 
> ---
>  drivers/acpi/video.c|   61 
> 
>  drivers/acpi/video_detect.c |   11 +++
>  drivers/gpu/drm/i915/i915_dma.c |2 -
>  include/acpi/video.h|   11 ++-
>  include/linux/acpi.h|6 +++
>  5 files changed, 84 insertions(+), 7 deletions(-)
> 
> Index: linux-pm/drivers/acpi/video.c
> ===
> --- linux-pm.orig/drivers/acpi/video.c
> +++ linux-pm/drivers/acpi/video.c
> @@ -1854,6 +1854,46 @@ static int acpi_video_bus_remove(struct
>   return 0;
>  }
>  
> +static acpi_status video_unregister_backlight(acpi_handle handle, u32 lvl,
> +   void *context, void **rv)
> +{
> + struct acpi_device *acpi_dev;
> + struct acpi_video_bus *video;
> + struct acpi_video_device *dev, *next;
> +
> + if (acpi_bus_get_device(handle, _dev))
> + return AE_OK;
> +
> + if (acpi_match_device_ids(acpi_dev, video_device_ids))
> + return AE_OK;
> +
> + video = acpi_driver_data(acpi_dev);
> + if (!video)
> + return AE_OK;
> +
> + acpi_video_bus_stop_devices(video);
> + mutex_lock(>device_list_lock);
> + list_for_each_entry_safe(dev, next, >video_device_list, entry) {
> + if (dev->backlight) {
> + backlight_device_unregister(dev->backlight);
> + dev->backlight = NULL;
> + kfree(dev->brightness->levels);
> + kfree(dev->brightness);
> + }
> + if (dev->cooling_dev) {
> + sysfs_remove_link(>dev->dev.kobj,
> +   "thermal_cooling");
> + sysfs_remove_link(>cooling_dev->device.kobj,
> +   "device");
> + thermal_cooling_device_unregister(dev->cooling_dev);
> + dev->cooling_dev = NULL;
> + }
> + }
> + mutex_unlock(>device_list_lock);
> + acpi_video_bus_start_devices(video);
> + return AE_OK;
> +}
> +
>  static int __init is_i740(struct pci_dev *dev)
>  {
>   if (dev->device == 0x00D1)
> @@ -1885,14 +1925,25 @@ static int __init intel_opregion_present
>   return opregion;
>  }
>  
> -int acpi_video_register(void)
> +int __acpi_video_register(bool backlight_quirks)

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-08 Thread Aaron Lu 
On 07/07/2013 09:19 PM, Rafael J. Wysocki wrote:
 OK, the patch is appended.  Please have a look and tell me what you think.
 
 Thanks,
 Rafael
 
 
 ---
 From: Rafael J. Wysocki rafael.j.wyso...@intel.com
 Subject: ACPI / video / i915: Remove ACPI backlight if firmware expects 
 Windows 8
 
 According to Matthew Garrett, Windows 8 leaves backlight control up
 to individual graphics drivers rather than making ACPI calls itself.
 There's plenty of evidence to suggest that the Intel driver for
 Windows [8] doesn't use the ACPI interface, including the fact that
 it's broken on a bunch of machines when the OS claims to support
 Windows 8.  The simplest thing to do appears to be to disable the
 ACPI backlight interface on these systems.
 
 There's a problem with that approach, however, because simply
 avoiding to register the ACPI backlight interface if the firmware
 calls _OSI for Windows 8 may not work in the following situations:
  (1) The ACPI backlight interface actually works on the given system
  and the i915 driver is not loaded (e.g. another graphics driver
  is used).
  (2) The ACPI backlight interface doesn't work on the given system,
  but there is a vendor platform driver that will register its
  own, equally broken, backlight interface if the ACPI one is not
  registered in advance.

The condition thinkpad_acpi checks to decide if it wants to create
backlight control interface is acpi_video_backlight_support function,
not that if ACPI video driver has registered previously. I'm sorry if my
previous words gave you such a conclusion.

 Therefore we need to keep the ACPI backlight interface registered
 until the i915 driver is loaded which then will unregister it if
 the firmware has called _OSI for Windows 8.
 
 For this reason, introduce an alternative function for registering
 ACPI video, acpi_video_register_with_quirks(), that will check
 whether or not the ACPI video driver has already been registered
 and whether or not the backlight Windows 8 quirk has to be applied.
 If the quirk has to be applied, it will block the ACPI backlight
 support and either unregister the backlight interface if the ACPI
 video driver has already been registered, or register the ACPI
 video driver without the backlight interface otherwise.  Make
 the i915 driver use acpi_video_register_with_quirks() instead of
 acpi_video_register() in i915_driver_load().
 
 This change is based on earlier patches from Matthew Garrett,
 Chun-Yi Lee and Seth Forshee.
 
 Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
 ---
  drivers/acpi/video.c|   61 
 
  drivers/acpi/video_detect.c |   11 +++
  drivers/gpu/drm/i915/i915_dma.c |2 -
  include/acpi/video.h|   11 ++-
  include/linux/acpi.h|6 +++
  5 files changed, 84 insertions(+), 7 deletions(-)
 
 Index: linux-pm/drivers/acpi/video.c
 ===
 --- linux-pm.orig/drivers/acpi/video.c
 +++ linux-pm/drivers/acpi/video.c
 @@ -1854,6 +1854,46 @@ static int acpi_video_bus_remove(struct
   return 0;
  }
  
 +static acpi_status video_unregister_backlight(acpi_handle handle, u32 lvl,
 +   void *context, void **rv)
 +{
 + struct acpi_device *acpi_dev;
 + struct acpi_video_bus *video;
 + struct acpi_video_device *dev, *next;
 +
 + if (acpi_bus_get_device(handle, acpi_dev))
 + return AE_OK;
 +
 + if (acpi_match_device_ids(acpi_dev, video_device_ids))
 + return AE_OK;
 +
 + video = acpi_driver_data(acpi_dev);
 + if (!video)
 + return AE_OK;
 +
 + acpi_video_bus_stop_devices(video);
 + mutex_lock(video-device_list_lock);
 + list_for_each_entry_safe(dev, next, video-video_device_list, entry) {
 + if (dev-backlight) {
 + backlight_device_unregister(dev-backlight);
 + dev-backlight = NULL;
 + kfree(dev-brightness-levels);
 + kfree(dev-brightness);
 + }
 + if (dev-cooling_dev) {
 + sysfs_remove_link(dev-dev-dev.kobj,
 +   thermal_cooling);
 + sysfs_remove_link(dev-cooling_dev-device.kobj,
 +   device);
 + thermal_cooling_device_unregister(dev-cooling_dev);
 + dev-cooling_dev = NULL;
 + }
 + }
 + mutex_unlock(video-device_list_lock);
 + acpi_video_bus_start_devices(video);
 + return AE_OK;
 +}
 +
  static int __init is_i740(struct pci_dev *dev)
  {
   if (dev-device == 0x00D1)
 @@ -1885,14 +1925,25 @@ static int __init intel_opregion_present
   return opregion;
  }
  
 -int acpi_video_register(void)
 +int __acpi_video_register(bool backlight_quirks)
  {
 - int result = 0;
 + bool no_backlight;
 +

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-07 Thread Rafael J. Wysocki 
On Saturday, July 06, 2013 03:33:01 PM Rafael J. Wysocki wrote:
> On Saturday, July 06, 2013 01:45:56 PM Aaron Lu wrote:
> > On 07/06/2013 06:23 AM, Rafael J. Wysocki wrote:
> > > On Friday, July 05, 2013 11:40:02 PM Rafael J. Wysocki wrote:
> > >> On Friday, July 05, 2013 10:00:55 PM Rafael J. Wysocki wrote:
> > >>> On Friday, July 05, 2013 02:20:14 PM Rafael J. Wysocki wrote:
> >  On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:

[...]

> > >>
> > >> Actually, I wonder what about

[...]

> > > 
> > > Or even something as simple as this one.
> > > 
> > > ---
> > >  drivers/acpi/video_detect.c |3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > Index: linux-pm/drivers/acpi/video_detect.c
> > > ===
> > > --- linux-pm.orig/drivers/acpi/video_detect.c
> > > +++ linux-pm/drivers/acpi/video_detect.c
> > > @@ -203,6 +203,9 @@ long acpi_video_get_capabilities(acpi_ha
> > >*/
> > >  
> > >   dmi_check_system(video_detect_dmi_table);
> > > +
> > > + if (acpi_gbl_osi_data >= ACPI_OSI_WIN_8)
> > > + acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;
> > 
> > Then vendor driver(thinkpad_acpi) will step in and create a backlight
> > interface for the system, which, unfortunately, is also broken for those
> > win8 thinkpads.
> > 
> > So we will need to do something in thinkpad_acpi to also not create
> > backlight interface for these systems too.
> > 
> > This actually doesn't feel bad to me, since the modules are blacklisting
> > their own interfaces. The downside is of course, two quirk code exist.
> > 
> > BTW, unregistering ACPI video's backlight interface in GPU driver doesn't
> > have this problem since it made the platform driver think the ACPI video
> > driver will control the backlight and then use the newly added API to
> > remove ACPI video created backlight interface.
> 
> Yes, I know this.
> 
> I think I also know how to introduce that change in a slightly cleaner way.
> 
> I'll post a patch for comments later today or tomorrow.

OK, the patch is appended.  Please have a look and tell me what you think.

Thanks,
Rafael


---
From: Rafael J. Wysocki 
Subject: ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 
8

According to Matthew Garrett, "Windows 8 leaves backlight control up
to individual graphics drivers rather than making ACPI calls itself.
There's plenty of evidence to suggest that the Intel driver for
Windows [8] doesn't use the ACPI interface, including the fact that
it's broken on a bunch of machines when the OS claims to support
Windows 8.  The simplest thing to do appears to be to disable the
ACPI backlight interface on these systems".

There's a problem with that approach, however, because simply
avoiding to register the ACPI backlight interface if the firmware
calls _OSI for Windows 8 may not work in the following situations:
 (1) The ACPI backlight interface actually works on the given system
 and the i915 driver is not loaded (e.g. another graphics driver
 is used).
 (2) The ACPI backlight interface doesn't work on the given system,
 but there is a vendor platform driver that will register its
 own, equally broken, backlight interface if the ACPI one is not
 registered in advance.
Therefore we need to keep the ACPI backlight interface registered
until the i915 driver is loaded which then will unregister it if
the firmware has called _OSI for Windows 8.

For this reason, introduce an alternative function for registering
ACPI video, acpi_video_register_with_quirks(), that will check
whether or not the ACPI video driver has already been registered
and whether or not the backlight Windows 8 quirk has to be applied.
If the quirk has to be applied, it will block the ACPI backlight
support and either unregister the backlight interface if the ACPI
video driver has already been registered, or register the ACPI
video driver without the backlight interface otherwise.  Make
the i915 driver use acpi_video_register_with_quirks() instead of
acpi_video_register() in i915_driver_load().

This change is based on earlier patches from Matthew Garrett,
Chun-Yi Lee and Seth Forshee.

Signed-off-by: Rafael J. Wysocki 
---
 drivers/acpi/video.c|   61 
 drivers/acpi/video_detect.c |   11 +++
 drivers/gpu/drm/i915/i915_dma.c |2 -
 include/acpi/video.h|   11 ++-
 include/linux/acpi.h|6 +++
 5 files changed, 84 insertions(+), 7 deletions(-)

Index: linux-pm/drivers/acpi/video.c
===
--- linux-pm.orig/drivers/acpi/video.c
+++ linux-pm/drivers/acpi/video.c
@@ -1854,6 +1854,46 @@ static int acpi_video_bus_remove(struct
return 0;
 }
 
+static acpi_status video_unregister_backlight(acpi_handle handle, u32 lvl,
+ void *context, void **rv)

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-07 Thread Rafael J. Wysocki 
On Saturday, July 06, 2013 03:33:01 PM Rafael J. Wysocki wrote:
 On Saturday, July 06, 2013 01:45:56 PM Aaron Lu wrote:
  On 07/06/2013 06:23 AM, Rafael J. Wysocki wrote:
   On Friday, July 05, 2013 11:40:02 PM Rafael J. Wysocki wrote:
   On Friday, July 05, 2013 10:00:55 PM Rafael J. Wysocki wrote:
   On Friday, July 05, 2013 02:20:14 PM Rafael J. Wysocki wrote:
   On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:

[...]

  
   Actually, I wonder what about

[...]

   
   Or even something as simple as this one.
   
   ---
drivers/acpi/video_detect.c |3 +++
1 file changed, 3 insertions(+)
   
   Index: linux-pm/drivers/acpi/video_detect.c
   ===
   --- linux-pm.orig/drivers/acpi/video_detect.c
   +++ linux-pm/drivers/acpi/video_detect.c
   @@ -203,6 +203,9 @@ long acpi_video_get_capabilities(acpi_ha
  */

 dmi_check_system(video_detect_dmi_table);
   +
   + if (acpi_gbl_osi_data = ACPI_OSI_WIN_8)
   + acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;
  
  Then vendor driver(thinkpad_acpi) will step in and create a backlight
  interface for the system, which, unfortunately, is also broken for those
  win8 thinkpads.
  
  So we will need to do something in thinkpad_acpi to also not create
  backlight interface for these systems too.
  
  This actually doesn't feel bad to me, since the modules are blacklisting
  their own interfaces. The downside is of course, two quirk code exist.
  
  BTW, unregistering ACPI video's backlight interface in GPU driver doesn't
  have this problem since it made the platform driver think the ACPI video
  driver will control the backlight and then use the newly added API to
  remove ACPI video created backlight interface.
 
 Yes, I know this.
 
 I think I also know how to introduce that change in a slightly cleaner way.
 
 I'll post a patch for comments later today or tomorrow.

OK, the patch is appended.  Please have a look and tell me what you think.

Thanks,
Rafael


---
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Subject: ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 
8

According to Matthew Garrett, Windows 8 leaves backlight control up
to individual graphics drivers rather than making ACPI calls itself.
There's plenty of evidence to suggest that the Intel driver for
Windows [8] doesn't use the ACPI interface, including the fact that
it's broken on a bunch of machines when the OS claims to support
Windows 8.  The simplest thing to do appears to be to disable the
ACPI backlight interface on these systems.

There's a problem with that approach, however, because simply
avoiding to register the ACPI backlight interface if the firmware
calls _OSI for Windows 8 may not work in the following situations:
 (1) The ACPI backlight interface actually works on the given system
 and the i915 driver is not loaded (e.g. another graphics driver
 is used).
 (2) The ACPI backlight interface doesn't work on the given system,
 but there is a vendor platform driver that will register its
 own, equally broken, backlight interface if the ACPI one is not
 registered in advance.
Therefore we need to keep the ACPI backlight interface registered
until the i915 driver is loaded which then will unregister it if
the firmware has called _OSI for Windows 8.

For this reason, introduce an alternative function for registering
ACPI video, acpi_video_register_with_quirks(), that will check
whether or not the ACPI video driver has already been registered
and whether or not the backlight Windows 8 quirk has to be applied.
If the quirk has to be applied, it will block the ACPI backlight
support and either unregister the backlight interface if the ACPI
video driver has already been registered, or register the ACPI
video driver without the backlight interface otherwise.  Make
the i915 driver use acpi_video_register_with_quirks() instead of
acpi_video_register() in i915_driver_load().

This change is based on earlier patches from Matthew Garrett,
Chun-Yi Lee and Seth Forshee.

Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
 drivers/acpi/video.c|   61 
 drivers/acpi/video_detect.c |   11 +++
 drivers/gpu/drm/i915/i915_dma.c |2 -
 include/acpi/video.h|   11 ++-
 include/linux/acpi.h|6 +++
 5 files changed, 84 insertions(+), 7 deletions(-)

Index: linux-pm/drivers/acpi/video.c
===
--- linux-pm.orig/drivers/acpi/video.c
+++ linux-pm/drivers/acpi/video.c
@@ -1854,6 +1854,46 @@ static int acpi_video_bus_remove(struct
return 0;
 }
 
+static acpi_status video_unregister_backlight(acpi_handle handle, u32 lvl,
+ void *context, void **rv)
+{
+   struct acpi_device *acpi_dev;
+   struct acpi_video_bus

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-06 Thread Rafael J. Wysocki 
On Saturday, July 06, 2013 01:45:56 PM Aaron Lu wrote:
> On 07/06/2013 06:23 AM, Rafael J. Wysocki wrote:
> > On Friday, July 05, 2013 11:40:02 PM Rafael J. Wysocki wrote:
> >> On Friday, July 05, 2013 10:00:55 PM Rafael J. Wysocki wrote:
> >>> On Friday, July 05, 2013 02:20:14 PM Rafael J. Wysocki wrote:
>  On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:
> > Windows 8 leaves backlight control up to individual graphics drivers 
> > rather
> > than making ACPI calls itself. There's plenty of evidence to suggest 
> > that
> > the Intel driver for Windows doesn't use the ACPI interface, including 
> > the
> > fact that it's broken on a bunch of machines when the OS claims to 
> > support
> > Windows 8. The simplest thing to do appears to be to disable the ACPI
> > backlight interface on these systems.
> >
> > Signed-off-by: Matthew Garrett 
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c 
> > b/drivers/gpu/drm/i915/i915_dma.c
> > index 3b315ba..23b6292 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, 
> > unsigned long flags)
> > /* Must be done after probing outputs */
> > intel_opregion_init(dev);
> > acpi_video_register();
> > +   /* Don't use ACPI backlight functions on Windows 8 
> > platforms */
> > +   if (acpi_osi_version() >= ACPI_OSI_WIN_8)
> > +   acpi_video_backlight_unregister();
> > }
> >  
> > if (IS_GEN5(dev))
> >
> 
>  Well, this causes build failures to happen when the ACPI video driver is
>  modular and the graphics driver is not.
> 
>  I'm not sure how to resolve that, so suggestions are welcome.
> >>>
> >>> Actually, that happened with the radeon patch.
> >>>
> >>> That said, ACPI_OSI_WIN_8 doesn't make much sense for !CONFIG_ACPI, for
> >>> example.
> >>>
> >>> What about making acpi_video_register() do the quirk instead?  We could 
> >>> add an
> >>> argument to it indicating whether or not quirks should be applied.
> >>
> >> Actually, I wonder what about the appended patch (on top of the Aaron's
> >> https://patchwork.kernel.org/patch/2812951/) instead of [1-3/3] from this 
> >> series.
> > 
> > Or even something as simple as this one.
> > 
> > ---
> >  drivers/acpi/video_detect.c |3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > Index: linux-pm/drivers/acpi/video_detect.c
> > ===
> > --- linux-pm.orig/drivers/acpi/video_detect.c
> > +++ linux-pm/drivers/acpi/video_detect.c
> > @@ -203,6 +203,9 @@ long acpi_video_get_capabilities(acpi_ha
> >  */
> >  
> > dmi_check_system(video_detect_dmi_table);
> > +
> > +   if (acpi_gbl_osi_data >= ACPI_OSI_WIN_8)
> > +   acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;
> 
> Then vendor driver(thinkpad_acpi) will step in and create a backlight
> interface for the system, which, unfortunately, is also broken for those
> win8 thinkpads.
> 
> So we will need to do something in thinkpad_acpi to also not create
> backlight interface for these systems too.
> 
> This actually doesn't feel bad to me, since the modules are blacklisting
> their own interfaces. The downside is of course, two quirk code exist.
> 
> BTW, unregistering ACPI video's backlight interface in GPU driver doesn't
> have this problem since it made the platform driver think the ACPI video
> driver will control the backlight and then use the newly added API to
> remove ACPI video created backlight interface.

Yes, I know this.

I think I also know how to introduce that change in a slightly cleaner way.

I'll post a patch for comments later today or tomorrow.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-06 Thread Rafael J. Wysocki 
On Saturday, July 06, 2013 01:45:56 PM Aaron Lu wrote:
 On 07/06/2013 06:23 AM, Rafael J. Wysocki wrote:
  On Friday, July 05, 2013 11:40:02 PM Rafael J. Wysocki wrote:
  On Friday, July 05, 2013 10:00:55 PM Rafael J. Wysocki wrote:
  On Friday, July 05, 2013 02:20:14 PM Rafael J. Wysocki wrote:
  On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:
  Windows 8 leaves backlight control up to individual graphics drivers 
  rather
  than making ACPI calls itself. There's plenty of evidence to suggest 
  that
  the Intel driver for Windows doesn't use the ACPI interface, including 
  the
  fact that it's broken on a bunch of machines when the OS claims to 
  support
  Windows 8. The simplest thing to do appears to be to disable the ACPI
  backlight interface on these systems.
 
  Signed-off-by: Matthew Garrett matthew.garr...@nebula.com
  ---
   drivers/gpu/drm/i915/i915_dma.c | 3 +++
   1 file changed, 3 insertions(+)
 
  diff --git a/drivers/gpu/drm/i915/i915_dma.c 
  b/drivers/gpu/drm/i915/i915_dma.c
  index 3b315ba..23b6292 100644
  --- a/drivers/gpu/drm/i915/i915_dma.c
  +++ b/drivers/gpu/drm/i915/i915_dma.c
  @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, 
  unsigned long flags)
  /* Must be done after probing outputs */
  intel_opregion_init(dev);
  acpi_video_register();
  +   /* Don't use ACPI backlight functions on Windows 8 
  platforms */
  +   if (acpi_osi_version() = ACPI_OSI_WIN_8)
  +   acpi_video_backlight_unregister();
  }
   
  if (IS_GEN5(dev))
 
 
  Well, this causes build failures to happen when the ACPI video driver is
  modular and the graphics driver is not.
 
  I'm not sure how to resolve that, so suggestions are welcome.
 
  Actually, that happened with the radeon patch.
 
  That said, ACPI_OSI_WIN_8 doesn't make much sense for !CONFIG_ACPI, for
  example.
 
  What about making acpi_video_register() do the quirk instead?  We could 
  add an
  argument to it indicating whether or not quirks should be applied.
 
  Actually, I wonder what about the appended patch (on top of the Aaron's
  https://patchwork.kernel.org/patch/2812951/) instead of [1-3/3] from this 
  series.
  
  Or even something as simple as this one.
  
  ---
   drivers/acpi/video_detect.c |3 +++
   1 file changed, 3 insertions(+)
  
  Index: linux-pm/drivers/acpi/video_detect.c
  ===
  --- linux-pm.orig/drivers/acpi/video_detect.c
  +++ linux-pm/drivers/acpi/video_detect.c
  @@ -203,6 +203,9 @@ long acpi_video_get_capabilities(acpi_ha
   */
   
  dmi_check_system(video_detect_dmi_table);
  +
  +   if (acpi_gbl_osi_data = ACPI_OSI_WIN_8)
  +   acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;
 
 Then vendor driver(thinkpad_acpi) will step in and create a backlight
 interface for the system, which, unfortunately, is also broken for those
 win8 thinkpads.
 
 So we will need to do something in thinkpad_acpi to also not create
 backlight interface for these systems too.
 
 This actually doesn't feel bad to me, since the modules are blacklisting
 their own interfaces. The downside is of course, two quirk code exist.
 
 BTW, unregistering ACPI video's backlight interface in GPU driver doesn't
 have this problem since it made the platform driver think the ACPI video
 driver will control the backlight and then use the newly added API to
 remove ACPI video created backlight interface.

Yes, I know this.

I think I also know how to introduce that change in a slightly cleaner way.

I'll post a patch for comments later today or tomorrow.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-05 Thread Aaron Lu 
On 07/06/2013 06:23 AM, Rafael J. Wysocki wrote:
> On Friday, July 05, 2013 11:40:02 PM Rafael J. Wysocki wrote:
>> On Friday, July 05, 2013 10:00:55 PM Rafael J. Wysocki wrote:
>>> On Friday, July 05, 2013 02:20:14 PM Rafael J. Wysocki wrote:
 On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:
> Windows 8 leaves backlight control up to individual graphics drivers 
> rather
> than making ACPI calls itself. There's plenty of evidence to suggest that
> the Intel driver for Windows doesn't use the ACPI interface, including the
> fact that it's broken on a bunch of machines when the OS claims to support
> Windows 8. The simplest thing to do appears to be to disable the ACPI
> backlight interface on these systems.
>
> Signed-off-by: Matthew Garrett 
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c 
> b/drivers/gpu/drm/i915/i915_dma.c
> index 3b315ba..23b6292 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, 
> unsigned long flags)
>   /* Must be done after probing outputs */
>   intel_opregion_init(dev);
>   acpi_video_register();
> + /* Don't use ACPI backlight functions on Windows 8 platforms */
> + if (acpi_osi_version() >= ACPI_OSI_WIN_8)
> + acpi_video_backlight_unregister();
>   }
>  
>   if (IS_GEN5(dev))
>

 Well, this causes build failures to happen when the ACPI video driver is
 modular and the graphics driver is not.

 I'm not sure how to resolve that, so suggestions are welcome.
>>>
>>> Actually, that happened with the radeon patch.
>>>
>>> That said, ACPI_OSI_WIN_8 doesn't make much sense for !CONFIG_ACPI, for
>>> example.
>>>
>>> What about making acpi_video_register() do the quirk instead?  We could add 
>>> an
>>> argument to it indicating whether or not quirks should be applied.
>>
>> Actually, I wonder what about the appended patch (on top of the Aaron's
>> https://patchwork.kernel.org/patch/2812951/) instead of [1-3/3] from this 
>> series.
> 
> Or even something as simple as this one.
> 
> ---
>  drivers/acpi/video_detect.c |3 +++
>  1 file changed, 3 insertions(+)
> 
> Index: linux-pm/drivers/acpi/video_detect.c
> ===
> --- linux-pm.orig/drivers/acpi/video_detect.c
> +++ linux-pm/drivers/acpi/video_detect.c
> @@ -203,6 +203,9 @@ long acpi_video_get_capabilities(acpi_ha
>*/
>  
>   dmi_check_system(video_detect_dmi_table);
> +
> + if (acpi_gbl_osi_data >= ACPI_OSI_WIN_8)
> + acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;

Then vendor driver(thinkpad_acpi) will step in and create a backlight
interface for the system, which, unfortunately, is also broken for those
win8 thinkpads.

So we will need to do something in thinkpad_acpi to also not create
backlight interface for these systems too.

This actually doesn't feel bad to me, since the modules are blacklisting
their own interfaces. The downside is of course, two quirk code exist.

BTW, unregistering ACPI video's backlight interface in GPU driver doesn't
have this problem since it made the platform driver think the ACPI video
driver will control the backlight and then use the newly added API to
remove ACPI video created backlight interface.

Thanks,
Aaron

>   } else {
>   status = acpi_bus_get_device(graphics_handle, _dev);
>   if (ACPI_FAILURE(status)) {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-05 Thread Rafael J. Wysocki 
On Friday, July 05, 2013 11:40:02 PM Rafael J. Wysocki wrote:
> On Friday, July 05, 2013 10:00:55 PM Rafael J. Wysocki wrote:
> > On Friday, July 05, 2013 02:20:14 PM Rafael J. Wysocki wrote:
> > > On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:
> > > > Windows 8 leaves backlight control up to individual graphics drivers 
> > > > rather
> > > > than making ACPI calls itself. There's plenty of evidence to suggest 
> > > > that
> > > > the Intel driver for Windows doesn't use the ACPI interface, including 
> > > > the
> > > > fact that it's broken on a bunch of machines when the OS claims to 
> > > > support
> > > > Windows 8. The simplest thing to do appears to be to disable the ACPI
> > > > backlight interface on these systems.
> > > > 
> > > > Signed-off-by: Matthew Garrett 
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_dma.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c 
> > > > b/drivers/gpu/drm/i915/i915_dma.c
> > > > index 3b315ba..23b6292 100644
> > > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > > @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, 
> > > > unsigned long flags)
> > > > /* Must be done after probing outputs */
> > > > intel_opregion_init(dev);
> > > > acpi_video_register();
> > > > +   /* Don't use ACPI backlight functions on Windows 8 
> > > > platforms */
> > > > +   if (acpi_osi_version() >= ACPI_OSI_WIN_8)
> > > > +   acpi_video_backlight_unregister();
> > > > }
> > > >  
> > > > if (IS_GEN5(dev))
> > > > 
> > > 
> > > Well, this causes build failures to happen when the ACPI video driver is
> > > modular and the graphics driver is not.
> > > 
> > > I'm not sure how to resolve that, so suggestions are welcome.
> > 
> > Actually, that happened with the radeon patch.
> > 
> > That said, ACPI_OSI_WIN_8 doesn't make much sense for !CONFIG_ACPI, for
> > example.
> > 
> > What about making acpi_video_register() do the quirk instead?  We could add 
> > an
> > argument to it indicating whether or not quirks should be applied.
> 
> Actually, I wonder what about the appended patch (on top of the Aaron's
> https://patchwork.kernel.org/patch/2812951/) instead of [1-3/3] from this 
> series.

Or even something as simple as this one.

---
 drivers/acpi/video_detect.c |3 +++
 1 file changed, 3 insertions(+)

Index: linux-pm/drivers/acpi/video_detect.c
===
--- linux-pm.orig/drivers/acpi/video_detect.c
+++ linux-pm/drivers/acpi/video_detect.c
@@ -203,6 +203,9 @@ long acpi_video_get_capabilities(acpi_ha
 */
 
dmi_check_system(video_detect_dmi_table);
+
+   if (acpi_gbl_osi_data >= ACPI_OSI_WIN_8)
+   acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;
} else {
status = acpi_bus_get_device(graphics_handle, _dev);
if (ACPI_FAILURE(status)) {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-05 Thread Rafael J. Wysocki 
On Friday, July 05, 2013 10:00:55 PM Rafael J. Wysocki wrote:
> On Friday, July 05, 2013 02:20:14 PM Rafael J. Wysocki wrote:
> > On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:
> > > Windows 8 leaves backlight control up to individual graphics drivers 
> > > rather
> > > than making ACPI calls itself. There's plenty of evidence to suggest that
> > > the Intel driver for Windows doesn't use the ACPI interface, including the
> > > fact that it's broken on a bunch of machines when the OS claims to support
> > > Windows 8. The simplest thing to do appears to be to disable the ACPI
> > > backlight interface on these systems.
> > > 
> > > Signed-off-by: Matthew Garrett 
> > > ---
> > >  drivers/gpu/drm/i915/i915_dma.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_dma.c 
> > > b/drivers/gpu/drm/i915/i915_dma.c
> > > index 3b315ba..23b6292 100644
> > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, 
> > > unsigned long flags)
> > >   /* Must be done after probing outputs */
> > >   intel_opregion_init(dev);
> > >   acpi_video_register();
> > > + /* Don't use ACPI backlight functions on Windows 8 platforms */
> > > + if (acpi_osi_version() >= ACPI_OSI_WIN_8)
> > > + acpi_video_backlight_unregister();
> > >   }
> > >  
> > >   if (IS_GEN5(dev))
> > > 
> > 
> > Well, this causes build failures to happen when the ACPI video driver is
> > modular and the graphics driver is not.
> > 
> > I'm not sure how to resolve that, so suggestions are welcome.
> 
> Actually, that happened with the radeon patch.
> 
> That said, ACPI_OSI_WIN_8 doesn't make much sense for !CONFIG_ACPI, for
> example.
> 
> What about making acpi_video_register() do the quirk instead?  We could add an
> argument to it indicating whether or not quirks should be applied.

Actually, I wonder what about the appended patch (on top of the Aaron's
https://patchwork.kernel.org/patch/2812951/) instead of [1-3/3] from this 
series.

Thanks,
Rafael


---
 drivers/acpi/video_detect.c |   11 +--
 include/linux/acpi.h|1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/acpi/video_detect.c
===
--- linux-pm.orig/drivers/acpi/video_detect.c
+++ linux-pm/drivers/acpi/video_detect.c
@@ -203,6 +203,9 @@ long acpi_video_get_capabilities(acpi_ha
 */
 
dmi_check_system(video_detect_dmi_table);
+
+   if (acpi_gbl_osi_data >= ACPI_OSI_WIN_8)
+   acpi_video_support |= ACPI_VIDEO_FORCE_NO_BACKLIGHT;
} else {
status = acpi_bus_get_device(graphics_handle, _dev);
if (ACPI_FAILURE(status)) {
@@ -258,13 +261,17 @@ int acpi_video_backlight_support(void)
 {
acpi_video_caps_check();
 
-   /* First check for boot param -> highest prio */
+   /* First, check if no backlight support has been forced upon us. */
+   if (acpi_video_support & ACPI_VIDEO_FORCE_NO_BACKLIGHT)
+   return 0;
+
+   /* Next check for boot param -> second highest prio */
if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR)
return 0;
else if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO)
return 1;
 
-   /* Then check for DMI blacklist -> second highest prio */
+   /* Then check for DMI blacklist -> third highest prio */
if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_DMI_VENDOR)
return 0;
else if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_DMI_VIDEO)
Index: linux-pm/include/linux/acpi.h
===
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -191,6 +191,7 @@ extern bool wmi_has_guid(const char *gui
 #define ACPI_VIDEO_BACKLIGHT_DMI_VIDEO 0x0200
 #define ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VENDOR 0x0400
 #define ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VIDEO  0x0800
+#define ACPI_VIDEO_FORCE_NO_BACKLIGHT  0x1000
 
 #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-05 Thread Rafael J. Wysocki 
On Friday, July 05, 2013 02:20:14 PM Rafael J. Wysocki wrote:
> On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:
> > Windows 8 leaves backlight control up to individual graphics drivers rather
> > than making ACPI calls itself. There's plenty of evidence to suggest that
> > the Intel driver for Windows doesn't use the ACPI interface, including the
> > fact that it's broken on a bunch of machines when the OS claims to support
> > Windows 8. The simplest thing to do appears to be to disable the ACPI
> > backlight interface on these systems.
> > 
> > Signed-off-by: Matthew Garrett 
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c 
> > b/drivers/gpu/drm/i915/i915_dma.c
> > index 3b315ba..23b6292 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned 
> > long flags)
> > /* Must be done after probing outputs */
> > intel_opregion_init(dev);
> > acpi_video_register();
> > +   /* Don't use ACPI backlight functions on Windows 8 platforms */
> > +   if (acpi_osi_version() >= ACPI_OSI_WIN_8)
> > +   acpi_video_backlight_unregister();
> > }
> >  
> > if (IS_GEN5(dev))
> > 
> 
> Well, this causes build failures to happen when the ACPI video driver is
> modular and the graphics driver is not.
> 
> I'm not sure how to resolve that, so suggestions are welcome.

Actually, that happened with the radeon patch.

That said, ACPI_OSI_WIN_8 doesn't make much sense for !CONFIG_ACPI, for
example.

What about making acpi_video_register() do the quirk instead?  We could add an
argument to it indicating whether or not quirks should be applied.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-05 Thread Rafael J. Wysocki 
On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:
> Windows 8 leaves backlight control up to individual graphics drivers rather
> than making ACPI calls itself. There's plenty of evidence to suggest that
> the Intel driver for Windows doesn't use the ACPI interface, including the
> fact that it's broken on a bunch of machines when the OS claims to support
> Windows 8. The simplest thing to do appears to be to disable the ACPI
> backlight interface on these systems.
> 
> Signed-off-by: Matthew Garrett 
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 3b315ba..23b6292 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned 
> long flags)
>   /* Must be done after probing outputs */
>   intel_opregion_init(dev);
>   acpi_video_register();
> + /* Don't use ACPI backlight functions on Windows 8 platforms */
> + if (acpi_osi_version() >= ACPI_OSI_WIN_8)
> + acpi_video_backlight_unregister();
>   }
>  
>   if (IS_GEN5(dev))
> 

Well, this causes build failures to happen when the ACPI video driver is
modular and the graphics driver is not.

I'm not sure how to resolve that, so suggestions are welcome.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-05 Thread Rafael J. Wysocki 
On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:
 Windows 8 leaves backlight control up to individual graphics drivers rather
 than making ACPI calls itself. There's plenty of evidence to suggest that
 the Intel driver for Windows doesn't use the ACPI interface, including the
 fact that it's broken on a bunch of machines when the OS claims to support
 Windows 8. The simplest thing to do appears to be to disable the ACPI
 backlight interface on these systems.
 
 Signed-off-by: Matthew Garrett matthew.garr...@nebula.com
 ---
  drivers/gpu/drm/i915/i915_dma.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
 index 3b315ba..23b6292 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned 
 long flags)
   /* Must be done after probing outputs */
   intel_opregion_init(dev);
   acpi_video_register();
 + /* Don't use ACPI backlight functions on Windows 8 platforms */
 + if (acpi_osi_version() = ACPI_OSI_WIN_8)
 + acpi_video_backlight_unregister();
   }
  
   if (IS_GEN5(dev))
 

Well, this causes build failures to happen when the ACPI video driver is
modular and the graphics driver is not.

I'm not sure how to resolve that, so suggestions are welcome.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-05 Thread Rafael J. Wysocki 
On Friday, July 05, 2013 02:20:14 PM Rafael J. Wysocki wrote:
 On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:
  Windows 8 leaves backlight control up to individual graphics drivers rather
  than making ACPI calls itself. There's plenty of evidence to suggest that
  the Intel driver for Windows doesn't use the ACPI interface, including the
  fact that it's broken on a bunch of machines when the OS claims to support
  Windows 8. The simplest thing to do appears to be to disable the ACPI
  backlight interface on these systems.
  
  Signed-off-by: Matthew Garrett matthew.garr...@nebula.com
  ---
   drivers/gpu/drm/i915/i915_dma.c | 3 +++
   1 file changed, 3 insertions(+)
  
  diff --git a/drivers/gpu/drm/i915/i915_dma.c 
  b/drivers/gpu/drm/i915/i915_dma.c
  index 3b315ba..23b6292 100644
  --- a/drivers/gpu/drm/i915/i915_dma.c
  +++ b/drivers/gpu/drm/i915/i915_dma.c
  @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned 
  long flags)
  /* Must be done after probing outputs */
  intel_opregion_init(dev);
  acpi_video_register();
  +   /* Don't use ACPI backlight functions on Windows 8 platforms */
  +   if (acpi_osi_version() = ACPI_OSI_WIN_8)
  +   acpi_video_backlight_unregister();
  }
   
  if (IS_GEN5(dev))
  
 
 Well, this causes build failures to happen when the ACPI video driver is
 modular and the graphics driver is not.
 
 I'm not sure how to resolve that, so suggestions are welcome.

Actually, that happened with the radeon patch.

That said, ACPI_OSI_WIN_8 doesn't make much sense for !CONFIG_ACPI, for
example.

What about making acpi_video_register() do the quirk instead?  We could add an
argument to it indicating whether or not quirks should be applied.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-05 Thread Rafael J. Wysocki 
On Friday, July 05, 2013 10:00:55 PM Rafael J. Wysocki wrote:
 On Friday, July 05, 2013 02:20:14 PM Rafael J. Wysocki wrote:
  On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:
   Windows 8 leaves backlight control up to individual graphics drivers 
   rather
   than making ACPI calls itself. There's plenty of evidence to suggest that
   the Intel driver for Windows doesn't use the ACPI interface, including the
   fact that it's broken on a bunch of machines when the OS claims to support
   Windows 8. The simplest thing to do appears to be to disable the ACPI
   backlight interface on these systems.
   
   Signed-off-by: Matthew Garrett matthew.garr...@nebula.com
   ---
drivers/gpu/drm/i915/i915_dma.c | 3 +++
1 file changed, 3 insertions(+)
   
   diff --git a/drivers/gpu/drm/i915/i915_dma.c 
   b/drivers/gpu/drm/i915/i915_dma.c
   index 3b315ba..23b6292 100644
   --- a/drivers/gpu/drm/i915/i915_dma.c
   +++ b/drivers/gpu/drm/i915/i915_dma.c
   @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, 
   unsigned long flags)
 /* Must be done after probing outputs */
 intel_opregion_init(dev);
 acpi_video_register();
   + /* Don't use ACPI backlight functions on Windows 8 platforms */
   + if (acpi_osi_version() = ACPI_OSI_WIN_8)
   + acpi_video_backlight_unregister();
 }

 if (IS_GEN5(dev))
   
  
  Well, this causes build failures to happen when the ACPI video driver is
  modular and the graphics driver is not.
  
  I'm not sure how to resolve that, so suggestions are welcome.
 
 Actually, that happened with the radeon patch.
 
 That said, ACPI_OSI_WIN_8 doesn't make much sense for !CONFIG_ACPI, for
 example.
 
 What about making acpi_video_register() do the quirk instead?  We could add an
 argument to it indicating whether or not quirks should be applied.

Actually, I wonder what about the appended patch (on top of the Aaron's
https://patchwork.kernel.org/patch/2812951/) instead of [1-3/3] from this 
series.

Thanks,
Rafael


---
 drivers/acpi/video_detect.c |   11 +--
 include/linux/acpi.h|1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/acpi/video_detect.c
===
--- linux-pm.orig/drivers/acpi/video_detect.c
+++ linux-pm/drivers/acpi/video_detect.c
@@ -203,6 +203,9 @@ long acpi_video_get_capabilities(acpi_ha
 */
 
dmi_check_system(video_detect_dmi_table);
+
+   if (acpi_gbl_osi_data = ACPI_OSI_WIN_8)
+   acpi_video_support |= ACPI_VIDEO_FORCE_NO_BACKLIGHT;
} else {
status = acpi_bus_get_device(graphics_handle, tmp_dev);
if (ACPI_FAILURE(status)) {
@@ -258,13 +261,17 @@ int acpi_video_backlight_support(void)
 {
acpi_video_caps_check();
 
-   /* First check for boot param - highest prio */
+   /* First, check if no backlight support has been forced upon us. */
+   if (acpi_video_support  ACPI_VIDEO_FORCE_NO_BACKLIGHT)
+   return 0;
+
+   /* Next check for boot param - second highest prio */
if (acpi_video_support  ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR)
return 0;
else if (acpi_video_support  ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO)
return 1;
 
-   /* Then check for DMI blacklist - second highest prio */
+   /* Then check for DMI blacklist - third highest prio */
if (acpi_video_support  ACPI_VIDEO_BACKLIGHT_DMI_VENDOR)
return 0;
else if (acpi_video_support  ACPI_VIDEO_BACKLIGHT_DMI_VIDEO)
Index: linux-pm/include/linux/acpi.h
===
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -191,6 +191,7 @@ extern bool wmi_has_guid(const char *gui
 #define ACPI_VIDEO_BACKLIGHT_DMI_VIDEO 0x0200
 #define ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VENDOR 0x0400
 #define ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VIDEO  0x0800
+#define ACPI_VIDEO_FORCE_NO_BACKLIGHT  0x1000
 
 #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-05 Thread Rafael J. Wysocki 
On Friday, July 05, 2013 11:40:02 PM Rafael J. Wysocki wrote:
 On Friday, July 05, 2013 10:00:55 PM Rafael J. Wysocki wrote:
  On Friday, July 05, 2013 02:20:14 PM Rafael J. Wysocki wrote:
   On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:
Windows 8 leaves backlight control up to individual graphics drivers 
rather
than making ACPI calls itself. There's plenty of evidence to suggest 
that
the Intel driver for Windows doesn't use the ACPI interface, including 
the
fact that it's broken on a bunch of machines when the OS claims to 
support
Windows 8. The simplest thing to do appears to be to disable the ACPI
backlight interface on these systems.

Signed-off-by: Matthew Garrett matthew.garr...@nebula.com
---
 drivers/gpu/drm/i915/i915_dma.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_dma.c 
b/drivers/gpu/drm/i915/i915_dma.c
index 3b315ba..23b6292 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, 
unsigned long flags)
/* Must be done after probing outputs */
intel_opregion_init(dev);
acpi_video_register();
+   /* Don't use ACPI backlight functions on Windows 8 
platforms */
+   if (acpi_osi_version() = ACPI_OSI_WIN_8)
+   acpi_video_backlight_unregister();
}
 
if (IS_GEN5(dev))

   
   Well, this causes build failures to happen when the ACPI video driver is
   modular and the graphics driver is not.
   
   I'm not sure how to resolve that, so suggestions are welcome.
  
  Actually, that happened with the radeon patch.
  
  That said, ACPI_OSI_WIN_8 doesn't make much sense for !CONFIG_ACPI, for
  example.
  
  What about making acpi_video_register() do the quirk instead?  We could add 
  an
  argument to it indicating whether or not quirks should be applied.
 
 Actually, I wonder what about the appended patch (on top of the Aaron's
 https://patchwork.kernel.org/patch/2812951/) instead of [1-3/3] from this 
 series.

Or even something as simple as this one.

---
 drivers/acpi/video_detect.c |3 +++
 1 file changed, 3 insertions(+)

Index: linux-pm/drivers/acpi/video_detect.c
===
--- linux-pm.orig/drivers/acpi/video_detect.c
+++ linux-pm/drivers/acpi/video_detect.c
@@ -203,6 +203,9 @@ long acpi_video_get_capabilities(acpi_ha
 */
 
dmi_check_system(video_detect_dmi_table);
+
+   if (acpi_gbl_osi_data = ACPI_OSI_WIN_8)
+   acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;
} else {
status = acpi_bus_get_device(graphics_handle, tmp_dev);
if (ACPI_FAILURE(status)) {

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-07-05 Thread Aaron Lu 
On 07/06/2013 06:23 AM, Rafael J. Wysocki wrote:
 On Friday, July 05, 2013 11:40:02 PM Rafael J. Wysocki wrote:
 On Friday, July 05, 2013 10:00:55 PM Rafael J. Wysocki wrote:
 On Friday, July 05, 2013 02:20:14 PM Rafael J. Wysocki wrote:
 On Sunday, June 09, 2013 07:01:39 PM Matthew Garrett wrote:
 Windows 8 leaves backlight control up to individual graphics drivers 
 rather
 than making ACPI calls itself. There's plenty of evidence to suggest that
 the Intel driver for Windows doesn't use the ACPI interface, including the
 fact that it's broken on a bunch of machines when the OS claims to support
 Windows 8. The simplest thing to do appears to be to disable the ACPI
 backlight interface on these systems.

 Signed-off-by: Matthew Garrett matthew.garr...@nebula.com
 ---
  drivers/gpu/drm/i915/i915_dma.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/drivers/gpu/drm/i915/i915_dma.c 
 b/drivers/gpu/drm/i915/i915_dma.c
 index 3b315ba..23b6292 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, 
 unsigned long flags)
   /* Must be done after probing outputs */
   intel_opregion_init(dev);
   acpi_video_register();
 + /* Don't use ACPI backlight functions on Windows 8 platforms */
 + if (acpi_osi_version() = ACPI_OSI_WIN_8)
 + acpi_video_backlight_unregister();
   }
  
   if (IS_GEN5(dev))


 Well, this causes build failures to happen when the ACPI video driver is
 modular and the graphics driver is not.

 I'm not sure how to resolve that, so suggestions are welcome.

 Actually, that happened with the radeon patch.

 That said, ACPI_OSI_WIN_8 doesn't make much sense for !CONFIG_ACPI, for
 example.

 What about making acpi_video_register() do the quirk instead?  We could add 
 an
 argument to it indicating whether or not quirks should be applied.

 Actually, I wonder what about the appended patch (on top of the Aaron's
 https://patchwork.kernel.org/patch/2812951/) instead of [1-3/3] from this 
 series.
 
 Or even something as simple as this one.
 
 ---
  drivers/acpi/video_detect.c |3 +++
  1 file changed, 3 insertions(+)
 
 Index: linux-pm/drivers/acpi/video_detect.c
 ===
 --- linux-pm.orig/drivers/acpi/video_detect.c
 +++ linux-pm/drivers/acpi/video_detect.c
 @@ -203,6 +203,9 @@ long acpi_video_get_capabilities(acpi_ha
*/
  
   dmi_check_system(video_detect_dmi_table);
 +
 + if (acpi_gbl_osi_data = ACPI_OSI_WIN_8)
 + acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;

Then vendor driver(thinkpad_acpi) will step in and create a backlight
interface for the system, which, unfortunately, is also broken for those
win8 thinkpads.

So we will need to do something in thinkpad_acpi to also not create
backlight interface for these systems too.

This actually doesn't feel bad to me, since the modules are blacklisting
their own interfaces. The downside is of course, two quirk code exist.

BTW, unregistering ACPI video's backlight interface in GPU driver doesn't
have this problem since it made the platform driver think the ACPI video
driver will control the backlight and then use the newly added API to
remove ACPI video created backlight interface.

Thanks,
Aaron

   } else {
   status = acpi_bus_get_device(graphics_handle, tmp_dev);
   if (ACPI_FAILURE(status)) {
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-acpi in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-15 Thread Daniel Vetter 
On Sat, Jun 15, 2013 at 10:27:06PM +0200, Rafael J. Wysocki wrote:
> On Saturday, June 15, 2013 08:29:42 PM Daniel Vetter wrote:
> > Hi all,
> > 
> > So to me it looks like the discussion is going in circles a bit, hence let
> > me drop my maintainer-opinion here:
> > 
> > 1. Matthew's patch series here looks reasonable, and if it fixes a bunch
> > of systems (which it seems to) it has my Ack and imo should go in. If acpi
> > maintainers can smash their Ack onto the acpi parts I'd very much like to
> > merge this into drm-intel-next, that should give us the most coverage for
> > intel systems.
> > 
> > Len, Rafael, are you ok with the acpi part of this and merging it through
> > drm-intel-next?
> 
> It has to go through the ACPI tree because of the ACPICA patch that needs to
> be synchronized with the ACPICA upstream.  Sorry.

No problem, since we're pretty close to the merge window that would have
at most resulted in a few more weeks of testing in i915 trees anyway. -rc
kernels should still give us plenty of time to catch fallout, if there is
any.

> That said, I'm going to take this patchset.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-15 Thread Rafael J. Wysocki 
On Saturday, June 15, 2013 08:29:42 PM Daniel Vetter wrote:
> Hi all,
> 
> So to me it looks like the discussion is going in circles a bit, hence let
> me drop my maintainer-opinion here:
> 
> 1. Matthew's patch series here looks reasonable, and if it fixes a bunch
> of systems (which it seems to) it has my Ack and imo should go in. If acpi
> maintainers can smash their Ack onto the acpi parts I'd very much like to
> merge this into drm-intel-next, that should give us the most coverage for
> intel systems.
> 
> Len, Rafael, are you ok with the acpi part of this and merging it through
> drm-intel-next?

It has to go through the ACPI tree because of the ACPICA patch that needs to
be synchronized with the ACPICA upstream.  Sorry.

That said, I'm going to take this patchset.

> 2. Imo the current amount of quirking we expose to users (we have kernel
> options to disable the acpi interface, blacklist platform modules, all
> backlights can be tested through sysfs and on top of that xf86-video-intel
> has an xorg.conf to select the backlight used by the driver). I haven't
> spotted a compelling reason in this thread to add another one, what we
> have seems to be good enough to debug backligh issues.
> 
> 3. Also, adding yet another backlight quirk mechanism isn't gonna
> magically fix broken systems.
> 
> We _really_ should strive to make this just work and not offer the angry
> user another roll of duct-tape for free.
> 
> 4. The currently established priority selection for backlights of platform
> > firmware > raw seems to be good enough. Note that the explicit list in
> xf86-vidoe-intel is only used as a fallback for really old kernels which
> do not expose this information. We could probably rip it out.
> 
> 5. We've recently looked at opregion again and couldn't spot a hint.
> Unfortnately it looks like both noodling better information out of Intel
> and trying to publish an updated opregion spec seem to be losing games :(
> We'll keep on trying though.
> 
> Aside at the end: If the gnome tool indeed has its own backlight code and
> doesn't just use that as a fallback if the xrandr backligh property isn't
> available, then that's just a serious bug in gnome and should be fixed
> asap. But imo that's not something we should try to (nor do I see any way
> how to) work around in the kernel.

Agreed.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-15 Thread Matthew Garrett 
On Sat, Jun 15, 2013 at 08:29:42PM +0200, Daniel Vetter wrote:

> Aside at the end: If the gnome tool indeed has its own backlight code and
> doesn't just use that as a fallback if the xrandr backligh property isn't
> available, then that's just a serious bug in gnome and should be fixed
> asap. But imo that's not something we should try to (nor do I see any way
> how to) work around in the kernel.

It's only used if there's no backlight property on the display.

-- 
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-15 Thread Daniel Vetter 
Hi all,

So to me it looks like the discussion is going in circles a bit, hence let
me drop my maintainer-opinion here:

1. Matthew's patch series here looks reasonable, and if it fixes a bunch
of systems (which it seems to) it has my Ack and imo should go in. If acpi
maintainers can smash their Ack onto the acpi parts I'd very much like to
merge this into drm-intel-next, that should give us the most coverage for
intel systems.

Len, Rafael, are you ok with the acpi part of this and merging it through
drm-intel-next?

2. Imo the current amount of quirking we expose to users (we have kernel
options to disable the acpi interface, blacklist platform modules, all
backlights can be tested through sysfs and on top of that xf86-video-intel
has an xorg.conf to select the backlight used by the driver). I haven't
spotted a compelling reason in this thread to add another one, what we
have seems to be good enough to debug backligh issues.

3. Also, adding yet another backlight quirk mechanism isn't gonna
magically fix broken systems.

We _really_ should strive to make this just work and not offer the angry
user another roll of duct-tape for free.

4. The currently established priority selection for backlights of platform
> firmware > raw seems to be good enough. Note that the explicit list in
xf86-vidoe-intel is only used as a fallback for really old kernels which
do not expose this information. We could probably rip it out.

5. We've recently looked at opregion again and couldn't spot a hint.
Unfortnately it looks like both noodling better information out of Intel
and trying to publish an updated opregion spec seem to be losing games :(
We'll keep on trying though.

Aside at the end: If the gnome tool indeed has its own backlight code and
doesn't just use that as a fallback if the xrandr backligh property isn't
available, then that's just a serious bug in gnome and should be fixed
asap. But imo that's not something we should try to (nor do I see any way
how to) work around in the kernel.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-15 Thread Matthew Garrett 
On Sat, Jun 15, 2013 at 08:29:15PM +0800, Aaron Lu wrote:
> On 06/15/2013 12:19 PM, Matthew Garrett wrote:
> > The vendor will presumably have tested that backlight control works - if 
> > the GPU driver uses the ACPI interface and backlight control is broken, 
> > then the vendor would fix it.
> 
> I don't think GPU driver uses ACPI interface, that's why for some
> systems, ACPI interface is broken while the GPU's is not.

On systems where the ACPI interface is broken, the GPU driver clearly 
doesn't use the ACPI interface. As yet, we don't know if that's always 
true, though.

> > Sure, but it still requires the replacement of existing userspace. We 
> > need to fix the problem with existing userspace, too.
> 
> Yes. The problem is two way. First, some of the backlight interface
> privoder module provides a broken interface; Two, the userspace picked a
> broken interface while there are usable ones. For example, in the win8
> thinkpad case, the ACPI video driver provides broken interface and the
> GPU driver provides usable interface, but userspace choose ACPI video's
> interface. To make things complicated, if we make the broken interface
> disappear in ACPI video module, then the platform driver thinkpad_acpi
> will start to provide another broken interface. I have to say, solve it
> in the GPU code is a clever way, it's just a little weird to me for a
> broken interface to be blacklisted, we have to do it in another module,
> not the broken module itself.

The ACPI driver has no way of making this kind of policy decision. The 
GPU driver does.

> > No, I think we need to fix the bugs that currently require users to pass 
> > options.
> 
> For ACPI video driver, since it relies on firmware to do the right thing,
> if the functionality is broken, then it is, there is hardly anything we
> can do...(not always, we can quirk in some cases, but there are cases we
> can do nothing). In this case, we can:
> 1 Ask user to add acpi_backlight=vendor, so that ACPI video driver will
>   not create backlight interface and userspace will not pick it;
> 2 Add that model in DMI table in video_detect.c, eliminating the need for
>   that command line;
> 3 Add that model in DMI table in another module, let that module
>   unregister backlight interface provided by ACPI video as is done in
>   acer-wmi, asus-wmi, and i915 in this case;
> 3 Use the backlight section in xorg.conf to make X uses another
>   backlight interface. This may not work for distros that use
>   gsd-backlight-helper, which always prefers firmware.
> 
> Which one do you prefer?

I'd prefer to figure out how it works in Windows and implement it the 
same way.

> > How do these machines work under Windows? Until we know the answer to 
> > that, we don't know what the correct way to handle the problem is under 
> > Linux.
> 
> Do you mean we need to understand Windows behavior so that we can
> decide where to place the code to do the backlight management thing?
> So for win8 case, we know MS will use GPU driver to do backlight
> control, all this means to me is, ACPI video's and platform driver's
> interface are more likely broken on those systems, but it doesn't
> qualify why Linux' GPU driver should do the decision, it's not that the
> GPU driver can poke some GPU register and then decide oh, this model
> does not have working ACPI backlight interface. As this patch shows, we
> make the decision by OSI, which is not specific to GPU driver.

Sure it is - for all we know there's a value in the opregion space that 
tells the Intel GPU driver which interface we should be using. We don't 
know because Intel haven't released a version of the opregion driver 
newer than 2007. It may be that all Windows 8 GPU drivers use the GPU 
backlight control registers directly and never use any firmware 
interface, but right now we simply don't know that. All we can do is 
look at the existing cases we have and say that it appears that 
Intel graphics systems don't use the ACPI interface. Anything more than 
that requires more evidence.

> BTW, I don't think any module knows something better than another
> module, all quirk logic and DMI entry we currently have in Linux are
> got by user's feedback(bug reports), it doesn't seem to me some module
> is natrually the one that should make this decision. Or do I miss
> something?

The GPU driver makes the policy decision under Windows 8, so it's the 
natural place to make this decision on Windows 8 systems.

-- 
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-15 Thread Aaron Lu 
On 06/15/2013 12:19 PM, Matthew Garrett wrote:
> On Sat, Jun 15, 2013 at 12:14:42PM +0800, Aaron Lu wrote:
>> On 06/15/2013 09:38 AM, Matthew Garrett wrote:
>>> Well, Windows 8 will only use the ACPI backlight interface if the GPU
>>> driver decides to, right? So the logic for deciding whether to remove
>>> the ACPI backlight control or not should be left up to the GPU. There's
>>
>> I don't know this. From the document I googled, Microsoft suggests under
>> win8, backlight should be controlled by the graphics driver for smooth
>> brightness level change, instead of ACPI or other methods. So it is
>> possible that OEM will not test the ACPI interface well and thus the
>> interface is likely broken. I don't see why GPU driver has any better
>> knowledge on which systems the firmware interface is broken or not.
> 
> The vendor will presumably have tested that backlight control works - if 
> the GPU driver uses the ACPI interface and backlight control is broken, 
> then the vendor would fix it.

I don't think GPU driver uses ACPI interface, that's why for some
systems, ACPI interface is broken while the GPU's is not.

> 
>>> no harm in refusing to expose a working method if there's another
>>> working method, but there is harm in exposing a broken one and expecting
>>> userspace to know the difference.
>>
>> BTW, the proposed solution is not meant to solve win8 problems alone, it
>> should make solving other problems easy and make individual backlight
>> control interface provider module independent with each other such as
>> the platform drivers will not need to check if ACPI video driver will
>> control backlight or not and can always create backlight interface(its
>> default priority is lower that ACPI video driver's so will not be taken
>> by user space by default, showing the same behavior of the current code).
> 
> Sure, but it still requires the replacement of existing userspace. We 
> need to fix the problem with existing userspace, too.

Yes. The problem is two way. First, some of the backlight interface
privoder module provides a broken interface; Two, the userspace picked a
broken interface while there are usable ones. For example, in the win8
thinkpad case, the ACPI video driver provides broken interface and the
GPU driver provides usable interface, but userspace choose ACPI video's
interface. To make things complicated, if we make the broken interface
disappear in ACPI video module, then the platform driver thinkpad_acpi
will start to provide another broken interface. I have to say, solve it
in the GPU code is a clever way, it's just a little weird to me for a
broken interface to be blacklisted, we have to do it in another module,
not the broken module itself.

> 
>> The current acpi_backlight=video/vendor kernel command line is pretty
>> misleading, for laptops that do not have vendor backlight interface,
>> adding acpi_backlight=vendor actually makes the system using the GPU's
>> interface. Some laptops are using this switch to work around problems in
>> ACPI video driver and users think they are using vendor interface.
>> That's why I think we need a new command line as the
>> backlight.force_interface=raw/firmware/platform.
> 
> No, I think we need to fix the bugs that currently require users to pass 
> options.

For ACPI video driver, since it relies on firmware to do the right thing,
if the functionality is broken, then it is, there is hardly anything we
can do...(not always, we can quirk in some cases, but there are cases we
can do nothing). In this case, we can:
1 Ask user to add acpi_backlight=vendor, so that ACPI video driver will
  not create backlight interface and userspace will not pick it;
2 Add that model in DMI table in video_detect.c, eliminating the need for
  that command line;
3 Add that model in DMI table in another module, let that module
  unregister backlight interface provided by ACPI video as is done in
  acer-wmi, asus-wmi, and i915 in this case;
3 Use the backlight section in xorg.conf to make X uses another
  backlight interface. This may not work for distros that use
  gsd-backlight-helper, which always prefers firmware.

Which one do you prefer?

> 
>> Instead of letting individual driver to make decisions on which
>> backlight interface this system should use(either in platform driver as
>> we currently did, see acer-wmi and asus-wmi, or GPU driver as this case),
>> I think we need a better and clear way to handle such things. For
>> example, suppose an acer laptop: vendor does not support backlight, ACPI
>> video's backlight interface is broken and GPU's works, to make it work,
>> user will need to select acer-wmi module while this module does not have
>> anything to do with the functionality, but simply because it serves as
>> the backlight manager for acer laptops.
> 
> How do these machines work under Windows? Until we know the answer to 
> that, we don't know what the correct way to handle the problem is under 
> Linux.

Do you mean we need to understand

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-15 Thread Aaron Lu 
On 06/15/2013 12:19 PM, Matthew Garrett wrote:
 On Sat, Jun 15, 2013 at 12:14:42PM +0800, Aaron Lu wrote:
 On 06/15/2013 09:38 AM, Matthew Garrett wrote:
 Well, Windows 8 will only use the ACPI backlight interface if the GPU
 driver decides to, right? So the logic for deciding whether to remove
 the ACPI backlight control or not should be left up to the GPU. There's

 I don't know this. From the document I googled, Microsoft suggests under
 win8, backlight should be controlled by the graphics driver for smooth
 brightness level change, instead of ACPI or other methods. So it is
 possible that OEM will not test the ACPI interface well and thus the
 interface is likely broken. I don't see why GPU driver has any better
 knowledge on which systems the firmware interface is broken or not.
 
 The vendor will presumably have tested that backlight control works - if 
 the GPU driver uses the ACPI interface and backlight control is broken, 
 then the vendor would fix it.

I don't think GPU driver uses ACPI interface, that's why for some
systems, ACPI interface is broken while the GPU's is not.

 
 no harm in refusing to expose a working method if there's another
 working method, but there is harm in exposing a broken one and expecting
 userspace to know the difference.

 BTW, the proposed solution is not meant to solve win8 problems alone, it
 should make solving other problems easy and make individual backlight
 control interface provider module independent with each other such as
 the platform drivers will not need to check if ACPI video driver will
 control backlight or not and can always create backlight interface(its
 default priority is lower that ACPI video driver's so will not be taken
 by user space by default, showing the same behavior of the current code).
 
 Sure, but it still requires the replacement of existing userspace. We 
 need to fix the problem with existing userspace, too.

Yes. The problem is two way. First, some of the backlight interface
privoder module provides a broken interface; Two, the userspace picked a
broken interface while there are usable ones. For example, in the win8
thinkpad case, the ACPI video driver provides broken interface and the
GPU driver provides usable interface, but userspace choose ACPI video's
interface. To make things complicated, if we make the broken interface
disappear in ACPI video module, then the platform driver thinkpad_acpi
will start to provide another broken interface. I have to say, solve it
in the GPU code is a clever way, it's just a little weird to me for a
broken interface to be blacklisted, we have to do it in another module,
not the broken module itself.

 
 The current acpi_backlight=video/vendor kernel command line is pretty
 misleading, for laptops that do not have vendor backlight interface,
 adding acpi_backlight=vendor actually makes the system using the GPU's
 interface. Some laptops are using this switch to work around problems in
 ACPI video driver and users think they are using vendor interface.
 That's why I think we need a new command line as the
 backlight.force_interface=raw/firmware/platform.
 
 No, I think we need to fix the bugs that currently require users to pass 
 options.

For ACPI video driver, since it relies on firmware to do the right thing,
if the functionality is broken, then it is, there is hardly anything we
can do...(not always, we can quirk in some cases, but there are cases we
can do nothing). In this case, we can:
1 Ask user to add acpi_backlight=vendor, so that ACPI video driver will
  not create backlight interface and userspace will not pick it;
2 Add that model in DMI table in video_detect.c, eliminating the need for
  that command line;
3 Add that model in DMI table in another module, let that module
  unregister backlight interface provided by ACPI video as is done in
  acer-wmi, asus-wmi, and i915 in this case;
3 Use the backlight section in xorg.conf to make X uses another
  backlight interface. This may not work for distros that use
  gsd-backlight-helper, which always prefers firmware.

Which one do you prefer?

 
 Instead of letting individual driver to make decisions on which
 backlight interface this system should use(either in platform driver as
 we currently did, see acer-wmi and asus-wmi, or GPU driver as this case),
 I think we need a better and clear way to handle such things. For
 example, suppose an acer laptop: vendor does not support backlight, ACPI
 video's backlight interface is broken and GPU's works, to make it work,
 user will need to select acer-wmi module while this module does not have
 anything to do with the functionality, but simply because it serves as
 the backlight manager for acer laptops.
 
 How do these machines work under Windows? Until we know the answer to 
 that, we don't know what the correct way to handle the problem is under 
 Linux.

Do you mean we need to understand Windows behavior so that we can
decide where to place the code to do the backlight management thing?
So

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-15 Thread Matthew Garrett 
On Sat, Jun 15, 2013 at 08:29:15PM +0800, Aaron Lu wrote:
 On 06/15/2013 12:19 PM, Matthew Garrett wrote:
  The vendor will presumably have tested that backlight control works - if 
  the GPU driver uses the ACPI interface and backlight control is broken, 
  then the vendor would fix it.
 
 I don't think GPU driver uses ACPI interface, that's why for some
 systems, ACPI interface is broken while the GPU's is not.

On systems where the ACPI interface is broken, the GPU driver clearly 
doesn't use the ACPI interface. As yet, we don't know if that's always 
true, though.

  Sure, but it still requires the replacement of existing userspace. We 
  need to fix the problem with existing userspace, too.
 
 Yes. The problem is two way. First, some of the backlight interface
 privoder module provides a broken interface; Two, the userspace picked a
 broken interface while there are usable ones. For example, in the win8
 thinkpad case, the ACPI video driver provides broken interface and the
 GPU driver provides usable interface, but userspace choose ACPI video's
 interface. To make things complicated, if we make the broken interface
 disappear in ACPI video module, then the platform driver thinkpad_acpi
 will start to provide another broken interface. I have to say, solve it
 in the GPU code is a clever way, it's just a little weird to me for a
 broken interface to be blacklisted, we have to do it in another module,
 not the broken module itself.

The ACPI driver has no way of making this kind of policy decision. The 
GPU driver does.

  No, I think we need to fix the bugs that currently require users to pass 
  options.
 
 For ACPI video driver, since it relies on firmware to do the right thing,
 if the functionality is broken, then it is, there is hardly anything we
 can do...(not always, we can quirk in some cases, but there are cases we
 can do nothing). In this case, we can:
 1 Ask user to add acpi_backlight=vendor, so that ACPI video driver will
   not create backlight interface and userspace will not pick it;
 2 Add that model in DMI table in video_detect.c, eliminating the need for
   that command line;
 3 Add that model in DMI table in another module, let that module
   unregister backlight interface provided by ACPI video as is done in
   acer-wmi, asus-wmi, and i915 in this case;
 3 Use the backlight section in xorg.conf to make X uses another
   backlight interface. This may not work for distros that use
   gsd-backlight-helper, which always prefers firmware.
 
 Which one do you prefer?

I'd prefer to figure out how it works in Windows and implement it the 
same way.

  How do these machines work under Windows? Until we know the answer to 
  that, we don't know what the correct way to handle the problem is under 
  Linux.
 
 Do you mean we need to understand Windows behavior so that we can
 decide where to place the code to do the backlight management thing?
 So for win8 case, we know MS will use GPU driver to do backlight
 control, all this means to me is, ACPI video's and platform driver's
 interface are more likely broken on those systems, but it doesn't
 qualify why Linux' GPU driver should do the decision, it's not that the
 GPU driver can poke some GPU register and then decide oh, this model
 does not have working ACPI backlight interface. As this patch shows, we
 make the decision by OSI, which is not specific to GPU driver.

Sure it is - for all we know there's a value in the opregion space that 
tells the Intel GPU driver which interface we should be using. We don't 
know because Intel haven't released a version of the opregion driver 
newer than 2007. It may be that all Windows 8 GPU drivers use the GPU 
backlight control registers directly and never use any firmware 
interface, but right now we simply don't know that. All we can do is 
look at the existing cases we have and say that it appears that 
Intel graphics systems don't use the ACPI interface. Anything more than 
that requires more evidence.

 BTW, I don't think any module knows something better than another
 module, all quirk logic and DMI entry we currently have in Linux are
 got by user's feedback(bug reports), it doesn't seem to me some module
 is natrually the one that should make this decision. Or do I miss
 something?

The GPU driver makes the policy decision under Windows 8, so it's the 
natural place to make this decision on Windows 8 systems.

-- 
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-15 Thread Daniel Vetter 
Hi all,

So to me it looks like the discussion is going in circles a bit, hence let
me drop my maintainer-opinion here:

1. Matthew's patch series here looks reasonable, and if it fixes a bunch
of systems (which it seems to) it has my Ack and imo should go in. If acpi
maintainers can smash their Ack onto the acpi parts I'd very much like to
merge this into drm-intel-next, that should give us the most coverage for
intel systems.

Len, Rafael, are you ok with the acpi part of this and merging it through
drm-intel-next?

2. Imo the current amount of quirking we expose to users (we have kernel
options to disable the acpi interface, blacklist platform modules, all
backlights can be tested through sysfs and on top of that xf86-video-intel
has an xorg.conf to select the backlight used by the driver). I haven't
spotted a compelling reason in this thread to add another one, what we
have seems to be good enough to debug backligh issues.

3. Also, adding yet another backlight quirk mechanism isn't gonna
magically fix broken systems.

We _really_ should strive to make this just work and not offer the angry
user another roll of duct-tape for free.

4. The currently established priority selection for backlights of platform
 firmware  raw seems to be good enough. Note that the explicit list in
xf86-vidoe-intel is only used as a fallback for really old kernels which
do not expose this information. We could probably rip it out.

5. We've recently looked at opregion again and couldn't spot a hint.
Unfortnately it looks like both noodling better information out of Intel
and trying to publish an updated opregion spec seem to be losing games :(
We'll keep on trying though.

Aside at the end: If the gnome tool indeed has its own backlight code and
doesn't just use that as a fallback if the xrandr backligh property isn't
available, then that's just a serious bug in gnome and should be fixed
asap. But imo that's not something we should try to (nor do I see any way
how to) work around in the kernel.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-15 Thread Matthew Garrett 
On Sat, Jun 15, 2013 at 08:29:42PM +0200, Daniel Vetter wrote:

 Aside at the end: If the gnome tool indeed has its own backlight code and
 doesn't just use that as a fallback if the xrandr backligh property isn't
 available, then that's just a serious bug in gnome and should be fixed
 asap. But imo that's not something we should try to (nor do I see any way
 how to) work around in the kernel.

It's only used if there's no backlight property on the display.

-- 
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-15 Thread Rafael J. Wysocki 
On Saturday, June 15, 2013 08:29:42 PM Daniel Vetter wrote:
 Hi all,
 
 So to me it looks like the discussion is going in circles a bit, hence let
 me drop my maintainer-opinion here:
 
 1. Matthew's patch series here looks reasonable, and if it fixes a bunch
 of systems (which it seems to) it has my Ack and imo should go in. If acpi
 maintainers can smash their Ack onto the acpi parts I'd very much like to
 merge this into drm-intel-next, that should give us the most coverage for
 intel systems.
 
 Len, Rafael, are you ok with the acpi part of this and merging it through
 drm-intel-next?

It has to go through the ACPI tree because of the ACPICA patch that needs to
be synchronized with the ACPICA upstream.  Sorry.

That said, I'm going to take this patchset.

 2. Imo the current amount of quirking we expose to users (we have kernel
 options to disable the acpi interface, blacklist platform modules, all
 backlights can be tested through sysfs and on top of that xf86-video-intel
 has an xorg.conf to select the backlight used by the driver). I haven't
 spotted a compelling reason in this thread to add another one, what we
 have seems to be good enough to debug backligh issues.
 
 3. Also, adding yet another backlight quirk mechanism isn't gonna
 magically fix broken systems.
 
 We _really_ should strive to make this just work and not offer the angry
 user another roll of duct-tape for free.
 
 4. The currently established priority selection for backlights of platform
  firmware  raw seems to be good enough. Note that the explicit list in
 xf86-vidoe-intel is only used as a fallback for really old kernels which
 do not expose this information. We could probably rip it out.
 
 5. We've recently looked at opregion again and couldn't spot a hint.
 Unfortnately it looks like both noodling better information out of Intel
 and trying to publish an updated opregion spec seem to be losing games :(
 We'll keep on trying though.
 
 Aside at the end: If the gnome tool indeed has its own backlight code and
 doesn't just use that as a fallback if the xrandr backligh property isn't
 available, then that's just a serious bug in gnome and should be fixed
 asap. But imo that's not something we should try to (nor do I see any way
 how to) work around in the kernel.

Agreed.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-15 Thread Daniel Vetter 
On Sat, Jun 15, 2013 at 10:27:06PM +0200, Rafael J. Wysocki wrote:
 On Saturday, June 15, 2013 08:29:42 PM Daniel Vetter wrote:
  Hi all,
  
  So to me it looks like the discussion is going in circles a bit, hence let
  me drop my maintainer-opinion here:
  
  1. Matthew's patch series here looks reasonable, and if it fixes a bunch
  of systems (which it seems to) it has my Ack and imo should go in. If acpi
  maintainers can smash their Ack onto the acpi parts I'd very much like to
  merge this into drm-intel-next, that should give us the most coverage for
  intel systems.
  
  Len, Rafael, are you ok with the acpi part of this and merging it through
  drm-intel-next?
 
 It has to go through the ACPI tree because of the ACPICA patch that needs to
 be synchronized with the ACPICA upstream.  Sorry.

No problem, since we're pretty close to the merge window that would have
at most resulted in a few more weeks of testing in i915 trees anyway. -rc
kernels should still give us plenty of time to catch fallout, if there is
any.

 That said, I'm going to take this patchset.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-14 Thread Matthew Garrett 
On Sat, Jun 15, 2013 at 12:14:42PM +0800, Aaron Lu wrote:
> On 06/15/2013 09:38 AM, Matthew Garrett wrote:
> > Well, Windows 8 will only use the ACPI backlight interface if the GPU
> > driver decides to, right? So the logic for deciding whether to remove
> > the ACPI backlight control or not should be left up to the GPU. There's
> 
> I don't know this. From the document I googled, Microsoft suggests under
> win8, backlight should be controlled by the graphics driver for smooth
> brightness level change, instead of ACPI or other methods. So it is
> possible that OEM will not test the ACPI interface well and thus the
> interface is likely broken. I don't see why GPU driver has any better
> knowledge on which systems the firmware interface is broken or not.

The vendor will presumably have tested that backlight control works - if 
the GPU driver uses the ACPI interface and backlight control is broken, 
then the vendor would fix it.

> > no harm in refusing to expose a working method if there's another
> > working method, but there is harm in exposing a broken one and expecting
> > userspace to know the difference.
> 
> BTW, the proposed solution is not meant to solve win8 problems alone, it
> should make solving other problems easy and make individual backlight
> control interface provider module independent with each other such as
> the platform drivers will not need to check if ACPI video driver will
> control backlight or not and can always create backlight interface(its
> default priority is lower that ACPI video driver's so will not be taken
> by user space by default, showing the same behavior of the current code).

Sure, but it still requires the replacement of existing userspace. We 
need to fix the problem with existing userspace, too.

> The current acpi_backlight=video/vendor kernel command line is pretty
> misleading, for laptops that do not have vendor backlight interface,
> adding acpi_backlight=vendor actually makes the system using the GPU's
> interface. Some laptops are using this switch to work around problems in
> ACPI video driver and users think they are using vendor interface.
> That's why I think we need a new command line as the
> backlight.force_interface=raw/firmware/platform.

No, I think we need to fix the bugs that currently require users to pass 
options.

> Instead of letting individual driver to make decisions on which
> backlight interface this system should use(either in platform driver as
> we currently did, see acer-wmi and asus-wmi, or GPU driver as this case),
> I think we need a better and clear way to handle such things. For
> example, suppose an acer laptop: vendor does not support backlight, ACPI
> video's backlight interface is broken and GPU's works, to make it work,
> user will need to select acer-wmi module while this module does not have
> anything to do with the functionality, but simply because it serves as
> the backlight manager for acer laptops.

How do these machines work under Windows? Until we know the answer to 
that, we don't know what the correct way to handle the problem is under 
Linux.

-- 
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-14 Thread Aaron Lu 
On 06/15/2013 09:38 AM, Matthew Garrett wrote:
> On Sat, 2013-06-15 at 09:26 +0800, Aaron Lu wrote:
>> It's not easy to decide if they work or not sometimes, e.g. I came
>> across a system that claims win8 in ACPI table and has an Intel GPU,
>> while its ACPI video interface also works. With this patch, the working
>> ACPI video interface is removed, while with the priority based solution,
>> the GPU's interface priority gets higher, but the ACPI video interface
>> still stays.
> 
> Well, Windows 8 will only use the ACPI backlight interface if the GPU
> driver decides to, right? So the logic for deciding whether to remove
> the ACPI backlight control or not should be left up to the GPU. There's

I don't know this. From the document I googled, Microsoft suggests under
win8, backlight should be controlled by the graphics driver for smooth
brightness level change, instead of ACPI or other methods. So it is
possible that OEM will not test the ACPI interface well and thus the
interface is likely broken. I don't see why GPU driver has any better
knowledge on which systems the firmware interface is broken or not.

> no harm in refusing to expose a working method if there's another
> working method, but there is harm in exposing a broken one and expecting
> userspace to know the difference.

BTW, the proposed solution is not meant to solve win8 problems alone, it
should make solving other problems easy and make individual backlight
control interface provider module independent with each other such as
the platform drivers will not need to check if ACPI video driver will
control backlight or not and can always create backlight interface(its
default priority is lower that ACPI video driver's so will not be taken
by user space by default, showing the same behavior of the current code).

The current acpi_backlight=video/vendor kernel command line is pretty
misleading, for laptops that do not have vendor backlight interface,
adding acpi_backlight=vendor actually makes the system using the GPU's
interface. Some laptops are using this switch to work around problems in
ACPI video driver and users think they are using vendor interface.
That's why I think we need a new command line as the
backlight.force_interface=raw/firmware/platform.

Instead of letting individual driver to make decisions on which
backlight interface this system should use(either in platform driver as
we currently did, see acer-wmi and asus-wmi, or GPU driver as this case),
I think we need a better and clear way to handle such things. For
example, suppose an acer laptop: vendor does not support backlight, ACPI
video's backlight interface is broken and GPU's works, to make it work,
user will need to select acer-wmi module while this module does not have
anything to do with the functionality, but simply because it serves as
the backlight manager for acer laptops.

The above information and idea is formed while solving bugs reported
in kernel bugzilla video component, the proposed solutin may not be good
enough, but I hope we can find a better way to handle backlight problems.

Thanks,
Aaron
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-14 Thread Matthew Garrett 
On Sat, 2013-06-15 at 09:26 +0800, Aaron Lu wrote:
> On 06/15/2013 01:29 AM, Matthew Garrett wrote: 
> > How would that work with existing userspace?
> 
> User space tool will need to be updated to use this as stated in the
> gist page, I've patches for gsd-backlight-helper and xorg-x11-drv-intel,
> for others we can add I think if the priority based solution is deemed
> useful.

Right, that's not a great solution.

> > We shouldn't export interfaces if we don't expect them to work.
> 
> It's not easy to decide if they work or not sometimes, e.g. I came
> across a system that claims win8 in ACPI table and has an Intel GPU,
> while its ACPI video interface also works. With this patch, the working
> ACPI video interface is removed, while with the priority based solution,
> the GPU's interface priority gets higher, but the ACPI video interface
> still stays.

Well, Windows 8 will only use the ACPI backlight interface if the GPU
driver decides to, right? So the logic for deciding whether to remove
the ACPI backlight control or not should be left up to the GPU. There's
no harm in refusing to expose a working method if there's another
working method, but there is harm in exposing a broken one and expecting
userspace to know the difference.

-- 
Matthew Garrett | mj...@srcf.ucam.org
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-14 Thread Aaron Lu 
On 06/15/2013 01:29 AM, Matthew Garrett wrote:
> On Fri, 2013-06-14 at 14:47 +0800, Aaron Lu wrote:
> 
>> What about a priority based solution? We can introduce a new field named
>> priority to backlight_device and instead of calling another module's
>> function like the unregister one here(which cause unnecessary module
>> dependency), we only need to boost priority for its own interface. This
>> field will be exported to sysfs, so user can change it during runtime
>> too. And we can also introduce a new kernel command line as
>> backlight.force_interface=raw/firmware/platform, to overcome the limited
>> functionality provided by acpi_backlight=video/vendor, which does not
>> involve GPU's interface.
> 
> How would that work with existing userspace?

User space tool will need to be updated to use this as stated in the
gist page, I've patches for gsd-backlight-helper and xorg-x11-drv-intel,
for others we can add I think if the priority based solution is deemed
useful.

> 
>> And we can place the quirk code in backlight layer instead of individual
>> backlight functionality provider module. Suppose we have a backlight
>> manager there, for all win8 systems, we can boost the raw type's
>> priority on its registration, so no need to add code in
>> intel/amd/etc./'s GPU driver code.
> 
> But we'd need to add code to every piece of userspace that currently
> uses the backlight, right?

Yes that's the case.

> 
>> With priority based solution, all backlight control interfaces stay,
>> the priority field is an indication given by kernel to user space.
> 
> We shouldn't export interfaces if we don't expect them to work.

It's not easy to decide if they work or not sometimes, e.g. I came
across a system that claims win8 in ACPI table and has an Intel GPU,
while its ACPI video interface also works. With this patch, the working
ACPI video interface is removed, while with the priority based solution,
the GPU's interface priority gets higher, but the ACPI video interface
still stays.

Thanks,
Aaron
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-14 Thread Matthew Garrett 
On Fri, 2013-06-14 at 14:47 +0800, Aaron Lu wrote:

> What about a priority based solution? We can introduce a new field named
> priority to backlight_device and instead of calling another module's
> function like the unregister one here(which cause unnecessary module
> dependency), we only need to boost priority for its own interface. This
> field will be exported to sysfs, so user can change it during runtime
> too. And we can also introduce a new kernel command line as
> backlight.force_interface=raw/firmware/platform, to overcome the limited
> functionality provided by acpi_backlight=video/vendor, which does not
> involve GPU's interface.

How would that work with existing userspace?

> And we can place the quirk code in backlight layer instead of individual
> backlight functionality provider module. Suppose we have a backlight
> manager there, for all win8 systems, we can boost the raw type's
> priority on its registration, so no need to add code in
> intel/amd/etc./'s GPU driver code.

But we'd need to add code to every piece of userspace that currently
uses the backlight, right?

> With priority based solution, all backlight control interfaces stay,
> the priority field is an indication given by kernel to user space.

We shouldn't export interfaces if we don't expect them to work.

-- 
Matthew Garrett | mj...@srcf.ucam.org

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-14 Thread Aaron Lu 
On 06/10/2013 07:01 AM, Matthew Garrett wrote:
> Windows 8 leaves backlight control up to individual graphics drivers rather
> than making ACPI calls itself. There's plenty of evidence to suggest that
> the Intel driver for Windows doesn't use the ACPI interface, including the
> fact that it's broken on a bunch of machines when the OS claims to support
> Windows 8. The simplest thing to do appears to be to disable the ACPI
> backlight interface on these systems.
> 
> Signed-off-by: Matthew Garrett 
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 3b315ba..23b6292 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned 
> long flags)
>   /* Must be done after probing outputs */
>   intel_opregion_init(dev);
>   acpi_video_register();
> + /* Don't use ACPI backlight functions on Windows 8 platforms */
> + if (acpi_osi_version() >= ACPI_OSI_WIN_8)
> + acpi_video_backlight_unregister();

What about the platform driver created backlight interface? Oh right,
thinkpad_acpi will not create backlight interface for them since they
are not in DMI table, so acpi_video_backlight_support() will return
true and thinkspad_acpi thinks ACPI video driver is controlling
backlight so it will not create backlight interface for them.

Then we will need to remember not to add any of those systems into
the DMI table of video_detect.c, or thinkpad_acpi module will create
backlight interface for them and according to reporter, that interface
does not work either.

What about a priority based solution? We can introduce a new field named
priority to backlight_device and instead of calling another module's
function like the unregister one here(which cause unnecessary module
dependency), we only need to boost priority for its own interface. This
field will be exported to sysfs, so user can change it during runtime
too. And we can also introduce a new kernel command line as
backlight.force_interface=raw/firmware/platform, to overcome the limited
functionality provided by acpi_backlight=video/vendor, which does not
involve GPU's interface.

And we can place the quirk code in backlight layer instead of individual
backlight functionality provider module. Suppose we have a backlight
manager there, for all win8 systems, we can boost the raw type's
priority on its registration, so no need to add code in
intel/amd/etc./'s GPU driver code.

With priority based solution, all backlight control interfaces stay,
the priority field is an indication given by kernel to user space.

For a complete description on backlight control background and the
proposed solution, please take a look at:
https://gist.github.com/aaronlu/5779828

It would be good to know your opinions, thanks.

-Aaron
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-14 Thread Aaron Lu 
On 06/10/2013 07:01 AM, Matthew Garrett wrote:
 Windows 8 leaves backlight control up to individual graphics drivers rather
 than making ACPI calls itself. There's plenty of evidence to suggest that
 the Intel driver for Windows doesn't use the ACPI interface, including the
 fact that it's broken on a bunch of machines when the OS claims to support
 Windows 8. The simplest thing to do appears to be to disable the ACPI
 backlight interface on these systems.
 
 Signed-off-by: Matthew Garrett matthew.garr...@nebula.com
 ---
  drivers/gpu/drm/i915/i915_dma.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
 index 3b315ba..23b6292 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned 
 long flags)
   /* Must be done after probing outputs */
   intel_opregion_init(dev);
   acpi_video_register();
 + /* Don't use ACPI backlight functions on Windows 8 platforms */
 + if (acpi_osi_version() = ACPI_OSI_WIN_8)
 + acpi_video_backlight_unregister();

What about the platform driver created backlight interface? Oh right,
thinkpad_acpi will not create backlight interface for them since they
are not in DMI table, so acpi_video_backlight_support() will return
true and thinkspad_acpi thinks ACPI video driver is controlling
backlight so it will not create backlight interface for them.

Then we will need to remember not to add any of those systems into
the DMI table of video_detect.c, or thinkpad_acpi module will create
backlight interface for them and according to reporter, that interface
does not work either.

What about a priority based solution? We can introduce a new field named
priority to backlight_device and instead of calling another module's
function like the unregister one here(which cause unnecessary module
dependency), we only need to boost priority for its own interface. This
field will be exported to sysfs, so user can change it during runtime
too. And we can also introduce a new kernel command line as
backlight.force_interface=raw/firmware/platform, to overcome the limited
functionality provided by acpi_backlight=video/vendor, which does not
involve GPU's interface.

And we can place the quirk code in backlight layer instead of individual
backlight functionality provider module. Suppose we have a backlight
manager there, for all win8 systems, we can boost the raw type's
priority on its registration, so no need to add code in
intel/amd/etc./'s GPU driver code.

With priority based solution, all backlight control interfaces stay,
the priority field is an indication given by kernel to user space.

For a complete description on backlight control background and the
proposed solution, please take a look at:
https://gist.github.com/aaronlu/5779828

It would be good to know your opinions, thanks.

-Aaron
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-14 Thread Matthew Garrett 
On Fri, 2013-06-14 at 14:47 +0800, Aaron Lu wrote:

 What about a priority based solution? We can introduce a new field named
 priority to backlight_device and instead of calling another module's
 function like the unregister one here(which cause unnecessary module
 dependency), we only need to boost priority for its own interface. This
 field will be exported to sysfs, so user can change it during runtime
 too. And we can also introduce a new kernel command line as
 backlight.force_interface=raw/firmware/platform, to overcome the limited
 functionality provided by acpi_backlight=video/vendor, which does not
 involve GPU's interface.

How would that work with existing userspace?

 And we can place the quirk code in backlight layer instead of individual
 backlight functionality provider module. Suppose we have a backlight
 manager there, for all win8 systems, we can boost the raw type's
 priority on its registration, so no need to add code in
 intel/amd/etc./'s GPU driver code.

But we'd need to add code to every piece of userspace that currently
uses the backlight, right?

 With priority based solution, all backlight control interfaces stay,
 the priority field is an indication given by kernel to user space.

We shouldn't export interfaces if we don't expect them to work.

-- 
Matthew Garrett | mj...@srcf.ucam.org

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-14 Thread Aaron Lu 
On 06/15/2013 01:29 AM, Matthew Garrett wrote:
 On Fri, 2013-06-14 at 14:47 +0800, Aaron Lu wrote:
 
 What about a priority based solution? We can introduce a new field named
 priority to backlight_device and instead of calling another module's
 function like the unregister one here(which cause unnecessary module
 dependency), we only need to boost priority for its own interface. This
 field will be exported to sysfs, so user can change it during runtime
 too. And we can also introduce a new kernel command line as
 backlight.force_interface=raw/firmware/platform, to overcome the limited
 functionality provided by acpi_backlight=video/vendor, which does not
 involve GPU's interface.
 
 How would that work with existing userspace?

User space tool will need to be updated to use this as stated in the
gist page, I've patches for gsd-backlight-helper and xorg-x11-drv-intel,
for others we can add I think if the priority based solution is deemed
useful.

 
 And we can place the quirk code in backlight layer instead of individual
 backlight functionality provider module. Suppose we have a backlight
 manager there, for all win8 systems, we can boost the raw type's
 priority on its registration, so no need to add code in
 intel/amd/etc./'s GPU driver code.
 
 But we'd need to add code to every piece of userspace that currently
 uses the backlight, right?

Yes that's the case.

 
 With priority based solution, all backlight control interfaces stay,
 the priority field is an indication given by kernel to user space.
 
 We shouldn't export interfaces if we don't expect them to work.

It's not easy to decide if they work or not sometimes, e.g. I came
across a system that claims win8 in ACPI table and has an Intel GPU,
while its ACPI video interface also works. With this patch, the working
ACPI video interface is removed, while with the priority based solution,
the GPU's interface priority gets higher, but the ACPI video interface
still stays.

Thanks,
Aaron
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-14 Thread Matthew Garrett 
On Sat, 2013-06-15 at 09:26 +0800, Aaron Lu wrote:
 On 06/15/2013 01:29 AM, Matthew Garrett wrote: 
  How would that work with existing userspace?
 
 User space tool will need to be updated to use this as stated in the
 gist page, I've patches for gsd-backlight-helper and xorg-x11-drv-intel,
 for others we can add I think if the priority based solution is deemed
 useful.

Right, that's not a great solution.

  We shouldn't export interfaces if we don't expect them to work.
 
 It's not easy to decide if they work or not sometimes, e.g. I came
 across a system that claims win8 in ACPI table and has an Intel GPU,
 while its ACPI video interface also works. With this patch, the working
 ACPI video interface is removed, while with the priority based solution,
 the GPU's interface priority gets higher, but the ACPI video interface
 still stays.

Well, Windows 8 will only use the ACPI backlight interface if the GPU
driver decides to, right? So the logic for deciding whether to remove
the ACPI backlight control or not should be left up to the GPU. There's
no harm in refusing to expose a working method if there's another
working method, but there is harm in exposing a broken one and expecting
userspace to know the difference.

-- 
Matthew Garrett | mj...@srcf.ucam.org
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-14 Thread Aaron Lu 
On 06/15/2013 09:38 AM, Matthew Garrett wrote:
 On Sat, 2013-06-15 at 09:26 +0800, Aaron Lu wrote:
 It's not easy to decide if they work or not sometimes, e.g. I came
 across a system that claims win8 in ACPI table and has an Intel GPU,
 while its ACPI video interface also works. With this patch, the working
 ACPI video interface is removed, while with the priority based solution,
 the GPU's interface priority gets higher, but the ACPI video interface
 still stays.
 
 Well, Windows 8 will only use the ACPI backlight interface if the GPU
 driver decides to, right? So the logic for deciding whether to remove
 the ACPI backlight control or not should be left up to the GPU. There's

I don't know this. From the document I googled, Microsoft suggests under
win8, backlight should be controlled by the graphics driver for smooth
brightness level change, instead of ACPI or other methods. So it is
possible that OEM will not test the ACPI interface well and thus the
interface is likely broken. I don't see why GPU driver has any better
knowledge on which systems the firmware interface is broken or not.

 no harm in refusing to expose a working method if there's another
 working method, but there is harm in exposing a broken one and expecting
 userspace to know the difference.

BTW, the proposed solution is not meant to solve win8 problems alone, it
should make solving other problems easy and make individual backlight
control interface provider module independent with each other such as
the platform drivers will not need to check if ACPI video driver will
control backlight or not and can always create backlight interface(its
default priority is lower that ACPI video driver's so will not be taken
by user space by default, showing the same behavior of the current code).

The current acpi_backlight=video/vendor kernel command line is pretty
misleading, for laptops that do not have vendor backlight interface,
adding acpi_backlight=vendor actually makes the system using the GPU's
interface. Some laptops are using this switch to work around problems in
ACPI video driver and users think they are using vendor interface.
That's why I think we need a new command line as the
backlight.force_interface=raw/firmware/platform.

Instead of letting individual driver to make decisions on which
backlight interface this system should use(either in platform driver as
we currently did, see acer-wmi and asus-wmi, or GPU driver as this case),
I think we need a better and clear way to handle such things. For
example, suppose an acer laptop: vendor does not support backlight, ACPI
video's backlight interface is broken and GPU's works, to make it work,
user will need to select acer-wmi module while this module does not have
anything to do with the functionality, but simply because it serves as
the backlight manager for acer laptops.

The above information and idea is formed while solving bugs reported
in kernel bugzilla video component, the proposed solutin may not be good
enough, but I hope we can find a better way to handle backlight problems.

Thanks,
Aaron
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-14 Thread Matthew Garrett 
On Sat, Jun 15, 2013 at 12:14:42PM +0800, Aaron Lu wrote:
 On 06/15/2013 09:38 AM, Matthew Garrett wrote:
  Well, Windows 8 will only use the ACPI backlight interface if the GPU
  driver decides to, right? So the logic for deciding whether to remove
  the ACPI backlight control or not should be left up to the GPU. There's
 
 I don't know this. From the document I googled, Microsoft suggests under
 win8, backlight should be controlled by the graphics driver for smooth
 brightness level change, instead of ACPI or other methods. So it is
 possible that OEM will not test the ACPI interface well and thus the
 interface is likely broken. I don't see why GPU driver has any better
 knowledge on which systems the firmware interface is broken or not.

The vendor will presumably have tested that backlight control works - if 
the GPU driver uses the ACPI interface and backlight control is broken, 
then the vendor would fix it.

  no harm in refusing to expose a working method if there's another
  working method, but there is harm in exposing a broken one and expecting
  userspace to know the difference.
 
 BTW, the proposed solution is not meant to solve win8 problems alone, it
 should make solving other problems easy and make individual backlight
 control interface provider module independent with each other such as
 the platform drivers will not need to check if ACPI video driver will
 control backlight or not and can always create backlight interface(its
 default priority is lower that ACPI video driver's so will not be taken
 by user space by default, showing the same behavior of the current code).

Sure, but it still requires the replacement of existing userspace. We 
need to fix the problem with existing userspace, too.

 The current acpi_backlight=video/vendor kernel command line is pretty
 misleading, for laptops that do not have vendor backlight interface,
 adding acpi_backlight=vendor actually makes the system using the GPU's
 interface. Some laptops are using this switch to work around problems in
 ACPI video driver and users think they are using vendor interface.
 That's why I think we need a new command line as the
 backlight.force_interface=raw/firmware/platform.

No, I think we need to fix the bugs that currently require users to pass 
options.

 Instead of letting individual driver to make decisions on which
 backlight interface this system should use(either in platform driver as
 we currently did, see acer-wmi and asus-wmi, or GPU driver as this case),
 I think we need a better and clear way to handle such things. For
 example, suppose an acer laptop: vendor does not support backlight, ACPI
 video's backlight interface is broken and GPU's works, to make it work,
 user will need to select acer-wmi module while this module does not have
 anything to do with the functionality, but simply because it serves as
 the backlight manager for acer laptops.

How do these machines work under Windows? Until we know the answer to 
that, we don't know what the correct way to handle the problem is under 
Linux.

-- 
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-10 Thread Alex Deucher 
Radeon probably needs something similar.  See attached untested patch.

Alex

On Sun, Jun 9, 2013 at 7:01 PM, Matthew Garrett
 wrote:
> Windows 8 leaves backlight control up to individual graphics drivers rather
> than making ACPI calls itself. There's plenty of evidence to suggest that
> the Intel driver for Windows doesn't use the ACPI interface, including the
> fact that it's broken on a bunch of machines when the OS claims to support
> Windows 8. The simplest thing to do appears to be to disable the ACPI
> backlight interface on these systems.
>
> Signed-off-by: Matthew Garrett 
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 3b315ba..23b6292 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned 
> long flags)
> /* Must be done after probing outputs */
> intel_opregion_init(dev);
> acpi_video_register();
> +   /* Don't use ACPI backlight functions on Windows 8 platforms 
> */
> +   if (acpi_osi_version() >= ACPI_OSI_WIN_8)
> +   acpi_video_backlight_unregister();
> }
>
> if (IS_GEN5(dev))
> --
> 1.8.2.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
From c6ead3429fd3977b4b2ee5748d83c72272592b29 Mon Sep 17 00:00:00 2001
From: Alex Deucher 
Date: Mon, 10 Jun 2013 10:05:43 -0400
Subject: [PATCH] drm/radeon: don't provide ACPI backlight if firmware expects
 Windows 8

Windows 8 leaves backlight control up to individual graphics drivers rather
than making ACPI calls itself.

Untested.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/radeon/atombios_encoders.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index 4120d35..bc9e007 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -233,6 +233,10 @@ void radeon_atom_backlight_init(struct radeon_encoder *radeon_encoder,
 
 	DRM_INFO("radeon atom DIG backlight initialized\n");
 
+	/* Don't use ACPI backlight functions on Windows 8 platforms */
+	if (acpi_osi_version() >= ACPI_OSI_WIN_8)
+		acpi_video_backlight_unregister();
+
 	return;
 
 error:
-- 
1.7.7.5

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-10 Thread joeyli 
於 日，2013-06-09 於 19:01 -0400，Matthew Garrett 提到：
> Windows 8 leaves backlight control up to individual graphics drivers rather
> than making ACPI calls itself. There's plenty of evidence to suggest that
> the Intel driver for Windows doesn't use the ACPI interface, including the
> fact that it's broken on a bunch of machines when the OS claims to support
> Windows 8. The simplest thing to do appears to be to disable the ACPI
> backlight interface on these systems.
> 
> Signed-off-by: Matthew Garrett 
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 3b315ba..23b6292 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned 
> long flags)
>   /* Must be done after probing outputs */
>   intel_opregion_init(dev);
>   acpi_video_register();
> + /* Don't use ACPI backlight functions on Windows 8 platforms */
> + if (acpi_osi_version() >= ACPI_OSI_WIN_8)
> + acpi_video_backlight_unregister();
>   }
>  
>   if (IS_GEN5(dev))

This patch set works to me on Acer Aspire V3 notebook for unregister the
backlight interface of acpi video driver when i915 loaded. Acer Aspire
V3 has the Windows8 support in DSDT.

Tested-by: Lee, Chun-Yi 


Thanks a lot!
Joey Lee

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-10 Thread Daniel Vetter 
On Sun, Jun 09, 2013 at 07:01:39PM -0400, Matthew Garrett wrote:
> Windows 8 leaves backlight control up to individual graphics drivers rather
> than making ACPI calls itself. There's plenty of evidence to suggest that
> the Intel driver for Windows doesn't use the ACPI interface, including the
> fact that it's broken on a bunch of machines when the OS claims to support
> Windows 8. The simplest thing to do appears to be to disable the ACPI
> backlight interface on these systems.
> 
> Signed-off-by: Matthew Garrett 

Make sense and I guess it's easier to merge this all through the acpi
tree. So

Acked-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/i915/i915_dma.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 3b315ba..23b6292 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned 
> long flags)
>   /* Must be done after probing outputs */
>   intel_opregion_init(dev);
>   acpi_video_register();
> + /* Don't use ACPI backlight functions on Windows 8 platforms */
> + if (acpi_osi_version() >= ACPI_OSI_WIN_8)
> + acpi_video_backlight_unregister();
>   }
>  
>   if (IS_GEN5(dev))
> -- 
> 1.8.2.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-10 Thread Daniel Vetter 
On Sun, Jun 09, 2013 at 07:01:39PM -0400, Matthew Garrett wrote:
 Windows 8 leaves backlight control up to individual graphics drivers rather
 than making ACPI calls itself. There's plenty of evidence to suggest that
 the Intel driver for Windows doesn't use the ACPI interface, including the
 fact that it's broken on a bunch of machines when the OS claims to support
 Windows 8. The simplest thing to do appears to be to disable the ACPI
 backlight interface on these systems.
 
 Signed-off-by: Matthew Garrett matthew.garr...@nebula.com

Make sense and I guess it's easier to merge this all through the acpi
tree. So

Acked-by: Daniel Vetter daniel.vet...@ffwll.ch

 ---
  drivers/gpu/drm/i915/i915_dma.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
 index 3b315ba..23b6292 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned 
 long flags)
   /* Must be done after probing outputs */
   intel_opregion_init(dev);
   acpi_video_register();
 + /* Don't use ACPI backlight functions on Windows 8 platforms */
 + if (acpi_osi_version() = ACPI_OSI_WIN_8)
 + acpi_video_backlight_unregister();
   }
  
   if (IS_GEN5(dev))
 -- 
 1.8.2.1
 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-10 Thread joeyli 
於 日，2013-06-09 於 19:01 -0400，Matthew Garrett 提到：
 Windows 8 leaves backlight control up to individual graphics drivers rather
 than making ACPI calls itself. There's plenty of evidence to suggest that
 the Intel driver for Windows doesn't use the ACPI interface, including the
 fact that it's broken on a bunch of machines when the OS claims to support
 Windows 8. The simplest thing to do appears to be to disable the ACPI
 backlight interface on these systems.
 
 Signed-off-by: Matthew Garrett matthew.garr...@nebula.com
 ---
  drivers/gpu/drm/i915/i915_dma.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
 index 3b315ba..23b6292 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned 
 long flags)
   /* Must be done after probing outputs */
   intel_opregion_init(dev);
   acpi_video_register();
 + /* Don't use ACPI backlight functions on Windows 8 platforms */
 + if (acpi_osi_version() = ACPI_OSI_WIN_8)
 + acpi_video_backlight_unregister();
   }
  
   if (IS_GEN5(dev))

This patch set works to me on Acer Aspire V3 notebook for unregister the
backlight interface of acpi video driver when i915 loaded. Acer Aspire
V3 has the Windows8 support in DSDT.

Tested-by: Lee, Chun-Yi j...@suse.com


Thanks a lot!
Joey Lee

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

2013-06-10 Thread Alex Deucher 
Radeon probably needs something similar.  See attached untested patch.

Alex

On Sun, Jun 9, 2013 at 7:01 PM, Matthew Garrett
matthew.garr...@nebula.com wrote:
 Windows 8 leaves backlight control up to individual graphics drivers rather
 than making ACPI calls itself. There's plenty of evidence to suggest that
 the Intel driver for Windows doesn't use the ACPI interface, including the
 fact that it's broken on a bunch of machines when the OS claims to support
 Windows 8. The simplest thing to do appears to be to disable the ACPI
 backlight interface on these systems.

 Signed-off-by: Matthew Garrett matthew.garr...@nebula.com
 ---
  drivers/gpu/drm/i915/i915_dma.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
 index 3b315ba..23b6292 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned 
 long flags)
 /* Must be done after probing outputs */
 intel_opregion_init(dev);
 acpi_video_register();
 +   /* Don't use ACPI backlight functions on Windows 8 platforms 
 */
 +   if (acpi_osi_version() = ACPI_OSI_WIN_8)
 +   acpi_video_backlight_unregister();
 }

 if (IS_GEN5(dev))
 --
 1.8.2.1

 ___
 dri-devel mailing list
 dri-de...@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
From c6ead3429fd3977b4b2ee5748d83c72272592b29 Mon Sep 17 00:00:00 2001
From: Alex Deucher alexander.deuc...@amd.com
Date: Mon, 10 Jun 2013 10:05:43 -0400
Subject: [PATCH] drm/radeon: don't provide ACPI backlight if firmware expects
 Windows 8

Windows 8 leaves backlight control up to individual graphics drivers rather
than making ACPI calls itself.

Untested.

Signed-off-by: Alex Deucher alexander.deuc...@amd.com
---
 drivers/gpu/drm/radeon/atombios_encoders.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c b/drivers/gpu/drm/radeon/atombios_encoders.c
index 4120d35..bc9e007 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -233,6 +233,10 @@ void radeon_atom_backlight_init(struct radeon_encoder *radeon_encoder,
 
 	DRM_INFO(radeon atom DIG backlight initialized\n);
 
+	/* Don't use ACPI backlight functions on Windows 8 platforms */
+	if (acpi_osi_version() = ACPI_OSI_WIN_8)
+		acpi_video_backlight_unregister();
+
 	return;
 
 error:
-- 
1.7.7.5