Message ID | 20200302222631.3861340-10-daniel.vetter@ffwll.ch (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm_device managed resources, v4 | expand |
> @@ -575,9 +574,12 @@ static int cirrus_pci_probe(struct pci_dev *pdev, > > dev = &cirrus->dev; > ret = drm_dev_init(dev, &cirrus_driver, &pdev->dev); > - if (ret) > - goto err_free_cirrus; > + if (ret) { > + kfree(cirrus); > + goto err_pci_release; > + } > dev->dev_private = cirrus; > + drmm_add_final_kfree(dev, cirrus); That doesn't look like an error path improvement. With patch #30 applied it'll looks alot better though. So maybe squash the patches? In any case: Acked-by: Gerd Hoffmann <kraxel@redhat.com> cheers, Gerd
On Tue, Mar 03, 2020 at 08:49:34AM +0100, Gerd Hoffmann wrote: > > @@ -575,9 +574,12 @@ static int cirrus_pci_probe(struct pci_dev *pdev, > > > > dev = &cirrus->dev; > > ret = drm_dev_init(dev, &cirrus_driver, &pdev->dev); > > - if (ret) > > - goto err_free_cirrus; > > + if (ret) { > > + kfree(cirrus); > > + goto err_pci_release; > > + } > > dev->dev_private = cirrus; > > + drmm_add_final_kfree(dev, cirrus); > > That doesn't look like an error path improvement. > With patch #30 applied it'll looks alot better though. > So maybe squash the patches? Breaks the patch set evolution, there's a _lot_ of dependencies in here to make sure we never break anything interim. But yeah that's why I created this entire series, since with just the first part it's really not any better. I also have a pile more ideas on top, so hopefully once this lands I can get around to them and make everything even better :-) Cheers, Daniel > > In any case: > Acked-by: Gerd Hoffmann <kraxel@redhat.com> > > cheers, > Gerd >
diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c index d2ff63ce8eaf..2232556ce34c 100644 --- a/drivers/gpu/drm/cirrus/cirrus.c +++ b/drivers/gpu/drm/cirrus/cirrus.c @@ -35,6 +35,7 @@ #include <drm/drm_gem_shmem_helper.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_ioctl.h> +#include <drm/drm_managed.h> #include <drm/drm_modeset_helper_vtables.h> #include <drm/drm_probe_helper.h> #include <drm/drm_simple_kms_helper.h> @@ -527,10 +528,8 @@ static void cirrus_mode_config_init(struct cirrus_device *cirrus) static void cirrus_release(struct drm_device *dev) { - struct cirrus_device *cirrus = dev->dev_private; - drm_mode_config_cleanup(dev); - kfree(cirrus); + drm_dev_fini(dev); } DEFINE_DRM_GEM_FOPS(cirrus_fops); @@ -575,9 +574,12 @@ static int cirrus_pci_probe(struct pci_dev *pdev, dev = &cirrus->dev; ret = drm_dev_init(dev, &cirrus_driver, &pdev->dev); - if (ret) - goto err_free_cirrus; + if (ret) { + kfree(cirrus); + goto err_pci_release; + } dev->dev_private = cirrus; + drmm_add_final_kfree(dev, cirrus); ret = -ENOMEM; cirrus->vram = ioremap(pci_resource_start(pdev, 0), @@ -618,8 +620,6 @@ static int cirrus_pci_probe(struct pci_dev *pdev, iounmap(cirrus->vram); err_dev_put: drm_dev_put(dev); -err_free_cirrus: - kfree(cirrus); err_pci_release: pci_release_regions(pdev); return ret;