[Mesa-dev] [PATCH] winsys/radeon: cleanup virtual memory nonsense

[Christian König]

Am 19.11.2013 14:41, schrieb Marek Olšák:
> On Tue, Nov 19, 2013 at 11:05 AM, Christian König
> <deathsimple at vodafone.de> wrote:
>> From: Christian König <christian.koenig at amd.com>
>>
>> The alignment of a virtual memory area must always be at least 4096 bytes.
>>
>> It only worked because size was aligned to 4096 outside of the function.
>>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> ---
>>   src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 37 +++++++++++++--------------
>>   src/gallium/winsys/radeon/drm/radeon_drm_bo.h |  1 -
>>   2 files changed, 18 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
>> index 19e2715..3019a52 100644
>> --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
>> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
>> @@ -202,15 +202,15 @@ static uint64_t radeon_bomgr_find_va(struct radeon_bomgr *mgr, uint64_t size, ui
>>       struct radeon_bo_va_hole *hole, *n;
>>       uint64_t offset = 0, waste = 0;
>>
>> +    alignment = MAX2(alignment, 4096);
>> +    size = align(size, 4096);
>> +
>>       pipe_mutex_lock(mgr->bo_va_mutex);
>>       /* first look for a hole */
>>       LIST_FOR_EACH_ENTRY_SAFE(hole, n, &mgr->va_holes, list) {
>>           offset = hole->offset;
>> -        waste = 0;
>> -        if (alignment) {
>> -            waste = offset % alignment;
>> -            waste = waste ? alignment - waste : 0;
>> -        }
>> +        waste = offset % alignment;
>> +        waste = waste ? alignment - waste : 0;
>>           offset += waste;
>>           if (offset >= (hole->offset + hole->size)) {
>>               continue;
>> @@ -242,11 +242,8 @@ static uint64_t radeon_bomgr_find_va(struct radeon_bomgr *mgr, uint64_t size, ui
>>       }
>>
>>       offset = mgr->va_offset;
>> -    waste = 0;
>> -    if (alignment) {
>> -        waste = offset % alignment;
>> -        waste = waste ? alignment - waste : 0;
>> -    }
>> +    waste = offset % alignment;
>> +    waste = waste ? alignment - waste : 0;
>>       if (waste) {
>>           n = CALLOC_STRUCT(radeon_bo_va_hole);
>>           n->size = waste;
>> @@ -261,6 +258,8 @@ static uint64_t radeon_bomgr_find_va(struct radeon_bomgr *mgr, uint64_t size, ui
>>
>>   static void radeon_bomgr_force_va(struct radeon_bomgr *mgr, uint64_t va, uint64_t size)
>>   {
>> +    size = align(size, 4096);
>> +
>>       pipe_mutex_lock(mgr->bo_va_mutex);
>>       if (va >= mgr->va_offset) {
>>           if (va > mgr->va_offset) {
>> @@ -303,6 +302,8 @@ static void radeon_bomgr_free_va(struct radeon_bomgr *mgr, uint64_t va, uint64_t
>>   {
>>       struct radeon_bo_va_hole *hole;
>>
>> +    size = align(size, 4096);
>> +
>>       pipe_mutex_lock(mgr->bo_va_mutex);
>>       if ((va + size) == mgr->va_offset) {
>>           mgr->va_offset = va;
>> @@ -385,7 +386,7 @@ static void radeon_bo_destroy(struct pb_buffer *_buf)
>>       drmIoctl(bo->rws->fd, DRM_IOCTL_GEM_CLOSE, &args);
>>
>>       if (mgr->va) {
>> -        radeon_bomgr_free_va(mgr, bo->va, bo->va_size);
>> +        radeon_bomgr_free_va(mgr, bo->va, bo->base.size);
>>       }
>>
>>       pipe_mutex_destroy(bo->map_mutex);
>> @@ -600,8 +601,7 @@ static struct pb_buffer *radeon_bomgr_create_bo(struct pb_manager *_mgr,
>>       if (mgr->va) {
>>           struct drm_radeon_gem_va va;
>>
>> -        bo->va_size = align(size,  4096);
>> -        bo->va = radeon_bomgr_find_va(mgr, bo->va_size, desc->alignment);
>> +        bo->va = radeon_bomgr_find_va(mgr, size, desc->alignment);
>>
>>           va.handle = bo->handle;
>>           va.vm_id = 0;
>> @@ -621,9 +621,9 @@ static struct pb_buffer *radeon_bomgr_create_bo(struct pb_manager *_mgr,
>>               return NULL;
>>           }
>>           if (va.operation == RADEON_VA_RESULT_VA_EXIST) {
>> -            radeon_bomgr_free_va(mgr, bo->va, bo->va_size);
>> +            radeon_bomgr_free_va(mgr, bo->va, size);
>>               bo->va = va.offset;
>> -            radeon_bomgr_force_va(mgr, bo->va, bo->va_size);
>> +            radeon_bomgr_force_va(mgr, bo->va, size);
>>           }
>>       }
>>
>> @@ -931,8 +931,7 @@ done:
>>       if (mgr->va && !bo->va) {
>>           struct drm_radeon_gem_va va;
>>
>> -        bo->va_size = ((bo->base.size + 4095) & ~4095);
>> -        bo->va = radeon_bomgr_find_va(mgr, bo->va_size, 1 << 20);
>> +        bo->va = radeon_bomgr_find_va(mgr, bo->base.size, 1 << 20);
> Why is the alignment 1<<20 (2M) here? Other than that:

I guess that the alignment isn't available at this point.

IIRC the maximum alignment for our hardware is 1<< (8 + log2(number of 
pipes) + log2(number of banks)) and I think it's going to be a while 
till we break the limit of 64 pipes/banks. So 1<<20 should work fine for 
now.

Christian.

> Reviewed-by: Marek Olšák <marek.olsak at amd.com>
>
> Marek