[PATCH] drm/radeon: adjust default radeon_vm_block_size

[Christian König]

Am 19.07.2014 14:37, schrieb Grigori Goronzy:
> On 19.07.2014 13:57, Christian König wrote:
>> Yeah, I'm still playing a bit with this.
>>
>> You need to consider the page directory size as well. If we have 4GB
>> address space (32bits) and 12bits in the page, 12bits in the page tables
>> then there are only 8bits for the page directory, right?
>>
>> Now 8bits for the page directory means we have 256 entries with 8bytes
>> for each entry that makes 2048 bytes for the page directory. But since
>> we allocate 4096 bytes for the page directory anyway we could support
>> 8GB address space as well.
>>
> Yes, that's right... good idea. Larger VM space should also help a bit
> with fragmentation under memory pressure, right?

Only when the application really needs a lot of RAM. Twice as much 
address space than memory should be perfectly sufficient in most cases.

>
>> How about the v2 I've just send out to the list? It also adjusts the
>> vm_size to a default of 8GB and let you get vm_block_sizes according to
>> the following table:
>>
> Looks good, I think.

Does that mean that I have you're rb on the v2 of the patch? A bit of 
testing might be a good idea as well, cause I just hacked together the 
patch in a bit spare time.

If all is well CC Alex as well, so he can replace the original patch 
with the v2 in hist 3.17-wip branch.

Regards,
Christian.

>
>> vm_size         vm_block_size
>> 1GB        =    9
>> 2GB        =    10
>> 4GB        =    11
>> 8GB        =    12
>> 16GB      =    12
>> 32GB      =    13
>> 64GB      =    13
>> 128GB    =    14
>> 256GB    =    14
>> 512GB    =    15
>> 1TB        =    15
>>
>> Regards,
>> Christian.
>>
>> Am 18.07.2014 22:03, schrieb Grigori Goronzy:
>>> On 18.07.2014 11:38, Christian König wrote:
>>>> From: Christian König <christian.koenig at amd.com>
>>>>
>>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>>> ---
>>>>    drivers/gpu/drm/radeon/radeon_device.c | 6 +++++-
>>>>    drivers/gpu/drm/radeon/radeon_drv.c    | 4 ++--
>>>>    2 files changed, 7 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/radeon/radeon_device.c
>>>> b/drivers/gpu/drm/radeon/radeon_device.c
>>>> index 03686fa..a2960db 100644
>>>> --- a/drivers/gpu/drm/radeon/radeon_device.c
>>>> +++ b/drivers/gpu/drm/radeon/radeon_device.c
>>>> @@ -1077,7 +1077,11 @@ static void radeon_check_arguments(struct
>>>> radeon_device *rdev)
>>>>        /* 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 */
>>>> -    if (radeon_vm_block_size < 9) {
>>>> +    if (radeon_vm_block_size == -1) {
>>>> +        radeon_vm_block_size = (ilog2(radeon_vm_size) + 10) / 2;
>>>> +        radeon_vm_block_size = max(radeon_vm_block_size, 9);
>>>> +
>>>> +    } else if (radeon_vm_block_size < 9) {
>>> Hm, that automatic calculation seems strange. Is there any particular
>>> reason why you haven chosen this? This will use vm_block_size=11 for
>>> 4096 MB VM space. I have used vm_block_size=12 with good results. This
>>> manages 16 MB of VM space per page table, which seems reasonable. I also
>>> see a noticeable CPU overhead reduction between 11 and 12.
>>>
>>> Grigori
>>>
>>>>            dev_warn(rdev->dev, "VM page table size (%d) to small\n",
>>>>                 radeon_vm_block_size);
>>>>            radeon_vm_block_size = 9;
>>>> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
>>>> b/drivers/gpu/drm/radeon/radeon_drv.c
>>>> index cb14213..fef5320 100644
>>>> --- a/drivers/gpu/drm/radeon/radeon_drv.c
>>>> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
>>>> @@ -174,7 +174,7 @@ int radeon_aspm = -1;
>>>>    int radeon_runtime_pm = -1;
>>>>    int radeon_hard_reset = 0;
>>>>    int radeon_vm_size = 4096;
>>>> -int radeon_vm_block_size = 9;
>>>> +int radeon_vm_block_size = -1;
>>>>    int radeon_deep_color = 0;
>>>>      MODULE_PARM_DESC(no_wb, "Disable AGP writeback for scratch
>>>> registers");
>>>> @@ -246,7 +246,7 @@ module_param_named(hard_reset, radeon_hard_reset,
>>>> int, 0444);
>>>>    MODULE_PARM_DESC(vm_size, "VM address space size in megabytes
>>>> (default 4GB)");
>>>>    module_param_named(vm_size, radeon_vm_size, int, 0444);
>>>>    -MODULE_PARM_DESC(vm_block_size, "VM page table size in bits
>>>> (default 9)");
>>>> +MODULE_PARM_DESC(vm_block_size, "VM page table size in bits (default
>>>> depending on vm_size)");
>>>>    module_param_named(vm_block_size, radeon_vm_block_size, int, 0444);
>>>>      MODULE_PARM_DESC(deep_color, "Deep Color support (1 = enable, 0 =
>>>> disable (default))");
>>>>
>