[PATCH 2/2] drm/amdgpu: cleanup PTE flag generation
Christian König
ckoenig.leichtzumerken at gmail.com
Wed Sep 4 07:59:29 UTC 2019
> the mapping structure still keep the UAPI flag and all the HW specific PTE setting is in the
> last step before real set to HW ?
I thought about that approach as well, but that would unfortunately not
work correctly.
The problem is that we have some mapping flags which don't have an
userspace representation.
Additional to that I want to avoid leaking the UAPI defines into the VM
subsystem.
We shouldn't have created the UAPI defines in the first place and should
have used the hardware flags directly.
Regards,
Christian.
Am 03.09.19 um 23:32 schrieb Liu, Shaoyun:
> Maybe it's not necessary to separate the mtype from the get_vm_pte
> function , so we just need one asic specific functions (get_vm_pte) .
>
> What make me confusing originally is the the logic to setup the PTE
> flag. We first map the UAPI flags to HW specific PTE flag , save it
> into mapping structure and in amdgpu_bo_split_mapping adjust the
> flag again before set the value to HW . Is it possible we just have one
> place to set the asic specific PTE flag , ex, the mapping structure
> still keep the UAPI flag and all the HW specific PTE setting is in the
> last step before real set to HW ?
>
> Regards
>
> shaoyun.liu
>
> On 2019-09-02 10:57 a.m., Christian König wrote:
>> Move the ASIC specific code into a new callback function.
>>
>> Signed-off-by: Christian König <christian.koenig 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 d5e4574afbc2..d3be51ba6349 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..ce591c995691 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)
>> +{
>> + BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
>> + BUG_ON(*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..e3f53fbfcd8f 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)
>> +{
>> + BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
>> + BUG_ON(*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);
>> +
>> + *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 22660e1005db..b3d2d70e84c9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -625,12 +625,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