On 04.08.2016 20:01, Daniel Vetter wrote:
On Thu, Aug 04, 2016 at 12:47:38PM +0200, Daniel Vetter wrote:
On Thu, Aug 04, 2016 at 12:39:36PM +0900, Michel Dänzer wrote:
@@ -5434,6 +5435,18 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, if (!crtc) return -ENOENT;
- if (crtc->funcs->page_flip_target) {
int r;r = drm_crtc_vblank_get(crtc);This leaks when e.g framebuffer_lookup fails.
Good catch.
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 9e6ab4a..ae1d9b6 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -579,6 +579,20 @@ struct drm_crtc_funcs { uint32_t flags);
/**
* @page_flip_target:** Same as @page_flip but with an additional parameter specifying the* target vertical blank period when the flip should take effect.*absolute target vertical blank period as reported by drm_crtc_vblank_count()
would imo be a good addition here.
[...]
** Note that the core code calls drm_crtc_vblank_get before this entry* point.I think you should add "Drivers must drop that reference again by calling drm_crtc_vblank_put()."
Thanks for the suggestions.
Also, who should drop the reference in case ->page_flip_target fails?
The core DRM code.
With all issues addressed:
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Thanks! I'll send out a v2 patch with your and Alex's feedback addressed. Will it be okay to merge this via Alex's tree?