On Wed, May 24, 2017 at 04:51:45PM +0200, Daniel Vetter wrote: [...] > -Resources allocated by :c:func:`drm_vblank_init()` must be freed > -with a call to :c:func:`drm_vblank_cleanup()` in the driver unload > -operation handler. I think perhaps this is the reason why this was cargo-culted. It's confusing to have the documentation say drivers have to call it, when in fact the core already does. > @@ -396,6 +436,8 @@ EXPORT_SYMBOL(drm_vblank_cleanup); > * @num_crtcs: number of CRTCs supported by @dev > * > * This function initializes vblank support for @num_crtcs display pipelines. > + * Drivers do not need to call drm_vblank_cleanup(), cleanup is already handled > + * by the DRM core. This is key, I think, in understanding why the patch series is correct. Maybe this should go into the cover letter to provide more context. That said, there is one case where the core wouldn't be calling the drm_vblank_cleanup() as part of teardown, and that is when the driver implements a ->release() callback. From a very cursory look i915 is the only driver that does so, and it ends up calling drm_dev_fini() and therefore drm_vblank_cleanup(). Given all of the above, for the series: Acked-by: Thierry Reding <treding@xxxxxxxxxx>
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel