[Mesa-dev] [PATCH] winsys/radeon: Unmap GPU VM address range when destroying BO

[Christian König]

On 16.06.2015 10:28, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer at amd.com>
>
> But only when doing so is safe according to the
> RADEON_INFO_VA_UNMAP_WORKING kernel query.
>
> This avoids GPU VM address range conflicts when the BO has other
> references than the GEM handle being closed, e.g. when the BO is shared.

That actually doesn't really helps, it just masks the problem for now. 
The other GEM handle could still require the address space just unmapped 
and freed.

What we would need to really clean that up is to make the VM mappings 
per GEM handle like you suggested or allow multiple mappings per BO like 
we did it for Amdgpu.

Another alternative is to teach radeon_bomgr_free_va that a certain 
address range could be allocated to more than one handle.

Regards,
Christian.

>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90537
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90873
>
> Cc: "10.5 10.6" <mesa-stable at lists.freedesktop.org>
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
> ---
>   src/gallium/winsys/radeon/drm/radeon_drm_bo.c     | 29 +++++++++++++++++++----
>   src/gallium/winsys/radeon/drm/radeon_drm_winsys.c |  4 ++++
>   src/gallium/winsys/radeon/drm/radeon_drm_winsys.h |  1 +
>   3 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> index fe98870..a2532cf 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> @@ -305,14 +305,35 @@ static void radeon_bo_destroy(struct pb_buffer *_buf)
>       if (bo->ptr)
>           os_munmap(bo->ptr, bo->base.size);
>   
> +    if (mgr->va) {
> +        int r = 0;
> +
> +        if (bo->rws->va_unmap_working) {
> +            struct drm_radeon_gem_va va;
> +
> +            va.handle = bo->handle;
> +            va.vm_id = 0;
> +            va.operation = RADEON_VA_UNMAP;
> +            va.flags = RADEON_VM_PAGE_READABLE |
> +                       RADEON_VM_PAGE_WRITEABLE |
> +                       RADEON_VM_PAGE_SNOOPED;
> +            va.offset = bo->va;
> +            r = drmCommandWriteRead(bo->rws->fd, DRM_RADEON_GEM_VA, &va, sizeof(va));
> +            if (r && va.operation == RADEON_VA_RESULT_ERROR) {
> +                fprintf(stderr, "radeon: Failed to deallocate virtual address for buffer:\n");
> +                fprintf(stderr, "radeon:    size      : %d bytes\n", bo->base.size);
> +                fprintf(stderr, "radeon:    va        : 0x%016llx\n", (unsigned long long)bo->va);
> +            }
> +	}
> +
> +        if (r == 0)
> +	    radeon_bomgr_free_va(mgr, bo->va, bo->base.size);
> +    }
> +
>       /* Close object. */
>       args.handle = bo->handle;
>       drmIoctl(bo->rws->fd, DRM_IOCTL_GEM_CLOSE, &args);
>   
> -    if (mgr->va) {
> -        radeon_bomgr_free_va(mgr, bo->va, bo->base.size);
> -    }
> -
>       pipe_mutex_destroy(bo->map_mutex);
>   
>       if (bo->initial_domain & RADEON_DOMAIN_VRAM)
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> index ba8d143..d457f8a 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> @@ -57,6 +57,8 @@
>   #define RADEON_INFO_READ_REG		0x24
>   #endif
>   
> +#define RADEON_INFO_VA_UNMAP_WORKING	0x25
> +
>   static struct util_hash_table *fd_tab = NULL;
>   pipe_static_mutex(fd_tab_mutex);
>   
> @@ -399,6 +401,8 @@ static boolean do_winsys_init(struct radeon_drm_winsys *ws)
>               if (!radeon_get_drm_value(ws->fd, RADEON_INFO_IB_VM_MAX_SIZE, NULL,
>                                         &ib_vm_max_size))
>                   ws->info.r600_virtual_address = FALSE;
> +            radeon_get_drm_value(ws->fd, RADEON_INFO_VA_UNMAP_WORKING, NULL,
> +                                 &ws->va_unmap_working);
>           }
>   	if (ws->gen == DRV_R600 && !debug_get_bool_option("RADEON_VA", FALSE))
>   		ws->info.r600_virtual_address = FALSE;
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
> index 166b6b9..99c8b8a 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
> @@ -74,6 +74,7 @@ struct radeon_drm_winsys {
>       enum radeon_generation gen;
>       struct radeon_info info;
>       uint32_t va_start;
> +    uint32_t va_unmap_working;
>       uint32_t accel_working2;
>   
>       struct pb_manager *kman;