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

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

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