Re: [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Daniel,

On Tuesday 13 Dec 2016 22:49:51 Daniel Vetter wrote:
> On Tue, Dec 13, 2016 at 04:32:30PM -0500, Sean Paul wrote:
> > On Tue, Dec 13, 2016 at 4:30 PM, Laurent Pinchart wrote:
> >> On Tuesday 13 Dec 2016 22:10:58 Daniel Vetter wrote:
> >>> On Tue, Dec 13, 2016 at 09:34:05PM +0200, Laurent Pinchart wrote:
> >>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>> 
> >>>> The drm driver .load() operation is prone to race conditions as it
> >>>> initializes the driver after registering the device nodes. Its usage
> >>>> is deprecated, inline it in the probe function and call
> >>>> drm_dev_alloc() and drm_dev_register() explicitly.
> >>>> 
> >>>> For consistency inline the .unload() handler in the remove function
> >>>> as well.
> >>>> 
> >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>> ---
> >>>> 
> >>>>  drivers/gpu/drm/exynos/exynos_dp.c       |   1 -
> >>>>  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   1 -
> >>>>  drivers/gpu/drm/exynos/exynos_drm_drv.c  | 245 ++++++++++++---------
> >>>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   1 -
> >>>>  drivers/gpu/drm/exynos/exynos_drm_vidi.c |   1 -
> >>>>  drivers/gpu/drm/exynos/exynos_hdmi.c     |   1 -
> >>>>  6 files changed, 127 insertions(+), 123 deletions(-)
> >> 
> >> [snip]
> >> 
> >>>>  static void exynos_drm_unbind(struct device *dev)
> >>>>  {
> >>>> 
> >>>> -   drm_put_dev(dev_get_drvdata(dev));
> >>>> +   struct drm_device *drm = dev_get_drvdata(dev);
> >>>> +
> >>>> +   drm_dev_unregister(drm);
> >>>> +
> >>>> +   exynos_drm_device_subdrv_remove(drm);
> >>>> +
> >>>> +   exynos_drm_fbdev_fini(drm);
> >>>> +   drm_kms_helper_poll_fini(drm);
> >>> 
> >>> Unbind order is inverted from the error paths in the probe function,
> >>> but meh, preexisting.
> >>> 
> >>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>o
> >>> 
> >>> ... because I really want to see drm_platform.c gone! Feel free to push
> >>> to drm-misc, you haz commit rights after all ;-)
> >> 
> >> Let's try to get the patch tested first :-) Is exynosdrm merged through
> >> drm- misc ?
> > 
> > Since 2/2 should go through misc, it makes sense to take the whole
> > series through there (instead of lockstepping with exynos tree)
> 
> Same for the one omapdrm patch if that's ok. drm-misc is resynced with
> drm-next-4.10, so there shouldn't be anything pending and it should all
> apply cleanly.

Except that the omapdrm patch is based on top of a series of ~20 pending 
patches. I'll respin the whole lot soon.

> >>>> +
> >>>> +   component_unbind_all(drm->dev, drm);
> >>>> +   drm_mode_config_cleanup(drm);
> >>>> +   drm_release_iommu_mapping(drm);
> >>>> +
> >>>> +   kfree(drm->dev_private);
> >>>> +   drm->dev_private = NULL;
> >>>> +
> >>>> +   drm_dev_unref(drm);
> >>>>  }

-- 
Regards,

Laurent Pinchart

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