On 3/15/21 8:48 AM, Thomas Zimmermann wrote:
Hi
Am 13.03.21 um 19:29 schrieb Thomas Hellström (Intel):
Hi, Christian
On 3/12/21 10:38 AM, Christian König wrote:
We seem to have some more driver bugs than thought.
Signed-off-by: Christian König <christian.koenig@xxxxxxx>
---
include/drm/ttm/ttm_bo_api.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/drm/ttm/ttm_bo_api.h
b/include/drm/ttm/ttm_bo_api.h
index 4fb523dfab32..df9fe596e7c5 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -603,9 +603,11 @@ static inline void ttm_bo_pin(struct
ttm_buffer_object *bo)
static inline void ttm_bo_unpin(struct ttm_buffer_object *bo)
{
dma_resv_assert_held(bo->base.resv);
- WARN_ON_ONCE(!bo->pin_count);
WARN_ON_ONCE(!kref_read(&bo->kref));
- --bo->pin_count;
+ if (bo->pin_count)
+ --bo->pin_count;
+ else
+ WARN_ON_ONCE(true);
}
int ttm_mem_evict_first(struct ttm_device *bdev,
Since I now have been staring for half a year at the code of the
driver that made pinning an art, I have a couple of suggestions here,
Could we use an atomic for pin_count, allowing unlocked unpinning but
require the lock only for pin_count transition 0->1, (but unlocked
for pin_if_already_pinned). In particular I think vmwgfx would
benefit from unlocked unpins. Also if the atomic were a refcount_t,
that would probably give you the above behaviour?
What's the benefit?
I'm asking because, there's been talk about streamlining all the GEM
locking and actually allowing dma-buf resv locking in pin and vmap
operations. Atomic ops might not contradict this, but also might not
be useful in the long term.
The benefit would be that at unpinning time it might be tricky to take
the reservation lock, because you might be already in a ww transaction.
But Christian pointed out the LRU complications...
/Thomas
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
