[PATCH 00/19] drm de-midlayer, part 1: device list and agp bus stuff

[PATCH 00/19] drm de-midlayer, part 1: device list and agp bus stuff

[Daniel Vetter]

Hi all,

So at ks there was a lot of talk about fixing up the drm device model to stop
insanity like imx. And I kinda promised Dave to resurrect my latest branch at
drm demidlayering - motivation kinda dissipated when I've started this a few
moons back.

So this is the first part. Patches 1-11 rework the device list. The ugly part is
that drm_platform_exit survived from the horrible days of shadow attache despite
that no platform drm driver with shadow attach was ever merged. And everyone
dutifully copypasted the crap out of this. Compiletested for all drm drivers
that work with arm multiplatforms. Luckily that now includes exynons (yay), but
the new kid msm on the block fails this (shame on Rob).

The remaining stuff flattens some of the agp indirection madness away. Plus a
tiny patch in between as a follow-up cleanup for David's vma offset manager
work. Also includes one patch from David, rebased into my series.

Next up on my plan is to get rid of drm_bus completely. I already have a plan
for the irq stuff and a few patches, which leaves the setversion/busid/...
bonghits madness. But that's luckily constraint to the drm core.

Once that's settled I think we can start to untangle the driver load/setup vs.
interface registration mess. David&Dave already have patches to play around with
that. Then we need to untangle all the drm lifetime rules around open files,
mmaps, sysfs and debugfs files so that finally we can try to fix the driver
unload madness. Or at least that's my overtly optimistic plan.

Flames, comments and more good&dry wood for the pyre highly welcome.

Cheers, Daniel

Daniel Vetter (18):
  drm/rcar: call drm_put_dev directly in the ->remove hook
  drm/exynos: call drm_put_dev directly from ->remove
  drm/imx: directly call drm_put_dev in ->remove
  drm/tilcdc: call drm_put_dev directly from ->remove
  drm/omap: call drm_put_dev directly in ->remove
  drm/shmob: call drm_put_dev directly from ->remove hook
  drm/host1x: Call drm_put_dev directly instead of drm_platform_exit
  drm/armada: directly call drm_put_dev in ->remove
  drm/msm: call drm_put_dev directly in ->remove
  drm: rip out drm_platform_exit
  drm: restrict the device list for shadow attached drivers
  drm/bufs: remove handling of _DRM_GEM mappings
  drm: kill DRIVER_REQUIRE_AGP
  drm: ->agp_init can't fail
  drm: rip out drm_core_has_AGP
  drm: inline drm_agp_destroy
  drm: kill the ->agp_destroy callback
  drm: remove global_mutex locking around agp_init

David Herrmann (1):
  drm: remove agp_init() bus callback

 drivers/gpu/drm/armada/armada_drv.c      |  3 ++-
 drivers/gpu/drm/drm_agpsupport.c         | 17 +-------------
 drivers/gpu/drm/drm_bufs.c               | 10 ++-------
 drivers/gpu/drm/drm_memory.c             |  9 +++-----
 drivers/gpu/drm/drm_pci.c                | 38 +++++++++++++++++---------------
 drivers/gpu/drm/drm_platform.c           | 12 ----------
 drivers/gpu/drm/drm_stub.c               | 19 +++-------------
 drivers/gpu/drm/drm_usb.c                |  1 -
 drivers/gpu/drm/drm_vm.c                 |  7 ++----
 drivers/gpu/drm/exynos/exynos_drm_drv.c  |  4 +++-
 drivers/gpu/drm/i810/i810_dma.c          |  4 ++++
 drivers/gpu/drm/i810/i810_drv.c          |  2 +-
 drivers/gpu/drm/i915/i915_dma.c          |  5 +++++
 drivers/gpu/drm/i915/i915_drv.c          |  5 ++---
 drivers/gpu/drm/msm/msm_drv.c            |  2 +-
 drivers/gpu/drm/omapdrm/omap_drv.c       |  4 ++--
 drivers/gpu/drm/radeon/radeon_ttm.c      |  2 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.c    |  4 +++-
 drivers/gpu/drm/shmobile/shmob_drm_drv.c |  4 +++-
 drivers/gpu/drm/tilcdc/tilcdc_drv.c      |  2 +-
 drivers/gpu/host1x/drm/drm.c             |  3 +--
 drivers/staging/imx-drm/imx-drm-core.c   |  4 +++-
 include/drm/drmP.h                       | 13 ++++-------
 include/drm/drm_agpsupport.h             | 17 --------------
 include/uapi/drm/drm.h                   |  1 -
 25 files changed, 67 insertions(+), 125 deletions(-)

-- 
1.8.4.rc3

[PATCH 01/19] drm/rcar: call drm_put_dev directly in the ->remove hook

[Daniel Vetter]

The magic dance drm_platform_exit does is actually a remnant of the
old legacy shadow attach support for platform devices. Modern modesetting
drm drivers shouldn't do this any more (and usb/pci devices actually don't
do this).

Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 0023f9719cf1..4ec6272a1c11 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -224,7 +224,9 @@ static int rcar_du_probe(struct platform_device *pdev)
 
 static int rcar_du_remove(struct platform_device *pdev)
 {
-	drm_platform_exit(&rcar_du_driver, pdev);
+	struct rcar_du_device *rcdu = platform_get_drvdata(pdev);
+
+	drm_put_dev(rcdu->ddev);
 
 	return 0;
 }
-- 
1.8.4.rc3

Re: [PATCH 01/19] drm/rcar: call drm_put_dev directly in the ->remove hook

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Sunday 03 November 2013 14:31:07 Daniel Vetter wrote:
> The magic dance drm_platform_exit does is actually a remnant of the
> old legacy shadow attach support for platform devices. Modern modesetting
> drm drivers shouldn't do this any more (and usb/pci devices actually don't
> do this).
> 
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

For 01/19 and 06/19,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 0023f9719cf1..4ec6272a1c11
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -224,7 +224,9 @@ static int rcar_du_probe(struct platform_device *pdev)
> 
>  static int rcar_du_remove(struct platform_device *pdev)
>  {
> -	drm_platform_exit(&rcar_du_driver, pdev);
> +	struct rcar_du_device *rcdu = platform_get_drvdata(pdev);
> +
> +	drm_put_dev(rcdu->ddev);
> 
>  	return 0;
>  }
-- 
Regards,

Laurent Pinchart

[PATCH 02/19] drm/exynos: call drm_put_dev directly from ->remove

[Daniel Vetter]

I didn't find any user of the driver data yet, so store the
drm_device pointer in there.

Cc: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 3a1e6d9b25f7..f1d066180056 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -119,6 +119,8 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
 
 	drm_vblank_offdelay = VBLANK_OFF_DELAY;
 
+	platform_set_drvdata(dev->platformdev, dev);
+
 	return 0;
 
 err_drm_device:
@@ -292,7 +294,7 @@ static int exynos_drm_platform_probe(struct platform_device *pdev)
 
 static int exynos_drm_platform_remove(struct platform_device *pdev)
 {
-	drm_platform_exit(&exynos_drm_driver, pdev);
+	drm_put_dev(platform_get_drvdata(pdev));
 
 	return 0;
 }
-- 
1.8.4.rc3

[PATCH 03/19] drm/imx: directly call drm_put_dev in ->remove

[Daniel Vetter]

Again no apparent user of the driver data field.

Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/staging/imx-drm/imx-drm-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
index c1014eb2907d..8ffd4a900bb9 100644
--- a/drivers/staging/imx-drm/imx-drm-core.c
+++ b/drivers/staging/imx-drm/imx-drm-core.c
@@ -432,6 +432,8 @@ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags)
 	if (!imx_drm_device_get())
 		ret = -EINVAL;
 
+	platform_set_drvdata(drm->platformdev, drm);
+
 	ret = 0;
 
 err_init:
@@ -807,7 +809,7 @@ static int imx_drm_platform_probe(struct platform_device *pdev)
 
 static int imx_drm_platform_remove(struct platform_device *pdev)
 {
-	drm_platform_exit(&imx_drm_driver, pdev);
+	drm_put_dev(platform_get_drvdata(pdev));
 
 	return 0;
 }
-- 
1.8.4.rc3

Re: [PATCH 03/19] drm/imx: directly call drm_put_dev in ->remove

[Sascha Hauer]

On Sun, Nov 03, 2013 at 02:31:09PM +0100, Daniel Vetter wrote:
> Again no apparent user of the driver data field.
> 
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Acked-by: Sascha Hauer <s.hauer@pengutronix.de>

Sascha

> ---
>  drivers/staging/imx-drm/imx-drm-core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
> index c1014eb2907d..8ffd4a900bb9 100644
> --- a/drivers/staging/imx-drm/imx-drm-core.c
> +++ b/drivers/staging/imx-drm/imx-drm-core.c
> @@ -432,6 +432,8 @@ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags)
>  	if (!imx_drm_device_get())
>  		ret = -EINVAL;
>  
> +	platform_set_drvdata(drm->platformdev, drm);
> +
>  	ret = 0;
>  
>  err_init:
> @@ -807,7 +809,7 @@ static int imx_drm_platform_probe(struct platform_device *pdev)
>  
>  static int imx_drm_platform_remove(struct platform_device *pdev)
>  {
> -	drm_platform_exit(&imx_drm_driver, pdev);
> +	drm_put_dev(platform_get_drvdata(pdev));
>  
>  	return 0;
>  }
> -- 
> 1.8.4.rc3
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 04/19] drm/tilcdc: call drm_put_dev directly from ->remove

[Daniel Vetter]

tilcdc already stores the drm_device in the driver data pointer. So
use that.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 116da199b942..e6d77d02e444 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -594,7 +594,7 @@ static int tilcdc_pdev_probe(struct platform_device *pdev)
 
 static int tilcdc_pdev_remove(struct platform_device *pdev)
 {
-	drm_platform_exit(&tilcdc_driver, pdev);
+	drm_put_dev(platform_get_drvdata(pdev));
 
 	return 0;
 }
-- 
1.8.4.rc3

[PATCH 05/19] drm/omap: call drm_put_dev directly in ->remove

[Daniel Vetter]

Again omap already sets the driver data pointer to the drm_device.

Also drop the driver unregister call, that should be (and already is)
done in the module unload hook.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index e7fa3cd96743..13f294aeaefd 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -665,9 +665,9 @@ static int pdev_probe(struct platform_device *device)
 static int pdev_remove(struct platform_device *device)
 {
 	DBG("");
-	drm_platform_exit(&omap_drm_driver, device);
 
-	platform_driver_unregister(&omap_dmm_driver);
+	drm_put_dev(platform_get_drvdata(device));
+
 	return 0;
 }
 
-- 
1.8.4.rc3

[PATCH 06/19] drm/shmob: call drm_put_dev directly from ->remove hook

[Daniel Vetter]

We need to chase one pointer here.

Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/shmobile/shmob_drm_drv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
index 015551866b4a..c839c9c89efb 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
@@ -336,7 +336,9 @@ static int shmob_drm_probe(struct platform_device *pdev)
 
 static int shmob_drm_remove(struct platform_device *pdev)
 {
-	drm_platform_exit(&shmob_drm_driver, pdev);
+	struct shmob_drm_device *sdev = platform_get_drvdata(pdev);
+
+	drm_put_dev(sdev->ddev);
 
 	return 0;
 }
-- 
1.8.4.rc3

[PATCH 07/19] drm/host1x: Call drm_put_dev directly instead of drm_platform_exit

[Daniel Vetter]

