[PATCH] drm/amdgpu: Set ttm caching flags during bo allocation

[PATCH] drm/amdgpu: Set ttm caching flags during bo allocation

[Oak Zeng]

The ttm caching flags (ttm_cached, ttm_write_combined etc) are
used to determine a buffer object's mapping attributes in both
CPU page table and GPU page table (when that buffer is also
accessed by GPU). Currently the ttm caching flags are set in
function amdgpu_ttm_io_mem_reserve which is called during
DRM_AMDGPU_GEM_MMAP ioctl. This has a problem since the GPU
mapping of the buffer object (ioctl DRM_AMDGPU_GEM_VA) can
happen earlier than the mmap time, thus the GPU page table
update code can't pick up the right ttm caching flags to
decide the right GPU page table attributes.

This patch moves the ttm caching flags setting to function
amdgpu_vram_mgr_new - this function is called during the
first step of a buffer object create (eg, DRM_AMDGPU_GEM_CREATE)
so the later both CPU and GPU mapping function calls will
pick up this flag for CPU/GPU page table set up.

Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
Suggested-by: Christian Koenig <Christian.Koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c      | 4 ----
 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 +++++
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 6297363..93acf6f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -607,10 +607,6 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_device *bdev,
 
 		mem->bus.offset += adev->gmc.aper_base;
 		mem->bus.is_iomem = true;
-		if (adev->gmc.xgmi.connected_to_cpu)
-			mem->bus.caching = ttm_cached;
-		else
-			mem->bus.caching = ttm_write_combined;
 		break;
 	default:
 		return -EINVAL;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index a52e17e..6cb66eb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -454,6 +454,11 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
 	if (i == 1)
 		mem->placement |= TTM_PL_FLAG_CONTIGUOUS;
 
+	if (adev->gmc.xgmi.connected_to_cpu)
+		mem->bus.caching = ttm_cached;
+	else
+		mem->bus.caching = ttm_write_combined;
+
 	atomic64_add(vis_usage, &mgr->vis_usage);
 	mem->mm_node = nodes;
 	return 0;
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: Set ttm caching flags during bo allocation

[Xu, Feifei]

[AMD Official Use Only]

Reviewed-by: Feifei Xu <Feifei.Xu@amd.com>

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Oak Zeng
Sent: Tuesday, June 29, 2021 7:16 AM
To: amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix <Felix.Kuehling@amd.com>; Zhu, James <James.Zhu@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Zeng, Oak <Oak.Zeng@amd.com>
Subject: [PATCH] drm/amdgpu: Set ttm caching flags during bo allocation

The ttm caching flags (ttm_cached, ttm_write_combined etc) are used to determine a buffer object's mapping attributes in both CPU page table and GPU page table (when that buffer is also accessed by GPU). Currently the ttm caching flags are set in function amdgpu_ttm_io_mem_reserve which is called during DRM_AMDGPU_GEM_MMAP ioctl. This has a problem since the GPU mapping of the buffer object (ioctl DRM_AMDGPU_GEM_VA) can happen earlier than the mmap time, thus the GPU page table update code can't pick up the right ttm caching flags to decide the right GPU page table attributes.

This patch moves the ttm caching flags setting to function amdgpu_vram_mgr_new - this function is called during the first step of a buffer object create (eg, DRM_AMDGPU_GEM_CREATE) so the later both CPU and GPU mapping function calls will pick up this flag for CPU/GPU page table set up.

Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
Suggested-by: Christian Koenig <Christian.Koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c      | 4 ----
 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 +++++
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 6297363..93acf6f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -607,10 +607,6 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_device *bdev,

                mem->bus.offset += adev->gmc.aper_base;
                mem->bus.is_iomem = true;
-               if (adev->gmc.xgmi.connected_to_cpu)
-                       mem->bus.caching = ttm_cached;
-               else
-                       mem->bus.caching = ttm_write_combined;
                break;
        default:
                return -EINVAL;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index a52e17e..6cb66eb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -454,6 +454,11 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
        if (i == 1)
                mem->placement |= TTM_PL_FLAG_CONTIGUOUS;

+       if (adev->gmc.xgmi.connected_to_cpu)
+               mem->bus.caching = ttm_cached;
+       else
+               mem->bus.caching = ttm_write_combined;
+
        atomic64_add(vis_usage, &mgr->vis_usage);
        mem->mm_node = nodes;
        return 0;
