[Mesa-dev] [PATCH 1/2] winsys/radeon: fix VA allocation

[Jerome Glisse]

On Fri, Aug 3, 2012 at 11:06 AM, Christian König
<deathsimple at vodafone.de> wrote:
> Wait for VA use to end before reusing it.
>
> Should fix: https://bugs.freedesktop.org/show_bug.cgi?id=45018
>
> Signed-off-by: Christian König <deathsimple at vodafone.de>
> ---
>  src/gallium/winsys/radeon/drm/radeon_drm_bo.c |   64 +++++++++++++++++--------
>  1 file changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> index 2626586..0c94461 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> @@ -102,6 +102,7 @@ static INLINE struct radeon_bo *radeon_bo(struct pb_buffer *bo)
>
>  struct radeon_bo_va_hole {
>      struct list_head list;
> +    uint32_t         handle;
>      uint64_t         offset;
>      uint64_t         size;
>  };
> @@ -204,7 +205,30 @@ static uint64_t radeon_bomgr_find_va(struct radeon_bomgr *mgr, uint64_t size, ui
>      pipe_mutex_lock(mgr->bo_va_mutex);
>      /* first look for a hole */
>      LIST_FOR_EACH_ENTRY_SAFE(hole, n, &mgr->va_holes, list) {
> +        if (hole->handle) {
> +            struct drm_radeon_gem_busy busy_args;
> +            struct drm_gem_close close_args;
> +
> +            memset(&busy_args, 0, sizeof(busy_args));
> +            busy_args.handle = hole->handle;
> +            if (drmCommandWriteRead(mgr->rws->fd, DRM_RADEON_GEM_BUSY,
> +                                    &busy_args, sizeof(busy_args)) != 0) {
> +                continue;
> +            }
> +
> +            memset(&close_args, 0, sizeof(close_args));
> +            close_args.handle = hole->handle;
> +            drmIoctl(mgr->rws->fd, DRM_IOCTL_GEM_CLOSE, &close_args);
> +
> +            hole->handle = 0;
> +        }
>          offset = hole->offset;
> +       if ((offset + hole->size) == mgr->va_offset) {
> +            mgr->va_offset = offset;
> +            list_del(&hole->list);
> +            FREE(hole);
> +            continue;
> +       }
>          waste = 0;
>          if (alignment) {
>              waste = offset % alignment;
> @@ -280,23 +304,21 @@ static void radeon_bomgr_force_va(struct radeon_bomgr *mgr, uint64_t va, uint64_
>      pipe_mutex_unlock(mgr->bo_va_mutex);
>  }
>
> -static void radeon_bomgr_free_va(struct radeon_bomgr *mgr, uint64_t va, uint64_t size)
> +static void radeon_bomgr_free_va(struct radeon_bomgr *mgr, uint64_t va,
> +                                 uint64_t size, uint32_t handle)
>  {
> +    struct radeon_bo_va_hole *hole;
>      pipe_mutex_lock(mgr->bo_va_mutex);
> -    if ((va + size) == mgr->va_offset) {
> -        mgr->va_offset = va;
> -    } else {
> -        struct radeon_bo_va_hole *hole;
>
> -        /* FIXME on allocation failure we just lose virtual address space
> -         * maybe print a warning
> -         */
> -        hole = CALLOC_STRUCT(radeon_bo_va_hole);
> -        if (hole) {
> -            hole->size = size;
> -            hole->offset = va;
> -            list_add(&hole->list, &mgr->va_holes);
> -        }
> +    /* FIXME on allocation failure we just lose virtual address space
> +     * maybe print a warning
> +     */
> +    hole = CALLOC_STRUCT(radeon_bo_va_hole);
> +    if (hole) {
> +        hole->handle = handle;
> +        hole->size = size;
> +        hole->offset = va;
> +        list_add(&hole->list, &mgr->va_holes);
>      }
>      pipe_mutex_unlock(mgr->bo_va_mutex);
>  }
> @@ -320,12 +342,12 @@ static void radeon_bo_destroy(struct pb_buffer *_buf)
>          os_munmap(bo->ptr, bo->base.size);
>
>      if (mgr->va) {
> -        radeon_bomgr_free_va(mgr, bo->va, bo->va_size);
> +        radeon_bomgr_free_va(mgr, bo->va, bo->va_size, bo->handle);
> +    } else {
> +        /* Close object. */
> +        args.handle = bo->handle;
> +        drmIoctl(bo->rws->fd, DRM_IOCTL_GEM_CLOSE, &args);
>      }
> -
> -    /* Close object. */
> -    args.handle = bo->handle;
> -    drmIoctl(bo->rws->fd, DRM_IOCTL_GEM_CLOSE, &args);
>      pipe_mutex_destroy(bo->map_mutex);
>      FREE(bo);
>  }
> @@ -540,7 +562,7 @@ 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, bo->va_size, 0);
>              bo->va = va.offset;
>              radeon_bomgr_force_va(mgr, bo->va, bo->va_size);
>          }
> @@ -865,7 +887,7 @@ done:
>              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, bo->va_size, 0);
>              bo->va = va.offset;
>              radeon_bomgr_force_va(mgr, bo->va, bo->va_size);
>          }
> --
> 1.7.9.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


As i said in the bug report this is not needed. As soon as you call in
kernel it should work (given you have my kernel patch).

Cheers,
Jerome