On Saturday 09 January 2010, Jesse Barnes wrote:
> On Fri, 8 Jan 2010 16:50:57 -0800 (PST)
> Linus Torvalds <torvalds@...> wrote:
>
> >
> >
> > On Sat, 9 Jan 2010, Rafael J. Wysocki wrote:
> > >
> > > Which is functionally equivalent to my patch, because
> > > i915_suspend/resume() won't be called by drm_class_suspend/resume()
> > > in the KMS case anyway.
> >
> > Ahh, right you are - that class suspend function does a check for
> > DRIVER_MODESET, and only does the suspend/resume if it's not a
> > MODESET driver.
> >
> > Ok, so I withdraw my objections to your original patch - it's
> > confusing, but that's just because DRM is such a horrible mess with
> > subtle things.
>
> Yeah the non-KMS paths just suck.
>
> Acked-by: Jesse Barnes <jbarnes@...>
>
> Though hopefully you can get the PCI driver registration working w/o
> too much trouble; that would be even better.
Actually, I have a working patch, with one tiny detail I'm not sure of.
Namely, I need to call pci_set_drvdata(pdev, dev) unconditionally in drm_stub.c
for the things to work, but I _think_ it won't hurt even if we're not going to
use the pdev's private data.
The benefit of this is having just one code path for suspend/resume instead of
two different code paths depending on whether the driver is using the KMS or
not, which is well worth it IMO.
The patch is appended.
Rafael
---
From: Rafael J. Wysocki <rjw@...>
Subject: DRM / i915: Always register as a PCI driver
Commit cbda12d77ea590082edb6d30bd342a67ebc459e0 (drm/i915: implement
new pm ops for i915), among other things, removed the .suspend and
.resume pointers from the struct drm_driver object in i915_drv.c,
which broke resume without KMS on my MSI Wind U100.
The reason of the breakage is that in the non-KMS case i915 is
not registered as a PCI driver and uses device class suspend and
resume callbacks that, in turn, execute the callbacks defined in the
struct drm_driver object. Consequently, after commit cbda12d77ea59,
the driver's suspend and resume callbacks are not executed at all in
the non-KMS case.
To prevent this from happening, set DRIVER_MODESET unconditinally
in driver.driver_features before calling drm_init(), so that it
always registeres us as a PCI driver, and make i915_pci_probe()
reset DRIVER_MODESET in the non-KMS case. This way the driver's PCI
suspend and resume callbacks will always be used and the problem
will be avoided.
Signed-off-by: Rafael J. Wysocki <rjw@...>
---
drivers/gpu/drm/drm_stub.c | 3 +-
drivers/gpu/drm/i915/i915_drv.c | 44 +++++++++++++++++++++-------------------
2 files changed, 26 insertions(+), 21 deletions(-)
Index: linux-2.6/drivers/gpu/drm/i915/i915_drv.c
===================================================================
--- linux-2.6.orig/drivers/gpu/drm/i915/i915_drv.c
+++ linux-2.6/drivers/gpu/drm/i915/i915_drv.c
@@ -372,6 +372,28 @@ int i965_reset(struct drm_device *dev, u
static int __devinit
i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
{
+ /*
+ * If CONFIG_DRM_I915_KMS is set, default to KMS unless
+ * explicitly disabled with the module pararmeter.
+ *
+ * Otherwise, just follow the parameter (defaulting to off).
+ *
+ * Allow optional vga_text_mode_force boot option to override
+ * the default behavior.
+ */
+ if (i915_modeset == 0)
+ driver.driver_features &= ~DRIVER_MODESET;
+
+#ifndef CONFIG_DRM_I915_KMS
+ if (i915_modeset != 1)
+ driver.driver_features &= ~DRIVER_MODESET;
+#endif
+
+#ifdef CONFIG_VGA_CONSOLE
+ if (vgacon_text_force() && i915_modeset == -1)
+ driver.driver_features &= ~DRIVER_MODESET;
+#endif
+
return drm_get_dev(pdev, ent, &driver);
}
@@ -520,26 +542,8 @@ static int __init i915_init(void)
i915_gem_shrinker_init();
- /*
- * If CONFIG_DRM_I915_KMS is set, default to KMS unless
- * explicitly disabled with the module pararmeter.
- *
- * Otherwise, just follow the parameter (defaulting to off).
- *
- * Allow optional vga_text_mode_force boot option to override
- * the default behavior.
- */
-#if defined(CONFIG_DRM_I915_KMS)
- if (i915_modeset != 0)
- driver.driver_features |= DRIVER_MODESET;
-#endif
- if (i915_modeset == 1)
- driver.driver_features |= DRIVER_MODESET;
-
-#ifdef CONFIG_VGA_CONSOLE
- if (vgacon_text_force() && i915_modeset == -1)
- driver.driver_features &= ~DRIVER_MODESET;
-#endif
+ /* Make drm_init() always register us as a PCI driver. */
+ driver.driver_features |= DRIVER_MODESET;
return drm_init(&driver);
}
Index: linux-2.6/drivers/gpu/drm/drm_stub.c
===================================================================
--- linux-2.6.orig/drivers/gpu/drm/drm_stub.c
+++ linux-2.6/drivers/gpu/drm/drm_stub.c
@@ -419,8 +419,9 @@ int drm_get_dev(struct pci_dev *pdev, co
goto err_g2;
}
+ pci_set_drvdata(pdev, dev);
+
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
- pci_set_drvdata(pdev, dev);
ret = drm_get_minor(dev, &dev->control, DRM_MINOR_CONTROL);
if (ret)
goto err_g2;
|