I'm a bit confused about how this all works wrt host1x clients, but
this patch looks like the right thing to me.

Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/host1x/drm/drm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/host1x/drm/drm.c b/drivers/gpu/host1x/drm/drm.c
index df7d90a3a4fa..e2eabc4078d9 100644
--- a/drivers/gpu/host1x/drm/drm.c
+++ b/drivers/gpu/host1x/drm/drm.c
@@ -161,7 +161,6 @@ int host1x_drm_init(struct host1x_drm *host1x, struct drm_device *drm)
 
 int host1x_drm_exit(struct host1x_drm *host1x)
 {
-	struct platform_device *pdev = to_platform_device(host1x->dev);
 	struct host1x_client *client;
 
 	if (!host1x->drm)
@@ -184,7 +183,7 @@ int host1x_drm_exit(struct host1x_drm *host1x)
 
 	mutex_unlock(&host1x->clients_lock);
 
-	drm_platform_exit(&tegra_drm_driver, pdev);
+	drm_put_dev(host1x->drm);
 	host1x->drm = NULL;
 
 	return 0;
-- 
1.8.4.rc3

Re: [PATCH 07/19] drm/host1x: Call drm_put_dev directly instead of drm_platform_exit

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 752 bytes --]

On Sun, Nov 03, 2013 at 02:31:13PM +0100, Daniel Vetter wrote:
> I'm a bit confused about how this all works wrt host1x clients, but
> this patch looks like the right thing to me.
> 
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/host1x/drm/drm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

This has been largely rewritten in the code that I submitted in my 3.13
pull request. In fact Tegra DRM no longer uses drm_platform, but a
custom bus type to deal with the quirkiness of the multi-driver
architecture.

So if you really want to get rid of drm_bus altogether I'd appreciate a
heads-up so I know what's coming and can prepare.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/19] drm/host1x: Call drm_put_dev directly instead of drm_platform_exit

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 967 bytes --]

On Mon, Nov 04, 2013 at 10:10:38AM +0100, Thierry Reding wrote:
> On Sun, Nov 03, 2013 at 02:31:13PM +0100, Daniel Vetter wrote:
> > I'm a bit confused about how this all works wrt host1x clients, but
> > this patch looks like the right thing to me.
> > 
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/host1x/drm/drm.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> This has been largely rewritten in the code that I submitted in my 3.13
> pull request. In fact Tegra DRM no longer uses drm_platform, but a
> custom bus type to deal with the quirkiness of the multi-driver
> architecture.

And for what it's worth, there's a drm_host1x_exit() that actually does
only drm_put_dev(), which I guess could be simplified in the way that
you've done here, but I sort of liked the symmetry between an *_init()/
*_exit() pair of functions.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/19] drm/host1x: Call drm_put_dev directly instead of drm_platform_exit

[Daniel Vetter]

On Mon, Nov 4, 2013 at 10:10 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Sun, Nov 03, 2013 at 02:31:13PM +0100, Daniel Vetter wrote:
>> I'm a bit confused about how this all works wrt host1x clients, but
>> this patch looks like the right thing to me.
>>
>> Cc: Thierry Reding <thierry.reding@gmail.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>  drivers/gpu/host1x/drm/drm.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> This has been largely rewritten in the code that I submitted in my 3.13
> pull request. In fact Tegra DRM no longer uses drm_platform, but a
> custom bus type to deal with the quirkiness of the multi-driver
> architecture.
>
> So if you really want to get rid of drm_bus altogether I'd appreciate a
> heads-up so I know what's coming and can prepare.

It's almost completely gone now, the one thing left to do is rip out
bus->set_busid. But I have that planned already.

http://cgit.freedesktop.org/~danvet/drm/log/?h=drm-init-cleanup

I guess I get to rebase the entire thing again, meh.

<rant>
It'd be awesome of other drm drivers would submit their stuff a bit
earlier, atm there's absolutely nothing in drm-next beside all of
intel and a few things for armada... Maybe we need to have a drm
integration tree to make this a bit less frustrating.
</rant>

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 07/19] drm/host1x: Call drm_put_dev directly instead of drm_platform_exit

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1686 bytes --]

On Mon, Nov 04, 2013 at 11:14:39AM +0100, Daniel Vetter wrote:
> On Mon, Nov 4, 2013 at 10:10 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Sun, Nov 03, 2013 at 02:31:13PM +0100, Daniel Vetter wrote:
> >> I'm a bit confused about how this all works wrt host1x clients, but
> >> this patch looks like the right thing to me.
> >>
> >> Cc: Thierry Reding <thierry.reding@gmail.com>
> >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> ---
> >>  drivers/gpu/host1x/drm/drm.c | 3 +--
> >>  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > This has been largely rewritten in the code that I submitted in my 3.13
> > pull request. In fact Tegra DRM no longer uses drm_platform, but a
> > custom bus type to deal with the quirkiness of the multi-driver
> > architecture.
> >
> > So if you really want to get rid of drm_bus altogether I'd appreciate a
> > heads-up so I know what's coming and can prepare.
> 
> It's almost completely gone now, the one thing left to do is rip out
> bus->set_busid. But I have that planned already.
> 
> http://cgit.freedesktop.org/~danvet/drm/log/?h=drm-init-cleanup
> 
> I guess I get to rebase the entire thing again, meh.
> 
> <rant>
> It'd be awesome of other drm drivers would submit their stuff a bit
> earlier, atm there's absolutely nothing in drm-next beside all of
> intel and a few things for armada... Maybe we need to have a drm
> integration tree to make this a bit less frustrating.
> </rant>

Yeah, I know. I had actually planned on sending out this pull request
much earlier, but then all the discussion around DRM panel and device
tree bindings started again...

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 08/19] drm/armada: directly call drm_put_dev in ->remove

[Daniel Vetter]

Again no apparent user of the driver data field.

Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/armada/armada_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index 4f2b28354915..069f64533ac3 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -128,6 +128,7 @@ static int armada_drm_load(struct drm_device *dev, unsigned long flags)
 		return -ENOMEM;
 	}
 
+	platform_set_drvdata(dev->platformdev, dev);
 	dev->dev_private = priv;
 
 	/* Get the implementation specific driver data. */
@@ -376,7 +377,7 @@ static int armada_drm_probe(struct platform_device *pdev)
 
 static int armada_drm_remove(struct platform_device *pdev)
 {
-	drm_platform_exit(&armada_drm_driver, pdev);
+	drm_put_dev(platform_get_drvdata(pdev));
 	return 0;
 }
 
-- 
1.8.4.rc3

[PATCH 09/19] drm/msm: call drm_put_dev directly in ->remove

[Daniel Vetter]

The drvdata pointer is already assigned to something useful.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/msm/msm_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index b3a2f1629041..b92155de0d31 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -747,7 +747,7 @@ static int msm_pdev_probe(struct platform_device *pdev)
 
 static int msm_pdev_remove(struct platform_device *pdev)
 {
-	drm_platform_exit(&msm_driver, pdev);
+	drm_put_dev(platform_get_drvdata(pdev));
 
 	return 0;
 }
-- 
1.8.4.rc3

[PATCH 10/19] drm: rip out drm_platform_exit

[Daniel Vetter]

This very much looks like a remnant of the old legady ums shadow
attach days. Now with the last users gone we can rip it out since
we won't ever support an ums drm driver again.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_platform.c | 11 -----------
 include/drm/drmP.h             |  1 -
 2 files changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
index fc24fee8ec83..56a48033eced 100644
--- a/drivers/gpu/drm/drm_platform.c
+++ b/drivers/gpu/drm/drm_platform.c
@@ -151,14 +151,3 @@ int drm_platform_init(struct drm_driver *driver, struct platform_device *platfor
 	return drm_get_platform_dev(platform_device, driver);
 }
 EXPORT_SYMBOL(drm_platform_init);
-
-void drm_platform_exit(struct drm_driver *driver, struct platform_device *platform_device)
-{
-	struct drm_device *dev, *tmp;
-	DRM_DEBUG("\n");
-
-	list_for_each_entry_safe(dev, tmp, &driver->device_list, driver_item)
-		drm_put_dev(dev);
-	DRM_INFO("Module unloaded\n");
-}
-EXPORT_SYMBOL(drm_platform_exit);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 7062307a4a2d..54d6c2d36a5b 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1675,7 +1675,6 @@ extern int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *speed_mask);
 
 /* platform section */
 extern int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device);
-extern void drm_platform_exit(struct drm_driver *driver, struct platform_device *platform_device);
 
 /* returns true if currently okay to sleep */
 static __inline__ bool drm_can_sleep(void)
-- 
1.8.4.rc3

[PATCH 11/19] drm: restrict the device list for shadow attached drivers

[Daniel Vetter]

There's really no need for the drm core to keep a list of all
devices of a given driver - the linux device model keeps perfect
track of this already for us.

The exception is old legacy ums drivers using pci shadow attaching.
So rename the lists to make the use case clearer and rip out everything
else.

v2: Rebase on top of David Herrmann's drm device register changes.
Also drop the bogus dev_set_drvdata for platform drivers that somehow
crept into the original version - drivers really should be in full
control of that field.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_pci.c      | 12 ++++++++++--
 drivers/gpu/drm/drm_platform.c |  1 -
 drivers/gpu/drm/drm_stub.c     |  4 ----
 drivers/gpu/drm/drm_usb.c      |  1 -
 include/drm/drmP.h             |  6 +++---
 5 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index f00d7a9671ea..ec78fc805ed0 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -346,6 +346,11 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 		 driver->name, driver->major, driver->minor, driver->patchlevel,
 		 driver->date, pci_name(pdev), dev->primary->index);
 
+	/* No locking needed since shadow-attach is single-threaded since it may
+	 * only be called from the per-driver module init hook. */
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		list_add_tail(&dev->legacy_dev_list, &driver->legacy_dev_list);
+
 	return 0;
 
 err_pci:
@@ -375,7 +380,6 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
 
 	DRM_DEBUG("\n");
 
-	INIT_LIST_HEAD(&driver->device_list);
 	driver->kdriver.pci = pdriver;
 	driver->bus = &drm_pci_bus;
 
@@ -384,6 +388,7 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
 
 	/* If not using KMS, fall back to stealth mode manual scanning. */
 	for (i = 0; pdriver->id_table[i].vendor != 0; i++) {
+		INIT_LIST_HEAD(&driver->legacy_dev_list);
 		pid = &pdriver->id_table[i];
 
 		/* Loop around setting up a DRM device for each PCI device
@@ -465,8 +470,11 @@ void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver)
 	if (driver->driver_features & DRIVER_MODESET) {
 		pci_unregister_driver(pdriver);
 	} else {
-		list_for_each_entry_safe(dev, tmp, &driver->device_list, driver_item)
+		list_for_each_entry_safe(dev, tmp, &driver->legacy_dev_list,
+					 legacy_dev_list) {
 			drm_put_dev(dev);
+			list_del(&dev->legacy_dev_list);
+		}
 	}
 	DRM_INFO("Module unloaded\n");
 }
diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
index 56a48033eced..21fc82006b78 100644
--- a/drivers/gpu/drm/drm_platform.c
+++ b/drivers/gpu/drm/drm_platform.c
@@ -147,7 +147,6 @@ int drm_platform_init(struct drm_driver *driver, struct platform_device *platfor
 
 	driver->kdriver.platform_device = platform_device;
 	driver->bus = &drm_platform_bus;
-	INIT_LIST_HEAD(&driver->device_list);
 	return drm_get_platform_dev(platform_device, driver);
 }
 EXPORT_SYMBOL(drm_platform_init);
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 26055abf94ee..047d1d1fb0e1 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -538,8 +538,6 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 			goto err_unload;
 	}
 
-	list_add_tail(&dev->driver_item, &dev->driver->device_list);
-
 	ret = 0;
 	goto out_unlock;
 
@@ -593,7 +591,5 @@ void drm_dev_unregister(struct drm_device *dev)
 	if (dev->render)
 		drm_put_minor(&dev->render);
 	drm_put_minor(&dev->primary);
-
-	list_del(&dev->driver_item);
 }
 EXPORT_SYMBOL(drm_dev_unregister);
diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c
index b179b70e7853..21ae8d96880b 100644
--- a/drivers/gpu/drm/drm_usb.c
+++ b/drivers/gpu/drm/drm_usb.c
@@ -63,7 +63,6 @@ int drm_usb_init(struct drm_driver *driver, struct usb_driver *udriver)
 	int res;
 	DRM_DEBUG("\n");
 
-	INIT_LIST_HEAD(&driver->device_list);
 	driver->kdriver.usb = udriver;
 	driver->bus = &drm_usb_bus;
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 54d6c2d36a5b..e3dfbae963a5 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -988,8 +988,8 @@ struct drm_driver {
 	} kdriver;
 	struct drm_bus *bus;
 
-	/* List of devices hanging off this driver */
-	struct list_head device_list;
+	/* List of devices hanging off this driver with stealth attach. */
+	struct list_head legacy_dev_list;
 };
 
 #define DRM_MINOR_UNASSIGNED 0
@@ -1099,7 +1099,7 @@ struct drm_vblank_crtc {
  * may contain multiple heads.
  */
 struct drm_device {
-	struct list_head driver_item;	/**< list of devices per driver */
+	struct list_head legacy_dev_list;/**< list of devices per driver for stealth attach cleanup */
 	char *devname;			/**< For /proc/interrupts */
 	int if_version;			/**< Highest interface version set */
 
-- 
1.8.4.rc3

Re: [PATCH 11/19] drm: restrict the device list for shadow attached drivers

[David Herrmann]

Hi Daniel

On Sun, Nov 3, 2013 at 2:31 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> There's really no need for the drm core to keep a list of all
> devices of a given driver - the linux device model keeps perfect
> track of this already for us.
>
> The exception is old legacy ums drivers using pci shadow attaching.
> So rename the lists to make the use case clearer and rip out everything
> else.
>
> v2: Rebase on top of David Herrmann's drm device register changes.
> Also drop the bogus dev_set_drvdata for platform drivers that somehow
> crept into the original version - drivers really should be in full
> control of that field.

You didn't really change any dev_set_drvdata, did you? And I guess you
mean pci_set_drvdata()? I had to keep it in place in drm_pci.c as it
has been there before my device-registration changes. However, with
your series you added the pci_set_drvdata() everywhere yourself, so
yes, please remove it.

>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_pci.c      | 12 ++++++++++--
>  drivers/gpu/drm/drm_platform.c |  1 -
>  drivers/gpu/drm/drm_stub.c     |  4 ----
>  drivers/gpu/drm/drm_usb.c      |  1 -
>  include/drm/drmP.h             |  6 +++---
>  5 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index f00d7a9671ea..ec78fc805ed0 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -346,6 +346,11 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
>                  driver->name, driver->major, driver->minor, driver->patchlevel,
>                  driver->date, pci_name(pdev), dev->primary->index);
>
> +       /* No locking needed since shadow-attach is single-threaded since it may
> +        * only be called from the per-driver module init hook. */
> +       if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +               list_add_tail(&dev->legacy_dev_list, &driver->legacy_dev_list);
> +
>         return 0;
>
>  err_pci:
> @@ -375,7 +380,6 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
>
>         DRM_DEBUG("\n");
>
> -       INIT_LIST_HEAD(&driver->device_list);
>         driver->kdriver.pci = pdriver;
>         driver->bus = &drm_pci_bus;
>
> @@ -384,6 +388,7 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
>
>         /* If not using KMS, fall back to stealth mode manual scanning. */
>         for (i = 0; pdriver->id_table[i].vendor != 0; i++) {
> +               INIT_LIST_HEAD(&driver->legacy_dev_list);

Why doing this inside of the loop? If the pci_get_dev() below
succeeds, you clear the list again?

Thanks
David

>                 pid = &pdriver->id_table[i];
>
>                 /* Loop around setting up a DRM device for each PCI device
> @@ -465,8 +470,11 @@ void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver)
>         if (driver->driver_features & DRIVER_MODESET) {
>                 pci_unregister_driver(pdriver);
>         } else {
> -               list_for_each_entry_safe(dev, tmp, &driver->device_list, driver_item)
> +               list_for_each_entry_safe(dev, tmp, &driver->legacy_dev_list,
> +                                        legacy_dev_list) {
>                         drm_put_dev(dev);
> +                       list_del(&dev->legacy_dev_list);
> +               }
>         }
>         DRM_INFO("Module unloaded\n");
>  }
> diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
> index 56a48033eced..21fc82006b78 100644
> --- a/drivers/gpu/drm/drm_platform.c
> +++ b/drivers/gpu/drm/drm_platform.c
> @@ -147,7 +147,6 @@ int drm_platform_init(struct drm_driver *driver, struct platform_device *platfor
>
>         driver->kdriver.platform_device = platform_device;
>         driver->bus = &drm_platform_bus;
> -       INIT_LIST_HEAD(&driver->device_list);
>         return drm_get_platform_dev(platform_device, driver);
>  }
>  EXPORT_SYMBOL(drm_platform_init);
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 26055abf94ee..047d1d1fb0e1 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -538,8 +538,6 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>                         goto err_unload;
>         }
>
> -       list_add_tail(&dev->driver_item, &dev->driver->device_list);
> -
>         ret = 0;
>         goto out_unlock;
>
> @@ -593,7 +591,5 @@ void drm_dev_unregister(struct drm_device *dev)
>         if (dev->render)
>                 drm_put_minor(&dev->render);
>         drm_put_minor(&dev->primary);
> -
> -       list_del(&dev->driver_item);
>  }
>  EXPORT_SYMBOL(drm_dev_unregister);
> diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c
> index b179b70e7853..21ae8d96880b 100644
> --- a/drivers/gpu/drm/drm_usb.c
> +++ b/drivers/gpu/drm/drm_usb.c
> @@ -63,7 +63,6 @@ int drm_usb_init(struct drm_driver *driver, struct usb_driver *udriver)
>         int res;
>         DRM_DEBUG("\n");
>
> -       INIT_LIST_HEAD(&driver->device_list);
>         driver->kdriver.usb = udriver;
>         driver->bus = &drm_usb_bus;
>
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 54d6c2d36a5b..e3dfbae963a5 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -988,8 +988,8 @@ struct drm_driver {
>         } kdriver;
>         struct drm_bus *bus;
>
> -       /* List of devices hanging off this driver */
> -       struct list_head device_list;
> +       /* List of devices hanging off this driver with stealth attach. */
> +       struct list_head legacy_dev_list;
>  };
>
>  #define DRM_MINOR_UNASSIGNED 0
> @@ -1099,7 +1099,7 @@ struct drm_vblank_crtc {
>   * may contain multiple heads.
>   */
>  struct drm_device {
> -       struct list_head driver_item;   /**< list of devices per driver */
> +       struct list_head legacy_dev_list;/**< list of devices per driver for stealth attach cleanup */
>         char *devname;                  /**< For /proc/interrupts */
>         int if_version;                 /**< Highest interface version set */
>
> --
> 1.8.4.rc3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 11/19] drm: restrict the device list for shadow attached drivers

[Daniel Vetter]

On Sun, Nov 3, 2013 at 3:05 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> On Sun, Nov 3, 2013 at 2:31 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> There's really no need for the drm core to keep a list of all
>> devices of a given driver - the linux device model keeps perfect
>> track of this already for us.
>>
>> The exception is old legacy ums drivers using pci shadow attaching.
>> So rename the lists to make the use case clearer and rip out everything
>> else.
>>
>> v2: Rebase on top of David Herrmann's drm device register changes.
>> Also drop the bogus dev_set_drvdata for platform drivers that somehow
>> crept into the original version - drivers really should be in full
>> control of that field.
>
> You didn't really change any dev_set_drvdata, did you? And I guess you
> mean pci_set_drvdata()? I had to keep it in place in drm_pci.c as it
> has been there before my device-registration changes. However, with
> your series you added the pci_set_drvdata() everywhere yourself, so
> yes, please remove it.

That was a bogus hunk in v1 of this patch, which iirc I've never
posted onto the list anywhere. I added a platfrom_set_drvdata call,
but with the previous series to make sure that each driver has that
it's a bit redundant.

Long term, when we split up the drm init code I think the drvdata
assignment should be the driver's job.

I'll fix the fumble with the list_head init, thanks for spotting that.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm: restrict the device list for shadow attached drivers

[Daniel Vetter]

There's really no need for the drm core to keep a list of all
devices of a given driver - the linux device model keeps perfect
track of this already for us.

The exception is old legacy ums drivers using pci shadow attaching.
So rename the lists to make the use case clearer and rip out everything
else.

v2: Rebase on top of David Herrmann's drm device register changes.
Also drop the bogus dev_set_drvdata for platform drivers that somehow
crept into the original version - drivers really should be in full
control of that field.

v3: Initialize driver->legacy_dev_list outside of the loop, spotted by
David Herrmann.

Cc: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_pci.c      | 12 ++++++++++--
 drivers/gpu/drm/drm_platform.c |  1 -
 drivers/gpu/drm/drm_stub.c     |  4 ----
 drivers/gpu/drm/drm_usb.c      |  1 -
 include/drm/drmP.h             |  6 +++---
 5 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index f00d7a9671ea..e92b9567485b 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -346,6 +346,11 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 		 driver->name, driver->major, driver->minor, driver->patchlevel,
 		 driver->date, pci_name(pdev), dev->primary->index);
 
+	/* No locking needed since shadow-attach is single-threaded since it may
+	 * only be called from the per-driver module init hook. */
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		list_add_tail(&dev->legacy_dev_list, &driver->legacy_dev_list);
+
 	return 0;
 
 err_pci:
@@ -375,7 +380,6 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
 
 	DRM_DEBUG("\n");
 
-	INIT_LIST_HEAD(&driver->device_list);
 	driver->kdriver.pci = pdriver;
 	driver->bus = &drm_pci_bus;
 
@@ -383,6 +387,7 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
 		return pci_register_driver(pdriver);
 
 	/* If not using KMS, fall back to stealth mode manual scanning. */
+	INIT_LIST_HEAD(&driver->legacy_dev_list);
 	for (i = 0; pdriver->id_table[i].vendor != 0; i++) {
 		pid = &pdriver->id_table[i];
 
@@ -465,8 +470,11 @@ void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver)
 	if (driver->driver_features & DRIVER_MODESET) {
 		pci_unregister_driver(pdriver);
 	} else {
-		list_for_each_entry_safe(dev, tmp, &driver->device_list, driver_item)
+		list_for_each_entry_safe(dev, tmp, &driver->legacy_dev_list,
+					 legacy_dev_list) {
 			drm_put_dev(dev);
+			list_del(&dev->legacy_dev_list);
+		}
 	}
 	DRM_INFO("Module unloaded\n");
 }
diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
index 56a48033eced..21fc82006b78 100644
--- a/drivers/gpu/drm/drm_platform.c
+++ b/drivers/gpu/drm/drm_platform.c
@@ -147,7 +147,6 @@ int drm_platform_init(struct drm_driver *driver, struct platform_device *platfor
 
 	driver->kdriver.platform_device = platform_device;
 	driver->bus = &drm_platform_bus;
-	INIT_LIST_HEAD(&driver->device_list);
 	return drm_get_platform_dev(platform_device, driver);
 }
 EXPORT_SYMBOL(drm_platform_init);
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 26055abf94ee..047d1d1fb0e1 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -538,8 +538,6 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 			goto err_unload;
 	}
 
-	list_add_tail(&dev->driver_item, &dev->driver->device_list);
-
 	ret = 0;
 	goto out_unlock;
 
@@ -593,7 +591,5 @@ void drm_dev_unregister(struct drm_device *dev)
 	if (dev->render)
 		drm_put_minor(&dev->render);
 	drm_put_minor(&dev->primary);
-
-	list_del(&dev->driver_item);
 }
 EXPORT_SYMBOL(drm_dev_unregister);
diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c
index b179b70e7853..21ae8d96880b 100644
--- a/drivers/gpu/drm/drm_usb.c
+++ b/drivers/gpu/drm/drm_usb.c
@@ -63,7 +63,6 @@ int drm_usb_init(struct drm_driver *driver, struct usb_driver *udriver)
 	int res;
 	DRM_DEBUG("\n");
 
