Re: [PATCH] Make agpsupport work with modversions

From: Rik Faith (faith@valinux.com)
Date: Wed Oct 18 2000 - 13:13:06 EST


On Wed 18 Oct 2000 09:43:43 -0700,
   Linus Torvalds <torvalds@transmeta.com> wrote:
>
>
> On Wed, 18 Oct 2000, John Levon wrote:
> >
> > I have only compile-tested the patch below with 2.4.0test10pre3 and
> > 2.2.18pre16 (some fuzz on apply). Hope it's right, I can't test if it
> > fixes the MODVERSIONS+in kernel agp+in kernel drm case. I tested kernel
> > and module cases.
>
> It looks better.

We apparently had never changed the Makefile over to the new format.
The dependency on CONFIG_MODULES is essentially orthogonal to this
problem.

> However, the fact that you need that dependency on CONFIG_MODULES _still_
> shows that something is wrong. That dependency should not be there, and
> the drm code should be fixed. Why does it care about CONFIG_MODULES at
> all? It should not, and it _must_ not do that.
>
> I have no idea what the get_module_symbol() code in question is trying to
> do, but this should be _fixed_ and not just worked around. That's a bug.

The get_module_symbol() call will get the agp symbols if agpgart is
available (either as a loaded module, or compiled into the kernel).
Some DRM drivers optionally make use of agpgart, so it's useful for
the DRM driver to look for agpgart support at run time even if it
isn't available (e.g., the R128 driver may use AGP on an AGP system,
but not on a PCI system; or you might have the same type of chip on an
AGP card and on a PCI card, and you'd like to be able to load the same
drm module for both cards).

There are several cases that the configuration can have:

    CONFIG_AGP CONFIG_MODULES
    n n or y no agpsupport.c, so no problem

    m or y y agpsupport.c uses get_module_symbol,
                                   and the drm runs with or without
                                   agpgart support (which may or may
                                   not be a module that may or may not
                                   be loaded). [This is the reason
                                   we're using get_module_symbol -- I
                                   didn't realize it wasn't available
                                   if CONFIG_MODULES was 'n'.]

    y n This is the broken case, which can
                                   be fixed if agpsupport.c uses the
                                   #ifdef below -- this case ensures
                                   that the agpgart module is built
                                   into the monolithic kernel and we
                                   can safely hardcode the function
                                   names at compile time.

#ifdef CONFIG_MODULES
        for (fill = &drm_agp_fill[0]; fill->name; fill++) {
                char *n = (char *)fill->name;
                *fill->f = (drm_agp_func_u)get_module_symbol(NULL, n);
                DRM_DEBUG("%s resolves to 0x%08lx\n", n, (*fill->f).address);
                if (!(*fill->f).address) agp_available = 0;
        }
#else
        drm_agp_fill[0].f = agp_free_memory;
        drm_agp_fill[1].f = agp_allocate_memory;
        drm_agp_fill[2].f = agp_bind_memory;
        drm_agp_fill[3].f = agp_unbind_memory;
        drm_agp_fill[4].f = agp_enable;
        drm_agp_fill[5].f = agp_backend_acquire;
        drm_agp_fill[6].f = agp_backend_release;
        drm_agp_fill[7].f = agp_copy_info;
#endif

[Note that the other way to fix this would be to export
get_module_symbol all the time, and have it just search the available
symbol space if CONFIG_MODULES is 'n'.]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Mon Oct 23 2000 - 21:00:13 EST