[PATCH 1/7] drm/amdgpu: cleanup adjust_mc_addr handling v2
Christian König
deathsimple at vodafone.de
Wed May 24 09:18:57 UTC 2017
Am 18.05.2017 um 07:33 schrieb Zhang, Jerry (Junwei):
> On 05/17/2017 05:22 PM, Christian König wrote:
>> [SNIP]
>> +static uint64_t gmc_v6_0_get_vm_pde(struct amdgpu_device *adev,
>> uint64_t addr)
>> +{
>> + BUG_ON(addr & 0xFFFFF0000000FFFULL);
>> + return addr;
>> +}
>
> Just wonder why we didn't return the address as below?
> adev->vm_manager.vram_base_offset + addr
That is a really good question and I couldn't figure out all the details
either.
I've tested it on an Kaveri and it still seems to work fine, so the
logic is indeed correct.
> Does that cause gmc6~gmc8 has no APU support officially?
No, only the pro driver doesn't support APUs. The open source driver
works perfectly fine on them.
> Or I miss any info about them?
The documentation for gfx6-8 is very unclear on this.
It says that the registers and PDE should use physical addresses, but
from my testing that isn't the case.
Going to ping the hardware dev on this once more when I have time.
Regards,
Christian.
>
> Jerry
>
>> +
>> static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device
>> *adev,
>> bool value)
>> {
>> @@ -1127,6 +1133,7 @@ static const struct amdgpu_gart_funcs
>> gmc_v6_0_gart_funcs = {
>> .flush_gpu_tlb = gmc_v6_0_gart_flush_gpu_tlb,
>> .set_pte_pde = gmc_v6_0_gart_set_pte_pde,
>> .set_prt = gmc_v6_0_set_prt,
>> + .get_vm_pde = gmc_v6_0_get_vm_pde,
>> .get_vm_pte_flags = gmc_v6_0_get_vm_pte_flags
>> };
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> index 967505b..4f140e8 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> @@ -472,6 +472,12 @@ static uint64_t gmc_v7_0_get_vm_pte_flags(struct
>> amdgpu_device *adev,
>> return pte_flag;
>> }
>>
>> +static uint64_t gmc_v7_0_get_vm_pde(struct amdgpu_device *adev,
>> uint64_t addr)
>> +{
>> + BUG_ON(addr & 0xFFFFF0000000FFFULL);
>> + return addr;
>> +}
>> +
>> /**
>> * gmc_v8_0_set_fault_enable_default - update VM fault handling
>> *
>> @@ -1293,7 +1299,8 @@ static const struct amdgpu_gart_funcs
>> gmc_v7_0_gart_funcs = {
>> .flush_gpu_tlb = gmc_v7_0_gart_flush_gpu_tlb,
>> .set_pte_pde = gmc_v7_0_gart_set_pte_pde,
>> .set_prt = gmc_v7_0_set_prt,
>> - .get_vm_pte_flags = gmc_v7_0_get_vm_pte_flags
>> + .get_vm_pte_flags = gmc_v7_0_get_vm_pte_flags,
>> + .get_vm_pde = gmc_v7_0_get_vm_pde
>> };
>>
>> 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 3b5ea0f..f05c034 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> @@ -656,6 +656,12 @@ static uint64_t gmc_v8_0_get_vm_pte_flags(struct
>> amdgpu_device *adev,
>> return pte_flag;
>> }
>>
>> +static uint64_t gmc_v8_0_get_vm_pde(struct amdgpu_device *adev,
>> uint64_t addr)
>> +{
>> + BUG_ON(addr & 0xFFF0000000000FFFULL);
>> + return addr;
>> +}
>> +
>> /**
>> * gmc_v8_0_set_fault_enable_default - update VM fault handling
>> *
>> @@ -1612,7 +1618,8 @@ static const struct amdgpu_gart_funcs
>> gmc_v8_0_gart_funcs = {
>> .flush_gpu_tlb = gmc_v8_0_gart_flush_gpu_tlb,
>> .set_pte_pde = gmc_v8_0_gart_set_pte_pde,
>> .set_prt = gmc_v8_0_set_prt,
>> - .get_vm_pte_flags = gmc_v8_0_get_vm_pte_flags
>> + .get_vm_pte_flags = gmc_v8_0_get_vm_pte_flags,
>> + .get_vm_pde = gmc_v8_0_get_vm_pde
>> };
>>
>> 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 19e1027..047b1a7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -358,17 +358,19 @@ static uint64_t
>> gmc_v9_0_get_vm_pte_flags(struct amdgpu_device *adev,
>> return pte_flag;
>> }
>>
>> -static u64 gmc_v9_0_adjust_mc_addr(struct amdgpu_device *adev, u64
>> mc_addr)
>> +static u64 gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, u64 addr)
>> {
>> - return adev->vm_manager.vram_base_offset + mc_addr -
>> adev->mc.vram_start;
>> + addr = adev->vm_manager.vram_base_offset + addr -
>> adev->mc.vram_start;
>> + BUG_ON(addr & 0xFFFF00000000003FULL);
>> + return addr;
>> }
>>
>> static const struct amdgpu_gart_funcs gmc_v9_0_gart_funcs = {
>> .flush_gpu_tlb = gmc_v9_0_gart_flush_gpu_tlb,
>> .set_pte_pde = gmc_v9_0_gart_set_pte_pde,
>> - .get_vm_pte_flags = gmc_v9_0_get_vm_pte_flags,
>> - .adjust_mc_addr = gmc_v9_0_adjust_mc_addr,
>> .get_invalidate_req = gmc_v9_0_get_invalidate_req,
>> + .get_vm_pte_flags = gmc_v9_0_get_vm_pte_flags,
>> + .get_vm_pde = gmc_v9_0_get_vm_pde
>> };
>>
>> static void gmc_v9_0_set_gart_funcs(struct amdgpu_device *adev)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> index 91cf7e6..fb6f4b3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> @@ -1143,10 +1143,8 @@ static void
>> sdma_v4_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
>> uint32_t req =
>> ring->adev->gart.gart_funcs->get_invalidate_req(vm_id);
>> unsigned eng = ring->vm_inv_eng;
>>
>> - pd_addr =
>> ring->adev->gart.gart_funcs->adjust_mc_addr(ring->adev, pd_addr);
>> - pd_addr = pd_addr | 0x1; /* valid bit */
>> - /* now only use physical base address of PDE and valid */
>> - BUG_ON(pd_addr & 0xFFFF00000000003EULL);
>> + pd_addr = amdgpu_gart_get_vm_pde(ring->adev, pd_addr);
>> + pd_addr |= AMDGPU_PTE_VALID;
>>
>> amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_SRBM_WRITE) |
>> SDMA_PKT_SRBM_WRITE_HEADER_BYTE_EN(0xf));
>> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
>> b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
>> index 22f42f3..1997ec8 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
>> @@ -1316,10 +1316,8 @@ static void uvd_v7_0_ring_emit_vm_flush(struct
>> amdgpu_ring *ring,
>> uint32_t data0, data1, mask;
>> unsigned eng = ring->vm_inv_eng;
>>
>> - pd_addr =
>> ring->adev->gart.gart_funcs->adjust_mc_addr(ring->adev, pd_addr);
>> - pd_addr = pd_addr | 0x1; /* valid bit */
>> - /* now only use physical base address of PDE and valid */
>> - BUG_ON(pd_addr & 0xFFFF00000000003EULL);
>> + pd_addr = amdgpu_gart_get_vm_pde(ring->adev, pd_addr);
>> + pd_addr |= AMDGPU_PTE_VALID;
>>
>> data0 = (hub->ctx0_ptb_addr_hi32 + vm_id * 2) << 2;
>> data1 = upper_32_bits(pd_addr);
>> @@ -1358,10 +1356,8 @@ static void
>> uvd_v7_0_enc_ring_emit_vm_flush(struct amdgpu_ring *ring,
>> uint32_t req =
>> ring->adev->gart.gart_funcs->get_invalidate_req(vm_id);
>> unsigned eng = ring->vm_inv_eng;
>>
>> - pd_addr =
>> ring->adev->gart.gart_funcs->adjust_mc_addr(ring->adev, pd_addr);
>> - pd_addr = pd_addr | 0x1; /* valid bit */
>> - /* now only use physical base address of PDE and valid */
>> - BUG_ON(pd_addr & 0xFFFF00000000003EULL);
>> + pd_addr = amdgpu_gart_get_vm_pde(ring->adev, pd_addr);
>> + pd_addr |= AMDGPU_PTE_VALID;
>>
>> amdgpu_ring_write(ring, HEVC_ENC_CMD_REG_WRITE);
>> amdgpu_ring_write(ring, (hub->ctx0_ptb_addr_hi32 + vm_id *
>> 2) << 2);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
>> b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
>> index 07b2ac7..8dbd0b7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
>> @@ -926,10 +926,8 @@ static void vce_v4_0_emit_vm_flush(struct
>> amdgpu_ring *ring,
>> uint32_t req =
>> ring->adev->gart.gart_funcs->get_invalidate_req(vm_id);
>> unsigned eng = ring->vm_inv_eng;
>>
>> - pd_addr =
>> ring->adev->gart.gart_funcs->adjust_mc_addr(ring->adev, pd_addr);
>> - pd_addr = pd_addr | 0x1; /* valid bit */
>> - /* now only use physical base address of PDE and valid */
>> - BUG_ON(pd_addr & 0xFFFF00000000003EULL);
>> + pd_addr = amdgpu_gart_get_vm_pde(ring->adev, pd_addr);
>> + pd_addr |= AMDGPU_PTE_VALID;
>>
>> amdgpu_ring_write(ring, VCE_CMD_REG_WRITE);
>> amdgpu_ring_write(ring, (hub->ctx0_ptb_addr_hi32 + vm_id *
>> 2) << 2);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> index ad1862f..b2d995b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> @@ -882,9 +882,8 @@ static void
>> vcn_v1_0_dec_ring_emit_vm_flush(struct amdgpu_ring *ring,
>> uint32_t data0, data1, mask;
>> unsigned eng = ring->vm_inv_eng;
>>
>> - pd_addr = pd_addr | 0x1; /* valid bit */
>> - /* now only use physical base address of PDE and valid */
>> - BUG_ON(pd_addr & 0xFFFF00000000003EULL);
>> + pd_addr = amdgpu_gart_get_vm_pde(ring->adev, pd_addr);
>> + pd_addr |= AMDGPU_PTE_VALID;
>>
>> data0 = (hub->ctx0_ptb_addr_hi32 + vm_id * 2) << 2;
>> data1 = upper_32_bits(pd_addr);
>>
More information about the amd-gfx
mailing list