On Thursday, April 03, 2008 2:23 pm Rafael J. Wysocki wrote: > > +static int i915_poweroff(struct device *dev) > > +{ > > + /* Shut down the device */ > > + pci_disable_device(dev->pdev); > > + pci_set_power_state(dev->pdev, PCI_D3hot); > > I think you may need to do that in ->suspend() too, as opposed to > ->freeze(), ... Because ->poweroff won't be called in the paths that do ->suspend? Ah yeah, must have skipped over that section of the documentation... > > > +} > > + > > +static struct pm_ops i915_pm_ops = { > > + .prepare = NULL, /* DRM core should prevent any new ioctls? */ > > + .complete = NULL, /* required to re-enable DRM client requests */ > > + .suspend = i915_save, > > + .resume = i915_restore, > > + .freeze = i915_save, > > ... so perhaps define ->suspend() as ->save() + ->poweroff()? Yep, I can just make a wrapper for it in the driver. Thanks a lot for making these changes to the core. My only worry is that all the old-style stuff will stick around forever... so fwiw you can add my Acked-by: Jesse Barnes <jesse.barnes@xxxxxxxxx> to the series. Thanks, Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html