> Otherwise we don't get pages larger than 2MB for the L1 on Vega10.
Yeah, I realized that after reviewing the rest of the patch series.
> But another question: Why do you want to clear VRAM on allocation? We
> perfectly support allocating VRAM without clearing it.
As Michel pointed out, that's a security problem because it can leak data
between processes. Therefore all VRAM allocated through KFD APIs is currently
cleared on allocation. However, we have no requirement from our user mode APIs
to provide initialized memory. So if we could skip the initialization it would
be a big performance improvement in applications that do lots of memory
management.
I was thinking, it should probably be easy to modify my drm_mm_mode caching
idea to deal with two node sizes: 2MB and 1GB. But having to deal with
arbitrary power-of-two sizes in between would make things more challenging and
less effective.
Regards,
Felix
________________________________________
From: Christian König <ckoenig.leichtzumer...@gmail.com>
Sent: Tuesday, September 11, 2018 2:49:44 AM
To: Kuehling, Felix; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 01/11] drm/amdgpu: try allocating VRAM as power of two
Yeah well the whole patch set depends on that :)
Otherwise we don't get pages larger than 2MB for the L1 on Vega10.
But another question: Why do you want to clear VRAM on allocation? We
perfectly support allocating VRAM without clearing it.
Regards,
Christian.
Am 11.09.2018 um 02:08 schrieb Felix Kuehling:
> This looks good. But it complicates something I've been looking at:
> Remembering which process drm_mm_nodes last belonged to, so that they
> don't need to be cleared next time they are allocated by the same
> process. Having most nodes the same size (vram_page_split pages) would
> make this very easy and efficient for the most common cases (large
> allocations without any exotic address limitations or alignment
> requirements).
>
> Does anything else in this patch series depend on this optimization?
>
> Regards,
> Felix
>
>
> On 2018-09-09 02:03 PM, Christian König wrote:
>> Try to allocate VRAM in power of two sizes and only fallback to vram
>> split sizes if that fails.
>>
>> Signed-off-by: Christian König <christian.koe...@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 52
>> +++++++++++++++++++++-------
>> 1 file changed, 40 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> index 9cfa8a9ada92..3f9d5d00c9b3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> @@ -124,6 +124,28 @@ u64 amdgpu_vram_mgr_bo_visible_size(struct amdgpu_bo
>> *bo)
>> return usage;
>> }
>>
>> +/**
>> + * amdgpu_vram_mgr_virt_start - update virtual start address
>> + *
>> + * @mem: ttm_mem_reg to update
>> + * @node: just allocated node
>> + *
>> + * Calculate a virtual BO start address to easily check if everything is CPU
>> + * accessible.
>> + */
>> +static void amdgpu_vram_mgr_virt_start(struct ttm_mem_reg *mem,
>> + struct drm_mm_node *node)
>> +{
>> + unsigned long start;
>> +
>> + start = node->start + node->size;
>> + if (start > mem->num_pages)
>> + start -= mem->num_pages;
>> + else
>> + start = 0;
>> + mem->start = max(mem->start, start);
>> +}
>> +
>> /**
>> * amdgpu_vram_mgr_new - allocate new ranges
>> *
>> @@ -176,10 +198,25 @@ static int amdgpu_vram_mgr_new(struct
>> ttm_mem_type_manager *man,
>> pages_left = mem->num_pages;
>>
>> spin_lock(&mgr->lock);
>> - for (i = 0; i < num_nodes; ++i) {
>> + for (i = 0; pages_left >= pages_per_node; ++i) {
>> + unsigned long pages = rounddown_pow_of_two(pages_left);
>> +
>> + r = drm_mm_insert_node_in_range(mm, &nodes[i], pages,
>> + pages_per_node, 0,
>> + place->fpfn, lpfn,
>> + mode);
>> + if (unlikely(r))
>> + break;
>> +
>> + usage += nodes[i].size << PAGE_SHIFT;
>> + vis_usage += amdgpu_vram_mgr_vis_size(adev, &nodes[i]);
>> + amdgpu_vram_mgr_virt_start(mem, &nodes[i]);
>> + pages_left -= pages;
>> + }
>> +
>> + for (; pages_left; ++i) {
>> unsigned long pages = min(pages_left, pages_per_node);
>> uint32_t alignment = mem->page_alignment;
>> - unsigned long start;
>>
>> if (pages == pages_per_node)
>> alignment = pages_per_node;
>> @@ -193,16 +230,7 @@ static int amdgpu_vram_mgr_new(struct
>> ttm_mem_type_manager *man,
>>
>> usage += nodes[i].size << PAGE_SHIFT;
>> vis_usage += amdgpu_vram_mgr_vis_size(adev, &nodes[i]);
>> -
>> - /* Calculate a virtual BO start address to easily check if
>> - * everything is CPU accessible.
>> - */
>> - start = nodes[i].start + nodes[i].size;
>> - if (start > mem->num_pages)
>> - start -= mem->num_pages;
>> - else
>> - start = 0;
>> - mem->start = max(mem->start, start);
>> + amdgpu_vram_mgr_virt_start(mem, &nodes[i]);
>> pages_left -= pages;
>> }
>> spin_unlock(&mgr->lock);
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
