Re: [PATCH] fixed sparkling issue on lcd panel when fb_blank mode is changed.

[InKi Dae <daeinki@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3614 bytes --]

I has modified my patch.

this patch would change order only in case of FB_BLANK_POWERDOWN
with Richard's opinion.

if requested blank mode is less then FB_BLANK_POWERDOWN then
it would call device specific fb_blank and then fb_notifer_call_chan
otherwise fb_notifier_call_chain first, this case means power up
and display controller should become on earlier then other devices
like lcd panel.

Please review this path again.

signed-off-by : InKi Dae <inki.dae@samsung.com>

Best Regards,
InKi Dae.

2010/3/22 InKi Dae <daeinki@gmail.com>:
> yes, you are right.
>
> when fb_blank mode is changed to FB_BLANK_NORMAL or
> FB_BLANK_UNBLANK(power down -> power up)
> it has a possibility that we would encounter same issue.
>
> I'd try to modify my patch to be considered with your opinion and will
> send it again.
> Thank you for your answer.
>
> Best Regards,
> InKi Dae.
>
> 2010/3/19 Richard Purdie <rpurdie@rpsys.net>:
>> Hi,
>>
>> On Fri, 2010-03-19 at 13:36 +0900, InKi Dae wrote:
>>> This issue is a problem that lcd panel is spakled when fb_blank mode is changed
>>> From FB_BLANK_UNBLANK to FB_BLANK_POWER or FB_BLANK_POWER to FB_BLANK_UNBLANK.
>>>
>>> In case of FB_BLANK_UNBLANK, screen = on, vsync = on, hsync = on and
>>> For FB_BLANK_POWERDOWN screen = blanked, vsync = off, hsync = off
>>> So like this when fb_blank mode becomes FB_BLANK_POWERDOWN,
>>> Display controller and lcd panel should become off to reduce power consumption.
>>>
>>> Let's see fb_blank function of fbmem.c file.
>>>
>>> fb_blank()
>>> {
>>>     if (info->fbops->fb_blank)
>>>             ret = info->fbops->fb_blank(blank, info(
>>>     if (!ret)
>>>             fb_notifier_call_chain(FB_EVENT_BLANK, &event);
>>> }
>>>
>>> This problem is because this code calls fb_blank (fb_blank of device
>>> specific framebuffer driver)
>>> Earlier then fb_notifier_call_chain.
>>>
>>> For example,
>>> Device specific fb_blank function is registered to generic framebuffer
>>> driver through register_framebuffer function
>>> And fb_notifier_call_chain calls any callback function registered to
>>> lcd class(lcd.c) through fb_notifier_callback function.
>>> So if fb_blank mode is changed to FB_BLANK_POWERDOWN then display
>>> controller would become off(clock disable)
>>> On the other hand, lcd panel would still be on. at this time, some
>>> situation on lcd panel occers like sparkling
>>> Because video clock to be delivered to ldi module of lcd panel is disabled also.
>>>
>>> This patch get fb_notifier_call_chain to be called earlier then fb_blank.
>>> Please, review this patch.
>>
>> So to summarise the problem more succinctly the LCD and the display
>> controllers power off in the wrong order causing visual defects
>> (sparkling) on the display.
>>
>> I was a bit worried about event order and loops with this change so I
>> checked and there are three consumers of these events in the kernel:
>> consoles, LCD and backlight. None of them have a problem with this
>> change of order for power down so from that point of view I'm ok with
>> the change.
>>
>> However this function also covers powerup from a screen blank to an
>> active screen. When powering up it makes sense to power up the video
>> device first, then the LCD, backlight and console as per the current
>> code, otherwise I can imagine "sparkling" upon powerup.
>>
>> So it looks like we need to change order only on powerdown, not upon
>> resume, otherwise we'll get further regression reports on poweron?
>>
>> Cheers,
>>
>> Richard
>>
>>
>>
>>
>>
>

[-- Attachment #2: fbmem.patch --]
[-- Type: application/octet-stream, Size: 1378 bytes --]

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index a15b44e..d80e097 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1007,24 +1007,33 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 
 int
 fb_blank(struct fb_info *info, int blank)
-{	
- 	int ret = -EINVAL;
+{
+	struct fb_event event;
+	int ret = -EINVAL;
 
- 	if (blank > FB_BLANK_POWERDOWN)
- 		blank = FB_BLANK_POWERDOWN;
+	if (blank > FB_BLANK_POWERDOWN)
+		blank = FB_BLANK_POWERDOWN;
 
-	if (info->fbops->fb_blank)
- 		ret = info->fbops->fb_blank(blank, info);
+	event.info = info;
+	event.data = &blank;
 
- 	if (!ret) {
-		struct fb_event event;
+	if (blank <  FB_BLANK_POWERDOWN) {
+		if (info->fbops->fb_blank)
+			ret = info->fbops->fb_blank(blank, info);
 
-		event.info = info;
-		event.data = &blank;
-		fb_notifier_call_chain(FB_EVENT_BLANK, &event);
+		ret = fb_notifier_call_chain(FB_EVENT_BLANK, &event);
+		if ((ret & NOTIFY_STOP_MASK) == NOTIFY_STOP_MASK)
+			printk(KERN_ERR "notifier_call failed.\n");
+	} else {
+		ret = fb_notifier_call_chain(FB_EVENT_BLANK, &event);
+		if ((ret & NOTIFY_STOP_MASK) == NOTIFY_STOP_MASK)
+			printk(KERN_ERR "notifier_call failed.\n");
+
+		if (info->fbops->fb_blank)
+			ret = info->fbops->fb_blank(blank, info);
 	}
 
- 	return ret;
+	return ret;
 }
 
 static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,