| Message ID | 1420925135-1589-1-git-send-email-jmmahler@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, 10 Jan 2015, Jeremiah Mahler <jmmahler@gmail.com> wrote: > Commit 6dda730e55f4 introduced a bug which resulted in inconsistent > brightness levels on different machines. If a suspended was entered > with the screen off some machines would resume with the screen > at minimum brightness and others at maximum brightness. > > The following commands can be used to produce this behavior. > > xset dpms force off > sleep 1 > sudo systemctl suspend > (resume ...) > > The root cause of this problem is a comparison which checks to see if > the backlight level is zero when the panel is enabled. If it is zero, > it is set to the maximum level. Unfortunately, not all machines have a > minimum level of zero. On those machines the level is left at the > minimum instead of begin set to the maximum. Good catch! I think part of the problem is that the userspace sets brightness to minimum before suspend, but apparently does not restore it after resume. The dmesg would confirm this. But I guess it doesn't matter, since we're pretty much stuck with having to do this anyway. > Fix the bug by updating the comparison to check for the minimum > backlight level instead of zero. > > Fixes: 6dda730e55f4 ("respect the VBT minimum backlight brightness") > Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com> > --- > drivers/gpu/drm/i915/intel_panel.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index 4d63839..4ef4d66 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -962,7 +962,7 @@ void intel_panel_enable_backlight(struct intel_connector *connector) > > WARN_ON(panel->backlight.max == 0); > > - if (panel->backlight.level == 0) { > + if (panel->backlight.level == panel->backlight.min) { Perhaps <= instead of == would be safest? BR, Jani. > panel->backlight.level = panel->backlight.max; > if (panel->backlight.device) > panel->backlight.device->props.brightness = > -- > 2.1.4 >
Jani, On Mon, Jan 12, 2015 at 12:31:09PM +0200, Jani Nikula wrote: > On Sat, 10 Jan 2015, Jeremiah Mahler <jmmahler@gmail.com> wrote: [...] > > I think part of the problem is that the userspace sets brightness to > minimum before suspend, but apparently does not restore it after > resume. The dmesg would confirm this. But I guess it doesn't matter, > since we're pretty much stuck with having to do this anyway. > I did notice it doing this. There were several calls to *_update_status as it was entering suspend which set it to the minimum. I am not familiar with the intricate details of this system but it seems like there must be a way to fix this. If the backlight can be powered off and back on with the correct level it seems like it should be possible when a suspend/resume is involved. [...] > > - if (panel->backlight.level == 0) { > > + if (panel->backlight.level == panel->backlight.min) { > > Perhaps <= instead of == would be safest? > We could do that too in case that corner case ever arises. [...] I will fix it up in v2.
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 4d63839..4ef4d66 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -962,7 +962,7 @@ void intel_panel_enable_backlight(struct intel_connector *connector) WARN_ON(panel->backlight.max == 0); - if (panel->backlight.level == 0) { + if (panel->backlight.level == panel->backlight.min) { panel->backlight.level = panel->backlight.max; if (panel->backlight.device) panel->backlight.device->props.brightness =
Commit 6dda730e55f4 introduced a bug which resulted in inconsistent brightness levels on different machines. If a suspended was entered with the screen off some machines would resume with the screen at minimum brightness and others at maximum brightness. The following commands can be used to produce this behavior. xset dpms force off sleep 1 sudo systemctl suspend (resume ...) The root cause of this problem is a comparison which checks to see if the backlight level is zero when the panel is enabled. If it is zero, it is set to the maximum level. Unfortunately, not all machines have a minimum level of zero. On those machines the level is left at the minimum instead of begin set to the maximum. Fix the bug by updating the comparison to check for the minimum backlight level instead of zero. Fixes: 6dda730e55f4 ("respect the VBT minimum backlight brightness") Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com> --- drivers/gpu/drm/i915/intel_panel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)