renderonly: remove layering violations

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 <l.stach@pengutronix.de>
Reviewed-by: Guido Günther <agx@sigxcpu.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>
Reviewed-by: Alyssa Rosenzweig <alyssa@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6983>

diff --git a/src/gallium/auxiliary/renderonly/renderonly.c b/src/gallium/auxiliary/renderonly/renderonly.c
index 2daf369..00c8a7e 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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;
 }