On 01/23/2014 01:44 PM, Jingoo Han wrote:
On Wednesday, January 22, 2014 6:36 PM, Jani Nikula wrote:
On Mon, 20 Jan 2014, Liu Ying Ying.Liu@freescale.com 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 blanking one of the framebuffers may turn the backlight off for all the other framebuffers, since they are likely being active to display something. This patch adds some logics to record each framebuffer's backlight usage to determine the backlight device use count and whether the backlight should be turned on or off. To be more specific, only one unblank operation on a certain blanked framebuffer may increase the backlight device's use count by one, while one blank operation on a certain unblanked framebuffer may decrease the use count by one, because the userspace is likely to unblank a unblanked framebuffer or blank a blanked framebuffer.
Signed-off-by: Liu Ying Ying.Liu@freescale.com
v1 can be found at https://lkml.org/lkml/2013/5/30/139
v1->v2:
- Make the commit message be more specific about the condition in which backlight device use count can be increased/decreased.
- Correct the setting for bd->props.fb_blank.
drivers/video/backlight/backlight.c | 28 +++++++++++++++++++++------- include/linux/backlight.h | 6 ++++++ 2 files changed, 27 insertions(+), 7 deletions(-)
[.....]
Anything backlight worries me a little, and there are actually three changes bundled into one patch here:
Changing bd->props.state and bd->props.fb_blank only when use_count changes from 0->1 or 1->0.
Calling backlight_update_status() only with the above change, and not on all notifier callbacks.
Setting bd->props.fb_blank always to either FB_BLANK_UNBLANK or FB_BLANK_POWERDOWN instead of *(int *)evdata->data.
Since I have already post v3(https://lkml.org/lkml/2014/1/22/126) to change the setting for bd->props.fb_blank, the idea of the 3rd point is not very appropriate any more.
The rationale in the commit message seems plausible, and AFAICT the code does what it says on the box, so for that (and for that alone) you can have my
Reviewed-by: Jani Nikula jani.nikula@intel.com
*BUT* it would be laborous to figure out whether this change in behaviour might regress some drivers. I'm just punting on that. And that brings us back to the three changes above - in a bisect POV it might be helpful to split the patch up. Up to the maintainers.
I agree with Jani Nikula's opinion. Please split this patch into three patches as above mentioned.
I am open to split the patch up. However, IMHO, this patch is somewhat self-contained. For example, if we try to create 2 patches for the 1st point and the 2nd point Jani mentioned, one patch would invent the use_count and call backlight_update_status() on all notifier callbacks(just ignore the use_count). Do you think this is a good patch?
It also doesn't look straightforward for me to create 2 patches for the 1st point and the 2nd point.
Please advice.
Regards, Liu Ying