Exports the symbol for drm agp entry points. This allows the new drm
linux-core module to get the symbol with symbol_get() instead of
inter_module_get(). After the new drm code arrives inter_module_xx
code in AGP can be deleted.
--
Jon Smirl
[email protected]
On Sat, Oct 23, 2004 at 12:45:42AM -0400, Jon Smirl wrote:
> Exports the symbol for drm agp entry points. This allows the new drm
> linux-core module to get the symbol with symbol_get() instead of
> inter_module_get(). After the new drm code arrives inter_module_xx
> code in AGP can be deleted.
Sorry, wrong API. At least export the individual functions and use them
directly (and without the symbol_get abnomination that's not any better
than inter_module_*).
>
> Sorry, wrong API. At least export the individual functions and use them
> directly (and without the symbol_get abnomination that's not any better
> than inter_module_*).
I wonder what the reasoning behind the old drm_agp structure was in
the first place?
What about this patch?
The 2.6 kernel module stuff should sort out the dependencies itself, I
haven't had a chance to test this yet....
Dave.
On Sun, Oct 24, 2004 at 12:28:18AM +1000, Dave Airlie wrote:
> >
> > Sorry, wrong API. At least export the individual functions and use them
> > directly (and without the symbol_get abnomination that's not any better
> > than inter_module_*).
>
> I wonder what the reasoning behind the old drm_agp structure was in
> the first place?
>
> What about this patch?
The patch looks good to me, I really like the reduced complexity.
Some minor nitpicks below:
+ if (!(head = DRM(alloc)(sizeof(*head), DRM_MEM_AGPLISTS)))
+ return NULL;
+ memset((void *)head, 0, sizeof(*head));
no need to cast to void * here
@@ -428,41 +409,37 @@
*/
void DRM(agp_uninit)(void)
{
- DRM_AGP_PUT;
- drm_agp = NULL;
}
is the function needed by BSD? Else it should probably go away completely.
How do I deal with something like a Redhat kernel where CONFIG_AGP is
set but the kernel may be running on hardware without AGP present. In
this case the AGP modules will not be loaded but DRM will still have
symbol references to the AGP symbols. You have to have some kind of
weak symbol reference in this case.
--
Jon Smirl
[email protected]
On Sat, Oct 23, 2004 at 10:35:41AM -0400, Jon Smirl wrote:
> How do I deal with something like a Redhat kernel where CONFIG_AGP is
> set but the kernel may be running on hardware without AGP present. In
> this case the AGP modules will not be loaded but DRM will still have
> symbol references to the AGP symbols. You have to have some kind of
> weak symbol reference in this case.
agpgart.o will load without any hardware present in 2.6, and you don't
need the lowlevel drivers. 2.4 was slightly messed up in this regard.
On Sat, 2004-10-23 at 10:35 -0400, Jon Smirl wrote:
> How do I deal with something like a Red Hat kernel where CONFIG_AGP is
> set but the kernel may be running on hardware without AGP present.
Nowadays RH builds AGP into the kernel always ;)
--
>
> What about the group that will complain about the 30K of memory wasted?
>
>
they'll build their own kernel with CONFIG_AGP turned off... so it won't
matter...
Dave.
--
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
pam_smb / Linux DECstation / Linux VAX / ILUG person
On Sat, 23 Oct 2004 15:39:12 +0100, Christoph Hellwig <[email protected]> wrote:
> agpgart.o will load without any hardware present in 2.6, and you don't
> need the lowlevel drivers. 2.4 was slightly messed up in this regard.
What about the group that will complain about the 30K of memory wasted?
--
Jon Smirl
[email protected]
On Sat, 23 Oct 2004 15:48:03 +0100 (IST), Dave Airlie <[email protected]> wrote:
> > What about the group that will complain about the 30K of memory wasted?
> they'll build their own kernel with CONFIG_AGP turned off... so it won't
> matter...
So the plan then is to eliminate the drm_agp structure and use the
entry points directly. The correct function entry points are already
exported from AGP so no kernel patch is needed.
Dave, with the AGP fix up, do you think linux-core is ready for kernel
submission yet?
--
Jon Smirl
[email protected]
>
> So the plan then is to eliminate the drm_agp structure and use the
> entry points directly. The correct function entry points are already
> exported from AGP so no kernel patch is needed.
It seems like that is the proper way to do it for 2.6....
> Dave, with the AGP fix up, do you think linux-core is ready for kernel
> submission yet?
I'm hoping to look at it tomorrow, I started on it today but got
sidetracked catching up on the changes in CVS and stuff in the kernel
tree.
Dave.
--
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
pam_smb / Linux DECstation / Linux VAX / ILUG person
On Sat, 23 Oct 2004 10:56:44 +0100, Christoph Hellwig <[email protected]> wrote:
> Sorry, wrong API. At least export the individual functions and use them
> directly (and without the symbol_get abnomination that's not any better
> than inter_module_*).
I put a new version in DRM CVS that uses the symbols directly. It
checks out with AGP compiled in and not in.
You might want to give DRM CVS another look. We are getting ready to
submit the code in the linux-core/shared-core directories to the
kernel.
There are still coding style issues. These are slowly getting fixed
but on the other hand we don't want to break the working code. We can
fix a few more on each round of submissions. Main highlights of this
version are elimination of the DRM() macros and elminination of
inter_module() use for both DRM and AGP.
--
Jon Smirl
[email protected]
On Sun, 24 Oct 2004 00:28:18 +1000, Dave Airlie <[email protected]> wrote:
> What about this patch?
>
> The 2.6 kernel module stuff should sort out the dependencies itself, I
> haven't had a chance to test this yet....
I made the same changes locally as new_drm_agp.patch. It works with
both AGP compiled in and not in.
--
Jon Smirl
[email protected]
On Sat, 2004-10-23 at 14:22 -0400, Jon Smirl wrote:
> There are still coding style issues. These are slowly getting fixed
> but on the other hand we don't want to break the working code. We can
> fix a few more on each round of submissions. Main highlights of this
> version are elimination of the DRM() macros and elminination of
> inter_module() use for both DRM and AGP.
Delighted to hear it. I've pushed this patch on and off for a couple of
years now; if you're working on this actively it's probably only good
for a laugh.
Thanks,
Rusty.
Name: Removed inter_module functions from DRM
Signed-off-by: Rusty Russell <[email protected]>
Status: Compiles
Gets rid of inter_module* from DRM drivers, in favour of exporting
symbols and using symbol_get.
This is a classic example of a soft-dependency: DRM wants to use AGP
if it's available, but doesn't want to rely on it. It's limited, in
that loading an AGP driver later won't be used, but it exists.
Index: linux-2.6.10-rc1-bk10-Module/drivers/char/drm/base.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.10-rc1-bk10-Module/drivers/char/drm/base.c 2004-11-01 16:34:56.472190960 +1100
@@ -0,0 +1,44 @@
+/* This (racy) hack exists because the DRM coders are taking
+ * Linux-incompatible drugs. The second drm module attempts to share
+ * the drm_stub_info with whoever got there first: this is detected by
+ * register_chrdev returning -EBUSY. There's no locking, and it's not
+ * clear that loading two drm modules actually does anything useful.
+ *
+ * Copyright (C) 2004 Rusty Russell IBM Corporation
+ */
+#include <linux/module.h>
+
+struct drm_stub_info;
+
+static struct drm_stub_info *drm_stub;
+static struct module *drm_stub_owner;
+
+/* Registers a new stub. */
+void drm_stub_set(struct drm_stub_info *stub_info, struct module *owner)
+{
+ drm_stub = stub_info;
+ drm_stub_owner = owner;
+}
+void drm_stub_unset(void)
+{
+ drm_stub = NULL;
+ drm_stub_owner = NULL;
+}
+
+/* Get and put. */
+struct drm_stub_info *drm_stub_get(void)
+{
+ if (drm_stub) {
+ __module_get(drm_stub_owner);
+ return drm_stub;
+ }
+ return NULL;
+}
+void drm_stub_put(void)
+{
+ module_put(drm_stub_owner);
+}
+EXPORT_SYMBOL(drm_stub_set);
+EXPORT_SYMBOL(drm_stub_unset);
+EXPORT_SYMBOL(drm_stub_get);
+EXPORT_SYMBOL(drm_stub_put);
Index: linux-2.6.10-rc1-bk10-Module/drivers/char/drm/Makefile
===================================================================
--- linux-2.6.10-rc1-bk10-Module.orig/drivers/char/drm/Makefile 2004-10-19 14:33:57.000000000 +1000
+++ linux-2.6.10-rc1-bk10-Module/drivers/char/drm/Makefile 2004-11-01 16:34:56.472190960 +1100
@@ -13,6 +13,8 @@
ffb-objs := ffb_drv.o ffb_context.o
sis-objs := sis_drv.o sis_ds.o sis_mm.o
+obj-y := base.o
+
obj-$(CONFIG_DRM_GAMMA) += gamma.o
obj-$(CONFIG_DRM_TDFX) += tdfx.o
obj-$(CONFIG_DRM_R128) += r128.o
Index: linux-2.6.10-rc1-bk10-Module/drivers/char/drm/drmP.h
===================================================================
--- linux-2.6.10-rc1-bk10-Module.orig/drivers/char/drm/drmP.h 2004-10-19 14:33:57.000000000 +1000
+++ linux-2.6.10-rc1-bk10-Module/drivers/char/drm/drmP.h 2004-11-01 16:34:56.474190656 +1100
@@ -724,6 +724,14 @@
extern void DRM(driver_register_fns)(struct drm_device *dev);
+/* drm functions for handling registration of stubs in base.c */
+struct drm_stub_info;
+extern void drm_stub_set(struct drm_stub_info *, struct module *owner);
+extern void drm_stub_unset(void);
+extern struct drm_stub_info *drm_stub_get(void);
+extern void drm_stub_put(void);
+
+
/******************************************************************/
/** \name Internal function definitions */
/*@{*/
Index: linux-2.6.10-rc1-bk10-Module/drivers/char/agp/backend.c
===================================================================
--- linux-2.6.10-rc1-bk10-Module.orig/drivers/char/agp/backend.c 2004-06-17 08:48:07.000000000 +1000
+++ linux-2.6.10-rc1-bk10-Module/drivers/char/agp/backend.c 2004-11-01 16:34:56.471191112 +1100
@@ -214,7 +214,7 @@
phys_to_virt(bridge->scratch_page_real));
}
-static const drm_agp_t drm_agp = {
+const drm_agp_t drm_agp = {
&agp_free_memory,
&agp_allocate_memory,
&agp_bind_memory,
@@ -224,6 +224,7 @@
&agp_backend_release,
&agp_copy_info
};
+EXPORT_SYMBOL(drm_agp);
/* XXX Kludge alert: agpgart isn't ready for multiple bridges yet */
struct agp_bridge_data *agp_alloc_bridge(void)
@@ -277,9 +278,6 @@
goto frontend_err;
}
- /* FIXME: What to do with this? */
- inter_module_register("drm_agp", THIS_MODULE, &drm_agp);
-
agp_count++;
return 0;
@@ -298,7 +296,6 @@
bridge->type = NOT_SUPPORTED;
agp_frontend_cleanup();
agp_backend_cleanup(bridge);
- inter_module_unregister("drm_agp");
agp_count--;
module_put(bridge->driver->owner);
}
Index: linux-2.6.10-rc1-bk10-Module/drivers/char/drm/drm_stub.h
===================================================================
--- linux-2.6.10-rc1-bk10-Module.orig/drivers/char/drm/drm_stub.h 2004-06-17 08:48:07.000000000 +1000
+++ linux-2.6.10-rc1-bk10-Module/drivers/char/drm/drm_stub.h 2004-11-01 16:34:56.477190200 +1100
@@ -145,9 +145,9 @@
DRM(stub_list)[minor].dev_root);
if (minor) {
class_simple_device_remove(MKDEV(DRM_MAJOR, minor));
- inter_module_put("drm");
+ drm_stub_put();
} else {
- inter_module_unregister("drm");
+ drm_stub_unset();
DRM(free)(DRM(stub_list),
sizeof(*DRM(stub_list)) * DRM_STUB_MAXCARDS,
DRM_MEM_STUB);
@@ -190,7 +190,7 @@
}
}
else if (ret1 == -EBUSY)
- i = (struct drm_stub_info *)inter_module_get("drm");
+ i = drm_stub_get();
else
return -1;
@@ -202,8 +202,8 @@
} else if (DRM(stub_info).info_register != DRM(stub_getminor)) {
DRM(stub_info).info_register = DRM(stub_getminor);
DRM(stub_info).info_unregister = DRM(stub_putminor);
- DRM_DEBUG("calling inter_module_register\n");
- inter_module_register("drm", THIS_MODULE, &DRM(stub_info));
+ DRM_DEBUG("calling drm_stub_set\n");
+ drm_stub_set(&DRM(stub_info), THIS_MODULE);
}
if (DRM(stub_info).info_register) {
ret2 = DRM(stub_info).info_register(name, fops, dev);
@@ -213,7 +213,7 @@
class_simple_destroy(drm_class);
}
if (!i)
- inter_module_unregister("drm");
+ drm_stub_unset();
}
return ret2;
}
Index: linux-2.6.10-rc1-bk10-Module/include/linux/agp_backend.h
===================================================================
--- linux-2.6.10-rc1-bk10-Module.orig/include/linux/agp_backend.h 2003-09-22 10:08:38.000000000 +1000
+++ linux-2.6.10-rc1-bk10-Module/include/linux/agp_backend.h 2004-11-01 16:34:56.484189136 +1100
@@ -112,7 +112,8 @@
int (*copy_info)(struct agp_kern_info *);
} drm_agp_t;
-extern const drm_agp_t *drm_agp_p;
+/* Used by drm. */
+extern const drm_agp_t drm_agp;
#endif /* __KERNEL__ */
#endif /* _AGP_BACKEND_H */
Index: linux-2.6.10-rc1-bk10-Module/drivers/char/drm/drm_agpsupport.h
===================================================================
--- linux-2.6.10-rc1-bk10-Module.orig/drivers/char/drm/drm_agpsupport.h 2004-10-19 14:33:57.000000000 +1000
+++ linux-2.6.10-rc1-bk10-Module/drivers/char/drm/drm_agpsupport.h 2004-11-01 16:36:21.824215480 +1100
@@ -36,13 +36,13 @@
#if __OS_HAS_AGP
-#define DRM_AGP_GET (drm_agp_t *)inter_module_get("drm_agp")
-#define DRM_AGP_PUT inter_module_put("drm_agp")
+#define DRM_AGP_GET symbol_get(drm_agp)
+#define DRM_AGP_PUT symbol_put(drm_agp)
/**
* Pointer to the drm_agp_t structure made available by the agpgart module.
*/
-static const drm_agp_t *drm_agp = NULL;
+static const drm_agp_t *drm_agp_p = NULL;
/**
* AGP information ioctl.
@@ -64,7 +64,7 @@
DRM_AGP_KERN *kern;
drm_agp_info_t info;
- if (!dev->agp || !dev->agp->acquired || !drm_agp->copy_info)
+ if (!dev->agp || !dev->agp->acquired || !drm_agp_p->copy_info)
return -EINVAL;
kern = &dev->agp->agp_info;
@@ -93,7 +93,7 @@
* \return zero on success or a negative number on failure.
*
* Verifies the AGP device hasn't been acquired before and calls
- * drm_agp->acquire().
+ * drm_agp_p->acquire().
*/
int DRM(agp_acquire)(struct inode *inode, struct file *filp,
unsigned int cmd, unsigned long arg)
@@ -106,9 +106,9 @@
return -ENODEV;
if (dev->agp->acquired)
return -EBUSY;
- if (!drm_agp->acquire)
+ if (!drm_agp_p->acquire)
return -EINVAL;
- if ((retcode = drm_agp->acquire()))
+ if ((retcode = drm_agp_p->acquire()))
return retcode;
dev->agp->acquired = 1;
return 0;
@@ -123,7 +123,7 @@
* \param arg user argument.
* \return zero on success or a negative number on failure.
*
- * Verifies the AGP device has been acquired and calls drm_agp->release().
+ * Verifies the AGP device has been acquired and calls drm_agp_p->release().
*/
int DRM(agp_release)(struct inode *inode, struct file *filp,
unsigned int cmd, unsigned long arg)
@@ -131,9 +131,9 @@
drm_file_t *priv = filp->private_data;
drm_device_t *dev = priv->dev;
- if (!dev->agp || !dev->agp->acquired || !drm_agp->release)
+ if (!dev->agp || !dev->agp->acquired || !drm_agp_p->release)
return -EINVAL;
- drm_agp->release();
+ drm_agp_p->release();
dev->agp->acquired = 0;
return 0;
@@ -142,12 +142,12 @@
/**
* Release the AGP device.
*
- * Calls drm_agp->release().
+ * Calls drm_agp_p->release().
*/
void DRM(agp_do_release)(void)
{
- if (drm_agp->release)
- drm_agp->release();
+ if (drm_agp_p->release)
+ drm_agp_p->release();
}
/**
@@ -160,7 +160,7 @@
* \return zero on success or a negative number on failure.
*
* Verifies the AGP device has been acquired but not enabled, and calls
- * drm_agp->enable().
+ * drm_agp_p->enable().
*/
int DRM(agp_enable)(struct inode *inode, struct file *filp,
unsigned int cmd, unsigned long arg)
@@ -169,14 +169,14 @@
drm_device_t *dev = priv->dev;
drm_agp_mode_t mode;
- if (!dev->agp || !dev->agp->acquired || !drm_agp->enable)
+ if (!dev->agp || !dev->agp->acquired || !drm_agp_p->enable)
return -EINVAL;
if (copy_from_user(&mode, (drm_agp_mode_t __user *)arg, sizeof(mode)))
return -EFAULT;
dev->agp->mode = mode.mode;
- drm_agp->enable(mode.mode);
+ drm_agp_p->enable(mode.mode);
dev->agp->base = dev->agp->agp_info.aper_base;
dev->agp->enabled = 1;
return 0;
@@ -325,7 +325,7 @@
int retcode;
int page;
- if (!dev->agp || !dev->agp->acquired || !drm_agp->bind_memory)
+ if (!dev->agp || !dev->agp->acquired || !drm_agp_p->bind_memory)
return -EINVAL;
if (copy_from_user(&request, (drm_agp_binding_t __user *)arg, sizeof(request)))
return -EFAULT;
@@ -392,19 +392,18 @@
* \return pointer to a drm_agp_head structure.
*
* Gets the drm_agp_t structure which is made available by the agpgart module
- * via the inter_module_* functions. Creates and initializes a drm_agp_head
- * structure.
+ * via symbol_get. Creates and initializes a drm_agp_head structure.
*/
drm_agp_head_t *DRM(agp_init)(void)
{
drm_agp_head_t *head = NULL;
- drm_agp = DRM_AGP_GET;
- if (drm_agp) {
+ drm_agp_p = DRM_AGP_GET;
+ if (drm_agp_p) {
if (!(head = DRM(alloc)(sizeof(*head), DRM_MEM_AGPLISTS)))
return NULL;
memset((void *)head, 0, sizeof(*head));
- drm_agp->copy_info(&head->agp_info);
+ drm_agp_p->copy_info(&head->agp_info);
if (head->agp_info.chipset == NOT_SUPPORTED) {
DRM(free)(head, sizeof(*head), DRM_MEM_AGPLISTS);
return NULL;
@@ -429,40 +428,40 @@
void DRM(agp_uninit)(void)
{
DRM_AGP_PUT;
- drm_agp = NULL;
+ drm_agp_p = NULL;
}
-/** Calls drm_agp->allocate_memory() */
+/** Calls drm_agp_p->allocate_memory() */
DRM_AGP_MEM *DRM(agp_allocate_memory)(size_t pages, u32 type)
{
- if (!drm_agp->allocate_memory)
+ if (!drm_agp_p->allocate_memory)
return NULL;
- return drm_agp->allocate_memory(pages, type);
+ return drm_agp_p->allocate_memory(pages, type);
}
/** Calls drm_agp->free_memory() */
int DRM(agp_free_memory)(DRM_AGP_MEM *handle)
{
- if (!handle || !drm_agp->free_memory)
+ if (!handle || !drm_agp_p->free_memory)
return 0;
- drm_agp->free_memory(handle);
+ drm_agp_p->free_memory(handle);
return 1;
}
/** Calls drm_agp->bind_memory() */
int DRM(agp_bind_memory)(DRM_AGP_MEM *handle, off_t start)
{
- if (!handle || !drm_agp->bind_memory)
+ if (!handle || !drm_agp_p->bind_memory)
return -EINVAL;
- return drm_agp->bind_memory(handle, start);
+ return drm_agp_p->bind_memory(handle, start);
}
/** Calls drm_agp->unbind_memory() */
int DRM(agp_unbind_memory)(DRM_AGP_MEM *handle)
{
- if (!handle || !drm_agp->unbind_memory)
+ if (!handle || !drm_agp_p->unbind_memory)
return -EINVAL;
- return drm_agp->unbind_memory(handle);
+ return drm_agp_p->unbind_memory(handle);
}
#endif /* __OS_HAS_AGP */
--
A bad analogy is like a leaky screwdriver -- Richard Braakman
I did notice that patch from a while ago.
The code in DRM CVS works and it has no DRM() macros, inter_module_xx
is gone, and it splits DRM in to DRM-core plus personality modules.
Now all we have to do is get it merged into the kernel. The diffs are
huge due to lindent reformatting and removal of DRM() macros.
cvs -z3 -d:pserver:[email protected]:/cvs/dri co drm
make in linux-core
I'm still working on the merged fbdev/DRM but it doesn't work right
yet. Making fbdev understand multiple heads is causing considerable
changes to fbdev since the fb_info structure does not separate per
head variables from per card ones. I don't have the split sorted out
yet.
The code is based off from DRM CVS. bk://[email protected]/drm-fb
make in linux-fb
--
Jon Smirl
[email protected]
>
> The code in DRM CVS works and it has no DRM() macros, inter_module_xx
> is gone, and it splits DRM in to DRM-core plus personality modules.
> Now all we have to do is get it merged into the kernel. The diffs are
> huge due to lindent reformatting and removal of DRM() macros.
I'm about 50percent through a working tree now ... hopefully I'll
release it for -mm in the next evening or two,
I've got it nearly building completly now then there'll be some
testing on it to do..
Dave.