From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maarten Lankhorst Subject: Re: [PATCH 6/7] drm/nouveau: embed gem object in nouveau_bo Date: Wed, 14 Aug 2013 16:31:15 +0200 Message-ID: <520B94B3.7060909@canonical.com> References: <1376485640-711-1-git-send-email-dh.herrmann@gmail.com> <1376485640-711-7-git-send-email-dh.herrmann@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from youngberry.canonical.com (youngberry.canonical.com [91.189.89.112]) by gabe.freedesktop.org (Postfix) with ESMTP id 16C8EE5C8B for ; Wed, 14 Aug 2013 07:31:17 -0700 (PDT) In-Reply-To: <1376485640-711-7-git-send-email-dh.herrmann@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: David Herrmann Cc: Martin Peres , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org Op 14-08-13 15:07, David Herrmann schreef: > There is no reason to keep the gem object separately allocated. nouveau is > the last user of gem_obj->driver_private, so if we embed it, we can get > rid of 8bytes per gem-object. > > The implementation follows the radeon driver. bo->gem is only valid, iff > the bo was created via the gem helpers _and_ iff the user holds a valid > gem reference. That is, as the gem object holds a reference to the > nouveau_bo. If you use nouveau_ref() to gain a bo reference, you are not > guaranteed to also hold a gem reference. The gem object might get > destroyed after the last user drops the gem-ref via > drm_gem_object_unreference(). Use drm_gem_object_reference() to gain a > gem-reference. > > For debugging, we can use bo->gem.filp != NULL to test whether a gem-bo is > valid. However, this shouldn't be used for real functionality to avoid > gem-internal dependencies. > > Note that the implementation follows the previous style. However, we no > longer can check for bo->gem != NULL to test for a valid gem object. This > wasn't done before, so we should be safe now. I'm all for getting rid of it, but I think it's not thorough enough. :P There's still a separate refcount in ttm for the bo, and I think it doesn't make sense to keep it like that. Instead of having that refcount in ttm, could it be put entirely in gem? ttm_buffer_object_transfer is the only time ttm creates a bo, and it's immediately destroyed, so instead of calling ttm_bo_unref it could call ttm_bo_release directly, in which case it doesn't matter that refcount is managed by gem. Oh except for vmwgfx of course, I always forget that 'special' case. Maybe something like this in memory? struct { struct ttm_buffer_object bo; struct drm_gem_object gem; // Can be anything, as long as first member is a refcount, and no hole between ttm and this.. }; This would allow ttm to do kref_put((kref_t*)(bo +1), driver->releasefn), where driver->releasefn has to call ttm_bo_release again. Unfortunately unless drm is fixed dma-buf is still going to be as buggy as before, not much I can do about that. :/ But this is something for a future where dma-buf in drm doesn't suck.. ~Maarten