-	INIT_LIST_HEAD(&driver->device_list);
 	driver->kdriver.usb = udriver;
 	driver->bus = &drm_usb_bus;
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 54d6c2d36a5b..e3dfbae963a5 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -988,8 +988,8 @@ struct drm_driver {
 	} kdriver;
 	struct drm_bus *bus;
 
-	/* List of devices hanging off this driver */
-	struct list_head device_list;
+	/* List of devices hanging off this driver with stealth attach. */
+	struct list_head legacy_dev_list;
 };
 
 #define DRM_MINOR_UNASSIGNED 0
@@ -1099,7 +1099,7 @@ struct drm_vblank_crtc {
  * may contain multiple heads.
  */
 struct drm_device {
-	struct list_head driver_item;	/**< list of devices per driver */
+	struct list_head legacy_dev_list;/**< list of devices per driver for stealth attach cleanup */
 	char *devname;			/**< For /proc/interrupts */
 	int if_version;			/**< Highest interface version set */
 
-- 
1.8.4.rc3

Re: [PATCH 11/19] drm: restrict the device list for shadow attached drivers

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1845 bytes --]

On Sun, Nov 03, 2013 at 03:14:16PM +0100, Daniel Vetter wrote:
> On Sun, Nov 3, 2013 at 3:05 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> > On Sun, Nov 3, 2013 at 2:31 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >> There's really no need for the drm core to keep a list of all
> >> devices of a given driver - the linux device model keeps perfect
> >> track of this already for us.
> >>
> >> The exception is old legacy ums drivers using pci shadow attaching.
> >> So rename the lists to make the use case clearer and rip out everything
> >> else.
> >>
> >> v2: Rebase on top of David Herrmann's drm device register changes.
> >> Also drop the bogus dev_set_drvdata for platform drivers that somehow
> >> crept into the original version - drivers really should be in full
> >> control of that field.
> >
> > You didn't really change any dev_set_drvdata, did you? And I guess you
> > mean pci_set_drvdata()? I had to keep it in place in drm_pci.c as it
> > has been there before my device-registration changes. However, with
> > your series you added the pci_set_drvdata() everywhere yourself, so
> > yes, please remove it.
> 
> That was a bogus hunk in v1 of this patch, which iirc I've never
> posted onto the list anywhere. I added a platfrom_set_drvdata call,
> but with the previous series to make sure that each driver has that
> it's a bit redundant.
> 
> Long term, when we split up the drm init code I think the drvdata
> assignment should be the driver's job.

I remember submitting a patch for that a while ago. It was applied about
a year ago, see commit a16d4f86019a ('drm: platform: Don't initialize
driver-private data'). The issue at the time was that I needed the
drvdata for other purposes and drm_platform_init() kept overwriting it,
which had me confused for days.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 12/19] drm/bufs: remove handling of _DRM_GEM mappings

[Daniel Vetter]

Gone with the new gem vma offset manager from David.

We can also ditch the uapi header definition from the enum since
userspace never used this. It ended up in there purely for historical
reasons (for reusing the old drm mmap code essentially), not because
userspace ever needed it.

Cc: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_bufs.c | 6 ------
 drivers/gpu/drm/drm_vm.c   | 3 ---
 include/uapi/drm/drm.h     | 1 -
 3 files changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index 471e051d295e..766a5474fdbd 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -303,9 +303,6 @@ static int drm_addmap_core(struct drm_device * dev, resource_size_t offset,
 
 		break;
 	}
-	case _DRM_GEM:
-		DRM_ERROR("tried to addmap GEM object\n");
-		break;
 	case _DRM_SCATTER_GATHER:
 		if (!dev->sg) {
 			kfree(map);
@@ -483,9 +480,6 @@ int drm_rmmap_locked(struct drm_device *dev, struct drm_local_map *map)
 		dmah.size = map->size;
 		__drm_pci_free(dev, &dmah);
 		break;
-	case _DRM_GEM:
-		DRM_ERROR("tried to rmmap GEM object\n");
-		break;
 	}
 	kfree(map);
 
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index b5c5af7328df..efd424da7be6 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -266,9 +266,6 @@ static void drm_vm_shm_close(struct vm_area_struct *vma)
 				dmah.size = map->size;
 				__drm_pci_free(dev, &dmah);
 				break;
-			case _DRM_GEM:
-				DRM_ERROR("tried to rmmap GEM object\n");
-				break;
 			}
 			kfree(map);
 		}
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 9b24d65fed72..3c9a833992e8 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -181,7 +181,6 @@ enum drm_map_type {
 	_DRM_AGP = 3,		  /**< AGP/GART */
 	_DRM_SCATTER_GATHER = 4,  /**< Scatter/gather memory for PCI DMA */
 	_DRM_CONSISTENT = 5,	  /**< Consistent memory for PCI DMA */
-	_DRM_GEM = 6,		  /**< GEM object (obsolete) */
 };
 
 /**
-- 
1.8.4.rc3

Re: [PATCH 12/19] drm/bufs: remove handling of _DRM_GEM mappings

[David Herrmann]

Hi Daniel

On Sun, Nov 3, 2013 at 2:31 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Gone with the new gem vma offset manager from David.
>
> We can also ditch the uapi header definition from the enum since
> userspace never used this. It ended up in there purely for historical
> reasons (for reusing the old drm mmap code essentially), not because
> userspace ever needed it.
>
> Cc: David Herrmann <dh.herrmann@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks for removing it. I haven't found any code using _DRM_GEM when
changing the vma-manager either, so:

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

> ---
>  drivers/gpu/drm/drm_bufs.c | 6 ------
>  drivers/gpu/drm/drm_vm.c   | 3 ---
>  include/uapi/drm/drm.h     | 1 -
>  3 files changed, 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
> index 471e051d295e..766a5474fdbd 100644
> --- a/drivers/gpu/drm/drm_bufs.c
> +++ b/drivers/gpu/drm/drm_bufs.c
> @@ -303,9 +303,6 @@ static int drm_addmap_core(struct drm_device * dev, resource_size_t offset,
>
>                 break;
>         }
> -       case _DRM_GEM:
> -               DRM_ERROR("tried to addmap GEM object\n");
> -               break;
>         case _DRM_SCATTER_GATHER:
>                 if (!dev->sg) {
>                         kfree(map);
> @@ -483,9 +480,6 @@ int drm_rmmap_locked(struct drm_device *dev, struct drm_local_map *map)
>                 dmah.size = map->size;
>                 __drm_pci_free(dev, &dmah);
>                 break;
> -       case _DRM_GEM:
> -               DRM_ERROR("tried to rmmap GEM object\n");
> -               break;
>         }
>         kfree(map);
>
> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
> index b5c5af7328df..efd424da7be6 100644
> --- a/drivers/gpu/drm/drm_vm.c
> +++ b/drivers/gpu/drm/drm_vm.c
> @@ -266,9 +266,6 @@ static void drm_vm_shm_close(struct vm_area_struct *vma)
>                                 dmah.size = map->size;
>                                 __drm_pci_free(dev, &dmah);
>                                 break;
> -                       case _DRM_GEM:
> -                               DRM_ERROR("tried to rmmap GEM object\n");
> -                               break;
>                         }
>                         kfree(map);
>                 }
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 9b24d65fed72..3c9a833992e8 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -181,7 +181,6 @@ enum drm_map_type {
>         _DRM_AGP = 3,             /**< AGP/GART */
>         _DRM_SCATTER_GATHER = 4,  /**< Scatter/gather memory for PCI DMA */
>         _DRM_CONSISTENT = 5,      /**< Consistent memory for PCI DMA */
> -       _DRM_GEM = 6,             /**< GEM object (obsolete) */
>  };
>
>  /**
> --
> 1.8.4.rc3
>

[PATCH 13/19] drm: kill DRIVER_REQUIRE_AGP

[Daniel Vetter]

Only the two intel drivers need this and they can easily check for
working agp support in their driver ->load callbacks.

This is the only reason why agp initialization could fail, so allows
us to rip out a bit of error handling code in the next patch.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_pci.c       | 5 -----
 drivers/gpu/drm/i810/i810_dma.c | 4 ++++
 drivers/gpu/drm/i810/i810_drv.c | 2 +-
 drivers/gpu/drm/i915/i915_dma.c | 5 +++++
 drivers/gpu/drm/i915/i915_drv.c | 5 ++---
 include/drm/drmP.h              | 1 -
 6 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index ec78fc805ed0..2b0d7deb4b6d 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -267,11 +267,6 @@ static int drm_pci_agp_init(struct drm_device *dev)
 	if (drm_core_has_AGP(dev)) {
 		if (drm_pci_device_is_agp(dev))
 			dev->agp = drm_agp_init(dev);
-		if (drm_core_check_feature(dev, DRIVER_REQUIRE_AGP)
-		    && (dev->agp == NULL)) {
-			DRM_ERROR("Cannot initialize the agpgart module.\n");
-			return -EINVAL;
-		}
 		if (dev->agp) {
 			dev->agp->agp_mtrr = arch_phys_wc_add(
 				dev->agp->agp_info.aper_base,
diff --git a/drivers/gpu/drm/i810/i810_dma.c b/drivers/gpu/drm/i810/i810_dma.c
index 249fdff305c6..aeace37415aa 100644
--- a/drivers/gpu/drm/i810/i810_dma.c
+++ b/drivers/gpu/drm/i810/i810_dma.c
@@ -1193,6 +1193,10 @@ static int i810_flip_bufs(struct drm_device *dev, void *data,
 
 int i810_driver_load(struct drm_device *dev, unsigned long flags)
 {
+	/* Our userspace depends upon the agp mapping support. */
+	if (!dev->agp)
+		return -EINVAL;
+
 	pci_set_master(dev->pdev);
 
 	return 0;
diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c
index d8180d22cedd..441ccf8f5bdc 100644
--- a/drivers/gpu/drm/i810/i810_drv.c
+++ b/drivers/gpu/drm/i810/i810_drv.c
@@ -57,7 +57,7 @@ static const struct file_operations i810_driver_fops = {
 
 static struct drm_driver driver = {
 	.driver_features =
-	    DRIVER_USE_AGP | DRIVER_REQUIRE_AGP |
+	    DRIVER_USE_AGP |
 	    DRIVER_HAVE_DMA,
 	.dev_priv_size = sizeof(drm_i810_buf_priv_t),
 	.load = i810_driver_load,
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 0cab2d045135..1244280946c2 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1476,7 +1476,12 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 		return -ENODEV;
 	}
 
+	/* UMS needs agp support. */
+	if (!drm_core_check_feature(dev, DRIVER_MODESET) && !dev->agp)
+		return -EINVAL;
+
 	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
+
 	if (dev_priv == NULL)
 		return -ENOMEM;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a0804fa1e306..d86c2e1733fd 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -801,8 +801,7 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 * (gasp!) to share buffers between X and the client. Hence we need to
 	 * keep around the fake agp stuff for gen3, even when kms is enabled. */
 	if (intel_info->gen != 3) {
-		driver.driver_features &=
-			~(DRIVER_USE_AGP | DRIVER_REQUIRE_AGP);
+		driver.driver_features &= ~DRIVER_USE_AGP;
 	} else if (!intel_agp_enabled) {
 		DRM_ERROR("drm/i915 can't work without intel_agp module!\n");
 		return -ENODEV;
@@ -914,7 +913,7 @@ static struct drm_driver driver = {
 	 * deal with them for Intel hardware.
 	 */
 	.driver_features =
-	    DRIVER_USE_AGP | DRIVER_REQUIRE_AGP |
+	    DRIVER_USE_AGP |
 	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME |
 	    DRIVER_RENDER,
 	.load = i915_driver_load,
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index e3dfbae963a5..0ae0f58e1988 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -136,7 +136,6 @@ int drm_err(const char *func, const char *format, ...);
 
 /* driver capabilities and requirements mask */
 #define DRIVER_USE_AGP     0x1
-#define DRIVER_REQUIRE_AGP 0x2
 #define DRIVER_PCI_DMA     0x8
 #define DRIVER_SG          0x10
 #define DRIVER_HAVE_DMA    0x20
-- 
1.8.4.rc3

[PATCH 14/19] drm: ->agp_init can't fail

[Daniel Vetter]

Thanks to the removal of REQUIRE_AGP we can use a void return value
and shed a bit of complexity.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_pci.c  | 3 +--
 drivers/gpu/drm/drm_stub.c | 7 ++-----
 include/drm/drmP.h         | 2 +-
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 2b0d7deb4b6d..3bd8b3fc6189 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -262,7 +262,7 @@ static int drm_pci_irq_by_busid(struct drm_device *dev, struct drm_irq_busid *p)
 	return 0;
 }
 
-static int drm_pci_agp_init(struct drm_device *dev)
+static void drm_pci_agp_init(struct drm_device *dev)
 {
 	if (drm_core_has_AGP(dev)) {
 		if (drm_pci_device_is_agp(dev))
@@ -274,7 +274,6 @@ static int drm_pci_agp_init(struct drm_device *dev)
 				1024 * 1024);
 		}
 	}
-	return 0;
 }
 
 static void drm_pci_agp_destroy(struct drm_device *dev)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 047d1d1fb0e1..dea432b2dfcf 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -502,11 +502,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 
 	mutex_lock(&drm_global_mutex);
 
