All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 04/12] drm/amdgpu/gmc9: Adjust GART and AGP location with xgmi offset
@ 2018-09-07 20:09 shaoyunl
       [not found] ` <1536350992-31224-1-git-send-email-Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 4+ messages in thread
From: shaoyunl @ 2018-09-07 20:09 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Alex Deucher, Shaoyun Liu

From: Alex Deucher <alexander.deucher@amd.com>

On hives with xgmi enabled, the fb_location aperture is a size
which defines the total framebuffer size of all nodes in the
hive.  Each GPU in the hive has the same view via the fb_location
aperture.  GPU0 starts at offset (0 * segment size),
GPU1 starts at offset (1 * segment size), etc.

For access to local vram on each GPU, we need to take this offset into
account. This including on setting up GPUVM page table and GART table

Change-Id: I9efd510bed68fdb9afdfbdc76e1046792471ee78
Acked-by: Huang Rui <ray.huang@amd.com>
Acked-by: Slava Abramov <slava.abramov@amd.com>
Signed-off-by: Shaoyun Liu <Shaoyun.Liu@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c  | 20 ++++++++++----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h  |  7 +++++++
 drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c |  3 +++
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c    |  6 ++++++
 drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c  |  7 +++++++
 5 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 6acdeeb..cf97c1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -147,8 +147,8 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
 	/* 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;
-	size_af = adev->gmc.mc_mask + 1 - ALIGN(mc->vram_end + 1, four_gb);
+	size_bf = mc->fb_start;
+	size_af = adev->gmc.mc_mask + 1 - ALIGN(mc->fb_end + 1, four_gb);
 
 	if (mc->gart_size > max(size_bf, size_af)) {
 		dev_warn(adev->dev, "limiting GART\n");
@@ -184,23 +184,23 @@ void amdgpu_gmc_agp_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
 	const uint64_t sixteen_gb_mask = ~(sixteen_gb - 1);
 	u64 size_af, size_bf;
 
-	if (mc->vram_start > mc->gart_start) {
-		size_bf = (mc->vram_start & sixteen_gb_mask) -
+	if (mc->fb_start > mc->gart_start) {
+		size_bf = (mc->fb_start & sixteen_gb_mask) -
 			ALIGN(mc->gart_end + 1, sixteen_gb);
-		size_af = mc->mc_mask + 1 - ALIGN(mc->vram_end + 1, sixteen_gb);
+		size_af = mc->mc_mask + 1 - ALIGN(mc->fb_end + 1, sixteen_gb);
 	} else {
-		size_bf = mc->vram_start & sixteen_gb_mask;
+		size_bf = mc->fb_start & sixteen_gb_mask;
 		size_af = (mc->gart_start & sixteen_gb_mask) -
-			ALIGN(mc->vram_end + 1, sixteen_gb);
+			ALIGN(mc->fb_end + 1, sixteen_gb);
 	}
 
 	if (size_bf > size_af) {
-		mc->agp_start = mc->vram_start > mc->gart_start ?
+		mc->agp_start = mc->fb_start > mc->gart_start ?
 			mc->gart_end + 1 : 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_start = (mc->fb_start > mc->gart_start ?
+			mc->fb_end : mc->gart_end) + 1,
 		mc->agp_size = size_af;
 	}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index a929a55..df96dfe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -114,6 +114,13 @@ struct amdgpu_gmc {
 	u64			gart_end;
 	u64			vram_start;
 	u64			vram_end;
+	/* FB region , it's same as local vram region in single GPU, in XGMI
+	 * configuration, this region covers all GPUs in the same hive ,
+	 * each GPU in the hive has the same view of this FB region .
+	 * GPU0's vram starts at offset (0 * segment size) ,
+	 * GPU1 starts at offset (1 * segment size), etc.   */
+	u64			fb_start;
+	u64			fb_end;
 	unsigned		vram_width;
 	u64			real_vram_size;
 	int			vram_mtrr;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
index d4170cb..5e9ab8e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
@@ -44,6 +44,9 @@ int gfxhub_v1_1_get_xgmi_info(struct amdgpu_device *adev)
 			REG_GET_FIELD(xgmi_lfb_cntl, MC_VM_XGMI_LFB_CNTL, PF_LFB_REGION);
 		if (adev->gmc.xgmi.physical_node_id > 3)
 			return -EINVAL;
+		adev->gmc.xgmi.node_segment_size = REG_GET_FIELD(
+			RREG32_SOC15(GC, 0, mmMC_VM_XGMI_LFB_SIZE),
+			MC_VM_XGMI_LFB_SIZE, PF_LFB_SIZE) << 24;
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index e9b5a13..b1c8489 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -771,12 +771,18 @@ 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);
+	/* add the xgmi offset of the physical node */
+	base += adev->gmc.xgmi.physical_node_id * adev->gmc.xgmi.node_segment_size;
 	amdgpu_gmc_vram_location(adev, &adev->gmc, base);
 	amdgpu_gmc_gart_location(adev, mc);
 	if (!amdgpu_sriov_vf(adev))
 		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);
+
+	/* XXX: add the xgmi offset of the physical node? */
+	adev->vm_manager.vram_base_offset +=
+		adev->gmc.xgmi.physical_node_id * adev->gmc.xgmi.node_segment_size;
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
index 73d7c07..0e09549 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
@@ -38,10 +38,17 @@
 u64 mmhub_v1_0_get_fb_location(struct amdgpu_device *adev)
 {
 	u64 base = RREG32_SOC15(MMHUB, 0, mmMC_VM_FB_LOCATION_BASE);
+	u64 top = RREG32_SOC15(MMHUB, 0, mmMC_VM_FB_LOCATION_TOP);
 
 	base &= MC_VM_FB_LOCATION_BASE__FB_BASE_MASK;
 	base <<= 24;
 
+	top &= MC_VM_FB_LOCATION_TOP__FB_TOP_MASK;
+	top <<= 24;
+
+	adev->gmc.fb_start = base;
+	adev->gmc.fb_end = top;
+
 	return base;
 }
 
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH 04/12] drm/amdgpu/gmc9: Adjust GART and AGP location with xgmi offset
       [not found] ` <1536350992-31224-1-git-send-email-Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>
@ 2018-09-08  9:47   ` Huang Rui
  2018-09-08 18:06   ` Christian König
  1 sibling, 0 replies; 4+ messages in thread
From: Huang Rui @ 2018-09-08  9:47 UTC (permalink / raw)
  To: shaoyunl; +Cc: Alex Deucher, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On Fri, Sep 07, 2018 at 04:09:52PM -0400, shaoyunl wrote:
> From: Alex Deucher <alexander.deucher@amd.com>
> 
> On hives with xgmi enabled, the fb_location aperture is a size
> which defines the total framebuffer size of all nodes in the
> hive.  Each GPU in the hive has the same view via the fb_location
> aperture.  GPU0 starts at offset (0 * segment size),
> GPU1 starts at offset (1 * segment size), etc.
> 
> For access to local vram on each GPU, we need to take this offset into
> account. This including on setting up GPUVM page table and GART table
> 
> Change-Id: I9efd510bed68fdb9afdfbdc76e1046792471ee78
> Acked-by: Huang Rui <ray.huang@amd.com>
> Acked-by: Slava Abramov <slava.abramov@amd.com>
> Signed-off-by: Shaoyun Liu <Shaoyun.Liu@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>

The same minor comment, and feel free to add

Acked-by: Huang Rui <ray.huang@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c  | 20 ++++++++++----------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h  |  7 +++++++
>  drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c |  3 +++
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c    |  6 ++++++
>  drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c  |  7 +++++++
>  5 files changed, 33 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 6acdeeb..cf97c1c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -147,8 +147,8 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>  	/* 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;
> -	size_af = adev->gmc.mc_mask + 1 - ALIGN(mc->vram_end + 1, four_gb);
> +	size_bf = mc->fb_start;
> +	size_af = adev->gmc.mc_mask + 1 - ALIGN(mc->fb_end + 1, four_gb);
>  
>  	if (mc->gart_size > max(size_bf, size_af)) {
>  		dev_warn(adev->dev, "limiting GART\n");
> @@ -184,23 +184,23 @@ void amdgpu_gmc_agp_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>  	const uint64_t sixteen_gb_mask = ~(sixteen_gb - 1);
>  	u64 size_af, size_bf;
>  
> -	if (mc->vram_start > mc->gart_start) {
> -		size_bf = (mc->vram_start & sixteen_gb_mask) -
> +	if (mc->fb_start > mc->gart_start) {
> +		size_bf = (mc->fb_start & sixteen_gb_mask) -
>  			ALIGN(mc->gart_end + 1, sixteen_gb);
> -		size_af = mc->mc_mask + 1 - ALIGN(mc->vram_end + 1, sixteen_gb);
> +		size_af = mc->mc_mask + 1 - ALIGN(mc->fb_end + 1, sixteen_gb);
>  	} else {
> -		size_bf = mc->vram_start & sixteen_gb_mask;
> +		size_bf = mc->fb_start & sixteen_gb_mask;
>  		size_af = (mc->gart_start & sixteen_gb_mask) -
> -			ALIGN(mc->vram_end + 1, sixteen_gb);
> +			ALIGN(mc->fb_end + 1, sixteen_gb);
>  	}
>  
>  	if (size_bf > size_af) {
> -		mc->agp_start = mc->vram_start > mc->gart_start ?
> +		mc->agp_start = mc->fb_start > mc->gart_start ?
>  			mc->gart_end + 1 : 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_start = (mc->fb_start > mc->gart_start ?
> +			mc->fb_end : mc->gart_end) + 1,
>  		mc->agp_size = size_af;
>  	}
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index a929a55..df96dfe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -114,6 +114,13 @@ struct amdgpu_gmc {
>  	u64			gart_end;
>  	u64			vram_start;
>  	u64			vram_end;
> +	/* FB region , it's same as local vram region in single GPU, in XGMI
> +	 * configuration, this region covers all GPUs in the same hive ,
> +	 * each GPU in the hive has the same view of this FB region .
> +	 * GPU0's vram starts at offset (0 * segment size) ,
> +	 * GPU1 starts at offset (1 * segment size), etc.   */

