On Wed, Feb 19, 2020 at 3:39 PM Laurent Pinchart laurent.pinchart@ideasonboard.com wrote:
Hi Daniel,
On Wed, Feb 19, 2020 at 03:30:59PM +0100, Daniel Vetter wrote:
On Wed, Feb 19, 2020 at 3:11 PM Laurent Pinchart wrote:
On Wed, Feb 19, 2020 at 11:20:49AM +0100, Daniel Vetter wrote:
These are the leftover drivers that didn't have a ->release hook that needed to be updated.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: "James (Qian) Wang" james.qian.wang@arm.com Cc: Liviu Dudau liviu.dudau@arm.com Cc: Mihail Atanassov mihail.atanassov@arm.com Cc: Russell King linux@armlinux.org.uk Cc: Hans de Goede hdegoede@redhat.com
drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 2 ++ drivers/gpu/drm/armada/armada_drv.c | 2 ++ drivers/gpu/drm/vboxvideo/vbox_drv.c | 2 ++ 3 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c index 442d4656150a..16dfd5cdb66c 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c @@ -14,6 +14,7 @@ #include <drm/drm_gem_cma_helper.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_irq.h> +#include <drm/drm_managed.h> #include <drm/drm_probe_helper.h> #include <drm/drm_vblank.h>
@@ -271,6 +272,7 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev) err = drm_dev_init(drm, &komeda_kms_driver, mdev->dev); if (err) goto free_kms;
drmm_add_final_kfree(drm, kms);Instead of sprinkling calls to drmm_add_final_kfree() everywhere, wouldn't it be better to pass the parent pointer to drm_dev_init() ?
Would lead to a horrendous monster patch, and even with this splitting there were a few corner cases.
It could be generated by coccinelle, with the semantic patch included in the commit message, so that regenerating it should be possible when merging if conflict arise.
It's not that easy, because drivers are buggy. So you need to review the remove/release implementation for all of them (which I've done) to make sure you're not making things worse with some additional use-after-free or something else horrible. That's why this is so much split up.
So automated patch for this is out of the window imo. -Daniel
My plan is to add a devm_drm_dev_alloc pattern which combines the usual pattern that most drivers use, see the last patch for all these glorious ideas.
OK I will.
So yeah I hope this will all go away (or mostly at least), but for bisecting I didn't come up with a better idea to get this all off the ground unfortunately.
drm->dev_private = mdev;diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c index 197dca3fc84c..dd9ed71ed942 100644 --- a/drivers/gpu/drm/armada/armada_drv.c +++ b/drivers/gpu/drm/armada/armada_drv.c @@ -12,6 +12,7 @@ #include <drm/drm_atomic_helper.h> #include <drm/drm_drv.h> #include <drm/drm_ioctl.h> +#include <drm/drm_managed.h> #include <drm/drm_prime.h> #include <drm/drm_probe_helper.h> #include <drm/drm_fb_helper.h> @@ -103,6 +104,7 @@ static int armada_drm_bind(struct device *dev) kfree(priv); return ret; }
drmm_add_final_kfree(&priv->drm, priv); /* Remove early framebuffers */ ret = drm_fb_helper_remove_conflicting_framebuffers(NULL,diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c index 8512d970a09f..13eaae7921f5 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c @@ -17,6 +17,7 @@ #include <drm/drm_fb_helper.h> #include <drm/drm_file.h> #include <drm/drm_ioctl.h> +#include <drm/drm_managed.h>
#include "vbox_drv.h"
@@ -54,6 +55,7 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) vbox->ddev.pdev = pdev; vbox->ddev.dev_private = vbox; pci_set_drvdata(pdev, vbox);
drmm_add_final_kfree(&vbox->ddev, vbox); mutex_init(&vbox->hw_mutex); ret = pci_enable_device(pdev);-- Regards,
Laurent Pinchart