From mboxrd@z Thu Jan 1 00:00:00 1970 From: Igor Gnatenko Subject: Re: [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8 Date: Mon, 15 Jul 2013 17:06:09 +0400 Message-ID: <1373893569.2002.3.camel@ThinkPad-X230.localdomain> References: <1370818899-8595-1-git-send-email-matthew.garrett@nebula.com> <12270512.1rshgKaUug@vostro.rjw.lan> <51DA7190.6080208@gmail.com> <1683364.CdnoW81WLH@vostro.rjw.lan> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mail-la0-f50.google.com ([209.85.215.50]:50347 "EHLO mail-la0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756487Ab3GONGP (ORCPT ); Mon, 15 Jul 2013 09:06:15 -0400 In-Reply-To: <1683364.CdnoW81WLH@vostro.rjw.lan> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: "Rafael J. Wysocki" Cc: Aaron Lu , linux-acpi@vger.kernel.org, Matthew Garrett , daniel.vetter@ffwll.ch, seth.forshee@canonical.com, joeyli.kernel@gmail.com, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, lenb@kernel.org On Sat, 2013-07-13 at 02:46 +0200, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki >=20 > 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". >=20 > 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 not prevented from > doing so by the ACPI subsystem. > Therefore we need to allow the ACPI backlight interface to be > registered until the i915 driver is loaded which then will unregister > it if the firmware has called _OSI for Windows 8 (or will register > the ACPI video driver without backlight support if not already > present). >=20 > 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(). >=20 > This change is based on earlier patches from Matthew Garrett, > Chun-Yi Lee and Seth Forshee and Aaron Lu's comments. >=20 > Signed-off-by: Rafael J. Wysocki > --- > drivers/acpi/internal.h | 11 ++++++ > drivers/acpi/video.c | 65 +++++++++++++++++++++++++++++= +++++++---- > drivers/acpi/video_detect.c | 21 ++++++++++++ > drivers/gpu/drm/i915/i915_dma.c | 2 - > include/acpi/video.h | 11 ++++++ > include/linux/acpi.h | 1=20 > 6 files changed, 103 insertions(+), 8 deletions(-) >=20 > Index: linux-pm/drivers/acpi/video.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- linux-pm.orig/drivers/acpi/video.c > +++ linux-pm/drivers/acpi/video.c > @@ -44,6 +44,8 @@ > #include > #include > =20 > +#include "internal.h" > + > #define PREFIX "ACPI: " > =20 > #define ACPI_VIDEO_BUS_NAME "Video Bus" > @@ -898,7 +900,7 @@ static void acpi_video_device_find_cap(s > device->cap._DDC =3D 1; > } > =20 > - if (acpi_video_backlight_support()) { > + if (acpi_video_verify_backlight_support()) { > struct backlight_properties props; > struct pci_dev *pdev; > acpi_handle acpi_parent; > @@ -1854,6 +1856,46 @@ static int acpi_video_bus_remove(struct > return 0; > } > =20 > +static acpi_status video_unregister_backlight(acpi_handle handle, u3= 2 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 =3D 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, entr= y) { > + if (dev->backlight) { > + backlight_device_unregister(dev->backlight); > + dev->backlight =3D 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 =3D 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 =3D=3D 0x00D1) > @@ -1885,14 +1927,25 @@ static int __init intel_opregion_present > return opregion; > } > =20 > -int acpi_video_register(void) > +int __acpi_video_register(bool backlight_quirks) > { > - int result =3D 0; > + bool no_backlight; > + int result; > + > + no_backlight =3D backlight_quirks ? acpi_video_backlight_quirks() := false; > + > if (register_count) { > /* > - * if the function of acpi_video_register is already called, > - * don't register the acpi_vide_bus again and return no error. > + * If acpi_video_register() has been called already, don't try > + * to register acpi_video_bus, but unregister backlight devices > + * if no backlight support is requested. > */ > + if (no_backlight) > + acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, > + ACPI_UINT32_MAX, > + video_unregister_backlight, > + NULL, NULL, NULL); > + > return 0; > } > =20 > @@ -1908,7 +1961,7 @@ int acpi_video_register(void) > =20 > return 0; > } > -EXPORT_SYMBOL(acpi_video_register); > +EXPORT_SYMBOL(__acpi_video_register); > =20 > void acpi_video_unregister(void) > { > Index: linux-pm/drivers/gpu/drm/i915/i915_dma.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- linux-pm.orig/drivers/gpu/drm/i915/i915_dma.c > +++ linux-pm/drivers/gpu/drm/i915/i915_dma.c > @@ -1660,7 +1660,7 @@ int i915_driver_load(struct drm_device * > if (INTEL_INFO(dev)->num_pipes) { > /* Must be done after probing outputs */ > intel_opregion_init(dev); > - acpi_video_register(); > + acpi_video_register_with_quirks(); > } > =20 > if (IS_GEN5(dev)) > Index: linux-pm/include/acpi/video.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- linux-pm.orig/include/acpi/video.h > +++ linux-pm/include/acpi/video.h > @@ -17,12 +17,21 @@ struct acpi_device; > #define ACPI_VIDEO_DISPLAY_LEGACY_TV 0x0200 > =20 > #if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE) > -extern int acpi_video_register(void); > +extern int __acpi_video_register(bool backlight_quirks); > +static inline int acpi_video_register(void) > +{ > + return __acpi_video_register(false); > +} > +static inline int acpi_video_register_with_quirks(void) > +{ > + return __acpi_video_register(true); > +} > extern void acpi_video_unregister(void); > extern int acpi_video_get_edid(struct acpi_device *device, int type, > int device_id, void **edid); > #else > static inline int acpi_video_register(void) { return 0; } > +static inline int acpi_video_register_with_quirks(void) { return 0; = } > static inline void acpi_video_unregister(void) { return; } > static inline int acpi_video_get_edid(struct acpi_device *device, in= t type, > int device_id, void **edid) > Index: linux-pm/drivers/acpi/video_detect.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- linux-pm.orig/drivers/acpi/video_detect.c > +++ linux-pm/drivers/acpi/video_detect.c > @@ -38,6 +38,8 @@ > #include > #include > =20 > +#include "internal.h" > + > #define PREFIX "ACPI: " > =20 > ACPI_MODULE_NAME("video"); > @@ -234,6 +236,17 @@ static void acpi_video_caps_check(void) > acpi_video_get_capabilities(NULL); > } > =20 > +bool acpi_video_backlight_quirks(void) > +{ > + if (acpi_gbl_osi_data >=3D ACPI_OSI_WIN_8) { > + acpi_video_caps_check(); > + acpi_video_support |=3D ACPI_VIDEO_SKIP_BACKLIGHT; > + return true; > + } > + return false; > +} > +EXPORT_SYMBOL(acpi_video_backlight_quirks); > + > /* Promote the vendor interface instead of the generic video module. > * This function allow DMI blacklists to be implemented by externals > * platform drivers instead of putting a big blacklist in video_dete= ct.c > @@ -278,6 +291,14 @@ int acpi_video_backlight_support(void) > } > EXPORT_SYMBOL(acpi_video_backlight_support); > =20 > +/* For the ACPI video driver's use only. */ > +bool acpi_video_verify_backlight_support(void) > +{ > + return (acpi_video_support & ACPI_VIDEO_SKIP_BACKLIGHT) ? > + false : acpi_video_backlight_support(); > +} > +EXPORT_SYMBOL(acpi_video_verify_backlight_support); > + > /* > * Use acpi_backlight=3Dvendor/video to force that backlight switchi= ng > * is processed by vendor specific acpi drivers or video.ko driver. > Index: linux-pm/include/linux/acpi.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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_SKIP_BACKLIGHT 0x1000 > =20 > #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE) > =20 > Index: linux-pm/drivers/acpi/internal.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- linux-pm.orig/drivers/acpi/internal.h > +++ linux-pm/drivers/acpi/internal.h > @@ -164,4 +164,15 @@ struct platform_device; > int acpi_create_platform_device(struct acpi_device *adev, > const struct acpi_device_id *id); > =20 > +/*------------------------------------------------------------------= -------- > + Video > + ------------------------------------------------------------------= -------- */ > +#ifdef CONFIG_ACPI_VIDEO > +bool acpi_video_backlight_quirks(void); > +bool acpi_video_verify_backlight_support(void); > +#else > +static inline bool acpi_video_backlight_quirks(void) { return false;= } > +static inline bool acpi_video_verify_backlight_support(void) { retur= n false; } > +#endif > + > #endif /* _ACPI_INTERNAL_H_ */ I can't build it. Where did I go wrong? drivers/acpi/video_detect.c:239:6: error: redefinition of 'acpi_video_backlight_quirks' bool acpi_video_backlight_quirks(void) ^ In file included from drivers/acpi/video_detect.c:41:0: drivers/acpi/internal.h:174:60: note: previous definition of 'acpi_video_backlight_quirks' was here static inline bool acpi_video_backlight_quirks(void) { return false; } ^ drivers/acpi/video_detect.c: In function 'acpi_video_backlight_quirks': drivers/acpi/video_detect.c:241:6: error: 'acpi_gbl_osi_data' undeclare= d (first use in this function) if (acpi_gbl_osi_data >=3D ACPI_OSI_WIN_8) { ^ drivers/acpi/video_detect.c:241:6: note: each undeclared identifier is reported only once for each function it appears in drivers/acpi/video_detect.c:241:27: error: 'ACPI_OSI_WIN_8' undeclared (first use in this function) if (acpi_gbl_osi_data >=3D ACPI_OSI_WIN_8) { ^ drivers/acpi/video_detect.c: At top level: drivers/acpi/video_detect.c:295:6: error: redefinition of 'acpi_video_verify_backlight_support' bool acpi_video_verify_backlight_support(void) ^ In file included from drivers/acpi/video_detect.c:41:0: drivers/acpi/internal.h:175:60: note: previous definition of 'acpi_video_verify_backlight_support' was here static inline bool acpi_video_verify_backlight_support(void) { return false; } ^ --=20 Igor Gnatenko =46edora release 19 (Schr=C3=B6dinger=E2=80=99s Cat) Linux 3.9.9-302.fc19.x86_64 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" i= n the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html