Skip to contentSkip to conversation

acer-wmi: add Acer Aspire 5750G to video vendor list but keep acpi video driver

PATCH Superseded Chun-Yi Lee 5 participants 9 messages Apr 22, 2013 – Apr 29, 2013

Subsystems (4)
ACPI x86 platform drivers ACPI component architecture (acpica) Acer WMI laptop extras

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Chun-Yi Lee

author
#0 lore raw

PATCH 2/2 acer-wmi: add Acer Aspire 5750G to video vendor list but keep acpi video driver

From: Lee, Chun-Yi <[email protected]>

After Andrzej's testing, we found the acpi backlight methods broken on Acer Aspire 5750G but the i915 backlight control works when we set to vendor mode. And, we still want to keep the acpi/video driver for transfer acpi event to key event but not unregister whole acpi/video driver.

This patch introduced a new capability flag is ACER_CAP_KEEP_VIDEO_KEY, it indicates the machine works fine with acpi/video driver for key event but want to unregister the backlight interface of acpi/video.

Reference: bko#35622

        https://bugzilla.kernel.org/show_bug.cgi?id=35622
Tested-by Andrzej Krentosz <[email protected]>
Cc Carlos Corbacho <[email protected]>
Cc Matthew Garrett <[email protected]>
Cc Dmitry Torokhov <[email protected]>
Cc Corentin Chary <[email protected]>
Cc Rafael J. Wysocki <[email protected]>
Cc Aaron Lu <[email protected]>
Cc Thomas Renninger <[email protected]>
Signed-off-by Lee, Chun-Yi <[email protected]>
---
 drivers/platform/x86/acer-wmi.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
