diff mbox

[2/4] drm/i915: Stop depending upon CONFIG_AGP_INTEL

Message ID 1453901881-26425-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter Jan. 27, 2016, 1:37 p.m. UTC
The AGP_INTEL driver provides an interface for very old userspace to
control the GART (though the GART itself was only ever emulated on Intel
systems). The pci bridge discovery code is also used by the i915.ko
driver to set up the GTT on old systems, but it does not require the
old userspace interface. When i915.ko selects the old interface, it
binds another user to the core GTT routines, and in particular creates a
second reference to the scratch pages allocated. This hinders resource
leak debugging for when we unload i915.ko as we want to assert that all
DMA pages have been released, but we appear to leak because of the
secondary interface which persists after i915.ko unloads.

All i915.ko users do not require the old /dev/agpgart interface so stop
selecting it and simplify our debugging by dropping the historical
baggage.

Note that by selecting AGP=n it was already possible to unselect
AGP_INTEL. But since we've dropped support for any of the AGP stuff
long ago there's really no point for this any more.

Also note that we still need INTEL_GTT, which is the underlying,
share, driver for the graphics GART on gen1-5.

v2: Entirely new commit message (Chris, Ville).

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/Kconfig | 2 --
 1 file changed, 2 deletions(-)

Comments

Chris Wilson Jan. 27, 2016, 2:55 p.m. UTC | #1
On Wed, Jan 27, 2016 at 02:37:59PM +0100, Daniel Vetter wrote:
> The AGP_INTEL driver provides an interface for very old userspace to
> control the GART (though the GART itself was only ever emulated on Intel
> systems). The pci bridge discovery code is also used by the i915.ko
> driver to set up the GTT on old systems, but it does not require the
> old userspace interface. When i915.ko selects the old interface, it
> binds another user to the core GTT routines, and in particular creates a
> second reference to the scratch pages allocated. This hinders resource
> leak debugging for when we unload i915.ko as we want to assert that all
> DMA pages have been released, but we appear to leak because of the
> secondary interface which persists after i915.ko unloads.
> 
> All i915.ko users do not require the old /dev/agpgart interface so stop
> selecting it and simplify our debugging by dropping the historical
> baggage.
> 
> Note that by selecting AGP=n it was already possible to unselect
> AGP_INTEL. But since we've dropped support for any of the AGP stuff
> long ago there's really no point for this any more.
> 
> Also note that we still need INTEL_GTT, which is the underlying,
> share, driver for the graphics GART on gen1-5.
/share,/shared
 
> v2: Entirely new commit message (Chris, Ville).
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Ville Syrjala Jan. 27, 2016, 3:05 p.m. UTC | #2
On Wed, Jan 27, 2016 at 02:37:59PM +0100, Daniel Vetter wrote:
> The AGP_INTEL driver provides an interface for very old userspace to
> control the GART (though the GART itself was only ever emulated on Intel
> systems). The pci bridge discovery code is also used by the i915.ko
> driver to set up the GTT on old systems, but it does not require the
> old userspace interface. When i915.ko selects the old interface, it
> binds another user to the core GTT routines, and in particular creates a
> second reference to the scratch pages allocated. This hinders resource
> leak debugging for when we unload i915.ko as we want to assert that all
> DMA pages have been released, but we appear to leak because of the
> secondary interface which persists after i915.ko unloads.
> 
> All i915.ko users do not require the old /dev/agpgart interface so stop
> selecting it and simplify our debugging by dropping the historical
> baggage.
> 
> Note that by selecting AGP=n it was already possible to unselect
> AGP_INTEL. But since we've dropped support for any of the AGP stuff
> long ago there's really no point for this any more.

Hmm. There surely was some kind of dependency between intel-agp and i915
which required the former to be builtin when i915 was, and that was the
reason for the kconfig magic IIRC. Not quite sure why I made i915 depend
on AGP and select INTEL_AGP though, instead of just
'depends on (INTEL_AGP || INTEL_AGP=n)'
but I guess there was some reason.

Anyways, after looking at git logs, I guess the dependency was
'extern int intel_agp_enabled' which got killed off in
commit 3bb6ce668663 ("drm/i915: Kill legeacy AGP for gen3 kms")
so perhaps note that in the commit message.

But yeah, with that stuff gone I think this should be fine, so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Also note that we still need INTEL_GTT, which is the underlying,
> share, driver for the graphics GART on gen1-5.
  ^^^^^

shared?

> 
> v2: Entirely new commit message (Chris, Ville).
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/Kconfig | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index fcd77b27514d..9aa7d2d98add 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -2,9 +2,7 @@ config DRM_I915
>  	tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics"
>  	depends on DRM
>  	depends on X86 && PCI
> -	depends on (AGP || AGP=n)
>  	select INTEL_GTT
> -	select AGP_INTEL if AGP
>  	select INTERVAL_TREE
>  	# we need shmfs for the swappable backing store, and in particular
>  	# the shmem_readpage() which depends upon tmpfs
> -- 
> 2.5.0
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index fcd77b27514d..9aa7d2d98add 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -2,9 +2,7 @@  config DRM_I915
 	tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics"
 	depends on DRM
 	depends on X86 && PCI
-	depends on (AGP || AGP=n)
 	select INTEL_GTT
-	select AGP_INTEL if AGP
 	select INTERVAL_TREE
 	# we need shmfs for the swappable backing store, and in particular
 	# the shmem_readpage() which depends upon tmpfs