[PATCH 2/2] drm/amdgpu: cleanup PTE flag generation v2
Kuehling, Felix
Felix.Kuehling at amd.com
Wed Sep 4 20:00:50 UTC 2019
On 2019-09-04 8:17 a.m., Christian König wrote:
> Move the ASIC specific code into a new callback function.
>
> v2: mask the flags for SI and CIK instead of a BUG_ON().
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
Nit-pick inline. Otherwise the series is
Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 5 +++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 29 ++-----------------------
> drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 22 ++++++++++++++++++-
> drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 9 ++++++++
> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 11 +++++++++-
> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 13 ++++++++++-
> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 24 +++++++++++++++++++-
> 7 files changed, 82 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 6a74059b776c..232a8ff5642b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -104,6 +104,10 @@ struct amdgpu_gmc_funcs {
> /* get the pde for a given mc addr */
> void (*get_vm_pde)(struct amdgpu_device *adev, int level,
> u64 *dst, u64 *flags);
> + /* get the pte flags to use for a BO VA mapping */
> + void (*get_vm_pte)(struct amdgpu_device *adev,
> + struct amdgpu_bo_va_mapping *mapping,
> + uint64_t *flags);
> };
>
> struct amdgpu_xgmi {
> @@ -185,6 +189,7 @@ struct amdgpu_gmc {
> #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))
> #define amdgpu_gmc_map_mtype(adev, flags) (adev)->gmc.gmc_funcs->map_mtype((adev),(flags))
> #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags))
> +#define amdgpu_gmc_get_vm_pte(adev, mapping, flags) (adev)->gmc.gmc_funcs->get_vm_pte((adev), (mapping), (flags))
>
> /**
> * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 2cb82b229802..b285ab25146d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1571,33 +1571,8 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
> if (!(mapping->flags & AMDGPU_PTE_WRITEABLE))
> flags &= ~AMDGPU_PTE_WRITEABLE;
>
> - if (adev->asic_type >= CHIP_TONGA) {
> - flags &= ~AMDGPU_PTE_EXECUTABLE;
> - flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
> - }
> -
> - if (adev->asic_type >= CHIP_NAVI10) {
> - flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
> - flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
> - } else {
> - flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
> - flags |= (mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK);
> - }
> -
> - if ((mapping->flags & AMDGPU_PTE_PRT) &&
> - (adev->asic_type >= CHIP_VEGA10)) {
> - flags |= AMDGPU_PTE_PRT;
> - if (adev->asic_type >= CHIP_NAVI10) {
> - flags |= AMDGPU_PTE_SNOOPED;
> - flags |= AMDGPU_PTE_LOG;
> - flags |= AMDGPU_PTE_SYSTEM;
> - }
> - flags &= ~AMDGPU_PTE_VALID;
> - }
> - if (adev->asic_type == CHIP_ARCTURUS &&
> - !(flags & AMDGPU_PTE_SYSTEM) &&
> - mapping->bo_va->is_xgmi)
> - flags |= AMDGPU_PTE_SNOOPED;
> + /* Apply ASIC specific mapping flags */
> + amdgpu_gmc_get_vm_pte(adev, mapping, &flags);
>
> trace_amdgpu_vm_bo_update(mapping);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> index 7eb0ba87fef9..1a8d8f528b01 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> @@ -440,12 +440,32 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
> }
> }
>
> +static void gmc_v10_0_get_vm_pte(struct amdgpu_device *adev,
> + struct amdgpu_bo_va_mapping *mapping,
> + uint64_t *flags)
> +{
> + *flags &= ~AMDGPU_PTE_EXECUTABLE;
> + *flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
> +
> + *flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
> + *flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
> +
> + if (mapping->flags & AMDGPU_PTE_PRT) {
> + *flags |= AMDGPU_PTE_PRT;
> + *flags |= AMDGPU_PTE_SNOOPED;
> + *flags |= AMDGPU_PTE_LOG;
> + *flags |= AMDGPU_PTE_SYSTEM;
> + *flags &= ~AMDGPU_PTE_VALID;
> + }
> +}
> +
> static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {
> .flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,
> .emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,
> .emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,
> .map_mtype = gmc_v10_0_map_mtype,
> - .get_vm_pde = gmc_v10_0_get_vm_pde
> + .get_vm_pde = gmc_v10_0_get_vm_pde,
> + .get_vm_pte = gmc_v10_0_get_vm_pte
> };
>
> static void gmc_v10_0_set_gmc_funcs(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index 2e305b21db69..10d46757c078 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -389,6 +389,14 @@ static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
> BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
> }
>
> +static void gmc_v6_0_get_vm_pte(struct amdgpu_device *adev,
> + struct amdgpu_bo_va_mapping *mapping,
> + uint64_t *flags)
> +{
> + *flags &= ~AMDGPU_PTE_EXECUTABLE;
> + *flags &= ~AMDGPU_PTE_PRT;
> +}
> +
> static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
> bool value)
> {
> @@ -1144,6 +1152,7 @@ static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {
> .emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,
> .set_prt = gmc_v6_0_set_prt,
> .get_vm_pde = gmc_v6_0_get_vm_pde,
> + .get_vm_pte = gmc_v6_0_get_vm_pte,
> };
>
> static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 3b77421234a7..f86c1f15f7bf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -466,6 +466,14 @@ static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
> BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
> }
>
> +static void gmc_v7_0_get_vm_pte(struct amdgpu_device *adev,
> + struct amdgpu_bo_va_mapping *mapping,
> + uint64_t *flags)
> +{
> + *flags &= ~AMDGPU_PTE_EXECUTABLE;
> + *flags &= ~AMDGPU_PTE_PRT;
> +}
> +
> /**
> * gmc_v8_0_set_fault_enable_default - update VM fault handling
> *
> @@ -1339,7 +1347,8 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
> .emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,
> .emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,
> .set_prt = gmc_v7_0_set_prt,
> - .get_vm_pde = gmc_v7_0_get_vm_pde
> + .get_vm_pde = gmc_v7_0_get_vm_pde,
> + .get_vm_pte = gmc_v7_0_get_vm_pte
> };
>
> static const struct amdgpu_irq_src_funcs gmc_v7_0_irq_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 58444aa0af05..256d1faacada 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -689,6 +689,16 @@ static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
> BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
> }
>
> +static void gmc_v8_0_get_vm_pte(struct amdgpu_device *adev,
> + struct amdgpu_bo_va_mapping *mapping,
> + uint64_t *flags)
> +{
> + BUG_ON(*flags & AMDGPU_PTE_PRT);
Is there a reason why this is still a BUG_ON here, when the gmc_v6/7
functions just clear the bit? Or just an oversight?
Regards,
Felix
> +
> + *flags &= ~AMDGPU_PTE_EXECUTABLE;
> + *flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
> +}
> +
> /**
> * gmc_v8_0_set_fault_enable_default - update VM fault handling
> *
> @@ -1705,7 +1715,8 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
> .emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,
> .emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,
> .set_prt = gmc_v8_0_set_prt,
> - .get_vm_pde = gmc_v8_0_get_vm_pde
> + .get_vm_pde = gmc_v8_0_get_vm_pde,
> + .get_vm_pte = gmc_v8_0_get_vm_pte
> };
>
> static const struct amdgpu_irq_src_funcs gmc_v8_0_irq_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 8ee0f6498a86..1a144ad3a2b1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -627,12 +627,34 @@ static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
> }
> }
>
> +static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
> + struct amdgpu_bo_va_mapping *mapping,
> + uint64_t *flags)
> +{
> + *flags &= ~AMDGPU_PTE_EXECUTABLE;
> + *flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
> +
> + *flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
> + *flags |= mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK;
> +
> + if (mapping->flags & AMDGPU_PTE_PRT) {
> + *flags |= AMDGPU_PTE_PRT;
> + *flags &= ~AMDGPU_PTE_VALID;
> + }
> +
> + if (adev->asic_type == CHIP_ARCTURUS &&
> + !(*flags & AMDGPU_PTE_SYSTEM) &&
> + mapping->bo_va->is_xgmi)
> + *flags |= AMDGPU_PTE_SNOOPED;
> +}
> +
> static const struct amdgpu_gmc_funcs gmc_v9_0_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,
> .map_mtype = gmc_v9_0_map_mtype,
> - .get_vm_pde = gmc_v9_0_get_vm_pde
> + .get_vm_pde = gmc_v9_0_get_vm_pde,
> + .get_vm_pte = gmc_v9_0_get_vm_pte
> };
>
> static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)
More information about the amd-gfx
mailing list