[PATCH][help?] Radeonfb acpi resume

[PATCH][help?] Radeonfb acpi resume

[Micheal Marineau]

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

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 <pavel@suse.cz>
  */

+#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;
-- 
Michael Marineau
marineam@engr.orst.edu
Oregon State University

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: [PATCH][help?] Radeonfb acpi resume

[Micheal Marineau]

[-- Attachment #1.1: Type: text/plain, Size: 4909 bytes --]

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

Micheal Marineau wrote:
> 
> 
> 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 <pavel@suse.cz>
>   */
> 
> +#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;


-- 
Michael Marineau
marineam@engr.orst.edu
Oregon State University


[-- Attachment #1.2: patch-radeonfb-2.6.12 --]
[-- Type: text/plain, Size: 4286 bytes --]

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 <pavel@suse.cz>
  */
 
+#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;


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: [PATCH][help?] Radeonfb acpi resume

[Pavel Machek]

Hi!

> 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
-- 
teflon -- maybe it is a trademark, but it should not be.

Re: [PATCH][help?] Radeonfb acpi resume

[Micheal Marineau]

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

Thanks for the reply,

Pavel Machek wrote:
> Hi!
> 
> 
>>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.

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

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

-- 
Michael Marineau
marineam@engr.orst.edu
Oregon State University

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: [PATCH][help?] Radeonfb acpi resume

[Pavel Machek]

Hi!

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

> > 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
-- 
teflon -- maybe it is a trademark, but it should not be.

Re: [PATCH][help?] Radeonfb acpi resume

[Micheal Marineau]

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

Pavel Machek wrote:
> Hi!
> 
> 
>>>>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.

Ok, thats an easy fix then :-)
> 
> 
>>>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

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.


-- 
Michael Marineau
marineam@engr.orst.edu
Oregon State University

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: [PATCH][help?] Radeonfb acpi resume

[Micheal Marineau]

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

Matthew Garrett wrote:
> Micheal Marineau <marineam@engr.orst.edu> wrote:
> 
> 
>>+	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.


-- 
Michael Marineau
marineam@engr.orst.edu
Oregon State University

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: [PATCH][help?] Radeonfb acpi resume

[Benjamin Herrenschmidt]

On Sun, 2005-07-10 at 15:19 -0700, Micheal Marineau wrote:
> 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?

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

   .../...

> 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;
>  }

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)

> +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;

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.
 

Re: [PATCH][help?] Radeonfb acpi resume

[Micheal Marineau]

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

Thanks for the input,

Benjamin Herrenschmidt wrote:
> On Sun, 2005-07-10 at 15:19 -0700, Micheal Marineau wrote:
> 
>>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?
> 
> 
> 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.

> 
> 
>>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;
>> }
> 
> 
> 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.

> 
> 
>>+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;
> 
> 
> 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.
>  

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


-- 
Michael Marineau
marineam@engr.orst.edu
Oregon State University


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]