[PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

Zeng, Oak Oak.Zeng at amd.com
Thu Aug 8 16:02:42 UTC 2019


Hi Christian,

My understanding of the snoop bit (C bit in the PTE definition) is to probe remote gpu's L2 cache after this gpu write remote gpu's vram. Is this correct? I am still checking this point with HW engineer.

If this is correct, then the snooping (or probing) is a way to maintain certain cache coherency when one memory is access by two masters (for example two gpu). With existing AMDGPU_VM_ definitions in amdgpu_drm.h, how does a user implement the request like: I want a trunk of vram physically in a remote gpu, I want to access it in a uncached way (AMDGPU_VM_MTYPE_UC) but I want to probe remote gpu's cache when I modify this vram.

From PTE's definition, both C bit and mtype and R/W/X bits are just flags to enable user to program page access behavior. Any detail reason why we shouldn't expose the snoop bit?

Regards,
Oak

-----Original Message-----
From: Christian K├Ânig <ckoenig.leichtzumerken at gmail.com> 
Sent: Wednesday, August 7, 2019 4:42 AM
To: Zeng, Oak <Oak.Zeng at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Kuehling, Felix <Felix.Kuehling at amd.com>; Koenig, Christian <Christian.Koenig at amd.com>; Keely, Sean <Sean.Keely at amd.com>
Subject: Re: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

Am 07.08.19 um 04:31 schrieb Zeng, Oak:
> Add definition of all supported mtypes. The RW mtype is recently 
> introduced for arcturus. Also add definition for the 
> cachable/snoopable bit, which will be used later in this series.
>
> Change-Id: I96fc9bb4b6b1e62bdc10b600d8aaa6a802128d6d
> Signed-off-by: Oak Zeng <Oak.Zeng at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 9 +++++++--
>   include/uapi/drm/amdgpu_drm.h          | 4 ++++
>   2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 2eda3a8..7a77477 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -80,8 +80,13 @@ struct amdgpu_bo_list_entry;
>   #define AMDGPU_PTE_MTYPE_VG10(a)	((uint64_t)(a) << 57)
>   #define AMDGPU_PTE_MTYPE_VG10_MASK	AMDGPU_PTE_MTYPE_VG10(3ULL)
>   
> -#define AMDGPU_MTYPE_NC 0
> -#define AMDGPU_MTYPE_CC 2
> +enum amdgpu_mtype {
> +	AMDGPU_MTYPE_NC = 0,
> +	AMDGPU_MTYPE_WC = 1,
> +	AMDGPU_MTYPE_CC = 2,
> +	AMDGPU_MTYPE_UC = 3,
> +	AMDGPU_MTYPE_RW = 4,
> +};
>   
>   #define AMDGPU_PTE_DEFAULT_ATC  (AMDGPU_PTE_SYSTEM      \
>                                   | AMDGPU_PTE_SNOOPED    \
> diff --git a/include/uapi/drm/amdgpu_drm.h 
> b/include/uapi/drm/amdgpu_drm.h index ca97b68..2889663 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -503,6 +503,10 @@ struct drm_amdgpu_gem_op {
>   #define AMDGPU_VM_MTYPE_CC		(3 << 5)
>   /* Use UC MTYPE instead of default MTYPE */
>   #define AMDGPU_VM_MTYPE_UC		(4 << 5)
> +/* Use RW MTYPE instead of default MTYPE */
> +#define AMDGPU_VM_MTYPE_RW		(5 << 5)

> +/* Cacheable/snoopable */
> +#define AMDGPU_VM_PAGE_SNOOPED		(1 << 9)

That's a rather big NAK. Cache snooping is not something userspace is allowed to be aware of.

Christian.

>   
>   struct drm_amdgpu_gem_va {
>   	/** GEM object handle */



More information about the amd-gfx mailing list