Block comments use a trailing */ on a separate line.

Thanks,
Ray

> +	u64			fb_start;
> +	u64			fb_end;
>  	unsigned		vram_width;
>  	u64			real_vram_size;
>  	int			vram_mtrr;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
> index d4170cb..5e9ab8e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
> @@ -44,6 +44,9 @@ int gfxhub_v1_1_get_xgmi_info(struct amdgpu_device *adev)
>  			REG_GET_FIELD(xgmi_lfb_cntl, MC_VM_XGMI_LFB_CNTL, PF_LFB_REGION);
>  		if (adev->gmc.xgmi.physical_node_id > 3)
>  			return -EINVAL;
> +		adev->gmc.xgmi.node_segment_size = REG_GET_FIELD(
> +			RREG32_SOC15(GC, 0, mmMC_VM_XGMI_LFB_SIZE),
> +			MC_VM_XGMI_LFB_SIZE, PF_LFB_SIZE) << 24;
>  	}
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index e9b5a13..b1c8489 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -771,12 +771,18 @@ 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);
> +	/* add the xgmi offset of the physical node */
> +	base += adev->gmc.xgmi.physical_node_id * adev->gmc.xgmi.node_segment_size;
>  	amdgpu_gmc_vram_location(adev, &adev->gmc, base);
>  	amdgpu_gmc_gart_location(adev, mc);
>  	if (!amdgpu_sriov_vf(adev))
>  		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);
> +
> +	/* XXX: add the xgmi offset of the physical node? */
> +	adev->vm_manager.vram_base_offset +=
> +		adev->gmc.xgmi.physical_node_id * adev->gmc.xgmi.node_segment_size;
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> index 73d7c07..0e09549 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> @@ -38,10 +38,17 @@
>  u64 mmhub_v1_0_get_fb_location(struct amdgpu_device *adev)
>  {
>  	u64 base = RREG32_SOC15(MMHUB, 0, mmMC_VM_FB_LOCATION_BASE);
> +	u64 top = RREG32_SOC15(MMHUB, 0, mmMC_VM_FB_LOCATION_TOP);
>  
>  	base &= MC_VM_FB_LOCATION_BASE__FB_BASE_MASK;
>  	base <<= 24;
>  
> +	top &= MC_VM_FB_LOCATION_TOP__FB_TOP_MASK;
> +	top <<= 24;
> +
> +	adev->gmc.fb_start = base;
> +	adev->gmc.fb_end = top;
> +
>  	return base;
>  }
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH 04/12] drm/amdgpu/gmc9: Adjust GART and AGP location with xgmi offset
       [not found] ` <1536350992-31224-1-git-send-email-Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>
  2018-09-08  9:47   ` Huang Rui
