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

[Thomas Hellstrom]

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