[PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Christian König]

Only use the lower address space on GMC9 for the system domain.
Otherwise we would need to sign extend GMC addresses.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index e44b5191735d..d982956c8329 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
 	if (r)
 		return r;
 
-	/* Set the internal MC address mask
-	 * This is the max address of the GPU's
-	 * internal address space.
+	/* Use only the lower range for the internal MC address mask. This is
+	 * the max address of the GPU's internal address space.
 	 */
-	adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
+	adev->gmc.mc_mask = 0x7fffffffffffULL;
 
 	/* set DMA mask + need_dma32 flags.
 	 * PCIE - can handle 44-bits.
-- 
2.17.1

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

[PATCH 02/10] drm/amdgpu: move amdgpu_device_(vram|gtt)_location

[Christian König]

Move that into amdgpu_gmc.c since we are really deadling with GMC
address space here.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  4 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 65 ----------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    | 64 +++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h    |  4 ++
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c      |  4 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c      |  4 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c      |  4 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c      |  4 +-
 8 files changed, 76 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 340e40d03d54..09bdedfc91c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1166,10 +1166,6 @@ bool amdgpu_device_need_post(struct amdgpu_device *adev);
 
 void amdgpu_cs_report_moved_bytes(struct amdgpu_device *adev, u64 num_bytes,
 				  u64 num_vis_bytes);
-void amdgpu_device_vram_location(struct amdgpu_device *adev,
-				 struct amdgpu_gmc *mc, u64 base);
-void amdgpu_device_gart_location(struct amdgpu_device *adev,
-				 struct amdgpu_gmc *mc);
 int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev);
 void amdgpu_device_program_register_sequence(struct amdgpu_device *adev,
 					     const u32 *registers,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 9f5e4be76d5e..0afc5e599683 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -651,71 +651,6 @@ void amdgpu_device_wb_free(struct amdgpu_device *adev, u32 wb)
 		__clear_bit(wb, adev->wb.used);
 }
 
-/**
- * amdgpu_device_vram_location - try to find VRAM location
- *
- * @adev: amdgpu device structure holding all necessary informations
- * @mc: memory controller structure holding memory informations
- * @base: base address at which to put VRAM
- *
- * Function will try to place VRAM at base address provided
- * as parameter.
- */
-void amdgpu_device_vram_location(struct amdgpu_device *adev,
-				 struct amdgpu_gmc *mc, u64 base)
-{
-	uint64_t limit = (uint64_t)amdgpu_vram_limit << 20;
-
-	mc->vram_start = base;
-	mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
-	if (limit && limit < mc->real_vram_size)
-		mc->real_vram_size = limit;
-	dev_info(adev->dev, "VRAM: %lluM 0x%016llX - 0x%016llX (%lluM used)\n",
-			mc->mc_vram_size >> 20, mc->vram_start,
-			mc->vram_end, mc->real_vram_size >> 20);
-}
-
-/**
- * amdgpu_device_gart_location - try to find GART location
- *
- * @adev: amdgpu device structure holding all necessary informations
- * @mc: memory controller structure holding memory informations
- *
- * Function will place try to place GART before or after VRAM.
- *
- * If GART size is bigger than space left then we ajust GART size.
- * Thus function will never fails.
- */
-void amdgpu_device_gart_location(struct amdgpu_device *adev,
-				 struct amdgpu_gmc *mc)
-{
-	u64 size_af, size_bf;
-
-	mc->gart_size += adev->pm.smu_prv_buffer_size;
-
-	size_af = adev->gmc.mc_mask - mc->vram_end;
-	size_bf = mc->vram_start;
-	if (size_bf > size_af) {
-		if (mc->gart_size > size_bf) {
-			dev_warn(adev->dev, "limiting GART\n");
-			mc->gart_size = size_bf;
-		}
-		mc->gart_start = 0;
-	} else {
-		if (mc->gart_size > size_af) {
-			dev_warn(adev->dev, "limiting GART\n");
-			mc->gart_size = size_af;
-		}
-		/* VCE doesn't like it when BOs cross a 4GB segment, so align
-		 * the GART base on a 4GB boundary as well.
-		 */
-		mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
-	}
-	mc->gart_end = mc->gart_start + mc->gart_size - 1;
-	dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
-			mc->gart_size >> 20, mc->gart_start, mc->gart_end);
-}
-
 /**
  * amdgpu_device_resize_fb_bar - try to resize FB BAR
  *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index a249931ef512..72dffa3fd194 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -78,3 +78,67 @@ uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo)
 	}
 	return pd_addr;
 }
+
+/**
+ * amdgpu_gmc_vram_location - try to find VRAM location
+ *
+ * @adev: amdgpu device structure holding all necessary informations
+ * @mc: memory controller structure holding memory informations
+ * @base: base address at which to put VRAM
+ *
+ * Function will try to place VRAM at base address provided
+ * as parameter.
+ */
+void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
+			      u64 base)
+{
+	uint64_t limit = (uint64_t)amdgpu_vram_limit << 20;
+
+	mc->vram_start = base;
+	mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
+	if (limit && limit < mc->real_vram_size)
+		mc->real_vram_size = limit;
+	dev_info(adev->dev, "VRAM: %lluM 0x%016llX - 0x%016llX (%lluM used)\n",
+			mc->mc_vram_size >> 20, mc->vram_start,
+			mc->vram_end, mc->real_vram_size >> 20);
+}
+
+/**
+ * amdgpu_gmc_gart_location - try to find GART location
+ *
+ * @adev: amdgpu device structure holding all necessary informations
+ * @mc: memory controller structure holding memory informations
+ *
+ * Function will place try to place GART before or after VRAM.
+ *
+ * If GART size is bigger than space left then we ajust GART size.
+ * Thus function will never fails.
+ */
+void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
+{
+	u64 size_af, size_bf;
+
+	mc->gart_size += adev->pm.smu_prv_buffer_size;
+
+	size_af = adev->gmc.mc_mask - mc->vram_end;
+	size_bf = mc->vram_start;
+	if (size_bf > size_af) {
+		if (mc->gart_size > size_bf) {
+			dev_warn(adev->dev, "limiting GART\n");
+			mc->gart_size = size_bf;
+		}
+		mc->gart_start = 0;
+	} else {
+		if (mc->gart_size > size_af) {
+			dev_warn(adev->dev, "limiting GART\n");
+			mc->gart_size = size_af;
+		}
+		/* VCE doesn't like it when BOs cross a 4GB segment, so align
+		 * the GART base on a 4GB boundary as well.
+		 */
+		mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
+	}
+	mc->gart_end = mc->gart_start + mc->gart_size - 1;
+	dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
+			mc->gart_size >> 20, mc->gart_start, mc->gart_end);
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index 0d2c9f65ca13..72fcc9338f5e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -134,5 +134,9 @@ static inline bool amdgpu_gmc_vram_full_visible(struct amdgpu_gmc *gmc)
 void amdgpu_gmc_get_pde_for_bo(struct amdgpu_bo *bo, int level,
 			       uint64_t *addr, uint64_t *flags);
 uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo);
+void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
+			      u64 base);
+void amdgpu_gmc_gart_location(struct amdgpu_device *adev,
+			      struct amdgpu_gmc *mc);
 
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index 543287e5d67b..c2866e1acbef 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -224,8 +224,8 @@ static void gmc_v6_0_vram_gtt_location(struct amdgpu_device *adev,
 	u64 base = RREG32(mmMC_VM_FB_LOCATION) & 0xFFFF;
 	base <<= 24;
 
-	amdgpu_device_vram_location(adev, &adev->gmc, base);
-	amdgpu_device_gart_location(adev, mc);
+	amdgpu_gmc_vram_location(adev, &adev->gmc, base);
+	amdgpu_gmc_gart_location(adev, mc);
 }
 
 static void gmc_v6_0_mc_program(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 060c79afef80..cbbbb6bbf782 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -244,8 +244,8 @@ static void gmc_v7_0_vram_gtt_location(struct amdgpu_device *adev,
 	u64 base = RREG32(mmMC_VM_FB_LOCATION) & 0xFFFF;
 	base <<= 24;
 
-	amdgpu_device_vram_location(adev, &adev->gmc, base);
-	amdgpu_device_gart_location(adev, mc);
+	amdgpu_gmc_vram_location(adev, &adev->gmc, base);
+	amdgpu_gmc_gart_location(adev, mc);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 3fe9b9755cf7..fe26bd08b593 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -410,8 +410,8 @@ static void gmc_v8_0_vram_gtt_location(struct amdgpu_device *adev,
 		base = RREG32(mmMC_VM_FB_LOCATION) & 0xFFFF;
 	base <<= 24;
 
-	amdgpu_device_vram_location(adev, &adev->gmc, base);
-	amdgpu_device_gart_location(adev, mc);
+	amdgpu_gmc_vram_location(adev, &adev->gmc, base);
+	amdgpu_gmc_gart_location(adev, mc);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index d982956c8329..60c8f848c102 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -749,8 +749,8 @@ static void gmc_v9_0_vram_gtt_location(struct amdgpu_device *adev,
 	u64 base = 0;
 	if (!amdgpu_sriov_vf(adev))
 		base = mmhub_v1_0_get_fb_location(adev);
-	amdgpu_device_vram_location(adev, &adev->gmc, base);
-	amdgpu_device_gart_location(adev, mc);
+	amdgpu_gmc_vram_location(adev, &adev->gmc, base);
+	amdgpu_gmc_gart_location(adev, mc);
 	/* base offset of vram pages */
 	adev->vm_manager.vram_base_offset = gfxhub_v1_0_get_mc_fb_offset(adev);
 }
-- 
2.17.1

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

Re: [PATCH 02/10] drm/amdgpu: move amdgpu_device_(vram|gtt)_location

[Alex Deucher]

On Mon, Aug 27, 2018 at 12:56 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Move that into amdgpu_gmc.c since we are really deadling with GMC
> address space here.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  4 --
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 65 ----------------------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    | 64 +++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h    |  4 ++
>  drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c      |  4 +-
>  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c      |  4 +-
>  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c      |  4 +-
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c      |  4 +-
>  8 files changed, 76 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 340e40d03d54..09bdedfc91c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1166,10 +1166,6 @@ bool amdgpu_device_need_post(struct amdgpu_device *adev);
>
>  void amdgpu_cs_report_moved_bytes(struct amdgpu_device *adev, u64 num_bytes,
>                                   u64 num_vis_bytes);
> -void amdgpu_device_vram_location(struct amdgpu_device *adev,
> -                                struct amdgpu_gmc *mc, u64 base);
> -void amdgpu_device_gart_location(struct amdgpu_device *adev,
> -                                struct amdgpu_gmc *mc);
>  int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev);
>  void amdgpu_device_program_register_sequence(struct amdgpu_device *adev,
>                                              const u32 *registers,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 9f5e4be76d5e..0afc5e599683 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -651,71 +651,6 @@ void amdgpu_device_wb_free(struct amdgpu_device *adev, u32 wb)
>                 __clear_bit(wb, adev->wb.used);
>  }
>
> -/**
> - * amdgpu_device_vram_location - try to find VRAM location
> - *
> - * @adev: amdgpu device structure holding all necessary informations
> - * @mc: memory controller structure holding memory informations
> - * @base: base address at which to put VRAM
> - *
> - * Function will try to place VRAM at base address provided
> - * as parameter.
> - */
> -void amdgpu_device_vram_location(struct amdgpu_device *adev,
> -                                struct amdgpu_gmc *mc, u64 base)
> -{
> -       uint64_t limit = (uint64_t)amdgpu_vram_limit << 20;
> -
> -       mc->vram_start = base;
> -       mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
> -       if (limit && limit < mc->real_vram_size)
> -               mc->real_vram_size = limit;
> -       dev_info(adev->dev, "VRAM: %lluM 0x%016llX - 0x%016llX (%lluM used)\n",
> -                       mc->mc_vram_size >> 20, mc->vram_start,
> -                       mc->vram_end, mc->real_vram_size >> 20);
> -}
> -
> -/**
> - * amdgpu_device_gart_location - try to find GART location
> - *
> - * @adev: amdgpu device structure holding all necessary informations
> - * @mc: memory controller structure holding memory informations
> - *
> - * Function will place try to place GART before or after VRAM.
> - *
> - * If GART size is bigger than space left then we ajust GART size.
> - * Thus function will never fails.
> - */
> -void amdgpu_device_gart_location(struct amdgpu_device *adev,
> -                                struct amdgpu_gmc *mc)
> -{
> -       u64 size_af, size_bf;
> -
> -       mc->gart_size += adev->pm.smu_prv_buffer_size;
> -
> -       size_af = adev->gmc.mc_mask - mc->vram_end;
> -       size_bf = mc->vram_start;
> -       if (size_bf > size_af) {
> -               if (mc->gart_size > size_bf) {
> -                       dev_warn(adev->dev, "limiting GART\n");
> -                       mc->gart_size = size_bf;
> -               }
> -               mc->gart_start = 0;
> -       } else {
> -               if (mc->gart_size > size_af) {
> -                       dev_warn(adev->dev, "limiting GART\n");
> -                       mc->gart_size = size_af;
> -               }
> -               /* VCE doesn't like it when BOs cross a 4GB segment, so align
> -                * the GART base on a 4GB boundary as well.
> -                */
> -               mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
> -       }
> -       mc->gart_end = mc->gart_start + mc->gart_size - 1;
> -       dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
> -                       mc->gart_size >> 20, mc->gart_start, mc->gart_end);
> -}
> -
>  /**
>   * amdgpu_device_resize_fb_bar - try to resize FB BAR
>   *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index a249931ef512..72dffa3fd194 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -78,3 +78,67 @@ uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo)
>         }
>         return pd_addr;
>  }
> +
> +/**
> + * amdgpu_gmc_vram_location - try to find VRAM location
> + *
> + * @adev: amdgpu device structure holding all necessary informations
> + * @mc: memory controller structure holding memory informations
> + * @base: base address at which to put VRAM
> + *
> + * Function will try to place VRAM at base address provided
> + * as parameter.
> + */
> +void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
> +                             u64 base)
> +{
> +       uint64_t limit = (uint64_t)amdgpu_vram_limit << 20;
> +
> +       mc->vram_start = base;
> +       mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
> +       if (limit && limit < mc->real_vram_size)
> +               mc->real_vram_size = limit;
> +       dev_info(adev->dev, "VRAM: %lluM 0x%016llX - 0x%016llX (%lluM used)\n",
> +                       mc->mc_vram_size >> 20, mc->vram_start,
> +                       mc->vram_end, mc->real_vram_size >> 20);
> +}
> +
> +/**
> + * amdgpu_gmc_gart_location - try to find GART location
> + *
> + * @adev: amdgpu device structure holding all necessary informations
> + * @mc: memory controller structure holding memory informations
> + *
> + * Function will place try to place GART before or after VRAM.
> + *
> + * If GART size is bigger than space left then we ajust GART size.
> + * Thus function will never fails.
> + */
> +void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
> +{
> +       u64 size_af, size_bf;
> +
> +       mc->gart_size += adev->pm.smu_prv_buffer_size;
> +
> +       size_af = adev->gmc.mc_mask - mc->vram_end;
> +       size_bf = mc->vram_start;
> +       if (size_bf > size_af) {
> +               if (mc->gart_size > size_bf) {
> +                       dev_warn(adev->dev, "limiting GART\n");
> +                       mc->gart_size = size_bf;
> +               }
> +               mc->gart_start = 0;
> +       } else {
> +               if (mc->gart_size > size_af) {
> +                       dev_warn(adev->dev, "limiting GART\n");
> +                       mc->gart_size = size_af;
> +               }
> +               /* VCE doesn't like it when BOs cross a 4GB segment, so align
> +                * the GART base on a 4GB boundary as well.
> +                */
> +               mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
> +       }
> +       mc->gart_end = mc->gart_start + mc->gart_size - 1;
> +       dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
> +                       mc->gart_size >> 20, mc->gart_start, mc->gart_end);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 0d2c9f65ca13..72fcc9338f5e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -134,5 +134,9 @@ static inline bool amdgpu_gmc_vram_full_visible(struct amdgpu_gmc *gmc)
>  void amdgpu_gmc_get_pde_for_bo(struct amdgpu_bo *bo, int level,
>                                uint64_t *addr, uint64_t *flags);
>  uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo);
> +void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
> +                             u64 base);
> +void amdgpu_gmc_gart_location(struct amdgpu_device *adev,
> +                             struct amdgpu_gmc *mc);
>
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index 543287e5d67b..c2866e1acbef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -224,8 +224,8 @@ static void gmc_v6_0_vram_gtt_location(struct amdgpu_device *adev,
>         u64 base = RREG32(mmMC_VM_FB_LOCATION) & 0xFFFF;
>         base <<= 24;
>
> -       amdgpu_device_vram_location(adev, &adev->gmc, base);
> -       amdgpu_device_gart_location(adev, mc);
> +       amdgpu_gmc_vram_location(adev, &adev->gmc, base);
> +       amdgpu_gmc_gart_location(adev, mc);
>  }
>
>  static void gmc_v6_0_mc_program(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 060c79afef80..cbbbb6bbf782 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -244,8 +244,8 @@ static void gmc_v7_0_vram_gtt_location(struct amdgpu_device *adev,
>         u64 base = RREG32(mmMC_VM_FB_LOCATION) & 0xFFFF;
>         base <<= 24;
>
> -       amdgpu_device_vram_location(adev, &adev->gmc, base);
> -       amdgpu_device_gart_location(adev, mc);
> +       amdgpu_gmc_vram_location(adev, &adev->gmc, base);
> +       amdgpu_gmc_gart_location(adev, mc);
>  }
>
>  /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 3fe9b9755cf7..fe26bd08b593 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -410,8 +410,8 @@ static void gmc_v8_0_vram_gtt_location(struct amdgpu_device *adev,
>                 base = RREG32(mmMC_VM_FB_LOCATION) & 0xFFFF;
>         base <<= 24;
>
> -       amdgpu_device_vram_location(adev, &adev->gmc, base);
> -       amdgpu_device_gart_location(adev, mc);
> +       amdgpu_gmc_vram_location(adev, &adev->gmc, base);
> +       amdgpu_gmc_gart_location(adev, mc);
>  }
>
>  /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index d982956c8329..60c8f848c102 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -749,8 +749,8 @@ static void gmc_v9_0_vram_gtt_location(struct amdgpu_device *adev,
>         u64 base = 0;
>         if (!amdgpu_sriov_vf(adev))
>                 base = mmhub_v1_0_get_fb_location(adev);
> -       amdgpu_device_vram_location(adev, &adev->gmc, base);
> -       amdgpu_device_gart_location(adev, mc);
> +       amdgpu_gmc_vram_location(adev, &adev->gmc, base);
> +       amdgpu_gmc_gart_location(adev, mc);
>         /* base offset of vram pages */
>         adev->vm_manager.vram_base_offset = gfxhub_v1_0_get_mc_fb_offset(adev);
>  }
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 02/10] drm/amdgpu: move amdgpu_device_(vram|gtt)_location

[Zhang, Jerry (Junwei)]

On 08/28/2018 02:59 AM, Alex Deucher wrote:
> On Mon, Aug 27, 2018 at 12:56 PM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>>
>> Move that into amdgpu_gmc.c since we are really deadling with GMC
>> address space here.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  4 --
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 65 ----------------------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    | 64 +++++++++++++++++++++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h    |  4 ++
>>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c      |  4 +-
>>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c      |  4 +-
>>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c      |  4 +-
>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c      |  4 +-
>>   8 files changed, 76 insertions(+), 77 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 340e40d03d54..09bdedfc91c7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -1166,10 +1166,6 @@ bool amdgpu_device_need_post(struct amdgpu_device *adev);
>>
>>   void amdgpu_cs_report_moved_bytes(struct amdgpu_device *adev, u64 num_bytes,
>>                                    u64 num_vis_bytes);
>> -void amdgpu_device_vram_location(struct amdgpu_device *adev,
>> -                                struct amdgpu_gmc *mc, u64 base);
>> -void amdgpu_device_gart_location(struct amdgpu_device *adev,
>> -                                struct amdgpu_gmc *mc);
>>   int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev);
>>   void amdgpu_device_program_register_sequence(struct amdgpu_device *adev,
>>                                               const u32 *registers,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 9f5e4be76d5e..0afc5e599683 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -651,71 +651,6 @@ void amdgpu_device_wb_free(struct amdgpu_device *adev, u32 wb)
>>                  __clear_bit(wb, adev->wb.used);
>>   }
>>
>> -/**
>> - * amdgpu_device_vram_location - try to find VRAM location
>> - *
>> - * @adev: amdgpu device structure holding all necessary informations
>> - * @mc: memory controller structure holding memory informations
>> - * @base: base address at which to put VRAM
>> - *
>> - * Function will try to place VRAM at base address provided
>> - * as parameter.
>> - */
>> -void amdgpu_device_vram_location(struct amdgpu_device *adev,
>> -                                struct amdgpu_gmc *mc, u64 base)
>> -{
>> -       uint64_t limit = (uint64_t)amdgpu_vram_limit << 20;
>> -
>> -       mc->vram_start = base;
>> -       mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
>> -       if (limit && limit < mc->real_vram_size)
>> -               mc->real_vram_size = limit;
>> -       dev_info(adev->dev, "VRAM: %lluM 0x%016llX - 0x%016llX (%lluM used)\n",
>> -                       mc->mc_vram_size >> 20, mc->vram_start,
>> -                       mc->vram_end, mc->real_vram_size >> 20);
>> -}
>> -
>> -/**
>> - * amdgpu_device_gart_location - try to find GART location
>> - *
>> - * @adev: amdgpu device structure holding all necessary informations
>> - * @mc: memory controller structure holding memory informations
>> - *
>> - * Function will place try to place GART before or after VRAM.
>> - *
>> - * If GART size is bigger than space left then we ajust GART size.
>> - * Thus function will never fails.
>> - */
>> -void amdgpu_device_gart_location(struct amdgpu_device *adev,
>> -                                struct amdgpu_gmc *mc)
>> -{
>> -       u64 size_af, size_bf;
>> -
>> -       mc->gart_size += adev->pm.smu_prv_buffer_size;
>> -
>> -       size_af = adev->gmc.mc_mask - mc->vram_end;
>> -       size_bf = mc->vram_start;
>> -       if (size_bf > size_af) {
>> -               if (mc->gart_size > size_bf) {
>> -                       dev_warn(adev->dev, "limiting GART\n");
>> -                       mc->gart_size = size_bf;
>> -               }
>> -               mc->gart_start = 0;
>> -       } else {
>> -               if (mc->gart_size > size_af) {
>> -                       dev_warn(adev->dev, "limiting GART\n");
>> -                       mc->gart_size = size_af;
>> -               }
>> -               /* VCE doesn't like it when BOs cross a 4GB segment, so align
>> -                * the GART base on a 4GB boundary as well.
>> -                */
>> -               mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
>> -       }
>> -       mc->gart_end = mc->gart_start + mc->gart_size - 1;
>> -       dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
>> -                       mc->gart_size >> 20, mc->gart_start, mc->gart_end);
>> -}
>> -
>>   /**
>>    * amdgpu_device_resize_fb_bar - try to resize FB BAR
>>    *
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> index a249931ef512..72dffa3fd194 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> @@ -78,3 +78,67 @@ uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo)
>>          }
>>          return pd_addr;
>>   }
>> +
>> +/**
>> + * amdgpu_gmc_vram_location - try to find VRAM location
>> + *
>> + * @adev: amdgpu device structure holding all necessary informations
>> + * @mc: memory controller structure holding memory informations
>> + * @base: base address at which to put VRAM
>> + *
>> + * Function will try to place VRAM at base address provided
>> + * as parameter.
>> + */
>> +void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
>> +                             u64 base)
>> +{
>> +       uint64_t limit = (uint64_t)amdgpu_vram_limit << 20;
>> +
>> +       mc->vram_start = base;
>> +       mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
>> +       if (limit && limit < mc->real_vram_size)
>> +               mc->real_vram_size = limit;
>> +       dev_info(adev->dev, "VRAM: %lluM 0x%016llX - 0x%016llX (%lluM used)\n",
>> +                       mc->mc_vram_size >> 20, mc->vram_start,
>> +                       mc->vram_end, mc->real_vram_size >> 20);
>> +}
>> +
>> +/**
>> + * amdgpu_gmc_gart_location - try to find GART location
>> + *
>> + * @adev: amdgpu device structure holding all necessary informations
>> + * @mc: memory controller structure holding memory informations
>> + *
>> + * Function will place try to place GART before or after VRAM.
>> + *
>> + * If GART size is bigger than space left then we ajust GART size.
>> + * Thus function will never fails.
>> + */
>> +void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>> +{
>> +       u64 size_af, size_bf;
>> +
>> +       mc->gart_size += adev->pm.smu_prv_buffer_size;
>> +
>> +       size_af = adev->gmc.mc_mask - mc->vram_end;
>> +       size_bf = mc->vram_start;
>> +       if (size_bf > size_af) {
>> +               if (mc->gart_size > size_bf) {
>> +                       dev_warn(adev->dev, "limiting GART\n");
>> +                       mc->gart_size = size_bf;
>> +               }
>> +               mc->gart_start = 0;
>> +       } else {
>> +               if (mc->gart_size > size_af) {
>> +                       dev_warn(adev->dev, "limiting GART\n");
>> +                       mc->gart_size = size_af;
>> +               }
>> +               /* VCE doesn't like it when BOs cross a 4GB segment, so align
>> +                * the GART base on a 4GB boundary as well.
>> +                */
>> +               mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
>> +       }
>> +       mc->gart_end = mc->gart_start + mc->gart_size - 1;
>> +       dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
>> +                       mc->gart_size >> 20, mc->gart_start, mc->gart_end);
>> +}
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> index 0d2c9f65ca13..72fcc9338f5e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> @@ -134,5 +134,9 @@ static inline bool amdgpu_gmc_vram_full_visible(struct amdgpu_gmc *gmc)
>>   void amdgpu_gmc_get_pde_for_bo(struct amdgpu_bo *bo, int level,
>>                                 uint64_t *addr, uint64_t *flags);
>>   uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo);
>> +void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
>> +                             u64 base);
>> +void amdgpu_gmc_gart_location(struct amdgpu_device *adev,
>> +                             struct amdgpu_gmc *mc);
>>
>>   #endif
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> index 543287e5d67b..c2866e1acbef 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> @@ -224,8 +224,8 @@ static void gmc_v6_0_vram_gtt_location(struct amdgpu_device *adev,
>>          u64 base = RREG32(mmMC_VM_FB_LOCATION) & 0xFFFF;
>>          base <<= 24;
>>
>> -       amdgpu_device_vram_location(adev, &adev->gmc, base);
>> -       amdgpu_device_gart_location(adev, mc);
>> +       amdgpu_gmc_vram_location(adev, &adev->gmc, base);
>> +       amdgpu_gmc_gart_location(adev, mc);
>>   }
>>
>>   static void gmc_v6_0_mc_program(struct amdgpu_device *adev)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> index 060c79afef80..cbbbb6bbf782 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> @@ -244,8 +244,8 @@ static void gmc_v7_0_vram_gtt_location(struct amdgpu_device *adev,
>>          u64 base = RREG32(mmMC_VM_FB_LOCATION) & 0xFFFF;
>>          base <<= 24;
>>
>> -       amdgpu_device_vram_location(adev, &adev->gmc, base);
>> -       amdgpu_device_gart_location(adev, mc);
>> +       amdgpu_gmc_vram_location(adev, &adev->gmc, base);
>> +       amdgpu_gmc_gart_location(adev, mc);
>>   }
>>
>>   /**
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> index 3fe9b9755cf7..fe26bd08b593 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> @@ -410,8 +410,8 @@ static void gmc_v8_0_vram_gtt_location(struct amdgpu_device *adev,
>>                  base = RREG32(mmMC_VM_FB_LOCATION) & 0xFFFF;
>>          base <<= 24;
>>
>> -       amdgpu_device_vram_location(adev, &adev->gmc, base);
>> -       amdgpu_device_gart_location(adev, mc);
>> +       amdgpu_gmc_vram_location(adev, &adev->gmc, base);
>> +       amdgpu_gmc_gart_location(adev, mc);
>>   }
>>
>>   /**
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index d982956c8329..60c8f848c102 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -749,8 +749,8 @@ static void gmc_v9_0_vram_gtt_location(struct amdgpu_device *adev,
>>          u64 base = 0;
>>          if (!amdgpu_sriov_vf(adev))
>>                  base = mmhub_v1_0_get_fb_location(adev);
>> -       amdgpu_device_vram_location(adev, &adev->gmc, base);
>> -       amdgpu_device_gart_location(adev, mc);
>> +       amdgpu_gmc_vram_location(adev, &adev->gmc, base);
>> +       amdgpu_gmc_gart_location(adev, mc);
>>          /* base offset of vram pages */
>>          adev->vm_manager.vram_base_offset = gfxhub_v1_0_get_mc_fb_offset(adev);
>>   }
>> --
>> 2.17.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 03/10] drm/amdgpu: fix amdgpu_gmc_gart_location a little bit

[Christian König]

Improve the VCE limitation handling.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 28 ++++++++++++-------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 72dffa3fd194..8269197df8e0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -120,24 +120,22 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
 
 	mc->gart_size += adev->pm.smu_prv_buffer_size;
 
-	size_af = adev->gmc.mc_mask - mc->vram_end;
+	/* VCE doesn't like it when BOs cross a 4GB segment, so align
+	 * the GART base on a 4GB boundary as well.
+	 */
 	size_bf = mc->vram_start;
-	if (size_bf > size_af) {
-		if (mc->gart_size > size_bf) {
-			dev_warn(adev->dev, "limiting GART\n");
-			mc->gart_size = size_bf;
-		}
+	size_af = adev->gmc.mc_mask + 1 -
+		ALIGN(mc->vram_end + 1, 0x100000000ULL);
+
+	if (mc->gart_size > max(size_bf, size_af)) {
+		dev_warn(adev->dev, "limiting GART\n");
+		mc->gart_size = max(size_bf, size_af);
+	}
+
+	if (size_bf > size_af)
 		mc->gart_start = 0;
-	} else {
-		if (mc->gart_size > size_af) {
-			dev_warn(adev->dev, "limiting GART\n");
-			mc->gart_size = size_af;
-		}
-		/* VCE doesn't like it when BOs cross a 4GB segment, so align
-		 * the GART base on a 4GB boundary as well.
-		 */
+	else
 		mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
-	}
 	mc->gart_end = mc->gart_start + mc->gart_size - 1;
 	dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
 			mc->gart_size >> 20, mc->gart_start, mc->gart_end);
-- 
2.17.1

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

Re: [PATCH 03/10] drm/amdgpu: fix amdgpu_gmc_gart_location a little bit

[Alex Deucher]

On Mon, Aug 27, 2018 at 12:55 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Improve the VCE limitation handling.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 28 ++++++++++++-------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 72dffa3fd194..8269197df8e0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -120,24 +120,22 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>
>         mc->gart_size += adev->pm.smu_prv_buffer_size;
>
> -       size_af = adev->gmc.mc_mask - mc->vram_end;
> +       /* VCE doesn't like it when BOs cross a 4GB segment, so align
> +        * the GART base on a 4GB boundary as well.
> +        */
>         size_bf = mc->vram_start;
> -       if (size_bf > size_af) {
> -               if (mc->gart_size > size_bf) {
> -                       dev_warn(adev->dev, "limiting GART\n");
> -                       mc->gart_size = size_bf;
> -               }
> +       size_af = adev->gmc.mc_mask + 1 -
> +               ALIGN(mc->vram_end + 1, 0x100000000ULL);

Is it worth limiting this to asics with VCE support?  Probably not a
big deal either way.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> +
> +       if (mc->gart_size > max(size_bf, size_af)) {
> +               dev_warn(adev->dev, "limiting GART\n");
> +               mc->gart_size = max(size_bf, size_af);
> +       }
> +
> +       if (size_bf > size_af)
>                 mc->gart_start = 0;
> -       } else {
> -               if (mc->gart_size > size_af) {
> -                       dev_warn(adev->dev, "limiting GART\n");
> -                       mc->gart_size = size_af;
> -               }
> -               /* VCE doesn't like it when BOs cross a 4GB segment, so align
> -                * the GART base on a 4GB boundary as well.
> -                */
> +       else
>                 mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
> -       }
>         mc->gart_end = mc->gart_start + mc->gart_size - 1;
>         dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
>                         mc->gart_size >> 20, mc->gart_start, mc->gart_end);
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 03/10] drm/amdgpu: fix amdgpu_gmc_gart_location a little bit

[Christian König]

Am 27.08.2018 um 21:03 schrieb Alex Deucher:
> On Mon, Aug 27, 2018 at 12:55 PM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Improve the VCE limitation handling.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 28 ++++++++++++-------------
>>   1 file changed, 13 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> index 72dffa3fd194..8269197df8e0 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> @@ -120,24 +120,22 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>>
>>          mc->gart_size += adev->pm.smu_prv_buffer_size;
>>
>> -       size_af = adev->gmc.mc_mask - mc->vram_end;
>> +       /* VCE doesn't like it when BOs cross a 4GB segment, so align
>> +        * the GART base on a 4GB boundary as well.
>> +        */
>>          size_bf = mc->vram_start;
>> -       if (size_bf > size_af) {
>> -               if (mc->gart_size > size_bf) {
>> -                       dev_warn(adev->dev, "limiting GART\n");
>> -                       mc->gart_size = size_bf;
>> -               }
>> +       size_af = adev->gmc.mc_mask + 1 -
>> +               ALIGN(mc->vram_end + 1, 0x100000000ULL);
> Is it worth limiting this to asics with VCE support?

Actually I think that a couple of blocks might have a problem with that. 
E.g. DCE/DCN seem to use a similar approach as VCE/VCN. Not 100% sure 
about UVD.

Only GFX/Compute/SDMA seems to have a true 48bit interface to the MC.

Regards,
Christian.

>    Probably not a
> big deal either way.
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
>> +
>> +       if (mc->gart_size > max(size_bf, size_af)) {
>> +               dev_warn(adev->dev, "limiting GART\n");
>> +               mc->gart_size = max(size_bf, size_af);
>> +       }
>> +
>> +       if (size_bf > size_af)
>>                  mc->gart_start = 0;
>> -       } else {
>> -               if (mc->gart_size > size_af) {
>> -                       dev_warn(adev->dev, "limiting GART\n");
>> -                       mc->gart_size = size_af;
>> -               }
>> -               /* VCE doesn't like it when BOs cross a 4GB segment, so align
>> -                * the GART base on a 4GB boundary as well.
>> -                */
>> +       else
>>                  mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
>> -       }
>>          mc->gart_end = mc->gart_start + mc->gart_size - 1;
>>          dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
>>                          mc->gart_size >> 20, mc->gart_start, mc->gart_end);
>> --
>> 2.17.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

Re: [PATCH 03/10] drm/amdgpu: fix amdgpu_gmc_gart_location a little bit

[Zhang, Jerry (Junwei)]

On 08/28/2018 12:53 AM, Christian König wrote:
> Improve the VCE limitation handling.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 28 ++++++++++++-------------
>   1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 72dffa3fd194..8269197df8e0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -120,24 +120,22 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>
>   	mc->gart_size += adev->pm.smu_prv_buffer_size;
>
> -	size_af = adev->gmc.mc_mask - mc->vram_end;
> +	/* VCE doesn't like it when BOs cross a 4GB segment, so align
> +	 * the GART base on a 4GB boundary as well.
> +	 */
>   	size_bf = mc->vram_start;
> -	if (size_bf > size_af) {
> -		if (mc->gart_size > size_bf) {
> -			dev_warn(adev->dev, "limiting GART\n");
> -			mc->gart_size = size_bf;
> -		}
> +	size_af = adev->gmc.mc_mask + 1 -
> +		ALIGN(mc->vram_end + 1, 0x100000000ULL);
> +
> +	if (mc->gart_size > max(size_bf, size_af)) {
> +		dev_warn(adev->dev, "limiting GART\n");
> +		mc->gart_size = max(size_bf, size_af);
> +	}
> +
> +	if (size_bf > size_af)
>   		mc->gart_start = 0;
> -	} else {
> -		if (mc->gart_size > size_af) {
> -			dev_warn(adev->dev, "limiting GART\n");
> -			mc->gart_size = size_af;
> -		}
> -		/* VCE doesn't like it when BOs cross a 4GB segment, so align
> -		 * the GART base on a 4GB boundary as well.
> -		 */
> +	else
>   		mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
> -	}
>   	mc->gart_end = mc->gart_start + mc->gart_size - 1;
>   	dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
>   			mc->gart_size >> 20, mc->gart_start, mc->gart_end);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 04/10] drm/amdgpu: use the smaller hole for GART

[Christian König]

Instead of the larger one use the smaller hole in the MC address
space for the GART mappings.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 8269197df8e0..265ec6807130 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -132,7 +132,8 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
 		mc->gart_size = max(size_bf, size_af);
 	}
 
-	if (size_bf > size_af)
+	if ((size_bf >= mc->gart_size && size_bf < size_af) ||
+	    (size_af < mc->gart_size))
 		mc->gart_start = 0;
 	else
 		mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
-- 
2.17.1

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

Re: [PATCH 04/10] drm/amdgpu: use the smaller hole for GART

[Alex Deucher]

On Mon, Aug 27, 2018 at 12:55 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Instead of the larger one use the smaller hole in the MC address
> space for the GART mappings.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 8269197df8e0..265ec6807130 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -132,7 +132,8 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>                 mc->gart_size = max(size_bf, size_af);
>         }
>
> -       if (size_bf > size_af)
> +       if ((size_bf >= mc->gart_size && size_bf < size_af) ||
> +           (size_af < mc->gart_size))
>                 mc->gart_start = 0;
>         else
>                 mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 04/10] drm/amdgpu: use the smaller hole for GART

[Zhang, Jerry (Junwei)]

On 08/28/2018 03:05 AM, Alex Deucher wrote:
> On Mon, Aug 27, 2018 at 12:55 PM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>>
>> Instead of the larger one use the smaller hole in the MC address
>> space for the GART mappings.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> index 8269197df8e0..265ec6807130 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> @@ -132,7 +132,8 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>>                  mc->gart_size = max(size_bf, size_af);
>>          }
>>
>> -       if (size_bf > size_af)
>> +       if ((size_bf >= mc->gart_size && size_bf < size_af) ||
>> +           (size_af < mc->gart_size))
>>                  mc->gart_start = 0;
>>          else
>>                  mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
>> --
>> 2.17.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 05/10] drm/amdgpu: put GART away from VRAM

[Christian König]

Always try to put the GART away from where VRAM is.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 265ec6807130..4331a0e25cdc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -136,7 +136,7 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
 	    (size_af < mc->gart_size))
 		mc->gart_start = 0;
 	else
-		mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
+		mc->gart_start = mc->mc_mask - mc->gart_size + 1;
 	mc->gart_end = mc->gart_start + mc->gart_size - 1;
 	dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
 			mc->gart_size >> 20, mc->gart_start, mc->gart_end);
-- 
2.17.1

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

Re: [PATCH 05/10] drm/amdgpu: put GART away from VRAM

[Zhang, Jerry (Junwei)]

On 08/28/2018 12:53 AM, Christian König wrote:
> Always try to put the GART away from where VRAM is.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 265ec6807130..4331a0e25cdc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -136,7 +136,7 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>   	    (size_af < mc->gart_size))
>   		mc->gart_start = 0;
>   	else
> -		mc->gart_start = ALIGN(mc->vram_end + 1, 0x100000000ULL);
> +		mc->gart_start = mc->mc_mask - mc->gart_size + 1;

it seems to break the VCE limitation about 4G alignment?

Jerry

>   	mc->gart_end = mc->gart_start + mc->gart_size - 1;
>   	dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
>   			mc->gart_size >> 20, mc->gart_start, mc->gart_end);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 06/10] drm/amdgpu: add amdgpu_gmc_agp_location

[Christian König]

Helper to figure out the location of the AGP BAR.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 42 +++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++
 2 files changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 4331a0e25cdc..eed5352f3136 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -141,3 +141,45 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
 	dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
 			mc->gart_size >> 20, mc->gart_start, mc->gart_end);
 }
+
+/**
+ * amdgpu_gmc_agp_location - try to find AGP location
+ * @adev: amdgpu device structure holding all necessary informations
+ * @mc: memory controller structure holding memory informations
+ *
+ * Function will place try to fina a place for the AGP BAR in the MC address
+ * space.
+ *
+ * AGP BAR will be assigned the largest available hole in the address space.
+ */
+void amdgpu_gmc_agp_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
+{
+	const uint64_t sixteen_gb = 1ULL << 34;
+	u64 size_af, size_bf;
+
+	if (mc->vram_start > mc->gart_start) {
+		size_bf = mc->vram_start - mc->gart_end + 1;
+		size_af = mc->mc_mask - mc->vram_end;
+	} else {
+		size_bf = mc->vram_start;
+		size_af = mc->mc_mask - mc->gart_end;
+	}
+
+	size_bf &= ~(sixteen_gb - 1);
+	size_af &= ~(sixteen_gb - 1);
+
+	if (size_bf > size_af) {
+		mc->agp_start = mc->vram_start > mc->gart_start ?
+			mc->gart_start : 0;
+		mc->agp_size = size_bf;
+	} else {
+		mc->agp_start = (mc->vram_start > mc->gart_start ?
+			mc->vram_end : mc->gart_end) + 1,
+		mc->agp_size = size_af;
+	}
+
+	mc->agp_start = ALIGN(mc->agp_start, sixteen_gb);
+	mc->agp_end = mc->agp_start + mc->agp_size - 1;
+	dev_info(adev->dev, "AGP: %lluM 0x%016llX - 0x%016llX\n",
+			mc->agp_size >> 20, mc->agp_start, mc->agp_end);
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index 72fcc9338f5e..163110fe375d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -81,6 +81,9 @@ struct amdgpu_gmc {
 	 * about vram size near mc fb location */
 	u64			mc_vram_size;
 	u64			visible_vram_size;
+	u64			agp_size;
+	u64			agp_start;
+	u64			agp_end;
 	u64			gart_size;
 	u64			gart_start;
 	u64			gart_end;
@@ -138,5 +141,7 @@ void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
 			      u64 base);
 void amdgpu_gmc_gart_location(struct amdgpu_device *adev,
 			      struct amdgpu_gmc *mc);
+void amdgpu_gmc_agp_location(struct amdgpu_device *adev,
+			     struct amdgpu_gmc *mc);
 
 #endif
-- 
2.17.1

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

Re: [PATCH 06/10] drm/amdgpu: add amdgpu_gmc_agp_location

[Felix Kuehling]

On 2018-08-27 12:53 PM, Christian König wrote:
> Helper to figure out the location of the AGP BAR.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 42 +++++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++
>  2 files changed, 47 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 4331a0e25cdc..eed5352f3136 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -141,3 +141,45 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>  	dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
>  			mc->gart_size >> 20, mc->gart_start, mc->gart_end);
>  }
> +
> +/**
> + * amdgpu_gmc_agp_location - try to find AGP location
> + * @adev: amdgpu device structure holding all necessary informations
> + * @mc: memory controller structure holding memory informations
> + *
> + * Function will place try to fina a place for the AGP BAR in the MC address

s/fina/find

> + * space.
> + *
> + * AGP BAR will be assigned the largest available hole in the address space.

I'd add a comment that this function must be called after
amdgpu_gmc_vram_location and amdgpu_gmc_gart_location.

> + */
> +void amdgpu_gmc_agp_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
> +{
> +	const uint64_t sixteen_gb = 1ULL << 34;
> +	u64 size_af, size_bf;
> +
> +	if (mc->vram_start > mc->gart_start) {
> +		size_bf = mc->vram_start - mc->gart_end + 1;
> +		size_af = mc->mc_mask - mc->vram_end;
> +	} else {
> +		size_bf = mc->vram_start;
> +		size_af = mc->mc_mask - mc->gart_end;
> +	}
> +
> +	size_bf &= ~(sixteen_gb - 1);
> +	size_af &= ~(sixteen_gb - 1);

This is not correct. E.g. vram_end  = 12GB, gart_start = 28GB, size =
16GB. agp_start will be rounded up to 16GB and AGP will run into the
GART aperture.

You need to align the addresses before calculating the sizes.

Regards,
  Felix

> +
> +	if (size_bf > size_af) {
> +		mc->agp_start = mc->vram_start > mc->gart_start ?
> +			mc->gart_start : 0;
> +		mc->agp_size = size_bf;
> +	} else {
> +		mc->agp_start = (mc->vram_start > mc->gart_start ?
> +			mc->vram_end : mc->gart_end) + 1,
> +		mc->agp_size = size_af;
> +	}
> +
> +	mc->agp_start = ALIGN(mc->agp_start, sixteen_gb);
> +	mc->agp_end = mc->agp_start + mc->agp_size - 1;
> +	dev_info(adev->dev, "AGP: %lluM 0x%016llX - 0x%016llX\n",
> +			mc->agp_size >> 20, mc->agp_start, mc->agp_end);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 72fcc9338f5e..163110fe375d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -81,6 +81,9 @@ struct amdgpu_gmc {
>  	 * about vram size near mc fb location */
>  	u64			mc_vram_size;
>  	u64			visible_vram_size;
> +	u64			agp_size;
> +	u64			agp_start;
> +	u64			agp_end;
>  	u64			gart_size;
>  	u64			gart_start;
>  	u64			gart_end;
> @@ -138,5 +141,7 @@ void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
>  			      u64 base);
>  void amdgpu_gmc_gart_location(struct amdgpu_device *adev,
>  			      struct amdgpu_gmc *mc);
> +void amdgpu_gmc_agp_location(struct amdgpu_device *adev,
> +			     struct amdgpu_gmc *mc);
>  
>  #endif

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

Re: [PATCH 06/10] drm/amdgpu: add amdgpu_gmc_agp_location

[Christian König]

Am 27.08.2018 um 20:50 schrieb Felix Kuehling:
> On 2018-08-27 12:53 PM, Christian König wrote:
>> Helper to figure out the location of the AGP BAR.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 42 +++++++++++++++++++++++++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++
>>   2 files changed, 47 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> index 4331a0e25cdc..eed5352f3136 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> @@ -141,3 +141,45 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>>   	dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
>>   			mc->gart_size >> 20, mc->gart_start, mc->gart_end);
>>   }
>> +
>> +/**
>> + * amdgpu_gmc_agp_location - try to find AGP location
>> + * @adev: amdgpu device structure holding all necessary informations
>> + * @mc: memory controller structure holding memory informations
>> + *
>> + * Function will place try to fina a place for the AGP BAR in the MC address
> s/fina/find
>
>> + * space.
>> + *
>> + * AGP BAR will be assigned the largest available hole in the address space.
> I'd add a comment that this function must be called after
> amdgpu_gmc_vram_location and amdgpu_gmc_gart_location.
>
>> + */
>> +void amdgpu_gmc_agp_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>> +{
>> +	const uint64_t sixteen_gb = 1ULL << 34;
>> +	u64 size_af, size_bf;
>> +
>> +	if (mc->vram_start > mc->gart_start) {
>> +		size_bf = mc->vram_start - mc->gart_end + 1;
>> +		size_af = mc->mc_mask - mc->vram_end;
>> +	} else {
>> +		size_bf = mc->vram_start;
>> +		size_af = mc->mc_mask - mc->gart_end;
>> +	}
>> +
>> +	size_bf &= ~(sixteen_gb - 1);
>> +	size_af &= ~(sixteen_gb - 1);
> This is not correct. E.g. vram_end  = 12GB, gart_start = 28GB, size =
> 16GB. agp_start will be rounded up to 16GB and AGP will run into the
> GART aperture.
>
> You need to align the addresses before calculating the sizes.

Ah, crap you are right. Going to fix this.

I'm really wondering if we shouldn't use "struct resource" for that 
handling?

This will only get more complicated in the future when even more 
apertures are enabled.

Alex what do you think?

Regards,
Christian.

>
> Regards,
>    Felix
>
>> +
>> +	if (size_bf > size_af) {
>> +		mc->agp_start = mc->vram_start > mc->gart_start ?
>> +			mc->gart_start : 0;
>> +		mc->agp_size = size_bf;
>> +	} else {
>> +		mc->agp_start = (mc->vram_start > mc->gart_start ?
>> +			mc->vram_end : mc->gart_end) + 1,
>> +		mc->agp_size = size_af;
>> +	}
>> +
>> +	mc->agp_start = ALIGN(mc->agp_start, sixteen_gb);
>> +	mc->agp_end = mc->agp_start + mc->agp_size - 1;
>> +	dev_info(adev->dev, "AGP: %lluM 0x%016llX - 0x%016llX\n",
>> +			mc->agp_size >> 20, mc->agp_start, mc->agp_end);
>> +}
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> index 72fcc9338f5e..163110fe375d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> @@ -81,6 +81,9 @@ struct amdgpu_gmc {
>>   	 * about vram size near mc fb location */
>>   	u64			mc_vram_size;
>>   	u64			visible_vram_size;
>> +	u64			agp_size;
>> +	u64			agp_start;
>> +	u64			agp_end;
>>   	u64			gart_size;
>>   	u64			gart_start;
>>   	u64			gart_end;
>> @@ -138,5 +141,7 @@ void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
>>   			      u64 base);
>>   void amdgpu_gmc_gart_location(struct amdgpu_device *adev,
>>   			      struct amdgpu_gmc *mc);
>> +void amdgpu_gmc_agp_location(struct amdgpu_device *adev,
>> +			     struct amdgpu_gmc *mc);
>>   
>>   #endif
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

