Daniel,
A while back you had a go at fixing the framebuffer reference counting. Unfortunately, I'm still seeing leaks, particularly with the framebuffer used with a mode set which is subsequently flipped. It is only this framebuffer which is leaked, flips of an already flipped framebuffer do not.
Looking at what happens to this framebuffer, I see these refcount changes:
ADDFB - 0->1 - kref_init - 1->2 - drm_framebuffer_reference() creation
SETCRTC - 2->3 - drm_framebuffer_lookup() in drm_mode_setcrtc() - 3->4 - drm_framebuffer_reference() in armada_drm_crtc_mode_set() - 4->5 - drm_framebuffer_reference() in drm_mode_set_config_internal() - 5->4 - drm_framebuffer_unreference() in drm_mode_setcrtc() * current scanout framebuffer has refcount of 4
PAGE_FLIP - 4->5 - drm_framebuffer_reference() in armada_drm_crtc_page_flip() - 5->4 - drm_framebuffer_unreference(old_fb) in drm_mode_page_flip_ioctl() - 4->3 - drm_framebuffer_unreference() in armada_drm_unref_work()
RMFB - 3->2 - __drm_framebuffer_unreference() - 2->1 - drm_framebuffer_unreference() - *LEAK*
For the framebuffer being flipped in:
ADDFB - 0->1 - kref_init - 1->2 - drm_framebuffer_reference() creation
PAGE_FLIP (incoming) - 2->3 - drm_framebuffer_lookup() in drm_mode_page_flip_ioctl() * current scanout framebuffer has refcount of 3 - different from setcrtc.
PAGE_FLIP (outgoing) - 3->4 - drm_framebuffer_reference() in armada_drm_crtc_page_flip() - 4->3 - drm_framebuffer_unreference(old_fb) in drm_mode_page_flip_ioctl() - 3->2 - drm_framebuffer_unreference() in armada_drm_unref_work()
RMFB - 2->1 - __drm_framebuffer_unreference() - 1->0 - drm_framebuffer_unreference() - *free*
Of course, this isn't going to be nice given appropriate timing of a subsequent SETCRTC removing the FB and dropping its refcounts.
If I get rid of the referencing of the old framebuffer in PAGE_FLIP, then:
ADDFB - 0->1 - kref_init - 1->2 - drm_framebuffer_reference() creation
SETCRTC - 2->3 - drm_framebuffer_lookup() in drm_mode_setcrtc() - 3->4 - drm_framebuffer_reference() in armada_drm_crtc_mode_set() - 4->5 - drm_framebuffer_reference() in drm_mode_set_config_internal() - 5->4 - drm_framebuffer_unreference() in drm_mode_setcrtc() * current scanout framebuffer has refcount of 4
PAGE_FLIP - 4->3 - drm_framebuffer_unreference(old_fb) in drm_mode_page_flip_ioctl() - 3->2 - drm_framebuffer_unreference() in armada_drm_unref_work()
RMFB - 2->1 - __drm_framebuffer_unreference() - 1->0 - drm_framebuffer_unreference() - *free*
So that looks good, but let's look at what happens to the framebuffer being flipped in:
ADDFB - 0->1 - kref_init - 1->2 - drm_framebuffer_reference() creation
PAGE_FLIP (incoming) - 2->3 - drm_framebuffer_lookup() in drm_mode_page_flip_ioctl() * current scanout framebuffer has refcount of 3 - different from setcrtc.
PAGE_FLIP (outgoing) - 3->2 - drm_framebuffer_unreference(old_fb) in drm_mode_page_flip_ioctl() - 2->1 - drm_framebuffer_unreference() in armada_drm_unref_work()
RMFB - 1->0 - __drm_framebuffer_unreference() - *OOPS* - 0->-1 - drm_framebuffer_unreference() - *broken*
So that doesn't work - and it still isn't good because the framebuffer which was flipped in has a different refcount to the one going out.
If I instead try taking a reference on the new framebuffer and dropping the previous ref in PAGE_FLIP:
ADDFB - 0->1 - kref_init - 1->2 - drm_framebuffer_reference() creation
SETCRTC - 2->3 - drm_framebuffer_lookup() in drm_mode_setcrtc() - 3->4 - drm_framebuffer_reference() in armada_drm_crtc_mode_set() - 4->5 - drm_framebuffer_reference() in drm_mode_set_config_internal() - 5->4 - drm_framebuffer_unreference() in drm_mode_setcrtc() * current scanout framebuffer has refcount of 4
PAGE_FLIP - 4->3 - drm_framebuffer_unreference() in armada_drm_crtc_page_flip() - 3->2 - drm_framebuffer_unreference(old_fb) in drm_mode_page_flip_ioctl() - 2->1 - drm_framebuffer_unreference() in armada_drm_unref_work()
RMFB - 1->0 - __drm_framebuffer_unreference() - *OOPS* - 0->-1 - drm_framebuffer_unreference() - (never happens)
I can see no way to solve this in driver code, other than tracking where the framebuffer came from, and conditionally adjusting the refcount to keep things sane. The real problem here is the different behaviour in the DRM core code between the refcounts resulting on the active scanout framebuffer.
Any other thoughts how to solve this without having such refcount hacks in drivers?
Thanks.