Re: [RFC] full suspend/resume support for i915 DRM driver
On Fri, 2007-10-26 at 11:12 -0700, Jesse Barnes wrote: > On Friday, October 26, 2007 10:10 am Kay Sievers wrote: > > The conversion is already queued in Greg's tree, and in -mm: > > http://git.kernel.org/?p=linux/kernel/git/gregkh/patches.git;a=blob;f > >=driver/drm-convert-from-class_device-to-device-in-drivers-char-drm.pa > >tch;h=f993183d1cb017f981cc2232d17930af40459bd8;hb=HEAD > > Hm, I've done it slightly differently, by adding an actual device to the > drm_device structure... I'll post it once I've cleaned it up a little > more. Sure, as long as "struct class_device" is no longer used in the code, and you don't embed two "struct device" device structures in a single object, all should be fine. We plan to remove "struct class_device" soon from the kernel. :) Thanks, Kay - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Friday, October 26, 2007 10:10 am Kay Sievers wrote: > The conversion is already queued in Greg's tree, and in -mm: > http://git.kernel.org/?p=linux/kernel/git/gregkh/patches.git;a=blob;f >=driver/drm-convert-from-class_device-to-device-in-drivers-char-drm.pa >tch;h=f993183d1cb017f981cc2232d17930af40459bd8;hb=HEAD Hm, I've done it slightly differently, by adding an actual device to the drm_device structure... I'll post it once I've cleaned it up a little more. > /sys/class/drm will look the same as with the class_device's, only if > !CONFIG_SYSFS_DEPRECATED, there will be symlinks instead of > directories, otherwise the same pathes, like for all other > (converted) classes too. Ok, thanks. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On 10/26/07, Jesse Barnes <[EMAIL PROTECTED]> wrote: > On Thursday, October 25, 2007 9:59 pm Greg KH wrote: > > On Thu, Oct 25, 2007 at 04:53:18PM -0700, Jesse Barnes wrote: > > > Ok, here's yet another version that uses the device model for the > > > suspend/resume, rather than pci hooks. > > > > > > Greg, DRM desperately needs review of its device model usage, can > > > you take a look at this patch and the current drm_sysfs.c code? > > > Right now, we're mixing class_devices and regular devices (the > > > latter seem to be required for suspend/resume to work correctly), > > > but this seems wrong. Any ideas? Should we just rip out the > > > class_device stuff and create full-on DRM device nodes? > > > > The class_device stuff is already ripped out in the latest -mm trees > > and I will be forwarding that change on for 2.6.25 after 2.6.24 is > > out. So yes, it should be taken away :) > > > > But converting from class_device to struct device does not mean you > > use a "device node". But you could if you want to :) > > Yeah, bad choice of words. :) > > To retain compatibility, we need to have directories under the DRM class > dir (/sys/class/drm) for each card (e.g. card0) that contains a file > describing which graphics driver is bound to the device. For class > devices, we could just add an attributes structure to the device. Can > we do the same with regular, non-class devices? The conversion is already queued in Greg's tree, and in -mm: http://git.kernel.org/?p=linux/kernel/git/gregkh/patches.git;a=blob;f=driver/drm-convert-from-class_device-to-device-in-drivers-char-drm.patch;h=f993183d1cb017f981cc2232d17930af40459bd8;hb=HEAD /sys/class/drm will look the same as with the class_device's, only if !CONFIG_SYSFS_DEPRECATED, there will be symlinks instead of directories, otherwise the same pathes, like for all other (converted) classes too. Thanks, Kay - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Thursday, October 25, 2007 9:59 pm Greg KH wrote: > On Thu, Oct 25, 2007 at 04:53:18PM -0700, Jesse Barnes wrote: > > Ok, here's yet another version that uses the device model for the > > suspend/resume, rather than pci hooks. > > > > Greg, DRM desperately needs review of its device model usage, can > > you take a look at this patch and the current drm_sysfs.c code? > > Right now, we're mixing class_devices and regular devices (the > > latter seem to be required for suspend/resume to work correctly), > > but this seems wrong. Any ideas? Should we just rip out the > > class_device stuff and create full-on DRM device nodes? > > The class_device stuff is already ripped out in the latest -mm trees > and I will be forwarding that change on for 2.6.25 after 2.6.24 is > out. So yes, it should be taken away :) > > But converting from class_device to struct device does not mean you > use a "device node". But you could if you want to :) Yeah, bad choice of words. :) To retain compatibility, we need to have directories under the DRM class dir (/sys/class/drm) for each card (e.g. card0) that contains a file describing which graphics driver is bound to the device. For class devices, we could just add an attributes structure to the device. Can we do the same with regular, non-class devices? Thanks, Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Thursday, October 25, 2007 9:59 pm Greg KH wrote: On Thu, Oct 25, 2007 at 04:53:18PM -0700, Jesse Barnes wrote: Ok, here's yet another version that uses the device model for the suspend/resume, rather than pci hooks. Greg, DRM desperately needs review of its device model usage, can you take a look at this patch and the current drm_sysfs.c code? Right now, we're mixing class_devices and regular devices (the latter seem to be required for suspend/resume to work correctly), but this seems wrong. Any ideas? Should we just rip out the class_device stuff and create full-on DRM device nodes? The class_device stuff is already ripped out in the latest -mm trees and I will be forwarding that change on for 2.6.25 after 2.6.24 is out. So yes, it should be taken away :) But converting from class_device to struct device does not mean you use a device node. But you could if you want to :) Yeah, bad choice of words. :) To retain compatibility, we need to have directories under the DRM class dir (/sys/class/drm) for each card (e.g. card0) that contains a file describing which graphics driver is bound to the device. For class devices, we could just add an attributes structure to the device. Can we do the same with regular, non-class devices? Thanks, Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On 10/26/07, Jesse Barnes [EMAIL PROTECTED] wrote: On Thursday, October 25, 2007 9:59 pm Greg KH wrote: On Thu, Oct 25, 2007 at 04:53:18PM -0700, Jesse Barnes wrote: Ok, here's yet another version that uses the device model for the suspend/resume, rather than pci hooks. Greg, DRM desperately needs review of its device model usage, can you take a look at this patch and the current drm_sysfs.c code? Right now, we're mixing class_devices and regular devices (the latter seem to be required for suspend/resume to work correctly), but this seems wrong. Any ideas? Should we just rip out the class_device stuff and create full-on DRM device nodes? The class_device stuff is already ripped out in the latest -mm trees and I will be forwarding that change on for 2.6.25 after 2.6.24 is out. So yes, it should be taken away :) But converting from class_device to struct device does not mean you use a device node. But you could if you want to :) Yeah, bad choice of words. :) To retain compatibility, we need to have directories under the DRM class dir (/sys/class/drm) for each card (e.g. card0) that contains a file describing which graphics driver is bound to the device. For class devices, we could just add an attributes structure to the device. Can we do the same with regular, non-class devices? The conversion is already queued in Greg's tree, and in -mm: http://git.kernel.org/?p=linux/kernel/git/gregkh/patches.git;a=blob;f=driver/drm-convert-from-class_device-to-device-in-drivers-char-drm.patch;h=f993183d1cb017f981cc2232d17930af40459bd8;hb=HEAD /sys/class/drm will look the same as with the class_device's, only if !CONFIG_SYSFS_DEPRECATED, there will be symlinks instead of directories, otherwise the same pathes, like for all other (converted) classes too. Thanks, Kay - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Friday, October 26, 2007 10:10 am Kay Sievers wrote: The conversion is already queued in Greg's tree, and in -mm: http://git.kernel.org/?p=linux/kernel/git/gregkh/patches.git;a=blob;f =driver/drm-convert-from-class_device-to-device-in-drivers-char-drm.pa tch;h=f993183d1cb017f981cc2232d17930af40459bd8;hb=HEAD Hm, I've done it slightly differently, by adding an actual device to the drm_device structure... I'll post it once I've cleaned it up a little more. /sys/class/drm will look the same as with the class_device's, only if !CONFIG_SYSFS_DEPRECATED, there will be symlinks instead of directories, otherwise the same pathes, like for all other (converted) classes too. Ok, thanks. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Fri, 2007-10-26 at 11:12 -0700, Jesse Barnes wrote: On Friday, October 26, 2007 10:10 am Kay Sievers wrote: The conversion is already queued in Greg's tree, and in -mm: http://git.kernel.org/?p=linux/kernel/git/gregkh/patches.git;a=blob;f =driver/drm-convert-from-class_device-to-device-in-drivers-char-drm.pa tch;h=f993183d1cb017f981cc2232d17930af40459bd8;hb=HEAD Hm, I've done it slightly differently, by adding an actual device to the drm_device structure... I'll post it once I've cleaned it up a little more. Sure, as long as struct class_device is no longer used in the code, and you don't embed two struct device device structures in a single object, all should be fine. We plan to remove struct class_device soon from the kernel. :) Thanks, Kay - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Thu, Oct 25, 2007 at 04:53:18PM -0700, Jesse Barnes wrote: > Ok, here's yet another version that uses the device model for the > suspend/resume, rather than pci hooks. > > Greg, DRM desperately needs review of its device model usage, can you > take a look at this patch and the current drm_sysfs.c code? Right now, > we're mixing class_devices and regular devices (the latter seem to be > required for suspend/resume to work correctly), but this seems wrong. > Any ideas? Should we just rip out the class_device stuff and create > full-on DRM device nodes? The class_device stuff is already ripped out in the latest -mm trees and I will be forwarding that change on for 2.6.25 after 2.6.24 is out. So yes, it should be taken away :) But converting from class_device to struct device does not mean you use a "device node". But you could if you want to :) Other than that, the driver model usage isn't the best, but I think the majority of the major issues are fixed up now, especially with the removal of class_device. Is there anything specific you are curious about? thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
Ok, here's yet another version that uses the device model for the
suspend/resume, rather than pci hooks.
Greg, DRM desperately needs review of its device model usage, can you
take a look at this patch and the current drm_sysfs.c code? Right now,
we're mixing class_devices and regular devices (the latter seem to be
required for suspend/resume to work correctly), but this seems wrong.
Any ideas? Should we just rip out the class_device stuff and create
full-on DRM device nodes?
This one also includes the feedback from Pavel & Rafael, along with
some fixes that were causing crashes during module unload or X startup.
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index d0ab2c9..39fce95 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -619,6 +619,8 @@ struct drm_driver {
void (*postclose) (struct drm_device *, struct drm_file *);
void (*lastclose) (struct drm_device *);
int (*unload) (struct drm_device *);
+ int (*suspend) (struct drm_device *);
+ int (*resume) (struct drm_device *);
int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file
*file_priv);
void (*dma_ready) (struct drm_device *);
int (*dma_quiescent) (struct drm_device *);
@@ -697,6 +699,7 @@ struct drm_head {
* may contain multiple heads.
*/
struct drm_device {
+ struct device dev; /**< Linux device */
char *unique; /**< Unique identifier: e.g., busid */
int unique_len; /**< Length of unique field */
char *devname; /**< For /proc/interrupts */
@@ -1164,7 +1167,8 @@ extern void drm_pci_free(struct drm_device *dev,
drm_dma_handle_t *dmah);
struct drm_sysfs_class;
extern struct class *drm_sysfs_create(struct module *owner, char *name);
extern void drm_sysfs_destroy(struct class *cs);
-extern struct class_device *drm_sysfs_device_add(struct class *cs,
+extern struct class_device *drm_sysfs_device_add(struct drm_device *dev,
+struct class *cs,
struct drm_head * head);
extern void drm_sysfs_device_remove(struct class_device *class_dev);
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
diff --git a/linux-core/drm_stub.c b/linux-core/drm_stub.c
index 9e140ac..b0b1001 100644
--- a/linux-core/drm_stub.c
+++ b/linux-core/drm_stub.c
@@ -183,7 +183,8 @@ static int drm_get_head(struct drm_device * dev, struct
drm_head * head)
goto err_g1;
}
- head->dev_class = drm_sysfs_device_add(drm_class, head);
+ head->dev_class = drm_sysfs_device_add(dev, drm_class,
+ head);
if (IS_ERR(head->dev_class)) {
printk(KERN_ERR
"DRM: Error sysfs_device_add.\n");
diff --git a/linux-core/drm_sysfs.c b/linux-core/drm_sysfs.c
index cf4349b..ce83c24 100644
--- a/linux-core/drm_sysfs.c
+++ b/linux-core/drm_sysfs.c
@@ -19,6 +19,30 @@
#include "drm_core.h"
#include "drmP.h"
+#define to_drm_device(d) container_of(d, struct drm_device, dev)
+
+static int drm_sysfs_suspend(struct device *dev, pm_message_t state)
+{
+ struct drm_device *drm_dev = to_drm_device(dev);
+
+ printk(KERN_ERR "%s\n", __FUNCTION__);
+
+ if (drm_dev->driver->suspend)
+ return drm_dev->driver->suspend(drm_dev);
+
+ return 0;
+}
+
+static int drm_sysfs_resume(struct device *dev)
+{
+ struct drm_device *drm_dev = to_drm_device(dev);
+
+ if (drm_dev->driver->resume)
+ return drm_dev->driver->resume(drm_dev);
+
+ return 0;
+}
+
/* Display the version of drm_core. This doesn't work right in current design
*/
static ssize_t version_show(struct class *dev, char *buf)
{
@@ -50,6 +74,9 @@ struct class *drm_sysfs_create(struct module *owner, char
*name)
goto err_out;
}
+ class->suspend = drm_sysfs_suspend;
+ class->resume = drm_sysfs_resume;
+
err = class_create_file(class, _attr_version);
if (err)
goto err_out_class;
@@ -73,7 +100,6 @@ void drm_sysfs_destroy(struct class *class)
{
if ((class == NULL) || (IS_ERR(class)))
return;
-
class_remove_file(class, _attr_version);
class_destroy(class);
}
@@ -104,10 +130,14 @@ static struct class_device_attribute class_device_attrs[]
= {
* Note: the struct class passed to this function must have previously been
* created with a call to drm_sysfs_create().
*/
-struct class_device *drm_sysfs_device_add(struct class *cs, struct drm_head
*head)
+struct class_device *drm_sysfs_device_add(struct drm_device *dev,
+ struct class *cs,
+
Re: [RFC] full suspend/resume support for i915 DRM driver
Ok, here's yet another version that uses the device model for the
suspend/resume, rather than pci hooks.
Greg, DRM desperately needs review of its device model usage, can you
take a look at this patch and the current drm_sysfs.c code? Right now,
we're mixing class_devices and regular devices (the latter seem to be
required for suspend/resume to work correctly), but this seems wrong.
Any ideas? Should we just rip out the class_device stuff and create
full-on DRM device nodes?
This one also includes the feedback from Pavel Rafael, along with
some fixes that were causing crashes during module unload or X startup.
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index d0ab2c9..39fce95 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -619,6 +619,8 @@ struct drm_driver {
void (*postclose) (struct drm_device *, struct drm_file *);
void (*lastclose) (struct drm_device *);
int (*unload) (struct drm_device *);
+ int (*suspend) (struct drm_device *);
+ int (*resume) (struct drm_device *);
int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file
*file_priv);
void (*dma_ready) (struct drm_device *);
int (*dma_quiescent) (struct drm_device *);
@@ -697,6 +699,7 @@ struct drm_head {
* may contain multiple heads.
*/
struct drm_device {
+ struct device dev; /** Linux device */
char *unique; /** Unique identifier: e.g., busid */
int unique_len; /** Length of unique field */
char *devname; /** For /proc/interrupts */
@@ -1164,7 +1167,8 @@ extern void drm_pci_free(struct drm_device *dev,
drm_dma_handle_t *dmah);
struct drm_sysfs_class;
extern struct class *drm_sysfs_create(struct module *owner, char *name);
extern void drm_sysfs_destroy(struct class *cs);
-extern struct class_device *drm_sysfs_device_add(struct class *cs,
+extern struct class_device *drm_sysfs_device_add(struct drm_device *dev,
+struct class *cs,
struct drm_head * head);
extern void drm_sysfs_device_remove(struct class_device *class_dev);
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
diff --git a/linux-core/drm_stub.c b/linux-core/drm_stub.c
index 9e140ac..b0b1001 100644
--- a/linux-core/drm_stub.c
+++ b/linux-core/drm_stub.c
@@ -183,7 +183,8 @@ static int drm_get_head(struct drm_device * dev, struct
drm_head * head)
goto err_g1;
}
- head-dev_class = drm_sysfs_device_add(drm_class, head);
+ head-dev_class = drm_sysfs_device_add(dev, drm_class,
+ head);
if (IS_ERR(head-dev_class)) {
printk(KERN_ERR
DRM: Error sysfs_device_add.\n);
diff --git a/linux-core/drm_sysfs.c b/linux-core/drm_sysfs.c
index cf4349b..ce83c24 100644
--- a/linux-core/drm_sysfs.c
+++ b/linux-core/drm_sysfs.c
@@ -19,6 +19,30 @@
#include drm_core.h
#include drmP.h
+#define to_drm_device(d) container_of(d, struct drm_device, dev)
+
+static int drm_sysfs_suspend(struct device *dev, pm_message_t state)
+{
+ struct drm_device *drm_dev = to_drm_device(dev);
+
+ printk(KERN_ERR %s\n, __FUNCTION__);
+
+ if (drm_dev-driver-suspend)
+ return drm_dev-driver-suspend(drm_dev);
+
+ return 0;
+}
+
+static int drm_sysfs_resume(struct device *dev)
+{
+ struct drm_device *drm_dev = to_drm_device(dev);
+
+ if (drm_dev-driver-resume)
+ return drm_dev-driver-resume(drm_dev);
+
+ return 0;
+}
+
/* Display the version of drm_core. This doesn't work right in current design
*/
static ssize_t version_show(struct class *dev, char *buf)
{
@@ -50,6 +74,9 @@ struct class *drm_sysfs_create(struct module *owner, char
*name)
goto err_out;
}
+ class-suspend = drm_sysfs_suspend;
+ class-resume = drm_sysfs_resume;
+
err = class_create_file(class, class_attr_version);
if (err)
goto err_out_class;
@@ -73,7 +100,6 @@ void drm_sysfs_destroy(struct class *class)
{
if ((class == NULL) || (IS_ERR(class)))
return;
-
class_remove_file(class, class_attr_version);
class_destroy(class);
}
@@ -104,10 +130,14 @@ static struct class_device_attribute class_device_attrs[]
= {
* Note: the struct class passed to this function must have previously been
* created with a call to drm_sysfs_create().
*/
-struct class_device *drm_sysfs_device_add(struct class *cs, struct drm_head
*head)
+struct class_device *drm_sysfs_device_add(struct drm_device *dev,
+ struct class *cs,
+
Re: [RFC] full suspend/resume support for i915 DRM driver
On Thu, Oct 25, 2007 at 04:53:18PM -0700, Jesse Barnes wrote: Ok, here's yet another version that uses the device model for the suspend/resume, rather than pci hooks. Greg, DRM desperately needs review of its device model usage, can you take a look at this patch and the current drm_sysfs.c code? Right now, we're mixing class_devices and regular devices (the latter seem to be required for suspend/resume to work correctly), but this seems wrong. Any ideas? Should we just rip out the class_device stuff and create full-on DRM device nodes? The class_device stuff is already ripped out in the latest -mm trees and I will be forwarding that change on for 2.6.25 after 2.6.24 is out. So yes, it should be taken away :) But converting from class_device to struct device does not mean you use a device node. But you could if you want to :) Other than that, the driver model usage isn't the best, but I think the majority of the major issues are fixed up now, especially with the removal of class_device. Is there anything specific you are curious about? thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Wednesday, October 24, 2007 1:17 pm Adrian Bunk wrote: > > diff --git a/linux-core/Kconfig b/linux-core/Kconfig > > index 2d02c76..5e73fc7 100644 > > --- a/linux-core/Kconfig > > +++ b/linux-core/Kconfig > > @@ -50,7 +50,7 @@ config DRM_I810 > > > > choice > > prompt "Intel 830M, 845G, 852GM, 855GM, 865G" > > - depends on DRM && AGP && AGP_INTEL > > + depends on DRM && AGP && AGP_INTEL && !FB_INTEL > > optional > >... > > This sounds like a bad regression. Well it's really documenting existing behavior. Unless you're very careful, running custom applications, the Intel FB and DRM drivers will conflict. IMO this is a long overdue change, but Dave has some custom stuff that requires both drivers so he'd rather not see it go in, so I'm fine with dropping this part. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Wednesday, 24 October 2007 17:18, Jesse Barnes wrote: > On Wednesday, October 24, 2007 6:35:14 am Pavel Machek wrote: > > Hi! > > > > > We seem to see a lot of bug reports along the lines of, "my machine > > > resumes but I can't see X" or, "I can see X but only with a bright > > > flashlight", etc. These sorts of problems are due to the fact that > > > the X server isn't designed to do full state save/restore, and none > > > of the available kernel drivers do it on its behalf. > > > > > > Since intelfb and the rest of the Intel drivers are fairly incompatible, > > > this patch makes the DRM bind to the PCI device so it can register real > > > suspend/resume handlers. Those handlers take care of saving and > > > restoring enough state for X to come back reliably on at least one of my > > > problematic test machines, but text mode still has trouble (still > > > debugging VGA state save/restore, including trying to save/restore > > > actual VRAM contents for possible hibernate support). > > > > > > How does this approach look? Is a new DRM driver flag a good thing for > > > similar situations with other drivers? Thoughts? > > > > Looks okay to me... from very quick look. > > > > > + if (!i915_pipe_enabled(dev, pipe)) > > > + return; > > > + > > > + if (pipe == PIPE_A) > > > + array = dev_priv->savePaletteA; > > > > coding style, we probably want save_palette_A. > > Yeah, I tried not to pull over uglies from the X code but I guess I forgot > this bit. I should also update the copyright. > > > > + unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B; > > > > Uff. Mixing = and == and ? in one expression is evil. > > I could put parens around it if you think that would help, or just move it to > a new line... I'd use parens around "pipe == PIPE_A". Greetings, Rafael - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Mon, Oct 22, 2007 at 09:15:43PM -0700, Jesse Barnes wrote: > On Friday, October 19, 2007, Jesse Barnes wrote: > > Dave can you take a look at the new flag and also see what you think > > about supporting suspend/resume in the event X hasn't started yet? > > There's some #if 0'd code to support that case, but I haven't tested > > it. > > Ok Dave, this one's been updated with support for suspend/resume with > or without X running (iow I removed the requirement that X initialize > the driver, which DRM drivers usually have). > > Hope it looks alright, it could definitely use testing on more > machines though... > > Jesse > > diff --git a/linux-core/Kconfig b/linux-core/Kconfig > index 2d02c76..5e73fc7 100644 > --- a/linux-core/Kconfig > +++ b/linux-core/Kconfig > @@ -50,7 +50,7 @@ config DRM_I810 > > choice > prompt "Intel 830M, 845G, 852GM, 855GM, 865G" > - depends on DRM && AGP && AGP_INTEL > + depends on DRM && AGP && AGP_INTEL && !FB_INTEL > optional >... This sounds like a bad regression. cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Wednesday, October 24, 2007 6:35:14 am Pavel Machek wrote: > Hi! > > > We seem to see a lot of bug reports along the lines of, "my machine > > resumes but I can't see X" or, "I can see X but only with a bright > > flashlight", etc. These sorts of problems are due to the fact that > > the X server isn't designed to do full state save/restore, and none > > of the available kernel drivers do it on its behalf. > > > > Since intelfb and the rest of the Intel drivers are fairly incompatible, > > this patch makes the DRM bind to the PCI device so it can register real > > suspend/resume handlers. Those handlers take care of saving and > > restoring enough state for X to come back reliably on at least one of my > > problematic test machines, but text mode still has trouble (still > > debugging VGA state save/restore, including trying to save/restore > > actual VRAM contents for possible hibernate support). > > > > How does this approach look? Is a new DRM driver flag a good thing for > > similar situations with other drivers? Thoughts? > > Looks okay to me... from very quick look. > > > + if (!i915_pipe_enabled(dev, pipe)) > > + return; > > + > > + if (pipe == PIPE_A) > > + array = dev_priv->savePaletteA; > > coding style, we probably want save_palette_A. Yeah, I tried not to pull over uglies from the X code but I guess I forgot this bit. I should also update the copyright. > > + unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B; > > Uff. Mixing = and == and ? in one expression is evil. I could put parens around it if you think that would help, or just move it to a new line... > I think I seen some #if 0 code just remove that. Oh I left some in there? Yeah I'll remove it. Thanks, Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
Hi! > We seem to see a lot of bug reports along the lines of, "my machine > resumes but I can't see X" or, "I can see X but only with a bright > flashlight", etc. These sorts of problems are due to the fact that > the X server isn't designed to do full state save/restore, and none > of the available kernel drivers do it on its behalf. > > Since intelfb and the rest of the Intel drivers are fairly incompatible, > this patch makes the DRM bind to the PCI device so it can register real > suspend/resume handlers. Those handlers take care of saving and > restoring enough state for X to come back reliably on at least one of my > problematic test machines, but text mode still has trouble (still > debugging VGA state save/restore, including trying to save/restore > actual VRAM contents for possible hibernate support). > > How does this approach look? Is a new DRM driver flag a good thing for > similar situations with other drivers? Thoughts? Looks okay to me... from very quick look. > + if (!i915_pipe_enabled(dev, pipe)) > + return; > + > + if (pipe == PIPE_A) > + array = dev_priv->savePaletteA; coding style, we probably want save_palette_A. > + unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B; Uff. Mixing = and == and ? in one expression is evil. I think I seen some #if 0 code just remove that. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
Hi! We seem to see a lot of bug reports along the lines of, my machine resumes but I can't see X or, I can see X but only with a bright flashlight, etc. These sorts of problems are due to the fact that the X server isn't designed to do full state save/restore, and none of the available kernel drivers do it on its behalf. Since intelfb and the rest of the Intel drivers are fairly incompatible, this patch makes the DRM bind to the PCI device so it can register real suspend/resume handlers. Those handlers take care of saving and restoring enough state for X to come back reliably on at least one of my problematic test machines, but text mode still has trouble (still debugging VGA state save/restore, including trying to save/restore actual VRAM contents for possible hibernate support). How does this approach look? Is a new DRM driver flag a good thing for similar situations with other drivers? Thoughts? Looks okay to me... from very quick look. + if (!i915_pipe_enabled(dev, pipe)) + return; + + if (pipe == PIPE_A) + array = dev_priv-savePaletteA; coding style, we probably want save_palette_A. + unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B; Uff. Mixing = and == and ? in one expression is evil. I think I seen some #if 0 code just remove that. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Wednesday, October 24, 2007 6:35:14 am Pavel Machek wrote: Hi! We seem to see a lot of bug reports along the lines of, my machine resumes but I can't see X or, I can see X but only with a bright flashlight, etc. These sorts of problems are due to the fact that the X server isn't designed to do full state save/restore, and none of the available kernel drivers do it on its behalf. Since intelfb and the rest of the Intel drivers are fairly incompatible, this patch makes the DRM bind to the PCI device so it can register real suspend/resume handlers. Those handlers take care of saving and restoring enough state for X to come back reliably on at least one of my problematic test machines, but text mode still has trouble (still debugging VGA state save/restore, including trying to save/restore actual VRAM contents for possible hibernate support). How does this approach look? Is a new DRM driver flag a good thing for similar situations with other drivers? Thoughts? Looks okay to me... from very quick look. + if (!i915_pipe_enabled(dev, pipe)) + return; + + if (pipe == PIPE_A) + array = dev_priv-savePaletteA; coding style, we probably want save_palette_A. Yeah, I tried not to pull over uglies from the X code but I guess I forgot this bit. I should also update the copyright. + unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B; Uff. Mixing = and == and ? in one expression is evil. I could put parens around it if you think that would help, or just move it to a new line... I think I seen some #if 0 code just remove that. Oh I left some in there? Yeah I'll remove it. Thanks, Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Mon, Oct 22, 2007 at 09:15:43PM -0700, Jesse Barnes wrote: On Friday, October 19, 2007, Jesse Barnes wrote: Dave can you take a look at the new flag and also see what you think about supporting suspend/resume in the event X hasn't started yet? There's some #if 0'd code to support that case, but I haven't tested it. Ok Dave, this one's been updated with support for suspend/resume with or without X running (iow I removed the requirement that X initialize the driver, which DRM drivers usually have). Hope it looks alright, it could definitely use testing on more machines though... Jesse diff --git a/linux-core/Kconfig b/linux-core/Kconfig index 2d02c76..5e73fc7 100644 --- a/linux-core/Kconfig +++ b/linux-core/Kconfig @@ -50,7 +50,7 @@ config DRM_I810 choice prompt Intel 830M, 845G, 852GM, 855GM, 865G - depends on DRM AGP AGP_INTEL + depends on DRM AGP AGP_INTEL !FB_INTEL optional ... This sounds like a bad regression. cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Wednesday, 24 October 2007 17:18, Jesse Barnes wrote: On Wednesday, October 24, 2007 6:35:14 am Pavel Machek wrote: Hi! We seem to see a lot of bug reports along the lines of, my machine resumes but I can't see X or, I can see X but only with a bright flashlight, etc. These sorts of problems are due to the fact that the X server isn't designed to do full state save/restore, and none of the available kernel drivers do it on its behalf. Since intelfb and the rest of the Intel drivers are fairly incompatible, this patch makes the DRM bind to the PCI device so it can register real suspend/resume handlers. Those handlers take care of saving and restoring enough state for X to come back reliably on at least one of my problematic test machines, but text mode still has trouble (still debugging VGA state save/restore, including trying to save/restore actual VRAM contents for possible hibernate support). How does this approach look? Is a new DRM driver flag a good thing for similar situations with other drivers? Thoughts? Looks okay to me... from very quick look. + if (!i915_pipe_enabled(dev, pipe)) + return; + + if (pipe == PIPE_A) + array = dev_priv-savePaletteA; coding style, we probably want save_palette_A. Yeah, I tried not to pull over uglies from the X code but I guess I forgot this bit. I should also update the copyright. + unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B; Uff. Mixing = and == and ? in one expression is evil. I could put parens around it if you think that would help, or just move it to a new line... I'd use parens around pipe == PIPE_A. Greetings, Rafael - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Wednesday, October 24, 2007 1:17 pm Adrian Bunk wrote: diff --git a/linux-core/Kconfig b/linux-core/Kconfig index 2d02c76..5e73fc7 100644 --- a/linux-core/Kconfig +++ b/linux-core/Kconfig @@ -50,7 +50,7 @@ config DRM_I810 choice prompt Intel 830M, 845G, 852GM, 855GM, 865G - depends on DRM AGP AGP_INTEL + depends on DRM AGP AGP_INTEL !FB_INTEL optional ... This sounds like a bad regression. Well it's really documenting existing behavior. Unless you're very careful, running custom applications, the Intel FB and DRM drivers will conflict. IMO this is a long overdue change, but Dave has some custom stuff that requires both drivers so he'd rather not see it go in, so I'm fine with dropping this part. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Friday, October 19, 2007, Jesse Barnes wrote:
> Dave can you take a look at the new flag and also see what you think
> about supporting suspend/resume in the event X hasn't started yet?
> There's some #if 0'd code to support that case, but I haven't tested
> it.
Ok Dave, this one's been updated with support for suspend/resume with
or without X running (iow I removed the requirement that X initialize
the driver, which DRM drivers usually have).
Hope it looks alright, it could definitely use testing on more
machines though...
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
index 2d02c76..5e73fc7 100644
--- a/linux-core/Kconfig
+++ b/linux-core/Kconfig
@@ -50,7 +50,7 @@ config DRM_I810
choice
prompt "Intel 830M, 845G, 852GM, 855GM, 865G"
- depends on DRM && AGP && AGP_INTEL
+ depends on DRM && AGP && AGP_INTEL && !FB_INTEL
optional
config DRM_I915
@@ -60,6 +60,9 @@ config DRM_I915
852GM, 855GM, 865G, 915G, 915GM, 945G, 945GM and 965G integrated
graphics. If M is selected, the module will be called i915.
AGP support is required for this driver to work.
+
+ Note that this driver is incompatible with the Intel framebuffer
+ driver.
endchoice
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index f8ca3f4..36ce266 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -106,6 +106,7 @@ struct drm_file;
#define DRIVER_DMA_QUEUE 0x200
#define DRIVER_FB_DMA 0x400
#define DRIVER_IRQ_VBL20x800
+#define DRIVER_BIND_PCI0x1000
/[EMAIL PROTECTED]/
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index a09fa96..3d14fb4 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -340,9 +340,10 @@ int drm_init(struct drm_driver *driver,
}
}
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver->driver_features & DRIVER_BIND_PCI)) {
+ printk(KERN_INFO "DRM binding to PCI device\n");
return pci_register_driver(>pci_driver);
- else {
+ } else {
for (i = 0; pciidlist[i].vendor != 0; i++) {
pid = [i];
@@ -394,7 +395,7 @@ static void drm_cleanup(struct drm_device * dev)
drm_mm_takedown(>offset_manager);
drm_ht_remove(>object_hash);
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (dev->driver->driver_features & DRIVER_BIND_PCI))
pci_disable_device(dev->pdev);
drm_ctxbitmap_cleanup(dev);
@@ -427,7 +428,7 @@ void drm_exit(struct drm_driver *driver)
struct drm_head *head;
DRM_DEBUG("\n");
- if (drm_fb_loaded) {
+ if (drm_fb_loaded && !(driver->driver_features & DRIVER_BIND_PCI)) {
for (i = 0; i < drm_cards_limit; i++) {
head = drm_heads[i];
if (!head)
diff --git a/linux-core/drm_stub.c b/linux-core/drm_stub.c
index 07ea91e..e504865 100644
--- a/linux-core/drm_stub.c
+++ b/linux-core/drm_stub.c
@@ -230,7 +230,7 @@ int drm_get_dev(struct pci_dev *pdev, const struct
pci_device_id *ent,
if (!dev)
return -ENOMEM;
- if (!drm_fb_loaded) {
+ if (!drm_fb_loaded || (driver->driver_features & DRIVER_BIND_PCI)) {
pci_set_drvdata(pdev, dev);
ret = pci_request_regions(pdev, driver->pci_driver.name);
if (ret)
@@ -256,13 +256,13 @@ int drm_get_dev(struct pci_dev *pdev, const struct
pci_device_id *ent,
return 0;
err_g3:
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver->driver_features & DRIVER_BIND_PCI))
pci_disable_device(pdev);
err_g2:
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver->driver_features & DRIVER_BIND_PCI))
pci_release_regions(pdev);
err_g1:
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver->driver_features & DRIVER_BIND_PCI))
pci_set_drvdata(pdev, NULL);
drm_free(dev, sizeof(*dev), DRM_MEM_STUB);
diff --git a/linux-core/i915_drv.c b/linux-core/i915_drv.c
index e337e1d..5b8b74f 100644
--- a/linux-core/i915_drv.c
+++ b/linux-core/i915_drv.c
@@ -69,6 +69,460 @@ static struct drm_bo_driver i915_bo_driver = {
};
#endif
+enum pipe {
+PIPE_A = 0,
+PIPE_B,
+};
+
+static bool i915_pipe_enabled(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ if (pipe == PIPE_A)
+ return (I915_READ(DPLL_A) & DPLL_VCO_ENABLE);
+ else
+ return (I915_READ(DPLL_B) & DPLL_VCO_ENABLE);
+}
+
+static void i915_save_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+
Re: [RFC] full suspend/resume support for i915 DRM driver
On Friday, October 19, 2007, Jesse Barnes wrote:
Dave can you take a look at the new flag and also see what you think
about supporting suspend/resume in the event X hasn't started yet?
There's some #if 0'd code to support that case, but I haven't tested
it.
Ok Dave, this one's been updated with support for suspend/resume with
or without X running (iow I removed the requirement that X initialize
the driver, which DRM drivers usually have).
Hope it looks alright, it could definitely use testing on more
machines though...
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
index 2d02c76..5e73fc7 100644
--- a/linux-core/Kconfig
+++ b/linux-core/Kconfig
@@ -50,7 +50,7 @@ config DRM_I810
choice
prompt Intel 830M, 845G, 852GM, 855GM, 865G
- depends on DRM AGP AGP_INTEL
+ depends on DRM AGP AGP_INTEL !FB_INTEL
optional
config DRM_I915
@@ -60,6 +60,9 @@ config DRM_I915
852GM, 855GM, 865G, 915G, 915GM, 945G, 945GM and 965G integrated
graphics. If M is selected, the module will be called i915.
AGP support is required for this driver to work.
+
+ Note that this driver is incompatible with the Intel framebuffer
+ driver.
endchoice
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index f8ca3f4..36ce266 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -106,6 +106,7 @@ struct drm_file;
#define DRIVER_DMA_QUEUE 0x200
#define DRIVER_FB_DMA 0x400
#define DRIVER_IRQ_VBL20x800
+#define DRIVER_BIND_PCI0x1000
/[EMAIL PROTECTED]/
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index a09fa96..3d14fb4 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -340,9 +340,10 @@ int drm_init(struct drm_driver *driver,
}
}
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI)) {
+ printk(KERN_INFO DRM binding to PCI device\n);
return pci_register_driver(driver-pci_driver);
- else {
+ } else {
for (i = 0; pciidlist[i].vendor != 0; i++) {
pid = pciidlist[i];
@@ -394,7 +395,7 @@ static void drm_cleanup(struct drm_device * dev)
drm_mm_takedown(dev-offset_manager);
drm_ht_remove(dev-object_hash);
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (dev-driver-driver_features DRIVER_BIND_PCI))
pci_disable_device(dev-pdev);
drm_ctxbitmap_cleanup(dev);
@@ -427,7 +428,7 @@ void drm_exit(struct drm_driver *driver)
struct drm_head *head;
DRM_DEBUG(\n);
- if (drm_fb_loaded) {
+ if (drm_fb_loaded !(driver-driver_features DRIVER_BIND_PCI)) {
for (i = 0; i drm_cards_limit; i++) {
head = drm_heads[i];
if (!head)
diff --git a/linux-core/drm_stub.c b/linux-core/drm_stub.c
index 07ea91e..e504865 100644
--- a/linux-core/drm_stub.c
+++ b/linux-core/drm_stub.c
@@ -230,7 +230,7 @@ int drm_get_dev(struct pci_dev *pdev, const struct
pci_device_id *ent,
if (!dev)
return -ENOMEM;
- if (!drm_fb_loaded) {
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI)) {
pci_set_drvdata(pdev, dev);
ret = pci_request_regions(pdev, driver-pci_driver.name);
if (ret)
@@ -256,13 +256,13 @@ int drm_get_dev(struct pci_dev *pdev, const struct
pci_device_id *ent,
return 0;
err_g3:
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI))
pci_disable_device(pdev);
err_g2:
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI))
pci_release_regions(pdev);
err_g1:
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI))
pci_set_drvdata(pdev, NULL);
drm_free(dev, sizeof(*dev), DRM_MEM_STUB);
diff --git a/linux-core/i915_drv.c b/linux-core/i915_drv.c
index e337e1d..5b8b74f 100644
--- a/linux-core/i915_drv.c
+++ b/linux-core/i915_drv.c
@@ -69,6 +69,460 @@ static struct drm_bo_driver i915_bo_driver = {
};
#endif
+enum pipe {
+PIPE_A = 0,
+PIPE_B,
+};
+
+static bool i915_pipe_enabled(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (pipe == PIPE_A)
+ return (I915_READ(DPLL_A) DPLL_VCO_ENABLE);
+ else
+ return (I915_READ(DPLL_B) DPLL_VCO_ENABLE);
+}
+
+static void i915_save_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array =
Re: [RFC] full suspend/resume support for i915 DRM driver
On Thursday, October 18, 2007 2:01 pm Jesse Barnes wrote:
> We seem to see a lot of bug reports along the lines of, "my machine
> resumes but I can't see X" or, "I can see X but only with a bright
> flashlight", etc. These sorts of problems are due to the fact that
> the X server isn't designed to do full state save/restore, and none
> of the available kernel drivers do it on its behalf.
>
> Since intelfb and the rest of the Intel drivers are fairly
> incompatible, this patch makes the DRM bind to the PCI device so it
> can register real suspend/resume handlers. Those handlers take care
> of saving and restoring enough state for X to come back reliably on
> at least one of my problematic test machines, but text mode still has
> trouble (still debugging VGA state save/restore, including trying to
> save/restore actual VRAM contents for possible hibernate support).
>
> How does this approach look? Is a new DRM driver flag a good thing
> for similar situations with other drivers? Thoughts?
Here's an updated one that actually works fully on my 965 based test
platform. It should also work on other 9xx platforms, but note that it
lacks TV out and SDVO state save/restore, so I don't expect those to
work. I also haven't tested on 8xx platforms, but a similar approach
should work, so I'd welcome additions to support those chipsets.
Dave can you take a look at the new flag and also see what you think
about supporting suspend/resume in the event X hasn't started yet?
There's some #if 0'd code to support that case, but I haven't tested
it.
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
index 2d02c76..5e73fc7 100644
--- a/linux-core/Kconfig
+++ b/linux-core/Kconfig
@@ -50,7 +50,7 @@ config DRM_I810
choice
prompt "Intel 830M, 845G, 852GM, 855GM, 865G"
- depends on DRM && AGP && AGP_INTEL
+ depends on DRM && AGP && AGP_INTEL && !FB_INTEL
optional
config DRM_I915
@@ -60,6 +60,9 @@ config DRM_I915
852GM, 855GM, 865G, 915G, 915GM, 945G, 945GM and 965G integrated
graphics. If M is selected, the module will be called i915.
AGP support is required for this driver to work.
+
+ Note that this driver is incompatible with the Intel framebuffer
+ driver.
endchoice
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index f8ca3f4..36ce266 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -106,6 +106,7 @@ struct drm_file;
#define DRIVER_DMA_QUEUE 0x200
#define DRIVER_FB_DMA 0x400
#define DRIVER_IRQ_VBL20x800
+#define DRIVER_BIND_PCI0x1000
/[EMAIL PROTECTED]/
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index a09fa96..4d11707 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -340,9 +340,10 @@ int drm_init(struct drm_driver *driver,
}
}
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver->driver_features & DRIVER_BIND_PCI)) {
+ printk(KERN_ERR "DRM binding to PCI device\n");
return pci_register_driver(>pci_driver);
- else {
+ } else {
for (i = 0; pciidlist[i].vendor != 0; i++) {
pid = [i];
diff --git a/linux-core/i915_drv.c b/linux-core/i915_drv.c
index e337e1d..809ef0d 100644
--- a/linux-core/i915_drv.c
+++ b/linux-core/i915_drv.c
@@ -69,6 +69,501 @@ static struct drm_bo_driver i915_bo_driver = {
};
#endif
+enum pipe {
+PIPE_A = 0,
+PIPE_B,
+};
+
+static bool i915_pipe_enabled(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ if (pipe == PIPE_A)
+ return (I915_READ(DPLL_A) & DPLL_VCO_ENABLE);
+ else
+ return (I915_READ(DPLL_B) & DPLL_VCO_ENABLE);
+}
+
+static void i915_save_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv->savePaletteA;
+ else
+ array = dev_priv->savePaletteB;
+
+ for(i = 0; i < 256; i++)
+ array[i] = I915_READ(reg + (i << 2));
+}
+
+static void i915_restore_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv->savePaletteA;
+ else
+ array = dev_priv->savePaletteB;
+
+ for(i = 0; i < 256; i++)
+ I915_WRITE(reg + (i << 2), array[i]);
+}
+
+static u8 i915_read_indexed(u16 index_port, u16 data_port, u8 reg)
+{
+ outb(reg, index_port);
+
Re: [RFC] full suspend/resume support for i915 DRM driver
On Thursday, October 18, 2007 2:01 pm Jesse Barnes wrote:
We seem to see a lot of bug reports along the lines of, my machine
resumes but I can't see X or, I can see X but only with a bright
flashlight, etc. These sorts of problems are due to the fact that
the X server isn't designed to do full state save/restore, and none
of the available kernel drivers do it on its behalf.
Since intelfb and the rest of the Intel drivers are fairly
incompatible, this patch makes the DRM bind to the PCI device so it
can register real suspend/resume handlers. Those handlers take care
of saving and restoring enough state for X to come back reliably on
at least one of my problematic test machines, but text mode still has
trouble (still debugging VGA state save/restore, including trying to
save/restore actual VRAM contents for possible hibernate support).
How does this approach look? Is a new DRM driver flag a good thing
for similar situations with other drivers? Thoughts?
Here's an updated one that actually works fully on my 965 based test
platform. It should also work on other 9xx platforms, but note that it
lacks TV out and SDVO state save/restore, so I don't expect those to
work. I also haven't tested on 8xx platforms, but a similar approach
should work, so I'd welcome additions to support those chipsets.
Dave can you take a look at the new flag and also see what you think
about supporting suspend/resume in the event X hasn't started yet?
There's some #if 0'd code to support that case, but I haven't tested
it.
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
index 2d02c76..5e73fc7 100644
--- a/linux-core/Kconfig
+++ b/linux-core/Kconfig
@@ -50,7 +50,7 @@ config DRM_I810
choice
prompt Intel 830M, 845G, 852GM, 855GM, 865G
- depends on DRM AGP AGP_INTEL
+ depends on DRM AGP AGP_INTEL !FB_INTEL
optional
config DRM_I915
@@ -60,6 +60,9 @@ config DRM_I915
852GM, 855GM, 865G, 915G, 915GM, 945G, 945GM and 965G integrated
graphics. If M is selected, the module will be called i915.
AGP support is required for this driver to work.
+
+ Note that this driver is incompatible with the Intel framebuffer
+ driver.
endchoice
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index f8ca3f4..36ce266 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -106,6 +106,7 @@ struct drm_file;
#define DRIVER_DMA_QUEUE 0x200
#define DRIVER_FB_DMA 0x400
#define DRIVER_IRQ_VBL20x800
+#define DRIVER_BIND_PCI0x1000
/[EMAIL PROTECTED]/
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index a09fa96..4d11707 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -340,9 +340,10 @@ int drm_init(struct drm_driver *driver,
}
}
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI)) {
+ printk(KERN_ERR DRM binding to PCI device\n);
return pci_register_driver(driver-pci_driver);
- else {
+ } else {
for (i = 0; pciidlist[i].vendor != 0; i++) {
pid = pciidlist[i];
diff --git a/linux-core/i915_drv.c b/linux-core/i915_drv.c
index e337e1d..809ef0d 100644
--- a/linux-core/i915_drv.c
+++ b/linux-core/i915_drv.c
@@ -69,6 +69,501 @@ static struct drm_bo_driver i915_bo_driver = {
};
#endif
+enum pipe {
+PIPE_A = 0,
+PIPE_B,
+};
+
+static bool i915_pipe_enabled(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (pipe == PIPE_A)
+ return (I915_READ(DPLL_A) DPLL_VCO_ENABLE);
+ else
+ return (I915_READ(DPLL_B) DPLL_VCO_ENABLE);
+}
+
+static void i915_save_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv-savePaletteA;
+ else
+ array = dev_priv-savePaletteB;
+
+ for(i = 0; i 256; i++)
+ array[i] = I915_READ(reg + (i 2));
+}
+
+static void i915_restore_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv-savePaletteA;
+ else
+ array = dev_priv-savePaletteB;
+
+ for(i = 0; i 256; i++)
+ I915_WRITE(reg + (i 2), array[i]);
+}
+
+static u8 i915_read_indexed(u16 index_port, u16 data_port, u8 reg)
+{
+ outb(reg, index_port);
+ return inb(data_port);
+}
+
+static
[RFC] full suspend/resume support for i915 DRM driver
We seem to see a lot of bug reports along the lines of, "my machine
resumes but I can't see X" or, "I can see X but only with a bright
flashlight", etc. These sorts of problems are due to the fact that
the X server isn't designed to do full state save/restore, and none
of the available kernel drivers do it on its behalf.
Since intelfb and the rest of the Intel drivers are fairly incompatible,
this patch makes the DRM bind to the PCI device so it can register real
suspend/resume handlers. Those handlers take care of saving and
restoring enough state for X to come back reliably on at least one of my
problematic test machines, but text mode still has trouble (still
debugging VGA state save/restore, including trying to save/restore
actual VRAM contents for possible hibernate support).
How does this approach look? Is a new DRM driver flag a good thing for
similar situations with other drivers? Thoughts?
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
index 2d02c76..5e73fc7 100644
--- a/linux-core/Kconfig
+++ b/linux-core/Kconfig
@@ -50,7 +50,7 @@ config DRM_I810
choice
prompt "Intel 830M, 845G, 852GM, 855GM, 865G"
- depends on DRM && AGP && AGP_INTEL
+ depends on DRM && AGP && AGP_INTEL && !FB_INTEL
optional
config DRM_I915
@@ -60,6 +60,9 @@ config DRM_I915
852GM, 855GM, 865G, 915G, 915GM, 945G, 945GM and 965G integrated
graphics. If M is selected, the module will be called i915.
AGP support is required for this driver to work.
+
+ Note that this driver is incompatible with the Intel framebuffer
+ driver.
endchoice
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index f8ca3f4..36ce266 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -106,6 +106,7 @@ struct drm_file;
#define DRIVER_DMA_QUEUE 0x200
#define DRIVER_FB_DMA 0x400
#define DRIVER_IRQ_VBL20x800
+#define DRIVER_BIND_PCI0x1000
/[EMAIL PROTECTED]/
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index a09fa96..4d11707 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -340,9 +340,10 @@ int drm_init(struct drm_driver *driver,
}
}
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver->driver_features & DRIVER_BIND_PCI)) {
+ printk(KERN_ERR "DRM binding to PCI device\n");
return pci_register_driver(>pci_driver);
- else {
+ } else {
for (i = 0; pciidlist[i].vendor != 0; i++) {
pid = [i];
diff --git a/linux-core/i915_drv.c b/linux-core/i915_drv.c
index e337e1d..1f99944 100644
--- a/linux-core/i915_drv.c
+++ b/linux-core/i915_drv.c
@@ -69,6 +69,621 @@ static struct drm_bo_driver i915_bo_driver = {
};
#endif
+enum pipe {
+PIPE_A = 0,
+PIPE_B,
+};
+
+static bool i915_pipe_enabled(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ if (pipe == PIPE_A)
+ return (I915_READ(DPLL_A) & DPLL_VCO_ENABLE);
+ else
+ return (I915_READ(DPLL_B) & DPLL_VCO_ENABLE);
+}
+
+static void i915_save_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv->savePaletteA;
+ else
+ array = dev_priv->savePaletteB;
+
+ for(i = 0; i < 256; i++)
+ array[i] = I915_READ(reg + (i << 2));
+}
+
+static void i915_restore_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv->savePaletteA;
+ else
+ array = dev_priv->savePaletteB;
+
+ for(i = 0; i < 256; i++)
+ I915_WRITE(reg + (i << 2), array[i]);
+}
+
+static u8 i915_read_indexed(u16 index_port, u16 data_port, u8 reg)
+{
+ outb(reg, index_port);
+ return inb(data_port);
+}
+
+static void i915_write_indexed(u16 index_port, u16 data_port, u8 reg, u8 val)
+{
+ outb(reg, index_port);
+ outb(val, data_port);
+}
+
+static void i915_save_vga(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned long mem_mode, mem_offset = 0, mem_size = 0;
+ u8 *mapped_offset;
+ int i;
+ u16 cr_index, cr_data, st01;
+ u8 msr;
+
+ /* MSR bits */
+ msr = dev_priv->saveMSR = inb(VGA_MSR_READ);
+ if (dev_priv->saveMSR & VGA_MSR_CGA_MODE) {
+ cr_index = VGA_CR_INDEX_CGA;
+
[RFC] full suspend/resume support for i915 DRM driver
We seem to see a lot of bug reports along the lines of, my machine
resumes but I can't see X or, I can see X but only with a bright
flashlight, etc. These sorts of problems are due to the fact that
the X server isn't designed to do full state save/restore, and none
of the available kernel drivers do it on its behalf.
Since intelfb and the rest of the Intel drivers are fairly incompatible,
this patch makes the DRM bind to the PCI device so it can register real
suspend/resume handlers. Those handlers take care of saving and
restoring enough state for X to come back reliably on at least one of my
problematic test machines, but text mode still has trouble (still
debugging VGA state save/restore, including trying to save/restore
actual VRAM contents for possible hibernate support).
How does this approach look? Is a new DRM driver flag a good thing for
similar situations with other drivers? Thoughts?
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
index 2d02c76..5e73fc7 100644
--- a/linux-core/Kconfig
+++ b/linux-core/Kconfig
@@ -50,7 +50,7 @@ config DRM_I810
choice
prompt Intel 830M, 845G, 852GM, 855GM, 865G
- depends on DRM AGP AGP_INTEL
+ depends on DRM AGP AGP_INTEL !FB_INTEL
optional
config DRM_I915
@@ -60,6 +60,9 @@ config DRM_I915
852GM, 855GM, 865G, 915G, 915GM, 945G, 945GM and 965G integrated
graphics. If M is selected, the module will be called i915.
AGP support is required for this driver to work.
+
+ Note that this driver is incompatible with the Intel framebuffer
+ driver.
endchoice
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index f8ca3f4..36ce266 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -106,6 +106,7 @@ struct drm_file;
#define DRIVER_DMA_QUEUE 0x200
#define DRIVER_FB_DMA 0x400
#define DRIVER_IRQ_VBL20x800
+#define DRIVER_BIND_PCI0x1000
/[EMAIL PROTECTED]/
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index a09fa96..4d11707 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -340,9 +340,10 @@ int drm_init(struct drm_driver *driver,
}
}
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI)) {
+ printk(KERN_ERR DRM binding to PCI device\n);
return pci_register_driver(driver-pci_driver);
- else {
+ } else {
for (i = 0; pciidlist[i].vendor != 0; i++) {
pid = pciidlist[i];
diff --git a/linux-core/i915_drv.c b/linux-core/i915_drv.c
index e337e1d..1f99944 100644
--- a/linux-core/i915_drv.c
+++ b/linux-core/i915_drv.c
@@ -69,6 +69,621 @@ static struct drm_bo_driver i915_bo_driver = {
};
#endif
+enum pipe {
+PIPE_A = 0,
+PIPE_B,
+};
+
+static bool i915_pipe_enabled(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (pipe == PIPE_A)
+ return (I915_READ(DPLL_A) DPLL_VCO_ENABLE);
+ else
+ return (I915_READ(DPLL_B) DPLL_VCO_ENABLE);
+}
+
+static void i915_save_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv-savePaletteA;
+ else
+ array = dev_priv-savePaletteB;
+
+ for(i = 0; i 256; i++)
+ array[i] = I915_READ(reg + (i 2));
+}
+
+static void i915_restore_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv-savePaletteA;
+ else
+ array = dev_priv-savePaletteB;
+
+ for(i = 0; i 256; i++)
+ I915_WRITE(reg + (i 2), array[i]);
+}
+
+static u8 i915_read_indexed(u16 index_port, u16 data_port, u8 reg)
+{
+ outb(reg, index_port);
+ return inb(data_port);
+}
+
+static void i915_write_indexed(u16 index_port, u16 data_port, u8 reg, u8 val)
+{
+ outb(reg, index_port);
+ outb(val, data_port);
+}
+
+static void i915_save_vga(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long mem_mode, mem_offset = 0, mem_size = 0;
+ u8 *mapped_offset;
+ int i;
+ u16 cr_index, cr_data, st01;
+ u8 msr;
+
+ /* MSR bits */
+ msr = dev_priv-saveMSR = inb(VGA_MSR_READ);
+ if (dev_priv-saveMSR VGA_MSR_CGA_MODE) {
+ cr_index = VGA_CR_INDEX_CGA;
+ cr_data = VGA_CR_DATA_CGA;
+
