[PATCH] drm/radeon: adjust default radeon_vm_block_size

[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?

> 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.

> 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))");
>>>
>>
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 246 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140719/30d5e92a/attachment-0001.sig>