| Message ID | 1270635860-10883-3-git-send-email-apw@canonical.com |
|---|---|
| State | Accepted |
| Delegated to: | Andy Whitcroft |
| Headers | show |
On Wed, Apr 7, 2010 at 6:24 AM, Andy Whitcroft <apw@canonical.com> wrote: > From: Zhenyu Wang <zhenyuw@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@linux.intel.com> > Signed-off-by: Eric Anholt <eric@anholt.net> > > (cherry picked from commit 1f7a6e372e9cb4d749f34c0738d832e6cadb4071 upstream) > BugLink: http://bugs.launchpad.net/bugs/542251 > > Signed-off-by: Andy Whitcroft <apw@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? > driver.num_ioctls = i915_max_ioctl; > > i915_gem_shrinker_init(); > -- > 1.7.0 -- Chase
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@canonical.com> wrote: > > From: Zhenyu Wang <zhenyuw@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@linux.intel.com> > > Signed-off-by: Eric Anholt <eric@anholt.net> > > > > (cherry picked from commit 1f7a6e372e9cb4d749f34c0738d832e6cadb4071 upstream) > > BugLink: http://bugs.launchpad.net/bugs/542251 > > > > Signed-off-by: Andy Whitcroft <apw@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. > > driver.num_ioctls = i915_max_ioctl; > > > > i915_gem_shrinker_init(); > > -- > > 1.7.0 > > -- Chase
On Wed, Apr 7, 2010 at 10:37 AM, Andy Whitcroft <apw@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@canonical.com> wrote: >> > From: Zhenyu Wang <zhenyuw@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@linux.intel.com> >> > Signed-off-by: Eric Anholt <eric@anholt.net> >> > >> > (cherry picked from commit 1f7a6e372e9cb4d749f34c0738d832e6cadb4071 upstream) >> > BugLink: http://bugs.launchpad.net/bugs/542251 >> > >> > Signed-off-by: Andy Whitcroft <apw@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@canonical.com>
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; + } + driver.num_ioctls = i915_max_ioctl; i915_gem_shrinker_init();