Hi,
> Yeah, my point was not really suggesting that we do this, but rather that
> people would rightfully get upset because the struct contains unused stuff.
Well, struct drm_gem_object isn't that big, lets have a look:
320 bytes in total, of which are:
184 bytes the embedded vma_mode
64 bytes the embedded _resv struct
8 bytes the resv pointer.
64 bytes everything else.
So, after applying this series it's 64 bytes more per bo for vmwgfx,
due to some unused fields being added.
And it's 256 bytes less per bo for all ttm+gem drivers, because they
don't have vma_mode + resv struct + resv pointer twice any more.
And that is just the low-hanging fruit, there is room for more
ttm_buffer_object elements being removed by using the drm_gem_object
struct elements instead. num_pages, kref, maybe more.
> Also a trap we might end up with in the future is that with the design
> suggested in this patch series is that people start assuming that the
> embedded gem object is actually initialized and working, which could lead to
> pretty severe problems for vmwgfx...
I guess I should reword patch #1 then, making clear that the
ttm_bo_uses_embedded_gem_object() helper function is going to stay.
What is the reason for vmwgfx to not use gem btw?
> > for all the ttm+gem drivers, one pointer they don't need twice). With
> > the side-by-side, which is the design all gem+ttm drivers used the
> > past few years, we still need that duplication. Same for the vma node
> > thing, which is also duplicated.
>
> To bemore precise I'd probably define a
>
> struct drm_bo_common {
> struct reservation_object r;
> struct drm_vma_node v;
> };
>
> Embed it in a struct drm_gem_object (and in a struct vmwgfx_buffer_object)
> and then have a pointer to a struct drm_bo_common in the struct
> ttm_buffer_object.
A pointer doesn't cut it.
Beside removing the duplication the other thing I want is to have a
standard way of finding the ttm_buffer_object for a given drm_gem_object
for all the ttm+gem drivers. With struct drm_gem_object being embedded
the usual containter_of() will work.
That'll allow to create drm_gem_ttm_helper.c with helper functions for
struct drm_gem_object_funcs. For starters some of the current vram
helpers can become generic ttm helpers because they loose the dependency
on struct drm_gem_vram_object for finding ttm_buffer_object.
> > > The vmwgfx driver is doing what it does mostly because all buffer
> > > objects do not need to be user-space visible, and do not need to be
> > > mapped by user-space. And there are other types of objects that DO need
> > > to be user-space visible, and that do need to be shared by processes.
> > > Hence user-space visibility is something that should be abstracted and
> > > made available to those objects. Not lumped together with all other
> > > potential buffer object functionality.
Well, ttm_buffer_object has a vma_node embedded, so it already is
"lumped together". This patch series only moves it around ...
cheers,
Gerd
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