Re: [PATCH 06/10] drm/amdgpu: add amdgpu_gmc_agp_location

[Zhang, Jerry (Junwei)]

On 08/28/2018 12:53 AM, Christian König wrote:
> Helper to figure out the location of the AGP BAR.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 42 +++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++
>   2 files changed, 47 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 4331a0e25cdc..eed5352f3136 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -141,3 +141,45 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>   	dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
>   			mc->gart_size >> 20, mc->gart_start, mc->gart_end);
>   }
> +
> +/**
> + * amdgpu_gmc_agp_location - try to find AGP location
> + * @adev: amdgpu device structure holding all necessary informations
> + * @mc: memory controller structure holding memory informations
> + *
> + * Function will place try to fina a place for the AGP BAR in the MC address
> + * space.
> + *
> + * AGP BAR will be assigned the largest available hole in the address space.
> + */
> +void amdgpu_gmc_agp_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
> +{
> +	const uint64_t sixteen_gb = 1ULL << 34;
> +	u64 size_af, size_bf;
> +
> +	if (mc->vram_start > mc->gart_start) {
> +		size_bf = mc->vram_start - mc->gart_end + 1;
> +		size_af = mc->mc_mask - mc->vram_end;
> +	} else {
> +		size_bf = mc->vram_start;
> +		size_af = mc->mc_mask - mc->gart_end;
> +	}
> +
> +	size_bf &= ~(sixteen_gb - 1);
> +	size_af &= ~(sixteen_gb - 1);
> +
> +	if (size_bf > size_af) {
> +		mc->agp_start = mc->vram_start > mc->gart_start ?
> +			mc->gart_start : 0;

Here looks mc->gart_end?

> +		mc->agp_size = size_bf;
> +	} else {
> +		mc->agp_start = (mc->vram_start > mc->gart_start ?
> +			mc->vram_end : mc->gart_end) + 1,
> +		mc->agp_size = size_af;
> +	}
> +
> +	mc->agp_start = ALIGN(mc->agp_start, sixteen_gb);

still needs mc->apg_start + 1 for alignment?

> +	mc->agp_end = mc->agp_start + mc->agp_size - 1;
> +	dev_info(adev->dev, "AGP: %lluM 0x%016llX - 0x%016llX\n",
> +			mc->agp_size >> 20, mc->agp_start, mc->agp_end);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 72fcc9338f5e..163110fe375d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -81,6 +81,9 @@ struct amdgpu_gmc {
>   	 * about vram size near mc fb location */
>   	u64			mc_vram_size;
>   	u64			visible_vram_size;
> +	u64			agp_size;
> +	u64			agp_start;
> +	u64			agp_end;
>   	u64			gart_size;
>   	u64			gart_start;
>   	u64			gart_end;
> @@ -138,5 +141,7 @@ void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
>   			      u64 base);
>   void amdgpu_gmc_gart_location(struct amdgpu_device *adev,
>   			      struct amdgpu_gmc *mc);
> +void amdgpu_gmc_agp_location(struct amdgpu_device *adev,
> +			     struct amdgpu_gmc *mc);
>
>   #endif
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 07/10] drm/amdgpu: stop using gart_start as offset for the GTT domain

[Christian König]

Further separate GART and GTT domain.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index da7b1b92d9cf..c2539f6821c0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -143,7 +143,8 @@ static int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 	spin_unlock(&mgr->lock);
 
 	if (!r)
-		mem->start = node->node.start;
+		mem->start = node->node.start +
+			(adev->gmc.gart_start >> PAGE_SHIFT);
 
 	return r;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 2f304f9dd543..5cadf4f1ee2c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -188,7 +188,7 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_TT:
 		/* GTT memory  */
 		man->func = &amdgpu_gtt_mgr_func;
-		man->gpu_offset = adev->gmc.gart_start;
+		man->gpu_offset = 0;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
@@ -1062,7 +1062,7 @@ static int amdgpu_ttm_backend_bind(struct ttm_tt *ttm,
 	flags = amdgpu_ttm_tt_pte_flags(adev, ttm, bo_mem);
 
 	/* bind pages into GART page tables */
-	gtt->offset = (u64)bo_mem->start << PAGE_SHIFT;
+	gtt->offset = ((u64)bo_mem->start << PAGE_SHIFT) - adev->gmc.gart_start;
 	r = amdgpu_gart_bind(adev, gtt->offset, ttm->num_pages,
 		ttm->pages, gtt->ttm.dma_address, flags);
 
@@ -1110,7 +1110,7 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
 	flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, &tmp);
 
 	/* Bind pages */
-	gtt->offset = (u64)tmp.start << PAGE_SHIFT;
+	gtt->offset = ((u64)tmp.start << PAGE_SHIFT) - adev->gmc.gart_start;
 	r = amdgpu_ttm_gart_bind(adev, bo, flags);
 	if (unlikely(r)) {
 		ttm_bo_mem_put(bo, &tmp);
-- 
2.17.1

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

Re: [PATCH 07/10] drm/amdgpu: stop using gart_start as offset for the GTT domain

[Alex Deucher]

On Mon, Aug 27, 2018 at 12:56 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Further separate GART and GTT domain.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 3 ++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 6 +++---
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index da7b1b92d9cf..c2539f6821c0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -143,7 +143,8 @@ static int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>         spin_unlock(&mgr->lock);
>
>         if (!r)
> -               mem->start = node->node.start;
> +               mem->start = node->node.start +
> +                       (adev->gmc.gart_start >> PAGE_SHIFT);
>
>         return r;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 2f304f9dd543..5cadf4f1ee2c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -188,7 +188,7 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>         case TTM_PL_TT:
>                 /* GTT memory  */
>                 man->func = &amdgpu_gtt_mgr_func;
> -               man->gpu_offset = adev->gmc.gart_start;
> +               man->gpu_offset = 0;
>                 man->available_caching = TTM_PL_MASK_CACHING;
>                 man->default_caching = TTM_PL_FLAG_CACHED;
>                 man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
> @@ -1062,7 +1062,7 @@ static int amdgpu_ttm_backend_bind(struct ttm_tt *ttm,
>         flags = amdgpu_ttm_tt_pte_flags(adev, ttm, bo_mem);
>
>         /* bind pages into GART page tables */
> -       gtt->offset = (u64)bo_mem->start << PAGE_SHIFT;
> +       gtt->offset = ((u64)bo_mem->start << PAGE_SHIFT) - adev->gmc.gart_start;
>         r = amdgpu_gart_bind(adev, gtt->offset, ttm->num_pages,
>                 ttm->pages, gtt->ttm.dma_address, flags);
>
> @@ -1110,7 +1110,7 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
>         flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, &tmp);
>
>         /* Bind pages */
> -       gtt->offset = (u64)tmp.start << PAGE_SHIFT;
> +       gtt->offset = ((u64)tmp.start << PAGE_SHIFT) - adev->gmc.gart_start;
>         r = amdgpu_ttm_gart_bind(adev, bo, flags);
>         if (unlikely(r)) {
>                 ttm_bo_mem_put(bo, &tmp);
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 07/10] drm/amdgpu: stop using gart_start as offset for the GTT domain

[Zhang, Jerry (Junwei)]

On 08/28/2018 03:07 AM, Alex Deucher wrote:
> On Mon, Aug 27, 2018 at 12:56 PM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>>
>> Further separate GART and GTT domain.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 3 ++-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 6 +++---
>>   2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> index da7b1b92d9cf..c2539f6821c0 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> @@ -143,7 +143,8 @@ static int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>>          spin_unlock(&mgr->lock);
>>
>>          if (!r)
>> -               mem->start = node->node.start;
>> +               mem->start = node->node.start +
>> +                       (adev->gmc.gart_start >> PAGE_SHIFT);
>>
>>          return r;
>>   }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 2f304f9dd543..5cadf4f1ee2c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -188,7 +188,7 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>          case TTM_PL_TT:
>>                  /* GTT memory  */
>>                  man->func = &amdgpu_gtt_mgr_func;
>> -               man->gpu_offset = adev->gmc.gart_start;
>> +               man->gpu_offset = 0;
>>                  man->available_caching = TTM_PL_MASK_CACHING;
>>                  man->default_caching = TTM_PL_FLAG_CACHED;
>>                  man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
>> @@ -1062,7 +1062,7 @@ static int amdgpu_ttm_backend_bind(struct ttm_tt *ttm,
>>          flags = amdgpu_ttm_tt_pte_flags(adev, ttm, bo_mem);
>>
>>          /* bind pages into GART page tables */
>> -       gtt->offset = (u64)bo_mem->start << PAGE_SHIFT;
>> +       gtt->offset = ((u64)bo_mem->start << PAGE_SHIFT) - adev->gmc.gart_start;
>>          r = amdgpu_gart_bind(adev, gtt->offset, ttm->num_pages,
>>                  ttm->pages, gtt->ttm.dma_address, flags);
>>
>> @@ -1110,7 +1110,7 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
>>          flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, &tmp);
>>
>>          /* Bind pages */
>> -       gtt->offset = (u64)tmp.start << PAGE_SHIFT;
>> +       gtt->offset = ((u64)tmp.start << PAGE_SHIFT) - adev->gmc.gart_start;
>>          r = amdgpu_ttm_gart_bind(adev, bo, flags);
>>          if (unlikely(r)) {
>>                  ttm_bo_mem_put(bo, &tmp);
>> --
>> 2.17.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 08/10] drm/amdgpu: distinct between allocated GART space and GMC addr

[Christian König]