drivers/platform/x86/acer-wmi.c +20 -0
index c9076bd..2a02409 100644
@@ -207,6 +207,7 @@ struct hotkey_function_type_aa {
 #define ACER_CAP_BRIGHTNESS		(1<<3)
 #define ACER_CAP_THREEG			(1<<4)
 #define ACER_CAP_ACCEL			(1<<5)
+#define ACER_CAP_KEEP_VIDEO_KEY		(1<<6)
 #define ACER_CAP_ANY			(0xFFFFFFFF)
 
 /*

@@ -539,6 +540,15 @@ static int video_set_backlight_video_vendor(const struct dmi_system_id *d)
 	return 0;
 }
 
+static int video_set_backlight_video_vendor_keep_acpi_video(
+		const struct dmi_system_id *d)
+{
+	video_set_backlight_video_vendor(d);
+	interface->capability |= ACER_CAP_KEEP_VIDEO_KEY;
+	pr_info("Keep acpi video driver for emit keycode against backlight change\n");
+	return 0;
+}
+
 static const struct dmi_system_id video_vendor_dmi_table[] = {
 	{
 		.callback = video_set_backlight_video_vendor,

@@ -572,6 +582,14 @@ static const struct dmi_system_id video_vendor_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 5750"),
 		},
 	},
+	{
+		.callback = video_set_backlight_video_vendor_keep_acpi_video,
+		.ident = "Acer Aspire 5750G",
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "Acer"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 5750G"),
+		},
+	},
 	{}
 };
 

@@ -2228,6 +2246,8 @@ static int __init acer_wmi_init(void)
 	if (acpi_video_backlight_support()) {
 		interface->capability &= ~ACER_CAP_BRIGHTNESS;
 		pr_info("Brightness must be controlled by acpi video driver\n");
+	} else if (interface->capability & ACER_CAP_KEEP_VIDEO_KEY) {
+		acpi_video_backlight_unregister();
 	} else {
 		pr_info("Disabling ACPI video driver\n");
 		acpi_video_unregister();
Signature · 1 line
1.6.0.2
author
#1 lore raw

PATCH 1/2 acpi: video: add function to support unregister backlight

From: "Lee, Chun-Yi" <[email protected]>

There have situation we unregister whole acpi/video driver by downstream driver just want to remove backlight control interface of acpi/video. It caues we lost other functions of acpi/video, e.g. transfer acpi event to input event.

So, this patch add a new function, find_video_unregister_backlight, it provide the interface let downstream driver can tell acpi/video to unregister backlight interface of all acpi video devices. Then we can keep functions of acpi/video but only remove backlight support.

Reference: bko#35622

        https://bugzilla.kernel.org/show_bug.cgi?id=35622
Tested-by Andrzej Krentosz <[email protected]>
Cc Carlos Corbacho <[email protected]>
Cc Matthew Garrett <[email protected]>
Cc Dmitry Torokhov <[email protected]>
Cc Corentin Chary <[email protected]>
Cc Rafael J. Wysocki <[email protected]>
Cc Aaron Lu <[email protected]>
Cc Thomas Renninger <[email protected]>
Signed-off-by Lee, Chun-Yi <[email protected]>
---
 drivers/acpi/video.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 include/acpi/video.h |  2 ++
 2 files changed, 48 insertions(+)
drivers/acpi/video.c +46 -0
index 313f959..acd2e7a 100644
@@ -1793,6 +1793,52 @@ static int __init intel_opregion_present(void)
 	return opregion;
 }
 
+static acpi_status
+find_video_unregister_backlight(acpi_handle handle, u32 lvl, void *context,
+				void **rv)
+{
+	struct acpi_device *acpi_dev;
+	struct acpi_video_bus *video = NULL;
+	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)) {
+		video = acpi_driver_data(acpi_dev);
+		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);
+			}
+		}
+		mutex_unlock(&video->device_list_lock);
+		acpi_video_bus_start_devices(video);
+	}
+	return AE_OK;
+}
+
+void acpi_video_backlight_unregister(void)
+{
+	if (!register_count) {
+		/*
+		 * If the acpi video bus is already unloaded, don't
+		 * unregister backlight of devices and return directly.
+		 */
+		return;
+	}
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+			    ACPI_UINT32_MAX, find_video_unregister_backlight,
+			    NULL, NULL, NULL);
+	return;
+}
+EXPORT_SYMBOL(acpi_video_backlight_unregister);
+
 int acpi_video_register(void)
 {
 	int result = 0;
include/acpi/video.h +2 -0
index 61109f2..1e810a1 100644
@@ -19,11 +19,13 @@ struct acpi_device;
 #if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
 extern int acpi_video_register(void);
 extern void acpi_video_unregister(void);
+extern void acpi_video_backlight_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 void acpi_video_unregister(void) { return; }
+static inline void acpi_video_backlight_unregister(void) { return; }
 static inline int acpi_video_get_edid(struct acpi_device *device, int type,
 				      int device_id, void **edid)
 {
Signature · 1 line
1.8.1.4
#2 lore raw

Re: [PATCH 1/2] acpi: video: add function to support unregister backlight

Quoted 6 lines · Chun-Yi Lee

From: "Lee, Chun-Yi" <[email protected]>

There have situation we unregister whole acpi/video driver by downstream driver just want to remove backlight control interface of acpi/video. It caues we lost other functions of acpi/video, e.g. transfer acpi event to input event.

Quoted 7 lines

So, this patch add a new function, find_video_unregister_backlight, it provide the interface let downstream driver can tell acpi/video to unregister backlight interface of all acpi video devices. Then we can keep functions of acpi/video but only remove backlight support.

Reference: bko#35622

        https://bugzilla.kernel.org/show_bug.cgi?id=35622

Rui, any comments?

Rafael

Quoted 90 lines
Tested-by Andrzej Krentosz <[email protected]>
Cc Carlos Corbacho <[email protected]>
Cc Matthew Garrett <[email protected]>
Cc Dmitry Torokhov <[email protected]>
Cc Corentin Chary <[email protected]>
Cc Rafael J. Wysocki <[email protected]>
Cc Aaron Lu <[email protected]>
Cc Thomas Renninger <[email protected]>
Signed-off-by Lee, Chun-Yi <[email protected]>
---
 drivers/acpi/video.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 include/acpi/video.h |  2 ++
 2 files changed, 48 insertions(+)
drivers/acpi/video.c +46 -0
index 313f959..acd2e7a 100644
@@ -1793,6 +1793,52 @@ static int __init intel_opregion_present(void)
 	return opregion;
 }
 
+static acpi_status
+find_video_unregister_backlight(acpi_handle handle, u32 lvl, void *context,
+				void **rv)
+{
+	struct acpi_device *acpi_dev;
+	struct acpi_video_bus *video = NULL;
+	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)) {
+		video = acpi_driver_data(acpi_dev);
+		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);
+			}
+		}
+		mutex_unlock(&video->device_list_lock);
+		acpi_video_bus_start_devices(video);
+	}
+	return AE_OK;
+}
+
+void acpi_video_backlight_unregister(void)
+{
+	if (!register_count) {
+		/*
+		 * If the acpi video bus is already unloaded, don't
+		 * unregister backlight of devices and return directly.
+		 */
+		return;
+	}
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+			    ACPI_UINT32_MAX, find_video_unregister_backlight,
+			    NULL, NULL, NULL);
+	return;
+}
+EXPORT_SYMBOL(acpi_video_backlight_unregister);
+
 int acpi_video_register(void)
 {
 	int result = 0;
include/acpi/video.h +2 -0
index 61109f2..1e810a1 100644
@@ -19,11 +19,13 @@ struct acpi_device;
 #if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
 extern int acpi_video_register(void);
 extern void acpi_video_unregister(void);
+extern void acpi_video_backlight_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 void acpi_video_unregister(void) { return; }
+static inline void acpi_video_backlight_unregister(void) { return; }
 static inline int acpi_video_get_edid(struct acpi_device *device, int type,
 				      int device_id, void **edid)
 {
Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
#3 lore raw

Re: [PATCH 1/2] acpi: video: add function to support unregister backlight

In reply to Chun-Yi Lee

drivers/acpi/video.c static int __init intel_opregion_present(void) · line 1801

Quoted 7 lines · Chun-Yi Lee

From: "Lee, Chun-Yi" <[email protected]>

drivers/acpi/video.c +6 -0
+static acpi_status
+find_video_unregister_backlight(acpi_handle handle, u32 lvl, void *context,
+				void **rv)
+{
+	struct acpi_device *acpi_dev;
+	struct acpi_video_bus *video = NULL;

Gratuitous initialization of local variables prevents compiler from warning when you using variable uninitialized.

Quoted 17 lines
drivers/acpi/video.c +17 -0
+	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)) {
+		video = acpi_driver_data(acpi_dev);
+		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);
+			}

