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@xxxxxxxx> wrote:
> > Wrapping a kfree is pointless.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > ---
>
> I added that one dutifully to document the API in:
>
> commit 28ec711cd427f8b61f73712a43b8100ba8ca933b
> Author: David Herrmann <dh.herrmann@xxxxxxxxx>
> 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@xxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
