[Mesa-dev] [PATCH 09/10] winsys/radeon: use pb_cache instead of pb_cache_manager

Nicolai Hähnle nhaehnle at gmail.com
Tue Dec 8 13:10:53 PST 2015


On 06.12.2015 19:01, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> This is a prerequisite for the removal of radeon_winsys_cs_handle.
> ---
>   src/gallium/winsys/radeon/drm/radeon_drm_bo.c     | 212 +++++++---------------
>   src/gallium/winsys/radeon/drm/radeon_drm_bo.h     |  14 +-
>   src/gallium/winsys/radeon/drm/radeon_drm_winsys.c |  22 +--
>   src/gallium/winsys/radeon/drm/radeon_drm_winsys.h |   4 +-
>   4 files changed, 74 insertions(+), 178 deletions(-)
>
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> index 4c38379..9532c77 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
...
>   static const struct pb_vtbl radeon_bo_vtbl = {
> -    radeon_bo_destroy,
> -    NULL, /* never called */
> -    NULL, /* never called */
> -    radeon_bo_validate,
> -    radeon_bo_fence,
> -    radeon_bo_get_base_buffer,
> +    radeon_bo_destroy_or_cache
>   };

I take it the other functions aren't called anymore? Perhaps this patch 
and #3 could use an explanation to that effect.

Nicolai

>
>   #ifndef RADEON_GEM_GTT_WC
> @@ -540,40 +490,39 @@ static const struct pb_vtbl radeon_bo_vtbl = {
>   #define RADEON_GEM_NO_CPU_ACCESS	(1 << 4)
>   #endif
>
> -static struct pb_buffer *radeon_bomgr_create_bo(struct pb_manager *_mgr,
> -                                                pb_size size,
> -                                                const struct pb_desc *desc)
> +static struct radeon_bo *radeon_create_bo(struct radeon_drm_winsys *rws,
> +                                          unsigned size, unsigned alignment,
> +                                          unsigned usage,
> +                                          unsigned initial_domains,
> +                                          unsigned flags)
>   {
> -    struct radeon_bomgr *mgr = radeon_bomgr(_mgr);
> -    struct radeon_drm_winsys *rws = mgr->rws;
>       struct radeon_bo *bo;
>       struct drm_radeon_gem_create args;
> -    struct radeon_bo_desc *rdesc = (struct radeon_bo_desc*)desc;
>       int r;
>
>       memset(&args, 0, sizeof(args));
>
> -    assert(rdesc->initial_domains);
> -    assert((rdesc->initial_domains &
> +    assert(initial_domains);
> +    assert((initial_domains &
>               ~(RADEON_GEM_DOMAIN_GTT | RADEON_GEM_DOMAIN_VRAM)) == 0);
>
>       args.size = size;
> -    args.alignment = desc->alignment;
> -    args.initial_domain = rdesc->initial_domains;
> +    args.alignment = alignment;
> +    args.initial_domain = initial_domains;
>       args.flags = 0;
>
> -    if (rdesc->flags & RADEON_FLAG_GTT_WC)
> +    if (flags & RADEON_FLAG_GTT_WC)
>           args.flags |= RADEON_GEM_GTT_WC;
> -    if (rdesc->flags & RADEON_FLAG_CPU_ACCESS)
> +    if (flags & RADEON_FLAG_CPU_ACCESS)
>           args.flags |= RADEON_GEM_CPU_ACCESS;
> -    if (rdesc->flags & RADEON_FLAG_NO_CPU_ACCESS)
> +    if (flags & RADEON_FLAG_NO_CPU_ACCESS)
>           args.flags |= RADEON_GEM_NO_CPU_ACCESS;
>
>       if (drmCommandWriteRead(rws->fd, DRM_RADEON_GEM_CREATE,
>                               &args, sizeof(args))) {
>           fprintf(stderr, "radeon: Failed to allocate a buffer:\n");
>           fprintf(stderr, "radeon:    size      : %d bytes\n", size);
> -        fprintf(stderr, "radeon:    alignment : %d bytes\n", desc->alignment);
> +        fprintf(stderr, "radeon:    alignment : %d bytes\n", alignment);
>           fprintf(stderr, "radeon:    domains   : %d\n", args.initial_domain);
>           fprintf(stderr, "radeon:    flags     : %d\n", args.flags);
>           return NULL;
> @@ -584,20 +533,21 @@ static struct pb_buffer *radeon_bomgr_create_bo(struct pb_manager *_mgr,
>           return NULL;
>
>       pipe_reference_init(&bo->base.reference, 1);
> -    bo->base.alignment = desc->alignment;
> -    bo->base.usage = desc->usage;
> +    bo->base.alignment = alignment;
> +    bo->base.usage = usage;
>       bo->base.size = size;
>       bo->base.vtbl = &radeon_bo_vtbl;
>       bo->rws = rws;
>       bo->handle = args.handle;
>       bo->va = 0;
> -    bo->initial_domain = rdesc->initial_domains;
> +    bo->initial_domain = initial_domains;
>       pipe_mutex_init(bo->map_mutex);
> +    pb_cache_init_entry(&rws->bo_cache, &bo->cache_entry, &bo->base);
>
>       if (rws->info.r600_virtual_address) {
>           struct drm_radeon_gem_va va;
>
> -        bo->va = radeon_bomgr_find_va(rws, size, desc->alignment);
> +        bo->va = radeon_bomgr_find_va(rws, size, alignment);
>
>           va.handle = bo->handle;
>           va.vm_id = 0;
> @@ -610,7 +560,7 @@ static struct pb_buffer *radeon_bomgr_create_bo(struct pb_manager *_mgr,
>           if (r && va.operation == RADEON_VA_RESULT_ERROR) {
>               fprintf(stderr, "radeon: Failed to allocate virtual address for buffer:\n");
>               fprintf(stderr, "radeon:    size      : %d bytes\n", size);
> -            fprintf(stderr, "radeon:    alignment : %d bytes\n", desc->alignment);
> +            fprintf(stderr, "radeon:    alignment : %d bytes\n", alignment);
>               fprintf(stderr, "radeon:    domains   : %d\n", args.initial_domain);
>               fprintf(stderr, "radeon:    va        : 0x%016llx\n", (unsigned long long)bo->va);
>               radeon_bo_destroy(&bo->base);
> @@ -631,56 +581,22 @@ static struct pb_buffer *radeon_bomgr_create_bo(struct pb_manager *_mgr,
>           pipe_mutex_unlock(rws->bo_handles_mutex);
>       }
>
> -    if (rdesc->initial_domains & RADEON_DOMAIN_VRAM)
> +    if (initial_domains & RADEON_DOMAIN_VRAM)
>           rws->allocated_vram += align(size, rws->size_align);
> -    else if (rdesc->initial_domains & RADEON_DOMAIN_GTT)
> +    else if (initial_domains & RADEON_DOMAIN_GTT)
>           rws->allocated_gtt += align(size, rws->size_align);
>
>       return &bo->base;
>   }
>
> -static void radeon_bomgr_flush(struct pb_manager *mgr)
> -{
> -    /* NOP */
> -}
> -
> -/* This is for the cache bufmgr. */
> -static boolean radeon_bomgr_is_buffer_busy(struct pb_manager *_mgr,
> -                                           struct pb_buffer *_buf)
> +bool radeon_bo_can_reclaim(struct pb_buffer *_buf)
>   {
>      struct radeon_bo *bo = radeon_bo(_buf);
>
> -   if (radeon_bo_is_referenced_by_any_cs(bo)) {
> -       return TRUE;
> -   }
> +   if (radeon_bo_is_referenced_by_any_cs(bo))
> +      return false;
>
> -   if (!radeon_bo_wait((struct pb_buffer*)bo, 0, RADEON_USAGE_READWRITE)) {
> -       return TRUE;
> -   }
> -
> -   return FALSE;
> -}
> -
> -static void radeon_bomgr_destroy(struct pb_manager *_mgr)
> -{
> -    FREE(_mgr);
> -}
> -
> -struct pb_manager *radeon_bomgr_create(struct radeon_drm_winsys *rws)
> -{
> -    struct radeon_bomgr *mgr;
> -
> -    mgr = CALLOC_STRUCT(radeon_bomgr);
> -    if (!mgr)
> -        return NULL;
> -
> -    mgr->base.destroy = radeon_bomgr_destroy;
> -    mgr->base.create_buffer = radeon_bomgr_create_bo;
> -    mgr->base.flush = radeon_bomgr_flush;
> -    mgr->base.is_buffer_busy = radeon_bomgr_is_buffer_busy;
> -
> -    mgr->rws = rws;
> -    return &mgr->base;
> +   return radeon_bo_wait(_buf, 0, RADEON_USAGE_READWRITE);
>   }
>
>   static unsigned eg_tile_split(unsigned tile_split)
> @@ -721,7 +637,7 @@ static void radeon_bo_get_tiling(struct pb_buffer *_buf,
>                                    unsigned *mtilea,
>                                    bool *scanout)
>   {
> -    struct radeon_bo *bo = get_radeon_bo(_buf);
> +    struct radeon_bo *bo = radeon_bo(_buf);
>       struct drm_radeon_gem_set_tiling args;
>
>       memset(&args, 0, sizeof(args));
> @@ -766,7 +682,7 @@ static void radeon_bo_set_tiling(struct pb_buffer *_buf,
>                                    uint32_t pitch,
>                                    bool scanout)
>   {
> -    struct radeon_bo *bo = get_radeon_bo(_buf);
> +    struct radeon_bo *bo = radeon_bo(_buf);
>       struct radeon_drm_cs *cs = radeon_drm_cs(rcs);
>       struct drm_radeon_gem_set_tiling args;
>
> @@ -818,7 +734,7 @@ static void radeon_bo_set_tiling(struct pb_buffer *_buf,
>   static struct radeon_winsys_cs_handle *radeon_drm_get_cs_handle(struct pb_buffer *_buf)
>   {
>       /* return radeon_bo. */
> -    return (struct radeon_winsys_cs_handle*)get_radeon_bo(_buf);
> +    return (struct radeon_winsys_cs_handle*)radeon_bo(_buf);
>   }
>
>   static struct pb_buffer *
> @@ -830,12 +746,8 @@ radeon_winsys_bo_create(struct radeon_winsys *rws,
>                           enum radeon_bo_flag flags)
>   {
>       struct radeon_drm_winsys *ws = radeon_drm_winsys(rws);
> -    struct radeon_bo_desc desc;
> -    struct pb_manager *provider;
> -    struct pb_buffer *buffer;
> -
> -    memset(&desc, 0, sizeof(desc));
> -    desc.base.alignment = alignment;
> +    struct radeon_bo *bo;
> +    unsigned usage = 0;
>
>       /* Align size to page size. This is the minimum alignment for normal
>        * BOs. Aligning this here helps the cached bufmgr. Especially small BOs,
> @@ -847,30 +759,29 @@ radeon_winsys_bo_create(struct radeon_winsys *rws,
>        * might consider different sets of domains / flags compatible
>        */
>       if (domain == RADEON_DOMAIN_VRAM_GTT)
> -        desc.base.usage = 1 << 2;
> -    else
> -        desc.base.usage = domain >> 1;
> -    assert(flags < sizeof(desc.base.usage) * 8 - 3);
> -    desc.base.usage |= 1 << (flags + 3);
> -
> -    desc.initial_domains = domain;
> -    desc.flags = flags;
> -
> -    /* Assign a buffer manager. */
> -    if (use_reusable_pool)
> -        provider = ws->cman;
> +        usage = 1 << 2;
>       else
> -        provider = ws->kman;
> +        usage = domain >> 1;
> +    assert(flags < sizeof(usage) * 8 - 3);
> +    usage |= 1 << (flags + 3);
> +
> +    if (use_reusable_pool) {
> +        bo = pb_cache_reclaim_buffer(&ws->bo_cache, size, alignment, usage);
> +        if (bo)
> +            return bo;
> +    }
>
> -    buffer = provider->create_buffer(provider, size, &desc.base);
> -    if (!buffer)
> +    bo = radeon_create_bo(ws, size, alignment, usage, domain, flags);
> +    if (!bo)
>           return NULL;
>
> +    bo->use_reusable_pool = use_reusable_pool;
> +
>       pipe_mutex_lock(ws->bo_handles_mutex);
> -    util_hash_table_set(ws->bo_handles, (void*)(uintptr_t)get_radeon_bo(buffer)->handle, buffer);
> +    util_hash_table_set(ws->bo_handles, (void*)(uintptr_t)bo->handle, bo);
>       pipe_mutex_unlock(ws->bo_handles_mutex);
>
> -    return (struct pb_buffer*)buffer;
> +    return &bo->base;
>   }
>
>   static struct pb_buffer *radeon_winsys_bo_from_ptr(struct radeon_winsys *rws,
> @@ -1101,13 +1012,12 @@ static boolean radeon_winsys_bo_get_handle(struct pb_buffer *buffer,
>                                              struct winsys_handle *whandle)
>   {
>       struct drm_gem_flink flink;
> -    struct radeon_bo *bo = get_radeon_bo(buffer);
> +    struct radeon_bo *bo = radeon_bo(buffer);
>       struct radeon_drm_winsys *ws = bo->rws;
>
>       memset(&flink, 0, sizeof(flink));
>
> -    if ((void*)bo != (void*)buffer)
> -       pb_cache_manager_remove_buffer(buffer);
> +    bo->use_reusable_pool = false;
>
>       if (whandle->type == DRM_API_HANDLE_TYPE_SHARED) {
>           if (!bo->flink_name) {
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.h b/src/gallium/winsys/radeon/drm/radeon_drm_bo.h
> index bee42c4..f7f4ce3 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.h
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.h
> @@ -36,17 +36,9 @@
>   #include "pipebuffer/pb_bufmgr.h"
>   #include "os/os_thread.h"
>
> -struct radeon_bomgr;
> -
> -struct radeon_bo_desc {
> -    struct pb_desc base;
> -
> -    unsigned initial_domains;
> -    unsigned flags;
> -};
> -
>   struct radeon_bo {
>       struct pb_buffer base;
> +    struct pb_cache_entry cache_entry;
>
>       struct radeon_drm_winsys *rws;
>       void *user_ptr; /* from buffer_from_ptr */
> @@ -59,6 +51,7 @@ struct radeon_bo {
>       uint32_t flink_name;
>       uint64_t va;
>       enum radeon_bo_domain initial_domain;
> +    bool use_reusable_pool;
>
>       /* how many command streams is this bo referenced in? */
>       int num_cs_references;
> @@ -68,7 +61,8 @@ struct radeon_bo {
>       int num_active_ioctls;
>   };
>
> -struct pb_manager *radeon_bomgr_create(struct radeon_drm_winsys *rws);
> +void radeon_bo_destroy(struct pb_buffer *_buf);
> +bool radeon_bo_can_reclaim(struct pb_buffer *_buf);
>   void radeon_drm_bo_init_functions(struct radeon_drm_winsys *ws);
>
>   static inline
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> index 2757192..fdfb95b 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> @@ -500,8 +500,8 @@ static void radeon_winsys_destroy(struct radeon_winsys *rws)
>       pipe_mutex_destroy(ws->bo_handles_mutex);
>       pipe_mutex_destroy(ws->bo_va_mutex);
>
> -    ws->cman->destroy(ws->cman);
> -    ws->kman->destroy(ws->kman);
> +    pb_cache_deinit(&ws->bo_cache);
> +
>       if (ws->gen >= DRV_R600) {
>           radeon_surface_manager_free(ws->surf_man);
>       }
> @@ -744,15 +744,10 @@ radeon_drm_winsys_create(int fd, radeon_screen_create_t screen_create)
>       if (!do_winsys_init(ws))
>           goto fail;
>
> -    /* Create managers. */
> -    ws->kman = radeon_bomgr_create(ws);
> -    if (!ws->kman)
> -        goto fail;
> -
> -    ws->cman = pb_cache_manager_create(ws->kman, 500000, 2.0f, 0,
> -                                       MIN2(ws->info.vram_size, ws->info.gart_size));
> -    if (!ws->cman)
> -        goto fail;
> +    pb_cache_init(&ws->bo_cache, 500000, 2.0f, 0,
> +                  MIN2(ws->info.vram_size, ws->info.gart_size),
> +                  radeon_bo_destroy,
> +                  radeon_bo_can_reclaim);
>
>       if (ws->gen >= DRV_R600) {
>           ws->surf_man = radeon_surface_manager_new(ws->fd);
> @@ -818,10 +813,7 @@ radeon_drm_winsys_create(int fd, radeon_screen_create_t screen_create)
>
>   fail:
>       pipe_mutex_unlock(fd_tab_mutex);
> -    if (ws->cman)
> -        ws->cman->destroy(ws->cman);
> -    if (ws->kman)
> -        ws->kman->destroy(ws->kman);
> +    pb_cache_deinit(&ws->bo_cache);
>       if (ws->surf_man)
>           radeon_surface_manager_free(ws->surf_man);
>       if (ws->fd >= 0)
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
> index f941b52..75c1bf4 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h
> @@ -31,6 +31,7 @@
>   #define RADEON_DRM_WINSYS_H
>
>   #include "gallium/drivers/radeon/radeon_winsys.h"
> +#include "pipebuffer/pb_cache.h"
>   #include "os/os_thread.h"
>   #include "util/list.h"
>   #include <radeon_drm.h>
> @@ -64,6 +65,7 @@ enum radeon_generation {
>   struct radeon_drm_winsys {
>       struct radeon_winsys base;
>       struct pipe_reference reference;
> +    struct pb_cache bo_cache;
>
>       int fd; /* DRM file descriptor */
>       int num_cs; /* The number of command streams created. */
> @@ -93,8 +95,6 @@ struct radeon_drm_winsys {
>       /* BO size alignment */
>       unsigned size_align;
>
> -    struct pb_manager *kman;
> -    struct pb_manager *cman;
>       struct radeon_surface_manager *surf_man;
>
>       uint32_t num_cpus;      /* Number of CPUs. */
>


More information about the mesa-dev mailing list