From 187218395d7c9e8257ac17c7cbf1cc7add5c9363 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Mon, 28 Jan 2019 20:22:57 +0100 Subject: [PATCH] renderonly: remove layering violations MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The renderonly object is something the winsys creates, so the pipe driver has no business in memcpying or freeing it. Move those bits to the winsys. Signed-off-by: Lucas Stach Reviewed-by: Guido Günther Reviewed-by: Eric Anholt Reviewed-by: Christian Gmeiner Reviewed-by: Alyssa Rosenzweig Part-of: --- src/gallium/auxiliary/renderonly/renderonly.c | 14 ---- src/gallium/auxiliary/renderonly/renderonly.h | 4 +- src/gallium/drivers/etnaviv/etnaviv_screen.c | 4 +- src/gallium/drivers/freedreno/freedreno_screen.c | 11 +-- src/gallium/drivers/lima/lima_screen.c | 13 +--- src/gallium/drivers/panfrost/pan_screen.c | 17 ++--- src/gallium/drivers/v3d/v3d_screen.c | 13 +--- src/gallium/drivers/vc4/vc4_screen.c | 11 +-- .../winsys/etnaviv/drm/etnaviv_drm_winsys.c | 27 +++++-- src/gallium/winsys/kmsro/drm/kmsro_drm_winsys.c | 85 +++++++++++++--------- 10 files changed, 91 insertions(+), 108 deletions(-) diff --git a/src/gallium/auxiliary/renderonly/renderonly.c b/src/gallium/auxiliary/renderonly/renderonly.c index 2daf369..00c8a7e 100644 --- a/src/gallium/auxiliary/renderonly/renderonly.c +++ b/src/gallium/auxiliary/renderonly/renderonly.c @@ -37,20 +37,6 @@ #include "util/u_inlines.h" #include "util/u_memory.h" -struct renderonly * -renderonly_dup(const struct renderonly *ro) -{ - struct renderonly *copy; - - copy = CALLOC_STRUCT(renderonly); - if (!copy) - return NULL; - - memcpy(copy, ro, sizeof(*ro)); - - return copy; -} - void renderonly_scanout_destroy(struct renderonly_scanout *scanout, struct renderonly *ro) diff --git a/src/gallium/auxiliary/renderonly/renderonly.h b/src/gallium/auxiliary/renderonly/renderonly.h index fe13fea..0d08a16 100644 --- a/src/gallium/auxiliary/renderonly/renderonly.h +++ b/src/gallium/auxiliary/renderonly/renderonly.h @@ -59,13 +59,11 @@ struct renderonly { struct renderonly_scanout *(*create_for_resource)(struct pipe_resource *rsc, struct renderonly *ro, struct winsys_handle *out_handle); + void (*destroy)(struct renderonly *ro); int kms_fd; int gpu_fd; }; -struct renderonly * -renderonly_dup(const struct renderonly *ro); - static inline struct renderonly_scanout * renderonly_scanout_for_resource(struct pipe_resource *rsc, struct renderonly *ro, diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c b/src/gallium/drivers/etnaviv/etnaviv_screen.c index e6a71c0..9992dd1 100644 --- a/src/gallium/drivers/etnaviv/etnaviv_screen.c +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c @@ -99,7 +99,7 @@ etna_screen_destroy(struct pipe_screen *pscreen) etna_gpu_del(screen->gpu); if (screen->ro) - FREE(screen->ro); + screen->ro->destroy(screen->ro); if (screen->dev) etna_device_del(screen->dev); @@ -939,7 +939,7 @@ etna_screen_create(struct etna_device *dev, struct etna_gpu *gpu, pscreen = &screen->base; screen->dev = dev; screen->gpu = gpu; - screen->ro = renderonly_dup(ro); + screen->ro = ro; screen->refcnt = 1; if (!screen->ro) { diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c b/src/gallium/drivers/freedreno/freedreno_screen.c index ab0bcbf..b274858 100644 --- a/src/gallium/drivers/freedreno/freedreno_screen.c +++ b/src/gallium/drivers/freedreno/freedreno_screen.c @@ -153,7 +153,7 @@ fd_screen_destroy(struct pipe_screen *pscreen) fd_device_del(screen->dev); if (screen->ro) - FREE(screen->ro); + screen->ro->destroy(screen->ro); fd_bc_fini(&screen->batch_cache); fd_gmem_screen_fini(pscreen); @@ -903,16 +903,9 @@ fd_screen_create(struct fd_device *dev, struct renderonly *ro) pscreen = &screen->base; screen->dev = dev; + screen->ro = ro; screen->refcnt = 1; - if (ro) { - screen->ro = renderonly_dup(ro); - if (!screen->ro) { - DBG("could not create renderonly object"); - goto fail; - } - } - // maybe this should be in context? screen->pipe = fd_pipe_new(screen->dev, FD_PIPE_3D); if (!screen->pipe) { diff --git a/src/gallium/drivers/lima/lima_screen.c b/src/gallium/drivers/lima/lima_screen.c index fcf27ac..e3c1eff 100644 --- a/src/gallium/drivers/lima/lima_screen.c +++ b/src/gallium/drivers/lima/lima_screen.c @@ -54,7 +54,7 @@ lima_screen_destroy(struct pipe_screen *pscreen) slab_destroy_parent(&screen->transfer_pool); if (screen->ro) - free(screen->ro); + screen->ro->destroy(screen->ro); if (screen->pp_buffer) lima_bo_unreference(screen->pp_buffer); @@ -621,6 +621,7 @@ lima_screen_create(int fd, struct renderonly *ro) return NULL; screen->fd = fd; + screen->ro = ro; lima_screen_parse_env(); @@ -692,14 +693,6 @@ lima_screen_create(int fd, struct renderonly *ro) pp_frame_rsw[9] = screen->pp_buffer->va + pp_clear_program_offset; pp_frame_rsw[13] = 0x00000100; - if (ro) { - screen->ro = renderonly_dup(ro); - if (!screen->ro) { - fprintf(stderr, "Failed to dup renderonly object\n"); - goto err_out3; - } - } - screen->base.destroy = lima_screen_destroy; screen->base.get_name = lima_screen_get_name; screen->base.get_vendor = lima_screen_get_vendor; @@ -722,8 +715,6 @@ lima_screen_create(int fd, struct renderonly *ro) return &screen->base; -err_out3: - lima_bo_unreference(screen->pp_buffer); err_out2: lima_bo_table_fini(screen); err_out1: diff --git a/src/gallium/drivers/panfrost/pan_screen.c b/src/gallium/drivers/panfrost/pan_screen.c index 8fa8d65..819a8eb 100644 --- a/src/gallium/drivers/panfrost/pan_screen.c +++ b/src/gallium/drivers/panfrost/pan_screen.c @@ -683,7 +683,11 @@ panfrost_get_compute_param(struct pipe_screen *pscreen, enum pipe_shader_ir ir_t static void panfrost_destroy_screen(struct pipe_screen *pscreen) { - panfrost_close_device(pan_device(pscreen)); + struct panfrost_device *dev = pan_device(pscreen); + + if (dev->ro) + dev->ro->destroy(dev->ro); + panfrost_close_device(dev); ralloc_free(pscreen); } @@ -810,16 +814,7 @@ panfrost_create_screen(int fd, struct renderonly *ro) if (dev->debug & PAN_DBG_NO_AFBC) dev->quirks |= MIDGARD_NO_AFBC; - if (ro) { - dev->ro = renderonly_dup(ro); - if (!dev->ro) { - if (dev->debug & PAN_DBG_MSGS) - fprintf(stderr, "Failed to dup renderonly object\n"); - - free(screen); - return NULL; - } - } + dev->ro = ro; /* Check if we're loading against a supported GPU model. */ diff --git a/src/gallium/drivers/v3d/v3d_screen.c b/src/gallium/drivers/v3d/v3d_screen.c index 7ba6ffa..6adeda0 100644 --- a/src/gallium/drivers/v3d/v3d_screen.c +++ b/src/gallium/drivers/v3d/v3d_screen.c @@ -76,7 +76,8 @@ v3d_screen_destroy(struct pipe_screen *pscreen) _mesa_hash_table_destroy(screen->bo_handles, NULL); v3d_bufmgr_destroy(pscreen); slab_destroy_parent(&screen->transfer_pool); - free(screen->ro); + if (screen->ro) + screen->ro->destroy(screen->ro); if (using_v3d_simulator) v3d_simulator_destroy(screen->sim_file); @@ -699,14 +700,8 @@ v3d_screen_create(int fd, const struct pipe_screen_config *config, pscreen->is_format_supported = v3d_screen_is_format_supported; screen->fd = fd; - if (ro) { - screen->ro = renderonly_dup(ro); - if (!screen->ro) { - fprintf(stderr, "Failed to dup renderonly object\n"); - ralloc_free(screen); - return NULL; - } - } + screen->ro = ro; + list_inithead(&screen->bo_cache.time_list); (void)mtx_init(&screen->bo_handles_mutex, mtx_plain); screen->bo_handles = util_hash_table_create_ptr_keys(); diff --git a/src/gallium/drivers/vc4/vc4_screen.c b/src/gallium/drivers/vc4/vc4_screen.c index bad27d5..5b78ff8 100644 --- a/src/gallium/drivers/vc4/vc4_screen.c +++ b/src/gallium/drivers/vc4/vc4_screen.c @@ -105,7 +105,7 @@ vc4_screen_destroy(struct pipe_screen *pscreen) _mesa_hash_table_destroy(screen->bo_handles, NULL); vc4_bufmgr_destroy(pscreen); slab_destroy_parent(&screen->transfer_pool); - free(screen->ro); + screen->ro->destroy(screen->ro); #ifdef USE_VC4_SIMULATOR vc4_simulator_destroy(screen); @@ -552,14 +552,7 @@ vc4_screen_create(int fd, struct renderonly *ro) pscreen->is_format_supported = vc4_screen_is_format_supported; screen->fd = fd; - if (ro) { - screen->ro = renderonly_dup(ro); - if (!screen->ro) { - fprintf(stderr, "Failed to dup renderonly object\n"); - ralloc_free(screen); - return NULL; - } - } + screen->ro = ro; list_inithead(&screen->bo_cache.time_list); (void) mtx_init(&screen->bo_handles_mutex, mtx_plain); diff --git a/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c b/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c index 6c40453..d5670af 100644 --- a/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c +++ b/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c @@ -185,14 +185,29 @@ unlock: return pscreen; } +static void etnaviv_ro_destroy(struct renderonly *ro) +{ + FREE(ro); +} + struct pipe_screen * etna_drm_screen_create(int fd) { - struct renderonly ro = { - .create_for_resource = renderonly_create_gpu_import_for_resource, - .kms_fd = -1, - .gpu_fd = fd - }; - return etna_drm_screen_create_renderonly(&ro); + struct renderonly *ro = CALLOC_STRUCT(renderonly); + struct pipe_screen *screen; + + if (!ro) + return NULL; + + ro->create_for_resource = renderonly_create_gpu_import_for_resource; + ro->destroy = etnaviv_ro_destroy; + ro->kms_fd = -1; + ro->gpu_fd = fd; + + screen = etna_drm_screen_create_renderonly(ro); + if (!screen) + FREE(ro); + + return screen; } diff --git a/src/gallium/winsys/kmsro/drm/kmsro_drm_winsys.c b/src/gallium/winsys/kmsro/drm/kmsro_drm_winsys.c index bf599a1..9000a6a 100644 --- a/src/gallium/winsys/kmsro/drm/kmsro_drm_winsys.c +++ b/src/gallium/winsys/kmsro/drm/kmsro_drm_winsys.c @@ -36,92 +36,109 @@ #include "pipe/p_screen.h" #include "renderonly/renderonly.h" +#include "util/u_memory.h" + +static void kmsro_ro_destroy(struct renderonly *ro) +{ + FREE(ro); +} struct pipe_screen *kmsro_drm_screen_create(int fd, const struct pipe_screen_config *config) { struct pipe_screen *screen = NULL; - struct renderonly ro = { - .kms_fd = fd, - .gpu_fd = -1, - }; + struct renderonly *ro = CALLOC_STRUCT(renderonly); + + if (!ro) + return NULL; + + ro->kms_fd = fd; + ro->gpu_fd = -1; + ro->destroy = kmsro_ro_destroy; #if defined(GALLIUM_VC4) - ro.gpu_fd = drmOpenWithType("vc4", NULL, DRM_NODE_RENDER); - if (ro.gpu_fd >= 0) { + ro->gpu_fd = drmOpenWithType("vc4", NULL, DRM_NODE_RENDER); + if (ro->gpu_fd >= 0) { /* Passes the vc4-allocated BO through to the KMS-only DRM device using * PRIME buffer sharing. The VC4 BO must be linear, which the SCANOUT * flag on allocation will have ensured. */ - ro.create_for_resource = renderonly_create_gpu_import_for_resource, - screen = vc4_drm_screen_create_renderonly(&ro, config); + ro->create_for_resource = renderonly_create_gpu_import_for_resource; + screen = vc4_drm_screen_create_renderonly(ro, config); if (!screen) - close(ro.gpu_fd); + goto out_free; return screen; } #endif #if defined(GALLIUM_ETNAVIV) - ro.gpu_fd = drmOpenWithType("etnaviv", NULL, DRM_NODE_RENDER); - if (ro.gpu_fd >= 0) { - ro.create_for_resource = renderonly_create_kms_dumb_buffer_for_resource, - screen = etna_drm_screen_create_renderonly(&ro); + ro->gpu_fd = drmOpenWithType("etnaviv", NULL, DRM_NODE_RENDER); + if (ro->gpu_fd >= 0) { + ro->create_for_resource = renderonly_create_kms_dumb_buffer_for_resource; + screen = etna_drm_screen_create_renderonly(ro); if (!screen) - close(ro.gpu_fd); + goto out_free; return screen; } #endif #if defined(GALLIUM_FREEDRENO) - ro.gpu_fd = drmOpenWithType("msm", NULL, DRM_NODE_RENDER); - if (ro.gpu_fd >= 0) { - ro.create_for_resource = renderonly_create_kms_dumb_buffer_for_resource, - screen = fd_drm_screen_create(ro.gpu_fd, &ro); + ro->gpu_fd = drmOpenWithType("msm", NULL, DRM_NODE_RENDER); + if (ro->gpu_fd >= 0) { + ro->create_for_resource = renderonly_create_kms_dumb_buffer_for_resource; + screen = fd_drm_screen_create(ro->gpu_fd, ro); if (!screen) - close(ro.gpu_fd); + goto out_free; return screen; } #endif #if defined(GALLIUM_PANFROST) - ro.gpu_fd = drmOpenWithType("panfrost", NULL, DRM_NODE_RENDER); + ro->gpu_fd = drmOpenWithType("panfrost", NULL, DRM_NODE_RENDER); - if (ro.gpu_fd >= 0) { - ro.create_for_resource = renderonly_create_kms_dumb_buffer_for_resource, - screen = panfrost_drm_screen_create_renderonly(&ro); + if (ro->gpu_fd >= 0) { + ro->create_for_resource = renderonly_create_kms_dumb_buffer_for_resource, + screen = panfrost_drm_screen_create_renderonly(ro); if (!screen) - close(ro.gpu_fd); + goto out_free; return screen; } #endif #if defined(GALLIUM_LIMA) - ro.gpu_fd = drmOpenWithType("lima", NULL, DRM_NODE_RENDER); - if (ro.gpu_fd >= 0) { - ro.create_for_resource = renderonly_create_kms_dumb_buffer_for_resource, - screen = lima_drm_screen_create_renderonly(&ro); + ro->gpu_fd = drmOpenWithType("lima", NULL, DRM_NODE_RENDER); + if (ro->gpu_fd >= 0) { + ro->create_for_resource = renderonly_create_kms_dumb_buffer_for_resource, + screen = lima_drm_screen_create_renderonly(ro); if (!screen) - close(ro.gpu_fd); + goto out_free; return screen; } #endif #if defined(GALLIUM_V3D) - ro.gpu_fd = drmOpenWithType("v3d", NULL, DRM_NODE_RENDER); - if (ro.gpu_fd >= 0) { - ro.create_for_resource = renderonly_create_kms_dumb_buffer_for_resource, - screen = v3d_drm_screen_create_renderonly(&ro, config); + ro->gpu_fd = drmOpenWithType("v3d", NULL, DRM_NODE_RENDER); + if (ro->gpu_fd >= 0) { + ro->create_for_resource = renderonly_create_kms_dumb_buffer_for_resource, + screen = v3d_drm_screen_create_renderonly(ro, config); if (!screen) - close(ro.gpu_fd); + goto out_free; return screen; } #endif return screen; + +out_free: + if (ro->gpu_fd >= 0) + close(ro->gpu_fd); + FREE(ro); + + return NULL; } -- 2.7.4