| Message ID | 1431376325-25741-1-git-send-email-seanpaul@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sean, On 5/12/2015 5:32 AM, Sean Paul wrote: > Don't clear the backlight level when we're going into suspend or > blanking. Instead, just temporarily set the level to 0 so we retain > the value when we resume. Could you describe what the problem is? I wrote same code in other backlight drivers. So maybe I need to look into them as well. > --- > drivers/video/backlight/lp855x_bl.c | 18 ++++++------------ > 1 file changed, 6 insertions(+), 12 deletions(-) > > diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c > index 08ae72f..74daf7c 100644 > --- a/drivers/video/backlight/lp855x_bl.c > +++ b/drivers/video/backlight/lp855x_bl.c > @@ -257,21 +257,15 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br) > static int lp855x_bl_update_status(struct backlight_device *bl) > { > struct lp855x *lp = bl_get_data(bl); > + int brightness = bl->props.brightness; > > if (bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK)) > - bl->props.brightness = 0; > + brightness = 0; After that, this driver can't show exact brightness value when the application tries to read 'brightness' through the sysfs. The backlight is off but read value of 'brightness' is non-zero. https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/video/backlight/backlight.c?id=refs/tags/v4.1-rc3#n159 Best regards, Milo -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sean, On 5/14/2015 4:33 AM, Sean Paul wrote: > > > On Mon, May 11, 2015 at 3:48 PM, Kim, Milo <milo.kim@ti.com > <mailto:milo.kim@ti.com>> wrote: > > Hi Sean, > > On 5/12/2015 5:32 AM, Sean Paul wrote: > > Don't clear the backlight level when we're going into suspend or > blanking. Instead, just temporarily set the level to 0 so we retain > the value when we resume. > > > Could you describe what the problem is? I wrote same code in other > backlight drivers. So maybe I need to look into them as well. > > > Hi Milo, > Well, I'm not sure what the expected behavior is, but our userspace > expects the driver to maintain its brightness across suspend/resume. The > way things are coded now, the driver will clear the brightness value on > suspend such that when we resume, the backlight is restored to 0. Got it. Your patch makes sense. Thanks for catching this. Best regards, Milo -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 5/12/2015 5:32 AM, Sean Paul wrote: > Don't clear the backlight level when we're going into suspend or > blanking. Instead, just temporarily set the level to 0 so we retain > the value when we resume. > > Reported-by: Benson Leung <bleung@chromium.org> > Tested-by: Stephen Barber <smbarber@chromium.org> > Signed-off-by: Sean Paul <seanpaul@chromium.org> Acked-by: Milo Kim <milo.kim@ti.com> -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 11, 2015 at 1:32 PM, Sean Paul <seanpaul@chromium.org> wrote: > Don't clear the backlight level when we're going into suspend or > blanking. Instead, just temporarily set the level to 0 so we retain > the value when we resume. > > Reported-by: Benson Leung <bleung@chromium.org> > Tested-by: Stephen Barber <smbarber@chromium.org> > Signed-off-by: Sean Paul <seanpaul@chromium.org> Reviewed-by: Benson Leung <bleung@chromium.org>
On Mon, 11 May 2015, Sean Paul wrote: > Don't clear the backlight level when we're going into suspend or > blanking. Instead, just temporarily set the level to 0 so we retain > the value when we resume. > > Reported-by: Benson Leung <bleung@chromium.org> > Tested-by: Stephen Barber <smbarber@chromium.org> > Signed-off-by: Sean Paul <seanpaul@chromium.org> > --- > drivers/video/backlight/lp855x_bl.c | 18 ++++++------------ > 1 file changed, 6 insertions(+), 12 deletions(-) Applied with Acks, thanks. > diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c > index 08ae72f..74daf7c 100644 > --- a/drivers/video/backlight/lp855x_bl.c > +++ b/drivers/video/backlight/lp855x_bl.c > @@ -257,21 +257,15 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br) > static int lp855x_bl_update_status(struct backlight_device *bl) > { > struct lp855x *lp = bl_get_data(bl); > + int brightness = bl->props.brightness; > > if (bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK)) > - bl->props.brightness = 0; > + brightness = 0; > > - if (lp->mode == PWM_BASED) { > - int br = bl->props.brightness; > - int max_br = bl->props.max_brightness; > - > - lp855x_pwm_ctrl(lp, br, max_br); > - > - } else if (lp->mode == REGISTER_BASED) { > - u8 val = bl->props.brightness; > - > - lp855x_write_byte(lp, lp->cfg->reg_brightness, val); > - } > + if (lp->mode == PWM_BASED) > + lp855x_pwm_ctrl(lp, brightness, bl->props.max_brightness); > + else if (lp->mode == REGISTER_BASED) > + lp855x_write_byte(lp, lp->cfg->reg_brightness, (u8)brightness); > > return 0; > }
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c index 08ae72f..74daf7c 100644 --- a/drivers/video/backlight/lp855x_bl.c +++ b/drivers/video/backlight/lp855x_bl.c @@ -257,21 +257,15 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br) static int lp855x_bl_update_status(struct backlight_device *bl) { struct lp855x *lp = bl_get_data(bl); + int brightness = bl->props.brightness; if (bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK)) - bl->props.brightness = 0; + brightness = 0; - if (lp->mode == PWM_BASED) { - int br = bl->props.brightness; - int max_br = bl->props.max_brightness; - - lp855x_pwm_ctrl(lp, br, max_br); - - } else if (lp->mode == REGISTER_BASED) { - u8 val = bl->props.brightness; - - lp855x_write_byte(lp, lp->cfg->reg_brightness, val); - } + if (lp->mode == PWM_BASED) + lp855x_pwm_ctrl(lp, brightness, bl->props.max_brightness); + else if (lp->mode == REGISTER_BASED) + lp855x_write_byte(lp, lp->cfg->reg_brightness, (u8)brightness); return 0; }