[RFC 3/9] drm/agp: fix AGP cleanup paths
Dave Airlie
airlied at gmail.com
Fri Jul 26 23:05:48 PDT 2013
On Wed, Jul 24, 2013 at 11:43 PM, David Herrmann <dh.herrmann at gmail.com> wrote:
> Introduce two new helpers, drm_agp_clear() and drm_agp_destroy() which
> clear all AGP mappings and destroy the AGP head. This allows to reduce the
> AGP code in core DRM and move it all to drm_agpsupport.c.
>
Could we do this as the first patch? it seems like it would be a nice
thing to have standalone even with the current init paths.
Dave.
> Also use these new helpers to clean up AGP allocations in the
> drm_dev_register() error path.
>
> Signed-off-by: David Herrmann <dh.herrmann at gmail.com>
> ---
> drivers/gpu/drm/drm_agpsupport.c | 51 ++++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_drv.c | 21 +----------------
> drivers/gpu/drm/drm_pci.c | 12 ++++++++++
> drivers/gpu/drm/drm_stub.c | 12 ++++------
> include/drm/drmP.h | 3 +++
> 5 files changed, 71 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_agpsupport.c b/drivers/gpu/drm/drm_agpsupport.c
> index 3d8fed1..e301d65 100644
> --- a/drivers/gpu/drm/drm_agpsupport.c
> +++ b/drivers/gpu/drm/drm_agpsupport.c
> @@ -424,6 +424,57 @@ struct drm_agp_head *drm_agp_init(struct drm_device *dev)
> }
>
> /**
> + * drm_agp_clear - Clear AGP resource list
> + * @dev: DRM device
> + *
> + * Iterate over all AGP resources and remove them. But keep the AGP head
> + * intact so it can still be used. It is safe to call this if AGP is disabled or
> + * was already removed.
> + *
> + * If DRIVER_MODESET is active, nothing is done to protect the modesetting
> + * resources from getting destroyed. Drivers are responsible of cleaning them up
> + * during device shutdown.
> + */
> +void drm_agp_clear(struct drm_device *dev)
> +{
> + struct drm_agp_mem *entry, *tempe;
> +
> + if (!drm_core_has_AGP(dev) || !dev->agp)
> + return;
> + if (drm_core_check_feature(dev, DRIVER_MODESET))
> + return;
> +
> + list_for_each_entry_safe(entry, tempe, &dev->agp->memory, head) {
> + if (entry->bound)
> + drm_unbind_agp(entry->memory);
> + drm_free_agp(entry->memory, entry->pages);
> + kfree(entry);
> + }
> + INIT_LIST_HEAD(&dev->agp->memory);
> +
> + if (dev->agp->acquired)
> + drm_agp_release(dev);
> +
> + dev->agp->acquired = 0;
> + dev->agp->enabled = 0;
> +}
> +
> +/**
> + * 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_drv.c b/drivers/gpu/drm/drm_drv.c
> index 36103d1..dddd799 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -195,27 +195,8 @@ int drm_lastclose(struct drm_device * dev)
>
> mutex_lock(&dev->struct_mutex);
>
> - /* Clear AGP information */
> - if (drm_core_has_AGP(dev) && dev->agp &&
> - !drm_core_check_feature(dev, DRIVER_MODESET)) {
> - struct drm_agp_mem *entry, *tempe;
> -
> - /* Remove AGP resources, but leave dev->agp
> - intact until drv_cleanup is called. */
> - list_for_each_entry_safe(entry, tempe, &dev->agp->memory, head) {
> - if (entry->bound)
> - drm_unbind_agp(entry->memory);
> - drm_free_agp(entry->memory, entry->pages);
> - kfree(entry);
> - }
> - INIT_LIST_HEAD(&dev->agp->memory);
> -
> - if (dev->agp->acquired)
> - drm_agp_release(dev);
> + drm_agp_clear(dev);
>
> - dev->agp->acquired = 0;
> - dev->agp->enabled = 0;
> - }
> if (drm_core_check_feature(dev, DRIVER_SG) && dev->sg &&
> !drm_core_check_feature(dev, DRIVER_MODESET)) {
> drm_sg_cleanup(dev->sg);
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index ab861ca..0daee24 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -283,6 +283,17 @@ static int drm_pci_agp_init(struct drm_device *dev)
> return 0;
> }
>
> +static void drm_pci_agp_destroy(struct drm_device *dev)
> +{
> + if (drm_core_has_AGP(dev) && dev->agp) {
> + if (drm_core_has_MTRR(dev))
> + arch_phys_wc_del(dev->agp->agp_mtrr);
> + drm_agp_clear(dev);
> + drm_agp_destroy(dev->agp);
> + dev->agp = NULL;
> + }
> +}
> +
> static struct drm_bus drm_pci_bus = {
> .bus_type = DRIVER_BUS_PCI,
> .get_irq = drm_pci_get_irq,
> @@ -291,6 +302,7 @@ static struct drm_bus drm_pci_bus = {
> .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,
> };
>
> /**
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index bc1d860..f016ed8 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -387,16 +387,11 @@ void drm_put_dev(struct drm_device *dev)
>
> drm_lastclose(dev);
>
> - if (drm_core_has_MTRR(dev) && drm_core_has_AGP(dev) && dev->agp)
> - arch_phys_wc_del(dev->agp->agp_mtrr);
> -
> if (dev->driver->unload)
> dev->driver->unload(dev);
>
> - if (drm_core_has_AGP(dev) && dev->agp) {
> - kfree(dev->agp);
> - dev->agp = NULL;
> - }
> + if (dev->driver->bus->agp_destroy)
> + dev->driver->bus->agp_destroy(dev);
>
> drm_vblank_cleanup(dev);
>
> @@ -575,7 +570,8 @@ err_control_node:
> if (drm_core_check_feature(dev, DRIVER_MODESET))
> drm_put_minor(&dev->control);
> err_agp:
> - /* FIXME: cleanup AGP leftovers */
> + 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 ea5e130..30f83ee 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -737,6 +737,7 @@ struct drm_bus {
> 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_destroy)(struct drm_device *dev);
>
> };
>
> @@ -1454,6 +1455,8 @@ extern int drm_modeset_ctl(struct drm_device *dev, void *data,
>
> /* AGP/GART support (drm_agpsupport.h) */
> extern struct drm_agp_head *drm_agp_init(struct drm_device *dev);
> +extern void drm_agp_destroy(struct drm_agp_head *agp);
> +extern void drm_agp_clear(struct drm_device *dev);
> extern int drm_agp_acquire(struct drm_device *dev);
> extern int drm_agp_acquire_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> --
> 1.8.3.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
More information about the dri-devel
mailing list