[PATCH 0/3] Provide struct ttm_global for TTM global state

[Koenig, Christian]

Am 19.10.18 um 09:44 schrieb Thomas Zimmermann:
> Hi
>
> Am 19.10.18 um 09:24 schrieb Koenig, Christian:
>> Hi Thomas,
>>
>> I honestly still find that way to complicated compared to what it
>> actually does.
>>
>> Both ttm_mem_global and ttm_bo_global can just be some static variables.
>> E.g. the whole handling with drm_global_reference is just superfluous.
>>
>> Can I just write a patch to clean up the mess we created?
> 'Can I?' I don't know :)
>
> Sure, it's a mess. For example, if a driver inits drm_global_reference()
> and is later unloaded, there are dangling function pointers to the
> driver's code.
>
> I do a slow clean-up of the code because TTM-based drivers depend on the
> current way of doing things.
>
> Maybe let me post my full patch set for RFC and discuss this instead.

Yeah, the key point is that this is a lot of stuff to review while the 
end result is we just trow away most of it.

I would rather just go ahead and do a few simple patches instead:
1. Use only a single instance of ttm_mem_global in ttm_memory.c and 
remove all "global" parameters from the functions.
2. Use only a single instance of ttm_bo_global in ttm_bo.c.
3. Remove bdev->global.

The *_global_init()/*_global_release() should just increase/decrease a 
static reference count protected by a static lock.

Regards,
Christian.

>
> Best regards
> Thomas
>
>
>> Regards,
>> Christian.
>>
>> Am 18.10.18 um 18:27 schrieb Thomas Zimmermann:
>>> TTM provides global memory and a global BO that is shared by all
>>> TTM-based drivers. The data structures are provided by struct drm_global
>>> and its helpers. All TTM-based DRM drivers copy the initialization and
>>> clean-up code for the global TTM state from each other; leading to code
>>> duplication.
>>>
>>> The new structure struct ttm_global and its helpers provide a unified
>>> implementation. Drivers only have to initialize it and forward the
>>> contained BO global object to ttm_bo_device_init().
>>>
>>> The amdgpu and radeon drivers are converted to struct ttm_global as
>>> part of this patch set. All other TTM-based drivers share exactly the
>>> same code patterns and can be converted in the same way.
>>>
>>> Future directions: I already have patches for converting the remaining
>>> TTM drivers to struct ttm_global. These patches can be merged after
>>> the structure has become available in upstream. struct ttm_global is
>>> implemented on top of struct drm_global. The latter actually maintains
>>> TTM state instead of DRM state. Once all drivers have been converted,
>>>       the code for struct drm_global can be merged into struct ttm_global.
>>>
>>> (Resending this patch set, as I forgot to CC the mailing lists as first.)
>>>
>>> Thomas Zimmermann (3):
>>>     drm/ttm: Provide struct ttm_global for referencing TTM global state
>>>     drm/amdgpu: Replace TTM initialization/release with ttm_global
>>>     drm/radeon: Replace TTM initialization/release with ttm_global
>>>
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 63 ++--------------
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  4 +-
>>>    drivers/gpu/drm/radeon/radeon.h         |  4 +-
>>>    drivers/gpu/drm/radeon/radeon_ttm.c     | 40 ++--------
>>>    drivers/gpu/drm/ttm/Makefile            |  2 +-
>>>    drivers/gpu/drm/ttm/ttm_global.c        | 98 +++++++++++++++++++++++++
>>>    include/drm/drm_global.h                |  8 ++
>>>    include/drm/ttm/ttm_global.h            | 79 ++++++++++++++++++++
>>>    8 files changed, 200 insertions(+), 98 deletions(-)
>>>    create mode 100644 drivers/gpu/drm/ttm/ttm_global.c
>>>    create mode 100644 include/drm/ttm/ttm_global.h
>>>
>>> --
>>> 2.19.1
>>>