| Message ID | 1350956055-3224-11-git-send-email-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
I mean to say this patch is RFC On Mon, 22 Oct 2012 18:34:15 -0700 Ben Widawsky <ben@bwidawsk.net> wrote: > Primarily for my own testing to make sure I actually killed off any > dependencies on AGP. > > I'd happily extend this with CONFIG_ options or some such to make it > upstreamable if people were interested. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/Kconfig | 2 -- > drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++ > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/i915_gem.c | 2 ++ > drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++- > 5 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 18321b68b..3afd84d 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -119,8 +119,6 @@ config DRM_I810 > config DRM_I915 > tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics" > depends on DRM > - depends on AGP > - depends on AGP_INTEL > # we need shmfs for the swappable backing store, and in > particular # the shmem_readpage() which depends upon tmpfs > select SHMEM > diff --git a/drivers/gpu/drm/i915/i915_drv.c > b/drivers/gpu/drm/i915/i915_drv.c index a7837e5..cb5a9d8 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -120,7 +120,9 @@ MODULE_PARM_DESC(i915_enable_ppgtt, > "Enable PPGTT (default: true)"); > > static struct drm_driver driver; > +#ifdef KEEP_AGP_DEPS > extern int intel_agp_enabled; > +#endif > > #define INTEL_VGA_DEVICE(id, info) { \ > .class = PCI_BASE_CLASS_DISPLAY << 16, \ > @@ -842,9 +844,15 @@ i915_pci_probe(struct pci_dev *pdev, const > struct pci_device_id *ent) if (intel_info->gen != 3) { > driver.driver_features &= > ~(DRIVER_USE_AGP | DRIVER_REQUIRE_AGP); > +#ifdef KEEP_AGP_DEPS > } else if (!intel_agp_enabled) { > DRM_ERROR("drm/i915 can't work without intel_agp > module!\n"); return -ENODEV; > +#else > + } else if (intel_info->gen < 6) { > + DRM_ERROR("drm/i915 can't work without intel_agp > module!\n"); > + return -ENODEV; > +#endif > } > > return drm_get_pci_dev(pdev, ent, &driver); > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h index 534d282..b331d73 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1511,8 +1511,10 @@ int i915_gem_gtt_init(struct drm_device *dev); > void i915_gem_gtt_fini(struct drm_device *dev); > extern inline void i915_gem_chipset_flush(struct drm_device *dev) > { > +#ifdef KEEP_AGP_DEPS > if (INTEL_INFO(dev)->gen < 6) > intel_gtt_chipset_flush(); > +#endif > } > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > b/drivers/gpu/drm/i915/i915_gem.c index ae3d4c1..2f4ce5d 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3960,8 +3960,10 @@ i915_gem_init_hw(struct drm_device *dev) > drm_i915_private_t *dev_priv = dev->dev_private; > int ret; > > +#ifdef KEEP_AGP_DEPS > if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt()) > return -EIO; > +#endif > > if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1)) > I915_WRITE(0x9008, I915_READ(0x9008) | 0xf0000); > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c index e5f0a7f..5097d0c 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -263,6 +263,7 @@ void i915_ppgtt_unbind_object(struct > i915_hw_ppgtt *ppgtt, obj->base.size >> PAGE_SHIFT); > } > > +#ifdef KEEP_AGP_DEPS > /* XXX kill agp_type! */ > static unsigned int cache_level_to_agp_type(struct drm_device *dev, > enum i915_cache_level > cache_level) @@ -282,6 +283,7 @@ static unsigned int > cache_level_to_agp_type(struct drm_device *dev, return > AGP_USER_MEMORY; } > } > +#endif > > static bool do_idling(struct drm_i915_private *dev_priv) > { > @@ -315,10 +317,12 @@ static void i915_ggtt_clear_range(struct > drm_device *dev, volatile void __iomem *gtt_base = > dev_priv->mm.gtt->gtt + first_entry; const int max_entries = > dev_priv->mm.gtt->gtt_total_entries - first_entry; > +#ifdef KEEP_AGP_DEPS > if (INTEL_INFO(dev)->gen < 6) { > intel_gtt_clear_range(first_entry, num_entries); > return; > } > +#endif > > if (WARN(num_entries > max_entries, > "First entry = %d; Num entries = %d (max=%d)\n", > @@ -404,10 +408,12 @@ void i915_gem_gtt_bind_object(struct > drm_i915_gem_object *obj, { > struct drm_device *dev = obj->base.dev; > if (INTEL_INFO(dev)->gen < 6) { > +#ifdef KEEP_AGP_DEPS > unsigned int agp_type = cache_level_to_agp_type(dev, > cache_level); intel_gtt_insert_sg_entries(obj->pages, > obj->gtt_space->start >> > PAGE_SHIFT, agp_type); > +#endif > } else { > gen6_ggtt_bind_object(obj, cache_level); > } > @@ -549,6 +555,7 @@ int i915_gem_gtt_init(struct drm_device *dev) > /* On modern platforms we need not worry ourself with the > legacy > * hostbridge query stuff. Skip it entirely > */ > +#ifdef KEEP_AGP_DEPS > if (INTEL_INFO(dev)->gen < 6) { > ret = intel_gmch_probe(dev_priv->bridge_dev, > dev->pdev, NULL); if (!ret) { > @@ -564,7 +571,7 @@ int i915_gem_gtt_init(struct drm_device *dev) > } > return 0; > } > - > +#endif > > dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), > GFP_KERNEL); if (!dev_priv->mm.gtt) > @@ -610,7 +617,9 @@ void i915_gem_gtt_fini(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > iounmap(dev_priv->mm.gtt->gtt); > teardown_scratch_page(dev); > +#ifdef KEEP_AGP_DEPS > if (INTEL_INFO(dev)->gen < 6) > intel_gmch_remove(); > +#endif > kfree(dev_priv->mm.gtt); > }
On Mon, 22 Oct 2012 18:34:15 -0700 Ben Widawsky <ben@bwidawsk.net> wrote: > Primarily for my own testing to make sure I actually killed off any > dependencies on AGP. > > I'd happily extend this with CONFIG_ options or some such to make it > upstreamable if people were interested. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/Kconfig | 2 -- > drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++ > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/i915_gem.c | 2 ++ > drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++- > 5 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 18321b68b..3afd84d 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -119,8 +119,6 @@ config DRM_I810 > config DRM_I915 > tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics" > depends on DRM > - depends on AGP > - depends on AGP_INTEL > # we need shmfs for the swappable backing store, and in particular > # the shmem_readpage() which depends upon tmpfs > select SHMEM > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index a7837e5..cb5a9d8 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -120,7 +120,9 @@ MODULE_PARM_DESC(i915_enable_ppgtt, > "Enable PPGTT (default: true)"); > > static struct drm_driver driver; > +#ifdef KEEP_AGP_DEPS > extern int intel_agp_enabled; > +#endif > > #define INTEL_VGA_DEVICE(id, info) { \ > .class = PCI_BASE_CLASS_DISPLAY << 16, \ > @@ -842,9 +844,15 @@ i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > if (intel_info->gen != 3) { > driver.driver_features &= > ~(DRIVER_USE_AGP | DRIVER_REQUIRE_AGP); > +#ifdef KEEP_AGP_DEPS > } else if (!intel_agp_enabled) { > DRM_ERROR("drm/i915 can't work without intel_agp module!\n"); > return -ENODEV; > +#else > + } else if (intel_info->gen < 6) { > + DRM_ERROR("drm/i915 can't work without intel_agp module!\n"); > + return -ENODEV; > +#endif > } > > return drm_get_pci_dev(pdev, ent, &driver); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 534d282..b331d73 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1511,8 +1511,10 @@ int i915_gem_gtt_init(struct drm_device *dev); > void i915_gem_gtt_fini(struct drm_device *dev); > extern inline void i915_gem_chipset_flush(struct drm_device *dev) > { > +#ifdef KEEP_AGP_DEPS > if (INTEL_INFO(dev)->gen < 6) > intel_gtt_chipset_flush(); > +#endif > } > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index ae3d4c1..2f4ce5d 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3960,8 +3960,10 @@ i915_gem_init_hw(struct drm_device *dev) > drm_i915_private_t *dev_priv = dev->dev_private; > int ret; > > +#ifdef KEEP_AGP_DEPS > if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt()) > return -EIO; > +#endif > > if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1)) > I915_WRITE(0x9008, I915_READ(0x9008) | 0xf0000); > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index e5f0a7f..5097d0c 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -263,6 +263,7 @@ void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt, > obj->base.size >> PAGE_SHIFT); > } > > +#ifdef KEEP_AGP_DEPS > /* XXX kill agp_type! */ > static unsigned int cache_level_to_agp_type(struct drm_device *dev, > enum i915_cache_level cache_level) > @@ -282,6 +283,7 @@ static unsigned int cache_level_to_agp_type(struct drm_device *dev, > return AGP_USER_MEMORY; > } > } > +#endif > > static bool do_idling(struct drm_i915_private *dev_priv) > { > @@ -315,10 +317,12 @@ static void i915_ggtt_clear_range(struct drm_device *dev, > volatile void __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry; > const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry; > > +#ifdef KEEP_AGP_DEPS > if (INTEL_INFO(dev)->gen < 6) { > intel_gtt_clear_range(first_entry, num_entries); > return; > } > +#endif > > if (WARN(num_entries > max_entries, > "First entry = %d; Num entries = %d (max=%d)\n", > @@ -404,10 +408,12 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj, > { > struct drm_device *dev = obj->base.dev; > if (INTEL_INFO(dev)->gen < 6) { > +#ifdef KEEP_AGP_DEPS > unsigned int agp_type = cache_level_to_agp_type(dev, cache_level); > intel_gtt_insert_sg_entries(obj->pages, > obj->gtt_space->start >> PAGE_SHIFT, > agp_type); > +#endif > } else { > gen6_ggtt_bind_object(obj, cache_level); > } > @@ -549,6 +555,7 @@ int i915_gem_gtt_init(struct drm_device *dev) > /* On modern platforms we need not worry ourself with the legacy > * hostbridge query stuff. Skip it entirely > */ > +#ifdef KEEP_AGP_DEPS > if (INTEL_INFO(dev)->gen < 6) { > ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL); > if (!ret) { > @@ -564,7 +571,7 @@ int i915_gem_gtt_init(struct drm_device *dev) > } > return 0; > } > - > +#endif > > dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), GFP_KERNEL); > if (!dev_priv->mm.gtt) > @@ -610,7 +617,9 @@ void i915_gem_gtt_fini(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > iounmap(dev_priv->mm.gtt->gtt); > teardown_scratch_page(dev); > +#ifdef KEEP_AGP_DEPS > if (INTEL_INFO(dev)->gen < 6) > intel_gmch_remove(); > +#endif > kfree(dev_priv->mm.gtt); > } It would be cool to actually remove the dep for pre-gen6. I don't think we need this patch in either case.
On Thu, 25 Oct 2012 14:09:09 -0700 Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > On Mon, 22 Oct 2012 18:34:15 -0700 > Ben Widawsky <ben@bwidawsk.net> wrote: > > > Primarily for my own testing to make sure I actually killed off any > > dependencies on AGP. > > > > I'd happily extend this with CONFIG_ options or some such to make it > > upstreamable if people were interested. > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/Kconfig | 2 -- > > drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++ > > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > > drivers/gpu/drm/i915/i915_gem.c | 2 ++ > > drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++- > > 5 files changed, 22 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > > index 18321b68b..3afd84d 100644 > > --- a/drivers/gpu/drm/Kconfig > > +++ b/drivers/gpu/drm/Kconfig > > @@ -119,8 +119,6 @@ config DRM_I810 > > config DRM_I915 > > tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics" > > depends on DRM > > - depends on AGP > > - depends on AGP_INTEL > > # we need shmfs for the swappable backing store, and in particular > > # the shmem_readpage() which depends upon tmpfs > > select SHMEM > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index a7837e5..cb5a9d8 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -120,7 +120,9 @@ MODULE_PARM_DESC(i915_enable_ppgtt, > > "Enable PPGTT (default: true)"); > > > > static struct drm_driver driver; > > +#ifdef KEEP_AGP_DEPS > > extern int intel_agp_enabled; > > +#endif > > > > #define INTEL_VGA_DEVICE(id, info) { \ > > .class = PCI_BASE_CLASS_DISPLAY << 16, \ > > @@ -842,9 +844,15 @@ i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > if (intel_info->gen != 3) { > > driver.driver_features &= > > ~(DRIVER_USE_AGP | DRIVER_REQUIRE_AGP); > > +#ifdef KEEP_AGP_DEPS > > } else if (!intel_agp_enabled) { > > DRM_ERROR("drm/i915 can't work without intel_agp module!\n"); > > return -ENODEV; > > +#else > > + } else if (intel_info->gen < 6) { > > + DRM_ERROR("drm/i915 can't work without intel_agp module!\n"); > > + return -ENODEV; > > +#endif > > } > > > > return drm_get_pci_dev(pdev, ent, &driver); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 534d282..b331d73 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1511,8 +1511,10 @@ int i915_gem_gtt_init(struct drm_device *dev); > > void i915_gem_gtt_fini(struct drm_device *dev); > > extern inline void i915_gem_chipset_flush(struct drm_device *dev) > > { > > +#ifdef KEEP_AGP_DEPS > > if (INTEL_INFO(dev)->gen < 6) > > intel_gtt_chipset_flush(); > > +#endif > > } > > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > index ae3d4c1..2f4ce5d 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -3960,8 +3960,10 @@ i915_gem_init_hw(struct drm_device *dev) > > drm_i915_private_t *dev_priv = dev->dev_private; > > int ret; > > > > +#ifdef KEEP_AGP_DEPS > > if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt()) > > return -EIO; > > +#endif > > > > if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1)) > > I915_WRITE(0x9008, I915_READ(0x9008) | 0xf0000); > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index e5f0a7f..5097d0c 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -263,6 +263,7 @@ void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt, > > obj->base.size >> PAGE_SHIFT); > > } > > > > +#ifdef KEEP_AGP_DEPS > > /* XXX kill agp_type! */ > > static unsigned int cache_level_to_agp_type(struct drm_device *dev, > > enum i915_cache_level cache_level) > > @@ -282,6 +283,7 @@ static unsigned int cache_level_to_agp_type(struct drm_device *dev, > > return AGP_USER_MEMORY; > > } > > } > > +#endif > > > > static bool do_idling(struct drm_i915_private *dev_priv) > > { > > @@ -315,10 +317,12 @@ static void i915_ggtt_clear_range(struct drm_device *dev, > > volatile void __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry; > > const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry; > > > > +#ifdef KEEP_AGP_DEPS > > if (INTEL_INFO(dev)->gen < 6) { > > intel_gtt_clear_range(first_entry, num_entries); > > return; > > } > > +#endif > > > > if (WARN(num_entries > max_entries, > > "First entry = %d; Num entries = %d (max=%d)\n", > > @@ -404,10 +408,12 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj, > > { > > struct drm_device *dev = obj->base.dev; > > if (INTEL_INFO(dev)->gen < 6) { > > +#ifdef KEEP_AGP_DEPS > > unsigned int agp_type = cache_level_to_agp_type(dev, cache_level); > > intel_gtt_insert_sg_entries(obj->pages, > > obj->gtt_space->start >> PAGE_SHIFT, > > agp_type); > > +#endif > > } else { > > gen6_ggtt_bind_object(obj, cache_level); > > } > > @@ -549,6 +555,7 @@ int i915_gem_gtt_init(struct drm_device *dev) > > /* On modern platforms we need not worry ourself with the legacy > > * hostbridge query stuff. Skip it entirely > > */ > > +#ifdef KEEP_AGP_DEPS > > if (INTEL_INFO(dev)->gen < 6) { > > ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL); > > if (!ret) { > > @@ -564,7 +571,7 @@ int i915_gem_gtt_init(struct drm_device *dev) > > } > > return 0; > > } > > - > > +#endif > > > > dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), GFP_KERNEL); > > if (!dev_priv->mm.gtt) > > @@ -610,7 +617,9 @@ void i915_gem_gtt_fini(struct drm_device *dev) > > struct drm_i915_private *dev_priv = dev->dev_private; > > iounmap(dev_priv->mm.gtt->gtt); > > teardown_scratch_page(dev); > > +#ifdef KEEP_AGP_DEPS > > if (INTEL_INFO(dev)->gen < 6) > > intel_gmch_remove(); > > +#endif > > kfree(dev_priv->mm.gtt); > > } > > It would be cool to actually remove the dep for pre-gen6. I don't > think we need this patch in either case. > Your linker is going to complain without it, unless I don't follow your meaning.
On Thu, 25 Oct 2012 15:15:02 -0700 Ben Widawsky <ben@bwidawsk.net> wrote: > On Thu, 25 Oct 2012 14:09:09 -0700 > Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > > > On Mon, 22 Oct 2012 18:34:15 -0700 > > Ben Widawsky <ben@bwidawsk.net> wrote: > > > > > Primarily for my own testing to make sure I actually killed off any > > > dependencies on AGP. > > > > > > I'd happily extend this with CONFIG_ options or some such to make it > > > upstreamable if people were interested. > > > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > > --- > > > drivers/gpu/drm/Kconfig | 2 -- > > > drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++ > > > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > > > drivers/gpu/drm/i915/i915_gem.c | 2 ++ > > > drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++- > > > 5 files changed, 22 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > > > index 18321b68b..3afd84d 100644 > > > --- a/drivers/gpu/drm/Kconfig > > > +++ b/drivers/gpu/drm/Kconfig > > > @@ -119,8 +119,6 @@ config DRM_I810 > > > config DRM_I915 > > > tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics" > > > depends on DRM > > > - depends on AGP > > > - depends on AGP_INTEL > > > # we need shmfs for the swappable backing store, and in particular > > > # the shmem_readpage() which depends upon tmpfs > > > select SHMEM > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > > index a7837e5..cb5a9d8 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -120,7 +120,9 @@ MODULE_PARM_DESC(i915_enable_ppgtt, > > > "Enable PPGTT (default: true)"); > > > > > > static struct drm_driver driver; > > > +#ifdef KEEP_AGP_DEPS > > > extern int intel_agp_enabled; > > > +#endif > > > > > > #define INTEL_VGA_DEVICE(id, info) { \ > > > .class = PCI_BASE_CLASS_DISPLAY << 16, \ > > > @@ -842,9 +844,15 @@ i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > > if (intel_info->gen != 3) { > > > driver.driver_features &= > > > ~(DRIVER_USE_AGP | DRIVER_REQUIRE_AGP); > > > +#ifdef KEEP_AGP_DEPS > > > } else if (!intel_agp_enabled) { > > > DRM_ERROR("drm/i915 can't work without intel_agp module!\n"); > > > return -ENODEV; > > > +#else > > > + } else if (intel_info->gen < 6) { > > > + DRM_ERROR("drm/i915 can't work without intel_agp module!\n"); > > > + return -ENODEV; > > > +#endif > > > } > > > > > > return drm_get_pci_dev(pdev, ent, &driver); > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > > index 534d282..b331d73 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -1511,8 +1511,10 @@ int i915_gem_gtt_init(struct drm_device *dev); > > > void i915_gem_gtt_fini(struct drm_device *dev); > > > extern inline void i915_gem_chipset_flush(struct drm_device *dev) > > > { > > > +#ifdef KEEP_AGP_DEPS > > > if (INTEL_INFO(dev)->gen < 6) > > > intel_gtt_chipset_flush(); > > > +#endif > > > } > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > > index ae3d4c1..2f4ce5d 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem.c > > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > > @@ -3960,8 +3960,10 @@ i915_gem_init_hw(struct drm_device *dev) > > > drm_i915_private_t *dev_priv = dev->dev_private; > > > int ret; > > > > > > +#ifdef KEEP_AGP_DEPS > > > if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt()) > > > return -EIO; > > > +#endif > > > > > > if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1)) > > > I915_WRITE(0x9008, I915_READ(0x9008) | 0xf0000); > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > > index e5f0a7f..5097d0c 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > > @@ -263,6 +263,7 @@ void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt, > > > obj->base.size >> PAGE_SHIFT); > > > } > > > > > > +#ifdef KEEP_AGP_DEPS > > > /* XXX kill agp_type! */ > > > static unsigned int cache_level_to_agp_type(struct drm_device *dev, > > > enum i915_cache_level cache_level) > > > @@ -282,6 +283,7 @@ static unsigned int cache_level_to_agp_type(struct drm_device *dev, > > > return AGP_USER_MEMORY; > > > } > > > } > > > +#endif > > > > > > static bool do_idling(struct drm_i915_private *dev_priv) > > > { > > > @@ -315,10 +317,12 @@ static void i915_ggtt_clear_range(struct drm_device *dev, > > > volatile void __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry; > > > const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry; > > > > > > +#ifdef KEEP_AGP_DEPS > > > if (INTEL_INFO(dev)->gen < 6) { > > > intel_gtt_clear_range(first_entry, num_entries); > > > return; > > > } > > > +#endif > > > > > > if (WARN(num_entries > max_entries, > > > "First entry = %d; Num entries = %d (max=%d)\n", > > > @@ -404,10 +408,12 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj, > > > { > > > struct drm_device *dev = obj->base.dev; > > > if (INTEL_INFO(dev)->gen < 6) { > > > +#ifdef KEEP_AGP_DEPS > > > unsigned int agp_type = cache_level_to_agp_type(dev, cache_level); > > > intel_gtt_insert_sg_entries(obj->pages, > > > obj->gtt_space->start >> PAGE_SHIFT, > > > agp_type); > > > +#endif > > > } else { > > > gen6_ggtt_bind_object(obj, cache_level); > > > } > > > @@ -549,6 +555,7 @@ int i915_gem_gtt_init(struct drm_device *dev) > > > /* On modern platforms we need not worry ourself with the legacy > > > * hostbridge query stuff. Skip it entirely > > > */ > > > +#ifdef KEEP_AGP_DEPS > > > if (INTEL_INFO(dev)->gen < 6) { > > > ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL); > > > if (!ret) { > > > @@ -564,7 +571,7 @@ int i915_gem_gtt_init(struct drm_device *dev) > > > } > > > return 0; > > > } > > > - > > > +#endif > > > > > > dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), GFP_KERNEL); > > > if (!dev_priv->mm.gtt) > > > @@ -610,7 +617,9 @@ void i915_gem_gtt_fini(struct drm_device *dev) > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > iounmap(dev_priv->mm.gtt->gtt); > > > teardown_scratch_page(dev); > > > +#ifdef KEEP_AGP_DEPS > > > if (INTEL_INFO(dev)->gen < 6) > > > intel_gmch_remove(); > > > +#endif > > > kfree(dev_priv->mm.gtt); > > > } > > > > It would be cool to actually remove the dep for pre-gen6. I don't > > think we need this patch in either case. > > > > Your linker is going to complain without it, unless I don't follow your > meaning. I mean unless we split the driver into gen6+ and pre-gen6 (with a config option or otherwise), we can't really kill the dependency unless we actually pull the rest of the AGP stuff we need into i915. I'd kind of like to do the latter, but the non-KMS configs would break.
On Thu, Oct 25, 2012 at 03:39:08PM -0700, Jesse Barnes wrote: > > > It would be cool to actually remove the dep for pre-gen6. I don't > > > think we need this patch in either case. > > > > > > > Your linker is going to complain without it, unless I don't follow your > > meaning. > > I mean unless we split the driver into gen6+ and pre-gen6 (with a > config option or otherwise), we can't really kill the dependency unless > we actually pull the rest of the AGP stuff we need into i915. I'd kind > of like to do the latter, but the non-KMS configs would break. Also, some kms config actually break because we've managed to ship userspace that uses kms, but in parts also still uses the legacy drmMap api. Which in our case means we need to have a working (but fake) agp driver around. This entire thing is one giant mess, but I think with the gen6+ stuff moved it's about as good as it will get. And I don't mind the old crap too much, we have an entire dungeon full of them in i915_dma.c, too ;-) -Daniel
On Fri, 26 Oct 2012 00:47:31 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Oct 25, 2012 at 03:39:08PM -0700, Jesse Barnes wrote: > > > > It would be cool to actually remove the dep for pre-gen6. I > > > > don't think we need this patch in either case. > > > > > > > > > > Your linker is going to complain without it, unless I don't > > > follow your meaning. > > > > I mean unless we split the driver into gen6+ and pre-gen6 (with a > > config option or otherwise), we can't really kill the dependency > > unless we actually pull the rest of the AGP stuff we need into > > i915. I'd kind of like to do the latter, but the non-KMS configs > > would break. > > Also, some kms config actually break because we've managed to ship > userspace that uses kms, but in parts also still uses the legacy > drmMap api. Which in our case means we need to have a working (but > fake) agp driver around. This entire thing is one giant mess, but I > think with the gen6+ stuff moved it's about as good as it will get. > And I don't mind the old crap too much, we have an entire dungeon > full of them in i915_dma.c, too ;-) > -Daniel The idea (as discussed on IRC) was to provide something like a CONFIG_GEN6 which will properly detect and error out for older platforms. This would allow embedded systems to save on the AGP size when they are trying to store everything in flash. Anyway, for now, I think we all agree that this is unnecessary, but if someone building an embedded system with i915 graphics wants to save roughly 500k of binary data size (and some smaller amount of runtime memory)...
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 18321b68b..3afd84d 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -119,8 +119,6 @@ config DRM_I810 config DRM_I915 tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics" depends on DRM - depends on AGP - depends on AGP_INTEL # we need shmfs for the swappable backing store, and in particular # the shmem_readpage() which depends upon tmpfs select SHMEM diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index a7837e5..cb5a9d8 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -120,7 +120,9 @@ MODULE_PARM_DESC(i915_enable_ppgtt, "Enable PPGTT (default: true)"); static struct drm_driver driver; +#ifdef KEEP_AGP_DEPS extern int intel_agp_enabled; +#endif #define INTEL_VGA_DEVICE(id, info) { \ .class = PCI_BASE_CLASS_DISPLAY << 16, \ @@ -842,9 +844,15 @@ i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (intel_info->gen != 3) { driver.driver_features &= ~(DRIVER_USE_AGP | DRIVER_REQUIRE_AGP); +#ifdef KEEP_AGP_DEPS } else if (!intel_agp_enabled) { DRM_ERROR("drm/i915 can't work without intel_agp module!\n"); return -ENODEV; +#else + } else if (intel_info->gen < 6) { + DRM_ERROR("drm/i915 can't work without intel_agp module!\n"); + return -ENODEV; +#endif } return drm_get_pci_dev(pdev, ent, &driver); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 534d282..b331d73 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1511,8 +1511,10 @@ int i915_gem_gtt_init(struct drm_device *dev); void i915_gem_gtt_fini(struct drm_device *dev); extern inline void i915_gem_chipset_flush(struct drm_device *dev) { +#ifdef KEEP_AGP_DEPS if (INTEL_INFO(dev)->gen < 6) intel_gtt_chipset_flush(); +#endif } diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ae3d4c1..2f4ce5d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3960,8 +3960,10 @@ i915_gem_init_hw(struct drm_device *dev) drm_i915_private_t *dev_priv = dev->dev_private; int ret; +#ifdef KEEP_AGP_DEPS if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt()) return -EIO; +#endif if (IS_HASWELL(dev) && (I915_READ(0x120010) == 1)) I915_WRITE(0x9008, I915_READ(0x9008) | 0xf0000); diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index e5f0a7f..5097d0c 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -263,6 +263,7 @@ void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt, obj->base.size >> PAGE_SHIFT); } +#ifdef KEEP_AGP_DEPS /* XXX kill agp_type! */ static unsigned int cache_level_to_agp_type(struct drm_device *dev, enum i915_cache_level cache_level) @@ -282,6 +283,7 @@ static unsigned int cache_level_to_agp_type(struct drm_device *dev, return AGP_USER_MEMORY; } } +#endif static bool do_idling(struct drm_i915_private *dev_priv) { @@ -315,10 +317,12 @@ static void i915_ggtt_clear_range(struct drm_device *dev, volatile void __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry; const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry; +#ifdef KEEP_AGP_DEPS if (INTEL_INFO(dev)->gen < 6) { intel_gtt_clear_range(first_entry, num_entries); return; } +#endif if (WARN(num_entries > max_entries, "First entry = %d; Num entries = %d (max=%d)\n", @@ -404,10 +408,12 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj, { struct drm_device *dev = obj->base.dev; if (INTEL_INFO(dev)->gen < 6) { +#ifdef KEEP_AGP_DEPS unsigned int agp_type = cache_level_to_agp_type(dev, cache_level); intel_gtt_insert_sg_entries(obj->pages, obj->gtt_space->start >> PAGE_SHIFT, agp_type); +#endif } else { gen6_ggtt_bind_object(obj, cache_level); } @@ -549,6 +555,7 @@ int i915_gem_gtt_init(struct drm_device *dev) /* On modern platforms we need not worry ourself with the legacy * hostbridge query stuff. Skip it entirely */ +#ifdef KEEP_AGP_DEPS if (INTEL_INFO(dev)->gen < 6) { ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL); if (!ret) { @@ -564,7 +571,7 @@ int i915_gem_gtt_init(struct drm_device *dev) } return 0; } - +#endif dev_priv->mm.gtt = kzalloc(sizeof(*dev_priv->mm.gtt), GFP_KERNEL); if (!dev_priv->mm.gtt) @@ -610,7 +617,9 @@ void i915_gem_gtt_fini(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; iounmap(dev_priv->mm.gtt->gtt); teardown_scratch_page(dev); +#ifdef KEEP_AGP_DEPS if (INTEL_INFO(dev)->gen < 6) intel_gmch_remove(); +#endif kfree(dev_priv->mm.gtt); }
Primarily for my own testing to make sure I actually killed off any dependencies on AGP. I'd happily extend this with CONFIG_ options or some such to make it upstreamable if people were interested. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/Kconfig | 2 -- drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++ drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_gem.c | 2 ++ drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++- 5 files changed, 22 insertions(+), 3 deletions(-)