freedreno/drm: Move suballoc_bo to device
Having it in msm_pipe isn't saving any locking. But it does mean that cleanup_fences() can drop the last pipe reference, which in turn drops the last suballoc_bo reference, which can cause recursion back into the bo cache. Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/5562 Signed-off-by: Rob Clark <robdclark@chromium.org> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13521>
This commit is contained in:
parent
2c6fb9780c
commit
d2a7afe34d
|
@ -86,6 +86,7 @@ out:
|
|||
|
||||
list_inithead(&dev->deferred_submits);
|
||||
simple_mtx_init(&dev->submit_lock, mtx_plain);
|
||||
simple_mtx_init(&dev->suballoc_lock, mtx_plain);
|
||||
|
||||
return dev;
|
||||
}
|
||||
|
@ -130,6 +131,9 @@ fd_device_del_impl(struct fd_device *dev)
|
|||
|
||||
assert(list_is_empty(&dev->deferred_submits));
|
||||
|
||||
if (dev->suballoc_bo)
|
||||
fd_bo_del_locked(dev->suballoc_bo);
|
||||
|
||||
fd_bo_cache_cleanup(&dev->bo_cache, 0);
|
||||
fd_bo_cache_cleanup(&dev->ring_cache, 0);
|
||||
_mesa_hash_table_destroy(dev->handle_table, NULL);
|
||||
|
|
|
@ -148,6 +148,23 @@ struct fd_device {
|
|||
struct list_head deferred_submits;
|
||||
unsigned deferred_cmds;
|
||||
simple_mtx_t submit_lock;
|
||||
|
||||
/**
|
||||
* BO for suballocating long-lived state objects.
|
||||
*
|
||||
* Note: one would be tempted to put this in fd_pipe to avoid locking.
|
||||
* But that is a bad idea for a couple of reasons:
|
||||
*
|
||||
* 1) With TC, stateobj allocation can happen in either frontend thread
|
||||
* (ie. most CSOs), and also driver thread (a6xx cached tex state)
|
||||
* 2) It is best for fd_pipe to not hold a reference to a BO that can
|
||||
* be free'd to bo cache, as that can cause unexpected re-entrancy
|
||||
* (fd_bo_cache_alloc() -> find_in_bucket() -> fd_bo_state() ->
|
||||
* cleanup_fences() -> drop pipe ref which free's bo's).
|
||||
*/
|
||||
struct fd_bo *suballoc_bo;
|
||||
uint32_t suballoc_offset;
|
||||
simple_mtx_t suballoc_lock;
|
||||
};
|
||||
|
||||
#define foreach_submit(name, list) \
|
||||
|
|
|
@ -172,9 +172,6 @@ msm_pipe_destroy(struct fd_pipe *pipe)
|
|||
{
|
||||
struct msm_pipe *msm_pipe = to_msm_pipe(pipe);
|
||||
|
||||
if (msm_pipe->suballoc_bo)
|
||||
fd_bo_del_locked(msm_pipe->suballoc_bo);
|
||||
|
||||
close_submitqueue(pipe, msm_pipe->queue_id);
|
||||
msm_pipe_sp_ringpool_init(msm_pipe);
|
||||
free(msm_pipe);
|
||||
|
|
|
@ -58,10 +58,6 @@ struct msm_pipe {
|
|||
uint32_t queue_id;
|
||||
struct slab_parent_pool ring_pool;
|
||||
|
||||
/* BO for suballocating long-lived objects on the pipe. */
|
||||
struct fd_bo *suballoc_bo;
|
||||
uint32_t suballoc_offset;
|
||||
|
||||
/**
|
||||
* The last fence seqno that was flushed to kernel (doesn't mean that it
|
||||
* is complete, just that the kernel knows about it)
|
||||
|
|
|
@ -825,34 +825,33 @@ msm_ringbuffer_sp_init(struct msm_ringbuffer_sp *msm_ring, uint32_t size,
|
|||
struct fd_ringbuffer *
|
||||
msm_ringbuffer_sp_new_object(struct fd_pipe *pipe, uint32_t size)
|
||||
{
|
||||
struct msm_pipe *msm_pipe = to_msm_pipe(pipe);
|
||||
struct fd_device *dev = pipe->dev;
|
||||
struct msm_ringbuffer_sp *msm_ring = malloc(sizeof(*msm_ring));
|
||||
|
||||
/* Lock access to the msm_pipe->suballoc_* since ringbuffer object allocation
|
||||
* can happen both on the frontend (most CSOs) and the driver thread (a6xx
|
||||
* cached tex state, for example)
|
||||
*/
|
||||
static simple_mtx_t suballoc_lock = _SIMPLE_MTX_INITIALIZER_NP;
|
||||
simple_mtx_lock(&suballoc_lock);
|
||||
simple_mtx_lock(&dev->suballoc_lock);
|
||||
|
||||
/* Maximum known alignment requirement is a6xx's TEX_CONST at 16 dwords */
|
||||
msm_ring->offset = align(msm_pipe->suballoc_offset, 64);
|
||||
if (!msm_pipe->suballoc_bo ||
|
||||
msm_ring->offset + size > fd_bo_size(msm_pipe->suballoc_bo)) {
|
||||
if (msm_pipe->suballoc_bo)
|
||||
fd_bo_del(msm_pipe->suballoc_bo);
|
||||
msm_pipe->suballoc_bo =
|
||||
fd_bo_new_ring(pipe->dev, MAX2(SUBALLOC_SIZE, align(size, 4096)));
|
||||
msm_ring->offset = align(dev->suballoc_offset, 64);
|
||||
if (!dev->suballoc_bo ||
|
||||
msm_ring->offset + size > fd_bo_size(dev->suballoc_bo)) {
|
||||
if (dev->suballoc_bo)
|
||||
fd_bo_del(dev->suballoc_bo);
|
||||
dev->suballoc_bo =
|
||||
fd_bo_new_ring(dev, MAX2(SUBALLOC_SIZE, align(size, 4096)));
|
||||
msm_ring->offset = 0;
|
||||
}
|
||||
|
||||
msm_ring->u.pipe = pipe;
|
||||
msm_ring->ring_bo = fd_bo_ref(msm_pipe->suballoc_bo);
|
||||
msm_ring->ring_bo = fd_bo_ref(dev->suballoc_bo);
|
||||
msm_ring->base.refcnt = 1;
|
||||
|
||||
msm_pipe->suballoc_offset = msm_ring->offset + size;
|
||||
dev->suballoc_offset = msm_ring->offset + size;
|
||||
|
||||
simple_mtx_unlock(&suballoc_lock);
|
||||
simple_mtx_unlock(&dev->suballoc_lock);
|
||||
|
||||
return msm_ringbuffer_sp_init(msm_ring, size, _FD_RINGBUFFER_OBJECT);
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue