Re: [PATCH 6/7] drm/nouveau: embed gem object in nouveau_bo

[Maarten Lankhorst <maarten.lankhorst@canonical.com>]

Op 15-08-13 14:44, Daniel Vetter schreef:
> On Thu, Aug 15, 2013 at 2:32 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>>> 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..
>> This same issue came up with the vma-access-management patches. I
>> dislike adding hacks to TTM to allow accessing the surrounding GEM
>> object only to keep TTM gem-agnostic. We could fix all this with a
>> simple gem_object* pointer in the ttm-bo (or a flag indicating
>> container_of is valid).
>>
>> Or we make gem TTM-aware by optionally embedding either a ttm_bo or a
>> kref object. Hm. Ideas welcome!
Oh original patch:

Acked-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>

I think with this series the normal ttm refcount could be removed entirely. The only place in ttm that
uses it is ttm_bo_mmap and ttm_bo_init. If struct vm_operations_struct is set by the driver the refcount
isn't required in ttm any more at all. The other places in core ttm that use refcount now can be cleaned up.

Radeon was already overriding the mmap ops for ttm, so this could now be done in a cleaner way. :-)

That leaves vmwgfx of course, I'll probably kill ttm_bo_create because that will no longer work without refcount,
and fixup their code.

ttm_bo_io and ttm_bo_fbdev_io can be removed entirely, nothing uses it (I'll send a patch for it).
> Imo embedding a gem bo into a ttm bo should be too harmful. I think we
> already have funny lifetime issues around
> ttm_bo->presistent_swap_storage, so this would imo be a win. Four
> downsides afaik:
> - gem shows up in drm/ttm/ ;-)
> - i915 gem crap like the read/write domains get splattered even more
> around. We could just move them to the i915 gem object struct and call
> it a day I think.
> - vmwgfx doesn't expose a gem interface. Imo this doesn't apply any
> more since we have support for driver private gem objects since a
> while. So shouldn't be too hard to do the final untangling and allow
> non-gem drivers to init gem objects, but not link them up anywhere.
> - gem_object_unref uses dev->struct_mutex. The last reason for that is
> the vma manager (which can be fixed to use the same
> kref_get_unless_zero trick ttm uses) and then the locking pushed down
> into drivers (probably needs a deferred free list to get going).
It's not required to move ttm to gem to kill the refcount. :-)

~Maarten