On Wed, 2005-04-06 at 19:40 +0200, Esteban Martinez wrote:
> Hi Ben! Here I'm again.
>
> > What about with none of my patches ? (Just plain 2.6.12-rc1-bk6).
> Ok. Those tests are with plain 2.6.12-rc1-bk6 kernel. I summarize you:
> 1) Boot in console mode.
> 2) Sleep/wakeup cycle correctly.
> 3) After wake up, I start X and it crashes (black screen, no keyboard, but hdd
> continues spinning)
You haven't tried:
1) Boot in console mode
2) Test X
(with no sleep wkaeup cycle and with DRI enabled). Can you try and let
me know ?
> Test with DRI _disabled_:
> 1) Boot in console mode.
> 2) Just after I've logged, I start X and it works. After that, I come back to
> console mode and the screens stays for a few seconds with vertical green
> slashes, and then appears the console. I can contine working properly.
> 3) After that I do a sleep/wakeup cycle and it crashes when it tries to wake
> up (black screen, no keyboard), but this time the hard disk continues
> spinning.
Ok. Can you also try the attached patch ?
>
> > I have no idea what's wrong for now, the log looks normal. Can you try
> > not enabling DRI see if it makes a difference ?
> This test is with 2.6.12-rc1-bk6 kernel plus your patches and the last
> change (lis -> li...). I've disabled DRI and it works. The test is:
> 1) Boot in console mode.
> 2) Start X and it works. I can work properly. After that I return to the
> console and the screens shows vertical green slashes for a few seconds, and
> then returns to console. I can continue working. All ok.
> 3) I repeat the point 2 and it ocurrs the same.
>
>
> > If it still crashes, I would appreciate some regression testing, that is
> > going backward in the bk snapshots to point out which one precisely
> > introduced the problem.
> After all, I only left to test the kernel 2.6.12-rc1-bk5 and below, and the
> 2.6.12-rc2 kernel. Let me know which one do you want I test.
>
> I wait news from you. Thanks a lot. :-)
>
> Esteban.
>
>
> --
> Keep it soulful and spread love!
> "Ask and it will given to you; seek and you will find;
> knock and the door will be opened to you." (Mathew 7:7)
>
>
--
Benjamin Herrenschmidt <[EMAIL PROTECTED]>
Index: linux-work/drivers/char/agp/uninorth-agp.c
===================================================================
--- linux-work.orig/drivers/char/agp/uninorth-agp.c 2005-03-15 11:57:17.000000000 +1100
+++ linux-work/drivers/char/agp/uninorth-agp.c 2005-04-05 15:20:29.000000000 +1000
@@ -10,6 +10,7 @@
#include <asm/uninorth.h>
#include <asm/pci-bridge.h>
#include <asm/prom.h>
+#include <asm/pmac_feature.h>
#include "agp.h"
/*
@@ -26,6 +27,7 @@
static int uninorth_rev;
static int is_u3;
+
static int uninorth_fetch_size(void)
{
int i;
@@ -264,7 +266,8 @@
&scratch);
} while ((scratch & PCI_AGP_COMMAND_AGP) == 0 && ++timeout < 1000);
if ((scratch & PCI_AGP_COMMAND_AGP) == 0)
- printk(KERN_ERR PFX "failed to write UniNorth AGP command reg\n");
+ printk(KERN_ERR PFX "failed to write UniNorth AGP"
+ " command register\n");
if (uninorth_rev >= 0x30) {
/* This is an AGP V3 */
@@ -278,13 +281,24 @@
}
#ifdef CONFIG_PM
-static int agp_uninorth_suspend(struct pci_dev *pdev, pm_message_t state)
+/*
+ * These Power Management routines are _not_ called by the normal PCI PM layer,
+ * but directly by the video driver through function pointers in the device
+ * tree.
+ */
+static int agp_uninorth_suspend(struct pci_dev *pdev)
{
+ struct agp_bridge_data *bridge;
u32 cmd;
u8 agp;
struct pci_dev *device = NULL;
- if (state != PMSG_SUSPEND)
+ bridge = agp_find_bridge(pdev);
+ if (bridge == NULL)
+ return -ENODEV;
+
+ /* Only one suspend supported */
+ if (bridge->dev_private_data)
return 0;
/* turn off AGP on the video chip, if it was enabled */
@@ -309,12 +323,13 @@
printk("uninorth-agp: disabling AGP on device %s\n",
pci_name(device));
cmd &= ~PCI_AGP_COMMAND_AGP;
- pci_write_config_dword(device, agp + PCI_AGP_COMMAND, cmd);
+ pci_write_config_dword(device, agp + PCI_AGP_COMMAND, cmd);
}
/* turn off AGP on the bridge */
agp = pci_find_capability(pdev, PCI_CAP_ID_AGP);
pci_read_config_dword(pdev, agp + PCI_AGP_COMMAND, &cmd);
+ bridge->dev_private_data = (void *)cmd;
if (cmd & PCI_AGP_COMMAND_AGP) {
printk("uninorth-agp: disabling AGP on bridge %s\n",
pci_name(pdev));
@@ -329,9 +344,23 @@
static int agp_uninorth_resume(struct pci_dev *pdev)
{
+ struct agp_bridge_data *bridge;
+ u32 command;
+
+ bridge = agp_find_bridge(pdev);
+ if (bridge == NULL)
+ return -ENODEV;
+
+ command = (u32)bridge->dev_private_data;
+ bridge->dev_private_data = NULL;
+ if (!(command & PCI_AGP_COMMAND_AGP))
+ return 0;
+
+ uninorth_agp_enable(bridge, command);
+
return 0;
}
-#endif
+#endif /* CONFIG_PM */
static int uninorth_create_gatt_table(struct agp_bridge_data *bridge)
{
@@ -575,6 +604,12 @@
of_node_put(uninorth_node);
}
+#ifdef CONFIG_PM
+ /* Inform platform of our suspend/resume caps */
+ pmac_register_agp_pm(pdev, agp_uninorth_suspend, agp_uninorth_resume);
+#endif
+
+ /* Allocate & setup our driver */
bridge = agp_alloc_bridge();
if (!bridge)
return -ENOMEM;
@@ -599,6 +634,11 @@
{
struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
+#ifdef CONFIG_PM
+ /* Inform platform of our suspend/resume caps */
+ pmac_register_agp_pm(pdev, NULL, NULL);
+#endif
+
agp_remove_bridge(bridge);
agp_put_bridge(bridge);
}
@@ -622,10 +662,6 @@
.id_table = agp_uninorth_pci_table,
.probe = agp_uninorth_probe,
.remove = agp_uninorth_remove,
-#ifdef CONFIG_PM
- .suspend = agp_uninorth_suspend,
- .resume = agp_uninorth_resume,
-#endif
};
static int __init agp_uninorth_init(void)
Index: linux-work/arch/ppc64/kernel/pmac_feature.c
===================================================================
--- linux-work.orig/arch/ppc64/kernel/pmac_feature.c 2005-03-29 15:44:35.000000000 +1000
+++ linux-work/arch/ppc64/kernel/pmac_feature.c 2005-04-05 14:39:52.000000000 +1000
@@ -674,3 +674,67 @@
dump_HT_speeds("PCI-X HT Downlink", cfg, freq);
#endif
}
+
+/*
+ * Early video resume hook
+ */
+
+static void (*pmac_early_vresume_proc)(void *data) __pmacdata;
+static void *pmac_early_vresume_data __pmacdata;
+
+void pmac_set_early_video_resume(void (*proc)(void *data), void *data)
+{
+ if (_machine != _MACH_Pmac)
+ return;
+ preempt_disable();
+ pmac_early_vresume_proc = proc;
+ pmac_early_vresume_data = data;
+ preempt_enable();
+}
+EXPORT_SYMBOL(pmac_set_early_video_resume);
+
+
+/*
+ * AGP related suspend/resume code
+ */
+
+static struct pci_dev *pmac_agp_bridge __pmacdata;
+static int (*pmac_agp_suspend)(struct pci_dev *bridge) __pmacdata;
+static int (*pmac_agp_resume)(struct pci_dev *bridge) __pmacdata;
+
+void __pmac pmac_register_agp_pm(struct pci_dev *bridge,
+ int (*suspend)(struct pci_dev *bridge),
+ int (*resume)(struct pci_dev *bridge))
+{
+ if (suspend || resume) {
+ pmac_agp_bridge = bridge;
+ pmac_agp_suspend = suspend;
+ pmac_agp_resume = resume;
+ return;
+ }
+ if (bridge != pmac_agp_bridge)
+ return;
+ pmac_agp_suspend = pmac_agp_resume = NULL;
+ return;
+}
+EXPORT_SYMBOL(pmac_register_agp_pm);
+
+void __pmac pmac_suspend_agp_for_card(struct pci_dev *dev)
+{
+ if (pmac_agp_bridge == NULL || pmac_agp_suspend == NULL)
+ return;
+ if (pmac_agp_bridge->bus != dev->bus)
+ return;
+ pmac_agp_suspend(pmac_agp_bridge);
+}
+EXPORT_SYMBOL(pmac_suspend_agp_for_card);
+
+void __pmac pmac_resume_agp_for_card(struct pci_dev *dev)
+{
+ if (pmac_agp_bridge == NULL || pmac_agp_resume == NULL)
+ return;
+ if (pmac_agp_bridge->bus != dev->bus)
+ return;
+ pmac_agp_resume(pmac_agp_bridge);
+}
+EXPORT_SYMBOL(pmac_resume_agp_for_card);
Index: linux-work/arch/ppc/platforms/pmac_feature.c
===================================================================
--- linux-work.orig/arch/ppc/platforms/pmac_feature.c 2005-04-05 14:29:30.000000000 +1000
+++ linux-work/arch/ppc/platforms/pmac_feature.c 2005-04-05 15:20:06.000000000 +1000
@@ -2944,3 +2944,48 @@
if (pmac_early_vresume_proc)
pmac_early_vresume_proc(pmac_early_vresume_data);
}
+
+/*
+ * AGP related suspend/resume code
+ */
+
+static struct pci_dev *pmac_agp_bridge __pmacdata;
+static int (*pmac_agp_suspend)(struct pci_dev *bridge) __pmacdata;
+static int (*pmac_agp_resume)(struct pci_dev *bridge) __pmacdata;
+
+void __pmac pmac_register_agp_pm(struct pci_dev *bridge,
+ int (*suspend)(struct pci_dev *bridge),
+ int (*resume)(struct pci_dev *bridge))
+{
+ if (suspend || resume) {
+ pmac_agp_bridge = bridge;
+ pmac_agp_suspend = suspend;
+ pmac_agp_resume = resume;
+ return;
+ }
+ if (bridge != pmac_agp_bridge)
+ return;
+ pmac_agp_suspend = pmac_agp_resume = NULL;
+ return;
+}
+EXPORT_SYMBOL(pmac_register_agp_pm);
+
+void __pmac pmac_suspend_agp_for_card(struct pci_dev *dev)
+{
+ if (pmac_agp_bridge == NULL || pmac_agp_suspend == NULL)
+ return;
+ if (pmac_agp_bridge->bus != dev->bus)
+ return;
+ pmac_agp_suspend(pmac_agp_bridge);
+}
+EXPORT_SYMBOL(pmac_suspend_agp_for_card);
+
+void __pmac pmac_resume_agp_for_card(struct pci_dev *dev)
+{
+ if (pmac_agp_bridge == NULL || pmac_agp_resume == NULL)
+ return;
+ if (pmac_agp_bridge->bus != dev->bus)
+ return;
+ pmac_agp_resume(pmac_agp_bridge);
+}
+EXPORT_SYMBOL(pmac_resume_agp_for_card);
Index: linux-work/drivers/video/aty/radeon_pm.c
===================================================================
--- linux-work.orig/drivers/video/aty/radeon_pm.c 2005-04-01 09:04:19.000000000 +1000
+++ linux-work/drivers/video/aty/radeon_pm.c 2005-04-05 15:21:54.000000000 +1000
@@ -2520,13 +2520,10 @@
}
-static/*extern*/ int susdisking = 0;
-
int radeonfb_pci_suspend(struct pci_dev *pdev, pm_message_t state)
{
struct fb_info *info = pci_get_drvdata(pdev);
struct radeonfb_info *rinfo = info->par;
- u8 agp;
int i;
if (state == pdev->dev.power.power_state)
@@ -2542,11 +2539,6 @@
*/
if (state != PM_SUSPEND_MEM)
goto done;
- if (susdisking) {
- printk("radeonfb (%s): suspending to disk but state = %d\n",
- pci_name(pdev), state);
- goto done;
- }
acquire_console_sem();
@@ -2567,27 +2559,13 @@
rinfo->lock_blank = 1;
del_timer_sync(&rinfo->lvds_timer);
- /* Disable AGP. The AGP host should have done it, but since ordering
- * isn't always properly guaranteed in this specific case, let's make
- * sure it's disabled on card side now. Ultimately, when merging fbdev
- * and dri into some common infrastructure, this will be handled
- * more nicely. The host bridge side will (or will not) be dealt with
- * by the bridge AGP driver, we don't attempt to touch it here.
+#ifdef CONFIG_PPC_PMAC
+ /* On powermac, we have hooks to properly suspend/resume AGP now,
+ * use them here. We'll ultimately need some generic support here,
+ * but the generic code isn't quite ready for that yet
*/
- agp = pci_find_capability(pdev, PCI_CAP_ID_AGP);
- if (agp) {
- u32 cmd;
-
- pci_read_config_dword(pdev, agp + PCI_AGP_COMMAND, &cmd);
- if (cmd & PCI_AGP_COMMAND_AGP) {
- printk(KERN_INFO "radeonfb (%s): AGP was enabled, "
- "disabling ...\n",
- pci_name(pdev));
- cmd &= ~PCI_AGP_COMMAND_AGP;
- pci_write_config_dword(pdev, agp + PCI_AGP_COMMAND,
- cmd);
- }
- }
+ pmac_suspend_agp_for_card(pdev);
+#endif /* CONFIG_PPC_PMAC */
/* If we support wakeup from poweroff, we save all regs we can including cfg
* space
@@ -2699,6 +2677,15 @@
rinfo->lock_blank = 0;
radeon_screen_blank(rinfo, FB_BLANK_UNBLANK, 1);
+#ifdef CONFIG_PPC_PMAC
+ /* On powermac, we have hooks to properly suspend/resume AGP now,
+ * use them here. We'll ultimately need some generic support here,
+ * but the generic code isn't quite ready for that yet
+ */
+ pmac_resume_agp_for_card(pdev);
+#endif /* CONFIG_PPC_PMAC */
+
+
/* Check status of dynclk */
if (rinfo->dynclk == 1)
radeon_pm_enable_dynamic_mode(rinfo);
Index: linux-work/include/asm-ppc/pmac_feature.h
===================================================================
--- linux-work.orig/include/asm-ppc/pmac_feature.h 2005-03-15 11:59:39.000000000 +1100
+++ linux-work/include/asm-ppc/pmac_feature.h 2005-04-05 14:29:31.000000000 +1000
@@ -305,6 +305,17 @@
#define PMAC_FTR_DEF(x) ((_MACH_Pmac << 16) | (x))
+/* The AGP driver registers itself here */
+extern void pmac_register_agp_pm(struct pci_dev *bridge,
+ int (*suspend)(struct pci_dev *bridge),
+ int (*resume)(struct pci_dev *bridge));
+
+/* Those are meant to be used by video drivers to deal with AGP
+ * suspend resume properly
+ */
+extern void pmac_suspend_agp_for_card(struct pci_dev *dev);
+extern void pmac_resume_agp_for_card(struct pci_dev *dev);
+
/*
* The part below is for use by macio_asic.c only, do not rely
Index: linux-work/drivers/video/aty/aty128fb.c
===================================================================
--- linux-work.orig/drivers/video/aty/aty128fb.c 2005-04-01 09:04:18.000000000 +1000
+++ linux-work/drivers/video/aty/aty128fb.c 2005-04-05 15:22:17.000000000 +1000
@@ -2331,7 +2331,6 @@
{
struct fb_info *info = pci_get_drvdata(pdev);
struct aty128fb_par *par = info->par;
- u8 agp;
/* We don't do anything but D2, for now we return 0, but
* we may want to change that. How do we know if the BIOS
@@ -2369,26 +2368,13 @@
par->asleep = 1;
par->lock_blank = 1;
- /* Disable AGP. The AGP host should have done it, but since ordering
- * isn't always properly guaranteed in this specific case, let's make
- * sure it's disabled on card side now. Ultimately, when merging fbdev
- * and dri into some common infrastructure, this will be handled
- * more nicely. The host bridge side will (or will not) be dealt with
- * by the bridge AGP driver, we don't attempt to touch it here.
+#ifdef CONFIG_PPC_PMAC
+ /* On powermac, we have hooks to properly suspend/resume AGP now,
+ * use them here. We'll ultimately need some generic support here,
+ * but the generic code isn't quite ready for that yet
*/
- agp = pci_find_capability(pdev, PCI_CAP_ID_AGP);
- if (agp) {
- u32 cmd;
-
- pci_read_config_dword(pdev, agp + PCI_AGP_COMMAND, &cmd);
- if (cmd & PCI_AGP_COMMAND_AGP) {
- printk(KERN_INFO "aty128fb: AGP was enabled, "
- "disabling ...\n");
- cmd &= ~PCI_AGP_COMMAND_AGP;
- pci_write_config_dword(pdev, agp + PCI_AGP_COMMAND,
- cmd);
- }
- }
+ pmac_suspend_agp_for_card(pdev);
+#endif /* CONFIG_PPC_PMAC */
/* We need a way to make sure the fbdev layer will _not_ touch the
* framebuffer before we put the chip to suspend state. On 2.4, I
@@ -2432,6 +2418,14 @@
par->lock_blank = 0;
aty128fb_blank(0, info);
+#ifdef CONFIG_PPC_PMAC
+ /* On powermac, we have hooks to properly suspend/resume AGP now,
+ * use them here. We'll ultimately need some generic support here,
+ * but the generic code isn't quite ready for that yet
+ */
+ pmac_resume_agp_for_card(pdev);
+#endif /* CONFIG_PPC_PMAC */
+
pdev->dev.power.power_state = PMSG_ON;
printk(KERN_DEBUG "aty128fb: resumed !\n");
