The snoop bit is for snooping the CPU cache by the GPU when doing system memory 
mappings.

Alex
________________________________
From: amd-gfx <[email protected]> on behalf of Zeng, Oak 
<[email protected]>
Sent: Thursday, August 8, 2019 12:02 PM
To: Koenig, Christian <[email protected]>; [email protected] 
<[email protected]>
Cc: Kuehling, Felix <[email protected]>; Keely, Sean <[email protected]>
Subject: RE: [PATCH 1/5] drm/amdgpu: Extends amdgpu vm definitions

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 <[email protected]>
Sent: Wednesday, August 7, 2019 4:42 AM
To: Zeng, Oak <[email protected]>; [email protected]
Cc: Kuehling, Felix <[email protected]>; Koenig, Christian 
<[email protected]>; Keely, Sean <[email protected]>
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 <[email protected]>
> ---
>   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 */

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to