--
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7CFeifei.Xu%40amd.com%7Caef2cb54c9a4489626e808d93a8aab8c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637605189533617890%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Pzl%2FSSuBHjSUjBFQVffr3UneV8hzrVb2cmN8cSnifaA%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Set ttm caching flags during bo allocation

[Christian König]

Am 29.06.21 um 01:15 schrieb Oak Zeng:
> The ttm caching flags (ttm_cached, ttm_write_combined etc) are
> used to determine a buffer object's mapping attributes in both
> CPU page table and GPU page table (when that buffer is also
> accessed by GPU). Currently the ttm caching flags are set in
> function amdgpu_ttm_io_mem_reserve which is called during
> DRM_AMDGPU_GEM_MMAP ioctl. This has a problem since the GPU
> mapping of the buffer object (ioctl DRM_AMDGPU_GEM_VA) can
> happen earlier than the mmap time, thus the GPU page table
> update code can't pick up the right ttm caching flags to
> decide the right GPU page table attributes.
>
> This patch moves the ttm caching flags setting to function
> amdgpu_vram_mgr_new - this function is called during the
> first step of a buffer object create (eg, DRM_AMDGPU_GEM_CREATE)
> so the later both CPU and GPU mapping function calls will
> pick up this flag for CPU/GPU page table set up.
>
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> Suggested-by: Christian Koenig <Christian.Koenig@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c      | 4 ----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 +++++
>   2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 6297363..93acf6f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -607,10 +607,6 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_device *bdev,
>   
>   		mem->bus.offset += adev->gmc.aper_base;
>   		mem->bus.is_iomem = true;
> -		if (adev->gmc.xgmi.connected_to_cpu)
> -			mem->bus.caching = ttm_cached;
> -		else
> -			mem->bus.caching = ttm_write_combined;
>   		break;
>   	default:
>   		return -EINVAL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index a52e17e..6cb66eb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> @@ -454,6 +454,11 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
>   	if (i == 1)
>   		mem->placement |= TTM_PL_FLAG_CONTIGUOUS;
>   
> +	if (adev->gmc.xgmi.connected_to_cpu)
> +		mem->bus.caching = ttm_cached;
> +	else
> +		mem->bus.caching = ttm_write_combined;
> +
>   	atomic64_add(vis_usage, &mgr->vis_usage);
>   	mem->mm_node = nodes;
>   	return 0;

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Set ttm caching flags during bo allocation

[Das, Nirmoy]

Thanks for the detailed commit message :)

Acked-by: Nirmoy Das <nirmoy.das@amd.com>

On 6/29/2021 1:15 AM, Oak Zeng wrote:
> The ttm caching flags (ttm_cached, ttm_write_combined etc) are
> used to determine a buffer object's mapping attributes in both
> CPU page table and GPU page table (when that buffer is also
> accessed by GPU). Currently the ttm caching flags are set in
> function amdgpu_ttm_io_mem_reserve which is called during
> DRM_AMDGPU_GEM_MMAP ioctl. This has a problem since the GPU
> mapping of the buffer object (ioctl DRM_AMDGPU_GEM_VA) can
> happen earlier than the mmap time, thus the GPU page table
> update code can't pick up the right ttm caching flags to
> decide the right GPU page table attributes.
>
> This patch moves the ttm caching flags setting to function
> amdgpu_vram_mgr_new - this function is called during the
> first step of a buffer object create (eg, DRM_AMDGPU_GEM_CREATE)
> so the later both CPU and GPU mapping function calls will
> pick up this flag for CPU/GPU page table set up.
>
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> Suggested-by: Christian Koenig <Christian.Koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c      | 4 ----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 +++++
>   2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 6297363..93acf6f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -607,10 +607,6 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_device *bdev,
>   
>   		mem->bus.offset += adev->gmc.aper_base;
>   		mem->bus.is_iomem = true;
> -		if (adev->gmc.xgmi.connected_to_cpu)
> -			mem->bus.caching = ttm_cached;
> -		else
> -			mem->bus.caching = ttm_write_combined;
>   		break;
>   	default:
>   		return -EINVAL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index a52e17e..6cb66eb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> @@ -454,6 +454,11 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
>   	if (i == 1)
>   		mem->placement |= TTM_PL_FLAG_CONTIGUOUS;
>   
> +	if (adev->gmc.xgmi.connected_to_cpu)
> +		mem->bus.caching = ttm_cached;
> +	else
> +		mem->bus.caching = ttm_write_combined;
> +
>   	atomic64_add(vis_usage, &mgr->vis_usage);
>   	mem->mm_node = nodes;
>   	return 0;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx