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: 3887 bytes --]

Sorry, I have modified typing error.

2010/3/22 InKi Dae <daeinki@gmail.com>:
> I have 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
----> otherwise fb_notifier_call_chain first, this case means power down

> 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,