On Tuesday, April 01, 2008 5:09 pm Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rjw@xxxxxxx> > > Introduce 'struct pm_ops' and 'struct pm_ext_ops' ('ext' meaning > 'extended') representing suspend and hibernation operations for bus > types, device classes, device types and device drivers. > > Modify the PM core to use 'struct pm_ops' and 'struct pm_ext_ops' > objects, if defined, instead of the ->suspend() and ->resume(), > ->suspend_late(), and ->resume_early() callbacks (the old callbacks > will be considered as legacy and gradually phased out). > > The main purpose of doing this is to separate suspend (aka S2RAM and > standby) callbacks from hibernation callbacks in such a way that the > new callbacks won't take arguments and the semantics of each of them > will be clearly specified. This has been requested for multiple > times by many people, including Linus himself, and the reason is that > within the current scheme if ->resume() is called, for example, it's > difficult to say why it's been called (ie. is it a resume from RAM or > from hibernation or a suspend/hibernation failure etc.?). I like the new ops much better; their purpose is clearer and better separated than before. I think the i915 changes should look something like this? Also, what about class devices? Right now, they just have suspend & resume callbacks, not full pm_ops structures. But maybe they're not really necessary anyway, I could set the pm_ops.prepare & complete callbacks to DRM core routines in order to suspend & resume DRM client requests... Also, it looks like the PCI bits I had in i915 aren't really necessary? Thanks, Jesse diff --git a/drivers/char/drm/i915_drv.c b/drivers/char/drm/i915_drv.c index b2b451d..ec6356a 100644 --- a/drivers/char/drm/i915_drv.c +++ b/drivers/char/drm/i915_drv.c @@ -239,8 +239,9 @@ static void i915_restore_vga(struct drm_device *dev) } -static int i915_suspend(struct drm_device *dev, pm_message_t state) +static int i915_save(struct device *device) { + struct drm_device *dev = container_of(device, struct drm_device, dev); struct drm_i915_private *dev_priv = dev->dev_private; int i; @@ -250,10 +251,6 @@ static int i915_suspend(struct drm_device *dev, pm_message_t state) return -ENODEV; } - if (state.event == PM_EVENT_PRETHAW) - return 0; - - pci_save_state(dev->pdev); pci_read_config_byte(dev->pdev, LBB, &dev_priv->saveLBB); /* Pipe & plane A info */ @@ -367,24 +364,16 @@ static int i915_suspend(struct drm_device *dev, pm_message_t state) i915_save_vga(dev); - if (state.event == PM_EVENT_SUSPEND) { - /* Shut down the device */ - pci_disable_device(dev->pdev); - pci_set_power_state(dev->pdev, PCI_D3hot); - } - return 0; } -static int i915_resume(struct drm_device *dev) +static int i915_restore(struct device *device) { + struct drm_device *dev = container_of(device, struct drm_device, dev); struct drm_i915_private *dev_priv = dev->dev_private; int i; pci_set_power_state(dev->pdev, PCI_D0); - pci_restore_state(dev->pdev); - if (pci_enable_device(dev->pdev)) - return -1; pci_write_config_byte(dev->pdev, LBB, dev_priv->saveLBB); @@ -527,6 +516,23 @@ static int i915_resume(struct drm_device *dev) return 0; } +static int i915_poweroff(struct device *dev) +{ + /* Shut down the device */ + pci_disable_device(dev->pdev); + pci_set_power_state(dev->pdev, PCI_D3hot); +} + +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, + .restore = i915_restore, + .poweroff = i915_poweroff, +}; + static struct drm_driver driver = { /* don't use mtrr's here, the Xserver or user space app should * deal with them for intel hardware. @@ -539,8 +545,6 @@ static struct drm_driver driver = { .unload = i915_driver_unload, .lastclose = i915_driver_lastclose, .preclose = i915_driver_preclose, - .suspend = i915_suspend, - .resume = i915_resume, .device_is_agp = i915_driver_device_is_agp, .vblank_wait = i915_driver_vblank_wait, .vblank_wait2 = i915_driver_vblank_wait2, @@ -581,6 +585,7 @@ static struct drm_driver driver = { static int __init i915_init(void) { driver.num_ioctls = i915_max_ioctl; + driver->dev.pm_ops = &i915_pm_ops; return drm_init(&driver); } _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm