thr3ads.net - Nouveau - [Nouveau] [PATCH 0/2] Provide init/release functions for struct ttm_bo

If this information is useful, please help other people find it:
Share via:

Thomas Zimmermann

2018-Aug-30 06:34 UTC

[Nouveau] [PATCH 0/2] Provide init/release functions for struct ttm_bo_global

Hi Christian,

I just wanted to ask if there's something else required to get this
patch set reviewed and landed.

On top of these two patches, I have a patch set that replaces the
driver-specific global-bo-and-mem combos with a single struct ttm_global
structure. It further merges struct drm_global into struct ttm_global
and implements it such that drivers can either share the global memory
or create their private instance. It's also a step towards drm device
hotplug, which someone just asked.

Best regards
Thomas


Am 13.08.2018 um 12:33 schrieb Christian König:> Yes, please! I had it on my TODO list to clean that up for an eternity.
> 
> Actually I never understood why that should be driver work to setup TTM?
> 
> I mean can't we just have a module_init/module_exit for TTM?
> 
> Thanks,
> Christian.
> 
> Am 13.08.2018 um 12:24 schrieb Thomas Zimmermann:
>> TTM uses global memory and BO for backing graphics buffers. These are
>> represented by struct ttm_mem_global and struct ttm_bo_global.
>>
>> Currently, struct ttm_bo_global can only be initialized and released
>> through
>> struct ttm_bo_global_ref. This is a workaround for passing an instance
of
>> ttm_mem_global to the BO global initialization code.
>>
>> The use of struct ttm_bo_global_ref makes driver code unnecessary hard
to
>> understand. At the same time drivers can use any combination of memory
>> and
>> BO for initializing the global instances. This can result in subtle
bugs
>> when the order of initializing and releasing drivers changes.
>>
>> As a first step for resolving these problems, the provided patch set
>> separates initialization and release of struct ttm_bo_global from
>> struct ttm_bo_global_ref.
>>
>> The first patch only renames ttm_bo_global_{init/release}. Hopefully
this
>> change can be applied at once for all drivers.
>>
>> Future directions: All TTM-based drivers follow the same pattern for
>> setting
>> up the TTM. In a follow-up patch, this code can be moved into a single
>> place
>> and shared among drivers.
>>
>> Thomas Zimmermann (2):
>>    drm/ttm: Rename ttm_bo_global_{init,release}() to
>>      ttm_bo_global_ref_*()
>>    drm/ttm: Provide ttm_bo_global_{init/release}() for struct
>>      ttm_bo_global
>>
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  4 +-
>>   drivers/gpu/drm/ast/ast_ttm.c               |  4 +-
>>   drivers/gpu/drm/bochs/bochs_mm.c            |  4 +-
>>   drivers/gpu/drm/cirrus/cirrus_ttm.c         |  4 +-
>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c |  4 +-
>>   drivers/gpu/drm/mgag200/mgag200_ttm.c       |  4 +-
>>   drivers/gpu/drm/nouveau/nouveau_ttm.c       |  4 +-
>>   drivers/gpu/drm/qxl/qxl_ttm.c               |  4 +-
>>   drivers/gpu/drm/radeon/radeon_ttm.c         |  4 +-
>>   drivers/gpu/drm/ttm/ttm_bo.c                | 12 ++---
>>   drivers/gpu/drm/virtio/virtgpu_ttm.c        |  4 +-
>>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c    |  4 +-
>>   drivers/staging/vboxvideo/vbox_ttm.c        |  4 +-
>>   include/drm/ttm/ttm_bo_driver.h             | 53
++++++++++++++++-----
>>   14 files changed, 70 insertions(+), 43 deletions(-)
>>
>> -- 
>> 2.18.0
>>
> 
-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstr. 5, D-90409 Nürnberg
Tel: +49-911-74053-0; Fax: +49-911-7417755;  https://www.suse.com/
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard,
Graham Norton, HRB 21284 (AG Nürnberg)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20180830/79a2028c/attachment-0001.sig>

Christian König

2018-Aug-30 06:45 UTC

head link

[Nouveau] [PATCH 0/2] Provide init/release functions for struct ttm_bo_global

Hi Thomas,

Am 30.08.2018 um 08:34 schrieb Thomas Zimmermann:> Hi Christian,
>
> I just wanted to ask if there's something else required to get this
> patch set reviewed and landed.
I just need to find some time to review them.
> On top of these two patches, I have a patch set that replaces the
> driver-specific global-bo-and-mem combos with a single struct ttm_global
> structure. It further merges struct drm_global into struct ttm_global
> and implements it such that drivers can either share the global memory
That sounds good.
> or create their private instance.
That doesn't sounds good. Drivers should not be allowed to create their 
own private instance of that.

Thanks for doing this,
Christian.
>   It's also a step towards drm device
> hotplug, which someone just asked.
>
> Best regards
> Thomas
>
>
> Am 13.08.2018 um 12:33 schrieb Christian König:
>> Yes, please! I had it on my TODO list to clean that up for an eternity.
>>
>> Actually I never understood why that should be driver work to setup
TTM?
>>
>> I mean can't we just have a module_init/module_exit for TTM?
>>
>> Thanks,
>> Christian.
>>
>> Am 13.08.2018 um 12:24 schrieb Thomas Zimmermann:
>>> TTM uses global memory and BO for backing graphics buffers. These
are
>>> represented by struct ttm_mem_global and struct ttm_bo_global.
>>>
>>> Currently, struct ttm_bo_global can only be initialized and
released
>>> through
>>> struct ttm_bo_global_ref. This is a workaround for passing an
instance of
>>> ttm_mem_global to the BO global initialization code.
>>>
>>> The use of struct ttm_bo_global_ref makes driver code unnecessary
hard to
>>> understand. At the same time drivers can use any combination of
memory
>>> and
>>> BO for initializing the global instances. This can result in subtle
bugs
>>> when the order of initializing and releasing drivers changes.
>>>
>>> As a first step for resolving these problems, the provided patch
set
>>> separates initialization and release of struct ttm_bo_global from
>>> struct ttm_bo_global_ref.
>>>
>>> The first patch only renames ttm_bo_global_{init/release}.
Hopefully this
>>> change can be applied at once for all drivers.
>>>
>>> Future directions: All TTM-based drivers follow the same pattern
for
>>> setting
>>> up the TTM. In a follow-up patch, this code can be moved into a
single
>>> place
>>> and shared among drivers.
>>>
>>> Thomas Zimmermann (2):
>>>     drm/ttm: Rename ttm_bo_global_{init,release}() to
>>>       ttm_bo_global_ref_*()
>>>     drm/ttm: Provide ttm_bo_global_{init/release}() for struct
>>>       ttm_bo_global
>>>
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  4 +-
>>>    drivers/gpu/drm/ast/ast_ttm.c               |  4 +-
>>>    drivers/gpu/drm/bochs/bochs_mm.c            |  4 +-
>>>    drivers/gpu/drm/cirrus/cirrus_ttm.c         |  4 +-
>>>    drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c |  4 +-
>>>    drivers/gpu/drm/mgag200/mgag200_ttm.c       |  4 +-
>>>    drivers/gpu/drm/nouveau/nouveau_ttm.c       |  4 +-
>>>    drivers/gpu/drm/qxl/qxl_ttm.c               |  4 +-
>>>    drivers/gpu/drm/radeon/radeon_ttm.c         |  4 +-
>>>    drivers/gpu/drm/ttm/ttm_bo.c                | 12 ++---
>>>    drivers/gpu/drm/virtio/virtgpu_ttm.c        |  4 +-
>>>    drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c    |  4 +-
>>>    drivers/staging/vboxvideo/vbox_ttm.c        |  4 +-
>>>    include/drm/ttm/ttm_bo_driver.h             | 53
++++++++++++++++-----
>>>    14 files changed, 70 insertions(+), 43 deletions(-)
>>>
>>> -- 
>>> 2.18.0
>>>

Thomas Zimmermann

2018-Aug-30 10:35 UTC

head link

[Nouveau] [PATCH 0/2] Provide init/release functions for struct ttm_bo_global

Am 30.08.2018 um 08:45 schrieb Christian König:
[...]>> or create their private instance.
> 
> That doesn't sounds good. Drivers should not be allowed to create their
> own private instance of that.
OK, will be changed in the patchset.

Best regards
Thomas
> 
> Thanks for doing this,
> Christian.
> 
>>   It's also a step towards drm device
>> hotplug, which someone just asked.
>>
>> Best regards
>> Thomas
>>
>>
>> Am 13.08.2018 um 12:33 schrieb Christian König:
>>> Yes, please! I had it on my TODO list to clean that up for an
eternity.
>>>
>>> Actually I never understood why that should be driver work to setup
TTM?
>>>
>>> I mean can't we just have a module_init/module_exit for TTM?
>>>
>>> Thanks,
>>> Christian.
>>>
>>> Am 13.08.2018 um 12:24 schrieb Thomas Zimmermann:
>>>> TTM uses global memory and BO for backing graphics buffers.
These are
>>>> represented by struct ttm_mem_global and struct ttm_bo_global.
>>>>
>>>> Currently, struct ttm_bo_global can only be initialized and
released
>>>> through
>>>> struct ttm_bo_global_ref. This is a workaround for passing an
>>>> instance of
>>>> ttm_mem_global to the BO global initialization code.
>>>>
>>>> The use of struct ttm_bo_global_ref makes driver code
unnecessary
>>>> hard to
>>>> understand. At the same time drivers can use any combination of
memory
>>>> and
>>>> BO for initializing the global instances. This can result in
subtle
>>>> bugs
>>>> when the order of initializing and releasing drivers changes.
>>>>
>>>> As a first step for resolving these problems, the provided
patch set
>>>> separates initialization and release of struct ttm_bo_global
from
>>>> struct ttm_bo_global_ref.
>>>>
>>>> The first patch only renames ttm_bo_global_{init/release}.
Hopefully
>>>> this
>>>> change can be applied at once for all drivers.
>>>>
>>>> Future directions: All TTM-based drivers follow the same
pattern for
>>>> setting
>>>> up the TTM. In a follow-up patch, this code can be moved into a
single
>>>> place
>>>> and shared among drivers.
>>>>
>>>> Thomas Zimmermann (2):
>>>>     drm/ttm: Rename ttm_bo_global_{init,release}() to
>>>>       ttm_bo_global_ref_*()
>>>>     drm/ttm: Provide ttm_bo_global_{init/release}() for struct
>>>>       ttm_bo_global
>>>>
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  4 +-
>>>>    drivers/gpu/drm/ast/ast_ttm.c               |  4 +-
>>>>    drivers/gpu/drm/bochs/bochs_mm.c            |  4 +-
>>>>    drivers/gpu/drm/cirrus/cirrus_ttm.c         |  4 +-
>>>>    drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c |  4 +-
>>>>    drivers/gpu/drm/mgag200/mgag200_ttm.c       |  4 +-
>>>>    drivers/gpu/drm/nouveau/nouveau_ttm.c       |  4 +-
>>>>    drivers/gpu/drm/qxl/qxl_ttm.c               |  4 +-
>>>>    drivers/gpu/drm/radeon/radeon_ttm.c         |  4 +-
>>>>    drivers/gpu/drm/ttm/ttm_bo.c                | 12 ++---
>>>>    drivers/gpu/drm/virtio/virtgpu_ttm.c        |  4 +-
>>>>    drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c    |  4 +-
>>>>    drivers/staging/vboxvideo/vbox_ttm.c        |  4 +-
>>>>    include/drm/ttm/ttm_bo_driver.h             | 53
>>>> ++++++++++++++++-----
>>>>    14 files changed, 70 insertions(+), 43 deletions(-)
>>>>
>>>> -- 
>>>> 2.18.0
>>>>
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstr. 5, D-90409 Nürnberg
Tel: +49-911-74053-0; Fax: +49-911-7417755;  https://www.suse.com/
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard,
Graham Norton, HRB 21284 (AG Nürnberg)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20180830/acb2ee84/attachment-0001.sig>

Possibly Parallel Threads

Search for more seemingly similar threads

Nouveau - Aug 2018 - [PATCH 0/2] Provide init/release functions for struct ttm_bo_global

[Nouveau] [PATCH 0/2] Provide init/release functions for struct ttm_bo_global

[Nouveau] [PATCH 0/2] Provide init/release functions for struct ttm_bo_global

[Nouveau] [PATCH 0/2] Provide init/release functions for struct ttm_bo_global

Possibly Parallel Threads