Re: Bug Report - [Acer Aspire V5-122P] Unable to adjust screen brightness

[Hans de Goede <hdegoede@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2057 bytes --]

Hi Lewis et all,

On 05/23/2014 02:07 PM, Lewis Toohey wrote:
> On 23 May 2014 02:34, Aaron Lu <aaron.lu@intel.com> wrote:
>> On 05/21/2014 09:02 PM, Lewis Toohey wrote:
>>> Hi Aaron
>>>
>>> I followed your instructions and can report some limited success. I
>>> have two interfaces listed in /sys/class/backlight namely:
>>>   acpi_video0  radeon_bl0
>>>
>>> max_brightness for acpi_video0 is 11. Echo-ing new values to
>>> brightness appears to have no effect.
>>>
>>> max_brightness for radeon_bl0 is 255. Echo-ing new values to
>>> brightness does adjust the screen brightness as you would expect.
>>>
>>> Results are the same on both battery and powered.
>>>
>>> I hope that is useful.
>>
>> I think Hans' patchset should solve your problem, can you please give it
>> a try? You will need to pass the video.use_native_backlight=1 to kernel
>> cmdline when testing, thanks.
>>
>> [PATCH resend 0/4] Make video.use_native_backlight=1 work properly with nouveau
>> http://www.spinics.net/lists/dri-devel/msg59941.html
>>
>> -Aaron
> 
> Aaron
> 
> Thank you for this. Unfortunately I am going to have to ask you for a
> bit of help here as I am now, officially, "out of my depth". I
> appreciate this is a pain as you will have to take time explaining it
> to me and I apologise for that.
> 
> I have figured out how to pass the kernel command line argument, this
> is not an issue.
> 
> I have located the patch you refer to here:
> https://bugzilla.redhat.com/attachment.cgi?id=894577
> (which I believe is correct) but I cannot figure out how to apply it.

That is not the right patch, you need a set of 2 patches. I've attached
them to this mail. Note please ignore the numbering starting at 6, these
are the 2 patches you need. You will need to build a Linux kernel with
these 2 patches applied, see your distributions documentation on how
to build a kernel from source.

I don't know which distro you are using, I've a Fedora kernel with this
patches included available here:

http://people.fedoraproject.org/~jwrdegoede/rhbz1093171/

Regards,

Hans

[-- Attachment #2: 0006-backlight-Add-backlight-device-un-registration-notif.patch --]
[-- Type: text/x-patch, Size: 4436 bytes --]

>From 6fbfb8e4474d01ab5b1dd7788534ee83a601926b Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Thu, 15 May 2014 14:36:04 +0200
Subject: [PATCH 6/8] backlight: Add backlight device (un)registration
 notification

Some firmware drivers, ie acpi-video want to get themselves out of the
way (in some cases) when their also is a raw backlight device available.

Due to module loading ordering being unknown, acpi-video cannot be certain
that the backlight_device_registered(BACKLIGHT_RAW) it does for this is
the final verdict wrt there being a BACKLIGHT_RAW device.

By adding notification acpi-video can listen for backlight devices showing
up after it has loaded, and unregister its backlight device if desired.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/video/backlight/backlight.c | 40 +++++++++++++++++++++++++++++++++++++
 include/linux/backlight.h           |  7 +++++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index bd2172c..4280890 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -23,6 +23,7 @@
 
 static struct list_head backlight_dev_list;
 static struct mutex backlight_dev_list_mutex;
+static struct blocking_notifier_head backlight_notifier;
 
 static const char *const backlight_types[] = {
 	[BACKLIGHT_RAW] = "raw",
@@ -370,6 +371,9 @@ struct backlight_device *backlight_device_register(const char *name,
 	list_add(&new_bd->entry, &backlight_dev_list);
 	mutex_unlock(&backlight_dev_list_mutex);
 
+	blocking_notifier_call_chain(&backlight_notifier,
+				     BACKLIGHT_REGISTERED, new_bd);
+
 	return new_bd;
 }
 EXPORT_SYMBOL(backlight_device_register);
@@ -413,6 +417,10 @@ void backlight_device_unregister(struct backlight_device *bd)
 		pmac_backlight = NULL;
 	mutex_unlock(&pmac_backlight_mutex);
 #endif
+
+	blocking_notifier_call_chain(&backlight_notifier,
+				     BACKLIGHT_UNREGISTERED, bd);
+
 	mutex_lock(&bd->ops_lock);
 	bd->ops = NULL;
 	mutex_unlock(&bd->ops_lock);
@@ -438,6 +446,36 @@ static int devm_backlight_device_match(struct device *dev, void *res,
 }
 
 /**
+ * backlight_register_notifier - get notified of backlight (un)registration
+ * @nb: notifier block with the notifier to call on backlight (un)registration
+ *
+ * @return 0 on success, otherwise a negative error code
+ *
+ * Register a notifier to get notified when backlight devices get registered
+ * or unregistered.
+ */
+int backlight_register_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&backlight_notifier, nb);
+}
+EXPORT_SYMBOL(backlight_register_notifier);
+
+/**
+ * backlight_unregister_notifier - unregister a backlight notifier
+ * @nb: notifier block to unregister
+ *
+ * @return 0 on success, otherwise a negative error code
+ *
+ * Register a notifier to get notified when backlight devices get registered
+ * or unregistered.
+ */
+int backlight_unregister_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&backlight_notifier, nb);
+}
+EXPORT_SYMBOL(backlight_unregister_notifier);
+
+/**
  * devm_backlight_device_register - resource managed backlight_device_register()
  * @dev: the device to register
  * @name: the name of the device
@@ -544,6 +582,8 @@ static int __init backlight_class_init(void)
 	backlight_class->pm = &backlight_class_dev_pm_ops;
 	INIT_LIST_HEAD(&backlight_dev_list);
 	mutex_init(&backlight_dev_list_mutex);
+	BLOCKING_INIT_NOTIFIER_HEAD(&backlight_notifier);
+
 	return 0;
 }
 
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 7264742..adb14a8 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -40,6 +40,11 @@ enum backlight_type {
 	BACKLIGHT_TYPE_MAX,
 };
 
+enum backlight_notification {
+	BACKLIGHT_REGISTERED,
+	BACKLIGHT_UNREGISTERED,
+};
+
 struct backlight_device;
 struct fb_info;
 
@@ -133,6 +138,8 @@ extern void devm_backlight_device_unregister(struct device *dev,
 extern void backlight_force_update(struct backlight_device *bd,
 				   enum backlight_update_reason reason);
 extern bool backlight_device_registered(enum backlight_type type);
+extern int backlight_register_notifier(struct notifier_block *nb);
+extern int backlight_unregister_notifier(struct notifier_block *nb);
 
 #define to_backlight_device(obj) container_of(obj, struct backlight_device, dev)
 
-- 
1.9.0


[-- Attachment #3: 0007-acpi-video-Unregister-the-backlight-device-if-a-raw-.patch --]
[-- Type: text/x-patch, Size: 3841 bytes --]

>From b865e1e6ef6e0abcc5b4084d854418ebf7cd7772 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Thu, 15 May 2014 15:50:20 +0200
Subject: [PATCH 7/8] acpi-video: Unregister the backlight device if a raw one
 shows up later

When video.use_native_backlight=1 and non intel gfx are in use, the raw
backlight device of the gfx driver will show up after acpi-video has done its
acpi_video_verify_backlight_support() check.

This causes video.use_native_backlight=1 to not have the desired result.

This patch fixes this by adding a backlight notifier and when a raw
backlight is registered or unregistered re-doing the
acpi_video_verify_backlight_support() check.

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

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index aee85c4..123f919 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -151,6 +151,7 @@ struct acpi_video_enumerated_device {
 struct acpi_video_bus {
 	struct acpi_device *device;
 	bool backlight_registered;
+	bool backlight_notifier_registered;
 	u8 dos_setting;
 	struct acpi_video_enumerated_device *attached_array;
 	u8 attached_count;
@@ -162,6 +163,7 @@ struct acpi_video_bus {
 	struct input_dev *input;
 	char phys[32];	/* for input device */
 	struct notifier_block pm_nb;
