On 03/29/2017 09:48 AM, Felix Kuehling wrote:
> 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.
Sorry for the typo, I'd like to say 512GB.
Maybe it will become 256TB, depending on the discussion in the future.
Jerry
>
> 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
>>>
>
