Re: [PATCH 1/2 v2] drm/amdgpu: fix vm size and block size for Vega10 VMPT

["Zhang, Jerry (Junwei)" <Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>]

On 03/31/2017 12:05 PM, Deucher, Alexander wrote:
>> -----Original Message-----
>> From: Junwei Zhang [mailto:Jerry.Zhang@amd.com]
>> Sent: Thursday, March 30, 2017 10:52 PM
>> To: Deucher, Alexander; Koenig, Christian
>> Cc: amd-gfx@lists.freedesktop.org; Zhang, Jerry
>> Subject: [PATCH 1/2 v2] drm/amdgpu: fix vm size and block size for Vega10
>> VMPT
>>
>> From: "Zhang, Jerry" <Jerry.Zhang@amd.com>
>>
>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  8 --------
>>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c      | 15 ++++++++-------
>>  2 files changed, 8 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 363d73c..98555fd 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -1041,14 +1041,6 @@ 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->asic_type >= CHIP_VEGA10) {
>> -             if (amdgpu_vm_block_size != 9)
>> -                     dev_warn(adev->dev,
>> -                              "Multi-VMPT limits block size to one
>> page!\n");
>> -             amdgpu_vm_block_size = 9;
>> -             return;
>> -     }
>>        /* defines number of bits in page table versus page directory,
>>         * a page is 4KB so we have 12 bits offset, minimum 9 bits in the
>>         * page table and the remaining bits are in the page directory */
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index df69aae..081a676 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -546,6 +546,14 @@ static int gmc_v9_0_sw_init(void *handle)
>>        } else {
>>                /* XXX Don't know how to get VRAM type yet. */
>>                adev->mc.vram_type = AMDGPU_VRAM_TYPE_HBM;
>> +
>> +             /*
>> +              * To fulfill 4-level page support,
>> +              * vm size is 256TB (48bit), maximum size of Vega10,
>> +              * block size 512 (9bit)
>> +              */
>> +             amdgpu_vm_size = 1U << 18;
>> +             amdgpu_vm_block_size = 9;
>
> This won't work.  You are still changing the amdgpu_vm_* global variables so if
> you have 2 GPUs in the systems, you'll change them both.
>
>>        }
>>
>>        /* This interrupt is VMC page fault.*/
>> @@ -557,13 +565,6 @@ static int gmc_v9_0_sw_init(void *handle)
>>        if (r)
>>                return r;
>>
>> -     /* Because of four level VMPTs, vm size is at least 512GB.
>> -      * The maximum size is 256TB (48bit).
>> -      */
>> -     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;
>
> Instead of changing amdgpu_vm_size, change adev->vm_manager.max_pfn directly,
> that way you won't be changing the global variable and possibly messing up
> other driver instances.  The alternative, and cleaner in my opinion, would be
> to add new variables to the vm_manager, e.g., adev->vm_manager.size and
> adev->vm_manager.block_size, and set them to something reasonable based on the
> module parameters and them use them in the code rather than the global variables.

Yeah, thanks to point it out.
I prepared another patch, please check it too.

Jerry

>
> Alex
>
>>
>>        /* Set the internal MC address mask
>> --
>> 1.9.1
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx