Re: [PATCH] drm: Allow platform devices to register as DRM devices
On Mon, Mar 15, 2010 at 10:56:02AM +1000, Dave Airlie wrote: On Tue, Mar 2, 2010 at 2:00 AM, Jordan Crouse jcro...@codeaurora.org wrote: Allow platform devices without PCI resources to be DRM devices. Signed-off-by: Jordan Crouse jcro...@codeaurora.org This patch has a bunch of whitespace damage at least in my inbox and also in patchwork Please also be careful with the places you add copyrights, you moved a bunch of code from drm_drv.c to drm_pci.c and added a copyright for Code Aurora in drm_pci.c? You can only assert copyright if you actually wrote the code, otherwise the patch contains your authorship details and who contributed the code. I'm guessing you might have copyright over one file in this that being drm_platform.c, the rest I'm less sure about. I also wonder if we could/should separate the arm drm support out from this patch (i.e. the __arm__ changes). Also note that pgprot_noncached() is a standard interface these days, so there is no longer any reason to special case it. There's no reason for the io prot mapping code turning in to the hell that was the fbmem.c mmap. -- Download Intel#174; Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [PATCH] drm: Allow platform devices to register as DRM devices
Dave Airlie wrote:
On Tue, Mar 2, 2010 at 2:00 AM, Jordan Crouse jcro...@codeaurora.org wrote:
Allow platform devices without PCI resources to be DRM devices.
Signed-off-by: Jordan Crouse jcro...@codeaurora.org
This patch has a bunch of whitespace damage at least in my inbox and
also in patchwork
Yes, PBCAK.
Please also be careful with the places you add copyrights, you moved a
bunch of code
from drm_drv.c to drm_pci.c and added a copyright for Code Aurora in
drm_pci.c? You can
only assert copyright if you actually wrote the code, otherwise the patch
contains your authorship details and who contributed the code. I'm
guessing you might have
copyright over one file in this that being drm_platform.c, the rest
I'm less sure about.
I also wonder if we could/should separate the arm drm support out from
this patch (i.e.
the __arm__ changes).
That might not be a bad idea, I'll do that.
Some other misc comments below:
#
menuconfig DRM
tristate Direct Rendering Manager (XFree86 4.1.0 and higher DRI
support)
- depends on (AGP || AGP=n) PCI !EMULATED_CMPXCHG MMU
From what I can see the AGP || AGP==n is still required, you just want
to drop the PCI
dependancy??
I guess technically we could also drop the AGP requirement, but since it worked
on my box with AGP=n it seemed to me like a NOP.
+ depends on !EMULATED_CMPXCHG MMU
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index 8417cc4..4177f60 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -11,6 +11,7 @@
*
* Copyright 1999, 2000 Precision Insight, Inc., Cedar Park, Texas.
* Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
+ * Copyright (c) 2009, Code Aurora Forum.
* All Rights Reserved.
There have been much bigger changes to these files without copyright
additions, I'm not sure how big something needs to be but I'm not 100% sure
this comes close.
resource_size_t drm_get_resource_start(struct drm_device *dev, unsigned int
resource)
{
+ if (drm_core_check_feature(dev, DRIVER_USE_PLATFORM_DEVICE)) {
+ struct resource *r;
+ r = platform_get_resource(dev-platformdev, IORESOURCE_MEM,
+resource);
+
+ return r ? r-start : 0;
+ }
+
+#ifdef CONFIG_PCI
return pci_resource_start(dev-pdev, resource);
+#endif
+
+ return 0;
}
EXPORT_SYMBOL(drm_get_resource_start);
resource_size_t drm_get_resource_len(struct drm_device *dev, unsigned int
resource)
{
+ if (drm_core_check_feature(dev, DRIVER_USE_PLATFORM_DEVICE)) {
+ struct resource *r;
+ r = platform_get_resource(dev-platformdev, IORESOURCE_MEM,
+ resource);
+
+ return r ? (r-end - r-start) : 0;
+ }
+
+#ifdef CONFIG_PCI
return pci_resource_len(dev-pdev, resource);
+#endif
+
+ return 0;
}
EXPORT_SYMBOL(drm_get_resource_len);
@@ -188,7 +213,7 @@ static int drm_addmap_core(struct drm_device * dev,
resource_size_t offset,
switch (map-type) {
case _DRM_REGISTERS:
case _DRM_FRAME_BUFFER:
-#if !defined(__sparc__) !defined(__alpha__) !defined(__ia64__)
!defined(__powerpc64__) !defined(__x86_64__)
+#if !defined(__sparc__) !defined(__alpha__) !defined(__ia64__)
!defined(__powerpc64__) !defined(__x86_64__) !defined(__arm__)
if (map-offset + (map-size-1) map-offset ||
map-offset virt_to_phys(high_memory)) {
kfree(map);
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 766c468..b5171ed 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -24,6 +24,7 @@
*
* Copyright 1999, 2000 Precision Insight, Inc., Cedar Park, Texas.
* Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
+ * Copyright (c) 2009, Code Aurora Forum.
* All Rights Reserved.
again similiar, but even less code since you mostly remove code.
*
* Permission is hereby granted, free of charge, to any person obtaining a
@@ -242,47 +243,20 @@ int drm_lastclose(struct drm_device * dev)
*
* Initializes an array of drm_device structures, and attempts to
* initialize all available devices, using consecutive minors, registering
the
- * stubs and initializing the AGP device.
+ * stubs and initializing the device.
*
* Expands the \c DRIVER_PREINIT and \c DRIVER_POST_INIT macros before and
* after the initialization for driver customization.
*/
int drm_init(struct drm_driver *driver)
{
- struct pci_dev *pdev = NULL;
- const struct pci_device_id *pid;
- int i;
-
DRM_DEBUG(\n);
-
INIT_LIST_HEAD(driver-device_list);
- if (driver-driver_features DRIVER_MODESET)
- return pci_register_driver(driver-pci_driver);
-
- /* If not using KMS, fall back to
Re: [PATCH] drm: Allow platform devices to register as DRM devices
On Mon, Mar 01, 2010 at 09:00:09AM -0700, Jordan Crouse wrote: Allow platform devices without PCI resources to be DRM devices. I really dislike the fact that drm has bus specific junk all over the generic code. Some ideas how to clean that up: Add 'struct device *dev' into drm_device so you don't have to go through the pdev/platformdev to get it every time. Also use dev_name() instead of pci_name() and whatever you used for the platform device case. Add 'int irq' into struct drm_device instead of adding more bus specific hoops to get at the irq. Not sure I like the generic code mucking about with the irq directly though but baby steps are easier to handle. Get rid of the drm_resource_start/len wrappers. AFAICS they're all called from the low level driver code anyway and the driver knows the bus type so there's no need for the wrappers. It would be nice to get the struct pci_driver out of the the drm_driver structure. Since you now have a new pci specific drm_get_dev() thing could you also pass the pci_driver as a function parameter instead of having it live inside the drm_driver? Also all cases where there's some PCI specific stuff (the busid stuff mostly) you could just check the drm_device.pdev pointer instead of having to add another driver flags to identify non-PCI devices. Although I don't really like having the pdev/platformdev pointers in there at all. That's sort of my secret drm TODO list but so far didn't have the time to actually do the coding part. -- Ville Syrjälä syrj...@sci.fi http://www.sci.fi/~syrjala/ -- Download Intel#174; Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [PATCH] drm: Allow platform devices to register as DRM devices
I guess technically we could also drop the AGP requirement, but since it worked on my box with AGP=n it seemed to me like a NOP. Its not a NOP, otherwise we'd remove it, AGP || AGP=n means if AGP is enabled DRM must be enabled similiarly, it stops AGP=m + DRM=y basically. EXPORT_SYMBOL(drm_init); diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index ab6c973..48a14a0 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1220,6 +1220,9 @@ struct edid *drm_get_edid(struct drm_connector *connector, int ret; struct edid *edid; + if (drm_core_check_feature(connector-dev, DRIVER_USE_PLATFORM_DEVICE)) + return NULL; + This makes no sense, having the ability to probe EDID or not is most definitely not a platform vs PCI problem. Yeah, that was my poor man's Don't probe EDID hack. I'm not sure if there is a smart way to implicitly check to see if EDID should be probed, but I'm worried about abusing the features flag too badly, though if a DRIVER_USE_EDID is needed then we shouldn't be shy about using it. The generic code never calls this only the driver, so there should be no need for flags at all. Not 100% sure about this, but if you only intend on KMS and don't need to inform userspace of irq support this should be okay. Again, a don't do this hack. I'll look at this more carefully and see if there is a good way to evaluate this based on the hooks that the platform has defined. Its mostly used in UMS to inform userspace for some strange reason its pretty legacy at this point, new driver should probably not hit it. @@ -60,7 +60,7 @@ static pgprot_t drm_io_prot(uint32_t map_type, struct This also doesn't address noueau/vmwgfx entry points. Yep, thats my bad. I'll refresh and push better patches without whitespace stupidity. Thanks, Dave. -- Download Intel#174; Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [PATCH] drm: Allow platform devices to register as DRM devices
On Tue, Mar 2, 2010 at 2:00 AM, Jordan Crouse jcro...@codeaurora.org wrote:
Allow platform devices without PCI resources to be DRM devices.
Signed-off-by: Jordan Crouse jcro...@codeaurora.org
This patch has a bunch of whitespace damage at least in my inbox and
also in patchwork
Please also be careful with the places you add copyrights, you moved a
bunch of code
from drm_drv.c to drm_pci.c and added a copyright for Code Aurora in
drm_pci.c? You can
only assert copyright if you actually wrote the code, otherwise the patch
contains your authorship details and who contributed the code. I'm
guessing you might have
copyright over one file in this that being drm_platform.c, the rest
I'm less sure about.
I also wonder if we could/should separate the arm drm support out from
this patch (i.e.
the __arm__ changes).
Some other misc comments below:
#
menuconfig DRM
tristate Direct Rendering Manager (XFree86 4.1.0 and higher DRI
support)
- depends on (AGP || AGP=n) PCI !EMULATED_CMPXCHG MMU
From what I can see the AGP || AGP==n is still required, you just want
to drop the PCI
dependancy??
+ depends on !EMULATED_CMPXCHG MMU
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index 8417cc4..4177f60 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -11,6 +11,7 @@
*
* Copyright 1999, 2000 Precision Insight, Inc., Cedar Park, Texas.
* Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
+ * Copyright (c) 2009, Code Aurora Forum.
* All Rights Reserved.
There have been much bigger changes to these files without copyright
additions, I'm not sure how big something needs to be but I'm not 100% sure
this comes close.
resource_size_t drm_get_resource_start(struct drm_device *dev, unsigned int
resource)
{
+ if (drm_core_check_feature(dev, DRIVER_USE_PLATFORM_DEVICE)) {
+ struct resource *r;
+ r = platform_get_resource(dev-platformdev, IORESOURCE_MEM,
+ resource);
+
+ return r ? r-start : 0;
+ }
+
+#ifdef CONFIG_PCI
return pci_resource_start(dev-pdev, resource);
+#endif
+
+ return 0;
}
EXPORT_SYMBOL(drm_get_resource_start);
resource_size_t drm_get_resource_len(struct drm_device *dev, unsigned int
resource)
{
+ if (drm_core_check_feature(dev, DRIVER_USE_PLATFORM_DEVICE)) {
+ struct resource *r;
+ r = platform_get_resource(dev-platformdev, IORESOURCE_MEM,
+ resource);
+
+ return r ? (r-end - r-start) : 0;
+ }
+
+#ifdef CONFIG_PCI
return pci_resource_len(dev-pdev, resource);
+#endif
+
+ return 0;
}
EXPORT_SYMBOL(drm_get_resource_len);
@@ -188,7 +213,7 @@ static int drm_addmap_core(struct drm_device * dev,
resource_size_t offset,
switch (map-type) {
case _DRM_REGISTERS:
case _DRM_FRAME_BUFFER:
-#if !defined(__sparc__) !defined(__alpha__) !defined(__ia64__)
!defined(__powerpc64__) !defined(__x86_64__)
+#if !defined(__sparc__) !defined(__alpha__) !defined(__ia64__)
!defined(__powerpc64__) !defined(__x86_64__) !defined(__arm__)
if (map-offset + (map-size-1) map-offset ||
map-offset virt_to_phys(high_memory)) {
kfree(map);
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 766c468..b5171ed 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -24,6 +24,7 @@
*
* Copyright 1999, 2000 Precision Insight, Inc., Cedar Park, Texas.
* Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
+ * Copyright (c) 2009, Code Aurora Forum.
* All Rights Reserved.
again similiar, but even less code since you mostly remove code.
*
* Permission is hereby granted, free of charge, to any person obtaining a
@@ -242,47 +243,20 @@ int drm_lastclose(struct drm_device * dev)
*
* Initializes an array of drm_device structures, and attempts to
* initialize all available devices, using consecutive minors, registering
the
- * stubs and initializing the AGP device.
+ * stubs and initializing the device.
*
* Expands the \c DRIVER_PREINIT and \c DRIVER_POST_INIT macros before and
* after the initialization for driver customization.
*/
int drm_init(struct drm_driver *driver)
{
- struct pci_dev *pdev = NULL;
- const struct pci_device_id *pid;
- int i;
-
DRM_DEBUG(\n);
-
INIT_LIST_HEAD(driver-device_list);
- if (driver-driver_features DRIVER_MODESET)
- return pci_register_driver(driver-pci_driver);
-
- /* If not using KMS, fall back to stealth mode manual scanning. */
- for (i = 0; driver-pci_driver.id_table[i].vendor != 0; i++) {
- pid = driver-pci_driver.id_table[i];
-
- /* Loop around setting up a DRM device for each PCI device