-	if (dev->driver->bus->agp_init) {
-		ret = dev->driver->bus->agp_init(dev);
-		if (ret)
-			goto out_unlock;
-	}
+	if (dev->driver->bus->agp_init)
+		dev->driver->bus->agp_init(dev);
 
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		ret = drm_get_minor(dev, &dev->control, DRM_MINOR_CONTROL);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 0ae0f58e1988..df502c292ef8 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -749,7 +749,7 @@ struct drm_bus {
 			  struct drm_unique *unique);
 	int (*irq_by_busid)(struct drm_device *dev, struct drm_irq_busid *p);
 	/* hooks that are for PCI */
-	int (*agp_init)(struct drm_device *dev);
+	void (*agp_init)(struct drm_device *dev);
 	void (*agp_destroy)(struct drm_device *dev);
 
 };
-- 
1.8.4.rc3

Re: [PATCH 14/19] drm: ->agp_init can't fail

[David Herrmann]

Hi Daniel

On Sun, Nov 3, 2013 at 2:31 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Thanks to the removal of REQUIRE_AGP we can use a void return value
> and shed a bit of complexity.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_pci.c  | 3 +--
>  drivers/gpu/drm/drm_stub.c | 7 ++-----
>  include/drm/drmP.h         | 2 +-
>  3 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 2b0d7deb4b6d..3bd8b3fc6189 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -262,7 +262,7 @@ static int drm_pci_irq_by_busid(struct drm_device *dev, struct drm_irq_busid *p)
>         return 0;
>  }
>
> -static int drm_pci_agp_init(struct drm_device *dev)
> +static void drm_pci_agp_init(struct drm_device *dev)
>  {
>         if (drm_core_has_AGP(dev)) {
>                 if (drm_pci_device_is_agp(dev))
> @@ -274,7 +274,6 @@ static int drm_pci_agp_init(struct drm_device *dev)
>                                 1024 * 1024);
>                 }
>         }
> -       return 0;
>  }
>
>  static void drm_pci_agp_destroy(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 047d1d1fb0e1..dea432b2dfcf 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -502,11 +502,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>
>         mutex_lock(&drm_global_mutex);
>
> -       if (dev->driver->bus->agp_init) {
> -               ret = dev->driver->bus->agp_init(dev);
> -               if (ret)
> -                       goto out_unlock;
> -       }
> +       if (dev->driver->bus->agp_init)
> +               dev->driver->bus->agp_init(dev);

Ok, technically, you need to remove "out_unlock" here. But patch #16
removes err_agp and reused out_unlock, so I think this is fine.

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

>
>         if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>                 ret = drm_get_minor(dev, &dev->control, DRM_MINOR_CONTROL);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 0ae0f58e1988..df502c292ef8 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -749,7 +749,7 @@ struct drm_bus {
>                           struct drm_unique *unique);
>         int (*irq_by_busid)(struct drm_device *dev, struct drm_irq_busid *p);
>         /* hooks that are for PCI */
> -       int (*agp_init)(struct drm_device *dev);
> +       void (*agp_init)(struct drm_device *dev);
>         void (*agp_destroy)(struct drm_device *dev);
>
>  };
> --
> 1.8.4.rc3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 15/19] drm: rip out drm_core_has_AGP

[Daniel Vetter]

