* Re: [PATCH] backlight/lp855x: Don't clear level on suspend/blank
2015-05-11 20:32 [PATCH] backlight/lp855x: Don't clear level on suspend/blank Sean Paul
@ 2015-05-11 22:48 ` Kim, Milo
2015-05-13 23:31 ` Kim, Milo
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Kim, Milo @ 2015-05-11 22:48 UTC (permalink / raw)
To: linux-fbdev
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] backlight/lp855x: Don't clear level on suspend/blank
2015-05-11 20:32 [PATCH] backlight/lp855x: Don't clear level on suspend/blank Sean Paul
2015-05-11 22:48 ` Kim, Milo
@ 2015-05-13 23:31 ` Kim, Milo
2015-05-13 23:32 ` Kim, Milo
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Kim, Milo @ 2015-05-13 23:31 UTC (permalink / raw)
To: linux-fbdev
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] backlight/lp855x: Don't clear level on suspend/blank
2015-05-11 20:32 [PATCH] backlight/lp855x: Don't clear level on suspend/blank Sean Paul
2015-05-11 22:48 ` Kim, Milo
2015-05-13 23:31 ` Kim, Milo
@ 2015-05-13 23:32 ` Kim, Milo
2015-05-13 23:40 ` Benson Leung
2015-05-14 7:25 ` Lee Jones
4 siblings, 0 replies; 6+ messages in thread
From: Kim, Milo @ 2015-05-13 23:32 UTC (permalink / raw)
To: linux-fbdev
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] backlight/lp855x: Don't clear level on suspend/blank
2015-05-11 20:32 [PATCH] backlight/lp855x: Don't clear level on suspend/blank Sean Paul
` (2 preceding siblings ...)
2015-05-13 23:32 ` Kim, Milo
@ 2015-05-13 23:40 ` Benson Leung
2015-05-14 7:25 ` Lee Jones
4 siblings, 0 replies; 6+ messages in thread
From: Benson Leung @ 2015-05-13 23:40 UTC (permalink / raw)
To: linux-fbdev
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>
--
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] backlight/lp855x: Don't clear level on suspend/blank
2015-05-11 20:32 [PATCH] backlight/lp855x: Don't clear level on suspend/blank Sean Paul
` (3 preceding siblings ...)
2015-05-13 23:40 ` Benson Leung
@ 2015-05-14 7:25 ` Lee Jones
4 siblings, 0 replies; 6+ messages in thread
From: Lee Jones @ 2015-05-14 7:25 UTC (permalink / raw)
To: linux-fbdev
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;
> }
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 6+ messages in thread