From: Thomas Zimmermann <tzimmermann@suse.de>
To: "Christian König" <ckoenig.leichtzumerken@gmail.com>,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED
Date: Fri, 24 Jul 2020 08:50:25 +0200 [thread overview]
Message-ID: <b41781bd-3f99-85d8-e08a-20fdc8f92488@suse.de> (raw)
In-Reply-To: <20200723151710.3591-3-christian.koenig@amd.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 11969 bytes --]
Am 23.07.20 um 17:17 schrieb Christian König:
> Instead use a boolean field in the memory manager structure.
>
> Also invert the meaning of the field since the use of a TT
> structure is the special case here.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
There's a comment further below. In any case
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 +---
> drivers/gpu/drm/drm_gem_vram_helper.c | 1 -
> drivers/gpu/drm/nouveau/nouveau_bo.c | 4 +---
> drivers/gpu/drm/qxl/qxl_ttm.c | 1 -
> drivers/gpu/drm/radeon/radeon_ttm.c | 3 +--
> drivers/gpu/drm/ttm/ttm_bo.c | 14 +++++++-------
> drivers/gpu/drm/ttm/ttm_bo_util.c | 12 ++++++------
> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 3 ++-
> include/drm/ttm/ttm_bo_driver.h | 4 +---
> 9 files changed, 19 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index e57c49a91b73..406bcb03df48 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -87,15 +87,14 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> break;
> case TTM_PL_TT:
> /* GTT memory */
> + man->use_tt = true;
> man->func = &amdgpu_gtt_mgr_func;
> man->available_caching = TTM_PL_MASK_CACHING;
> man->default_caching = TTM_PL_FLAG_CACHED;
> - man->flags = 0;
> break;
> case TTM_PL_VRAM:
> /* "On-card" video ram */
> man->func = &amdgpu_vram_mgr_func;
> - man->flags = TTM_MEMTYPE_FLAG_FIXED;
> man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> man->default_caching = TTM_PL_FLAG_WC;
> break;
> @@ -104,7 +103,6 @@ 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;
> man->available_caching = TTM_PL_FLAG_UNCACHED;
> man->default_caching = TTM_PL_FLAG_UNCACHED;
> break;
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index be177afdeb9a..801a14c6e9e0 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -1012,7 +1012,6 @@ static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> break;
> case TTM_PL_VRAM:
> man->func = &ttm_bo_manager_func;
> - man->flags = TTM_MEMTYPE_FLAG_FIXED;
> man->available_caching = TTM_PL_FLAG_UNCACHED |
> TTM_PL_FLAG_WC;
> man->default_caching = TTM_PL_FLAG_WC;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 53af25020bb2..a3ad66ad3817 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -657,7 +657,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> case TTM_PL_SYSTEM:
> break;
> case TTM_PL_VRAM:
> - man->flags = TTM_MEMTYPE_FLAG_FIXED;
> man->available_caching = TTM_PL_FLAG_UNCACHED |
> TTM_PL_FLAG_WC;
> man->default_caching = TTM_PL_FLAG_WC;
> @@ -685,13 +684,12 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> else
> man->func = &ttm_bo_manager_func;
>
> + man->use_tt = true;
> if (drm->agp.bridge) {
> - man->flags = 0;
> man->available_caching = TTM_PL_FLAG_UNCACHED |
> TTM_PL_FLAG_WC;
> man->default_caching = TTM_PL_FLAG_WC;
> } else {
> - man->flags = 0;
> man->available_caching = TTM_PL_MASK_CACHING;
> man->default_caching = TTM_PL_FLAG_CACHED;
> }
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index e9b8c921c1f0..abb9fa4d80cf 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -59,7 +59,6 @@ static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> case TTM_PL_PRIV:
> /* "On-card" video ram */
> man->func = &ttm_bo_manager_func;
> - man->flags = TTM_MEMTYPE_FLAG_FIXED;
> man->available_caching = TTM_PL_MASK_CACHING;
> man->default_caching = TTM_PL_FLAG_CACHED;
> break;
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index b4cb75361577..9aba18a143e7 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -81,7 +81,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 = 0;
> + man->use_tt = true;
> #if IS_ENABLED(CONFIG_AGP)
> if (rdev->flags & RADEON_IS_AGP) {
> if (!rdev->ddev->agp) {
> @@ -98,7 +98,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> case TTM_PL_VRAM:
> /* "On-card" video ram */
> man->func = &ttm_bo_manager_func;
> - man->flags = TTM_MEMTYPE_FLAG_FIXED;
> man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> man->default_caching = TTM_PL_FLAG_WC;
> break;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 1f1f9e463265..6dea56dce350 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -84,7 +84,7 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, struct drm_printer *p
>
> drm_printf(p, " has_type: %d\n", man->has_type);
> drm_printf(p, " use_type: %d\n", man->use_type);
> - drm_printf(p, " flags: 0x%08X\n", man->flags);
> + drm_printf(p, " use_tt: %d\n", man->use_tt);
> drm_printf(p, " size: %llu\n", man->size);
> drm_printf(p, " available_caching: 0x%08X\n", man->available_caching);
> drm_printf(p, " default_caching: 0x%08X\n", man->default_caching);
> @@ -159,7 +159,7 @@ static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object *bo,
> man = &bdev->man[mem->mem_type];
> list_add_tail(&bo->lru, &man->lru[bo->priority]);
>
> - if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED) && bo->ttm &&
> + if (man->use_tt && bo->ttm &&
> !(bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
> TTM_PAGE_FLAG_SWAPPED))) {
> list_add_tail(&bo->swap, &ttm_bo_glob.swap_lru[bo->priority]);
> @@ -286,8 +286,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
> * Create and bind a ttm if required.
> */
>
> - if (!(new_man->flags & TTM_MEMTYPE_FLAG_FIXED)) {
> - bool zero = !(old_man->flags & TTM_MEMTYPE_FLAG_FIXED);
> + if (new_man->use_tt) {
> + bool zero = old_man->use_tt;
There's little use in copying to zero.
>
> ret = ttm_tt_create(bo, zero);
Maybe rather pass old_man->use_tt directly and leave a comment why that
makes sense.
Best regards
Thomas
> if (ret)
> @@ -314,8 +314,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
> if (bdev->driver->move_notify)
> bdev->driver->move_notify(bo, evict, mem);
>
> - if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) &&
> - !(new_man->flags & TTM_MEMTYPE_FLAG_FIXED))
> + if (old_man->use_tt && new_man->use_tt)
> ret = ttm_bo_move_ttm(bo, ctx, mem);
> else if (bdev->driver->move)
> ret = bdev->driver->move(bo, evict, ctx, mem);
> @@ -340,7 +339,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>
> out_err:
> new_man = &bdev->man[bo->mem.mem_type];
> - if (new_man->flags & TTM_MEMTYPE_FLAG_FIXED) {
> + if (!new_man->use_tt) {
> ttm_tt_destroy(bo->ttm);
> bo->ttm = NULL;
> }
> @@ -1677,6 +1676,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
> * Initialize the system memory buffer type.
> * Other types need to be driver / IOCTL initialized.
> */
> + bdev->man[TTM_PL_SYSTEM].use_tt = true;
> bdev->man[TTM_PL_SYSTEM].available_caching = TTM_PL_MASK_CACHING;
> bdev->man[TTM_PL_SYSTEM].default_caching = TTM_PL_FLAG_CACHED;
> ret = ttm_bo_init_mm(bdev, TTM_PL_SYSTEM, 0);
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 7fb3e0bcbab4..1f502be0b646 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -384,7 +384,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
> *old_mem = *new_mem;
> new_mem->mm_node = NULL;
>
> - if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
> + if (!man->use_tt) {
> ttm_tt_destroy(ttm);
> bo->ttm = NULL;
> }
> @@ -645,7 +645,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
> if (ret)
> return ret;
>
> - if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
> + if (!man->use_tt) {
> ttm_tt_destroy(bo->ttm);
> bo->ttm = NULL;
> }
> @@ -674,7 +674,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
> * bo to be unbound and destroyed.
> */
>
> - if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED))
> + if (man->use_tt)
> ghost_obj->ttm = NULL;
> else
> bo->ttm = NULL;
> @@ -730,7 +730,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
> * bo to be unbound and destroyed.
> */
>
> - if (!(to->flags & TTM_MEMTYPE_FLAG_FIXED))
> + if (to->use_tt)
> ghost_obj->ttm = NULL;
> else
> bo->ttm = NULL;
> @@ -738,7 +738,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
> dma_resv_unlock(&ghost_obj->base._resv);
> ttm_bo_put(ghost_obj);
>
> - } else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
> + } else if (!from->use_tt) {
>
> /**
> * BO doesn't have a TTM we need to bind/unbind. Just remember
> @@ -768,7 +768,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
> if (ret)
> return ret;
>
> - if (to->flags & TTM_MEMTYPE_FLAG_FIXED) {
> + if (!to->use_tt) {
> ttm_tt_destroy(bo->ttm);
> bo->ttm = NULL;
> }
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index 00cef1a3a178..5d8179d9f394 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -746,7 +746,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> case TTM_PL_VRAM:
> /* "On-card" video ram */
> man->func = &vmw_thp_func;
> - man->flags = TTM_MEMTYPE_FLAG_FIXED;
> man->available_caching = TTM_PL_FLAG_CACHED;
> man->default_caching = TTM_PL_FLAG_CACHED;
> break;
> @@ -760,6 +759,8 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> man->func = &vmw_gmrid_manager_func;
> man->available_caching = TTM_PL_FLAG_CACHED;
> man->default_caching = TTM_PL_FLAG_CACHED;
> + /* TODO: This is most likely not correct */
> + man->use_tt = true;
> break;
> default:
> DRM_ERROR("Unsupported memory type %u\n", (unsigned)type);
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 9b251853afe2..adac4cd0ba23 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -45,8 +45,6 @@
>
> #define TTM_MAX_BO_PRIORITY 4U
>
> -#define TTM_MEMTYPE_FLAG_FIXED (1 << 0) /* Fixed (on-card) PCI memory */
> -
> struct ttm_mem_type_manager;
>
> struct ttm_mem_type_manager_func {
> @@ -173,7 +171,7 @@ struct ttm_mem_type_manager {
>
> bool has_type;
> bool use_type;
> - uint32_t flags;
> + bool use_tt;
> uint64_t size;
> uint32_t available_caching;
> uint32_t default_caching;
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-07-24 6:50 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-23 15:17 More TTM cleanups Christian König
2020-07-23 15:17 ` [PATCH 1/9] drm/ttm: initialize the system domain with defaults Christian König
2020-07-24 6:43 ` Thomas Zimmermann
2020-07-29 6:21 ` Dave Airlie
2020-07-29 6:23 ` Dave Airlie
2020-07-29 9:47 ` Christian König
2020-07-23 15:17 ` [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED Christian König
2020-07-24 6:50 ` Thomas Zimmermann [this message]
2020-07-24 7:27 ` Christian König
2020-07-23 15:17 ` [PATCH 3/9] drm/radeon: stop implementing init_mem_type Christian König
2020-07-23 15:17 ` [PATCH 4/9] drm/amdgpu: " Christian König
2020-07-23 15:17 ` [PATCH 5/9] drm/vmwgfx: " Christian König
2020-07-23 15:17 ` [PATCH 6/9] drm/nouveau: " Christian König
2020-07-23 15:17 ` [PATCH 7/9] drm/qxl: " Christian König
2020-07-23 15:17 ` [PATCH 8/9] drm/vram-helper: " Christian König
2020-07-24 6:51 ` Thomas Zimmermann
2020-07-23 15:17 ` [PATCH 9/9] drm/ttm: remove the init_mem_type callback Christian König
2020-07-23 20:31 ` Alex Deucher
2020-07-24 6:51 ` Thomas Zimmermann
-- strict thread matches above, loose matches on Subject: below --
2020-07-23 15:16 [PATCH 1/9] drm/ttm: initialize the system domain with defaults Christian König
2020-07-23 15:16 ` [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED Christian König
2020-07-27 9:48 ` daniel
2020-07-27 9:54 ` Christian König
2020-07-27 10:27 ` daniel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b41781bd-3f99-85d8-e08a-20fdc8f92488@suse.de \
--to=tzimmermann@suse.de \
--cc=ckoenig.leichtzumerken@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.