On 17-03-28 09:39 PM, Zhang, Jerry (Junwei) wrote:
> On 03/29/2017 09:00 AM, Felix Kuehling wrote:
>> adev->family is not initialized yet when amdgpu_get_block_size is
>> called. Use adev->asic_type instead.
>>
>> Minimum VM size is 512GB, not 256GB, for a single page table entry
>> in the root page table.
>>
>> gmc_v9_0_vm_init is called after adev->vm_manager.max_pfn is
>> initialized. Move the minimum VM-size enforcement ahead of max_pfn
>> initializtion. Cast to 64-bit before the left-shift.
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
> Reviewed-by: Junwei Zhang <Jerry.Zhang at amd.com>
>
> Just note:
> For now, it's OK to set the minimum vm size 256G,
> In this case, there is no PD bit anyway.
With VM size of 256GB, the amdgpu_vm_bo_size(adev, 0) calculates 0, and
the page directory allocation with amdgpu_bo_create fails in amdgpu_vm_init.
In other words, amdgpu_vm_num_entries(adev, 0) needs to return at least
1. That means vm_size needs to be at least 512GB.
Regards,
Felix
>
> Christian also mentioned that PD should be 4k size, then 256T was
> required.
> When reach an agreement, we may update them all.
>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 +++---
>> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 17 +++++++----------
>> 2 files changed, 10 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 3500da3..57ccac4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -1042,10 +1042,10 @@ static bool amdgpu_check_pot_argument(int arg)
>> static void amdgpu_get_block_size(struct amdgpu_device *adev)
>> {
>> /* from AI, asic starts to support multiple level VMPT */
>> - if (adev->family >= AMDGPU_FAMILY_AI) {
>> + if (adev->asic_type >= CHIP_VEGA10) {
>> if (amdgpu_vm_block_size != 9)
>> - dev_warn(adev->dev, "Multi-VMPT limits block size to"
>> - "one page!\n");
>> + dev_warn(adev->dev,
>> + "Multi-VMPT limits block size to one page!\n");
>> amdgpu_vm_block_size = 9;
>> return;
>> }
>
> Nice catch.
>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index 1e4734d..df69aae 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -511,12 +511,6 @@ static int gmc_v9_0_vm_init(struct amdgpu_device
>> *adev)
>> * amdkfd will use VMIDs 8-15
>> */
>> adev->vm_manager.num_ids = AMDGPU_NUM_OF_VMIDS;
>> - /* Because of four level VMPTs, vm size at least is 256GB.
>> - 256TB is OK as well */
>> - if (amdgpu_vm_size < 256) {
>> - DRM_WARN("vm size at least is 256GB!\n");
>> - amdgpu_vm_size = 256;
>> - }
>
> David had a patch to fix it yesterday.
> But your patch involves by vm size checking. :)
>
>> adev->vm_manager.num_level = 3;
>> amdgpu_vm_manager_init(adev);
>>
>> @@ -563,11 +557,14 @@ static int gmc_v9_0_sw_init(void *handle)
>> if (r)
>> return r;
>>
>> - /* Adjust VM size here.
>> - * Currently default to 64GB ((16 << 20) 4k pages).
>> - * Max GPUVM size is 48 bits.
>> + /* Because of four level VMPTs, vm size is at least 512GB.
>> + * The maximum size is 256TB (48bit).
>> */
>> - adev->vm_manager.max_pfn = amdgpu_vm_size << 18;
>> + if (amdgpu_vm_size < 512) {
>> + DRM_WARN("VM size is at least 512GB!\n");
>> + amdgpu_vm_size = 512;
>> + }
>> + adev->vm_manager.max_pfn = (uint64_t)amdgpu_vm_size << 18;
>>
>> /* Set the internal MC address mask
>> * This is the max address of the GPU's
>>
