[PATCH] backlight: Turn backlight on/off when necessary

[PATCH] backlight: Turn backlight on/off when necessary

[Liu Ying]

We don't have to turn backlight on/off everytime a blanking
or unblanking event comes because the backlight status may have
already been what we want. Another thought is that one backlight
device may be shared by multiple framebuffers. We don't hope that
blanking one of the framebuffers would turn the backlight off for
all the other framebuffers, since they are likely active to show
display content. This patch adds logic to record each framebuffer's
backlight status to determine the backlight device use count and
whether the backlight should be turned on or off.

Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
 drivers/video/backlight/backlight.c |   23 +++++++++++++++++------
 include/linux/backlight.h           |    6 ++++++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index c74e7aa..97ea2b8 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -31,13 +31,14 @@ static const char *const backlight_types[] = {
 			   defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
 /* This callback gets called when something important happens inside a
  * framebuffer driver. We're looking if that important event is blanking,
- * and if it is, we're switching backlight power as well ...
+ * and if it is and necessary, we're switching backlight power as well ...
  */
 static int fb_notifier_callback(struct notifier_block *self,
 				unsigned long event, void *data)
 {
 	struct backlight_device *bd;
 	struct fb_event *evdata = data;
+	int node = evdata->info->node;
 
 	/* If we aren't interested in this event, skip it immediately ... */
 	if (event != FB_EVENT_BLANK && event != FB_EVENT_CONBLANK)
@@ -49,11 +50,21 @@ static int fb_notifier_callback(struct notifier_block *self,
 		if (!bd->ops->check_fb ||
 		    bd->ops->check_fb(bd, evdata->info)) {
 			bd->props.fb_blank = *(int *)evdata->data;
-			if (bd->props.fb_blank = FB_BLANK_UNBLANK)
-				bd->props.state &= ~BL_CORE_FBBLANK;
-			else
-				bd->props.state |= BL_CORE_FBBLANK;
-			backlight_update_status(bd);
+			if (bd->props.fb_blank = FB_BLANK_UNBLANK &&
+			    !bd->fb_bl_on[node]) {
+				bd->fb_bl_on[node] = true;
+				if (!bd->use_count++) {
+					bd->props.state &= ~BL_CORE_FBBLANK;
+					backlight_update_status(bd);
+				}
+			} else if (bd->props.fb_blank != FB_BLANK_UNBLANK &&
+				   bd->fb_bl_on[node]) {
+				bd->fb_bl_on[node] = false;
+				if (!(--bd->use_count)) {
+					bd->props.state |= BL_CORE_FBBLANK;
+					backlight_update_status(bd);
+				}
+			}
 		}
 	mutex_unlock(&bd->ops_lock);
 	return 0;
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index da9a082..5de71a0 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -9,6 +9,7 @@
 #define _LINUX_BACKLIGHT_H
 
 #include <linux/device.h>
+#include <linux/fb.h>
 #include <linux/mutex.h>
 #include <linux/notifier.h>
 
@@ -101,6 +102,11 @@ struct backlight_device {
 	struct notifier_block fb_notif;
 
 	struct device dev;
+
+	/* Multiple framebuffers may share one backlight device */
+	bool fb_bl_on[FB_MAX];
+
+	int use_count;
 };
 
 static inline void backlight_update_status(struct backlight_device *bd)
-- 
1.7.1

Re: [PATCH] backlight: Turn backlight on/off when necessary

[Jean-Christophe PLAGNIOL-VILLARD]

On 16:13 Thu 30 May     , Liu Ying wrote:
> We don't have to turn backlight on/off everytime a blanking
> or unblanking event comes because the backlight status may have
> already been what we want. Another thought is that one backlight
> device may be shared by multiple framebuffers. We don't hope that
> blanking one of the framebuffers would turn the backlight off for
> all the other framebuffers, since they are likely active to show
> display content. This patch adds logic to record each framebuffer's
> backlight status to determine the backlight device use count and
> whether the backlight should be turned on or off.
> 
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> ---
>  drivers/video/backlight/backlight.c |   23 +++++++++++++++++------
>  include/linux/backlight.h           |    6 ++++++
>  2 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index c74e7aa..97ea2b8 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -31,13 +31,14 @@ static const char *const backlight_types[] = {
>  			   defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
>  /* This callback gets called when something important happens inside a
>   * framebuffer driver. We're looking if that important event is blanking,
> - * and if it is, we're switching backlight power as well ...
> + * and if it is and necessary, we're switching backlight power as well ...
>   */
>  static int fb_notifier_callback(struct notifier_block *self,
>  				unsigned long event, void *data)
>  {
>  	struct backlight_device *bd;
>  	struct fb_event *evdata = data;
> +	int node = evdata->info->node;
>  
>  	/* If we aren't interested in this event, skip it immediately ... */
>  	if (event != FB_EVENT_BLANK && event != FB_EVENT_CONBLANK)
> @@ -49,11 +50,21 @@ static int fb_notifier_callback(struct notifier_block *self,
>  		if (!bd->ops->check_fb ||
>  		    bd->ops->check_fb(bd, evdata->info)) {
>  			bd->props.fb_blank = *(int *)evdata->data;
> -			if (bd->props.fb_blank = FB_BLANK_UNBLANK)
> -				bd->props.state &= ~BL_CORE_FBBLANK;
> -			else
> -				bd->props.state |= BL_CORE_FBBLANK;
> -			backlight_update_status(bd);
> +			if (bd->props.fb_blank = FB_BLANK_UNBLANK &&
> +			    !bd->fb_bl_on[node]) {
> +				bd->fb_bl_on[node] = true;
> +				if (!bd->use_count++) {
> +					bd->props.state &= ~BL_CORE_FBBLANK;
> +					backlight_update_status(bd);
> +				}
> +			} else if (bd->props.fb_blank != FB_BLANK_UNBLANK &&
> +				   bd->fb_bl_on[node]) {
> +				bd->fb_bl_on[node] = false;
> +				if (!(--bd->use_count)) {
> +					bd->props.state |= BL_CORE_FBBLANK;
> +					backlight_update_status(bd);
> +				}
> +			}
>  		}
>  	mutex_unlock(&bd->ops_lock);
>  	return 0;
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index da9a082..5de71a0 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -9,6 +9,7 @@
>  #define _LINUX_BACKLIGHT_H
>  
>  #include <linux/device.h>
> +#include <linux/fb.h>
>  #include <linux/mutex.h>
>  #include <linux/notifier.h>
>  
> @@ -101,6 +102,11 @@ struct backlight_device {
>  	struct notifier_block fb_notif;
>  
>  	struct device dev;
> +
> +	/* Multiple framebuffers may share one backlight device */
> +	bool fb_bl_on[FB_MAX];


I don't like such array at all

I understand the fact you will have only on hw backlight for x fb or overlay
but have a static on no

if you want to track all user create a strcut and register it or do more
simple just as a int to count the number of user and shut down it if 0 and
enable it otherwise

Best Regards,
J.
> +
> +	int use_count;
>  };
>  
>  static inline void backlight_update_status(struct backlight_device *bd)
> -- 
> 1.7.1
> 
> 
> --
> 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

Re: [PATCH] backlight: Turn backlight on/off when necessary

[Jean-Christophe PLAGNIOL-VILLARD]

On 10:31 Fri 31 May     , Liu Ying wrote:
>    2013/5/30 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
>      On 16:13 Thu 30 May     , Liu Ying wrote:
>      > We don't have to turn backlight on/off everytime a blanking
>      > or unblanking event comes because the backlight status may have
>      > already been what we want. Another thought is that one backlight
>      > device may be shared by multiple framebuffers. We don't hope that
>      > blanking one of the framebuffers would turn the backlight off for
>      > all the other framebuffers, since they are likely active to show
>      > display content. This patch adds logic to record each framebuffer's
>      > backlight status to determine the backlight device use count and
>      > whether the backlight should be turned on or off.
>      >
>      > Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
>      > ---
>      >  drivers/video/backlight/backlight.c |   23 +++++++++++++++++------
>      >  include/linux/backlight.h           |    6 ++++++
>      >  2 files changed, 23 insertions(+), 6 deletions(-)
>      >
>      > diff --git a/drivers/video/backlight/backlight.c
>      b/drivers/video/backlight/backlight.c
>      > index c74e7aa..97ea2b8 100644
>      > --- a/drivers/video/backlight/backlight.c
>      > +++ b/drivers/video/backlight/backlight.c
>      > @@ -31,13 +31,14 @@ static const char *const backlight_types[] = {
>      >                        
>       defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
>      >  /* This callback gets called when something important happens inside
>      a
>      >   * framebuffer driver. We're looking if that important event is
>      blanking,
>      > - * and if it is, we're switching backlight power as well ...
>      > + * and if it is and necessary, we're switching backlight power as
>      well ...
>      >   */
>      >  static int fb_notifier_callback(struct notifier_block *self,
>      >                               unsigned long event, void *data)
>      >  {
>      >       struct backlight_device *bd;
>      >       struct fb_event *evdata = data;
>      > +     int node = evdata->info->node;
>      >
>      >       /* If we aren't interested in this event, skip it immediately
>      ... */
>      >       if (event != FB_EVENT_BLANK && event != FB_EVENT_CONBLANK)
>      > @@ -49,11 +50,21 @@ static int fb_notifier_callback(struct
>      notifier_block *self,
>      >               if (!bd->ops->check_fb ||
>      >                   bd->ops->check_fb(bd, evdata->info)) {
>      >                       bd->props.fb_blank = *(int *)evdata->data;
>      > -                     if (bd->props.fb_blank = FB_BLANK_UNBLANK)
>      > -                             bd->props.state &= ~BL_CORE_FBBLANK;
>      > -                     else
>      > -                             bd->props.state |= BL_CORE_FBBLANK;
>      > -                     backlight_update_status(bd);
>      > +                     if (bd->props.fb_blank = FB_BLANK_UNBLANK &&
>      > +                         !bd->fb_bl_on[node]) {
>      > +                             bd->fb_bl_on[node] = true;
>      > +                             if (!bd->use_count++) {
>      > +                                     bd->props.state &>      ~BL_CORE_FBBLANK;
>      > +                                     backlight_update_status(bd);
>      > +                             }
>      > +                     } else if (bd->props.fb_blank !>      FB_BLANK_UNBLANK &&
>      > +                                bd->fb_bl_on[node]) {
>      > +                             bd->fb_bl_on[node] = false;
>      > +                             if (!(--bd->use_count)) {
>      > +                                     bd->props.state |>      BL_CORE_FBBLANK;
>      > +                                     backlight_update_status(bd);
>      > +                             }
>      > +                     }
>      >               }
>      >       mutex_unlock(&bd->ops_lock);
>      >       return 0;
>      > diff --git a/include/linux/backlight.h b/include/linux/backlight.h
>      > index da9a082..5de71a0 100644
>      > --- a/include/linux/backlight.h
>      > +++ b/include/linux/backlight.h
>      > @@ -9,6 +9,7 @@
>      >  #define _LINUX_BACKLIGHT_H
>      >
>      >  #include <linux/device.h>
>      > +#include <linux/fb.h>
>      >  #include <linux/mutex.h>
>      >  #include <linux/notifier.h>
>      >
>      > @@ -101,6 +102,11 @@ struct backlight_device {
>      >       struct notifier_block fb_notif;
>      >
>      >       struct device dev;
>      > +
>      > +     /* Multiple framebuffers may share one backlight device */
>      > +     bool fb_bl_on[FB_MAX];
> 
>      I don't like such array at all
> 
>      I understand the fact you will have only on hw backlight for x fb or
>      overlay
>      but have a static on no
> 
>     
>    My board has two LVDS display panels. They share one PWM backlight.
>    The framebuffer HW engine may drive a background framebuffer and an
>    overlay framebuffer on one panel, and only one background framebuffer on
>    the other panel. The three framebuffers may be active simultaneously.
>     
> 
>      if you want to track all user create a strcut and register it or do more
>      simple just as a int to count the number of user and shut down it if 0
>      and
>      enable it otherwise
> 
>    Users may unblank a framebuffer for multiple times continuously and then
>    trigger a blanking operation.
>    If that framebuffer is the only user of the backlight, the backlight will
>    be turned off after the blanking operation.
>    This is the behavior before this patch is applied to kernel. And, this
>    patch doesn't change the behavior here.
>    So, it seems that it is reasonable to record backlight status(on or off)
>    for framebuffers. And, I use a straightforward array for the recording. 
>    I thought about changing to use a list instead for the recording, but it
>    appears to me it would take more CPU cycles to search and update entries.
>    It is basically a kind of space-against-speed trade-off.
>    You probably have already provided me a better way to do this, but it
>    looks I didn't catch it. If this is the case, would you please shed more
>    light on this? Thanks!

so just use a int

check who we do for clk_enable/disable on at91

arch/arm/mach-at91/clock.c

Best Regards,
J.
> 
>      Best Regards,
>      J.
>      > +
>      > +     int use_count;
>      >  };
>      >
>      >  static inline void backlight_update_status(struct backlight_device
>      *bd)
>      > --
>      > 1.7.1
>      >
>      >
>      > --
>      > 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
> 
>    --
>    Best Regards,
>    Liu Ying