Re: [PATCH 1/7] drm/amdgpu: cleanup adjust_mc_addr handling v2

Wed, 24 May 2017 02:19:34 -0700 

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);



_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx























					Reply via email to