@ 2018-09-08 18:06   ` Christian König
       [not found]     ` <d68e0f2f-4734-b386-359e-039de7591ef2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  1 sibling, 1 reply; 4+ messages in thread
From: Christian König @ 2018-09-08 18:06 UTC (permalink / raw)
  To: shaoyunl, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Alex Deucher

Am 07.09.2018 um 22:09 schrieb shaoyunl:
> From: Alex Deucher <alexander.deucher@amd.com>
>
> On hives with xgmi enabled, the fb_location aperture is a size
> which defines the total framebuffer size of all nodes in the
> hive.  Each GPU in the hive has the same view via the fb_location
> aperture.  GPU0 starts at offset (0 * segment size),
> GPU1 starts at offset (1 * segment size), etc.
>
> For access to local vram on each GPU, we need to take this offset into
> account. This including on setting up GPUVM page table and GART table

Please make the change to add fb_start/fb_end a separate patch.

And also initialize fb_start/fb_end with vram_start/vram_end in 
amdgpu_gmc_vram_location for compatibility with older chips.

Apart from that it looks good to me.

Regards,
Christian.

>
> Change-Id: I9efd510bed68fdb9afdfbdc76e1046792471ee78
> Acked-by: Huang Rui <ray.huang@amd.com>
> Acked-by: Slava Abramov <slava.abramov@amd.com>
> Signed-off-by: Shaoyun Liu <Shaoyun.Liu@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c  | 20 ++++++++++----------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h  |  7 +++++++
>   drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c |  3 +++
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c    |  6 ++++++
>   drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c  |  7 +++++++
>   5 files changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 6acdeeb..cf97c1c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -147,8 +147,8 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>   	/* 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;
> -	size_af = adev->gmc.mc_mask + 1 - ALIGN(mc->vram_end + 1, four_gb);
> +	size_bf = mc->fb_start;
> +	size_af = adev->gmc.mc_mask + 1 - ALIGN(mc->fb_end + 1, four_gb);
>   
>   	if (mc->gart_size > max(size_bf, size_af)) {
>   		dev_warn(adev->dev, "limiting GART\n");
> @@ -184,23 +184,23 @@ void amdgpu_gmc_agp_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>   	const uint64_t sixteen_gb_mask = ~(sixteen_gb - 1);
>   	u64 size_af, size_bf;
>   
> -	if (mc->vram_start > mc->gart_start) {
> -		size_bf = (mc->vram_start & sixteen_gb_mask) -
> +	if (mc->fb_start > mc->gart_start) {
> +		size_bf = (mc->fb_start & sixteen_gb_mask) -
>   			ALIGN(mc->gart_end + 1, sixteen_gb);
> -		size_af = mc->mc_mask + 1 - ALIGN(mc->vram_end + 1, sixteen_gb);
> +		size_af = mc->mc_mask + 1 - ALIGN(mc->fb_end + 1, sixteen_gb);
>   	} else {
> -		size_bf = mc->vram_start & sixteen_gb_mask;
> +		size_bf = mc->fb_start & sixteen_gb_mask;
>   		size_af = (mc->gart_start & sixteen_gb_mask) -
> -			ALIGN(mc->vram_end + 1, sixteen_gb);
> +			ALIGN(mc->fb_end + 1, sixteen_gb);
>   	}
>   
>   	if (size_bf > size_af) {
> -		mc->agp_start = mc->vram_start > mc->gart_start ?
> +		mc->agp_start = mc->fb_start > mc->gart_start ?
>   			mc->gart_end + 1 : 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_start = (mc->fb_start > mc->gart_start ?
> +			mc->fb_end : mc->gart_end) + 1,
>   		mc->agp_size = size_af;
>   	}
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index a929a55..df96dfe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -114,6 +114,13 @@ struct amdgpu_gmc {
>   	u64			gart_end;
>   	u64			vram_start;
>   	u64			vram_end;
> +	/* FB region , it's same as local vram region in single GPU, in XGMI
> +	 * configuration, this region covers all GPUs in the same hive ,
> +	 * each GPU in the hive has the same view of this FB region .
> +	 * GPU0's vram starts at offset (0 * segment size) ,
> +	 * GPU1 starts at offset (1 * segment size), etc.   */
> +	u64			fb_start;
> +	u64			fb_end;
>   	unsigned		vram_width;
>   	u64			real_vram_size;
>   	int			vram_mtrr;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
> index d4170cb..5e9ab8e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
> @@ -44,6 +44,9 @@ int gfxhub_v1_1_get_xgmi_info(struct amdgpu_device *adev)
>   			REG_GET_FIELD(xgmi_lfb_cntl, MC_VM_XGMI_LFB_CNTL, PF_LFB_REGION);
>   		if (adev->gmc.xgmi.physical_node_id > 3)
>   			return -EINVAL;
> +		adev->gmc.xgmi.node_segment_size = REG_GET_FIELD(
> +			RREG32_SOC15(GC, 0, mmMC_VM_XGMI_LFB_SIZE),
> +			MC_VM_XGMI_LFB_SIZE, PF_LFB_SIZE) << 24;
>   	}
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index e9b5a13..b1c8489 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -771,12 +771,18 @@ 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);
> +	/* add the xgmi offset of the physical node */
> +	base += adev->gmc.xgmi.physical_node_id * adev->gmc.xgmi.node_segment_size;
>   	amdgpu_gmc_vram_location(adev, &adev->gmc, base);
>   	amdgpu_gmc_gart_location(adev, mc);
>   	if (!amdgpu_sriov_vf(adev))
>   		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);
> +
> +	/* XXX: add the xgmi offset of the physical node? */
> +	adev->vm_manager.vram_base_offset +=
> +		adev->gmc.xgmi.physical_node_id * adev->gmc.xgmi.node_segment_size;
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> index 73d7c07..0e09549 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> @@ -38,10 +38,17 @@
>   u64 mmhub_v1_0_get_fb_location(struct amdgpu_device *adev)
>   {
>   	u64 base = RREG32_SOC15(MMHUB, 0, mmMC_VM_FB_LOCATION_BASE);
> +	u64 top = RREG32_SOC15(MMHUB, 0, mmMC_VM_FB_LOCATION_TOP);
>   
>   	base &= MC_VM_FB_LOCATION_BASE__FB_BASE_MASK;
>   	base <<= 24;
>   
> +	top &= MC_VM_FB_LOCATION_TOP__FB_TOP_MASK;
> +	top <<= 24;
> +
> +	adev->gmc.fb_start = base;
> +	adev->gmc.fb_end = top;
> +
>   	return base;
>   }
>   

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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* RE: [PATCH 04/12] drm/amdgpu/gmc9: Adjust GART and AGP location with xgmi offset
       [not found]     ` <d68e0f2f-4734-b386-359e-039de7591ef2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-09-10 15:55       ` Liu, Shaoyun
  0 siblings, 0 replies; 4+ messages in thread