drivers/acpi/video.c static int __init intel_opregion_present(void) · line 1828

Quoted 10 lines
drivers/acpi/video.c +10 -0
+		}
+		mutex_unlock(&video->device_list_lock);
+		acpi_video_bus_start_devices(video);
+	}
+	return AE_OK;
+}
+
+void acpi_video_backlight_unregister(void)
+{
+	if (!register_count) {

Locking? It looks like the rest of the driver ignores locking too...

Quoted 38 lines
drivers/acpi/video.c +13 -0
+		/*
+		 * If the acpi video bus is already unloaded, don't
+		 * unregister backlight of devices and return directly.
+		 */
+		return;
+	}
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+			    ACPI_UINT32_MAX, find_video_unregister_backlight,
+			    NULL, NULL, NULL);
+	return;
+}
+EXPORT_SYMBOL(acpi_video_backlight_unregister);
+
 int acpi_video_register(void)
 {
 	int result = 0;
include/acpi/video.h +2 -0
index 61109f2..1e810a1 100644
@@ -19,11 +19,13 @@ struct acpi_device;
 #if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
 extern int acpi_video_register(void);
 extern void acpi_video_unregister(void);
+extern void acpi_video_backlight_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 void acpi_video_unregister(void) { return; }
+static inline void acpi_video_backlight_unregister(void) { return; }
 static inline int acpi_video_get_edid(struct acpi_device *device, int type,
 				      int device_id, void **edid)
 {
Signature · 1 line
1.8.1.4

Thanks.

Signature · 1 line
Dmitry
maintainer
#4 lore raw

Re: [PATCH 1/2] acpi: video: add function to support unregister backlight

Hi Dmitry,

Thanks for your review and suggestions first!

於 一，2013-04-22 於 21:12 -0700，Dmitry Torokhov 提到：

Quoted 11 lines · Chun-Yi Lee

drivers/acpi/video.c static int __init intel_opregion_present(void) · line 1801

Quoted 7 lines

From: "Lee, Chun-Yi" <[email protected]>

drivers/acpi/video.c +6 -0
+static acpi_status
+find_video_unregister_backlight(acpi_handle handle, u32 lvl, void *context,
+				void **rv)
+{
+	struct acpi_device *acpi_dev;
+	struct acpi_video_bus *video = NULL;

Gratuitous initialization of local variables prevents compiler from warning when you using variable uninitialized.

Yes, I agreed should not put gratuitous initialization, I will remove it in v2 patch.

Quoted 22 lines
Quoted 22 lines
drivers/acpi/video.c +21 -0
+	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)) {
+		video = acpi_driver_data(acpi_dev);
+		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);
+			}
+		}
+		mutex_unlock(&video->device_list_lock);
+		acpi_video_bus_start_devices(video);
+	}
Quoted 8 lines

drivers/acpi/video.c static int __init intel_opregion_present(void) · line 1828

Quoted 6 lines
drivers/acpi/video.c +6 -0
+	return AE_OK;
+}
+
+void acpi_video_backlight_unregister(void)
+{
+	if (!register_count) {

Locking? It looks like the rest of the driver ignores locking too...

Did you mean locking video_device_list?

The acpi/video locks video_device_list when add, remove and notify acpi video bus driver. It always do the mutex lock before control video_device_list, so I also add lock when unregister all backlight of devices.

Thanks a lot!
Joey Lee

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw

Re: [PATCH 1/2] acpi: video: add function to support unregister backlight

In reply to Chun-Yi Lee
Quoted 49 lines · Chun-Yi Lee

From: "Lee, Chun-Yi" <[email protected]>

There have situation we unregister whole acpi/video driver by downstream driver just want to remove backlight control interface of acpi/video. It caues we lost other functions of acpi/video, e.g. transfer acpi event to input event.

So, this patch add a new function, find_video_unregister_backlight, it provide the interface let downstream driver can tell acpi/video to unregister backlight interface of all acpi video devices. Then we can keep functions of acpi/video but only remove backlight support.

Reference: bko#35622

         https://bugzilla.kernel.org/show_bug.cgi?id=35622
Tested-by Andrzej Krentosz <[email protected]>
Cc Carlos Corbacho <[email protected]>
Cc Matthew Garrett <[email protected]>
Cc Dmitry Torokhov <[email protected]>
Cc Corentin Chary <[email protected]>
Cc Rafael J. Wysocki <[email protected]>
Cc Aaron Lu <[email protected]>
Cc Thomas Renninger <[email protected]>
Signed-off-by Lee, Chun-Yi <[email protected]>
---
  drivers/acpi/video.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
  include/acpi/video.h |  2 ++
  2 files changed, 48 insertions(+)
drivers/acpi/video.c +13 -0
index 313f959..acd2e7a 100644
@@ -1793,6 +1793,52 @@ static int __init intel_opregion_present(void)
  	return opregion;
  }
 
+static acpi_status
+find_video_unregister_backlight(acpi_handle handle, u32 lvl, void *context,
+				void **rv)
+{
+	struct acpi_device *acpi_dev;
+	struct acpi_video_bus *video = NULL;
+	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)) {
+		video = acpi_driver_data(acpi_dev);

drivers/acpi/video.c static int __init intel_opregion_present(void) · line 1818

Quoted 10 lines
drivers/acpi/video.c +10 -0
+		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);
+			}

The cooling_dev should also be unregistered I think.

Thanks,
Aaron

Quoted 45 lines
drivers/acpi/video.c +23 -0
+		}
+		mutex_unlock(&video->device_list_lock);
+		acpi_video_bus_start_devices(video);
+	}
+	return AE_OK;
+}
+
+void acpi_video_backlight_unregister(void)
+{
+	if (!register_count) {
+		/*
+		 * If the acpi video bus is already unloaded, don't
+		 * unregister backlight of devices and return directly.
+		 */
+		return;
+	}
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+			    ACPI_UINT32_MAX, find_video_unregister_backlight,
+			    NULL, NULL, NULL);
+	return;
+}
+EXPORT_SYMBOL(acpi_video_backlight_unregister);
+
  int acpi_video_register(void)
  {
  	int result = 0;
include/acpi/video.h +2 -0
index 61109f2..1e810a1 100644
@@ -19,11 +19,13 @@ struct acpi_device;
  #if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
  extern int acpi_video_register(void);
  extern void acpi_video_unregister(void);
+extern void acpi_video_backlight_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 void acpi_video_unregister(void) { return; }
+static inline void acpi_video_backlight_unregister(void) { return; }
  static inline int acpi_video_get_edid(struct acpi_device *device, int type,
  				      int device_id, void **edid)
  {
maintainer
#6 lore raw

Re: [PATCH 1/2] acpi: video: add function to support unregister backlight

於 五，2013-04-26 於 13:24 +0800，Aaron Lu 提到：

Quoted 63 lines · Chun-Yi Lee
Quoted 49 lines

From: "Lee, Chun-Yi" <[email protected]>

There have situation we unregister whole acpi/video driver by downstream driver just want to remove backlight control interface of acpi/video. It caues we lost other functions of acpi/video, e.g. transfer acpi event to input event.

So, this patch add a new function, find_video_unregister_backlight, it provide the interface let downstream driver can tell acpi/video to unregister backlight interface of all acpi video devices. Then we can keep functions of acpi/video but only remove backlight support.

Reference: bko#35622

         https://bugzilla.kernel.org/show_bug.cgi?id=35622
Tested-by Andrzej Krentosz <[email protected]>
Cc Carlos Corbacho <[email protected]>
Cc Matthew Garrett <[email protected]>
Cc Dmitry Torokhov <[email protected]>
Cc Corentin Chary <[email protected]>
Cc Rafael J. Wysocki <[email protected]>
Cc Aaron Lu <[email protected]>
Cc Thomas Renninger <[email protected]>
Signed-off-by Lee, Chun-Yi <[email protected]>
---
  drivers/acpi/video.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
  include/acpi/video.h |  2 ++
  2 files changed, 48 insertions(+)
drivers/acpi/video.c +13 -0
index 313f959..acd2e7a 100644
@@ -1793,6 +1793,52 @@ static int __init intel_opregion_present(void)
  	return opregion;
  }
 
+static acpi_status
+find_video_unregister_backlight(acpi_handle handle, u32 lvl, void *context,
+				void **rv)
+{
+	struct acpi_device *acpi_dev;
+	struct acpi_video_bus *video = NULL;
+	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)) {
+		video = acpi_driver_data(acpi_dev);

drivers/acpi/video.c static int __init intel_opregion_present(void) · line 1818

Quoted 10 lines
drivers/acpi/video.c +10 -0
+		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);
+			}

The cooling_dev should also be unregistered I think.

Quoted 3 lines

Thanks,
Aaron

Yes, you are right!

Unregistering backlight interface of video devices means the backlight control functions broken, so it can not used for throttling.

I will unregister cooling_dev in v2 patch.

Thanks a lot!
Joey Lee

#7 lore raw
In reply to Chun-Yi Lee
Quoted 40 lines · Chun-Yi Lee

From: Lee, Chun-Yi <[email protected]>

After Andrzej's testing, we found the acpi backlight methods broken on Acer Aspire 5750G but the i915 backlight control works when we set to vendor mode. And, we still want to keep the acpi/video driver for transfer acpi event to key event but not unregister whole acpi/video driver.

This patch introduced a new capability flag is ACER_CAP_KEEP_VIDEO_KEY, it indicates the machine works fine with acpi/video driver for key event but want to unregister the backlight interface of acpi/video.

Reference: bko#35622

        https://bugzilla.kernel.org/show_bug.cgi?id=35622
Tested-by Andrzej Krentosz <[email protected]>
Cc Carlos Corbacho <[email protected]>
Cc Matthew Garrett <[email protected]>
Cc Dmitry Torokhov <[email protected]>
Cc Corentin Chary <[email protected]>
Cc Rafael J. Wysocki <[email protected]>
Cc Aaron Lu <[email protected]>
Cc Thomas Renninger <[email protected]>
Signed-off-by Lee, Chun-Yi <[email protected]>
---
 drivers/platform/x86/acer-wmi.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
drivers/platform/x86/acer-wmi.c +1 -0
index c9076bd..2a02409 100644
@@ -207,6 +207,7 @@ struct hotkey_function_type_aa {
 #define ACER_CAP_BRIGHTNESS		(1<<3)
 #define ACER_CAP_THREEG			(1<<4)
 #define ACER_CAP_ACCEL			(1<<5)
+#define ACER_CAP_KEEP_VIDEO_KEY		(1<<6)
 #define ACER_CAP_ANY			(0xFFFFFFFF)
 
 /*

drivers/platform/x86/acer-wmi.c static int video_set_backlight_video_vendor(const struct dmi_system_id *d) · line 548

Quoted 9 lines
drivers/platform/x86/acer-wmi.c +6 -0
@@ -539,6 +540,15 @@ static int video_set_backlight_video_vendor(const struct dmi_system_id *d)
 	return 0;
 }
 
+static int video_set_backlight_video_vendor_keep_acpi_video(
+		const struct dmi_system_id *d)
+{
+	video_set_backlight_video_vendor(d);
+	interface->capability |= ACER_CAP_KEEP_VIDEO_KEY;
+	pr_info("Keep acpi video driver for emit keycode against backlight change\n");

Huh?

"Keeping acpi video driver active to emit backlight brightness change key events"?

Thanks.

Signature · 1 line
Dmitry
maintainer
#8 lore raw

Hi Dmitry,

Thanks for your review and suggestion!

於 一，2013-04-22 於 21:04 -0700，Dmitry Torokhov 提到：

Quoted 53 lines · Chun-Yi Lee
Quoted 40 lines

From: Lee, Chun-Yi <[email protected]>

After Andrzej's testing, we found the acpi backlight methods broken on Acer Aspire 5750G but the i915 backlight control works when we set to vendor mode. And, we still want to keep the acpi/video driver for transfer acpi event to key event but not unregister whole acpi/video driver.

This patch introduced a new capability flag is ACER_CAP_KEEP_VIDEO_KEY, it indicates the machine works fine with acpi/video driver for key event but want to unregister the backlight interface of acpi/video.

Reference: bko#35622

        https://bugzilla.kernel.org/show_bug.cgi?id=35622
Tested-by Andrzej Krentosz <[email protected]>
Cc Carlos Corbacho <[email protected]>
Cc Matthew Garrett <[email protected]>
Cc Dmitry Torokhov <[email protected]>
Cc Corentin Chary <[email protected]>
Cc Rafael J. Wysocki <[email protected]>
Cc Aaron Lu <[email protected]>
Cc Thomas Renninger <[email protected]>
Signed-off-by Lee, Chun-Yi <[email protected]>
---
 drivers/platform/x86/acer-wmi.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
drivers/platform/x86/acer-wmi.c +1 -0
index c9076bd..2a02409 100644
@@ -207,6 +207,7 @@ struct hotkey_function_type_aa {
 #define ACER_CAP_BRIGHTNESS		(1<<3)
 #define ACER_CAP_THREEG			(1<<4)
 #define ACER_CAP_ACCEL			(1<<5)
+#define ACER_CAP_KEEP_VIDEO_KEY		(1<<6)
 #define ACER_CAP_ANY			(0xFFFFFFFF)
 
 /*

drivers/platform/x86/acer-wmi.c static int video_set_backlight_video_vendor(const struct dmi_system_id *d) · line 548

Quoted 9 lines
drivers/platform/x86/acer-wmi.c +6 -0
@@ -539,6 +540,15 @@ static int video_set_backlight_video_vendor(const struct dmi_system_id *d)
 	return 0;
 }
 
+static int video_set_backlight_video_vendor_keep_acpi_video(
+		const struct dmi_system_id *d)
+{
+	video_set_backlight_video_vendor(d);
+	interface->capability |= ACER_CAP_KEEP_VIDEO_KEY;
+	pr_info("Keep acpi video driver for emit keycode against backlight change\n");

Huh?

Quoted 5 lines

"Keeping acpi video driver active to emit backlight brightness change key events"?

Thanks.

Yes, this statement is more fluent, I will put to v2 patch.

Thanks a lot!
Joey Lee