[PATCH 3/3] eeepc-laptop: Update the backlight state when we change brightness

[PATCH 3/3] eeepc-laptop: Update the backlight state when we change brightness

[Matthew Garrett]

Trigger a status update when the user hits a brightness key, allowing
userspace to present appropriate UI.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/platform/x86/eeepc-laptop.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
index ec560f1..832db81 100644
--- a/drivers/platform/x86/eeepc-laptop.c
+++ b/drivers/platform/x86/eeepc-laptop.c
@@ -641,7 +641,7 @@ static int notify_brn(void)
 	struct backlight_device *bd = eeepc_backlight_device;
 	if (bd) {
 		int old = bd->props.brightness;
-		bd->props.brightness = read_brightness(bd);
+		backlight_force_update(bd);
 		return old;
 	}
 	return -1;
-- 
1.6.2.5

[PATCH 1/3] backlight: Allow drivers to update the core, and generate events on changes

[Matthew Garrett]

Certain hardware will send us events when the backlight brightness
changes. Add a function to update the value in the core, and
additionally send a uevent so that userspace can pop up appropriate
UI. The uevents are flagged depending on whether the update originated
in the kernel or from userspace, making it easier to only display UI
at the appropriate time.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---

Updated to allow drivers to provide a reason for the change

 drivers/video/backlight/backlight.c |   41 +++++++++++++++++++++++++++++++++++
 include/linux/backlight.h           |    7 ++++++
 2 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 157057c..01efb2d 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -73,6 +73,26 @@ static inline void backlight_unregister_fb(struct backlight_device *bd)
 }
 #endif /* CONFIG_FB */
 
+static void backlight_generate_event(struct backlight_device *bd,
+				     enum backlight_update_reason reason)
+{
+	char *envp[2];
+
+	switch (reason) {
+	case BACKLIGHT_UPDATE_SYSFS:
+		envp[0] = "SOURCE=sysfs";
+		break;
+	case BACKLIGHT_UPDATE_HOTKEY:
+		envp[0] = "SOURCE=hotkey";
+		break;
+	default:
+		envp[0] = "SORUCE=unknown";
+		break;
+	}
+	envp[1] = NULL;
+	kobject_uevent_env(&bd->dev.kobj, KOBJ_CHANGE, envp);
+}
+
 static ssize_t backlight_show_power(struct device *dev,
 		struct device_attribute *attr,char *buf)
 {
@@ -142,6 +162,8 @@ static ssize_t backlight_store_brightness(struct device *dev,
 	}
 	mutex_unlock(&bd->ops_lock);
 
+	backlight_generate_event(bd, BACKLIGHT_UPDATE_SYSFS);
+
 	return rc;
 }
 
@@ -214,6 +236,25 @@ static struct device_attribute bl_device_attributes[] = {
 };
 
 /**
+ * backlight_force_update - tell the backlight subsystem that hardware state
+ *   has changed
+ * @bd: the backlight device to update
+ *
+ * Updates the internal state of the backlight in response to a hardware event,
+ * and generate a uevent to notify userspace
+ */
+void backlight_force_update(struct backlight_device *bd,
+			    enum backlight_update_reason reason)
+{
+	mutex_lock(&bd->ops_lock);
+	if (bd->ops && bd->ops->get_brightness)
+		bd->props.brightness = bd->ops->get_brightness(bd);
+	mutex_unlock(&bd->ops_lock);
+	backlight_generate_event(bd, reason);
+}
+EXPORT_SYMBOL(backlight_force_update);
+
+/**
  * backlight_device_register - create and register a new object of
  *   backlight_device class.
  * @name: the name of the new object(must be the same as the name of the
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 79ca2da..0f5f578 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -27,6 +27,11 @@
  * Any other use of the locks below is probably wrong.
  */
 
+enum backlight_update_reason {
+	BACKLIGHT_UPDATE_HOTKEY,
+	BACKLIGHT_UPDATE_SYSFS,
+};
+
 struct backlight_device;
 struct fb_info;
 
@@ -100,6 +105,8 @@ static inline void backlight_update_status(struct backlight_device *bd)
 extern struct backlight_device *backlight_device_register(const char *name,
 	struct device *dev, void *devdata, struct backlight_ops *ops);
 extern void backlight_device_unregister(struct backlight_device *bd);
+extern void backlight_force_update(struct backlight_device *bd,
+				   enum backlight_update_reason reason);
 
 #define to_backlight_device(obj) container_of(obj, struct backlight_device, dev)
 
-- 
1.6.2.5

[PATCH 2/3] ACPI: Update the backlight state when we change brightness

[Matthew Garrett]

Trigger a status update when we change the brightness in the driver, thus
allowing userspace to present appropriate UI.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/acpi/video.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 8851315..2c906c7 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1960,6 +1960,10 @@ acpi_video_switch_brightness(struct acpi_video_device *device, int event)
 
 	result = acpi_video_device_lcd_set_level(device, level_next);
 
+	if (!result)
+		backlight_force_update(device->backlight,
+				       BACKLIGHT_UPDATE_HOTKEY);
+
 out:
 	if (result)
 		printk(KERN_ERR PREFIX "Failed to switch the brightness\n");
-- 
1.6.2.5

[PATCH 3/3] eeepc-laptop: Update the backlight state when we change brightness

[Matthew Garrett]

Trigger a status update when the user hits a brightness key, allowing
userspace to present appropriate UI.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/platform/x86/eeepc-laptop.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
index ec560f1..fde86b3 100644
--- a/drivers/platform/x86/eeepc-laptop.c
+++ b/drivers/platform/x86/eeepc-laptop.c
@@ -641,7 +641,7 @@ static int notify_brn(void)
 	struct backlight_device *bd = eeepc_backlight_device;
 	if (bd) {
 		int old = bd->props.brightness;
-		bd->props.brightness = read_brightness(bd);
+		backlight_force_update(bd, BACKLIGHT_UPDATE_HOTKEY);
 		return old;
 	}
 	return -1;
-- 
1.6.2.5

Re: [PATCH 3/3] eeepc-laptop: Update the backlight state when we change brightness

[Corentin Chary]

On Tue, Jul 14, 2009 at 6:06 PM, Matthew Garrett<mjg@redhat.com> wrote:
> Trigger a status update when the user hits a brightness key, allowing
> userspace to present appropriate UI.
>
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> ---
>  drivers/platform/x86/eeepc-laptop.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
> index ec560f1..fde86b3 100644
> --- a/drivers/platform/x86/eeepc-laptop.c
> +++ b/drivers/platform/x86/eeepc-laptop.c
> @@ -641,7 +641,7 @@ static int notify_brn(void)
>        struct backlight_device *bd = eeepc_backlight_device;
>        if (bd) {
>                int old = bd->props.brightness;
> -               bd->props.brightness = read_brightness(bd);
> +               backlight_force_update(bd, BACKLIGHT_UPDATE_HOTKEY);
>                return old;
>        }
>        return -1;
> --
> 1.6.2.5
>
>

This could be added to asus-laptop too. It doesn't handle brightness
changes right now (only backlight on/off) but it works the same way,
so it won't be hard to add. I'll do that as soon as your main patch is
merged.
Thanks,


-- 
Corentin Chary
http://xf.iksaif.net - http://uffs.org
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] backlight: Allow drivers to update the core, and generate events on changes

[Michal Schmidt]

Dne Tue, 14 Jul 2009 17:06:02 +0100 Matthew Garrett napsal:
> +static void backlight_generate_event(struct backlight_device *bd,
> +				     enum backlight_update_reason
> reason) +{
> +	char *envp[2];
> +
> +	switch (reason) {
> +	case BACKLIGHT_UPDATE_SYSFS:
> +		envp[0] = "SOURCE=sysfs";
> +		break;
> +	case BACKLIGHT_UPDATE_HOTKEY:
> +		envp[0] = "SOURCE=hotkey";
> +		break;
> +	default:
> +		envp[0] = "SORUCE=unknown";

There's a typo here. s/SORUCE/SOURCE/

Michal

Re: [PATCH 1/3] backlight: Allow drivers to update the core, and generate events on changes

[Pavel Machek]

On Tue 2009-07-14 17:06:02, Matthew Garrett wrote:
> Certain hardware will send us events when the backlight brightness
> changes. Add a function to update the value in the core, and
> additionally send a uevent so that userspace can pop up appropriate
> UI. The uevents are flagged depending on whether the update originated
> in the kernel or from userspace, making it easier to only display UI
> at the appropriate time.

This adds new kernel API. Should it be documented somewhere?

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/3] backlight: Allow drivers to update the core, and generate events on changes

[Henrique de Moraes Holschuh]

On Tue, 14 Jul 2009, Matthew Garrett wrote:
> Certain hardware will send us events when the backlight brightness
> changes. Add a function to update the value in the core, and
> additionally send a uevent so that userspace can pop up appropriate
> UI. The uevents are flagged depending on whether the update originated
> in the kernel or from userspace, making it easier to only display UI
> at the appropriate time.

Ok.  I have it working (event-based AND poll-based) in thinkpad-acpi,
although it needs to be tested by dual-GPU, ACPI-based and OpRegion-based
owners.

Should the driver notify of *any* changes, even when the user is hitting the
"ceiling" or the "floor" and the backlight value didn't really change?

If the answer is no, shouldn't the backlight class filter these out itself,
to guarantee a consistent experience?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH 1/3] backlight: Allow drivers to update the core, and generate events on changes

[Matthew Garrett]

On Sat, Sep 19, 2009 at 08:24:24PM -0300, Henrique de Moraes Holschuh wrote:

> Should the driver notify of *any* changes, even when the user is hitting the
> "ceiling" or the "floor" and the backlight value didn't really change?

I'd say yes - the user will expect the UI to appear whenever they hit 
the button.

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

Re: [PATCH 1/3] backlight: Allow drivers to update the core, and generate events on changes

[Henrique de Moraes Holschuh]

On Sun, 20 Sep 2009, Matthew Garrett wrote:
> On Sat, Sep 19, 2009 at 08:24:24PM -0300, Henrique de Moraes Holschuh wrote:
> > Should the driver notify of *any* changes, even when the user is hitting the
> > "ceiling" or the "floor" and the backlight value didn't really change?
> 
> I'd say yes - the user will expect the UI to appear whenever they hit 
> the button.

Ok.  The thinkpad-acpi changes depends on a large patchset that I should
send to Len today.  After that, I will send the patch that adds brightness
event support.

Are you backlight patches in any tree?  Are them expected to hit mainline
this merge window?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh