| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 31 May 2013 07:02:51 +0200
From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>
To: Liu Ying <liu.y.victor@...il.com>
Cc: Liu Ying <Ying.Liu@...escale.com>,
Florian Tobias Schandinat <FlorianSchandinat@....de>,
linux-fbdev@...r.kernel.org,
linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] backlight: Turn backlight on/off when necessary
On 10:31 Fri 31 May , Liu Ying wrote:
> 2013/5/30 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.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@...escale.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@...r.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Best Regards,
> Liu Ying
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists