Radeonfb acpi resume

PATCH In review Micheal Marineau 3 participants 9 messages Jul 10, 2005 – Jul 13, 2005 1 message not in the archive Framebuffer layer

lore

Share & tools

lore

Micheal Marineau

author

2005-07-10 22:22 UTC #0 lore raw

PATCH 1/1 Radeonfb acpi resume

I've been forward porting this patch for a while now and need some input on it. You can see the last time someone posted it to the list here:
http://www.ussg.iu.edu/hypermail/linux/kernel/0410.0/0600.html

The big issue mentioned in that thread, that it reqires a key press during the resume process to keep going still exists and I have been unable to understand why. The issue is in radeon_pm.c in this block that follows the last hunk of the diff:

        if (rinfo->no_schedule) {
                if (try_acquire_console_sem())
                        return 0;
        } else
                acquire_console_sem();

Specificly it's acquire_console_sem(); where the resume stops waiting for a key press. What could be stopping things?

btw, I also have a suspend2 friendly version of the patch:
http://dev.gentoo.org/~marineam/files/patch-radeonfb-2.6.12-suspend2

diff -ru linux-2.6.12.orig/arch/i386/kernel/acpi/sleep.c linux-2.6.12/arch/i386/kernel/acpi/sleep.c
--- linux-2.6.12.orig/arch/i386/kernel/acpi/sleep.c 2005-06-17
12:48:29.000000000 -0700
+++ linux-2.6.12/arch/i386/kernel/acpi/sleep.c 2005-07-03
18:01:34.000000000 -0700
@@ -5,6 +5,7 @@
* Copyright (C) 2001-2003 Pavel Machek <[email protected]> */

+#include <linux/module.h>
#include <linux/acpi.h>
#include <linux/bootmem.h>
#include <asm/smp.h>
@@ -55,6 +56,31 @@
zap_low_mappings();
}

+/*
+ * acpi_vgapost
+ */
+
+extern void do_vgapost_lowlevel (unsigned long);
+
+void acpi_vgapost (unsigned long slot)
+{

+  unsigned long flags, saved_video_flags = acpi_video_flags;
+  acpi_video_flags = (slot & 0xffff) << 16 | 1;
+  /* Map low memory and copy information */
+  init_low_mapping(swapper_pg_dir, USER_PTRS_PER_PGD);
+  memcpy((void *) acpi_wakeup_address, &wakeup_start, &wakeup_end -
&wakeup_start);
+  acpi_copy_wakeup_routine(acpi_wakeup_address);
+  /* Tunnel thru real mode */
+  local_irq_save(flags);
+  do_vgapost_lowlevel(acpi_wakeup_address);
+  local_irq_restore(flags);
+  /* Restore mapping etc */
+  zap_low_mappings();
+  acpi_video_flags = saved_video_flags;

+}
+
+EXPORT_SYMBOL (acpi_vgapost);
+
/**
* acpi_reserve_bootmem - do _very_ early ACPI initialisation *
diff -ru linux-2.6.12.orig/arch/i386/kernel/acpi/wakeup.S linux-2.6.12/arch/i386/kernel/acpi/wakeup.S
--- linux-2.6.12.orig/arch/i386/kernel/acpi/wakeup.S 2005-06-17
12:48:29.000000000 -0700
+++ linux-2.6.12/arch/i386/kernel/acpi/wakeup.S 2005-07-03
18:01:34.000000000 -0700
@@ -170,6 +170,32 @@

_setbad: jmp setbad

+#
+# Real mode switch - verbatim from reboot.c
+#
+go_real:
+ movl %cr0, %eax
+ andl $0x00000011, %eax
+ orl $0x60000000, %eax
+ movl %eax, %cr0
+ movl %eax, %cr3
+ movl %cr0, %ebx
+ andl $0x60000000, %ebx
+ jz 1f
+ wbinvd
+1: andb $0x10, %al
+ movl %eax, %cr0
+go_real_jmp: .byte 0xea
+go_real_jmp_off: .word 0x0000
+go_real_jmp_seg: .word 0x0000
+#
+# Real mode descriptor table
+#
+ .align 8
+go_real_desc: .quad 0x0000000000000000
+go_real_cseg: .quad 0x00009a000000ffff
+go_real_dseg: .quad 0x000092000000ffff
+
.code32
ALIGN

@@ -309,6 +335,59 @@
call acpi_enter_sleep_state_s4bios
ret

+ENTRY(do_vgapost_lowlevel)
+ # Convert target offset to physical address
+ movl %eax, %ecx
+ subl $__PAGE_OFFSET, %ecx
+ # Fixup GDT pointer
+ movl %ecx, %edx
+ addl $go_real_desc - wakeup_start, %edx
+ movl %edx, go_real_gdt + 2
+ # Fixup 16-bit CS descriptor
+ movl %ecx, %edx
+ movw %dx, go_real_cseg - wakeup_start + 2 (%eax)
+ shrl $16, %edx
+ movb %dl, go_real_cseg - wakeup_start + 4 (%eax)
+ movb %dh, go_real_cseg - wakeup_start + 7 (%eax)
+ # Fixup 16-bit jump
+ movl %ecx, %edx
+ shrl $4, %edx
+ movw %dx, go_real_jmp_seg - wakeup_start (%eax)
+ # Save state and registers
+ call save_processor_state
+ call save_registers
+ # Reload page table with low mapping
+ movl $swapper_pg_dir-__PAGE_OFFSET, %eax
+ movl %eax, %cr3
+ # Load IDTR and GDTR for real mode
+ lidt go_real_idt
+ lgdt go_real_gdt
+ # Load DS & al
+ movl $0x0010, %eax
+ movl %eax, %ss
+ movl %eax, %ds
+ movl %eax, %es
+ movl %eax, %fs
+ movl %eax, %gs
+ # Load CS
+ call $0x0008, $(go_real - wakeup_start)
+ # Phony return code
+ call restore_registers
+ call restore_processor_state
+ ret
+
+ .align 4
+ .word 0
+go_real_idt:
+ .word 0x3ff
+ .long 0
+
+ .align 4
+ .word 0
+go_real_gdt:
+ .word 3*8-1
+ .long 0
+
ALIGN

 # saved registers
 saved_gdt:	.long	0,0
diff -ru linux-2.6.12.orig/drivers/video/aty/radeon_pm.c
linux-2.6.12/drivers/video/aty/radeon_pm.c
--- linux-2.6.12.orig/drivers/video/aty/radeon_pm.c	2005-06-17
12:48:29.000000000 -0700
+++ linux-2.6.12/drivers/video/aty/radeon_pm.c	2005-07-03
19:55:36.000000000 -0700
@@ -2606,10 +2606,13 @@

done:
pdev->dev.power.power_state = state;
+ pci_save_state (pdev);

return 0;
}

+extern void acpi_vgapost (unsigned long slot);
+
int radeonfb_pci_resume(struct pci_dev *pdev)

 {
         struct fb_info *info = pci_get_drvdata(pdev);

@@ -2619,6 +2622,12 @@
if (pdev->dev.power.power_state == 0)
return 0;

+ if (pdev->dev.power.power_state != 4)
+ {
+ pci_restore_state (pdev);
+ acpi_vgapost (pdev->devfn);
+ }
+
if (rinfo->no_schedule) {
if (try_acquire_console_sem())
return 0;

Signature · 3 lines

Michael Marineau
[email protected]
Oregon State University

Micheal Marineau

author

2005-07-10 22:27 UTC #1 lore raw

Aww crap, thunderbird screwed up the white space...

A usable version of the patch is attached, or here is a link:
http://dev.gentoo.org/~marineam/files/patch-radeonfb-2.6.12

Quoted 181 lines · Micheal Marineau

(quoted diff) +26 -0

@@ -5,6 +5,7 @@
  *  Copyright (C) 2001-2003 Pavel Machek <[email protected]>
  */
 
+#include <linux/module.h>
 #include <linux/acpi.h>
 #include <linux/bootmem.h>
 #include <asm/smp.h>

@@ -55,6 +56,31 @@
 	zap_low_mappings();
 }
 
+/*
+ * acpi_vgapost
+ */
+
+extern void do_vgapost_lowlevel (unsigned long);
+
+void acpi_vgapost (unsigned long slot)
+{
+  unsigned long flags, saved_video_flags = acpi_video_flags;
+  acpi_video_flags = (slot & 0xffff) << 16 | 1;
+  /* Map low memory and copy information */
+  init_low_mapping(swapper_pg_dir, USER_PTRS_PER_PGD);
+  memcpy((void *) acpi_wakeup_address, &wakeup_start, &wakeup_end -
 &wakeup_start);
+  acpi_copy_wakeup_routine(acpi_wakeup_address);
+  /* Tunnel thru real mode */
+  local_irq_save(flags);
+  do_vgapost_lowlevel(acpi_wakeup_address);
+  local_irq_restore(flags);
+  /* Restore mapping etc */
+  zap_low_mappings();
+  acpi_video_flags = saved_video_flags;
+}
+
+EXPORT_SYMBOL (acpi_vgapost);
+
 /**
  * acpi_reserve_bootmem - do _very_ early ACPI initialisation

*
diff -ru linux-2.6.12.orig/arch/i386/kernel/acpi/wakeup.S linux-2.6.12/arch/i386/kernel/acpi/wakeup.S
--- linux-2.6.12.orig/arch/i386/kernel/acpi/wakeup.S 2005-06-17
12:48:29.000000000 -0700
+++ linux-2.6.12/arch/i386/kernel/acpi/wakeup.S 2005-07-03
18:01:34.000000000 -0700

(quoted diff) +79 -0

@@ -170,6 +170,32 @@
 
 _setbad: jmp setbad
 
+#
+#	Real mode switch - verbatim from reboot.c
+#
+go_real:
+	movl	%cr0, %eax
+	andl	$0x00000011, %eax
+	orl	$0x60000000, %eax
+	movl	%eax, %cr0
+	movl	%eax, %cr3
+	movl	%cr0, %ebx
+	andl	$0x60000000, %ebx
+	jz	1f
+	wbinvd
+1:	andb	$0x10, %al
+	movl	%eax, %cr0
+go_real_jmp:		.byte	0xea
+go_real_jmp_off:	.word	0x0000
+go_real_jmp_seg:	.word	0x0000
+#
+# 	Real mode descriptor table
+#
+		.align	8
+go_real_desc:	.quad	0x0000000000000000
+go_real_cseg:	.quad	0x00009a000000ffff
+go_real_dseg:	.quad	0x000092000000ffff
+
 	.code32
 	ALIGN

@@ -309,6 +335,59 @@
 	call acpi_enter_sleep_state_s4bios
 	ret
 
+ENTRY(do_vgapost_lowlevel)
+	# Convert target offset to physical address
+	movl	%eax, %ecx
+	subl	$__PAGE_OFFSET, %ecx
+	# Fixup GDT pointer
+	movl	%ecx, %edx
+	addl	$go_real_desc - wakeup_start, %edx
+	movl	%edx, go_real_gdt + 2
+	# Fixup 16-bit CS descriptor
+	movl	%ecx, %edx
+	movw	%dx, go_real_cseg - wakeup_start + 2 (%eax)
+	shrl	$16, %edx
+	movb	%dl, go_real_cseg - wakeup_start + 4 (%eax)
+	movb	%dh, go_real_cseg - wakeup_start + 7 (%eax)
+	# Fixup 16-bit jump
+	movl	%ecx, %edx
+	shrl	$4, %edx
+	movw	%dx, go_real_jmp_seg - wakeup_start (%eax)
+	# Save state and registers
+	call	save_processor_state
+	call	save_registers
+	# Reload page table with low mapping
+	movl	$swapper_pg_dir-__PAGE_OFFSET, %eax
+	movl	%eax, %cr3
+	# Load IDTR and GDTR for real mode
+	lidt	go_real_idt
+	lgdt	go_real_gdt
+	# Load DS & al
+	movl	$0x0010, %eax
+	movl	%eax, %ss
+	movl	%eax, %ds
+	movl	%eax, %es
+	movl	%eax, %fs
+	movl	%eax, %gs
+	# Load CS
+	call	$0x0008, $(go_real - wakeup_start)
+	# Phony return code
+	call	restore_registers
+	call	restore_processor_state
+	ret
+	
+	.align	4
+	.word	0
+go_real_idt:
+	.word	0x3ff
+	.long	0
+	
+	.align	4
+	.word	0
+go_real_gdt:
+	.word	3*8-1
+	.long	0
+
 ALIGN
 # saved registers
 saved_gdt:	.long	0,0

diff -ru linux-2.6.12.orig/drivers/video/aty/radeon_pm.c linux-2.6.12/drivers/video/aty/radeon_pm.c
--- linux-2.6.12.orig/drivers/video/aty/radeon_pm.c 2005-06-17
12:48:29.000000000 -0700
+++ linux-2.6.12/drivers/video/aty/radeon_pm.c 2005-07-03
19:55:36.000000000 -0700

(quoted diff) +9 -0

@@ -2606,10 +2606,13 @@
 
  done:
 	pdev->dev.power.power_state = state;
+	pci_save_state (pdev);
 
 	return 0;
 }
 
+extern void acpi_vgapost (unsigned long slot);
+
 int radeonfb_pci_resume(struct pci_dev *pdev)
 {
         struct fb_info *info = pci_get_drvdata(pdev);

@@ -2619,6 +2622,12 @@
 	if (pdev->dev.power.power_state == 0)
 		return 0;
 
+	if (pdev->dev.power.power_state != 4)
+	{
+		pci_restore_state (pdev);
+		acpi_vgapost (pdev->devfn);
+	}
+
 	if (rinfo->no_schedule) {
 		if (try_acquire_console_sem())
 			return 0;

Signature · 3 lines

Michael Marineau
[email protected]
Oregon State University

arch/i386/kernel/acpi/sleep.c +26 -0

diff -ru linux-2.6.12.orig/arch/i386/kernel/acpi/sleep.c linux-2.6.12/arch/i386/kernel/acpi/sleep.c
@@ -5,6 +5,7 @@
  *  Copyright (C) 2001-2003 Pavel Machek <[email protected]>
  */
 
+#include <linux/module.h>
 #include <linux/acpi.h>
 #include <linux/bootmem.h>
 #include <asm/smp.h>

@@ -55,6 +56,31 @@
 	zap_low_mappings();
 }
 
+/*
+ * acpi_vgapost
+ */
+
+extern void do_vgapost_lowlevel (unsigned long);
+
+void acpi_vgapost (unsigned long slot)
+{
+  unsigned long flags, saved_video_flags = acpi_video_flags;
+  acpi_video_flags = (slot & 0xffff) << 16 | 1;
+  /* Map low memory and copy information */
+  init_low_mapping(swapper_pg_dir, USER_PTRS_PER_PGD);
+  memcpy((void *) acpi_wakeup_address, &wakeup_start, &wakeup_end - &wakeup_start);
+  acpi_copy_wakeup_routine(acpi_wakeup_address);
+  /* Tunnel thru real mode */
+  local_irq_save(flags);
+  do_vgapost_lowlevel(acpi_wakeup_address);
+  local_irq_restore(flags);
+  /* Restore mapping etc */
+  zap_low_mappings();
+  acpi_video_flags = saved_video_flags;
+}
+
+EXPORT_SYMBOL (acpi_vgapost);
+
 /**
  * acpi_reserve_bootmem - do _very_ early ACPI initialisation
  *

arch/i386/kernel/acpi/wakeup.S +79 -0

91 lines not on this page · raw message

drivers/video/aty/radeon_pm.c +9 -0

diff -ru linux-2.6.12.orig/drivers/video/aty/radeon_pm.c linux-2.6.12/drivers/video/aty/radeon_pm.c
@@ -2606,10 +2606,13 @@
 
  done:
 	pdev->dev.power.power_state = state;
+	pci_save_state (pdev);
 
 	return 0;
 }
 
+extern void acpi_vgapost (unsigned long slot);
+
 int radeonfb_pci_resume(struct pci_dev *pdev)
 {
         struct fb_info *info = pci_get_drvdata(pdev);

@@ -2619,6 +2622,12 @@
 	if (pdev->dev.power.power_state == 0)
 		return 0;
 
+	if (pdev->dev.power.power_state != 4)
+	{ 
+		pci_restore_state (pdev);
+		acpi_vgapost (pdev->devfn);
+	}
+
 	if (rinfo->no_schedule) {
 		if (try_acquire_console_sem())
 			return 0;

Pavel Machek

2005-07-11 15:38 UTC #2 lore raw

Hi!

Quoted 4 lines

Aww crap, thunderbird screwed up the white space...
A usable version of the patch is attached, or here is a link:
http://dev.gentoo.org/~marineam/files/patch-radeonfb-2.6.12

Wrong indentation in acpi_vgapost; I remember there was better patch to fix this out there.

Anyway, are you sure machine you have can't be fixed by any methods listed in Doc*/power/video.txt? I guess they are preferable to acpi_vgapost...

If not... indent it acording to the coding style and drop "phony return code" (it is unneeded, anyway, right?) and try again. (Oh and Cc me ;-).
Pavel

Signature · 1 line

teflon -- maybe it is a trademark, but it should not be.

Micheal Marineau

author

2005-07-11 18:54 UTC #3 lore raw

Thanks for the reply,

Quoted 11 lines · Pavel Machek

Hi!
Quoted 4 lines
Aww crap, thunderbird screwed up the white space...
A usable version of the patch is attached, or here is a link:
http://dev.gentoo.org/~marineam/files/patch-radeonfb-2.6.12
Wrong indentation in acpi_vgapost; I remember there was better patch to fix this out there.

Ok, I'll go through and fix any coding style problems. I've only seen older versions of this same patch, but if there is a better way I'd love to hear it. I'll google around a little more just in case.

Quoted 4 lines

Anyway, are you sure machine you have can't be fixed by any methods listed in Doc*/power/video.txt? I guess they are preferable to acpi_vgapost...

Actually, this is one of the metholds listed in video.txt. Take a look at #7 ;-). I just tried acpi_sleep=s3_bios to see what that does, but just caused an instant reboot on resume. The only other solutions that works is to disable the frame buffer and use X or some other app to do the job as listed in #5 and #6, but something in kernel like this patch is required to be able to use the framebuffer.

Quoted 5 lines

If not... indent it acording to the coding style and drop "phony return code" (it is unneeded, anyway, right?) and try again. (Oh and Cc me ;-).
Pavel

Ok, I'll try it without. I don't know if it's needed or not, that code is untouched from Ole Rohne's origional version.

Signature · 3 lines

Michael Marineau
[email protected]
Oregon State University

Pavel Machek

2005-07-11 18:58 UTC #4 lore raw

Hi!

Quoted 11 lines

Quoted 8 lines
Quoted 4 lines
Aww crap, thunderbird screwed up the white space...
A usable version of the patch is attached, or here is a link:
http://dev.gentoo.org/~marineam/files/patch-radeonfb-2.6.12
Wrong indentation in acpi_vgapost; I remember there was better patch to fix this out there.
Ok, I'll go through and fix any coding style problems. I've only seen older versions of this same patch, but if there is a better way I'd love to hear it. I'll google around a little more just in case.

It *was* version of the same patch, but it had codingstyle fixed, IIRC.

Quoted 9 lines

Quoted 3 lines
Anyway, are you sure machine you have can't be fixed by any methods listed in Doc*/power/video.txt? I guess they are preferable to acpi_vgapost...
Actually, this is one of the metholds listed in video.txt. Take a look at #7 ;-). I just tried acpi_sleep=s3_bios to see what that does, but just caused an instant reboot on resume. The only other solutions that works is to disable the frame buffer and use X or some other app to do the job as listed in #5 and #6, but something in kernel like this patch is required to be able to use the framebuffer.

I'd say that disabling framebuffer and going #5 or #6 is still prefered, but given nice patch, I'll probably accept it. Oh, and do note that (7) is listed near just one notebook.

Pavel

Signature · 1 line

teflon -- maybe it is a trademark, but it should not be.

Micheal Marineau

author

2005-07-12 04:36 UTC #5 lore raw

Quoted 12 lines · Pavel Machek

Hi!
Quoted 9 lines
Quoted 8 lines
Quoted 4 lines
Aww crap, thunderbird screwed up the white space...
A usable version of the patch is attached, or here is a link:
http://dev.gentoo.org/~marineam/files/patch-radeonfb-2.6.12
Wrong indentation in acpi_vgapost; I remember there was better patch to fix this out there.

Quoted 6 lines

Quoted 3 lines
Ok, I'll go through and fix any coding style problems. I've only seen older versions of this same patch, but if there is a better way I'd love to hear it. I'll google around a little more just in case.
It *was* version of the same patch, but it had codingstyle fixed, IIRC.

Ok, thats an easy fix then :-)

Quoted 14 lines

Quoted 6 lines
Quoted 5 lines
Anyway, are you sure machine you have can't be fixed by any methods listed in Doc*/power/video.txt? I guess they are preferable to acpi_vgapost...
Quoted 6 lines
Actually, this is one of the metholds listed in video.txt. Take a look at #7 ;-). I just tried acpi_sleep=s3_bios to see what that does, but just caused an instant reboot on resume. The only other solutions that works is to disable the frame buffer and use X or some other app to do the job as listed in #5 and #6, but something in kernel like this patch is required to be able to use the framebuffer.

Quoted 5 lines

I'd say that disabling framebuffer and going #5 or #6 is still prefered, but given nice patch, I'll probably accept it. Oh, and do note that (7) is listed near just one notebook.
Pavel

Well, before the patch is ever concidered for the mainline I think the issue with acquire_console_sem() causing the resume process to stop and wait for a key press would have to be addressed.

This option could be listed for more notebooks with radeon cards, for example mine is an Dell Inspiron 8500.

Signature · 3 lines

Michael Marineau
[email protected]
Oregon State University

A message that is not in the archive (sent off-list or not yet received): [email protected]

Micheal Marineau

author

2005-07-12 13:49 UTC #7 lore raw

Quoted 14 lines · Matthew Garrett

Review
Quoted 5 lines · Micheal Marineau
(quoted diff) +5 -0
+	if (pdev->dev.power.power_state != 4)
+	{
+		pci_restore_state (pdev);
+		acpi_vgapost (pdev->devfn);
+	}
Please *don't* make this unconditional. There's no guarantee that the video BIOS is postable after initial system boot. A boot-time parameter to enable it seems reasonable, but in general we're likely to stand a much better chance getting it working in userspace.

A kernel parameter seems reasonable, I assume that pci_restore_state shoule remain unconditional right? Userspace works just fine if the frame buffer is not used, but it has to be done in kernel when using a frame buffer.

Signature · 3 lines

Michael Marineau
[email protected]
Oregon State University

Benjamin Herrenschmidt

2005-07-13 00:34 UTC #8 lore raw

In reply to Micheal Marineau

Quoted 10 lines · Micheal Marineau

I've been forward porting this patch for a while now and need some input on it. You can see the last time someone posted it to the list here:
http://www.ussg.iu.edu/hypermail/linux/kernel/0410.0/0600.html
The big issue mentioned in that thread, that it reqires a key press during the resume process to keep going still exists and I have been unable to understand why. The issue is in radeon_pm.c in this block that follows the last hunk of the diff:

Quoted 8 lines

        if (rinfo->no_schedule) {
                if (try_acquire_console_sem())
                        return 0;
        } else
                acquire_console_sem();
Specificly it's acquire_console_sem(); where the resume stops waiting for a key press. What could be stopping things?

I don't see any reason for that. That definitely shouldn't happen.

.../...

Quoted 7 lines

diff -ru linux-2.6.12.orig/drivers/video/aty/radeon_pm.c linux-2.6.12/drivers/video/aty/radeon_pm.c
--- linux-2.6.12.orig/drivers/video/aty/radeon_pm.c 2005-06-17
12:48:29.000000000 -0700
+++ linux-2.6.12/drivers/video/aty/radeon_pm.c 2005-07-03
19:55:36.000000000 -0700

Review line 2612

Quoted 7 lines

(quoted diff) +1 -0

@@ -2606,10 +2606,13 @@
 
  done:
 	pdev->dev.power.power_state = state;
+	pci_save_state (pdev);
 
 	return 0;
 }

Hrm... radeonfb already saves the config space elsewhere. Note that it could maybe be converted to pci_save_state() / pci_restore_state() but currently, I do my own as I use that for "testing" if the card is still in it's previous state or was shut down. (I could probably use the content of a register instead, like CLK_PIN_CNTL)

Quoted 8 lines

(quoted diff) +2 -0

+extern void acpi_vgapost (unsigned long slot);
+
 int radeonfb_pci_resume(struct pci_dev *pdev)
 {
         struct fb_info *info = pci_get_drvdata(pdev);

@@ -2619,6 +2622,12 @@
 	if (pdev->dev.power.power_state == 0)
 		return 0;

Review line 2633

Quoted 10 lines

(quoted diff) +6 -0

 
+	if (pdev->dev.power.power_state != 4)
+	{
+		pci_restore_state (pdev);
+		acpi_vgapost (pdev->devfn);
+	}
+
 	if (rinfo->no_schedule) {
 		if (try_acquire_console_sem())
 			return 0;

The above will probably blow up anything that is not an x86 with ACPI. Besides there is already a mecanism in that file for calling functions for re-posting cards (since I have code to explicitely re-post rv280 and rv350 mobility on macs), you should hook into that existing mecanism when you detect ACPI.

Ben.

Micheal Marineau

author

2005-07-13 05:31 UTC #9 lore raw

Thanks for the input,

Quoted 18 lines · Benjamin Herrenschmidt

Quoted 16 lines · Micheal Marineau
I've been forward porting this patch for a while now and need some input on it. You can see the last time someone posted it to the list here:
http://www.ussg.iu.edu/hypermail/linux/kernel/0410.0/0600.html
The big issue mentioned in that thread, that it reqires a key press during the resume process to keep going still exists and I have been unable to understand why. The issue is in radeon_pm.c in this block that follows the last hunk of the diff:
       if (rinfo->no_schedule) {
               if (try_acquire_console_sem())
                       return 0;
       } else
               acquire_console_sem();

Quoted 7 lines

Quoted 2 lines
Specificly it's acquire_console_sem(); where the resume stops waiting for a key press. What could be stopping things?
I don't see any reason for that. That definitely shouldn't happen.
   .../...

Yea, so it would seem. Hopefully that'll get fixed magicly once I clean up the rest a little bit.

Quoted 14 lines

Quoted 14 lines
diff -ru linux-2.6.12.orig/drivers/video/aty/radeon_pm.c linux-2.6.12/drivers/video/aty/radeon_pm.c
--- linux-2.6.12.orig/drivers/video/aty/radeon_pm.c 2005-06-17
12:48:29.000000000 -0700
+++ linux-2.6.12/drivers/video/aty/radeon_pm.c 2005-07-03
19:55:36.000000000 -0700
(quoted diff) +1 -0
@@ -2606,10 +2606,13 @@
 
 done:
 	pdev->dev.power.power_state = state;
+	pci_save_state (pdev);
 

Quoted 9 lines

Review line 2612
Quoted 2 lines
(quoted diff) +0 -0
 	return 0;
 }
Hrm... radeonfb already saves the config space elsewhere. Note that it could maybe be converted to pci_save_state() / pci_restore_state() but currently, I do my own as I use that for "testing" if the card is still in it's previous state or was shut down. (I could probably use the content of a register instead, like CLK_PIN_CNTL)

Hrm, maybe it works now, but a few kernel versions back something broke and I added that in as an easy fix. I'll try taking pci_save/restore out again, hopefully all is well again.

Quoted 28 lines

Quoted 10 lines
(quoted diff) +2 -0
+extern void acpi_vgapost (unsigned long slot);
+
 int radeonfb_pci_resume(struct pci_dev *pdev)
 {
        struct fb_info *info = pci_get_drvdata(pdev);
@@ -2619,6 +2622,12 @@
 	if (pdev->dev.power.power_state == 0)
 		return 0;
Review line 2633
Quoted 10 lines
(quoted diff) +6 -0
 
+	if (pdev->dev.power.power_state != 4)
+	{
+		pci_restore_state (pdev);
+		acpi_vgapost (pdev->devfn);
+	}
+
 	if (rinfo->no_schedule) {
 		if (try_acquire_console_sem())
 			return 0;
The above will probably blow up anything that is not an x86 with ACPI. Besides there is already a mecanism in that file for calling functions for re-posting cards (since I have code to explicitely re-post rv280 and rv350 mobility on macs), you should hook into that existing mecanism when you detect ACPI.

Quoted 2 lines

Ben.

ok, I haven't looked at the existing code very extensivly, I should check it out :-)

Signature · 3 lines

Michael Marineau
[email protected]
Oregon State University