i810fb_blank

i810fb_blank

[Lucas Correia Villa Real]

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

Hi!

While looking at i810fb_blank() on 2.4 and 2.6, I've seen that there's a 
difference on how pwr mask is handled: 2.4 unsets the pwr bit on 
FB_BLANK_VSYNC_SUSPEND and FB_BLANK_HSYNC_SUSPEND commands, 
while 2.6 turns that bit on. Was that modification expected, or was it a 
typo? The attached patch shows the code in question.

Thanks!

-- 
Lucas
powered by /dev/dsp

[-- Attachment #2: 2.6.14.3-i810-suspend.patch --]
[-- Type: text/x-diff, Size: 554 bytes --]

Signed-off-by: Lucas Correia Villa Real <lucasvr@gobolinux.org>

--- linux-2.6.14.3/drivers/video/i810/i810_main.c.orig	2005-12-03 16:44:06.000000000 -0200
+++ linux-2.6.14.3/drivers/video/i810/i810_main.c	2005-12-03 16:45:55.000000000 -0200
@@ -1359,12 +1359,12 @@ static int i810fb_blank (int blank_mode,
 		break;
 	case FB_BLANK_VSYNC_SUSPEND:
 		mode = STANDBY;
-		pwr |= 1;
+		pwr &= ~1;
 		scr_off = OFF;
 		break;
 	case FB_BLANK_HSYNC_SUSPEND:
 		mode = SUSPEND;
-		pwr |= 1;
+		pwr &= ~1;
 		scr_off = OFF;
 		break;
 	case FB_BLANK_POWERDOWN:

Re: i810fb_blank

[Antonino A. Daplas]

Lucas Correia Villa Real wrote:
> Hi!
> 
> While looking at i810fb_blank() on 2.4 and 2.6, I've seen that there's a 
> difference on how pwr mask is handled: 2.4 unsets the pwr bit on 
> FB_BLANK_VSYNC_SUSPEND and FB_BLANK_HSYNC_SUSPEND commands, 
> while 2.6 turns that bit on. Was that modification expected, or was it a 
> typo? The attached patch shows the code in question.

That's the expected change.  We only want to completely power down
the adapter on the deepest powersaving mode.

Are you experiencing any problems with the new/old code?

Tony


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: Re: i810fb_blank

[Lucas Correia Villa Real]

On Thursday 15 December 2005 21:15, Antonino A. Daplas wrote:
> Lucas Correia Villa Real wrote:
> > Hi!
> >
> > While looking at i810fb_blank() on 2.4 and 2.6, I've seen that
> > there's a difference on how pwr mask is handled: 2.4 unsets
> > the pwr bit on FB_BLANK_VSYNC_SUSPEND and
> > FB_BLANK_HSYNC_SUSPEND commands, while 2.6 turns that bit on.
> > Was that modification expected, or was it a typo? The attached
> > patch shows the code in question.
>
> That's the expected change.  We only want to completely power
> down the adapter on the deepest powersaving mode.
>
> Are you experiencing any problems with the new/old code?

A fellow worker has been working with software suspend 2 on a set 
top box which has an i810 card. The suspend command was not making 
the card to power off, so I looked at the code to give him a help, 
when I saw that different behaviour from 2.4 (2.4 hasn't been 
tested there, though).

I haven't seen this guy since then, so I don't know if that change 
did the trick for him. I'll let you know as soon as possible.

Thanks for the help!

-- 
Lucas
powered by /dev/dsp


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: Re: i810fb_blank

[Antonino A. Daplas]

Lucas Correia Villa Real wrote:
> On Thursday 15 December 2005 21:15, Antonino A. Daplas wrote:
>> Lucas Correia Villa Real wrote:
>>> Hi!
>>>
>>> While looking at i810fb_blank() on 2.4 and 2.6, I've seen that
>>> there's a difference on how pwr mask is handled: 2.4 unsets
>>> the pwr bit on FB_BLANK_VSYNC_SUSPEND and
>>> FB_BLANK_HSYNC_SUSPEND commands, while 2.6 turns that bit on.
>>> Was that modification expected, or was it a typo? The attached
>>> patch shows the code in question.
>> That's the expected change.  We only want to completely power
>> down the adapter on the deepest powersaving mode.
>>
>> Are you experiencing any problems with the new/old code?
> 
> A fellow worker has been working with software suspend 2 on a set 
> top box which has an i810 card. The suspend command was not making 
> the card to power off, so I looked at the code to give him a help, 
> when I saw that different behaviour from 2.4 (2.4 hasn't been 
> tested there, though).
> 
> I haven't seen this guy since then, so I don't know if that change 
> did the trick for him. I'll let you know as soon as possible.

I have neglected the suspend and resume hooks of i810fb so it's not
updated with the current code.  I don't have the hardware so I cannot
test.  Can you tell your friend to also try this patch if he cannot
solve the problem?

Tony

diff --git a/drivers/video/i810/i810_main.c b/drivers/video/i810/i810_main.c
index 4954a8e..563abc9 100644
--- a/drivers/video/i810/i810_main.c
+++ b/drivers/video/i810/i810_main.c
@@ -1558,35 +1558,28 @@ static int i810fb_suspend(struct pci_dev
 {
 	struct fb_info *info = pci_get_drvdata(dev);
 	struct i810fb_par *par = info->par;
-	int blank = 0, prev_state = par->cur_state;
-
-	if (state.event == prev_state)
-		return 0;
 
 	par->cur_state = state.event;
 
-	switch (state.event) {
-	case 1:
-		blank = VESA_VSYNC_SUSPEND;
-		break;
-	case 2:
-		blank = VESA_HSYNC_SUSPEND;
-		break;
-	case 3:
-		blank = VESA_POWERDOWN;
-		break;
-	default:
-		return -EINVAL;
-	}
-	info->fbops->fb_blank(blank, info);
-
-	if (!prev_state) { 
-		agp_unbind_memory(par->i810_gtt.i810_fb_memory);
-		agp_unbind_memory(par->i810_gtt.i810_cursor_memory);
-		pci_disable_device(dev);
+	if (state.event == PM_EVENT_FREEZE) {
+		dev->dev.power.power_state = state;
+		return 0;
 	}
+
+	acquire_console_sem();
+	fb_set_suspend(info, 1);
+
+	if (info->fbops->fb_sync)
+		info->fbops->fb_sync(info);
+
+	i810fb_blank(FB_BLANK_POWERDOWN, info);
+	agp_unbind_memory(par->i810_gtt.i810_fb_memory);
+	agp_unbind_memory(par->i810_gtt.i810_cursor_memory);
+	
 	pci_save_state(dev);
+	pci_disable_device(dev);
 	pci_set_power_state(dev, pci_choose_state(dev, state));
+	release_console_sem();
 
 	return 0;
 }
@@ -1595,22 +1588,28 @@ static int i810fb_resume(struct pci_dev 
 {
 	struct fb_info *info = pci_get_drvdata(dev);
 	struct i810fb_par *par = info->par;
+	int cur_state = par->cur_state;
 
-	if (par->cur_state == 0)
+	par->cur_state = PM_EVENT_ON;
+
+	if (cur_state == PM_EVENT_FREEZE) {
+		pci_set_power_state(dev, PCI_D0);
 		return 0;
+	}
 
-	pci_restore_state(dev);
+	acquire_console_sem();
 	pci_set_power_state(dev, PCI_D0);
+	pci_restore_state(dev);
 	pci_enable_device(dev);
+	pci_set_master(dev);
 	agp_bind_memory(par->i810_gtt.i810_fb_memory,
 			par->fb.offset);
 	agp_bind_memory(par->i810_gtt.i810_cursor_memory,
 			par->cursor_heap.offset);
-
+	i810fb_set_par(info);
+	fb_set_suspend (info, 0);
 	info->fbops->fb_blank(VESA_NO_BLANKING, info);
-
-	par->cur_state = 0;
-
+	release_console_sem();
 	return 0;
 }
 /***********************************************************************


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: Re: i810fb_blank

[Lucas Correia Villa Real]

On Friday 16 December 2005 04:00, Antonino A. Daplas wrote:
> I have neglected the suspend and resume hooks of i810fb so it's not
> updated with the current code.  I don't have the hardware so I cannot
> test.  Can you tell your friend to also try this patch if he cannot
> solve the problem?

Hi Tony,

The patch worked fine for suspend, except for resume, which shows some weird 
patterns on the screen. However, I'm not sure if this is i810fb's fault, 
since i810fb_resume() isn't getting called at the time the resume hangs. 
I'll dig a little bit into software suspend 2's resume operation, maybe 
it's relying on something else which is broken.

Thanks,

-- 
Lucas
powered by /dev/dsp


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: Re: i810fb_blank

[Antonino A. Daplas]

Lucas Correia Villa Real wrote:
> On Friday 16 December 2005 04:00, Antonino A. Daplas wrote:
>> I have neglected the suspend and resume hooks of i810fb so it's not
>> updated with the current code.  I don't have the hardware so I cannot
>> test.  Can you tell your friend to also try this patch if he cannot
>> solve the problem?
> 
> Hi Tony,
> 
> The patch worked fine for suspend, except for resume, which shows some weird 
> patterns on the screen. However, I'm not sure if this is i810fb's fault, 
> since i810fb_resume() isn't getting called at the time the resume hangs. 
> I'll dig a little bit into software suspend 2's resume operation, maybe 
> it's relying on something else which is broken.

You can try it first on a bare system.  No X, no DRM.

Tony


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click