Re: [PATCH v2 00/18] drm/ttm: make ttm bo a gem bo subclass

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Daniel,

On 6/21/19 5:57 PM, Daniel Vetter wrote:
On Fri, Jun 21, 2019 at 05:12:19PM +0200, Thomas Hellström (VMware) wrote:

On 6/21/19 1:57 PM, Gerd Hoffmann wrote:

Aargh. Please don't do this. Multiple reasons:

1) I think It's bad to dump all buffer object functionality we can possibly
think of in a single struct and force that on all (well at least most)
users. It's better to isolate functionality in structs, have utility
functions for those and let the drivers derive their buffer objects from
whatever functionality they actually need.
2) vmwgfx is not using gem and we don't want to carry that extra payload in
the buffer object.
3) TTM historically hasn't been using the various drm layers except for
later when common helpers have been used, (like the vma manager and the
cache utilities). It's desirable to keep that layer distinction. (which is
really what I'm saying in 1.)

Now if more and more functionality that originated in TTM is moving into GEM
we need to find a better way to do that without duplicating functionality. I
suggest adding pointers in the TTM structs and defaulting those pointers to
the member in the TTM struct. Optionally to to the member in the GEM struct.
If we need to migrate those members out of the TTM struct, vmwgfx would have
to provide them in its own buffer class.

NAK from the vmwgfx side.
It's 59 DRIVER_GEM vs 1 which is not. I think the verdict is clear what
the reasonable thing to do is here, and this will allow us to
substantially improve code and concept sharing across drm drivers.

10 years ago it was indeed not clear whether everyone doing the same is a
bright idea, but that's no more. If you want I guess you can keep a
private copy of ttm in vmwgfx, but not sure that's really worth it
long-term.
-Daniel

It's not a question about whether GEM or TTM, or even the number of drivers using one or the other. (GEM would actually be a good choice for the latter vmwgfx device versions). But this is going against all recent effort to make different parts of drm functionality recently self-contained.

Just stop and think for a while what would happen if someone would suggest doing the opposite: making a gem object a derived class of a TTM object, arguing that a lot of GEM drivers are using TTM as a backend. There would probably be a lot of people claiming "we don't want to unnecesarily carry that stuff". That's because that would also be a poor design.

What I'm suggesting is, build that improved code and concept sharing around

struct gem_ttm_object {
   struct gem_object;
   struct ttm_object;
};

And lets work toghether to eliminate what's duplicated.

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.

/Thomas




_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux