On Fri, Sep 07, 2018 at 04:09:52PM -0400, shaoyunl wrote:
> From: Alex Deucher <[email protected]>
> 
> 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 <[email protected]>
> Acked-by: Slava Abramov <[email protected]>
> Signed-off-by: Shaoyun Liu <[email protected]>
> Signed-off-by: Alex Deucher <[email protected]>
> Reviewed-by: Felix Kuehling <[email protected]>

The same minor comment, and feel free to add

Acked-by: Huang Rui <[email protected]>

> ---
>  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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to