Yeah, I thought so. In this case we don't need this patch or is there anything I'm still missing? The use of min/max here is exactly to avoid having a SRIOV dependency here. Regards, Christian. Am 19.08.19 um 09:17 schrieb Min, Frank: > Hi Christian, > Thanks for your review. > For SRIOV, amdgpu_gmc_agp_location() would not be called, since it do not use AGP. Also there is no need to use the min and max to judge which range is correct for using. > > Best Regards, > Frank > > -----邮件原件----- > 发件人: Christian König <ckoenig.leichtzumerken@xxxxxxxxx> > 发送时间: 2019年8月19日 15:07 > 收件人: Min, Frank <Frank.Min@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx > 主题: Re: [PATCH 3/3] amd/amdgpu: seperate sriov fb aperture setting > > Am 16.08.19 um 10:59 schrieb Frank.Min: >> sriov would not use agp, so seperate the fb aperture setting. > That won't work correctly. This way we don't program the AGP space into the hardware any more, but would still try to use it. > > We rather need to adjust the amdgpu_gmc_agp_location() function or it's caller to not assign an AGP space in the first place. > > Christian. > >> Change-Id: I1372cd355326731a31361bff13d79e12121b8651 >> Signed-off-by: Frank.Min <Frank.Min@xxxxxxx> >> --- >> drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 39 ++++++++++++++++++++------------ >> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 12 +++++----- >> drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c | 27 +++++++++++++++------- >> 3 files changed, 49 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c >> b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c >> index 6ce37ce..ec78c8b 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c >> @@ -75,23 +75,32 @@ static void gfxhub_v1_0_init_system_aperture_regs(struct amdgpu_device *adev) >> WREG32_SOC15_RLC(GC, 0, mmMC_VM_AGP_BOT, adev->gmc.agp_start >> 24); >> WREG32_SOC15_RLC(GC, 0, mmMC_VM_AGP_TOP, adev->gmc.agp_end >> 24); >> >> - /* Program the system aperture low logical page number. */ >> - WREG32_SOC15_RLC(GC, 0, mmMC_VM_SYSTEM_APERTURE_LOW_ADDR, >> - min(adev->gmc.fb_start, adev->gmc.agp_start) >> 18); >> + if (amdgpu_sriov_vf(adev)) { >> + /* Program the system aperture low logical page number. */ >> + WREG32_SOC15_RLC(GC, 0, mmMC_VM_SYSTEM_APERTURE_LOW_ADDR, >> + adev->gmc.fb_start >> 18); >> >> - if (adev->asic_type == CHIP_RAVEN && adev->rev_id >= 0x8) >> - /* >> - * Raven2 has a HW issue that it is unable to use the vram which >> - * is out of MC_VM_SYSTEM_APERTURE_HIGH_ADDR. So here is the >> - * workaround that increase system aperture high address (add 1) >> - * to get rid of the VM fault and hardware hang. >> - */ >> WREG32_SOC15_RLC(GC, 0, mmMC_VM_SYSTEM_APERTURE_HIGH_ADDR, >> - max((adev->gmc.fb_end >> 18) + 0x1, >> - adev->gmc.agp_end >> 18)); >> - else >> - WREG32_SOC15_RLC(GC, 0, mmMC_VM_SYSTEM_APERTURE_HIGH_ADDR, >> - max(adev->gmc.fb_end, adev->gmc.agp_end) >> 18); >> + adev->gmc.fb_end >> 18); >> + } else { >> + /* Program the system aperture low logical page number. */ >> + WREG32_SOC15_RLC(GC, 0, mmMC_VM_SYSTEM_APERTURE_LOW_ADDR, >> + min(adev->gmc.fb_start, adev->gmc.agp_start) >> 18); >> + >> + if (adev->asic_type == CHIP_RAVEN && adev->rev_id >= 0x8) >> + /* >> + * Raven2 has a HW issue that it is unable to use the vram which >> + * is out of MC_VM_SYSTEM_APERTURE_HIGH_ADDR. So here is the >> + * workaround that increase system aperture high address (add 1) >> + * to get rid of the VM fault and hardware hang. >> + */ >> + WREG32_SOC15_RLC(GC, 0, mmMC_VM_SYSTEM_APERTURE_HIGH_ADDR, >> + max((adev->gmc.fb_end >> 18) + 0x1, >> + adev->gmc.agp_end >> 18)); >> + else >> + WREG32_SOC15_RLC(GC, 0, mmMC_VM_SYSTEM_APERTURE_HIGH_ADDR, >> + max(adev->gmc.fb_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 6de1726..1f8bdfa 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c >> @@ -920,12 +920,12 @@ static void gmc_v9_0_vram_gtt_location(struct amdgpu_device *adev, >> struct amdgpu_gmc *mc) >> { >> u64 base = 0; >> - if (!amdgpu_sriov_vf(adev)) { >> - if (adev->asic_type == CHIP_ARCTURUS) >> - base = mmhub_v9_4_get_fb_location(adev); >> - else >> - base = mmhub_v1_0_get_fb_location(adev); >> - } >> + >> + if (adev->asic_type == CHIP_ARCTURUS) >> + base = mmhub_v9_4_get_fb_location(adev); >> + else >> + 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, mc, base); diff --git >> a/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c >> b/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c >> index 0cf7ef4..ea3359f 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c >> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c >> @@ -118,14 +118,25 @@ static void mmhub_v9_4_init_system_aperture_regs(struct amdgpu_device *adev, >> adev->gmc.agp_start >> 24); >> >> /* Program the system aperture low logical page number. */ >> - WREG32_SOC15_OFFSET(MMHUB, 0, >> - mmVMSHAREDVC0_MC_VM_SYSTEM_APERTURE_LOW_ADDR, >> - hubid * MMHUB_INSTANCE_REGISTER_OFFSET, >> - min(adev->gmc.fb_start, adev->gmc.agp_start) >> 18); >> - WREG32_SOC15_OFFSET(MMHUB, 0, >> - mmVMSHAREDVC0_MC_VM_SYSTEM_APERTURE_HIGH_ADDR, >> - hubid * MMHUB_INSTANCE_REGISTER_OFFSET, >> - max(adev->gmc.fb_end, adev->gmc.agp_end) >> 18); >> + if (amdgpu_sriov_vf(adev)) { >> + WREG32_SOC15_OFFSET(MMHUB, 0, >> + mmVMSHAREDVC0_MC_VM_SYSTEM_APERTURE_LOW_ADDR, >> + hubid * MMHUB_INSTANCE_REGISTER_OFFSET, >> + adev->gmc.fb_start >> 18); >> + WREG32_SOC15_OFFSET(MMHUB, 0, >> + mmVMSHAREDVC0_MC_VM_SYSTEM_APERTURE_HIGH_ADDR, >> + hubid * MMHUB_INSTANCE_REGISTER_OFFSET, >> + adev->gmc.fb_end >> 18); >> + } else { >> + WREG32_SOC15_OFFSET(MMHUB, 0, >> + mmVMSHAREDVC0_MC_VM_SYSTEM_APERTURE_LOW_ADDR, >> + hubid * MMHUB_INSTANCE_REGISTER_OFFSET, >> + min(adev->gmc.fb_start, adev->gmc.agp_start) >> 18); >> + WREG32_SOC15_OFFSET(MMHUB, 0, >> + mmVMSHAREDVC0_MC_VM_SYSTEM_APERTURE_HIGH_ADDR, >> + hubid * MMHUB_INSTANCE_REGISTER_OFFSET, >> + max(adev->gmc.fb_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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx