Bug 3549 - drm power management does not dereference sysdev correctly
Summary: drm power management does not dereference sysdev correctly
Status: RESOLVED FIXED
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/other (show other bugs)
Version: DRI git
Hardware: Other Linux (All)
: high major
Assignee: Default DRI bug account
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-06-16 06:21 UTC by Lincoln Stein
Modified: 2005-06-26 10:06 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Lincoln Stein 2005-06-16 06:21:20 UTC
In linux-core drm_pm.c, CVS version 1.1, the drm_pm_setup function registers 
the drm device's sysdev field with the power manager, but when the drm_suspend 
or drm_resume functions are called, the functions think that they have received 
a drm_device ptr, and not a power management sys_device ptr, and perform an 
incorrect typecast. They dereference the sys_device as if it were a drm_device, 
get an invalid value for the drm driver field, and go off into the weeds. 
 
Another issue is that Linux kernel 2.6.11 (?) has changed the argument that it 
passes to power management functions from an unsigned int to a pm_message_t 
struct. 
 
The attached patch fixes this behavior using a union struct, and also makes 
drm_pm.c and drmP.h compatible with the new style pm_message_t arguments. 
Unfortunately it doesn't preserve backward compatibility with older kernels.  
 
Index: drmP.h 
=================================================================== 
RCS file: /cvs/dri/drm/linux-core/drmP.h,v 
retrieving revision 1.147 
diff -c -r1.147 drmP.h 
*** drmP.h	4 Jun 2005 06:18:10 -0000	1.147 
--- drmP.h	16 Jun 2005 13:09:59 -0000 
*************** 
*** 515,520 **** 
--- 515,534 ---- 
  	struct task_struct *task; 
  } drm_vbl_sig_t; 
   
+  
+ /** 
+  * union struct that can either be a PM sys_device or a drm_sys_device 
+  */ 
+ typedef struct drm_sysdev { 
+   struct sys_device    sysdev; 
+   struct drm_device*   dev; 
+ } drm_sysdev_t; 
+  
+ union drm_sysdev_data { 
+   struct sys_device     sysdev; 
+   struct drm_sysdev     drmdev; 
+ }; 
+  
  /** 
   * DRM driver structure. This structure represent the common code for 
   * a family of cards. There will one drm_device for each card present 
*************** 
*** 723,729 **** 
  	drm_local_map_t *agp_buffer_map; 
  	drm_head_t primary;		/**< primary screen head */ 
   
! 	struct sys_device sysdev;	/**< Power Management device structure 
*/ 
  	int sysdev_registered;		/**< Whether the device has been 
registered */ 
  } drm_device_t; 
   
--- 737,743 ---- 
  	drm_local_map_t *agp_buffer_map; 
  	drm_head_t primary;		/**< primary screen head */ 
   
!         union drm_sysdev_data  sysdev;  /** union PM structure */ 
  	int sysdev_registered;		/**< Whether the device has been 
registered */ 
  } drm_device_t; 
   
Index: drm_pm.c 
=================================================================== 
RCS file: /cvs/dri/drm/linux-core/drm_pm.c,v 
retrieving revision 1.1 
diff -c -r1.1 drm_pm.c 
*** drm_pm.c	28 May 2005 00:00:08 -0000	1.1 
--- drm_pm.c	16 Jun 2005 13:09:59 -0000 
*************** 
*** 36,56 **** 
  #include <linux/device.h> 
  #include <linux/sysdev.h> 
   
! static int drm_suspend(struct sys_device *sysdev, u32 state) 
  { 
! 	drm_device_t *dev = (drm_device_t *)sysdev; 
! 	 
! 	DRM_DEBUG("%s state=%d\n", __FUNCTION__, state); 
  	 
          if (dev->driver->power) 
! 		return dev->driver->power(dev, state); 
  	else 
  		return 0; 
  } 
   
  static int drm_resume(struct sys_device *sysdev) 
  { 
! 	drm_device_t *dev = (drm_device_t *)sysdev; 
  	 
  	DRM_DEBUG("%s\n", __FUNCTION__); 
  	 
--- 36,60 ---- 
  #include <linux/device.h> 
  #include <linux/sysdev.h> 
   
! static int drm_suspend(struct sys_device *sysdev, pm_message_t state) 
  { 
!  
!         int event; 
!  
!         drm_device_t *dev = ((drm_sysdev_t *) sysdev)->dev; 
! 	event = state.event; 
  	 
+ 	DRM_DEBUG("%s state=%d\n", __FUNCTION__, event); 
+  
          if (dev->driver->power) 
! 		return dev->driver->power(dev, event); 
  	else 
  		return 0; 
  } 
   
  static int drm_resume(struct sys_device *sysdev) 
  { 
!   drm_device_t *dev = ((drm_sysdev_t *) sysdev)->dev; 
  	 
  	DRM_DEBUG("%s\n", __FUNCTION__); 
  	 
*************** 
*** 79,91 **** 
  	 
  	DRM_DEBUG("%s\n", __FUNCTION__); 
  	 
! 	dev->sysdev.id = dev->primary.minor; 
! 	dev->sysdev.cls = &drm_sysdev_class; 
   
  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,4) 
! 	error = sys_device_register(&dev->sysdev); 
  #else 
! 	error = sysdev_register(&dev->sysdev); 
  #endif 
  	if(!error) 
  		dev->sysdev_registered = 1; 
--- 83,96 ---- 
  	 
  	DRM_DEBUG("%s\n", __FUNCTION__); 
  	 
! 	dev->sysdev.sysdev.id  = dev->primary.minor; 
! 	dev->sysdev.sysdev.cls = &drm_sysdev_class; 
! 	dev->sysdev.drmdev.dev = dev; 
   
  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,4) 
! 	error = sys_device_register(&dev->sysdev.sysdev); 
  #else 
! 	error = sysdev_register(&dev->sysdev.sysdev); 
  #endif 
  	if(!error) 
  		dev->sysdev_registered = 1; 
*************** 
*** 103,111 **** 
  	 
  	if(dev->sysdev_registered) { 
  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,4) 
! 		sys_device_unregister(&dev->sysdev); 
  #else 
! 		sysdev_unregister(&dev->sysdev); 
  #endif 
  		dev->sysdev_registered = 0; 
  	} 
--- 108,116 ---- 
  	 
  	if(dev->sysdev_registered) { 
  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,4) 
! 		sys_device_unregister(&dev->sysdev.sysdev); 
  #else 
! 		sysdev_unregister(&dev->sysdev.sysdev); 
  #endif 
  		dev->sysdev_registered = 0; 
  	}
Comment 1 Lincoln Stein 2005-06-16 06:22:28 UTC
Created attachment 2896 [details]
Mockup with buttons based on Benji's.
Comment 2 Alan Hourihane 2005-06-24 02:31:49 UTC
Created attachment 2947 [details]
The offending code.

The kernel already has a call which does the work for us - container_of().

Using this patch fixes the problem.
Comment 3 Alan Hourihane 2005-06-27 03:06:44 UTC
Closing.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.