On Mon, May 29, 2017 at 01:07:40PM +0200, Philipp Zabel wrote: > Hi Daniel, > > On Wed, 2017-05-24 at 16:51 +0200, Daniel Vetter wrote: > > It's only done in the driver load error path, where vblanks don't need > > to be quiescent anyway. And that's all drm_vblank_cleanup does, since > > the core will release the vblank allocations on its own already. So > > drop it. > > Thank you for cleaning this up and improving the docs. > From the function name and kerneldoc comment, it was really not clear > that this function is already called in the drm_device release path. > > I think the comment is slightly misleading though, as drm_vblank_cleanup > does call kfree(dev->vblank). Yeah I got a bit sloppy with the commit message. Since this is error code resetting ->num_crtcs doesn't do anything useful, and the kfree happens anyway (because the core calls drm_vblank_cleanup for you). That only leaves the timer shutdown, and the timer can't run yet here at this point. Hence this does nothing useful (except move the kfree around). > > > Cc: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > > > > --- > > drivers/gpu/drm/imx/imx-drm-core.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c > > index 50add2f9e250..95e2181963d9 100644 > > --- a/drivers/gpu/drm/imx/imx-drm-core.c > > +++ b/drivers/gpu/drm/imx/imx-drm-core.c > > @@ -278,7 +278,7 @@ static int imx_drm_bind(struct device *dev) > > /* Now try and bind all our sub-components */ > > ret = component_bind_all(dev, drm); > > if (ret) > > - goto err_vblank; > > + goto err_kms; > > > > drm_mode_config_reset(drm); > > > > @@ -316,8 +316,6 @@ static int imx_drm_bind(struct device *dev) > > err_unbind: > > #endif > > component_unbind_all(drm->dev, drm); > > -err_vblank: > > - drm_vblank_cleanup(drm); > > err_kms: > > drm_mode_config_cleanup(drm); > > err_unref: > > As I understand, the drm_dev_unref(drm) that follows this causes > drm_dev_release -> drm_dev_fini -> drm_vblank_cleanup to be called, so > > Acked-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> Thanks, applied, as I did all the other patches which gathered an ack or r-b already. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