From: Liu, Shaoyun @ 2018-09-10 15:55 UTC (permalink / raw)
  To: Koenig, Christian, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Deucher, Alexander

Sorry , miss this comments and already submitted the  serial of the patch . 
It's a good catch .   I will  send out another review to  initialize fb_start/fb_end with vram_start/vram_end for older chips

Regards
Shaoyun.liu

-----Original Message-----
From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com] 
Sent: Saturday, September 08, 2018 2:06 PM
To: Liu, Shaoyun <Shaoyun.Liu@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: Re: [PATCH 04/12] drm/amdgpu/gmc9: Adjust GART and AGP location with xgmi offset

Am 07.09.2018 um 22:09 schrieb shaoyunl:
> From: Alex Deucher <alexander.deucher@amd.com>
>
> On hives with xgmi enabled, the fb_location aperture is a size which 
> defines the total framebuffer size of all nodes in the hive.  Each GPU 
> in the hive has the same view via the fb_location aperture.  GPU0 
> starts at offset (0 * segment size),
> GPU1 starts at offset (1 * segment size), etc.
>
> For access to local vram on each GPU, we need to take this offset into 
> account. This including on setting up GPUVM page table and GART table

Please make the change to add fb_start/fb_end a separate patch.

And also initialize fb_start/fb_end with vram_start/vram_end in amdgpu_gmc_vram_location for compatibility with older chips.

Apart from that it looks good to me.

Regards,
Christian.

>
> Change-Id: I9efd510bed68fdb9afdfbdc76e1046792471ee78
> Acked-by: Huang Rui <ray.huang@amd.com>
> Acked-by: Slava Abramov <slava.abramov@amd.com>
> Signed-off-by: Shaoyun Liu <Shaoyun.Liu@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c  | 20 ++++++++++----------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h  |  7 +++++++
>   drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c |  3 +++
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c    |  6 ++++++
>   drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c  |  7 +++++++
>   5 files changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 6acdeeb..cf97c1c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -147,8 +147,8 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>   	/* 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;
> -	size_af = adev->gmc.mc_mask + 1 - ALIGN(mc->vram_end + 1, four_gb);
> +	size_bf = mc->fb_start;
> +	size_af = adev->gmc.mc_mask + 1 - ALIGN(mc->fb_end + 1, four_gb);
>   
>   	if (mc->gart_size > max(size_bf, size_af)) {
>   		dev_warn(adev->dev, "limiting GART\n"); @@ -184,23 +184,23 @@ void 
> amdgpu_gmc_agp_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>   	const uint64_t sixteen_gb_mask = ~(sixteen_gb - 1);
>   	u64 size_af, size_bf;
>   
> -	if (mc->vram_start > mc->gart_start) {
> -		size_bf = (mc->vram_start & sixteen_gb_mask) -
> +	if (mc->fb_start > mc->gart_start) {
> +		size_bf = (mc->fb_start & sixteen_gb_mask) -
>   			ALIGN(mc->gart_end + 1, sixteen_gb);
> -		size_af = mc->mc_mask + 1 - ALIGN(mc->vram_end + 1, sixteen_gb);
> +		size_af = mc->mc_mask + 1 - ALIGN(mc->fb_end + 1, sixteen_gb);
>   	} else {
> -		size_bf = mc->vram_start & sixteen_gb_mask;
> +		size_bf = mc->fb_start & sixteen_gb_mask;
>   		size_af = (mc->gart_start & sixteen_gb_mask) -
> -			ALIGN(mc->vram_end + 1, sixteen_gb);
> +			ALIGN(mc->fb_end + 1, sixteen_gb);
>   	}
>   
>   	if (size_bf > size_af) {
> -		mc->agp_start = mc->vram_start > mc->gart_start ?
> +		mc->agp_start = mc->fb_start > mc->gart_start ?
>   			mc->gart_end + 1 : 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_start = (mc->fb_start > mc->gart_start ?
> +			mc->fb_end : mc->gart_end) + 1,
>   		mc->agp_size = size_af;
>   	}
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index a929a55..df96dfe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -114,6 +114,13 @@ struct amdgpu_gmc {
>   	u64			gart_end;
>   	u64			vram_start;
>   	u64			vram_end;
> +	/* FB region , it's same as local vram region in single GPU, in XGMI
> +	 * configuration, this region covers all GPUs in the same hive ,
> +	 * each GPU in the hive has the same view of this FB region .
> +	 * GPU0's vram starts at offset (0 * segment size) ,
> +	 * GPU1 starts at offset (1 * segment size), etc.   */
> +	u64			fb_start;
> +	u64			fb_end;
>   	unsigned		vram_width;
>   	u64			real_vram_size;
>   	int			vram_mtrr;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c 
> b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
> index d4170cb..5e9ab8e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.c
> @@ -44,6 +44,9 @@ int gfxhub_v1_1_get_xgmi_info(struct amdgpu_device *adev)
>   			REG_GET_FIELD(xgmi_lfb_cntl, MC_VM_XGMI_LFB_CNTL, PF_LFB_REGION);
>   		if (adev->gmc.xgmi.physical_node_id > 3)
>   			return -EINVAL;
> +		adev->gmc.xgmi.node_segment_size = REG_GET_FIELD(
> +			RREG32_SOC15(GC, 0, mmMC_VM_XGMI_LFB_SIZE),
> +			MC_VM_XGMI_LFB_SIZE, PF_LFB_SIZE) << 24;
>   	}
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index e9b5a13..b1c8489 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -771,12 +771,18 @@ 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);
> +	/* add the xgmi offset of the physical node */
> +	base += adev->gmc.xgmi.physical_node_id * 
> +adev->gmc.xgmi.node_segment_size;
>   	amdgpu_gmc_vram_location(adev, &adev->gmc, base);
>   	amdgpu_gmc_gart_location(adev, mc);
>   	if (!amdgpu_sriov_vf(adev))
>   		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);
> +
> +	/* XXX: add the xgmi offset of the physical node? */
> +	adev->vm_manager.vram_base_offset +=
> +		adev->gmc.xgmi.physical_node_id * adev->gmc.xgmi.node_segment_size;
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c 
> b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> index 73d7c07..0e09549 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> @@ -38,10 +38,17 @@
>   u64 mmhub_v1_0_get_fb_location(struct amdgpu_device *adev)
>   {
>   	u64 base = RREG32_SOC15(MMHUB, 0, mmMC_VM_FB_LOCATION_BASE);
> +	u64 top = RREG32_SOC15(MMHUB, 0, mmMC_VM_FB_LOCATION_TOP);
>   
>   	base &= MC_VM_FB_LOCATION_BASE__FB_BASE_MASK;
>   	base <<= 24;
>   
> +	top &= MC_VM_FB_LOCATION_TOP__FB_TOP_MASK;
> +	top <<= 24;
> +
> +	adev->gmc.fb_start = base;
> +	adev->gmc.fb_end = top;
> +
>   	return base;
>   }
>   

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

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-09-10 15:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-07 20:09 [PATCH 04/12] drm/amdgpu/gmc9: Adjust GART and AGP location with xgmi offset shaoyunl
     [not found] ` <1536350992-31224-1-git-send-email-Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>
2018-09-08  9:47   ` Huang Rui
2018-09-08 18:06   ` Christian König
     [not found]     ` <d68e0f2f-4734-b386-359e-039de7591ef2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-09-10 15:55       ` Liu, Shaoyun

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.