[Intel-gfx] [PATCH 3/4] agp/intel-gtt: Only register fake agp driver for gen1

[Daniel Vetter]

On Thu, Feb 11, 2016 at 12:31:45PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 27, 2016 at 02:38:00PM +0100, Daniel Vetter wrote:
> > The fake agp driver for the intel graphics gart is only needed for ums
> > support. And we ditched that a long time ago:
> > 
> > commit 03dae59c72ffffd8ef6e005f48ba356c863e0587
> > Author: Daniel Vetter <daniel.vetter at ffwll.ch>
> > Date:   Wed Jul 23 16:27:25 2014 +0200
> > 
> >     drm/i915: Ditch UMS config option
> > 
> > With this there's no longer the problem that 2 drivers (fake agp
> > driver and the drm/i915 driver) fight over the same piece, which fixes
> > apparent dma leaks detected by CONFIG_DMA_API_DEBUG.
> > 
> > Note that the leak isn't real since intel-gtt refcounts and will tear
> > down eventually. But the debug code assumes that when the i915 driver
> > unbinds from the pci device everything should be gone. Which isn't the
> > case if we have intel-agp enabled - userspace might need it. But by
> > ditching this intel-gtt setup and teardown is completely tied to the
> > livetime of the "real" driver.
> > 
> > While at it untangle the init ordering a bit - the fake agp wouldn't
> > be initialized correctly if i915.ko loads first. Which isn't a problem
> > since when i915 loads in kms mode you won't need the fake agp support
> > needed by the ums driver ...
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93793
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > ---
> >  drivers/char/agp/intel-gtt.c | 24 ++++++++++++------------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> > index e657f989745e..aef87fdbd187 100644
> > --- a/drivers/char/agp/intel-gtt.c
> > +++ b/drivers/char/agp/intel-gtt.c
> > @@ -1348,16 +1348,6 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
> >  {
> >  	int i, mask;
> >  
> > -	/*
> > -	 * Can be called from the fake agp driver but also directly from
> > -	 * drm/i915.ko. Hence we need to check whether everything is set up
> > -	 * already.
> > -	 */
> > -	if (intel_private.driver) {
> > -		intel_private.refcount++;
> > -		return 1;
> > -	}
> > -
> >  	for (i = 0; intel_gtt_chipsets[i].name != NULL; i++) {
> >  		if (gpu_pdev) {
> >  			if (gpu_pdev->device ==
> > @@ -1378,16 +1368,26 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
> >  	if (!intel_private.driver)
> >  		return 0;
> >  
> > -	intel_private.refcount++;
> > -
> >  #if IS_ENABLED(CONFIG_AGP_INTEL)
> >  	if (bridge) {
> > +		if (INTEL_GTT_GEN > 1)
> > +			return 0;
> 
> So if this happens first, we set up intel_private.driver but leave the
> refcount at 0. Then i915 loads and we set up intel_private.driver again,
> and bump the refcount to 0. Well, we should end up pointing
> intel_privatee.driver at the same thing both times so not really a
> problem I suppose.
> 
> So yeah, I think this ought to work
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Thanks for the review, applied.

> I guess we could also trim the gen>=2 gmch pci ids from the agp
> driver's pci id table, to avoid even probing it.

Hm yeah, forgot about that. I'll try to remember and do a follow-up patch.
-Daniel

> 
> > +
> >  		bridge->driver = &intel_fake_agp_driver;
> >  		bridge->dev_private_data = &intel_private;
> >  		bridge->dev = bridge_pdev;
> >  	}
> >  #endif
> >  
> > +
> > +	/*
> > +	 * Can be called from the fake agp driver but also directly from
> > +	 * drm/i915.ko. Hence we need to check whether everything is set up
> > +	 * already.
> > +	 */
> > +	if (intel_private.refcount++)
> > +		return 1;
> > +
> >  	intel_private.bridge_dev = pci_dev_get(bridge_pdev);
> >  
> >  	dev_info(&bridge_pdev->dev, "Intel %s Chipset\n", intel_gtt_chipsets[i].name);
> > -- 
> > 2.5.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch