freedreno/drm: Start prying apart table_lock

We want to get back to the point where table_lock is only doing what
it's named for (ie. protecting handle/name tables).  This is the first
step.  Add new locks to protect fencing and the bo caches.  A single
global lock for fences, to avoid atomic instructions in the loop per
submit bo attaching fences.

This couldn't be broken appart into different commits, otherwise there
would have been an ABBA deadlock in BO cache's call to fd_bo_state()
to check if a BO is idle.

Signed-off-by: Rob Clark <robdclark@chromium.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20263>

src/freedreno/drm/freedreno_bo.c

@@ -30,6 +30,7 @@
 #include "freedreno_priv.h"
 
 simple_mtx_t table_lock = SIMPLE_MTX_INITIALIZER;
+simple_mtx_t fence_lock = SIMPLE_MTX_INITIALIZER;
 void bo_del(struct fd_bo *bo);
 void bo_del_flush(struct fd_device *dev);
 
@@ -347,7 +348,7 @@ bo_del(struct fd_bo *bo)
    simple_mtx_assert_locked(&table_lock);
 
    for (int i = 0; i < bo->nr_fences; i++)
-      fd_pipe_del_locked(bo->fences[i].pipe);
+      fd_pipe_del(bo->fences[i].pipe);
 
    if (bo->fences != &bo->_inline_fence)
       free(bo->fences);
@@ -524,9 +525,7 @@ fd_bo_prefer_upload(struct fd_bo *bo, unsigned len)
 int
 fd_bo_cpu_prep(struct fd_bo *bo, struct fd_pipe *pipe, uint32_t op)
 {
-   simple_mtx_lock(&table_lock);
    enum fd_bo_state state = fd_bo_state(bo);
-   simple_mtx_unlock(&table_lock);
 
    if (state == FD_BO_STATE_IDLE)
       return 0;
@@ -570,7 +569,7 @@ fd_bo_cpu_prep(struct fd_bo *bo, struct fd_pipe *pipe, uint32_t op)
 static void
 cleanup_fences(struct fd_bo *bo)
 {
-   simple_mtx_assert_locked(&table_lock);
+   simple_mtx_assert_locked(&fence_lock);
 
    for (int i = 0; i < bo->nr_fences; i++) {
       struct fd_bo_fence *f = &bo->fences[i];
@@ -595,7 +594,7 @@ cleanup_fences(struct fd_bo *bo)
 void
 fd_bo_add_fence(struct fd_bo *bo, struct fd_pipe *pipe, uint32_t fence)
 {
-   simple_mtx_assert_locked(&table_lock);
+   simple_mtx_assert_locked(&fence_lock);
 
    if (bo->nosync)
       return;
@@ -634,13 +633,18 @@ fd_bo_add_fence(struct fd_bo *bo, struct fd_pipe *pipe, uint32_t fence)
 enum fd_bo_state
 fd_bo_state(struct fd_bo *bo)
 {
-   simple_mtx_assert_locked(&table_lock);
-
-   cleanup_fences(bo);
-
+   /* NOTE: check the nosync case before touching fence_lock in case we end
+    * up here recursively from dropping pipe reference in cleanup_fences().
+    * The pipe's control buffer is specifically nosync to avoid recursive
+    * lock problems here.
+    */
    if (bo->shared || bo->nosync)
       return FD_BO_STATE_UNKNOWN;
 
+   simple_mtx_lock(&fence_lock);
+   cleanup_fences(bo);
+   simple_mtx_unlock(&fence_lock);
+
    if (!bo->nr_fences)
       return FD_BO_STATE_IDLE;
 

src/freedreno/drm/freedreno_bo_cache.c

@@ -37,7 +37,6 @@
 
 void bo_del(struct fd_bo *bo);
 void bo_del_flush(struct fd_device *dev);
-extern simple_mtx_t table_lock;
 
 static void
 bo_remove_from_bucket(struct fd_bo_bucket *bucket, struct fd_bo *bo)
@@ -116,6 +115,7 @@ fd_bo_cache_init(struct fd_bo_cache *cache, int coarse, const char *name)
    unsigned long size, cache_max_size = 64 * 1024 * 1024;
 
    cache->name = name;
+   simple_mtx_init(&cache->lock, mtx_plain);
 
    /* OK, so power of two buckets was too wasteful of memory.
     * Give 3 other sizes between each power of two, to hopefully
@@ -153,6 +153,11 @@ fd_bo_cache_cleanup(struct fd_bo_cache *cache, time_t time)
    if (cache->time == time)
       return;
 
+   struct list_head freelist;
+
+   list_inithead(&freelist);
+
+   simple_mtx_lock(&cache->lock);
    for (i = 0; i < cache->num_buckets; i++) {
       struct fd_bo_bucket *bucket = &cache->cache_bucket[i];
       struct fd_bo *bo;
@@ -174,11 +179,17 @@ fd_bo_cache_cleanup(struct fd_bo_cache *cache, time_t time)
          VG_BO_OBTAIN(bo);
          bo_remove_from_bucket(bucket, bo);
          bucket->expired++;
-         bo_del(bo);
+         list_addtail(&bo->node, &freelist);
 
          cnt++;
       }
    }
+   simple_mtx_unlock(&cache->lock);
+
+   foreach_bo_safe (bo, &freelist) {
+      list_del(&bo->node);
+      bo_del(bo);
+   }
 
    /* Note: when called in bo_del_or_recycle() -> fd_bo_cache_free() path,
     * the caller will handle bo_del_flush().
@@ -224,7 +235,6 @@ find_in_bucket(struct fd_bo_bucket *bucket, uint32_t flags)
     * NOTE that intel takes ALLOC_FOR_RENDER bo's from the list tail
     * (MRU, since likely to be in GPU cache), rather than head (LRU)..
     */
-   simple_mtx_lock(&table_lock);
    foreach_bo (entry, &bucket->list) {
       if (fd_bo_state(entry) != FD_BO_STATE_IDLE) {
          break;
@@ -235,7 +245,6 @@ find_in_bucket(struct fd_bo_bucket *bucket, uint32_t flags)
          break;
       }
    }
-   simple_mtx_unlock(&table_lock);
 
    return bo;
 }
@@ -254,7 +263,9 @@ fd_bo_cache_alloc(struct fd_bo_cache *cache, uint32_t *size, uint32_t flags)
 retry:
    if (bucket) {
       *size = bucket->size;
+      simple_mtx_lock(&cache->lock);
       bo = find_in_bucket(bucket, flags);
+      simple_mtx_unlock(&cache->lock);
       if (bo) {
          VG_BO_OBTAIN(bo);
          if (bo->funcs->madvise(bo, true) <= 0) {
@@ -301,9 +312,11 @@ fd_bo_cache_free(struct fd_bo_cache *cache, struct fd_bo *bo)
 
       bo->free_time = time.tv_sec;
       VG_BO_RELEASE(bo);
-      list_addtail(&bo->node, &bucket->list);
 
+      simple_mtx_lock(&cache->lock);
+      list_addtail(&bo->node, &bucket->list);
       bucket->count++;
+      simple_mtx_unlock(&cache->lock);
 
       fd_bo_cache_cleanup(cache, time.tv_sec);
 

src/freedreno/drm/freedreno_device.c

@@ -178,14 +178,6 @@ fd_device_del_impl(struct fd_device *dev)
    free(dev);
 }
 
-void
-fd_device_del_locked(struct fd_device *dev)
-{
-   if (!p_atomic_dec_zero(&dev->refcnt))
-      return;
-   fd_device_del_impl(dev);
-}
-
 void
 fd_device_del(struct fd_device *dev)
 {

src/freedreno/drm/freedreno_pipe.c

@@ -94,46 +94,38 @@ fd_pipe_new(struct fd_device *dev, enum fd_pipe_id id)
 struct fd_pipe *
 fd_pipe_ref(struct fd_pipe *pipe)
 {
-   simple_mtx_lock(&table_lock);
+   simple_mtx_lock(&fence_lock);
    fd_pipe_ref_locked(pipe);
-   simple_mtx_unlock(&table_lock);
+   simple_mtx_unlock(&fence_lock);
    return pipe;
 }
 
 struct fd_pipe *
 fd_pipe_ref_locked(struct fd_pipe *pipe)
 {
-   simple_mtx_assert_locked(&table_lock);
+   simple_mtx_assert_locked(&fence_lock);
    pipe->refcnt++;
    return pipe;
 }
 
-
-static void
-pipe_del_locked(struct fd_pipe *pipe)
-{
-   fd_bo_del_locked(pipe->control_mem);
-   fd_device_del_locked(pipe->dev);
-   pipe->funcs->destroy(pipe);
-}
-
 void
 fd_pipe_del(struct fd_pipe *pipe)
 {
-   if (!p_atomic_dec_zero(&pipe->refcnt))
-      return;
-   simple_mtx_lock(&table_lock);
-   pipe_del_locked(pipe);
-   simple_mtx_unlock(&table_lock);
+   simple_mtx_lock(&fence_lock);
+   fd_pipe_del_locked(pipe);
+   simple_mtx_unlock(&fence_lock);
 }
 
 void
 fd_pipe_del_locked(struct fd_pipe *pipe)
 {
-   simple_mtx_assert_locked(&table_lock);
-   if (!p_atomic_dec_zero(&pipe->refcnt))
+   simple_mtx_assert_locked(&fence_lock);
+   if (--pipe->refcnt)
       return;
-   pipe_del_locked(pipe);
+
+   fd_bo_del(pipe->control_mem);
+   fd_device_del(pipe->dev);
+   pipe->funcs->destroy(pipe);
 }
 
 /**

src/freedreno/drm/freedreno_priv.h

@@ -52,6 +52,7 @@
 #include "freedreno_ringbuffer.h"
 
 extern simple_mtx_t table_lock;
+extern simple_mtx_t fence_lock;
 
 #define SUBALLOC_SIZE (32 * 1024)
 /* Maximum known alignment requirement is a6xx's TEX_CONST at 16 dwords */
@@ -119,6 +120,7 @@ struct fd_bo_bucket {
 
 struct fd_bo_cache {
    const char *name;
+   simple_mtx_t lock;
    struct fd_bo_bucket cache_bucket[14 * 4];
    int num_buckets;
    time_t time;
@@ -216,7 +218,7 @@ int fd_bo_cache_free(struct fd_bo_cache *cache, struct fd_bo *bo);
 
 /* for where @table_lock is already held: */
 void fd_bo_del_locked(struct fd_bo *bo);
-void fd_device_del_locked(struct fd_device *dev);
+/* for where @fence_lock is already held: */
 void fd_pipe_del_locked(struct fd_pipe *pipe);
 
 struct fd_pipe_funcs {
@@ -251,8 +253,8 @@ struct fd_pipe {
    struct fd_dev_id dev_id;
 
    /**
-    * Note refcnt is *not* atomic, but protected by table_lock, since the
-    * table_lock is held in fd_bo_add_fence(), which is the hotpath.
+    * Note refcnt is *not* atomic, but protected by fence_lock, since the
+    * fence_lock is held in fd_bo_add_fence(), which is the hotpath.
     */
    int32_t refcnt;
 

src/freedreno/drm/freedreno_ringbuffer_sp.c

@@ -179,12 +179,12 @@ fd_submit_sp_flush_prep(struct fd_submit *submit, int in_fence_fd,
    for (unsigned i = 0; i < primary->u.nr_cmds; i++)
       fd_submit_append_bo(fd_submit, primary->u.cmds[i].ring_bo);
 
-   simple_mtx_lock(&table_lock);
+   simple_mtx_lock(&fence_lock);
    for (unsigned i = 0; i < fd_submit->nr_bos; i++) {
       fd_bo_add_fence(fd_submit->bos[i], submit->pipe, submit->fence);
       has_shared |= fd_submit->bos[i]->shared;
    }
-   simple_mtx_unlock(&table_lock);
+   simple_mtx_unlock(&fence_lock);
 
    fd_submit->out_fence   = out_fence;
    fd_submit->in_fence_fd = (in_fence_fd == -1) ?

src/freedreno/drm/msm/msm_ringbuffer.c

@@ -342,11 +342,11 @@ msm_submit_flush(struct fd_submit *submit, int in_fence_fd,
       }
    }
 
-   simple_mtx_lock(&table_lock);
+   simple_mtx_lock(&fence_lock);
    for (unsigned j = 0; j < msm_submit->nr_bos; j++) {
       fd_bo_add_fence(msm_submit->bos[j], submit->pipe, submit->fence);
    }
-   simple_mtx_unlock(&table_lock);
+   simple_mtx_unlock(&fence_lock);
 
    if (in_fence_fd != -1) {
       req.flags |= MSM_SUBMIT_FENCE_FD_IN | MSM_SUBMIT_NO_IMPLICIT;