On Saturday 09 January 2010, Jesse Barnes wrote: > On Fri, 8 Jan 2010 16:01:46 -0800 (PST) > Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > > > > On Sat, 9 Jan 2010, Rafael J. Wysocki wrote: > > > > > > From: Rafael J. Wysocki <rjw@xxxxxxx> > > > > > > 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. > > > > > > Fix this by reverting that part of commit cbda12d77ea59. > > > > Hmm. I get the feeling that perhaps the of the drm_driver callbacks > > was very muchintentional, and that the code presumably wants to be > > called purely through the PCI layer, and not through the "drm class" > > logic at all? > > > > Your patch seems like it would always execute the silly class suspend > > even though we explicitly don't want to. And a much nicer fix would > > seem to register the thing properly as a PCI driver even if you don't > > then use KMS. > > > > So it looks to me like the problem is that drm_init() will register > > the driver as a real PCI driver only if > > > > driver->driver_features & DRIVER_MODESET > > > > and otherwise it does that very odd "stealth mode manual scanning" > > thing which doesn't register it as a proper PCI driver. > > > > So could we instead make that "disable KSM" _just_ disable the mode > > setting part, not disable the "I'm a real driver" part? > > This is the minimal fix I think (totally untested): > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > b/drivers/gpu/drm/i915/i915_drv.c index a0a2cad..1364c3e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -541,6 +541,11 @@ static int __init i915_init(void) > driver.driver_features &= ~DRIVER_MODESET; > #endif > > + if (!(driver.driver_features & DRIVER_MODESET)) { > + driver.suspend = i915_suspend; > + driver.resume = i915_resume; > + } > + > return drm_init(&driver); > } 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. Rafael _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm