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 <[email protected]>
> 发送时间: 2019年8月19日 15:07
> 收件人: Min, Frank <[email protected]>; [email protected]
> 主题: 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 <[email protected]>
>> ---
>>    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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to