On 2019-08-06 22:31, Zeng, Oak wrote:
> Arcturus repurposed mtype WC to RW. Modify gmc functions
> to support the new mtype
>
> Change-Id: Idc338e5386a57020f45262025e2664ab4ba9f291
> Signed-off-by: Oak Zeng <oak.z...@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c     | 61 
> ++++++++++++++++++++++++++++++-
>   drivers/gpu/drm/amd/include/vega10_enum.h |  7 ++++
>   2 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index a2aa35e..89064d9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -599,6 +599,50 @@ static uint64_t gmc_v9_0_get_vm_pte_flags(struct 
> amdgpu_device *adev,
>       return pte_flag;
>   }
>   
> +static uint64_t gmc_v9_0_arcturus_get_vm_pte_flags(struct amdgpu_device 
> *adev,
> +                                             uint32_t flags)
> +
> +{
> +     uint64_t pte_flag = 0;
> +
> +     if (flags & AMDGPU_VM_PAGE_EXECUTABLE)
> +             pte_flag |= AMDGPU_PTE_EXECUTABLE;
> +     if (flags & AMDGPU_VM_PAGE_READABLE)
> +             pte_flag |= AMDGPU_PTE_READABLE;
> +     if (flags & AMDGPU_VM_PAGE_WRITEABLE)
> +             pte_flag |= AMDGPU_PTE_WRITEABLE;
> +
> +     if (adev->gmc.zfb_size > 0)
> +             pte_flag |= AMDGPU_PTE_SYSTEM;
> +
> +     switch (flags & AMDGPU_VM_MTYPE_MASK) {
> +     case AMDGPU_VM_MTYPE_DEFAULT:
> +             pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_NC);
> +             break;
> +     case AMDGPU_VM_MTYPE_NC:
> +             pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_NC);
> +             break;
> +     case AMDGPU_VM_MTYPE_RW:
> +             pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_RW);
> +             break;
> +     case AMDGPU_VM_MTYPE_CC:
> +             pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_CC);
> +             break;
> +     case AMDGPU_VM_MTYPE_UC:
> +             pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_UC);
> +             break;
> +     default:
> +             pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_ARCTURUS_NC);
> +             break;
> +     }
> +
> +     if (flags & AMDGPU_VM_PAGE_PRT)
> +             pte_flag |= AMDGPU_PTE_PRT;
> +
> +     return pte_flag;
> +}

Adding a whole new function for Arcturus is overkill. Just add MTYPE_RW 
to the existing function.

         switch(flags & AMDGPU_VM_MTYPE_MASK) {
         ...
         case AMDGPU_VM_MTYPE_WC:
                 pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_WC);
                 break;
         case AMDGPU_VM_MTYPE_RW:
                 pte_flag |= AMDGPU_PTE_MTYPE_VG10(MTYPE_RW);
                 break;
         ...
         }

The fact that WC isn't valid on Arcturus and RW isn't valid on other 
Vega GPUs could be handled in higher level code.


> +
> +
>   static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
>                               uint64_t *addr, uint64_t *flags)
>   {
> @@ -631,9 +675,24 @@ static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs 
> = {
>       .get_vm_pde = gmc_v9_0_get_vm_pde
>   };
>   
> +static const struct amdgpu_gmc_funcs gmc_v9_0_arcturus_gmc_funcs = {
> +     .flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
> +     .emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,
> +     .emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
> +     .get_vm_pte_flags = gmc_v9_0_arcturus_get_vm_pte_flags,
> +     .get_vm_pde = gmc_v9_0_get_vm_pde
> +};
> +
>   static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)
>   {
> -     adev->gmc.gmc_funcs = &gmc_v9_0_gmc_funcs;
> +     switch (adev->asic_type) {
> +     case CHIP_ARCTURUS:
> +             adev->gmc.gmc_funcs = &gmc_v9_0_arcturus_gmc_funcs;
> +             break;
> +     default:
> +             adev->gmc.gmc_funcs = &gmc_v9_0_gmc_funcs;
> +             break;
> +     }
>   }
>   
>   static void gmc_v9_0_set_umc_funcs(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/include/vega10_enum.h 
> b/drivers/gpu/drm/amd/include/vega10_enum.h
> index c14ba65..0d6d97d 100644
> --- a/drivers/gpu/drm/amd/include/vega10_enum.h
> +++ b/drivers/gpu/drm/amd/include/vega10_enum.h
> @@ -1041,6 +1041,13 @@ MTYPE_CC                                 = 0x00000002,
>   MTYPE_UC                                 = 0x00000003,
>   } MTYPE;
>   
> +typedef enum MTYPE_ARCTURUS {
> +MTYPE_ARCTURUS_NC                                 = 0x00000000,
> +MTYPE_ARCTURUS_RW                                 = 0x00000001,
> +MTYPE_ARCTURUS_CC                                 = 0x00000002,
> +MTYPE_ARCTURUS_UC                                 = 0x00000003,
> +} MTYPE_ARCTURUS;

This is overkill. Most of the MTYPEs are the same on Arcturus. You can 
just define MTYPE_RW as an alias of MTYPE_WC:

typedef enum MTYPE {
MTYPE_NC                                 = 0x00000000,
MTYPE_WC                                 = 0x00000001,
MTYPE_RW                                 = 0x00000001, /* Arcturus */
MTYPE_CC                                 = 0x00000002,
MTYPE_UC                                 = 0x00000003,
} MTYPE;

Regards,
   Felix


> +
>   /*
>    * RMI_CID enum
>    */
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to