+	struct notifier_block backlight_nb;
 };
 
 struct acpi_video_device_flags {
@@ -1836,6 +1838,9 @@ static int acpi_video_bus_register_backlight(struct acpi_video_bus *video)
 {
 	struct acpi_video_device *dev;
 
+	if (video->backlight_registered)
+		return 0;
+
 	if (!acpi_video_verify_backlight_support())
 		return 0;
 
@@ -1980,6 +1985,56 @@ static void acpi_video_bus_remove_notify_handler(struct acpi_video_bus *video)
 	video->input = NULL;
 }
 
+static int acpi_video_backlight_notify(struct notifier_block *nb,
+					unsigned long val, void *bd)
+{
+	struct backlight_device *backlight = bd;
+	struct acpi_video_bus *video;
+
+	/* acpi_video_verify_backlight_support only cares about raw devices */
+	if (backlight->props.type != BACKLIGHT_RAW)
+		return NOTIFY_DONE;
+
+	video = container_of(nb, struct acpi_video_bus, backlight_nb);
+
+	switch (val) {
+	case BACKLIGHT_REGISTERED:
+		if (!acpi_video_verify_backlight_support())
+			acpi_video_bus_unregister_backlight(video);
+		break;
+	case BACKLIGHT_UNREGISTERED:
+		acpi_video_bus_register_backlight(video);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static int acpi_video_bus_add_backlight_notify_handler(
+						struct acpi_video_bus *video)
+{
+	int error;
+
+	video->backlight_nb.notifier_call = acpi_video_backlight_notify;
+	video->backlight_nb.priority = 0;
+	error = backlight_register_notifier(&video->backlight_nb);
+	if (error == 0)
+		video->backlight_notifier_registered = true;
+
+	return error;
+}
+
+static int acpi_video_bus_remove_backlight_notify_handler(
+						struct acpi_video_bus *video)
+{
+	if (!video->backlight_notifier_registered)
+		return 0;
+
+	video->backlight_notifier_registered = false;
+
+	return backlight_unregister_notifier(&video->backlight_nb);
+}
+
 static int acpi_video_bus_put_devices(struct acpi_video_bus *video)
 {
 	struct acpi_video_device *dev, *next;
@@ -2061,6 +2116,7 @@ static int acpi_video_bus_add(struct acpi_device *device)
 
 	acpi_video_bus_register_backlight(video);
 	acpi_video_bus_add_notify_handler(video);
+	acpi_video_bus_add_backlight_notify_handler(video);
 
 	return 0;
 
@@ -2084,6 +2140,7 @@ static int acpi_video_bus_remove(struct acpi_device *device)
 
 	video = acpi_driver_data(device);
 
+	acpi_video_bus_remove_backlight_notify_handler(video);
 	acpi_video_bus_remove_notify_handler(video);
 	acpi_video_bus_unregister_backlight(video);
 	acpi_video_bus_put_devices(video);
-- 
1.9.0