[PATCH] apple-gmux: Add suspend/resume support for the backlight

[PATCH] apple-gmux: Add suspend/resume support for the backlight

[Seth Forshee]

After S3, the brightness might not be restored to the pre-suspend value.
Request status update calls from the backlight core on suspend/resume to
ensure the brightness value is restored.

Reported-and-tested-by: Austin Lund <austin.lund@gmail.com>
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/platform/x86/apple-gmux.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 8a582bd..6dcef4f 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -87,6 +87,9 @@ static int gmux_update_status(struct backlight_device *bd)
 	struct apple_gmux_data *gmux_data = bl_get_data(bd);
 	u32 brightness = bd->props.brightness;
 
+	if (bd->props.state & BL_CORE_SUSPENDED)
+		brightness = 0;
+
 	/*
 	 * Older gmux versions require writing out lower bytes first then
 	 * setting the upper byte to 0 to flush the values. Newer versions
@@ -102,6 +105,7 @@ static int gmux_update_status(struct backlight_device *bd)
 }
 
 static const struct backlight_ops gmux_bl_ops = {
+	.options = BL_CORE_SUSPENDRESUME,
 	.get_brightness = gmux_get_brightness,
 	.update_status = gmux_update_status,
 };
-- 
1.7.9.5

Re: [PATCH] apple-gmux: Add suspend/resume support for the backlight

[Seth Forshee]

Ping. Any chance of getting this into 3.4?

On Thu, Apr 19, 2012 at 10:55:35AM -0500, Seth Forshee wrote:
> After S3, the brightness might not be restored to the pre-suspend value.
> Request status update calls from the backlight core on suspend/resume to
> ensure the brightness value is restored.
> 
> Reported-and-tested-by: Austin Lund <austin.lund@gmail.com>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>  drivers/platform/x86/apple-gmux.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
> index 8a582bd..6dcef4f 100644
> --- a/drivers/platform/x86/apple-gmux.c
> +++ b/drivers/platform/x86/apple-gmux.c
> @@ -87,6 +87,9 @@ static int gmux_update_status(struct backlight_device *bd)
>  	struct apple_gmux_data *gmux_data = bl_get_data(bd);
>  	u32 brightness = bd->props.brightness;
>  
> +	if (bd->props.state & BL_CORE_SUSPENDED)
> +		brightness = 0;
> +
>  	/*
>  	 * Older gmux versions require writing out lower bytes first then
>  	 * setting the upper byte to 0 to flush the values. Newer versions
> @@ -102,6 +105,7 @@ static int gmux_update_status(struct backlight_device *bd)
>  }
>  
>  static const struct backlight_ops gmux_bl_ops = {
> +	.options = BL_CORE_SUSPENDRESUME,
>  	.get_brightness = gmux_get_brightness,
>  	.update_status = gmux_update_status,
>  };
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] apple-gmux: Add suspend/resume support for the backlight

[Matthew Garrett]

Looks reasonable. I'll send today or tomorrow.

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

Re: [PATCH] apple-gmux: Add suspend/resume support for the backlight

[Matthew Garrett]

Applied to -next, thanks

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

Re: [PATCH] apple-gmux: Add suspend/resume support for the backlight

[Seth Forshee]

On Thu, May 03, 2012 at 09:33:32PM +0100, Matthew Garrett wrote:
> Applied to -next, thanks

I thought you were going to send this one for 3.4?

Re: [PATCH] apple-gmux: Add suspend/resume support for the backlight

[David Woodhouse]

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

On Thu, 2012-04-19 at 10:55 -0500, Seth Forshee wrote:
> --- a/drivers/platform/x86/apple-gmux.c
> +++ b/drivers/platform/x86/apple-gmux.c
> @@ -87,6 +87,9 @@ static int gmux_update_status(struct backlight_device *bd)
>         struct apple_gmux_data *gmux_data = bl_get_data(bd);
>         u32 brightness = bd->props.brightness;
>  
> +       if (bd->props.state & BL_CORE_SUSPENDED)
> +               brightness = 0;
> +
>         /*
>          * Older gmux versions require writing out lower bytes first then
>          * setting the upper byte to 0 to flush the values. Newer versions

Is this part necessary? It's about to be turned off *anyway*, and
turning it off sooner will achieve little other than to make it harder
to debug suspend failures, surely?

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [PATCH] apple-gmux: Add suspend/resume support for the backlight

[Seth Forshee]

On Thu, May 10, 2012 at 09:06:52AM -0700, David Woodhouse wrote:
> On Thu, 2012-04-19 at 10:55 -0500, Seth Forshee wrote:
> > --- a/drivers/platform/x86/apple-gmux.c
> > +++ b/drivers/platform/x86/apple-gmux.c
> > @@ -87,6 +87,9 @@ static int gmux_update_status(struct backlight_device *bd)
> >         struct apple_gmux_data *gmux_data = bl_get_data(bd);
> >         u32 brightness = bd->props.brightness;
> >  
> > +       if (bd->props.state & BL_CORE_SUSPENDED)
> > +               brightness = 0;
> > +
> >         /*
> >          * Older gmux versions require writing out lower bytes first then
> >          * setting the upper byte to 0 to flush the values. Newer versions
> 
> Is this part necessary? It's about to be turned off *anyway*, and
> turning it off sooner will achieve little other than to make it harder
> to debug suspend failures, surely?

No, I suppose it isn't strictly necessary. I added it to avoid having
the backlight turned off when the lid is closed and then turned back on
while suspending.

Seth

Re: [PATCH] apple-gmux: Add suspend/resume support for the backlight

[David Woodhouse]

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

On Thu, 2012-05-10 at 09:23 -0700, Seth Forshee wrote:
> On Thu, May 10, 2012 at 09:06:52AM -0700, David Woodhouse wrote:
> > On Thu, 2012-04-19 at 10:55 -0500, Seth Forshee wrote:
> > > --- a/drivers/platform/x86/apple-gmux.c
> > > +++ b/drivers/platform/x86/apple-gmux.c
> > > @@ -87,6 +87,9 @@ static int gmux_update_status(struct backlight_device *bd)
> > >         struct apple_gmux_data *gmux_data = bl_get_data(bd);
> > >         u32 brightness = bd->props.brightness;
> > >  
> > > +       if (bd->props.state & BL_CORE_SUSPENDED)
> > > +               brightness = 0;
> > > +
> > >         /*
> > >          * Older gmux versions require writing out lower bytes first then
> > >          * setting the upper byte to 0 to flush the values. Newer versions
> > 
> > Is this part necessary? It's about to be turned off *anyway*, and
> > turning it off sooner will achieve little other than to make it harder
> > to debug suspend failures, surely?
> 
> No, I suppose it isn't strictly necessary. I added it to avoid having
> the backlight turned off when the lid is closed and then turned back on
> while suspending.

That's achievable with

 if (bd->props.state & BL_CORE_SUSPENDED)
    return;

right?

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [PATCH] apple-gmux: Add suspend/resume support for the backlight

[Seth Forshee]

On Thu, May 10, 2012 at 09:49:48AM -0700, David Woodhouse wrote:
> On Thu, 2012-05-10 at 09:23 -0700, Seth Forshee wrote:
> > On Thu, May 10, 2012 at 09:06:52AM -0700, David Woodhouse wrote:
> > > On Thu, 2012-04-19 at 10:55 -0500, Seth Forshee wrote:
> > > > --- a/drivers/platform/x86/apple-gmux.c
> > > > +++ b/drivers/platform/x86/apple-gmux.c
> > > > @@ -87,6 +87,9 @@ static int gmux_update_status(struct backlight_device *bd)
> > > >         struct apple_gmux_data *gmux_data = bl_get_data(bd);
> > > >         u32 brightness = bd->props.brightness;
> > > >  
> > > > +       if (bd->props.state & BL_CORE_SUSPENDED)
> > > > +               brightness = 0;
> > > > +
> > > >         /*
> > > >          * Older gmux versions require writing out lower bytes first then
> > > >          * setting the upper byte to 0 to flush the values. Newer versions
> > > 
> > > Is this part necessary? It's about to be turned off *anyway*, and
> > > turning it off sooner will achieve little other than to make it harder
> > > to debug suspend failures, surely?
> > 
> > No, I suppose it isn't strictly necessary. I added it to avoid having
> > the backlight turned off when the lid is closed and then turned back on
> > while suspending.
> 
> That's achievable with
> 
>  if (bd->props.state & BL_CORE_SUSPENDED)
>     return;
> 
> right?

Good point. I'll send an updated patch.

Seth