| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <200804031150.55967.jbarnes@virtuousgeek.org>
Date: Thu, 3 Apr 2008 11:50:55 -0700
From: Jesse Barnes <jbarnes@...tuousgeek.org>
To: "Rafael J. Wysocki" <rjw@...k.pl>
Cc: pm list <linux-pm@...ts.linux-foundation.org>,
ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
Alan Stern <stern@...land.harvard.edu>,
Greg KH <greg@...ah.com>, Len Brown <lenb@...nel.org>,
LKML <linux-kernel@...r.kernel.org>,
Alexey Starikovskiy <astarikovskiy@...e.de>,
David Brownell <david-b@...bell.net>,
Pavel Machek <pavel@....cz>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Oliver Neukum <oliver@...kum.org>,
Nigel Cunningham <ncunningham@...a.org.au>,
Dave Airlie <airlied@...ux.ie>
Subject: Re: [RFC][PATCH 1/3] use new pm_ops in DRM drivers
On Tuesday, April 01, 2008 5:09 pm Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@...k.pl>
>
> 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);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists