Re: [PATCH] drm/i915: Don't unregister fbdev twice

From: Daniel Vetter <daniel@ffwll.ch>
To: Lukas Wunner <lukas@wunner.de>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Don't unregister fbdev twice
Date: Mon, 13 Jun 2016 16:28:41 +0200	[thread overview]
Message-ID: <20160613142841.GD1338@phenom.ffwll.local> (raw)
In-Reply-To: <20160608170302.GB18955@wunner.de>

On Wed, Jun 08, 2016 at 07:03:02PM +0200, Lukas Wunner wrote:
> On Wed, Jun 08, 2016 at 02:09:40PM +0200, Daniel Vetter wrote:
> > On Wed, Jun 08, 2016 at 01:15:22PM +0200, Lukas Wunner wrote:
> > > Calling drm_framebuffer_unregister_private() in intel_fbdev_destroy() is
> > > superfluous because the framebuffer will subsequently be unregistered by
> > > drm_framebuffer_free() when unreferenced in drm_framebuffer_remove().
> > > The call is a leftover, when it was introduced by commit 362063619cf6
> > > ("drm: revamp framebuffer cleanup interfaces"), struct intel_framebuffer
> > > was still embedded in struct intel_fbdev rather than being a pointer as
> > > it is today, and drm_framebuffer_remove() wasn't used yet.
> > > 
> > > As a bonus, the ID of the framebuffer is no longer 0 in the debug log:
> > > 
> > > Before:
> > >     [   39.680874] [drm:drm_mode_object_unreference] OBJ ID: 0 (3)
> > >     [   39.680878] [drm:drm_mode_object_unreference] OBJ ID: 0 (2)
> > >     [   39.680884] [drm:drm_mode_object_unreference] OBJ ID: 0 (1)
> > > 
> > > After:
> > >     [  102.504649] [drm:drm_mode_object_unreference] OBJ ID: 45 (3)
> > >     [  102.504651] [drm:drm_mode_object_unreference] OBJ ID: 45 (2)
> > >     [  102.504654] [drm:drm_mode_object_unreference] OBJ ID: 45 (1)
> > > 
> > > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > 
> > Hm yeah. But there's a pile of that particaluar cargo-culting copied all
> > over the place, would be really good to audit all callers of
> > unregister_private and fix them all up. A few older drivers still embed
> > the fbdev fb, but most don't but still use the unregister_private +
> > cleanup combo.
> 
> Yes, I noticed that but i915 was the only one that I could actually test,
> the others I can only compile test. So fixing those up requires very
> careful examination and takes more time, but I'll keep it on my todo list.
> 
> 
> > Nitpick in your subject: s/fbdev/fbdev's fb/
> 
> Right, should I post a v2 or are you going to fix it up if/when merging?

Fixed up while applying - I just waited for CI to get around (and then
w/e). Going through the other drivers to nuke the cargo-culting would
still be awesome.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx