Possible fb ref count issue with drm_plane_force_disable()

[Archit Taneja]

On Friday 11 April 2014 12:10 PM, Tomi Valkeinen wrote:

<snip>

>> Does drm_framebuffer_remove get called when we abort the application, or
>> unload omapdrm, or both?
>
> Both. When we abort an app, drm_framebuffer_remove gets called for the
> fb that the app created. When we unload omapdrm, it gets called for the
> main fb, used for fb console.
>
>>> - fb->refcount.refcount > 1, so it goes to disable stuff
>>>
>>> - drm_plane_force_disable is called for the primary plane
>>
>> Maybe we need to make sure that this func is called only when our driver
>> has unreferenced it. In that case, we would probably need to flush our
>> queue and disable interrupts(so that we don't queue more work).
>
> Hmm, sorry, call which func, unreferenced what? =)
>
> Do you mean we should call drm_framebuffer_remove() only if
> fb->refcount.refcount == 1. That should be possible, and would probably
> remove the issue, but it would just be going around the actual problem.

Yes, I meant our driver should call drm_framebuffer_remove() only when 
we are know that the fb reference omapdrm had taken(the one in 
update_pin), has a corresponding fb unref called(in unpin_worker).

Isn't that something omapdrm should take care of?

<snip>

>> I can't get the corresponding reference for it either. But I can count 3
>> of them when you run fbcon with drm's fb helper.
>>
>> - One ref is taken in the drm_framebuffer_init called from
>> omap_fbdev_create.
>>
>> - fbcon will call fb_set_par, which calls drm_fb_helper_set_par, that
>> seems to take a refernce to the fb in the end.
>
> This one is not called for a normal app, as there's no fbdev framebuffer
> for it. Note that the funcs I mention deal with drm framebuffer, which
> is not the fbdev framebuffer (confusing =). fbdev fb is only used for
> the "root" framebuffer. And, of course, fbdev fb uses the drm fb

Yes. In the case of a normal app, we would call the ADD_FB2 ioctl to get 
a drm_framebuffer, that will internally take a fb reference. So the 
count won't seem to go beyond 2.

> functionality.
>
>> - drm_crtc_helper_set_config() calls the omadrm specific mode_set
>> drm_crtc_funcs, which eventually calls a drm_framebuffer_reference in
>> update_pin().
>
> Yep.
>
> I forgot to mention that if I comment out the unref in
> drm_plane_force_disable(), the ref counts match and all looks fine. And
> also that I didn't see this issue with 3.14.

That's strange. I guess there must be an extra unref happening somewhere 
in the newer commits. Did you try a diff of drm code between the 2 
kernels? :)

Archit