Most of the time we only need to know if the BO has a valid GMC addr.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  2 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 13 +++++--------
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 5ddd4e87480b..b5f20b42439e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1362,8 +1362,6 @@ void amdgpu_bo_fence(struct amdgpu_bo *bo, struct dma_fence *fence,
 u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
 {
 	WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_SYSTEM);
-	WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_TT &&
-		     !amdgpu_gtt_mgr_has_gart_addr(&bo->tbo.mem));
 	WARN_ON_ONCE(!ww_mutex_is_locked(&bo->tbo.resv->lock) &&
 		     !bo->pin_count);
 	WARN_ON_ONCE(bo->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 18945dd6982d..37c79ae3574e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -200,7 +200,7 @@ static inline u64 amdgpu_bo_mmap_offset(struct amdgpu_bo *bo)
 static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo)
 {
 	switch (bo->tbo.mem.mem_type) {
-	case TTM_PL_TT: return amdgpu_gtt_mgr_has_gart_addr(&bo->tbo.mem);
+	case TTM_PL_TT: return bo->tbo.mem.start != AMDGPU_BO_INVALID_OFFSET;
 	case TTM_PL_VRAM: return true;
 	default: return false;
 	}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 5cadf4f1ee2c..d9f3201c9e5c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -345,7 +345,7 @@ static uint64_t amdgpu_mm_node_addr(struct ttm_buffer_object *bo,
 {
 	uint64_t addr = 0;
 
-	if (mem->mem_type != TTM_PL_TT || amdgpu_gtt_mgr_has_gart_addr(mem)) {
+	if (mm_node->start != AMDGPU_BO_INVALID_OFFSET) {
 		addr = mm_node->start << PAGE_SHIFT;
 		addr += bo->bdev->man[mem->mem_type].gpu_offset;
 	}
@@ -433,8 +433,7 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
 		/* Map only what needs to be accessed. Map src to window 0 and
 		 * dst to window 1
 		 */
-		if (src->mem->mem_type == TTM_PL_TT &&
-		    !amdgpu_gtt_mgr_has_gart_addr(src->mem)) {
+		if (src->mem->start == AMDGPU_BO_INVALID_OFFSET) {
 			r = amdgpu_map_buffer(src->bo, src->mem,
 					PFN_UP(cur_size + src_page_offset),
 					src_node_start, 0, ring,
@@ -447,8 +446,7 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
 			from += src_page_offset;
 		}
 
-		if (dst->mem->mem_type == TTM_PL_TT &&
-		    !amdgpu_gtt_mgr_has_gart_addr(dst->mem)) {
+		if (dst->mem->start == AMDGPU_BO_INVALID_OFFSET) {
 			r = amdgpu_map_buffer(dst->bo, dst->mem,
 					PFN_UP(cur_size + dst_page_offset),
 					dst_node_start, 1, ring,
@@ -1086,11 +1084,10 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
 	uint64_t flags;
 	int r;
 
-	if (bo->mem.mem_type != TTM_PL_TT ||
-	    amdgpu_gtt_mgr_has_gart_addr(&bo->mem))
+	if (bo->mem.start != AMDGPU_BO_INVALID_OFFSET)
 		return 0;
 
-	/* allocate GTT space */
+	/* allocate GART space */
 	tmp = bo->mem;
 	tmp.mm_node = NULL;
 	placement.num_placement = 1;
-- 
2.17.1

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

Re: [PATCH 08/10] drm/amdgpu: distinct between allocated GART space and GMC addr

[Felix Kuehling]

On 2018-08-27 12:53 PM, Christian König wrote:
> Most of the time we only need to know if the BO has a valid GMC addr.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  2 --
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 13 +++++--------
>  3 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 5ddd4e87480b..b5f20b42439e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1362,8 +1362,6 @@ void amdgpu_bo_fence(struct amdgpu_bo *bo, struct dma_fence *fence,
>  u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
>  {
>  	WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_SYSTEM);
> -	WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_TT &&
> -		     !amdgpu_gtt_mgr_has_gart_addr(&bo->tbo.mem));
>  	WARN_ON_ONCE(!ww_mutex_is_locked(&bo->tbo.resv->lock) &&
>  		     !bo->pin_count);
>  	WARN_ON_ONCE(bo->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 18945dd6982d..37c79ae3574e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -200,7 +200,7 @@ static inline u64 amdgpu_bo_mmap_offset(struct amdgpu_bo *bo)
>  static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo)

As far as I can tell, this function is completely unused. So your change
here has no effect.

Other than that Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>

Regards,
  Felix

>  {
>  	switch (bo->tbo.mem.mem_type) {
> -	case TTM_PL_TT: return amdgpu_gtt_mgr_has_gart_addr(&bo->tbo.mem);
> +	case TTM_PL_TT: return bo->tbo.mem.start != AMDGPU_BO_INVALID_OFFSET;
>  	case TTM_PL_VRAM: return true;
>  	default: return false;
>  	}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 5cadf4f1ee2c..d9f3201c9e5c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -345,7 +345,7 @@ static uint64_t amdgpu_mm_node_addr(struct ttm_buffer_object *bo,
>  {
>  	uint64_t addr = 0;
>  
> -	if (mem->mem_type != TTM_PL_TT || amdgpu_gtt_mgr_has_gart_addr(mem)) {
> +	if (mm_node->start != AMDGPU_BO_INVALID_OFFSET) {
>  		addr = mm_node->start << PAGE_SHIFT;
>  		addr += bo->bdev->man[mem->mem_type].gpu_offset;
>  	}
> @@ -433,8 +433,7 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
>  		/* Map only what needs to be accessed. Map src to window 0 and
>  		 * dst to window 1
>  		 */
> -		if (src->mem->mem_type == TTM_PL_TT &&
> -		    !amdgpu_gtt_mgr_has_gart_addr(src->mem)) {
> +		if (src->mem->start == AMDGPU_BO_INVALID_OFFSET) {
>  			r = amdgpu_map_buffer(src->bo, src->mem,
>  					PFN_UP(cur_size + src_page_offset),
>  					src_node_start, 0, ring,
> @@ -447,8 +446,7 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
>  			from += src_page_offset;
>  		}
>  
> -		if (dst->mem->mem_type == TTM_PL_TT &&
> -		    !amdgpu_gtt_mgr_has_gart_addr(dst->mem)) {
> +		if (dst->mem->start == AMDGPU_BO_INVALID_OFFSET) {
>  			r = amdgpu_map_buffer(dst->bo, dst->mem,
>  					PFN_UP(cur_size + dst_page_offset),
>  					dst_node_start, 1, ring,
> @@ -1086,11 +1084,10 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
>  	uint64_t flags;
>  	int r;
>  
> -	if (bo->mem.mem_type != TTM_PL_TT ||
> -	    amdgpu_gtt_mgr_has_gart_addr(&bo->mem))
> +	if (bo->mem.start != AMDGPU_BO_INVALID_OFFSET)
>  		return 0;
>  
> -	/* allocate GTT space */
> +	/* allocate GART space */
>  	tmp = bo->mem;
>  	tmp.mm_node = NULL;
>  	placement.num_placement = 1;

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

Re: [PATCH 08/10] drm/amdgpu: distinct between allocated GART space and GMC addr

[Zhang, Jerry (Junwei)]

On 08/28/2018 12:53 AM, Christian König wrote:
> Most of the time we only need to know if the BO has a valid GMC addr.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

good to see this cleanup :)

Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  2 --
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 13 +++++--------
>   3 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 5ddd4e87480b..b5f20b42439e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1362,8 +1362,6 @@ void amdgpu_bo_fence(struct amdgpu_bo *bo, struct dma_fence *fence,
>   u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
>   {
>   	WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_SYSTEM);
> -	WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_TT &&
> -		     !amdgpu_gtt_mgr_has_gart_addr(&bo->tbo.mem));
>   	WARN_ON_ONCE(!ww_mutex_is_locked(&bo->tbo.resv->lock) &&
>   		     !bo->pin_count);
>   	WARN_ON_ONCE(bo->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 18945dd6982d..37c79ae3574e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -200,7 +200,7 @@ static inline u64 amdgpu_bo_mmap_offset(struct amdgpu_bo *bo)
>   static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo)
>   {
>   	switch (bo->tbo.mem.mem_type) {
> -	case TTM_PL_TT: return amdgpu_gtt_mgr_has_gart_addr(&bo->tbo.mem);
> +	case TTM_PL_TT: return bo->tbo.mem.start != AMDGPU_BO_INVALID_OFFSET;
>   	case TTM_PL_VRAM: return true;
>   	default: return false;
>   	}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 5cadf4f1ee2c..d9f3201c9e5c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -345,7 +345,7 @@ static uint64_t amdgpu_mm_node_addr(struct ttm_buffer_object *bo,
>   {
>   	uint64_t addr = 0;
>
> -	if (mem->mem_type != TTM_PL_TT || amdgpu_gtt_mgr_has_gart_addr(mem)) {
> +	if (mm_node->start != AMDGPU_BO_INVALID_OFFSET) {
>   		addr = mm_node->start << PAGE_SHIFT;
>   		addr += bo->bdev->man[mem->mem_type].gpu_offset;
>   	}
> @@ -433,8 +433,7 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
>   		/* Map only what needs to be accessed. Map src to window 0 and
>   		 * dst to window 1
>   		 */
> -		if (src->mem->mem_type == TTM_PL_TT &&
> -		    !amdgpu_gtt_mgr_has_gart_addr(src->mem)) {
> +		if (src->mem->start == AMDGPU_BO_INVALID_OFFSET) {
>   			r = amdgpu_map_buffer(src->bo, src->mem,
>   					PFN_UP(cur_size + src_page_offset),
>   					src_node_start, 0, ring,
> @@ -447,8 +446,7 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
>   			from += src_page_offset;
>   		}
>
> -		if (dst->mem->mem_type == TTM_PL_TT &&
> -		    !amdgpu_gtt_mgr_has_gart_addr(dst->mem)) {
> +		if (dst->mem->start == AMDGPU_BO_INVALID_OFFSET) {
>   			r = amdgpu_map_buffer(dst->bo, dst->mem,
>   					PFN_UP(cur_size + dst_page_offset),
>   					dst_node_start, 1, ring,
> @@ -1086,11 +1084,10 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
>   	uint64_t flags;
>   	int r;
>
> -	if (bo->mem.mem_type != TTM_PL_TT ||
> -	    amdgpu_gtt_mgr_has_gart_addr(&bo->mem))
> +	if (bo->mem.start != AMDGPU_BO_INVALID_OFFSET)
>   		return 0;
>
> -	/* allocate GTT space */
> +	/* allocate GART space */
>   	tmp = bo->mem;
>   	tmp.mm_node = NULL;
>   	placement.num_placement = 1;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 09/10] drm/amdgpu: use the AGP aperture for system memory access

[Christian König]

Start to use the old AGP aperture for system memory access.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c     | 24 +++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h     |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c |  9 ++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 21 ++++++++++--------
 4 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index eed5352f3136..54d353951e21 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -79,6 +79,30 @@ uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo)
 	return pd_addr;
 }
 
+/**
+ * amdgpu_gmc_agp_addr - return the address in the AGP address space
+ *
+ * @tbo: TTM BO which needs the address, must be in GTT domain
+ *
+ * Tries to figure out how to access the BO through the AGP aperture. Returns
+ * AMDGPU_BO_INVALID_OFFSET if that is not possible.
+ */
+uint64_t amdgpu_gmc_agp_addr(struct ttm_buffer_object *tbo)
+{
+	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
+	struct ttm_dma_tt *ttm;
+
+	if (tbo->num_pages != 1 || !tbo->ttm ||
+	    tbo->ttm->caching_state == tt_cached)
+		return AMDGPU_BO_INVALID_OFFSET;
+
+	ttm = container_of(tbo->ttm, struct ttm_dma_tt, ttm);
+	if (ttm->dma_address[0] + PAGE_SIZE >= adev->gmc.agp_size)
+		return AMDGPU_BO_INVALID_OFFSET;
+
+	return adev->gmc.agp_start + ttm->dma_address[0];
+}
+
 /**
  * amdgpu_gmc_vram_location - try to find VRAM location
  *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index 163110fe375d..6e8432fd3309 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -137,6 +137,7 @@ static inline bool amdgpu_gmc_vram_full_visible(struct amdgpu_gmc *gmc)
 void amdgpu_gmc_get_pde_for_bo(struct amdgpu_bo *bo, int level,
 			       uint64_t *addr, uint64_t *flags);
 uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo);
+uint64_t amdgpu_gmc_agp_addr(struct ttm_buffer_object *tbo);
 void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
 			      u64 base);
 void amdgpu_gmc_gart_location(struct amdgpu_device *adev,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index c2539f6821c0..deaea11eb39a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -132,6 +132,15 @@ static int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 	else
 		lpfn = adev->gart.num_cpu_pages;
 
+	if (fpfn == 0 && lpfn == adev->gart.num_cpu_pages) {
+		uint64_t addr = amdgpu_gmc_agp_addr(tbo);
+
+		if (addr != AMDGPU_BO_INVALID_OFFSET) {
+			mem->start = addr >> PAGE_SHIFT;
+			return 0;
+		}
+	}
+
 	mode = DRM_MM_INSERT_BEST;
 	if (place && place->flags & TTM_PL_FLAG_TOPDOWN)
 		mode = DRM_MM_INSERT_HIGH;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index d9f3201c9e5c..281611f6bcd4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1103,15 +1103,18 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
 	if (unlikely(r))
 		return r;
 
-	/* compute PTE flags for this buffer object */
-	flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, &tmp);
-
-	/* Bind pages */
-	gtt->offset = ((u64)tmp.start << PAGE_SHIFT) - adev->gmc.gart_start;
-	r = amdgpu_ttm_gart_bind(adev, bo, flags);
-	if (unlikely(r)) {
-		ttm_bo_mem_put(bo, &tmp);
-		return r;
+	if (amdgpu_gtt_mgr_has_gart_addr(&tmp)) {
+		/* compute PTE flags for this buffer object */
+		flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, &tmp);
+
+		/* Bind pages */
+		gtt->offset = ((u64)tmp.start << PAGE_SHIFT) -
+			adev->gmc.gart_start;
+		r = amdgpu_ttm_gart_bind(adev, bo, flags);
+		if (unlikely(r)) {
+			ttm_bo_mem_put(bo, &tmp);
+			return r;
+		}
 	}
 
 	ttm_bo_mem_put(bo, &bo->mem);
-- 
2.17.1

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

Re: [PATCH 09/10] drm/amdgpu: use the AGP aperture for system memory access

[Zhang, Jerry (Junwei)]

On 08/28/2018 12:53 AM, Christian König wrote:
> Start to use the old AGP aperture for system memory access.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c     | 24 +++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h     |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c |  9 ++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 21 ++++++++++--------
>   4 files changed, 46 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index eed5352f3136..54d353951e21 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -79,6 +79,30 @@ uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo)
>   	return pd_addr;
>   }
>
> +/**
> + * amdgpu_gmc_agp_addr - return the address in the AGP address space
> + *
> + * @tbo: TTM BO which needs the address, must be in GTT domain
> + *
> + * Tries to figure out how to access the BO through the AGP aperture. Returns
> + * AMDGPU_BO_INVALID_OFFSET if that is not possible.
> + */
> +uint64_t amdgpu_gmc_agp_addr(struct ttm_buffer_object *tbo)
> +{
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
> +	struct ttm_dma_tt *ttm;
> +
> +	if (tbo->num_pages != 1 || !tbo->ttm ||
> +	    tbo->ttm->caching_state == tt_cached)
> +		return AMDGPU_BO_INVALID_OFFSET;
> +
> +	ttm = container_of(tbo->ttm, struct ttm_dma_tt, ttm);
> +	if (ttm->dma_address[0] + PAGE_SIZE >= adev->gmc.agp_size)
> +		return AMDGPU_BO_INVALID_OFFSET;
> +
> +	return adev->gmc.agp_start + ttm->dma_address[0];
> +}
> +
>   /**
>    * amdgpu_gmc_vram_location - try to find VRAM location
>    *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 163110fe375d..6e8432fd3309 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -137,6 +137,7 @@ static inline bool amdgpu_gmc_vram_full_visible(struct amdgpu_gmc *gmc)
>   void amdgpu_gmc_get_pde_for_bo(struct amdgpu_bo *bo, int level,
>   			       uint64_t *addr, uint64_t *flags);
>   uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo);
> +uint64_t amdgpu_gmc_agp_addr(struct ttm_buffer_object *tbo);
>   void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
>   			      u64 base);
>   void amdgpu_gmc_gart_location(struct amdgpu_device *adev,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index c2539f6821c0..deaea11eb39a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -132,6 +132,15 @@ static int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>   	else
>   		lpfn = adev->gart.num_cpu_pages;
>
> +	if (fpfn == 0 && lpfn == adev->gart.num_cpu_pages) {
> +		uint64_t addr = amdgpu_gmc_agp_addr(tbo);
> +
> +		if (addr != AMDGPU_BO_INVALID_OFFSET) {
> +			mem->start = addr >> PAGE_SHIFT;
> +			return 0;
> +		}
> +	}
> +
>   	mode = DRM_MM_INSERT_BEST;
>   	if (place && place->flags & TTM_PL_FLAG_TOPDOWN)
>   		mode = DRM_MM_INSERT_HIGH;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index d9f3201c9e5c..281611f6bcd4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1103,15 +1103,18 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
>   	if (unlikely(r))
>   		return r;
>
> -	/* compute PTE flags for this buffer object */
> -	flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, &tmp);
> -
> -	/* Bind pages */
> -	gtt->offset = ((u64)tmp.start << PAGE_SHIFT) - adev->gmc.gart_start;
> -	r = amdgpu_ttm_gart_bind(adev, bo, flags);
> -	if (unlikely(r)) {
> -		ttm_bo_mem_put(bo, &tmp);
> -		return r;
> +	if (amdgpu_gtt_mgr_has_gart_addr(&tmp)) {
> +		/* compute PTE flags for this buffer object */
> +		flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, &tmp);
> +
> +		/* Bind pages */
> +		gtt->offset = ((u64)tmp.start << PAGE_SHIFT) -
> +			adev->gmc.gart_start;
> +		r = amdgpu_ttm_gart_bind(adev, bo, flags);
> +		if (unlikely(r)) {
> +			ttm_bo_mem_put(bo, &tmp);
> +			return r;
> +		}
>   	}
>
>   	ttm_bo_mem_put(bo, &bo->mem);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 10/10] drm/amdgpu: enable AGP aperture for GMC9

[Christian König]

Enable the old AGP aperture to avoid GART mappings.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 10 +++++-----
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c    |  1 +
 drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c  | 10 +++++-----
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
index 3403ded39d13..ffd0ec9586d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
@@ -65,16 +65,16 @@ static void gfxhub_v1_0_init_system_aperture_regs(struct amdgpu_device *adev)
 {
 	uint64_t value;
 
-	/* Disable AGP. */
+	/* Program the AGP BAR */
 	WREG32_SOC15(GC, 0, mmMC_VM_AGP_BASE, 0);
-	WREG32_SOC15(GC, 0, mmMC_VM_AGP_TOP, 0);
-	WREG32_SOC15(GC, 0, mmMC_VM_AGP_BOT, 0xFFFFFFFF);
+	WREG32_SOC15(GC, 0, mmMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
+	WREG32_SOC15(GC, 0, mmMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
 
 	/* Program the system aperture low logical page number. */
 	WREG32_SOC15(GC, 0, mmMC_VM_SYSTEM_APERTURE_LOW_ADDR,
-		     adev->gmc.vram_start >> 18);
+		     min(adev->gmc.vram_start, adev->gmc.agp_start) >> 18);
 	WREG32_SOC15(GC, 0, mmMC_VM_SYSTEM_APERTURE_HIGH_ADDR,
-		     adev->gmc.vram_end >> 18);
+		     max(adev->gmc.vram_end, adev->gmc.agp_end) >> 18);
 
 	/* Set default page address. */
 	value = adev->vram_scratch.gpu_addr - adev->gmc.vram_start
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 60c8f848c102..3f40c153039f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -751,6 +751,7 @@ static void gmc_v9_0_vram_gtt_location(struct amdgpu_device *adev,
 		base = mmhub_v1_0_get_fb_location(adev);
 	amdgpu_gmc_vram_location(adev, &adev->gmc, base);
 	amdgpu_gmc_gart_location(adev, mc);
+	amdgpu_gmc_agp_location(adev, mc);
 	/* base offset of vram pages */
 	adev->vm_manager.vram_base_offset = gfxhub_v1_0_get_mc_fb_offset(adev);
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
index 5f6a9c85488f..73d7c075dd33 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
@@ -76,16 +76,16 @@ static void mmhub_v1_0_init_system_aperture_regs(struct amdgpu_device *adev)
 	uint64_t value;
 	uint32_t tmp;
 
-	/* Disable AGP. */
+	/* Program the AGP BAR */
 	WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_BASE, 0);
-	WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_TOP, 0);
-	WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_BOT, 0x00FFFFFF);
+	WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
+	WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
 
 	/* Program the system aperture low logical page number. */
 	WREG32_SOC15(MMHUB, 0, mmMC_VM_SYSTEM_APERTURE_LOW_ADDR,
-		     adev->gmc.vram_start >> 18);
+		     min(adev->gmc.vram_start, adev->gmc.agp_start) >> 18);
 	WREG32_SOC15(MMHUB, 0, mmMC_VM_SYSTEM_APERTURE_HIGH_ADDR,
-		     adev->gmc.vram_end >> 18);
+		     max(adev->gmc.vram_end, adev->gmc.agp_end) >> 18);
 
 	/* Set default page address. */
 	value = adev->vram_scratch.gpu_addr - adev->gmc.vram_start +
-- 
2.17.1

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

Re: [PATCH 10/10] drm/amdgpu: enable AGP aperture for GMC9

[Zhang, Jerry (Junwei)]

On 08/28/2018 12:53 AM, Christian König wrote:
> Enable the old AGP aperture to avoid GART mappings.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 10 +++++-----
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c    |  1 +
>   drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c  | 10 +++++-----
>   3 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
> index 3403ded39d13..ffd0ec9586d1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
> @@ -65,16 +65,16 @@ static void gfxhub_v1_0_init_system_aperture_regs(struct amdgpu_device *adev)
>   {
>   	uint64_t value;
>
> -	/* Disable AGP. */
> +	/* Program the AGP BAR */
>   	WREG32_SOC15(GC, 0, mmMC_VM_AGP_BASE, 0);
> -	WREG32_SOC15(GC, 0, mmMC_VM_AGP_TOP, 0);
> -	WREG32_SOC15(GC, 0, mmMC_VM_AGP_BOT, 0xFFFFFFFF);
> +	WREG32_SOC15(GC, 0, mmMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
> +	WREG32_SOC15(GC, 0, mmMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
>
>   	/* Program the system aperture low logical page number. */
>   	WREG32_SOC15(GC, 0, mmMC_VM_SYSTEM_APERTURE_LOW_ADDR,
> -		     adev->gmc.vram_start >> 18);
> +		     min(adev->gmc.vram_start, adev->gmc.agp_start) >> 18);
>   	WREG32_SOC15(GC, 0, mmMC_VM_SYSTEM_APERTURE_HIGH_ADDR,
> -		     adev->gmc.vram_end >> 18);
> +		     max(adev->gmc.vram_end, adev->gmc.agp_end) >> 18);

It may not work for AGP after GART range.

|--- vram ---|--- GART ---|--- AGP ---|

in this case, we may have to replace AGP and GART range.

Regards,
Jerry

>
>   	/* Set default page address. */
>   	value = adev->vram_scratch.gpu_addr - adev->gmc.vram_start
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 60c8f848c102..3f40c153039f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -751,6 +751,7 @@ static void gmc_v9_0_vram_gtt_location(struct amdgpu_device *adev,
>   		base = mmhub_v1_0_get_fb_location(adev);
>   	amdgpu_gmc_vram_location(adev, &adev->gmc, base);
>   	amdgpu_gmc_gart_location(adev, mc);
> +	amdgpu_gmc_agp_location(adev, mc);
>   	/* base offset of vram pages */
>   	adev->vm_manager.vram_base_offset = gfxhub_v1_0_get_mc_fb_offset(adev);
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> index 5f6a9c85488f..73d7c075dd33 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> @@ -76,16 +76,16 @@ static void mmhub_v1_0_init_system_aperture_regs(struct amdgpu_device *adev)
>   	uint64_t value;
>   	uint32_t tmp;
>
> -	/* Disable AGP. */
> +	/* Program the AGP BAR */
>   	WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_BASE, 0);
> -	WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_TOP, 0);
> -	WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_BOT, 0x00FFFFFF);
> +	WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
> +	WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
>
>   	/* Program the system aperture low logical page number. */
>   	WREG32_SOC15(MMHUB, 0, mmMC_VM_SYSTEM_APERTURE_LOW_ADDR,
> -		     adev->gmc.vram_start >> 18);
> +		     min(adev->gmc.vram_start, adev->gmc.agp_start) >> 18);
>   	WREG32_SOC15(MMHUB, 0, mmMC_VM_SYSTEM_APERTURE_HIGH_ADDR,
> -		     adev->gmc.vram_end >> 18);
> +		     max(adev->gmc.vram_end, adev->gmc.agp_end) >> 18);
>
>   	/* Set default page address. */
>   	value = adev->vram_scratch.gpu_addr - adev->gmc.vram_start +
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 10/10] drm/amdgpu: enable AGP aperture for GMC9

[Christian König]

Am 28.08.2018 um 04:52 schrieb Zhang, Jerry (Junwei):
> On 08/28/2018 12:53 AM, Christian König wrote:
>> Enable the old AGP aperture to avoid GART mappings.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 10 +++++-----
>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c    |  1 +
>>   drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c  | 10 +++++-----
>>   3 files changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
>> index 3403ded39d13..ffd0ec9586d1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
>> @@ -65,16 +65,16 @@ static void 
>> gfxhub_v1_0_init_system_aperture_regs(struct amdgpu_device *adev)
>>   {
>>       uint64_t value;
>>
>> -    /* Disable AGP. */
>> +    /* Program the AGP BAR */
>>       WREG32_SOC15(GC, 0, mmMC_VM_AGP_BASE, 0);
>> -    WREG32_SOC15(GC, 0, mmMC_VM_AGP_TOP, 0);
>> -    WREG32_SOC15(GC, 0, mmMC_VM_AGP_BOT, 0xFFFFFFFF);
>> +    WREG32_SOC15(GC, 0, mmMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
>> +    WREG32_SOC15(GC, 0, mmMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
>>
>>       /* Program the system aperture low logical page number. */
>>       WREG32_SOC15(GC, 0, mmMC_VM_SYSTEM_APERTURE_LOW_ADDR,
>> -             adev->gmc.vram_start >> 18);
>> +             min(adev->gmc.vram_start, adev->gmc.agp_start) >> 18);
>>       WREG32_SOC15(GC, 0, mmMC_VM_SYSTEM_APERTURE_HIGH_ADDR,
>> -             adev->gmc.vram_end >> 18);
>> +             max(adev->gmc.vram_end, adev->gmc.agp_end) >> 18);
>
> It may not work for AGP after GART range.
>
> |--- vram ---|--- GART ---|--- AGP ---|
>
> in this case, we may have to replace AGP and GART range.

Yeah, that is exactly the reason why I've came up with "drm/amdgpu: put 
GART away from VRAM v2".

With this patch that scenario should never happen.

Regards,
Christian.

>
> Regards,
> Jerry
>
>>
>>       /* Set default page address. */
>>       value = adev->vram_scratch.gpu_addr - adev->gmc.vram_start
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index 60c8f848c102..3f40c153039f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -751,6 +751,7 @@ static void gmc_v9_0_vram_gtt_location(struct 
>> amdgpu_device *adev,
>>           base = mmhub_v1_0_get_fb_location(adev);
>>       amdgpu_gmc_vram_location(adev, &adev->gmc, base);
>>       amdgpu_gmc_gart_location(adev, mc);
>> +    amdgpu_gmc_agp_location(adev, mc);
>>       /* base offset of vram pages */
>>       adev->vm_manager.vram_base_offset = 
>> gfxhub_v1_0_get_mc_fb_offset(adev);
>>   }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
>> index 5f6a9c85488f..73d7c075dd33 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
>> @@ -76,16 +76,16 @@ static void 
>> mmhub_v1_0_init_system_aperture_regs(struct amdgpu_device *adev)
>>       uint64_t value;
>>       uint32_t tmp;
>>
>> -    /* Disable AGP. */
>> +    /* Program the AGP BAR */
>>       WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_BASE, 0);
>> -    WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_TOP, 0);
>> -    WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_BOT, 0x00FFFFFF);
>> +    WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
>> +    WREG32_SOC15(MMHUB, 0, mmMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
>>
>>       /* Program the system aperture low logical page number. */
>>       WREG32_SOC15(MMHUB, 0, mmMC_VM_SYSTEM_APERTURE_LOW_ADDR,
>> -             adev->gmc.vram_start >> 18);
>> +             min(adev->gmc.vram_start, adev->gmc.agp_start) >> 18);
>>       WREG32_SOC15(MMHUB, 0, mmMC_VM_SYSTEM_APERTURE_HIGH_ADDR,
>> -             adev->gmc.vram_end >> 18);
>> +             max(adev->gmc.vram_end, adev->gmc.agp_end) >> 18);
>>
>>       /* Set default page address. */
>>       value = adev->vram_scratch.gpu_addr - adev->gmc.vram_start +
>>

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

Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Alex Deucher]

On Mon, Aug 27, 2018 at 12:56 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Only use the lower address space on GMC9 for the system domain.
> Otherwise we would need to sign extend GMC addresses.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index e44b5191735d..d982956c8329 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>         if (r)
>                 return r;
>
> -       /* Set the internal MC address mask
> -        * This is the max address of the GPU's
> -        * internal address space.
> +       /* Use only the lower range for the internal MC address mask. This is
> +        * the max address of the GPU's internal address space.
>          */

Update this comment to note the sign extension requirements and note
that this max address is only for the system context (vmid 0).  With
that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Alex

> -       adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
> +       adev->gmc.mc_mask = 0x7fffffffffffULL;
>
>         /* set DMA mask + need_dma32 flags.
>          * PCIE - can handle 44-bits.
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Felix Kuehling]

The point of this series seems to be to allow access to small system
memory BOs (one page) without a GART mapping. I'm guessing that reduces
pressure on the GART and removes the need for HDP and TLB flushes. Why
does Patch 10 only enable that on GFXv9? Is there less benefit on older
chips?

Is this related to your recent changes to allow page tables in system
memory?

See my replies to patch 6 and 8. Other than that, the series is
Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>

Regards,
  Felix


On 2018-08-27 12:53 PM, Christian König wrote:
> Only use the lower address space on GMC9 for the system domain.
> Otherwise we would need to sign extend GMC addresses.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index e44b5191735d..d982956c8329 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>  	if (r)
>  		return r;
>  
> -	/* Set the internal MC address mask
> -	 * This is the max address of the GPU's
> -	 * internal address space.
> +	/* Use only the lower range for the internal MC address mask. This is
> +	 * the max address of the GPU's internal address space.
>  	 */
> -	adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
> +	adev->gmc.mc_mask = 0x7fffffffffffULL;
>  
>  	/* set DMA mask + need_dma32 flags.
>  	 * PCIE - can handle 44-bits.

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

Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[zhoucm1]

On 2018年08月28日 03:03, Felix Kuehling wrote:
> The point of this series seems to be to allow access to small system
> memory BOs (one page) without a GART mapping. I'm guessing that reduces
> pressure on the GART and removes the need for HDP and TLB flushes.
I think if adding these explain/reason to comments are better to enable 
AGP apperture. If that's true, it's really a clever idea.

Regards,
David Zhou

> Why
> does Patch 10 only enable that on GFXv9? Is there less benefit on older
> chips?
>
> Is this related to your recent changes to allow page tables in system
> memory?
>
> See my replies to patch 6 and 8. Other than that, the series is
> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
>
> Regards,
>    Felix
>
>
> On 2018-08-27 12:53 PM, Christian König wrote:
>> Only use the lower address space on GMC9 for the system domain.
>> Otherwise we would need to sign extend GMC addresses.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index e44b5191735d..d982956c8329 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>>   	if (r)
>>   		return r;
>>   
>> -	/* Set the internal MC address mask
>> -	 * This is the max address of the GPU's
>> -	 * internal address space.
>> +	/* Use only the lower range for the internal MC address mask. This is
>> +	 * the max address of the GPU's internal address space.
>>   	 */
>> -	adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>> +	adev->gmc.mc_mask = 0x7fffffffffffULL;
>>   
>>   	/* set DMA mask + need_dma32 flags.
>>   	 * PCIE - can handle 44-bits.
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Christian König]

Am 28.08.2018 um 06:21 schrieb zhoucm1:
>
>
> On 2018年08月28日 03:03, Felix Kuehling wrote:
>> The point of this series seems to be to allow access to small system
>> memory BOs (one page) without a GART mapping. I'm guessing that reduces
>> pressure on the GART and removes the need for HDP and TLB flushes.
> I think if adding these explain/reason to comments are better to 
> enable AGP apperture. If that's true, it's really a clever idea.

Well that is one use case. Another one is that we can enable dGPU 
without any VRAM.

That is really important for emulation and early ASIC testing.

>
> Regards,
> David Zhou
>
>> Why
>> does Patch 10 only enable that on GFXv9? Is there less benefit on older
>> chips?

Can easily be enabled on older chips as well, but I haven't yet had time 
to test it.

>>
>> Is this related to your recent changes to allow page tables in system
>> memory?

Yes, as it turned out that otherwise results in a huge bunch of extra 
GART mappings.

Regards,
Christian.

>>
>> See my replies to patch 6 and 8. Other than that, the series is
>> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>
>> Regards,
>>    Felix
>>
>>
>> On 2018-08-27 12:53 PM, Christian König wrote:
>>> Only use the lower address space on GMC9 for the system domain.
>>> Otherwise we would need to sign extend GMC addresses.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> index e44b5191735d..d982956c8329 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>>>       if (r)
>>>           return r;
>>>   -    /* Set the internal MC address mask
>>> -     * This is the max address of the GPU's
>>> -     * internal address space.
>>> +    /* Use only the lower range for the internal MC address mask. 
>>> This is
>>> +     * the max address of the GPU's internal address space.
>>>        */
>>> -    adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>> +    adev->gmc.mc_mask = 0x7fffffffffffULL;
>>>         /* set DMA mask + need_dma32 flags.
>>>        * PCIE - can handle 44-bits.
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>

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

Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Felix Kuehling]

On 2018-08-28 02:53 AM, Christian König wrote:
> Am 28.08.2018 um 06:21 schrieb zhoucm1:
>>
>>
>> On 2018年08月28日 03:03, Felix Kuehling wrote:
>>> The point of this series seems to be to allow access to small system
>>> memory BOs (one page) without a GART mapping. I'm guessing that reduces
>>> pressure on the GART and removes the need for HDP and TLB flushes.
>> I think if adding these explain/reason to comments are better to
>> enable AGP apperture. If that's true, it's really a clever idea.
>
> Well that is one use case. Another one is that we can enable dGPU
> without any VRAM.
>
> That is really important for emulation and early ASIC testing.
>
>>
>> Regards,
>> David Zhou
>>
>>> Why
>>> does Patch 10 only enable that on GFXv9? Is there less benefit on older
>>> chips?
>
> Can easily be enabled on older chips as well, but I haven't yet had
> time to test it.

But on older ASICs, page tables are typically bigger than one page. So
you'd still get GART mappings for them. BTW, how do you make sure those
page tables are physically contiguous in the first place?

>>>
>>> Is this related to your recent changes to allow page tables in system
>>> memory?
>
> Yes, as it turned out that otherwise results in a huge bunch of extra
> GART mappings.

Oh, so the point is to avoid GART mappings for page tables, which are a
single page on GFXv9. Now it makes sense. I was wondering what the
killer use case was.

Thanks,
  Felix

>
> Regards,
> Christian.
>
>>>
>>> See my replies to patch 6 and 8. Other than that, the series is
>>> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>>
>>> Regards,
>>>    Felix
>>>
>>>
>>> On 2018-08-27 12:53 PM, Christian König wrote:
>>>> Only use the lower address space on GMC9 for the system domain.
>>>> Otherwise we would need to sign extend GMC addresses.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>>>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> index e44b5191735d..d982956c8329 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>>>>       if (r)
>>>>           return r;
>>>>   -    /* Set the internal MC address mask
>>>> -     * This is the max address of the GPU's
>>>> -     * internal address space.
>>>> +    /* Use only the lower range for the internal MC address mask.
>>>> This is
>>>> +     * the max address of the GPU's internal address space.
>>>>        */
>>>> -    adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>>> +    adev->gmc.mc_mask = 0x7fffffffffffULL;
>>>>         /* set DMA mask + need_dma32 flags.
>>>>        * PCIE - can handle 44-bits.
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>

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

RE: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Xiao, Jack]

This series patches seems to make AGP aperture allowed to access any system memory (16GB) bypass GPU VM protection. 
If someone made a wrong logic requesting an illegal address which occasionally was located inside AGP aperture, 
but without any VM protection, the illegal address would be finally translated into a system memory address; 
if GPU read/wrote such system memory address, the system memory address might belong to kernel or any user application, 
the r/w operation would result in any unpredictable issue. 
The most important is that such kind of issue is so hard to be addressed.
Is it worth doing this, but exposing risk?

Regards,
Jack

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Felix Kuehling
Sent: Tuesday, August 28, 2018 3:03 AM
To: Christian König <ckoenig.leichtzumerken@gmail.com>; amd-gfx@lists.freedesktop.org; Koenig, Christian <Christian.Koenig@amd.com>
Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

The point of this series seems to be to allow access to small system memory BOs (one page) without a GART mapping. I'm guessing that reduces pressure on the GART and removes the need for HDP and TLB flushes. Why does Patch 10 only enable that on GFXv9? Is there less benefit on older chips?

Is this related to your recent changes to allow page tables in system memory?

See my replies to patch 6 and 8. Other than that, the series is
Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>

Regards,
  Felix


On 2018-08-27 12:53 PM, Christian König wrote:
> Only use the lower address space on GMC9 for the system domain.
> Otherwise we would need to sign extend GMC addresses.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index e44b5191735d..d982956c8329 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>  	if (r)
>  		return r;
>  
> -	/* Set the internal MC address mask
> -	 * This is the max address of the GPU's
> -	 * internal address space.
> +	/* Use only the lower range for the internal MC address mask. This is
> +	 * the max address of the GPU's internal address space.
>  	 */
> -	adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
> +	adev->gmc.mc_mask = 0x7fffffffffffULL;
>  
>  	/* set DMA mask + need_dma32 flags.
>  	 * PCIE - can handle 44-bits.

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

Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Christian König]

> This series patches seems to make AGP aperture allowed to access any system memory (16GB) bypass GPU VM protection.
The system aperture should only be active in the system domain, or 
otherwise applications would have access to local memory as well.

There are some bits in the VM registers to enable/disable that, but when 
we would have that setting incorrect we would see quite a bunch of other 
problems.

Might still be a good idea to double check if all the bits are setup 
correctly.

Regards,
Christian.

Am 28.08.2018 um 07:31 schrieb Xiao, Jack:
> This series patches seems to make AGP aperture allowed to access any system memory (16GB) bypass GPU VM protection.
> If someone made a wrong logic requesting an illegal address which occasionally was located inside AGP aperture,
> but without any VM protection, the illegal address would be finally translated into a system memory address;
> if GPU read/wrote such system memory address, the system memory address might belong to kernel or any user application,
> the r/w operation would result in any unpredictable issue.
> The most important is that such kind of issue is so hard to be addressed.
> Is it worth doing this, but exposing risk?
>
> Regards,
> Jack
>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Felix Kuehling
> Sent: Tuesday, August 28, 2018 3:03 AM
> To: Christian König <ckoenig.leichtzumerken@gmail.com>; amd-gfx@lists.freedesktop.org; Koenig, Christian <Christian.Koenig@amd.com>
> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9
>
> The point of this series seems to be to allow access to small system memory BOs (one page) without a GART mapping. I'm guessing that reduces pressure on the GART and removes the need for HDP and TLB flushes. Why does Patch 10 only enable that on GFXv9? Is there less benefit on older chips?
>
> Is this related to your recent changes to allow page tables in system memory?
>
> See my replies to patch 6 and 8. Other than that, the series is
> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
>
> Regards,
>    Felix
>
>
> On 2018-08-27 12:53 PM, Christian König wrote:
>> Only use the lower address space on GMC9 for the system domain.
>> Otherwise we would need to sign extend GMC addresses.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index e44b5191735d..d982956c8329 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>>   	if (r)
>>   		return r;
>>   
>> -	/* Set the internal MC address mask
>> -	 * This is the max address of the GPU's
>> -	 * internal address space.
>> +	/* Use only the lower range for the internal MC address mask. This is
>> +	 * the max address of the GPU's internal address space.
>>   	 */
>> -	adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>> +	adev->gmc.mc_mask = 0x7fffffffffffULL;
>>   
>>   	/* set DMA mask + need_dma32 flags.
>>   	 * PCIE - can handle 44-bits.
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

RE: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Xiao, Jack]

I mean it has risk to make GPU allowed to access to most system memory without explicit claiming, it's easier to mask problem.

Regards,
Jack

-----Original Message-----
From: Koenig, Christian 
Sent: Tuesday, August 28, 2018 2:46 PM
To: Xiao, Jack <Jack.Xiao@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>; Christian König <ckoenig.leichtzumerken@gmail.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

> This series patches seems to make AGP aperture allowed to access any system memory (16GB) bypass GPU VM protection.
The system aperture should only be active in the system domain, or otherwise applications would have access to local memory as well.

There are some bits in the VM registers to enable/disable that, but when we would have that setting incorrect we would see quite a bunch of other problems.

Might still be a good idea to double check if all the bits are setup correctly.

Regards,
Christian.

Am 28.08.2018 um 07:31 schrieb Xiao, Jack:
> This series patches seems to make AGP aperture allowed to access any system memory (16GB) bypass GPU VM protection.
> If someone made a wrong logic requesting an illegal address which 
> occasionally was located inside AGP aperture, but without any VM 
> protection, the illegal address would be finally translated into a 
> system memory address; if GPU read/wrote such system memory address, the system memory address might belong to kernel or any user application, the r/w operation would result in any unpredictable issue.
> The most important is that such kind of issue is so hard to be addressed.
> Is it worth doing this, but exposing risk?
>
> Regards,
> Jack
>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of 
> Felix Kuehling
> Sent: Tuesday, August 28, 2018 3:03 AM
> To: Christian König <ckoenig.leichtzumerken@gmail.com>; 
> amd-gfx@lists.freedesktop.org; Koenig, Christian 
> <Christian.Koenig@amd.com>
> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address 
> space on GMC9
>
> The point of this series seems to be to allow access to small system memory BOs (one page) without a GART mapping. I'm guessing that reduces pressure on the GART and removes the need for HDP and TLB flushes. Why does Patch 10 only enable that on GFXv9? Is there less benefit on older chips?
>
> Is this related to your recent changes to allow page tables in system memory?
>
> See my replies to patch 6 and 8. Other than that, the series is
> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
>
> Regards,
>    Felix
>
>
> On 2018-08-27 12:53 PM, Christian König wrote:
>> Only use the lower address space on GMC9 for the system domain.
>> Otherwise we would need to sign extend GMC addresses.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index e44b5191735d..d982956c8329 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>>   	if (r)
>>   		return r;
>>   
>> -	/* Set the internal MC address mask
>> -	 * This is the max address of the GPU's
>> -	 * internal address space.
>> +	/* Use only the lower range for the internal MC address mask. This is
>> +	 * the max address of the GPU's internal address space.
>>   	 */
>> -	adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>> +	adev->gmc.mc_mask = 0x7fffffffffffULL;
>>   
>>   	/* set DMA mask + need_dma32 flags.
>>   	 * PCIE - can handle 44-bits.
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Christian König]

Well that is indeed true, but we still have IOMMU between the GPU and 
the system memory.

So we should still catch issues when something goes terrible wrong.

Additional to that only the system domain, e.g. kernel copies, page 
table updates etc are allowed to use it.

Regards,
Christian.

Am 28.08.2018 um 09:06 schrieb Xiao, Jack:
> I mean it has risk to make GPU allowed to access to most system memory without explicit claiming, it's easier to mask problem.
>
> Regards,
> Jack
>
> -----Original Message-----
> From: Koenig, Christian
> Sent: Tuesday, August 28, 2018 2:46 PM
> To: Xiao, Jack <Jack.Xiao@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>; Christian König <ckoenig.leichtzumerken@gmail.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9
>
>> This series patches seems to make AGP aperture allowed to access any system memory (16GB) bypass GPU VM protection.
> The system aperture should only be active in the system domain, or otherwise applications would have access to local memory as well.
>
> There are some bits in the VM registers to enable/disable that, but when we would have that setting incorrect we would see quite a bunch of other problems.
>
> Might still be a good idea to double check if all the bits are setup correctly.
>
> Regards,
> Christian.
>
> Am 28.08.2018 um 07:31 schrieb Xiao, Jack:
>> This series patches seems to make AGP aperture allowed to access any system memory (16GB) bypass GPU VM protection.
>> If someone made a wrong logic requesting an illegal address which
>> occasionally was located inside AGP aperture, but without any VM
>> protection, the illegal address would be finally translated into a
>> system memory address; if GPU read/wrote such system memory address, the system memory address might belong to kernel or any user application, the r/w operation would result in any unpredictable issue.
>> The most important is that such kind of issue is so hard to be addressed.
>> Is it worth doing this, but exposing risk?
>>
>> Regards,
>> Jack
>>
>> -----Original Message-----
>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>> Felix Kuehling
>> Sent: Tuesday, August 28, 2018 3:03 AM
>> To: Christian König <ckoenig.leichtzumerken@gmail.com>;
>> amd-gfx@lists.freedesktop.org; Koenig, Christian
>> <Christian.Koenig@amd.com>
>> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address
>> space on GMC9
>>
>> The point of this series seems to be to allow access to small system memory BOs (one page) without a GART mapping. I'm guessing that reduces pressure on the GART and removes the need for HDP and TLB flushes. Why does Patch 10 only enable that on GFXv9? Is there less benefit on older chips?
>>
>> Is this related to your recent changes to allow page tables in system memory?
>>
>> See my replies to patch 6 and 8. Other than that, the series is
>> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>
>> Regards,
>>     Felix
>>
>>
>> On 2018-08-27 12:53 PM, Christian König wrote:
>>> Only use the lower address space on GMC9 for the system domain.
>>> Otherwise we would need to sign extend GMC addresses.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>>>    1 file changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> index e44b5191735d..d982956c8329 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>>>    	if (r)
>>>    		return r;
>>>    
>>> -	/* Set the internal MC address mask
>>> -	 * This is the max address of the GPU's
>>> -	 * internal address space.
>>> +	/* Use only the lower range for the internal MC address mask. This is
>>> +	 * the max address of the GPU's internal address space.
>>>    	 */
>>> -	adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>> +	adev->gmc.mc_mask = 0x7fffffffffffULL;
>>>    
>>>    	/* set DMA mask + need_dma32 flags.
>>>    	 * PCIE - can handle 44-bits.
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Felix Kuehling]

The GPU always has access to the entire guest physical address space.
You can just program arbitrary addresses into page table entries and set
the system bit. That's how GART and GPUVM mappings work. They will not
go through the AGP aperture. And there is no mechanism (other than an
IOMMU) to protect system memory from GPU access.

Regards,
  Felix


On 2018-08-28 07:41 AM, Christian König wrote:
> Well that is indeed true, but we still have IOMMU between the GPU and
> the system memory.
>
> So we should still catch issues when something goes terrible wrong.
>
> Additional to that only the system domain, e.g. kernel copies, page
> table updates etc are allowed to use it.
>
> Regards,
> Christian.
>
> Am 28.08.2018 um 09:06 schrieb Xiao, Jack:
>> I mean it has risk to make GPU allowed to access to most system
>> memory without explicit claiming, it's easier to mask problem.
>>
>> Regards,
>> Jack
>>
>> -----Original Message-----
>> From: Koenig, Christian
>> Sent: Tuesday, August 28, 2018 2:46 PM
>> To: Xiao, Jack <Jack.Xiao@amd.com>; Kuehling, Felix
>> <Felix.Kuehling@amd.com>; Christian König
>> <ckoenig.leichtzumerken@gmail.com>; amd-gfx@lists.freedesktop.org
>> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address
>> space on GMC9
>>
>>> This series patches seems to make AGP aperture allowed to access any
>>> system memory (16GB) bypass GPU VM protection.
>> The system aperture should only be active in the system domain, or
>> otherwise applications would have access to local memory as well.
>>
>> There are some bits in the VM registers to enable/disable that, but
>> when we would have that setting incorrect we would see quite a bunch
>> of other problems.
>>
>> Might still be a good idea to double check if all the bits are setup
>> correctly.
>>
>> Regards,
>> Christian.
>>
>> Am 28.08.2018 um 07:31 schrieb Xiao, Jack:
>>> This series patches seems to make AGP aperture allowed to access any
>>> system memory (16GB) bypass GPU VM protection.
>>> If someone made a wrong logic requesting an illegal address which
>>> occasionally was located inside AGP aperture, but without any VM
>>> protection, the illegal address would be finally translated into a
>>> system memory address; if GPU read/wrote such system memory address,
>>> the system memory address might belong to kernel or any user
>>> application, the r/w operation would result in any unpredictable issue.
>>> The most important is that such kind of issue is so hard to be
>>> addressed.
>>> Is it worth doing this, but exposing risk?
>>>
>>> Regards,
>>> Jack
>>>
>>> -----Original Message-----
>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>>> Felix Kuehling
>>> Sent: Tuesday, August 28, 2018 3:03 AM
>>> To: Christian König <ckoenig.leichtzumerken@gmail.com>;
>>> amd-gfx@lists.freedesktop.org; Koenig, Christian
>>> <Christian.Koenig@amd.com>
>>> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address
>>> space on GMC9
>>>
>>> The point of this series seems to be to allow access to small system
>>> memory BOs (one page) without a GART mapping. I'm guessing that
>>> reduces pressure on the GART and removes the need for HDP and TLB
>>> flushes. Why does Patch 10 only enable that on GFXv9? Is there less
>>> benefit on older chips?
>>>
>>> Is this related to your recent changes to allow page tables in
>>> system memory?
>>>
>>> See my replies to patch 6 and 8. Other than that, the series is
>>> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>>
>>> Regards,
>>>     Felix
>>>
>>>
>>> On 2018-08-27 12:53 PM, Christian König wrote:
>>>> Only use the lower address space on GMC9 for the system domain.
>>>> Otherwise we would need to sign extend GMC addresses.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>>>>    1 file changed, 3 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> index e44b5191735d..d982956c8329 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>>>>        if (r)
>>>>            return r;
>>>>    -    /* Set the internal MC address mask
>>>> -     * This is the max address of the GPU's
>>>> -     * internal address space.
>>>> +    /* Use only the lower range for the internal MC address mask.
>>>> This is
>>>> +     * the max address of the GPU's internal address space.
>>>>         */
>>>> -    adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>>> +    adev->gmc.mc_mask = 0x7fffffffffffULL;
>>>>           /* set DMA mask + need_dma32 flags.
>>>>         * PCIE - can handle 44-bits.
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>

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

Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Christian König]

Completely agree with Felix here.

It makes system memory access slightly simpler, but I would say that you 
accidentally corrupt the GART table and that you accidentally corrupt 
the the AGP aperture is equally likely.

Regards,
Christian.

Am 28.08.2018 um 20:12 schrieb Felix Kuehling:
> The GPU always has access to the entire guest physical address space.
> You can just program arbitrary addresses into page table entries and set
> the system bit. That's how GART and GPUVM mappings work. They will not
> go through the AGP aperture. And there is no mechanism (other than an
> IOMMU) to protect system memory from GPU access.
>
> Regards,
>    Felix
>
>
> On 2018-08-28 07:41 AM, Christian König wrote:
>> Well that is indeed true, but we still have IOMMU between the GPU and
>> the system memory.
>>
>> So we should still catch issues when something goes terrible wrong.
>>
>> Additional to that only the system domain, e.g. kernel copies, page
>> table updates etc are allowed to use it.
>>
>> Regards,
>> Christian.
>>
>> Am 28.08.2018 um 09:06 schrieb Xiao, Jack:
>>> I mean it has risk to make GPU allowed to access to most system
>>> memory without explicit claiming, it's easier to mask problem.
>>>
>>> Regards,
>>> Jack
>>>
>>> -----Original Message-----
>>> From: Koenig, Christian
>>> Sent: Tuesday, August 28, 2018 2:46 PM
>>> To: Xiao, Jack <Jack.Xiao@amd.com>; Kuehling, Felix
>>> <Felix.Kuehling@amd.com>; Christian König
>>> <ckoenig.leichtzumerken@gmail.com>; amd-gfx@lists.freedesktop.org
>>> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address
>>> space on GMC9
>>>
>>>> This series patches seems to make AGP aperture allowed to access any
>>>> system memory (16GB) bypass GPU VM protection.
>>> The system aperture should only be active in the system domain, or
>>> otherwise applications would have access to local memory as well.
>>>
>>> There are some bits in the VM registers to enable/disable that, but
>>> when we would have that setting incorrect we would see quite a bunch
>>> of other problems.
>>>
>>> Might still be a good idea to double check if all the bits are setup
>>> correctly.
>>>
>>> Regards,
>>> Christian.
>>>
>>> Am 28.08.2018 um 07:31 schrieb Xiao, Jack:
>>>> This series patches seems to make AGP aperture allowed to access any
>>>> system memory (16GB) bypass GPU VM protection.
>>>> If someone made a wrong logic requesting an illegal address which
>>>> occasionally was located inside AGP aperture, but without any VM
>>>> protection, the illegal address would be finally translated into a
>>>> system memory address; if GPU read/wrote such system memory address,
>>>> the system memory address might belong to kernel or any user
>>>> application, the r/w operation would result in any unpredictable issue.
>>>> The most important is that such kind of issue is so hard to be
>>>> addressed.
>>>> Is it worth doing this, but exposing risk?
>>>>
>>>> Regards,
>>>> Jack
>>>>
>>>> -----Original Message-----
>>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>>>> Felix Kuehling
>>>> Sent: Tuesday, August 28, 2018 3:03 AM
>>>> To: Christian König <ckoenig.leichtzumerken@gmail.com>;
>>>> amd-gfx@lists.freedesktop.org; Koenig, Christian
>>>> <Christian.Koenig@amd.com>
>>>> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address
>>>> space on GMC9
>>>>
>>>> The point of this series seems to be to allow access to small system
>>>> memory BOs (one page) without a GART mapping. I'm guessing that
>>>> reduces pressure on the GART and removes the need for HDP and TLB
>>>> flushes. Why does Patch 10 only enable that on GFXv9? Is there less
>>>> benefit on older chips?
>>>>
>>>> Is this related to your recent changes to allow page tables in
>>>> system memory?
>>>>
>>>> See my replies to patch 6 and 8. Other than that, the series is
>>>> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>>>
>>>> Regards,
>>>>      Felix
>>>>
>>>>
>>>> On 2018-08-27 12:53 PM, Christian König wrote:
>>>>> Only use the lower address space on GMC9 for the system domain.
>>>>> Otherwise we would need to sign extend GMC addresses.
>>>>>
>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>> ---
>>>>>     drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>>>>>     1 file changed, 3 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> index e44b5191735d..d982956c8329 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>>>>>         if (r)
>>>>>             return r;
>>>>>     -    /* Set the internal MC address mask
>>>>> -     * This is the max address of the GPU's
>>>>> -     * internal address space.
>>>>> +    /* Use only the lower range for the internal MC address mask.
>>>>> This is
>>>>> +     * the max address of the GPU's internal address space.
>>>>>          */
>>>>> -    adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>>>> +    adev->gmc.mc_mask = 0x7fffffffffffULL;
>>>>>            /* set DMA mask + need_dma32 flags.
>>>>>          * PCIE - can handle 44-bits.
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

RE: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Xiao, Jack]

Can you explain what the benefits can be gained from AGP aperture enablement? Otherwise, it would increase our maintenance workload.

Regards,
Jack

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 
Sent: Wednesday, August 29, 2018 4:47 PM
To: Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Xiao, Jack <Jack.Xiao@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

Completely agree with Felix here.

It makes system memory access slightly simpler, but I would say that you accidentally corrupt the GART table and that you accidentally corrupt the the AGP aperture is equally likely.

Regards,
Christian.

Am 28.08.2018 um 20:12 schrieb Felix Kuehling:
> The GPU always has access to the entire guest physical address space.
> You can just program arbitrary addresses into page table entries and 
> set the system bit. That's how GART and GPUVM mappings work. They will 
> not go through the AGP aperture. And there is no mechanism (other than 
> an
> IOMMU) to protect system memory from GPU access.
>
> Regards,
>    Felix
>
>
> On 2018-08-28 07:41 AM, Christian König wrote:
>> Well that is indeed true, but we still have IOMMU between the GPU and 
>> the system memory.
>>
>> So we should still catch issues when something goes terrible wrong.
>>
>> Additional to that only the system domain, e.g. kernel copies, page 
>> table updates etc are allowed to use it.
>>
>> Regards,
>> Christian.
>>
>> Am 28.08.2018 um 09:06 schrieb Xiao, Jack:
>>> I mean it has risk to make GPU allowed to access to most system 
>>> memory without explicit claiming, it's easier to mask problem.
>>>
>>> Regards,
>>> Jack
>>>
>>> -----Original Message-----
>>> From: Koenig, Christian
>>> Sent: Tuesday, August 28, 2018 2:46 PM
>>> To: Xiao, Jack <Jack.Xiao@amd.com>; Kuehling, Felix 
>>> <Felix.Kuehling@amd.com>; Christian König 
>>> <ckoenig.leichtzumerken@gmail.com>; amd-gfx@lists.freedesktop.org
>>> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address 
>>> space on GMC9
>>>
>>>> This series patches seems to make AGP aperture allowed to access 
>>>> any system memory (16GB) bypass GPU VM protection.
>>> The system aperture should only be active in the system domain, or 
>>> otherwise applications would have access to local memory as well.
>>>
>>> There are some bits in the VM registers to enable/disable that, but 
>>> when we would have that setting incorrect we would see quite a bunch 
>>> of other problems.
>>>
>>> Might still be a good idea to double check if all the bits are setup 
>>> correctly.
>>>
>>> Regards,
>>> Christian.
>>>
>>> Am 28.08.2018 um 07:31 schrieb Xiao, Jack:
>>>> This series patches seems to make AGP aperture allowed to access 
>>>> any system memory (16GB) bypass GPU VM protection.
>>>> If someone made a wrong logic requesting an illegal address which 
>>>> occasionally was located inside AGP aperture, but without any VM 
>>>> protection, the illegal address would be finally translated into a 
>>>> system memory address; if GPU read/wrote such system memory 
>>>> address, the system memory address might belong to kernel or any 
>>>> user application, the r/w operation would result in any unpredictable issue.
>>>> The most important is that such kind of issue is so hard to be 
>>>> addressed.
>>>> Is it worth doing this, but exposing risk?
>>>>
>>>> Regards,
>>>> Jack
>>>>
>>>> -----Original Message-----
>>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of 
>>>> Felix Kuehling
>>>> Sent: Tuesday, August 28, 2018 3:03 AM
>>>> To: Christian König <ckoenig.leichtzumerken@gmail.com>;
>>>> amd-gfx@lists.freedesktop.org; Koenig, Christian 
>>>> <Christian.Koenig@amd.com>
>>>> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address 
>>>> space on GMC9
>>>>
>>>> The point of this series seems to be to allow access to small 
>>>> system memory BOs (one page) without a GART mapping. I'm guessing 
>>>> that reduces pressure on the GART and removes the need for HDP and 
>>>> TLB flushes. Why does Patch 10 only enable that on GFXv9? Is there 
>>>> less benefit on older chips?
>>>>
>>>> Is this related to your recent changes to allow page tables in 
>>>> system memory?
>>>>
>>>> See my replies to patch 6 and 8. Other than that, the series is
>>>> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>>>
>>>> Regards,
>>>>      Felix
>>>>
>>>>
>>>> On 2018-08-27 12:53 PM, Christian König wrote:
>>>>> Only use the lower address space on GMC9 for the system domain.
>>>>> Otherwise we would need to sign extend GMC addresses.
>>>>>
>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>> ---
>>>>>     drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>>>>>     1 file changed, 3 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> index e44b5191735d..d982956c8329 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>>>>>         if (r)
>>>>>             return r;
>>>>>     -    /* Set the internal MC address mask
>>>>> -     * This is the max address of the GPU's
>>>>> -     * internal address space.
>>>>> +    /* Use only the lower range for the internal MC address mask.
>>>>> This is
>>>>> +     * the max address of the GPU's internal address space.
>>>>>          */
>>>>> -    adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>>>> +    adev->gmc.mc_mask = 0x7fffffffffffULL;
>>>>>            /* set DMA mask + need_dma32 flags.
>>>>>          * PCIE - can handle 44-bits.
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Christian König]

As explained before we don't need a GART mapping any more for small 
allocations.

Additional to that we want it to support early bringup where no VRAM is 
attached to the ASIC.

Regards,
Christian.

Am 30.08.2018 um 05:48 schrieb Xiao, Jack:
> Can you explain what the benefits can be gained from AGP aperture enablement? Otherwise, it would increase our maintenance workload.
>
> Regards,
> Jack
>
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Wednesday, August 29, 2018 4:47 PM
> To: Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Xiao, Jack <Jack.Xiao@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9
>
> Completely agree with Felix here.
>
> It makes system memory access slightly simpler, but I would say that you accidentally corrupt the GART table and that you accidentally corrupt the the AGP aperture is equally likely.
>
> Regards,
> Christian.
>
> Am 28.08.2018 um 20:12 schrieb Felix Kuehling:
>> The GPU always has access to the entire guest physical address space.
>> You can just program arbitrary addresses into page table entries and
>> set the system bit. That's how GART and GPUVM mappings work. They will
>> not go through the AGP aperture. And there is no mechanism (other than
>> an
>> IOMMU) to protect system memory from GPU access.
>>
>> Regards,
>>     Felix
>>
>>
>> On 2018-08-28 07:41 AM, Christian König wrote:
>>> Well that is indeed true, but we still have IOMMU between the GPU and
>>> the system memory.
>>>
>>> So we should still catch issues when something goes terrible wrong.
>>>
>>> Additional to that only the system domain, e.g. kernel copies, page
>>> table updates etc are allowed to use it.
>>>
>>> Regards,
>>> Christian.
>>>
>>> Am 28.08.2018 um 09:06 schrieb Xiao, Jack:
>>>> I mean it has risk to make GPU allowed to access to most system
>>>> memory without explicit claiming, it's easier to mask problem.
>>>>
>>>> Regards,
>>>> Jack
>>>>
>>>> -----Original Message-----
>>>> From: Koenig, Christian
>>>> Sent: Tuesday, August 28, 2018 2:46 PM
>>>> To: Xiao, Jack <Jack.Xiao@amd.com>; Kuehling, Felix
>>>> <Felix.Kuehling@amd.com>; Christian König
>>>> <ckoenig.leichtzumerken@gmail.com>; amd-gfx@lists.freedesktop.org
>>>> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address
>>>> space on GMC9
>>>>
>>>>> This series patches seems to make AGP aperture allowed to access
>>>>> any system memory (16GB) bypass GPU VM protection.
>>>> The system aperture should only be active in the system domain, or
>>>> otherwise applications would have access to local memory as well.
>>>>
>>>> There are some bits in the VM registers to enable/disable that, but
>>>> when we would have that setting incorrect we would see quite a bunch
>>>> of other problems.
>>>>
>>>> Might still be a good idea to double check if all the bits are setup
>>>> correctly.
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>> Am 28.08.2018 um 07:31 schrieb Xiao, Jack:
>>>>> This series patches seems to make AGP aperture allowed to access
>>>>> any system memory (16GB) bypass GPU VM protection.
>>>>> If someone made a wrong logic requesting an illegal address which
>>>>> occasionally was located inside AGP aperture, but without any VM
>>>>> protection, the illegal address would be finally translated into a
>>>>> system memory address; if GPU read/wrote such system memory
>>>>> address, the system memory address might belong to kernel or any
>>>>> user application, the r/w operation would result in any unpredictable issue.
>>>>> The most important is that such kind of issue is so hard to be
>>>>> addressed.
>>>>> Is it worth doing this, but exposing risk?
>>>>>
>>>>> Regards,
>>>>> Jack
>>>>>
>>>>> -----Original Message-----
>>>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>>>>> Felix Kuehling
>>>>> Sent: Tuesday, August 28, 2018 3:03 AM
>>>>> To: Christian König <ckoenig.leichtzumerken@gmail.com>;
>>>>> amd-gfx@lists.freedesktop.org; Koenig, Christian
>>>>> <Christian.Koenig@amd.com>
>>>>> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address
>>>>> space on GMC9
>>>>>
>>>>> The point of this series seems to be to allow access to small
>>>>> system memory BOs (one page) without a GART mapping. I'm guessing
>>>>> that reduces pressure on the GART and removes the need for HDP and
>>>>> TLB flushes. Why does Patch 10 only enable that on GFXv9? Is there
>>>>> less benefit on older chips?
>>>>>
>>>>> Is this related to your recent changes to allow page tables in
>>>>> system memory?
>>>>>
>>>>> See my replies to patch 6 and 8. Other than that, the series is
>>>>> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>>>>
>>>>> Regards,
>>>>>       Felix
>>>>>
>>>>>
>>>>> On 2018-08-27 12:53 PM, Christian König wrote:
>>>>>> Only use the lower address space on GMC9 for the system domain.
>>>>>> Otherwise we would need to sign extend GMC addresses.
>>>>>>
>>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>>> ---
>>>>>>      drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>>>>>>      1 file changed, 3 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>>> index e44b5191735d..d982956c8329 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>>> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>>>>>>          if (r)
>>>>>>              return r;
>>>>>>      -    /* Set the internal MC address mask
>>>>>> -     * This is the max address of the GPU's
>>>>>> -     * internal address space.
>>>>>> +    /* Use only the lower range for the internal MC address mask.
>>>>>> This is
>>>>>> +     * the max address of the GPU's internal address space.
>>>>>>           */
>>>>>> -    adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>>>>> +    adev->gmc.mc_mask = 0x7fffffffffffULL;
>>>>>>             /* set DMA mask + need_dma32 flags.
>>>>>>           * PCIE - can handle 44-bits.
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

RE: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Xiao, Jack]

> Well that is indeed true, but we still have IOMMU between the GPU and the system memory.
Some platform hasn't IOMMU, or IOMMU isn’t enabled by default. 
If such kind of issue was reported by customer, it was so hard to narrow down.

> Additional to that only the system domain, e.g. kernel copies, page table updates etc are allowed to use it.
What was the motivation of this series patch? 
I guess it is for performance improvement, right? But AGP aperture memory is translated as mtype=UC same with GART (mtype=UC). 
The more overhead on GART is gpuVM pagetable walker, but I guess the overhead is little.

Regards,
Jack

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 
Sent: Tuesday, August 28, 2018 7:42 PM
To: Xiao, Jack <Jack.Xiao@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

Well that is indeed true, but we still have IOMMU between the GPU and the system memory.

So we should still catch issues when something goes terrible wrong.

Additional to that only the system domain, e.g. kernel copies, page table updates etc are allowed to use it.

Regards,
Christian.

Am 28.08.2018 um 09:06 schrieb Xiao, Jack:
> I mean it has risk to make GPU allowed to access to most system memory without explicit claiming, it's easier to mask problem.
>
> Regards,
> Jack
>
> -----Original Message-----
> From: Koenig, Christian
> Sent: Tuesday, August 28, 2018 2:46 PM
> To: Xiao, Jack <Jack.Xiao@amd.com>; Kuehling, Felix 
> <Felix.Kuehling@amd.com>; Christian König 
> <ckoenig.leichtzumerken@gmail.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address 
> space on GMC9
>
>> This series patches seems to make AGP aperture allowed to access any system memory (16GB) bypass GPU VM protection.
> The system aperture should only be active in the system domain, or otherwise applications would have access to local memory as well.
>
> There are some bits in the VM registers to enable/disable that, but when we would have that setting incorrect we would see quite a bunch of other problems.
>
> Might still be a good idea to double check if all the bits are setup correctly.
>
> Regards,
> Christian.
>
> Am 28.08.2018 um 07:31 schrieb Xiao, Jack:
>> This series patches seems to make AGP aperture allowed to access any system memory (16GB) bypass GPU VM protection.
>> If someone made a wrong logic requesting an illegal address which 
>> occasionally was located inside AGP aperture, but without any VM 
>> protection, the illegal address would be finally translated into a 
>> system memory address; if GPU read/wrote such system memory address, the system memory address might belong to kernel or any user application, the r/w operation would result in any unpredictable issue.
>> The most important is that such kind of issue is so hard to be addressed.
>> Is it worth doing this, but exposing risk?
>>
>> Regards,
>> Jack
>>
>> -----Original Message-----
>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of 
>> Felix Kuehling
>> Sent: Tuesday, August 28, 2018 3:03 AM
>> To: Christian König <ckoenig.leichtzumerken@gmail.com>;
>> amd-gfx@lists.freedesktop.org; Koenig, Christian 
>> <Christian.Koenig@amd.com>
>> Subject: Re: [PATCH 01/10] drm/amdgpu: use only the lower address 
>> space on GMC9
>>
>> The point of this series seems to be to allow access to small system memory BOs (one page) without a GART mapping. I'm guessing that reduces pressure on the GART and removes the need for HDP and TLB flushes. Why does Patch 10 only enable that on GFXv9? Is there less benefit on older chips?
>>
>> Is this related to your recent changes to allow page tables in system memory?
>>
>> See my replies to patch 6 and 8. Other than that, the series is
>> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>
>> Regards,
>>     Felix
>>
>>
>> On 2018-08-27 12:53 PM, Christian König wrote:
>>> Only use the lower address space on GMC9 for the system domain.
>>> Otherwise we would need to sign extend GMC addresses.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>>>    1 file changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> index e44b5191735d..d982956c8329 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>>>    	if (r)
>>>    		return r;
>>>    
>>> -	/* Set the internal MC address mask
>>> -	 * This is the max address of the GPU's
>>> -	 * internal address space.
>>> +	/* Use only the lower range for the internal MC address mask. This is
>>> +	 * the max address of the GPU's internal address space.
>>>    	 */
>>> -	adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>> +	adev->gmc.mc_mask = 0x7fffffffffffULL;
>>>    
>>>    	/* set DMA mask + need_dma32 flags.
>>>    	 * PCIE - can handle 44-bits.
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Zhang, Jerry (Junwei)]

On 08/28/2018 12:53 AM, Christian König wrote:
> Only use the lower address space on GMC9 for the system domain.
> Otherwise we would need to sign extend GMC addresses.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index e44b5191735d..d982956c8329 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>   	if (r)
>   		return r;
>
> -	/* Set the internal MC address mask
> -	 * This is the max address of the GPU's
> -	 * internal address space.
> +	/* Use only the lower range for the internal MC address mask. This is
> +	 * the max address of the GPU's internal address space.
>   	 */
> -	adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
> +	adev->gmc.mc_mask = 0x7fffffffffffULL;

do we need to update vm_size as 128T at the same time?

Jerry

>
>   	/* set DMA mask + need_dma32 flags.
>   	 * PCIE - can handle 44-bits.
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 01/10] drm/amdgpu: use only the lower address space on GMC9

[Zhang, Jerry (Junwei)]

On 08/28/2018 09:56 AM, Zhang, Jerry (Junwei) wrote:
> On 08/28/2018 12:53 AM, Christian König wrote:
>> Only use the lower address space on GMC9 for the system domain.
>> Otherwise we would need to sign extend GMC addresses.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index e44b5191735d..d982956c8329 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -938,11 +938,10 @@ static int gmc_v9_0_sw_init(void *handle)
>>       if (r)
>>           return r;
>>
>> -    /* Set the internal MC address mask
>> -     * This is the max address of the GPU's
>> -     * internal address space.
>> +    /* Use only the lower range for the internal MC address mask. This is
>> +     * the max address of the GPU's internal address space.
>>        */
>> -    adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>> +    adev->gmc.mc_mask = 0x7fffffffffffULL;
>
> do we need to update vm_size as 128T at the same time?

Likely no, since we use that for system domain only.

BTW, how do we decide it's size limitation.
looks we always use that less than 40bit?

Jerry

>
> Jerry
>
>>
>>       /* set DMA mask + need_dma32 flags.
>>        * PCIE - can handle 44-bits.
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx