[PATCH 2/2] drm/i915: Add dependency on the intel agp module

[Chase Douglas]

On Wed, Apr 7, 2010 at 10:37 AM, Andy Whitcroft <apw at canonical.com> wrote:
> On Wed, Apr 07, 2010 at 09:48:15AM -0400, Chase Douglas wrote:
>> On Wed, Apr 7, 2010 at 6:24 AM, Andy Whitcroft <apw at canonical.com> wrote:
>> > From: Zhenyu Wang <zhenyuw at linux.intel.com>
>> >
>> > See http://bugzilla.kernel.org/show_bug.cgi?id=15021
>> >
>> > Make sure that the appropriate AGP module is loaded and probed before
>> > trying to set up the DRM.  The DRM already depends on the AGP core,
>> > but in this case we know the specific AGP driver we need too, and can
>> > help users avoid the trap of loading the AGP driver after the DRM
>> > driver.
>> >
>> > Signed-off-by: Zhenyu Wang <zhenyuw at linux.intel.com>
>> > Signed-off-by: Eric Anholt <eric at anholt.net>
>> >
>> > (cherry picked from commit 1f7a6e372e9cb4d749f34c0738d832e6cadb4071 upstream)
>> > BugLink: http://bugs.launchpad.net/bugs/542251
>> >
>> > Signed-off-by: Andy Whitcroft <apw at canonical.com>
>> > ---
>> >  drivers/char/agp/intel-agp.c    |   10 ++++++++--
>> >  drivers/gpu/drm/i915/i915_drv.c |    6 ++++++
>> >  2 files changed, 14 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
>> > index 4dcfef0..89f5609 100644
>> > --- a/drivers/char/agp/intel-agp.c
>> > +++ b/drivers/char/agp/intel-agp.c
>> > @@ -10,6 +10,9 @@
>> >  #include <linux/agp_backend.h>
>> >  #include "agp.h"
>> >
>> > +int intel_agp_enabled;
>> > +EXPORT_SYMBOL(intel_agp_enabled);
>> > +
>> >  /*
>> >  * If we have Intel graphics, we're not going to have anything other than
>> >  * an Intel IOMMU. So make the correct use of the PCI DMA API contingent
>> > @@ -2378,7 +2381,7 @@ static int __devinit agp_intel_probe(struct pci_dev *pdev,
>> >        struct agp_bridge_data *bridge;
>> >        u8 cap_ptr = 0;
>> >        struct resource *r;
>> > -       int i;
>> > +       int i, err;
>> >
>> >        cap_ptr = pci_find_capability(pdev, PCI_CAP_ID_AGP);
>> >
>> > @@ -2466,7 +2469,10 @@ static int __devinit agp_intel_probe(struct pci_dev *pdev,
>> >                                "set gfx device dma mask 36bit failed!\n");
>> >
>> >        pci_set_drvdata(pdev, bridge);
>> > -       return agp_add_bridge(bridge);
>> > +       err = agp_add_bridge(bridge);
>> > +       if (!err)
>> > +               intel_agp_enabled = 1;
>> > +       return err;
>> >  }
>> >
>> >  static void __devexit agp_intel_remove(struct pci_dev *pdev)
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> > index 4746bfe..f7d7c12 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.c
>> > +++ b/drivers/gpu/drm/i915/i915_drv.c
>> > @@ -49,6 +49,7 @@ unsigned int i915_lvds_downclock = 0;
>> >  module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400);
>> >
>> >  static struct drm_driver driver;
>> > +extern int intel_agp_enabled;
>> >
>> >  #define INTEL_VGA_DEVICE(id, info) {           \
>> >        .class = PCI_CLASS_DISPLAY_VGA << 8,    \
>> > @@ -546,6 +547,11 @@ static struct drm_driver driver = {
>> >
>> >  static int __init i915_init(void)
>> >  {
>> > +       if (!intel_agp_enabled) {
>> > +               DRM_ERROR("drm/i915 can't work without intel_agp module!\n");
>> > +               return -ENODEV;
>> > +       }
>> > +
>>
>> What happens here if the agp isn't enabled yet? It seems like the
>> driver just gives up. How does this help?
>
> The new dependancy between the modules created by the reference to this
> variable triggers the other module to be loaded before this one.  If it
> loads but does not manage to find a valid GART then there is little
> point in loading i915.
>

I didn't realize module loading was this clever :). This sounds good to me.

>> >        driver.num_ioctls = i915_max_ioctl;
>> >
>> >        i915_gem_shrinker_init();
>> > --
>> > 1.7.0

Acked-by: Chase Douglas <chase.douglas at canonical.com>