Most place actually want to just check for dev->agp (most do, but a
few don't so this fixes a few potential NULL derefs). The only
exception is the agp init code which should check for the AGP driver
feature flag.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_agpsupport.c    |  2 +-
 drivers/gpu/drm/drm_bufs.c          |  4 ++--
 drivers/gpu/drm/drm_memory.c        |  9 +++------
 drivers/gpu/drm/drm_pci.c           |  4 ++--
 drivers/gpu/drm/drm_vm.c            |  4 ++--
 drivers/gpu/drm/radeon/radeon_ttm.c |  2 +-
 include/drm/drm_agpsupport.h        | 12 ------------
 7 files changed, 11 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_agpsupport.c b/drivers/gpu/drm/drm_agpsupport.c
index e301d653d97e..084a674e4b56 100644
--- a/drivers/gpu/drm/drm_agpsupport.c
+++ b/drivers/gpu/drm/drm_agpsupport.c
@@ -439,7 +439,7 @@ void drm_agp_clear(struct drm_device *dev)
 {
 	struct drm_agp_mem *entry, *tempe;
 
-	if (!drm_core_has_AGP(dev) || !dev->agp)
+	if (!dev->agp)
 		return;
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		return;
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index 766a5474fdbd..edec31fe3fed 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -261,7 +261,7 @@ static int drm_addmap_core(struct drm_device * dev, resource_size_t offset,
 		struct drm_agp_mem *entry;
 		int valid = 0;
 
-		if (!drm_core_has_AGP(dev)) {
+		if (!dev->agp) {
 			kfree(map);
 			return -EINVAL;
 		}
@@ -1390,7 +1390,7 @@ int drm_mapbufs(struct drm_device *dev, void *data,
 	spin_unlock(&dev->count_lock);
 
 	if (request->count >= dma->buf_count) {
-		if ((drm_core_has_AGP(dev) && (dma->flags & _DRM_DMA_USE_AGP))
+		if ((dev->agp && (dma->flags & _DRM_DMA_USE_AGP))
 		    || (drm_core_check_feature(dev, DRIVER_SG)
 			&& (dma->flags & _DRM_DMA_USE_SG))) {
 			struct drm_local_map *map = dev->agp_buffer_map;
diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
index 64e44fad8ae8..3359bc453e11 100644
--- a/drivers/gpu/drm/drm_memory.c
+++ b/drivers/gpu/drm/drm_memory.c
@@ -110,8 +110,7 @@ static inline void *agp_remap(unsigned long offset, unsigned long size,
 
 void drm_core_ioremap(struct drm_local_map *map, struct drm_device *dev)
 {
-	if (drm_core_has_AGP(dev) &&
-	    dev->agp && dev->agp->cant_use_aperture && map->type == _DRM_AGP)
+	if (dev->agp && dev->agp->cant_use_aperture && map->type == _DRM_AGP)
 		map->handle = agp_remap(map->offset, map->size, dev);
 	else
 		map->handle = ioremap(map->offset, map->size);
@@ -120,8 +119,7 @@ EXPORT_SYMBOL(drm_core_ioremap);
 
 void drm_core_ioremap_wc(struct drm_local_map *map, struct drm_device *dev)
 {
-	if (drm_core_has_AGP(dev) &&
-	    dev->agp && dev->agp->cant_use_aperture && map->type == _DRM_AGP)
+	if (dev->agp && dev->agp->cant_use_aperture && map->type == _DRM_AGP)
 		map->handle = agp_remap(map->offset, map->size, dev);
 	else
 		map->handle = ioremap_wc(map->offset, map->size);
@@ -133,8 +131,7 @@ void drm_core_ioremapfree(struct drm_local_map *map, struct drm_device *dev)
 	if (!map->handle || !map->size)
 		return;
 
-	if (drm_core_has_AGP(dev) &&
-	    dev->agp && dev->agp->cant_use_aperture && map->type == _DRM_AGP)
+	if (dev->agp && dev->agp->cant_use_aperture && map->type == _DRM_AGP)
 		vunmap(map->handle);
 	else
 		iounmap(map->handle);
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 3bd8b3fc6189..4ec43b8d42f5 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -264,7 +264,7 @@ static int drm_pci_irq_by_busid(struct drm_device *dev, struct drm_irq_busid *p)
 
 static void drm_pci_agp_init(struct drm_device *dev)
 {
-	if (drm_core_has_AGP(dev)) {
+	if (drm_core_check_feature(dev, DRIVER_USE_AGP)) {
 		if (drm_pci_device_is_agp(dev))
 			dev->agp = drm_agp_init(dev);
 		if (dev->agp) {
@@ -278,7 +278,7 @@ static void drm_pci_agp_init(struct drm_device *dev)
 
 static void drm_pci_agp_destroy(struct drm_device *dev)
 {
-	if (drm_core_has_AGP(dev) && dev->agp) {
+	if (dev->agp) {
 		arch_phys_wc_del(dev->agp->agp_mtrr);
 		drm_agp_clear(dev);
 		drm_agp_destroy(dev->agp);
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index efd424da7be6..008c8c0e221f 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -101,7 +101,7 @@ static int drm_do_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	/*
 	 * Find the right map
 	 */
-	if (!drm_core_has_AGP(dev))
+	if (!dev->agp)
 		goto vm_fault_error;
 
 	if (!dev->agp || !dev->agp->cant_use_aperture)
@@ -592,7 +592,7 @@ int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma)
 	switch (map->type) {
 #if !defined(__arm__)
 	case _DRM_AGP:
-		if (drm_core_has_AGP(dev) && dev->agp->cant_use_aperture) {
+		if (dev->agp && dev->agp->cant_use_aperture) {
 			/*
 			 * On some platforms we can't talk to bus dma address from the CPU, so for
 			 * memory of type DRM_AGP, we'll deal with sorting out the real physical
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 71245d6f34a2..051fa874065a 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -142,7 +142,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
 #if __OS_HAS_AGP
 		if (rdev->flags & RADEON_IS_AGP) {
-			if (!(drm_core_has_AGP(rdev->ddev) && rdev->ddev->agp)) {
+			if (!rdev->ddev->agp) {
 				DRM_ERROR("AGP is not enabled for memory type %u\n",
 					  (unsigned)type);
 				return -EINVAL;
diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h
index a184eeee9c96..a12b0e011e44 100644
--- a/include/drm/drm_agpsupport.h
+++ b/include/drm/drm_agpsupport.h
@@ -46,12 +46,6 @@ int drm_agp_unbind_ioctl(struct drm_device *dev, void *data,
 int drm_agp_bind(struct drm_device *dev, struct drm_agp_binding *request);
 int drm_agp_bind_ioctl(struct drm_device *dev, void *data,
 		       struct drm_file *file_priv);
-
-static inline int drm_core_has_AGP(struct drm_device *dev)
-{
-	return drm_core_check_feature(dev, DRIVER_USE_AGP);
-}
-
 #else /* __OS_HAS_AGP */
 
 static inline void drm_free_agp(DRM_AGP_MEM * handle, int pages)
@@ -183,12 +177,6 @@ static inline int drm_agp_bind_ioctl(struct drm_device *dev, void *data,
 {
 	return -ENODEV;
 }
-
-static inline int drm_core_has_AGP(struct drm_device *dev)
-{
-	return 0;
-}
-
 #endif /* __OS_HAS_AGP */
 
 #endif /* _DRM_AGPSUPPORT_H_ */
-- 
1.8.4.rc3

[PATCH 16/19] drm: remove agp_init() bus callback

[Daniel Vetter]

From: David Herrmann <dh.herrmann@gmail.com>

The PCI bus helper is the only user of it. Call it directly before
device-registration to get rid of the callback.

Note that all drm_agp_*() calls are locked with the drm-global-mutex so we
need to explicitly lock it during initialization. It's not really clear
why it's needed, but lets be safe.

v2: Rebase on top of the agp_init interface change.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_pci.c  | 13 +++++++++----
 drivers/gpu/drm/drm_stub.c |  8 +-------
 include/drm/drmP.h         |  1 -
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 4ec43b8d42f5..54eae6d83e5d 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -292,8 +292,6 @@ static struct drm_bus drm_pci_bus = {
 	.get_name = drm_pci_get_name,
 	.set_busid = drm_pci_set_busid,
 	.set_unique = drm_pci_set_unique,
-	.irq_by_busid = drm_pci_irq_by_busid,
-	.agp_init = drm_pci_agp_init,
 	.agp_destroy = drm_pci_agp_destroy,
 };
 
@@ -332,9 +330,13 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		pci_set_drvdata(pdev, dev);
 
+	mutex_lock(&drm_global_mutex);
+	drm_pci_agp_init(dev);
+	mutex_unlock(&drm_global_mutex);
+
 	ret = drm_dev_register(dev, ent->driver_data);
 	if (ret)
-		goto err_pci;
+		goto err_agp;
 
 	DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
 		 driver->name, driver->major, driver->minor, driver->patchlevel,
@@ -347,7 +349,10 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 
 	return 0;
 
-err_pci:
+err_agp:
+	mutex_lock(&drm_global_mutex);
+	drm_pci_agp_destroy(dev);
+	mutex_unlock(&drm_global_mutex);
 	pci_disable_device(pdev);
 err_free:
 	drm_dev_free(dev);
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index dea432b2dfcf..4b25f693ae89 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -502,13 +502,10 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 
 	mutex_lock(&drm_global_mutex);
 
-	if (dev->driver->bus->agp_init)
-		dev->driver->bus->agp_init(dev);
-
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		ret = drm_get_minor(dev, &dev->control, DRM_MINOR_CONTROL);
 		if (ret)
-			goto err_agp;
+			goto out_unlock;
 	}
 
 	if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) {
@@ -549,9 +546,6 @@ err_render_node:
 err_control_node:
 	if (dev->control)
 		drm_put_minor(&dev->control);
-err_agp:
-	if (dev->driver->bus->agp_destroy)
-		dev->driver->bus->agp_destroy(dev);
 out_unlock:
 	mutex_unlock(&drm_global_mutex);
 	return ret;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index df502c292ef8..8cf8cfef8c56 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -749,7 +749,6 @@ struct drm_bus {
 			  struct drm_unique *unique);
 	int (*irq_by_busid)(struct drm_device *dev, struct drm_irq_busid *p);
 	/* hooks that are for PCI */
-	void (*agp_init)(struct drm_device *dev);
 	void (*agp_destroy)(struct drm_device *dev);
 
 };
-- 
1.8.4.rc3

Re: [PATCH 16/19] drm: remove agp_init() bus callback

[David Herrmann]

Hi Daniel

On Sun, Nov 3, 2013 at 2:31 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> From: David Herrmann <dh.herrmann@gmail.com>
>
> The PCI bus helper is the only user of it. Call it directly before
> device-registration to get rid of the callback.
>
> Note that all drm_agp_*() calls are locked with the drm-global-mutex so we
> need to explicitly lock it during initialization. It's not really clear
> why it's needed, but lets be safe.
>
> v2: Rebase on top of the agp_init interface change.
>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com> (v1)
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_pci.c  | 13 +++++++++----
>  drivers/gpu/drm/drm_stub.c |  8 +-------
>  include/drm/drmP.h         |  1 -
>  3 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 4ec43b8d42f5..54eae6d83e5d 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -292,8 +292,6 @@ static struct drm_bus drm_pci_bus = {
>         .get_name = drm_pci_get_name,
>         .set_busid = drm_pci_set_busid,
>         .set_unique = drm_pci_set_unique,
> -       .irq_by_busid = drm_pci_irq_by_busid,

Err, where does that come from?

Cheers
David

> -       .agp_init = drm_pci_agp_init,
>         .agp_destroy = drm_pci_agp_destroy,
>  };
>
> @@ -332,9 +330,13 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
>         if (drm_core_check_feature(dev, DRIVER_MODESET))
>                 pci_set_drvdata(pdev, dev);
>
> +       mutex_lock(&drm_global_mutex);
> +       drm_pci_agp_init(dev);
> +       mutex_unlock(&drm_global_mutex);
> +
>         ret = drm_dev_register(dev, ent->driver_data);
>         if (ret)
> -               goto err_pci;
> +               goto err_agp;
>
>         DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
>                  driver->name, driver->major, driver->minor, driver->patchlevel,
> @@ -347,7 +349,10 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
>
>         return 0;
>
> -err_pci:
> +err_agp:
> +       mutex_lock(&drm_global_mutex);
> +       drm_pci_agp_destroy(dev);
> +       mutex_unlock(&drm_global_mutex);
>         pci_disable_device(pdev);
>  err_free:
>         drm_dev_free(dev);
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index dea432b2dfcf..4b25f693ae89 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -502,13 +502,10 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>
>         mutex_lock(&drm_global_mutex);
>
> -       if (dev->driver->bus->agp_init)
> -               dev->driver->bus->agp_init(dev);
> -
>         if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>                 ret = drm_get_minor(dev, &dev->control, DRM_MINOR_CONTROL);
>                 if (ret)
> -                       goto err_agp;
> +                       goto out_unlock;
>         }
>
>         if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) {
> @@ -549,9 +546,6 @@ err_render_node:
>  err_control_node:
>         if (dev->control)
>                 drm_put_minor(&dev->control);
> -err_agp:
> -       if (dev->driver->bus->agp_destroy)
> -               dev->driver->bus->agp_destroy(dev);
>  out_unlock:
>         mutex_unlock(&drm_global_mutex);
>         return ret;
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index df502c292ef8..8cf8cfef8c56 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -749,7 +749,6 @@ struct drm_bus {
>                           struct drm_unique *unique);
>         int (*irq_by_busid)(struct drm_device *dev, struct drm_irq_busid *p);
>         /* hooks that are for PCI */
> -       void (*agp_init)(struct drm_device *dev);
>         void (*agp_destroy)(struct drm_device *dev);
>
>  };
> --
> 1.8.4.rc3
>

Re: [PATCH 16/19] drm: remove agp_init() bus callback

[Daniel Vetter]

On Sun, Nov 3, 2013 at 3:14 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
>> index 4ec43b8d42f5..54eae6d83e5d 100644
>> --- a/drivers/gpu/drm/drm_pci.c
>> +++ b/drivers/gpu/drm/drm_pci.c
>> @@ -292,8 +292,6 @@ static struct drm_bus drm_pci_bus = {
>>         .get_name = drm_pci_get_name,
>>         .set_busid = drm_pci_set_busid,
>>         .set_unique = drm_pci_set_unique,
>> -       .irq_by_busid = drm_pci_irq_by_busid,
>
> Err, where does that come from?

Pretty bad case of rebase fail - it's all ripped out in the in-flux
irq rework I have here on top of these patches. Thanks for spotting
this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm: remove agp_init() bus callback

[Daniel Vetter]

The PCI bus helper is the only user of it. Call it directly before
device-registration to get rid of the callback.

Note that all drm_agp_*() calls are locked with the drm-global-mutex so we
need to explicitly lock it during initialization. It's not really clear
why it's needed, but lets be safe.

v2: Rebase on top of the agp_init interface change.

v3: Remove the rebase-fail where I've accidentally killed the ->irq_by_busid
callback a bit too early.

Cc: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_pci.c  | 12 +++++++++---
 drivers/gpu/drm/drm_stub.c |  8 +-------
 include/drm/drmP.h         |  1 -
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 4ec43b8d42f5..02688858785b 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -293,7 +293,6 @@ static struct drm_bus drm_pci_bus = {
 	.set_busid = drm_pci_set_busid,
 	.set_unique = drm_pci_set_unique,
 	.irq_by_busid = drm_pci_irq_by_busid,
-	.agp_init = drm_pci_agp_init,
 	.agp_destroy = drm_pci_agp_destroy,
 };
 
@@ -332,9 +331,13 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		pci_set_drvdata(pdev, dev);
 
+	mutex_lock(&drm_global_mutex);
+	drm_pci_agp_init(dev);
+	mutex_unlock(&drm_global_mutex);
+
 	ret = drm_dev_register(dev, ent->driver_data);
 	if (ret)
-		goto err_pci;
+		goto err_agp;
 
 	DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
 		 driver->name, driver->major, driver->minor, driver->patchlevel,
@@ -347,7 +350,10 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 
 	return 0;
 
-err_pci:
+err_agp:
+	mutex_lock(&drm_global_mutex);
+	drm_pci_agp_destroy(dev);
+	mutex_unlock(&drm_global_mutex);
 	pci_disable_device(pdev);
 err_free:
 	drm_dev_free(dev);
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index dea432b2dfcf..4b25f693ae89 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -502,13 +502,10 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 
 	mutex_lock(&drm_global_mutex);
 
-	if (dev->driver->bus->agp_init)
-		dev->driver->bus->agp_init(dev);
-
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		ret = drm_get_minor(dev, &dev->control, DRM_MINOR_CONTROL);
 		if (ret)
-			goto err_agp;
+			goto out_unlock;
 	}
 
 	if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) {
@@ -549,9 +546,6 @@ err_render_node:
 err_control_node:
 	if (dev->control)
 		drm_put_minor(&dev->control);
-err_agp:
-	if (dev->driver->bus->agp_destroy)
-		dev->driver->bus->agp_destroy(dev);
 out_unlock:
 	mutex_unlock(&drm_global_mutex);
 	return ret;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index df502c292ef8..8cf8cfef8c56 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -749,7 +749,6 @@ struct drm_bus {
 			  struct drm_unique *unique);
 	int (*irq_by_busid)(struct drm_device *dev, struct drm_irq_busid *p);
 	/* hooks that are for PCI */
-	void (*agp_init)(struct drm_device *dev);
 	void (*agp_destroy)(struct drm_device *dev);
 
 };
-- 
1.8.4.rc3

[PATCH 17/19] drm: inline drm_agp_destroy

[Daniel Vetter]

Wrapping a kfree is pointless.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_agpsupport.c | 15 ---------------
 drivers/gpu/drm/drm_pci.c        |  2 +-
 include/drm/drm_agpsupport.h     |  5 -----
 3 files changed, 1 insertion(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_agpsupport.c b/drivers/gpu/drm/drm_agpsupport.c
index 084a674e4b56..c5f663bb8bc1 100644
--- a/drivers/gpu/drm/drm_agpsupport.c
+++ b/drivers/gpu/drm/drm_agpsupport.c
@@ -460,21 +460,6 @@ void drm_agp_clear(struct drm_device *dev)
 }
 
 /**
- * drm_agp_destroy - Destroy AGP head
- * @dev: DRM device
- *
- * Destroy resources that were previously allocated via drm_agp_initp. Caller
- * must ensure to clean up all AGP resources before calling this. See
- * drm_agp_clear().
- *
- * Call this to destroy AGP heads allocated via drm_agp_init().
- */
-void drm_agp_destroy(struct drm_agp_head *agp)
-{
-	kfree(agp);
-}
-
-/**
  * Binds a collection of pages into AGP memory at the given offset, returning
  * the AGP memory structure containing them.
  *
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 54eae6d83e5d..d0937dd62899 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -281,7 +281,7 @@ static void drm_pci_agp_destroy(struct drm_device *dev)
 	if (dev->agp) {
 		arch_phys_wc_del(dev->agp->agp_mtrr);
 		drm_agp_clear(dev);
-		drm_agp_destroy(dev->agp);
+		kfree(dev->agp);
 		dev->agp = NULL;
 	}
 }
diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h
index a12b0e011e44..56a861b2ceaa 100644
--- a/include/drm/drm_agpsupport.h
+++ b/include/drm/drm_agpsupport.h
@@ -20,7 +20,6 @@ DRM_AGP_MEM *drm_agp_bind_pages(struct drm_device *dev,
 				uint32_t type);
 
 struct drm_agp_head *drm_agp_init(struct drm_device *dev);
-void drm_agp_destroy(struct drm_agp_head *agp);
 void drm_agp_clear(struct drm_device *dev);
 int drm_agp_acquire(struct drm_device *dev);
 int drm_agp_acquire_ioctl(struct drm_device *dev, void *data,
@@ -76,10 +75,6 @@ static inline struct drm_agp_head *drm_agp_init(struct drm_device *dev)
 	return NULL;
 }
 
-static inline void drm_agp_destroy(struct drm_agp_head *agp)
-{
-}
-
 static inline void drm_agp_clear(struct drm_device *dev)
 {
 }
-- 
1.8.4.rc3

Re: [PATCH 17/19] drm: inline drm_agp_destroy

[David Herrmann]

Hi

On Sun, Nov 3, 2013 at 2:31 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Wrapping a kfree is pointless.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---

I added that one dutifully to document the API in:

commit 28ec711cd427f8b61f73712a43b8100ba8ca933b
Author: David Herrmann <dh.herrmann@gmail.com>
Date:   Sat Jul 27 16:37:00 2013 +0200

    drm/agp: move AGP cleanup paths to drm_agpsupport.c

By removing it, we loose the comment on how to clean it up. Maybe we
can copy the comment to drm_agp_init()? Other than that, I'm fine with
it:

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

>  drivers/gpu/drm/drm_agpsupport.c | 15 ---------------
>  drivers/gpu/drm/drm_pci.c        |  2 +-
>  include/drm/drm_agpsupport.h     |  5 -----
>  3 files changed, 1 insertion(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_agpsupport.c b/drivers/gpu/drm/drm_agpsupport.c
> index 084a674e4b56..c5f663bb8bc1 100644
> --- a/drivers/gpu/drm/drm_agpsupport.c
> +++ b/drivers/gpu/drm/drm_agpsupport.c
> @@ -460,21 +460,6 @@ void drm_agp_clear(struct drm_device *dev)
>  }
>
>  /**
> - * drm_agp_destroy - Destroy AGP head
> - * @dev: DRM device
> - *
> - * Destroy resources that were previously allocated via drm_agp_initp. Caller
> - * must ensure to clean up all AGP resources before calling this. See
> - * drm_agp_clear().
> - *
> - * Call this to destroy AGP heads allocated via drm_agp_init().
> - */
> -void drm_agp_destroy(struct drm_agp_head *agp)
> -{
> -       kfree(agp);
> -}
> -
> -/**
>   * Binds a collection of pages into AGP memory at the given offset, returning
>   * the AGP memory structure containing them.
>   *
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 54eae6d83e5d..d0937dd62899 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -281,7 +281,7 @@ static void drm_pci_agp_destroy(struct drm_device *dev)
>         if (dev->agp) {
>                 arch_phys_wc_del(dev->agp->agp_mtrr);
>                 drm_agp_clear(dev);
> -               drm_agp_destroy(dev->agp);
> +               kfree(dev->agp);
>                 dev->agp = NULL;
>         }
>  }
> diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h
> index a12b0e011e44..56a861b2ceaa 100644
> --- a/include/drm/drm_agpsupport.h
> +++ b/include/drm/drm_agpsupport.h
> @@ -20,7 +20,6 @@ DRM_AGP_MEM *drm_agp_bind_pages(struct drm_device *dev,
>                                 uint32_t type);
>
>  struct drm_agp_head *drm_agp_init(struct drm_device *dev);
> -void drm_agp_destroy(struct drm_agp_head *agp);
>  void drm_agp_clear(struct drm_device *dev);
>  int drm_agp_acquire(struct drm_device *dev);
>  int drm_agp_acquire_ioctl(struct drm_device *dev, void *data,
> @@ -76,10 +75,6 @@ static inline struct drm_agp_head *drm_agp_init(struct drm_device *dev)
>         return NULL;
>  }
>
> -static inline void drm_agp_destroy(struct drm_agp_head *agp)
> -{
> -}
> -
>  static inline void drm_agp_clear(struct drm_device *dev)
>  {
>  }
> --
> 1.8.4.rc3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 17/19] drm: inline drm_agp_destroy

[Daniel Vetter]

On Sun, Nov 03, 2013 at 02:43:44PM +0100, David Herrmann wrote:
> Hi
> 
> On Sun, Nov 3, 2013 at 2:31 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Wrapping a kfree is pointless.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> 
> I added that one dutifully to document the API in:
> 
> commit 28ec711cd427f8b61f73712a43b8100ba8ca933b
> Author: David Herrmann <dh.herrmann@gmail.com>
> Date:   Sat Jul 27 16:37:00 2013 +0200
> 
>     drm/agp: move AGP cleanup paths to drm_agpsupport.c
> 
> By removing it, we loose the comment on how to clean it up. Maybe we
> can copy the comment to drm_agp_init()? Other than that, I'm fine with
> it:

Hm, I haven't really noticed your original patch fly by. But I think agp
setup/teardown is neatly fubar anyway, e.g. for modesetting drivers we
seem to leak refcount on the agp backend driver.

Long term (if anyone actually cares) I think we should shovel all the
legacy agp stuff somewhere far away and switch the modesetting drivers to
using the agp system directly and making sure that stuff gets cleaned up
with devres.c.
-Daniel

> 
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> 
> Thanks
> David
> 
> >  drivers/gpu/drm/drm_agpsupport.c | 15 ---------------
> >  drivers/gpu/drm/drm_pci.c        |  2 +-
> >  include/drm/drm_agpsupport.h     |  5 -----
> >  3 files changed, 1 insertion(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_agpsupport.c b/drivers/gpu/drm/drm_agpsupport.c
> > index 084a674e4b56..c5f663bb8bc1 100644
> > --- a/drivers/gpu/drm/drm_agpsupport.c
> > +++ b/drivers/gpu/drm/drm_agpsupport.c
> > @@ -460,21 +460,6 @@ void drm_agp_clear(struct drm_device *dev)
> >  }
> >
> >  /**
> > - * drm_agp_destroy - Destroy AGP head
> > - * @dev: DRM device
> > - *
> > - * Destroy resources that were previously allocated via drm_agp_initp. Caller
> > - * must ensure to clean up all AGP resources before calling this. See
> > - * drm_agp_clear().
> > - *
> > - * Call this to destroy AGP heads allocated via drm_agp_init().
> > - */
> > -void drm_agp_destroy(struct drm_agp_head *agp)
> > -{
> > -       kfree(agp);
> > -}
> > -
> > -/**
> >   * Binds a collection of pages into AGP memory at the given offset, returning
> >   * the AGP memory structure containing them.
> >   *
> > diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> > index 54eae6d83e5d..d0937dd62899 100644
> > --- a/drivers/gpu/drm/drm_pci.c
> > +++ b/drivers/gpu/drm/drm_pci.c
> > @@ -281,7 +281,7 @@ static void drm_pci_agp_destroy(struct drm_device *dev)
> >         if (dev->agp) {
> >                 arch_phys_wc_del(dev->agp->agp_mtrr);
> >                 drm_agp_clear(dev);
> > -               drm_agp_destroy(dev->agp);
> > +               kfree(dev->agp);
> >                 dev->agp = NULL;
> >         }
> >  }
> > diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h
> > index a12b0e011e44..56a861b2ceaa 100644
> > --- a/include/drm/drm_agpsupport.h
> > +++ b/include/drm/drm_agpsupport.h
> > @@ -20,7 +20,6 @@ DRM_AGP_MEM *drm_agp_bind_pages(struct drm_device *dev,
> >                                 uint32_t type);
> >
> >  struct drm_agp_head *drm_agp_init(struct drm_device *dev);
> > -void drm_agp_destroy(struct drm_agp_head *agp);
> >  void drm_agp_clear(struct drm_device *dev);
> >  int drm_agp_acquire(struct drm_device *dev);
> >  int drm_agp_acquire_ioctl(struct drm_device *dev, void *data,
> > @@ -76,10 +75,6 @@ static inline struct drm_agp_head *drm_agp_init(struct drm_device *dev)
> >         return NULL;
> >  }
> >
> > -static inline void drm_agp_destroy(struct drm_agp_head *agp)
> > -{
> > -}
> > -
> >  static inline void drm_agp_clear(struct drm_device *dev)
> >  {
> >  }
> > --
> > 1.8.4.rc3
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 18/19] drm: kill the ->agp_destroy callback

[Daniel Vetter]

Call drm_pci_agp_destroy directly, there's no point in the
indirection. Long term we want to shuffle this into each driver's
unload logic, but that needs cleared-up drm lifetime rules first.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_pci.c  | 3 +--
 drivers/gpu/drm/drm_stub.c | 4 ++--
 include/drm/drmP.h         | 4 +---
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index d0937dd62899..36e7f8e74027 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -276,7 +276,7 @@ static void drm_pci_agp_init(struct drm_device *dev)
 	}
 }
 
-static void drm_pci_agp_destroy(struct drm_device *dev)
+void drm_pci_agp_destroy(struct drm_device *dev)
 {
 	if (dev->agp) {
 		arch_phys_wc_del(dev->agp->agp_mtrr);
@@ -292,7 +292,6 @@ static struct drm_bus drm_pci_bus = {
 	.get_name = drm_pci_get_name,
 	.set_busid = drm_pci_set_busid,
 	.set_unique = drm_pci_set_unique,
-	.agp_destroy = drm_pci_agp_destroy,
 };
 
 /**
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 4b25f693ae89..3ec8d4f9f09a 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -569,8 +569,8 @@ void drm_dev_unregister(struct drm_device *dev)
 	if (dev->driver->unload)
 		dev->driver->unload(dev);
 
-	if (dev->driver->bus->agp_destroy)
-		dev->driver->bus->agp_destroy(dev);
+	if (dev->agp)
+		drm_pci_agp_destroy(dev);
 
 	drm_vblank_cleanup(dev);
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 8cf8cfef8c56..f67104aa7b51 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -748,9 +748,6 @@ struct drm_bus {
 	int (*set_unique)(struct drm_device *dev, struct drm_master *master,
 			  struct drm_unique *unique);
 	int (*irq_by_busid)(struct drm_device *dev, struct drm_irq_busid *p);
-	/* hooks that are for PCI */
-	void (*agp_destroy)(struct drm_device *dev);
-
 };
 
 /**
@@ -1658,6 +1655,7 @@ static __inline__ int drm_pci_device_is_agp(struct drm_device *dev)
 
 	return pci_find_capability(dev->pdev, PCI_CAP_ID_AGP);
 }
+void drm_pci_agp_destroy(struct drm_device *dev);
 
 extern int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver);
 extern void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver);
-- 
1.8.4.rc3

Re: [PATCH 18/19] drm: kill the ->agp_destroy callback

[David Herrmann]

Hi Daniel

On Sun, Nov 3, 2013 at 2:31 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Call drm_pci_agp_destroy directly, there's no point in the
> indirection. Long term we want to shuffle this into each driver's
> unload logic, but that needs cleared-up drm lifetime rules first.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_pci.c  | 3 +--
>  drivers/gpu/drm/drm_stub.c | 4 ++--
>  include/drm/drmP.h         | 4 +---
>  3 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index d0937dd62899..36e7f8e74027 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -276,7 +276,7 @@ static void drm_pci_agp_init(struct drm_device *dev)
>         }
>  }
>
> -static void drm_pci_agp_destroy(struct drm_device *dev)
> +void drm_pci_agp_destroy(struct drm_device *dev)

That one is #ifdef CONFIG_PCI
So please provide a dummy below, same as for drm_pci_init().

If that's fixed:

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

>  {
>         if (dev->agp) {
>                 arch_phys_wc_del(dev->agp->agp_mtrr);
> @@ -292,7 +292,6 @@ static struct drm_bus drm_pci_bus = {
>         .get_name = drm_pci_get_name,
>         .set_busid = drm_pci_set_busid,
>         .set_unique = drm_pci_set_unique,
> -       .agp_destroy = drm_pci_agp_destroy,
>  };
>
>  /**
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 4b25f693ae89..3ec8d4f9f09a 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -569,8 +569,8 @@ void drm_dev_unregister(struct drm_device *dev)
>         if (dev->driver->unload)
>                 dev->driver->unload(dev);
>
> -       if (dev->driver->bus->agp_destroy)
> -               dev->driver->bus->agp_destroy(dev);
> +       if (dev->agp)
> +               drm_pci_agp_destroy(dev);
>
>         drm_vblank_cleanup(dev);
>
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 8cf8cfef8c56..f67104aa7b51 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -748,9 +748,6 @@ struct drm_bus {
>         int (*set_unique)(struct drm_device *dev, struct drm_master *master,
>                           struct drm_unique *unique);
>         int (*irq_by_busid)(struct drm_device *dev, struct drm_irq_busid *p);
> -       /* hooks that are for PCI */
> -       void (*agp_destroy)(struct drm_device *dev);
> -
>  };
>
>  /**
> @@ -1658,6 +1655,7 @@ static __inline__ int drm_pci_device_is_agp(struct drm_device *dev)
>
>         return pci_find_capability(dev->pdev, PCI_CAP_ID_AGP);
>  }
> +void drm_pci_agp_destroy(struct drm_device *dev);
>
>  extern int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver);
>  extern void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver);
> --
> 1.8.4.rc3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: kill the ->agp_destroy callback

[Daniel Vetter]

Call drm_pci_agp_destroy directly, there's no point in the
indirection. Long term we want to shuffle this into each driver's
unload logic, but that needs cleared-up drm lifetime rules first.

v2: Add a dummy function for !CONFIG_PCI, spotted my David Herrmann.

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Cc: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_pci.c  | 4 ++--
 drivers/gpu/drm/drm_stub.c | 4 ++--
 include/drm/drmP.h         | 4 +---
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 71b84997abc5..2a6ac87b2e82 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -276,7 +276,7 @@ static void drm_pci_agp_init(struct drm_device *dev)
 	}
 }
 
-static void drm_pci_agp_destroy(struct drm_device *dev)
+void drm_pci_agp_destroy(struct drm_device *dev)
 {
 	if (dev->agp) {
 		arch_phys_wc_del(dev->agp->agp_mtrr);
@@ -293,7 +293,6 @@ static struct drm_bus drm_pci_bus = {
 	.set_busid = drm_pci_set_busid,
 	.set_unique = drm_pci_set_unique,
 	.irq_by_busid = drm_pci_irq_by_busid,
-	.agp_destroy = drm_pci_agp_destroy,
 };
 
 /**
@@ -457,6 +456,7 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
 	return -1;
 }
 
+void drm_pci_agp_destroy(struct drm_device *dev) { };
 #endif
 
 EXPORT_SYMBOL(drm_pci_init);
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 4b25f693ae89..3ec8d4f9f09a 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -569,8 +569,8 @@ void drm_dev_unregister(struct drm_device *dev)
 	if (dev->driver->unload)
 		dev->driver->unload(dev);
 
-	if (dev->driver->bus->agp_destroy)
-		dev->driver->bus->agp_destroy(dev);
+	if (dev->agp)
+		drm_pci_agp_destroy(dev);
 
 	drm_vblank_cleanup(dev);
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 8cf8cfef8c56..f67104aa7b51 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -748,9 +748,6 @@ struct drm_bus {
 	int (*set_unique)(struct drm_device *dev, struct drm_master *master,
 			  struct drm_unique *unique);
 	int (*irq_by_busid)(struct drm_device *dev, struct drm_irq_busid *p);
-	/* hooks that are for PCI */
-	void (*agp_destroy)(struct drm_device *dev);
-
 };
 
 /**
@@ -1658,6 +1655,7 @@ static __inline__ int drm_pci_device_is_agp(struct drm_device *dev)
 
 	return pci_find_capability(dev->pdev, PCI_CAP_ID_AGP);
 }
+void drm_pci_agp_destroy(struct drm_device *dev);
 
 extern int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver);
 extern void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver);
-- 
1.8.4.rc3

Re: [PATCH] drm: kill the ->agp_destroy callback

[David Herrmann]

Hey Daniel

On Sun, Nov 3, 2013 at 3:24 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Call drm_pci_agp_destroy directly, there's no point in the
> indirection. Long term we want to shuffle this into each driver's
> unload logic, but that needs cleared-up drm lifetime rules first.
>
> v2: Add a dummy function for !CONFIG_PCI, spotted my David Herrmann.
>
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> Cc: David Herrmann <dh.herrmann@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_pci.c  | 4 ++--
>  drivers/gpu/drm/drm_stub.c | 4 ++--
>  include/drm/drmP.h         | 4 +---
>  3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 71b84997abc5..2a6ac87b2e82 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -276,7 +276,7 @@ static void drm_pci_agp_init(struct drm_device *dev)
>         }
>  }
>
> -static void drm_pci_agp_destroy(struct drm_device *dev)
> +void drm_pci_agp_destroy(struct drm_device *dev)
>  {
>         if (dev->agp) {
>                 arch_phys_wc_del(dev->agp->agp_mtrr);
> @@ -293,7 +293,6 @@ static struct drm_bus drm_pci_bus = {
>         .set_busid = drm_pci_set_busid,
>         .set_unique = drm_pci_set_unique,
>         .irq_by_busid = drm_pci_irq_by_busid,
> -       .agp_destroy = drm_pci_agp_destroy,
>  };
>
>  /**
> @@ -457,6 +456,7 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
>         return -1;
>  }
>
> +void drm_pci_agp_destroy(struct drm_device *dev) { };

Looks good, but if you want to avoid getting a coding-style fixup
patch in the next week from an eager linux-next tester, I'd recommend
removing that unnecessary semicolon.

Thanks
David

>  #endif
>
>  EXPORT_SYMBOL(drm_pci_init);
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 4b25f693ae89..3ec8d4f9f09a 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -569,8 +569,8 @@ void drm_dev_unregister(struct drm_device *dev)
>         if (dev->driver->unload)
>                 dev->driver->unload(dev);
>
> -       if (dev->driver->bus->agp_destroy)
> -               dev->driver->bus->agp_destroy(dev);
> +       if (dev->agp)
> +               drm_pci_agp_destroy(dev);
>
>         drm_vblank_cleanup(dev);
>
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 8cf8cfef8c56..f67104aa7b51 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -748,9 +748,6 @@ struct drm_bus {
>         int (*set_unique)(struct drm_device *dev, struct drm_master *master,
>                           struct drm_unique *unique);
>         int (*irq_by_busid)(struct drm_device *dev, struct drm_irq_busid *p);
> -       /* hooks that are for PCI */
> -       void (*agp_destroy)(struct drm_device *dev);
> -
>  };
>
>  /**
> @@ -1658,6 +1655,7 @@ static __inline__ int drm_pci_device_is_agp(struct drm_device *dev)
>
>         return pci_find_capability(dev->pdev, PCI_CAP_ID_AGP);
>  }
> +void drm_pci_agp_destroy(struct drm_device *dev);
>
>  extern int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver);
>  extern void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver);
> --
> 1.8.4.rc3
>

[PATCH] drm: kill the ->agp_destroy callback

[Daniel Vetter]

Call drm_pci_agp_destroy directly, there's no point in the
indirection. Long term we want to shuffle this into each driver's
unload logic, but that needs cleared-up drm lifetime rules first.

v2: Add a dummy function for !CONFIG_PCI, spotted my David Herrmann.

v3: Fixup for the coding style police.

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Cc: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_pci.c  | 4 ++--
 drivers/gpu/drm/drm_stub.c | 4 ++--
 include/drm/drmP.h         | 4 +---
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 71b84997abc5..250c3b378bd6 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -276,7 +276,7 @@ static void drm_pci_agp_init(struct drm_device *dev)
 	}
 }
 
-static void drm_pci_agp_destroy(struct drm_device *dev)
+void drm_pci_agp_destroy(struct drm_device *dev)
 {
 	if (dev->agp) {
 		arch_phys_wc_del(dev->agp->agp_mtrr);
@@ -293,7 +293,6 @@ static struct drm_bus drm_pci_bus = {
 	.set_busid = drm_pci_set_busid,
 	.set_unique = drm_pci_set_unique,
 	.irq_by_busid = drm_pci_irq_by_busid,
-	.agp_destroy = drm_pci_agp_destroy,
 };
 
 /**
@@ -457,6 +456,7 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
 	return -1;
 }
 
+void drm_pci_agp_destroy(struct drm_device *dev) {}
 #endif
 
 EXPORT_SYMBOL(drm_pci_init);
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 4b25f693ae89..3ec8d4f9f09a 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -569,8 +569,8 @@ void drm_dev_unregister(struct drm_device *dev)
 	if (dev->driver->unload)
 		dev->driver->unload(dev);
 
-	if (dev->driver->bus->agp_destroy)
-		dev->driver->bus->agp_destroy(dev);
+	if (dev->agp)
+		drm_pci_agp_destroy(dev);
 
 	drm_vblank_cleanup(dev);
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 8cf8cfef8c56..f67104aa7b51 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -748,9 +748,6 @@ struct drm_bus {
 	int (*set_unique)(struct drm_device *dev, struct drm_master *master,
 			  struct drm_unique *unique);
 	int (*irq_by_busid)(struct drm_device *dev, struct drm_irq_busid *p);
-	/* hooks that are for PCI */
-	void (*agp_destroy)(struct drm_device *dev);
-
 };
 
 /**
@@ -1658,6 +1655,7 @@ static __inline__ int drm_pci_device_is_agp(struct drm_device *dev)
 
 	return pci_find_capability(dev->pdev, PCI_CAP_ID_AGP);
 }
+void drm_pci_agp_destroy(struct drm_device *dev);
 
 extern int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver);
 extern void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver);
-- 
1.8.4.rc3

[PATCH 19/19] drm: remove global_mutex locking around agp_init

[Daniel Vetter]

David Herrmann dutifully moved this locking along when moving the
agp_init call out of the generic drm_dev_register into the pci
specific load helpers.

But afaict there's no need and the reason for that locking has been
purely a historical accident - we need the lock around the driver dev
node registration to paper over the midlayer init races, and the agp
init simply ended up in there. The real fix for all this is of course
to delay the dev (and sysfs/debugfs) interface registration until
everything is fully set up.

Until then stop the cargo-cult locking from spreading and remove the
locking.

Cc: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_pci.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 36e7f8e74027..86a8484a1bf4 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -329,9 +329,7 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		pci_set_drvdata(pdev, dev);
 
-	mutex_lock(&drm_global_mutex);
 	drm_pci_agp_init(dev);
-	mutex_unlock(&drm_global_mutex);
 
 	ret = drm_dev_register(dev, ent->driver_data);
 	if (ret)
@@ -349,9 +347,7 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 	return 0;
 
 err_agp:
-	mutex_lock(&drm_global_mutex);
 	drm_pci_agp_destroy(dev);
-	mutex_unlock(&drm_global_mutex);
 	pci_disable_device(pdev);
 err_free:
 	drm_dev_free(dev);
-- 
1.8.4.rc3