[v2,14/37] panfrost: Stop using panfrost_bo_release() outside of pan_bo.c

Submitted by Boris Brezillon on Sept. 16, 2019, 9:36 a.m.

Details

Message ID 20190916093715.32203-15-boris.brezillon@collabora.com
State Accepted
Commit cb71ae5572a16a974e3f60ad66fcae329c85961a
Headers show
Series "panfrost: Support batch pipelining" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Boris Brezillon Sept. 16, 2019, 9:36 a.m.
panfrost_bo_unreference() should be used instead.

The only difference caused by this change is that the scratchpad,
tiler_heap and tiler_dummy BOs are now returned to the cache instead
of being freed when a context is destroyed. This is only a problem if
we care about context isolation, which apparently is not the case since
transient BOs are already returned to the per-FD cache (and all contexts
share the same address space anyway, so enforcing context isolation
is almost impossible).

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 src/gallium/drivers/panfrost/pan_bo.c       | 5 ++++-
 src/gallium/drivers/panfrost/pan_bo.h       | 2 --
 src/gallium/drivers/panfrost/pan_context.c  | 6 +++---
 src/gallium/drivers/panfrost/pan_resource.c | 2 +-
 4 files changed, 8 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/drivers/panfrost/pan_bo.c b/src/gallium/drivers/panfrost/pan_bo.c
index d9c4cb208bc8..396b25230c46 100644
--- a/src/gallium/drivers/panfrost/pan_bo.c
+++ b/src/gallium/drivers/panfrost/pan_bo.c
@@ -84,6 +84,9 @@  pan_bucket(struct panfrost_screen *screen, unsigned size)
         return &screen->bo_cache[pan_bucket_index(size)];
 }
 
+static void
+panfrost_bo_release(struct panfrost_bo *bo, bool cacheable);
+
 /* Tries to fetch a BO of sufficient size with the appropriate flags from the
  * BO cache. If it succeeds, it returns that BO and removes the BO from the
  * cache. If it fails, it returns NULL signaling the caller to allocate a new
@@ -287,7 +290,7 @@  panfrost_bo_create(struct panfrost_screen *screen, size_t size,
         return bo;
 }
 
-void
+static void
 panfrost_bo_release(struct panfrost_bo *bo, bool cacheable)
 {
         if (!bo)
diff --git a/src/gallium/drivers/panfrost/pan_bo.h b/src/gallium/drivers/panfrost/pan_bo.h
index dfdb202e5d34..2858d3782eff 100644
--- a/src/gallium/drivers/panfrost/pan_bo.h
+++ b/src/gallium/drivers/panfrost/pan_bo.h
@@ -83,8 +83,6 @@  panfrost_bo_create(struct panfrost_screen *screen, size_t size,
                    uint32_t flags);
 void
 panfrost_bo_mmap(struct panfrost_bo *bo);
-void
-panfrost_bo_release(struct panfrost_bo *bo, bool cacheable);
 struct panfrost_bo *
 panfrost_bo_import(struct panfrost_screen *screen, int fd);
 int
diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
index 55fe9c264548..c5139a21f9a3 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -2425,9 +2425,9 @@  panfrost_destroy(struct pipe_context *pipe)
         if (panfrost->blitter_wallpaper)
                 util_blitter_destroy(panfrost->blitter_wallpaper);
 
-        panfrost_bo_release(panfrost->scratchpad, false);
-        panfrost_bo_release(panfrost->tiler_heap, false);
-        panfrost_bo_release(panfrost->tiler_dummy, false);
+        panfrost_bo_unreference(panfrost->scratchpad);
+        panfrost_bo_unreference(panfrost->tiler_heap);
+        panfrost_bo_unreference(panfrost->tiler_dummy);
 
         ralloc_free(pipe);
 }
diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c
index 1e8a1eadb51d..363a330c4fd0 100644
--- a/src/gallium/drivers/panfrost/pan_resource.c
+++ b/src/gallium/drivers/panfrost/pan_resource.c
@@ -839,7 +839,7 @@  panfrost_resource_hint_layout(
 
         /* If we grew in size, reallocate the BO */
         if (new_size > rsrc->bo->size) {
-                panfrost_bo_release(rsrc->bo, true);
+                panfrost_bo_unreference(rsrc->bo);
                 rsrc->bo = panfrost_bo_create(screen, new_size, PAN_BO_DELAY_MMAP);
         }
 }

Comments

> The only difference caused by this change is that the scratchpad,
> tiler_heap and tiler_dummy BOs are now returned to the cache instead
> of being freed when a context is destroyed.

That sounds fine to me.

> This is only a problem if
> we care about context isolation, which apparently is not the case since
> transient BOs are already returned to the per-FD cache (and all contexts
> share the same address space anyway, so enforcing context isolation
> is almost impossible).

We care about process isolation, not context isolation. Different
contexts within the same process (i.e. all corresponding to the same
_screen) are not meant to be isolated, as far as I know.

So r-b