From mboxrd@z Thu Jan 1 00:00:00 1970 From: Archit Taneja Subject: Re: Possible fb ref count issue with drm_plane_force_disable() Date: Fri, 11 Apr 2014 12:27:52 +0530 Message-ID: <53479270.6070501@ti.com> References: <534684E8.9000203@ti.com> <53478C42.5080502@ti.com> <53478E65.6090305@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from arroyo.ext.ti.com (arroyo.ext.ti.com [192.94.94.40]) by gabe.freedesktop.org (Postfix) with ESMTP id 99E1A6E345 for ; Thu, 10 Apr 2014 23:58:45 -0700 (PDT) In-Reply-To: <53478E65.6090305@ti.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Tomi Valkeinen Cc: dri-devel List-Id: dri-devel@lists.freedesktop.org On Friday 11 April 2014 12:10 PM, Tomi Valkeinen wrote: >> 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? >> 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