[PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA
Christian König
ckoenig.leichtzumerken at gmail.com
Mon Jul 20 11:10:50 UTC 2020
Am 16.07.20 um 15:05 schrieb Thomas Zimmermann:
> Hi,
>
> this patchset could have benefited from a cover letter.
Yeah, I didn't thought it would result in such an extensive patch set.
> Am 16.07.20 um 14:50 schrieb Christian König:
>> The original intention was to avoid CPU page table unmaps
>> when BOs move between the GTT and SYSTEM domain.
>>
>> The problem is that this never correctly handled changes
>> in the caching attributes or backing pages.
>>
>> Just drop this for now and simply unmap the CPU page
>> tables in all cases.
>>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 +--
>> drivers/gpu/drm/nouveau/nouveau_bo.c | 3 +-
>> drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
>> drivers/gpu/drm/ttm/ttm_bo.c | 34 ++++------------------
>> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 +-
>> include/drm/ttm/ttm_bo_driver.h | 1 -
>> 6 files changed, 11 insertions(+), 35 deletions(-)
> Why's CMA cleaned up in one patch and MAPPABLE in the other 7?
There is a chance that the CMA removal fixes some bugs and needs to be
back-ported. But the other 7 are pure cleanup.
> Wouldn't it have been better to kill both the flags from ttm in 2
> patches, then have one patch per driver and finally remove both flags
> from the ttm header?
That's what I did initially. Basically I found that the
TTM_MEMTYPE_FLAG_CMA flag might be the root of some problems because of
the problematic implementation.
While removing it I've found that the TTM_MEMTYPE_FLAG_MAPPABLE is
completely unused in TTM as well, so I removed both at the same time.
But that turned out to break drivers because they use the
TTM_MEMTYPE_FLAG_MAPPABLE flag for no good reason at all :)
Anyway, I want to keep the order now since it might be a good idea to
back port the CMA flag removal (but I'm not 100% sure of that).
Thanks,
Christian.
>
> Best regards
> Thomas
>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 9c0f12f74af9..44fa8bc49d18 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -93,7 +93,7 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>> man->func = &amdgpu_gtt_mgr_func;
>> man->available_caching = TTM_PL_MASK_CACHING;
>> man->default_caching = TTM_PL_FLAG_CACHED;
>> - man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
>> + man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>> break;
>> case TTM_PL_VRAM:
>> /* "On-card" video ram */
>> @@ -108,7 +108,7 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>> case AMDGPU_PL_OA:
>> /* On-chip GDS memory*/
>> man->func = &ttm_bo_manager_func;
>> - man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_CMA;
>> + man->flags = TTM_MEMTYPE_FLAG_FIXED;
>> man->available_caching = TTM_PL_FLAG_UNCACHED;
>> man->default_caching = TTM_PL_FLAG_UNCACHED;
>> break;
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
>> index a1037478fa3f..7883341f8c83 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
>> @@ -695,8 +695,7 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>> TTM_PL_FLAG_WC;
>> man->default_caching = TTM_PL_FLAG_WC;
>> } else {
>> - man->flags = TTM_MEMTYPE_FLAG_MAPPABLE |
>> - TTM_MEMTYPE_FLAG_CMA;
>> + man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>> man->available_caching = TTM_PL_MASK_CACHING;
>> man->default_caching = TTM_PL_FLAG_CACHED;
>> }
>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
>> index 73085523fad7..54af06df865b 100644
>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
>> @@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>> man->func = &ttm_bo_manager_func;
>> man->available_caching = TTM_PL_MASK_CACHING;
>> man->default_caching = TTM_PL_FLAG_CACHED;
>> - man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
>> + man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>> #if IS_ENABLED(CONFIG_AGP)
>> if (rdev->flags & RADEON_IS_AGP) {
>> if (!rdev->ddev->agp) {
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 8b9e7f62bea7..0768a054a916 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -272,20 +272,15 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>> struct ttm_operation_ctx *ctx)
>> {
>> struct ttm_bo_device *bdev = bo->bdev;
>> - bool old_is_pci = ttm_mem_reg_is_pci(bdev, &bo->mem);
>> - bool new_is_pci = ttm_mem_reg_is_pci(bdev, mem);
>> struct ttm_mem_type_manager *old_man = &bdev->man[bo->mem.mem_type];
>> struct ttm_mem_type_manager *new_man = &bdev->man[mem->mem_type];
>> - int ret = 0;
>> + int ret;
>>
>> - if (old_is_pci || new_is_pci ||
>> - ((mem->placement & bo->mem.placement & TTM_PL_MASK_CACHING) == 0)) {
>> - ret = ttm_mem_io_lock(old_man, true);
>> - if (unlikely(ret != 0))
>> - goto out_err;
>> - ttm_bo_unmap_virtual_locked(bo);
>> - ttm_mem_io_unlock(old_man);
>> - }
>> + ret = ttm_mem_io_lock(old_man, true);
>> + if (unlikely(ret != 0))
>> + goto out_err;
>> + ttm_bo_unmap_virtual_locked(bo);
>> + ttm_mem_io_unlock(old_man);
>>
>> /*
>> * Create and bind a ttm if required.
>> @@ -1698,23 +1693,6 @@ EXPORT_SYMBOL(ttm_bo_device_init);
>> * buffer object vm functions.
>> */
>>
>> -bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
>> -{
>> - struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>> -
>> - if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED)) {
>> - if (mem->mem_type == TTM_PL_SYSTEM)
>> - return false;
>> -
>> - if (man->flags & TTM_MEMTYPE_FLAG_CMA)
>> - return false;
>> -
>> - if (mem->placement & TTM_PL_FLAG_CACHED)
>> - return false;
>> - }
>> - return true;
>> -}
>> -
>> void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo)
>> {
>> struct ttm_bo_device *bdev = bo->bdev;
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>> index bfd0c54ec30a..6bea7548aee0 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>> @@ -762,7 +762,7 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>> * slots as well as the bo size.
>> */
>> man->func = &vmw_gmrid_manager_func;
>> - man->flags = TTM_MEMTYPE_FLAG_CMA | TTM_MEMTYPE_FLAG_MAPPABLE;
>> + man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>> man->available_caching = TTM_PL_FLAG_CACHED;
>> man->default_caching = TTM_PL_FLAG_CACHED;
>> break;
>> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
>> index 45522e4fbd6b..71b195e78c7c 100644
>> --- a/include/drm/ttm/ttm_bo_driver.h
>> +++ b/include/drm/ttm/ttm_bo_driver.h
>> @@ -47,7 +47,6 @@
>>
>> #define TTM_MEMTYPE_FLAG_FIXED (1 << 0) /* Fixed (on-card) PCI memory */
>> #define TTM_MEMTYPE_FLAG_MAPPABLE (1 << 1) /* Memory mappable */
>> -#define TTM_MEMTYPE_FLAG_CMA (1 << 3) /* Can't map aperture */
>>
>> struct ttm_mem_type_manager;
>>
>>
More information about the dri-devel
mailing list