The mbp_nvidia_bl driver allows to change the display brightness on
Apple MacBook Pro with Nvidia graphics adapters. However, after
resuming from suspend the brightness is at its maximum instead of
the last recently used value. This patch converts the driver into a
platform_driver in order to register a resume hook to re-send brightness.
In addition, resources will be allocated through platform_driver means
instead of calling request_region/release_region directly.
A patch incorporating MacBook 5 MacBook Air 2, and MacBook Pro 5
support is also available and will be sent next.
Acked-by: Henrik Rydberg <[email protected]>
Signed-off-by: Mario Schwalbe <[email protected]>
---
drivers/video/backlight/mbp_nvidia_bl.c | 95 +++++++++++++++++++++++++------
1 files changed, 77 insertions(+), 18 deletions(-)
diff --git a/drivers/video/backlight/mbp_nvidia_bl.c b/drivers/video/backlight/mbp_nvidia_bl.c
index 06964af..40a6b79 100644
--- a/drivers/video/backlight/mbp_nvidia_bl.c
+++ b/drivers/video/backlight/mbp_nvidia_bl.c
@@ -25,8 +25,6 @@
#include <linux/dmi.h>
#include <linux/io.h>
-static struct backlight_device *mbp_backlight_device;
-
static struct dmi_system_id __initdata mbp_device_table[] = {
{
.ident = "3,1",
@@ -74,35 +72,96 @@ static struct backlight_ops mbp_ops = {
.update_status = mbp_send_intensity,
};
+static int mbp_probe(struct platform_device *pdev)
+{
+ struct backlight_device *bd;
+
+ bd = backlight_device_register("mbp_backlight", NULL, NULL, &mbp_ops);
+ if (IS_ERR(bd))
+ return PTR_ERR(bd);
+
+ bd->props.max_brightness = 15;
+ bd->props.brightness = mbp_get_intensity(bd);
+ backlight_update_status(bd);
+
+ platform_set_drvdata(pdev, bd);
+ return 0;
+}
+
+static int mbp_remove(struct platform_device *pdev)
+{
+ struct backlight_device *bd = platform_get_drvdata(pdev);
+
+ backlight_device_unregister(bd);
+ return 0;
+}
+
+#ifdef CONFIG_PM
+static int mbp_resume(struct platform_device *pdev)
+{
+ /*
+ * Nvidia GeForce 8600M GT in MacBookPro 3,1 and MacBookPro 4,1:
+ * 1. Resume after suspend:
+ * SMI still reports the last recently written value, while
+ * the brightness is actually at its maximum.
+ * 2. Resume after hibernation:
+ * SMI reports the actual (maximum) value, but the state
+ * in props.brightness might be different.
+ * -> Re-send to fix (1), but do not re-read to fix (2).
+ */
+ struct backlight_device *bd = platform_get_drvdata(pdev);
+
+ backlight_update_status(bd);
+ return 0;
+}
+#else
+#define mbp_resume NULL
+#endif
+
+static struct platform_driver mbp_driver = {
+ .probe = mbp_probe,
+ .remove = mbp_remove,
+ .resume = mbp_resume,
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = "mbp_nvidia_bl"
+ },
+};
+
+static struct resource mbp_iores = {
+ .start = 0xb2,
+ .end = 0xb3,
+ .name = "mbp_nvidia_bl",
+ .flags = IORESOURCE_IO
+};
+
+static struct platform_device *mbp_device;
+
static int __init mbp_init(void)
{
+ int ret;
+
if (!dmi_check_system(mbp_device_table))
return -ENODEV;
- if (!request_region(0xb2, 2, "Macbook Pro backlight"))
- return -ENXIO;
+ ret = platform_driver_register(&mbp_driver);
+ if (ret)
+ return ret;
- mbp_backlight_device = backlight_device_register("mbp_backlight",
- NULL, NULL,
- &mbp_ops);
- if (IS_ERR(mbp_backlight_device)) {
- release_region(0xb2, 2);
- return PTR_ERR(mbp_backlight_device);
+ mbp_device = platform_device_register_simple("mbp_nvidia_bl", -1,
+ &mbp_iores, 1);
+ if (!mbp_device) {
+ platform_driver_unregister(&mbp_driver);
+ return -ENOMEM;
}
- mbp_backlight_device->props.max_brightness = 15;
- mbp_backlight_device->props.brightness =
- mbp_get_intensity(mbp_backlight_device);
- backlight_update_status(mbp_backlight_device);
-
return 0;
}
static void __exit mbp_exit(void)
{
- backlight_device_unregister(mbp_backlight_device);
-
- release_region(0xb2, 2);
+ platform_device_unregister(mbp_device);
+ platform_driver_unregister(&mbp_driver);
}
module_init(mbp_init);
--
1.5.6.3
On Wed, 2008-12-10 at 21:30 +0100, Mario Schwalbe wrote:
> The mbp_nvidia_bl driver allows to change the display brightness on
> Apple MacBook Pro with Nvidia graphics adapters. However, after
> resuming from suspend the brightness is at its maximum instead of
> the last recently used value. This patch converts the driver into a
> platform_driver in order to register a resume hook to re-send brightness.
> In addition, resources will be allocated through platform_driver means
> instead of calling request_region/release_region directly.
I understand the problem, I'm not sure adding more platform devices and
drivers into the equation is a good way to solve it. I've been thinking
of adding suspend/resume support into the backlight core for a while and
this would solve this problem. How about the following patch (only
compile tested so far)?
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 592d714..fc95829 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -40,6 +40,10 @@ static int fb_notifier_callback(struct notifier_block *self,
if (!bd->ops->check_fb ||
bd->ops->check_fb(evdata->info)) {
bd->props.fb_blank = *(int *)evdata->data;
+ if (bd->props.fb_blank == FB_BLANK_UNBLANK)
+ bd->props.flags &= ~BL_CORE_FBBLANK;
+ else
+ bd->props.flags |= BL_CORE_FBBLANK;
backlight_update_status(bd);
}
mutex_unlock(&bd->ops_lock);
@@ -167,6 +171,34 @@ static ssize_t backlight_show_actual_brightness(struct device *dev,
static struct class *backlight_class;
+static int backlight_suspend(struct device *dev, pm_message_t state)
+{
+ struct backlight_device *bd = to_backlight_device(dev);
+
+ if (bd->ops->flags & BL_CORE_SUSPENDRESUME) {
+ mutex_lock(&bd->ops_lock);
+ bd->props.flags |= BL_CORE_SUSPENDED;
+ backlight_update_status(bd);
+ mutex_unlock(&bd->ops_lock);
+ }
+
+ return 0;
+}
+
+static int backlight_resume(struct device *dev)
+{
+ struct backlight_device *bd = to_backlight_device(dev);
+
+ if (bd->ops->flags & BL_CORE_SUSPENDRESUME) {
+ mutex_lock(&bd->ops_lock);
+ bd->props.flags &= ~BL_CORE_SUSPENDED;
+ backlight_update_status(bd);
+ mutex_unlock(&bd->ops_lock);
+ }
+
+ return 0;
+}
+
static void bl_device_release(struct device *dev)
{
struct backlight_device *bd = to_backlight_device(dev);
@@ -283,6 +315,8 @@ static int __init backlight_class_init(void)
}
backlight_class->dev_attrs = bl_device_attributes;
+ backlight_class->suspend = backlight_suspend;
+ backlight_class->resume = backlight_resume;
return 0;
}
diff --git a/drivers/video/backlight/corgi_bl.c b/drivers/video/backlight/corgi_bl.c
index 4d4d037..02ff4bb 100644
--- a/drivers/video/backlight/corgi_bl.c
+++ b/drivers/video/backlight/corgi_bl.c
@@ -25,8 +25,7 @@ static struct backlight_device *corgi_backlight_device;
static struct generic_bl_info *bl_machinfo;
static unsigned long corgibl_flags;
-#define CORGIBL_SUSPENDED 0x01
-#define CORGIBL_BATTLOW 0x02
+#define CORGIBL_BATTLOW 0x01
static int corgibl_send_intensity(struct backlight_device *bd)
{
@@ -34,9 +33,9 @@ static int corgibl_send_intensity(struct backlight_device *bd)
if (bd->props.power != FB_BLANK_UNBLANK)
intensity = 0;
- if (bd->props.fb_blank != FB_BLANK_UNBLANK)
+ if (bd->props.flags & BL_CORE_FBBLANK)
intensity = 0;
- if (corgibl_flags & CORGIBL_SUSPENDED)
+ if (bd->props.flags & BL_CORE_SUSPENDED)
intensity = 0;
if (corgibl_flags & CORGIBL_BATTLOW)
intensity &= bl_machinfo->limit_mask;
@@ -51,29 +50,6 @@ static int corgibl_send_intensity(struct backlight_device *bd)
return 0;
}
-#ifdef CONFIG_PM
-static int corgibl_suspend(struct platform_device *pdev, pm_message_t state)
-{
- struct backlight_device *bd = platform_get_drvdata(pdev);
-
- corgibl_flags |= CORGIBL_SUSPENDED;
- backlight_update_status(bd);
- return 0;
-}
-
-static int corgibl_resume(struct platform_device *pdev)
-{
- struct backlight_device *bd = platform_get_drvdata(pdev);
-
- corgibl_flags &= ~CORGIBL_SUSPENDED;
- backlight_update_status(bd);
- return 0;
-}
-#else
-#define corgibl_suspend NULL
-#define corgibl_resume NULL
-#endif
-
static int corgibl_get_intensity(struct backlight_device *bd)
{
return corgibl_intensity;
@@ -95,6 +71,7 @@ EXPORT_SYMBOL(corgibl_limit_intensity);
static struct backlight_ops corgibl_ops = {
+ .flags = BL_CORE_SUSPENDRESUME,
.get_brightness = corgibl_get_intensity,
.update_status = corgibl_send_intensity,
};
@@ -144,8 +121,6 @@ static int corgibl_remove(struct platform_device *pdev)
static struct platform_driver corgibl_driver = {
.probe = corgibl_probe,
.remove = corgibl_remove,
- .suspend = corgibl_suspend,
- .resume = corgibl_resume,
.driver = {
.name = "generic-bl",
},
diff --git a/drivers/video/backlight/mbp_nvidia_bl.c b/drivers/video/backlight/mbp_nvidia_bl.c
index 06964af..f5bd0b4 100644
--- a/drivers/video/backlight/mbp_nvidia_bl.c
+++ b/drivers/video/backlight/mbp_nvidia_bl.c
@@ -70,6 +70,7 @@ static int mbp_get_intensity(struct backlight_device *bd)
}
static struct backlight_ops mbp_ops = {
+ .flags = BL_CORE_SUSPENDRESUME,
.get_brightness = mbp_get_intensity,
.update_status = mbp_send_intensity,
};
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 1ee9488..516fbc7 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -31,6 +31,10 @@ struct backlight_device;
struct fb_info;
struct backlight_ops {
+ unsigned int flags;
+
+#define BL_CORE_SUSPENDRESUME (1 << 0)
+
/* Notify the backlight driver some property has changed */
int (*update_status)(struct backlight_device *);
/* Return the current backlight brightness (accounting for power,
@@ -52,6 +56,12 @@ struct backlight_properties {
int power;
/* FB Blanking active? (values as for power) */
int fb_blank;
+ /* Flags used to signal drivers of state changes */
+ unsigned int flags;
+
+#define BL_CORE_SUSPENDED (1 << 0) /* backlight is suspended */
+#define BL_CORE_FBBLANK (1 << 1) /* backlight is under an fb blank event */
+
};
struct backlight_device {
--
Richard Purdie
Intel Open Source Technology Centre
Hi,
Richard Purdie schrieb:
> On Wed, 2008-12-10 at 21:30 +0100, Mario Schwalbe wrote:
>> The mbp_nvidia_bl driver allows to change the display brightness on
>> Apple MacBook Pro with Nvidia graphics adapters. However, after
>> resuming from suspend the brightness is at its maximum instead of
>> the last recently used value. This patch converts the driver into a
>> platform_driver in order to register a resume hook to re-send brightness.
>> In addition, resources will be allocated through platform_driver means
>> instead of calling request_region/release_region directly.
>
> I understand the problem, I'm not sure adding more platform devices and
> drivers into the equation is a good way to solve it.
I don't know what the problem of adding platform_drivers is. I just thought
it's a reasonable solution after looking at the drivers next to it.
> I've been thinking
> of adding suspend/resume support into the backlight core for a while and
> this would solve this problem. How about the following patch (only
> compile tested so far)?
That looks like it could work. (Well, I didn't even compile the code.)
However, i'd like to note:
1. props.flags and ops->flags is somewhat confusing. i had to read several
times to see the difference. what about calling props.flags props.state?
what it actually is.
2. props.power, props.fb_blank, and the newly props.flags looks quite confusing
to me. what is it all used for? they are all about the current state?
3. Right now it's possible to add code specific for the resume process, e.g.,
to circumvent hardware bugs. after applying your patch the drivers won't
see a difference to normal updates anymore, due to backlight_resume clearing
the BL_CORE_SUSPENDED flag before calling backlight_update_status, and there's
no 'resume in progress' flag. i wouldn't need it anyway, but maybe someone else.
What about the remaining patches for 5th gen models support? They are against
the last patch. Am I supposed to send them now? Or wait and adapt them to your
patch once it's in Linus's tree (the one I cloned).
ciao,
Mario
--
Wo das Chaos auf die Ordnung trifft, gewinnt meist das Chaos,
weil es besser organisiert ist.
- Friedrich Nietzsche -
--
_____ ________
/ \ / ____/ Mario Schwalbe
/ \ / \ \____ \
/ Y \/ \ eMail: [email protected],
\____|__ /______ / [email protected]
\/ \/
key ID: 7DA9 DAFF
public key: https://www1.inf.tu-dresden.de/~ms790178/key.asc
key fingerprint: 2979 AA20 4A93 B527 90CC 45E5 4B28 511A 7DA9 DAFF