Re: [PATCH] i915: Fix bug where screen brightness is not restored

Re: [PATCH] i915: Fix bug where screen brightness is not restored

[Kamal Mostafa]

[-- Attachment #1.1: Type: text/plain, Size: 2603 bytes --]

On Mon, 2011-11-14 at 18:42 -0800, Alex Davis wrote:
> From: Alex Davis<alex14641@yahoo.com>
> 
> This patch fixes an issue where the screen would remain dark when
> 
> a key was pressed when the laptop lid was reopened or after the
> laptop had gone into power-save mode.

[cross-posting to intel-gfx]

Keith, also note that Alex and I did respond to your request to test the
already committed patch "drm/i915/panel: Always record the backlight
level again (but cleverly)".  We both determined that patch does *not*
fix the problem.  Alex's patch below does fix it.

Matthew, any comment on the question that I fielded?:

>> Why does intel_panel_disable_backlight even want to set the
>> brightness to 0 anyway?...  Its only caller is just about to turn
>> off power to the panel.  Is that call to
>> intel_panel_{,actually_}set_backlight(dev, 0) really necessary
>> or useful on *any* systems?

 -Kamal


> It seems that there are a number of people with different machines
> that have this problem:
> 
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/872652
> https://launchpad.net/~kamalmostafa/+archive/stuck-backlight
> and https://bugs.freedesktop.org/show_bug.cgi?id=41926
> 
> This patch is against Linux 3.1
> 
> Putting printk's in ./drivers/gpu/drm/i915/intel_panel.c showed that 
> intel_get_brightness was being called after the panel was disabled,
> which caused a 0 to be saved as the value to restore the brightness. 
> intel_panel_disable_backlight merely sets the brightness to 0. Commenting
> out this call allows the correct brightness value to be saved.
> 
> Signed-off-by: Alex Davis <alex14641@yahoo.com>
> Tested-by: Kamal Mostafa <kamal@canonical.com>
> -----
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index a9e0c7b..6f56676 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -262,8 +262,6 @@ void intel_panel_disable_backlight(struct drm_device *dev)
>                 dev_priv->backlight_level = intel_panel_get_backlight(dev);
>                 dev_priv->backlight_enabled = false;
>         }
> -
> -       intel_panel_set_backlight(dev, 0);
>  }
> 
>  void intel_panel_enable_backlight(struct drm_device *dev)
> 
>  
> 
> I code, therefore I am
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] i915: Fix bug where screen brightness is not restored

[Takashi Iwai]

At Tue, 15 Nov 2011 11:58:38 -0800,
Kamal Mostafa wrote:
> 
> [1  <multipart/signed (7bit)>]
> [1.1  <text/plain; UTF-8 (quoted-printable)>]
> On Mon, 2011-11-14 at 18:42 -0800, Alex Davis wrote:
> > From: Alex Davis<alex14641@yahoo.com>
> > 
> > This patch fixes an issue where the screen would remain dark when
> > 
> > a key was pressed when the laptop lid was reopened or after the
> > laptop had gone into power-save mode.
> 
> [cross-posting to intel-gfx]
> 
> Keith, also note that Alex and I did respond to your request to test the
> already committed patch "drm/i915/panel: Always record the backlight
> level again (but cleverly)".  We both determined that patch does *not*
> fix the problem.  Alex's patch below does fix it.

My rough guess is the inconsistency of property taken during the
backlight disabled.  How about the patch below (untested!) in addition
to the fix in 3.2?


Takashi

---
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 499d4c0..21f60b7 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -326,7 +326,8 @@ static int intel_panel_update_status(struct backlight_device *bd)
 static int intel_panel_get_brightness(struct backlight_device *bd)
 {
 	struct drm_device *dev = bl_get_data(bd);
-	return intel_panel_get_backlight(dev);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	return dev_priv->backlight_level;
 }
 
 static const struct backlight_ops intel_panel_bl_ops = {

Re: [Intel-gfx] [PATCH] i915: Fix bug where screen brightness is not restored

[Kamal Mostafa]

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

On Tue, 2011-11-15 at 21:28 +0100, Takashi Iwai wrote:
> My rough guess is the inconsistency of property taken during the
> backlight disabled.  How about the patch below (untested!) in addition
> to the fix in 3.2?
> 
> 
> Takashi

Yes Takashi, your patch below (plus the already committed fix[0]) does
indeed fix the problem[1] for me.  Thanks!

Tested-by: Kamal Mostafa <kamal@canonical.com>

 -Kamal

[0] f52c619a590fa75276c07dfcaf380dee53e4ea4c
    drm/i915/panel: Always record the backlight level again (but cleverly)

[1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/872652



> ---
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 499d4c0..21f60b7 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -326,7 +326,8 @@ static int intel_panel_update_status(struct backlight_device *bd)
>  static int intel_panel_get_brightness(struct backlight_device *bd)
>  {
>  	struct drm_device *dev = bl_get_data(bd);
> -	return intel_panel_get_backlight(dev);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	return dev_priv->backlight_level;
>  }
>  
>  static const struct backlight_ops intel_panel_bl_ops = {
> 


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Intel-gfx] [PATCH] i915: Fix bug where screen brightness is not restored

[Takashi Iwai]

At Tue, 15 Nov 2011 13:54:44 -0800,
Kamal Mostafa wrote:
> 
> On Tue, 2011-11-15 at 21:28 +0100, Takashi Iwai wrote:
> > My rough guess is the inconsistency of property taken during the
> > backlight disabled.  How about the patch below (untested!) in addition
> > to the fix in 3.2?
> > 
> > 
> > Takashi
> 
> Yes Takashi, your patch below (plus the already committed fix[0]) does
> indeed fix the problem[1] for me.  Thanks!
> 
> Tested-by: Kamal Mostafa <kamal@canonical.com>

OK, I'll resend the patch with proper sign-off, etc.

thanks,

Takashi

> 
>  -Kamal
> 
> [0] f52c619a590fa75276c07dfcaf380dee53e4ea4c
>     drm/i915/panel: Always record the backlight level again (but cleverly)
> 
> [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/872652
> 
> 
> 
> > ---
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index 499d4c0..21f60b7 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -326,7 +326,8 @@ static int intel_panel_update_status(struct backlight_device *bd)
> >  static int intel_panel_get_brightness(struct backlight_device *bd)
> >  {
> >  	struct drm_device *dev = bl_get_data(bd);
> > -	return intel_panel_get_backlight(dev);
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	return dev_priv->backlight_level;
> >  }
> >  
> >  static const struct backlight_ops intel_panel_bl_ops = {
> > 
>