From: "Ruhl, Michael J" <michael.j.ruhl@intel.com>
To: "Christian König" <ckoenig.leichtzumerken@gmail.com>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
"ray.huang@amd.com" <ray.huang@amd.com>,
"airlied@gmail.com" <airlied@gmail.com>,
"daniel@ffwll.ch" <daniel@ffwll.ch>
Subject: RE: [PATCH 4/8] drm/ttm: rename TTM caching enum
Date: Mon, 5 Oct 2020 15:05:13 +0000 [thread overview]
Message-ID: <d73b28c453454cf981496b2f28b848de@intel.com> (raw)
In-Reply-To: <20201001112817.20967-4-christian.koenig@amd.com>
>-----Original Message-----
>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>Christian König
>Sent: Thursday, October 1, 2020 7:28 AM
>To: dri-devel@lists.freedesktop.org; ray.huang@amd.com;
>airlied@gmail.com; daniel@ffwll.ch
>Subject: [PATCH 4/8] drm/ttm: rename TTM caching enum
>
>Cleanup the enum name and its values and give it a separate header file.
This clean up looks straight forward.
I am not really clear on why you are giving it a separate header. If you could
detail that a little more in the commit?
Otherwise:
Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
m
>Signed-off-by: Christian König <christian.koenig@amd.com>
>---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
> drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
> drivers/gpu/drm/ttm/ttm_page_alloc.c | 26 +++++++++---------
> drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 20 +++++++-------
> drivers/gpu/drm/ttm/ttm_tt.c | 17 ++++++------
> include/drm/ttm/ttm_caching.h | 34 ++++++++++++++++++++++++
> include/drm/ttm/ttm_tt.h | 9 ++-----
> 8 files changed, 70 insertions(+), 42 deletions(-)
> create mode 100644 include/drm/ttm/ttm_caching.h
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>index 213ef090bb0e..3c5ad69eff19 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>@@ -124,7 +124,7 @@ uint64_t amdgpu_gmc_agp_addr(struct
>ttm_buffer_object *bo)
> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
> struct ttm_dma_tt *ttm;
>
>- if (bo->num_pages != 1 || bo->ttm->caching_state == tt_cached)
>+ if (bo->num_pages != 1 || bo->ttm->caching == ttm_cached)
> return AMDGPU_BO_INVALID_OFFSET;
>
> ttm = container_of(bo->ttm, struct ttm_dma_tt, ttm);
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>index 399961035ae6..2af24cf39e94 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>@@ -1525,7 +1525,7 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt
>*ttm, struct ttm_resource *mem)
> if (mem && mem->mem_type == TTM_PL_TT) {
> flags |= AMDGPU_PTE_SYSTEM;
>
>- if (ttm->caching_state == tt_cached)
>+ if (ttm->caching == ttm_cached)
> flags |= AMDGPU_PTE_SNOOPED;
> }
>
>diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
>b/drivers/gpu/drm/radeon/radeon_ttm.c
>index 63e38b05a5bc..f01e74113f40 100644
>--- a/drivers/gpu/drm/radeon/radeon_ttm.c
>+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
>@@ -546,7 +546,7 @@ static int radeon_ttm_backend_bind(struct
>ttm_bo_device *bdev,
> WARN(1, "nothing to bind %lu pages for mreg %p back
>%p!\n",
> ttm->num_pages, bo_mem, ttm);
> }
>- if (ttm->caching_state == tt_cached)
>+ if (ttm->caching == ttm_cached)
> flags |= RADEON_GART_PAGE_SNOOP;
> r = radeon_gart_bind(rdev, gtt->offset, ttm->num_pages,
> ttm->pages, gtt->ttm.dma_address, flags);
>diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c
>b/drivers/gpu/drm/ttm/ttm_page_alloc.c
>index 111031cbb6df..c8f6790962b9 100644
>--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
>+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
>@@ -220,14 +220,14 @@ static struct ttm_pool_manager *_manager;
> /**
> * Select the right pool or requested caching state and ttm flags. */
> static struct ttm_page_pool *ttm_get_pool(int flags, bool huge,
>- enum ttm_caching_state cstate)
>+ enum ttm_caching cstate)
> {
> int pool_index;
>
>- if (cstate == tt_cached)
>+ if (cstate == ttm_cached)
> return NULL;
>
>- if (cstate == tt_wc)
>+ if (cstate == ttm_write_combined)
> pool_index = 0x0;
> else
> pool_index = 0x1;
>@@ -441,17 +441,17 @@ static void ttm_pool_mm_shrink_fini(struct
>ttm_pool_manager *manager)
> }
>
> static int ttm_set_pages_caching(struct page **pages,
>- enum ttm_caching_state cstate, unsigned cpages)
>+ enum ttm_caching cstate, unsigned cpages)
> {
> int r = 0;
> /* Set page caching */
> switch (cstate) {
>- case tt_uncached:
>+ case ttm_uncached:
> r = ttm_set_pages_array_uc(pages, cpages);
> if (r)
> pr_err("Failed to set %d pages to uc!\n", cpages);
> break;
>- case tt_wc:
>+ case ttm_write_combined:
> r = ttm_set_pages_array_wc(pages, cpages);
> if (r)
> pr_err("Failed to set %d pages to wc!\n", cpages);
>@@ -486,7 +486,7 @@ static void ttm_handle_caching_failure(struct page
>**failed_pages,
> * pages returned in pages array.
> */
> static int ttm_alloc_new_pages(struct list_head *pages, gfp_t gfp_flags,
>- int ttm_flags, enum ttm_caching_state cstate,
>+ int ttm_flags, enum ttm_caching cstate,
> unsigned count, unsigned order)
> {
> struct page **caching_array;
>@@ -566,7 +566,7 @@ static int ttm_alloc_new_pages(struct list_head
>*pages, gfp_t gfp_flags,
> * pages is small.
> */
> static void ttm_page_pool_fill_locked(struct ttm_page_pool *pool, int
>ttm_flags,
>- enum ttm_caching_state cstate,
>+ enum ttm_caching cstate,
> unsigned count, unsigned long *irq_flags)
> {
> struct page *p;
>@@ -626,7 +626,7 @@ static void ttm_page_pool_fill_locked(struct
>ttm_page_pool *pool, int ttm_flags,
> static int ttm_page_pool_get_pages(struct ttm_page_pool *pool,
> struct list_head *pages,
> int ttm_flags,
>- enum ttm_caching_state cstate,
>+ enum ttm_caching cstate,
> unsigned count, unsigned order)
> {
> unsigned long irq_flags;
>@@ -703,7 +703,7 @@ static int ttm_page_pool_get_pages(struct
>ttm_page_pool *pool,
>
> /* Put all pages in pages list to correct pool to wait for reuse */
> static void ttm_put_pages(struct page **pages, unsigned npages, int flags,
>- enum ttm_caching_state cstate)
>+ enum ttm_caching cstate)
> {
> struct ttm_page_pool *pool = ttm_get_pool(flags, false, cstate);
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>@@ -821,7 +821,7 @@ static void ttm_put_pages(struct page **pages,
>unsigned npages, int flags,
> * cached pages.
> */
> static int ttm_get_pages(struct page **pages, unsigned npages, int flags,
>- enum ttm_caching_state cstate)
>+ enum ttm_caching cstate)
> {
> struct ttm_page_pool *pool = ttm_get_pool(flags, false, cstate);
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>@@ -1040,7 +1040,7 @@ ttm_pool_unpopulate_helper(struct ttm_tt *ttm,
>unsigned mem_count_update)
>
> put_pages:
> ttm_put_pages(ttm->pages, ttm->num_pages, ttm->page_flags,
>- ttm->caching_state);
>+ ttm->caching);
> ttm_tt_set_unpopulated(ttm);
> }
>
>@@ -1057,7 +1057,7 @@ int ttm_pool_populate(struct ttm_tt *ttm, struct
>ttm_operation_ctx *ctx)
> return -ENOMEM;
>
> ret = ttm_get_pages(ttm->pages, ttm->num_pages, ttm->page_flags,
>- ttm->caching_state);
>+ ttm->caching);
> if (unlikely(ret != 0)) {
> ttm_pool_unpopulate_helper(ttm, 0);
> return ret;
>diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
>b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
>index 1045a5c26ee3..6625b43f6256 100644
>--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
>+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
>@@ -325,15 +325,15 @@ static struct dma_page
>*__ttm_dma_alloc_page(struct dma_pool *pool)
> }
> return d_page;
> }
>-static enum pool_type ttm_to_type(int flags, enum ttm_caching_state
>cstate)
>+static enum pool_type ttm_to_type(int flags, enum ttm_caching cstate)
> {
> enum pool_type type = IS_UNDEFINED;
>
> if (flags & TTM_PAGE_FLAG_DMA32)
> type |= IS_DMA32;
>- if (cstate == tt_cached)
>+ if (cstate == ttm_cached)
> type |= IS_CACHED;
>- else if (cstate == tt_uncached)
>+ else if (cstate == ttm_uncached)
> type |= IS_UC;
> else
> type |= IS_WC;
>@@ -663,7 +663,7 @@ static struct dma_pool *ttm_dma_find_pool(struct
>device *dev,
> * are pages that have changed their caching state already put them to the
> * pool.
> */
>-static void ttm_dma_handle_caching_state_failure(struct dma_pool *pool,
>+static void ttm_dma_handle_caching_failure(struct dma_pool *pool,
> struct list_head *d_pages,
> struct page **failed_pages,
> unsigned cpages)
>@@ -734,7 +734,7 @@ static int ttm_dma_pool_alloc_new_pages(struct
>dma_pool *pool,
> r = ttm_set_pages_caching(pool,
>caching_array,
> cpages);
> if (r)
>-
> ttm_dma_handle_caching_state_failure(
>+ ttm_dma_handle_caching_failure(
> pool, d_pages, caching_array,
> cpages);
> }
>@@ -760,7 +760,7 @@ static int ttm_dma_pool_alloc_new_pages(struct
>dma_pool *pool,
> r = ttm_set_pages_caching(pool,
>caching_array,
> cpages);
> if (r) {
>-
> ttm_dma_handle_caching_state_failure(
>+ ttm_dma_handle_caching_failure(
> pool, d_pages, caching_array,
> cpages);
> goto out;
>@@ -773,7 +773,7 @@ static int ttm_dma_pool_alloc_new_pages(struct
>dma_pool *pool,
> if (cpages) {
> r = ttm_set_pages_caching(pool, caching_array, cpages);
> if (r)
>- ttm_dma_handle_caching_state_failure(pool,
>d_pages,
>+ ttm_dma_handle_caching_failure(pool, d_pages,
> caching_array, cpages);
> }
> out:
>@@ -904,7 +904,7 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma,
>struct device *dev,
> INIT_LIST_HEAD(&ttm_dma->pages_list);
> i = 0;
>
>- type = ttm_to_type(ttm->page_flags, ttm->caching_state);
>+ type = ttm_to_type(ttm->page_flags, ttm->caching);
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> if (ttm->page_flags & TTM_PAGE_FLAG_DMA32)
>@@ -1000,7 +1000,7 @@ void ttm_dma_unpopulate(struct ttm_dma_tt
>*ttm_dma, struct device *dev)
> unsigned count, i, npages = 0;
> unsigned long irq_flags;
>
>- type = ttm_to_type(ttm->page_flags, ttm->caching_state);
>+ type = ttm_to_type(ttm->page_flags, ttm->caching);
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> pool = ttm_dma_find_pool(dev, type | IS_HUGE);
>@@ -1032,7 +1032,7 @@ void ttm_dma_unpopulate(struct ttm_dma_tt
>*ttm_dma, struct device *dev)
> return;
>
> is_cached = (ttm_dma_find_pool(pool->dev,
>- ttm_to_type(ttm->page_flags, tt_cached)) == pool);
>+ ttm_to_type(ttm->page_flags, ttm_cached)) == pool);
>
> /* make sure pages array match list and count number of pages */
> count = 0;
>diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
>index 23e9604bc924..d4c80e0f3b3b 100644
>--- a/drivers/gpu/drm/ttm/ttm_tt.c
>+++ b/drivers/gpu/drm/ttm/ttm_tt.c
>@@ -114,31 +114,30 @@ static int ttm_sg_tt_alloc_page_directory(struct
>ttm_dma_tt *ttm)
> return 0;
> }
>
>-static int ttm_tt_set_caching(struct ttm_tt *ttm,
>- enum ttm_caching_state c_state)
>+static int ttm_tt_set_caching(struct ttm_tt *ttm, enum ttm_caching caching)
> {
>- if (ttm->caching_state == c_state)
>+ if (ttm->caching == caching)
> return 0;
>
> /* Can't change the caching state after TT is populated */
> if (WARN_ON_ONCE(ttm_tt_is_populated(ttm)))
> return -EINVAL;
>
>- ttm->caching_state = c_state;
>+ ttm->caching = caching;
>
> return 0;
> }
>
> int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement)
> {
>- enum ttm_caching_state state;
>+ enum ttm_caching state;
>
> if (placement & TTM_PL_FLAG_WC)
>- state = tt_wc;
>+ state = ttm_write_combined;
> else if (placement & TTM_PL_FLAG_UNCACHED)
>- state = tt_uncached;
>+ state = ttm_uncached;
> else
>- state = tt_cached;
>+ state = ttm_cached;
>
> return ttm_tt_set_caching(ttm, state);
> }
>@@ -165,7 +164,7 @@ static void ttm_tt_init_fields(struct ttm_tt *ttm,
> uint32_t page_flags)
> {
> ttm->num_pages = bo->num_pages;
>- ttm->caching_state = tt_cached;
>+ ttm->caching = ttm_cached;
> ttm->page_flags = page_flags;
> ttm_tt_set_unpopulated(ttm);
> ttm->swap_storage = NULL;
>diff --git a/include/drm/ttm/ttm_caching.h b/include/drm/ttm/ttm_caching.h
>new file mode 100644
>index 000000000000..161624dcf6be
>--- /dev/null
>+++ b/include/drm/ttm/ttm_caching.h
>@@ -0,0 +1,34 @@
>+/*
>+ * Copyright 2020 Advanced Micro Devices, Inc.
>+ *
>+ * Permission is hereby granted, free of charge, to any person obtaining a
>+ * copy of this software and associated documentation files (the
>"Software"),
>+ * to deal in the Software without restriction, including without limitation
>+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>+ * and/or sell copies of the Software, and to permit persons to whom the
>+ * Software is furnished to do so, subject to the following conditions:
>+ *
>+ * The above copyright notice and this permission notice shall be included in
>+ * all copies or substantial portions of the Software.
>+ *
>+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>EXPRESS OR
>+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>MERCHANTABILITY,
>+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
>EVENT SHALL
>+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM,
>DAMAGES OR
>+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>OTHERWISE,
>+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
>USE OR
>+ * OTHER DEALINGS IN THE SOFTWARE.
>+ *
>+ * Authors: Christian König
>+ */
>+
>+#ifndef _TTM_CACHING_H_
>+#define _TTM_CACHING_H_
>+
>+enum ttm_caching {
>+ ttm_uncached,
>+ ttm_write_combined,
>+ ttm_cached
>+};
>+
>+#endif
>diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
>index 5d1835d44084..b90c8c451a54 100644
>--- a/include/drm/ttm/ttm_tt.h
>+++ b/include/drm/ttm/ttm_tt.h
>@@ -28,6 +28,7 @@
> #define _TTM_TT_H_
>
> #include <linux/types.h>
>+#include <drm/ttm/ttm_caching.h>
>
> struct ttm_tt;
> struct ttm_resource;
>@@ -42,12 +43,6 @@ struct ttm_operation_ctx;
>
> #define TTM_PAGE_FLAG_PRIV_POPULATED (1 << 31)
>
>-enum ttm_caching_state {
>- tt_uncached,
>- tt_wc,
>- tt_cached
>-};
>-
> /**
> * struct ttm_tt
> *
>@@ -69,7 +64,7 @@ struct ttm_tt {
> unsigned long num_pages;
> struct sg_table *sg; /* for SG objects via dma-buf */
> struct file *swap_storage;
>- enum ttm_caching_state caching_state;
>+ enum ttm_caching caching;
> };
>
> static inline bool ttm_tt_is_populated(struct ttm_tt *tt)
>--
>2.17.1
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-10-05 19:15 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-01 11:28 [PATCH 1/8] drm/ttm: remove TTM_PAGE_FLAG_WRITE Christian König
2020-10-01 11:28 ` [PATCH 2/8] drm/ttm: move ttm_set_memory.h out of include Christian König
2020-10-05 15:01 ` Ruhl, Michael J
2020-10-07 8:31 ` Christian König
2020-10-07 11:46 ` Ruhl, Michael J
2020-10-07 12:10 ` Christian König
2020-10-01 11:28 ` [PATCH 3/8] drm/ttm: cleanup ttm_handle_caching_state_failure Christian König
2020-10-05 15:01 ` Ruhl, Michael J
2020-10-01 11:28 ` [PATCH 4/8] drm/ttm: rename TTM caching enum Christian König
2020-10-05 15:05 ` Ruhl, Michael J [this message]
2020-10-07 8:08 ` Christian König
2020-10-01 11:28 ` [PATCH 5/8] drm/ttm: set the tt caching state at creation time Christian König
2020-10-05 15:18 ` Ruhl, Michael J
2020-10-01 11:28 ` [PATCH 6/8] drm/ttm: add caching state to ttm_bus_placement Christian König
2020-10-05 15:39 ` Ruhl, Michael J
2020-10-07 8:59 ` Christian König
2020-10-01 11:28 ` [PATCH 7/8] drm/ttm: use caching instead of placement for ttm_io_prot Christian König
2020-10-05 15:51 ` Ruhl, Michael J
2020-10-07 8:59 ` Christian König
2020-10-01 11:28 ` [PATCH 8/8] drm/ttm: nuke caching placement flags Christian König
2020-10-05 16:17 ` Ruhl, Michael J
2020-10-07 9:07 ` Christian König
2020-10-05 13:29 ` [PATCH 1/8] drm/ttm: remove TTM_PAGE_FLAG_WRITE Christian König
2020-10-05 14:59 ` Ruhl, Michael J
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=d73b28c453454cf981496b2f28b848de@intel.com \
--to=michael.j.ruhl@intel.com \
--cc=airlied@gmail.com \
--cc=ckoenig.leichtzumerken@gmail.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=ray.huang@amd.com \
/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.