On 7/31/23 15:27, Dmitry Osipenko wrote: > On 7/25/23 11:32, Boris Brezillon wrote: >>> Can we make it an atomic_t, so we can avoid taking the lock when the >>> GEM has already been pinned. That's something I need to be able to grab >>> a pin-ref in a path where the GEM resv lock is already held[1]. We could >>> of course expose the locked version, >> My bad, that's actually not true. The problem is not that I call >> drm_gem_shmem_pin() with the resv lock already held, but that I call >> drm_gem_shmem_pin() in a dma-signaling path where I'm not allowed to >> take a resv lock. I know for sure pin_count > 0, because all GEM objects >> mapped to a VM have their memory pinned right now, and this should >> stand until we decide to add support for live-GEM eviction, at which >> point we'll probably have a way to detect when a GEM is evicted, and >> avoid calling drm_gem_shmem_pin() on it. >> >> TLDR; I can't trade the atomic_t for a drm_gem_shmem_pin_locked(), >> because that wouldn't solve my problem. The other solution would be to >> add an atomic_t at the driver-GEM level, and only call >> drm_gem_shmem_[un]pin() on 0 <-> 1 transitions, but I thought using an >> atomic at the GEM-shmem level, to avoid locking when we can, would be >> beneficial to the rest of the eco-system. Let me know if that's not an >> option, and I'll go back to the driver-specific atomic_t. > > Could you please explain why do you need to pin GEM in a signal handler? > This is not something drivers usually do or need to do. You likely also > shouldn't need to detect that GEM is evicted in yours driver. I'd expect > that Panthor shouldn't differ from Panfrost in regards to how GEM memory > management is done and Panfrost doesn't need to do anything special. > > Note that patch #14 makes locked pin/unpin functions public and turns > the unlocked variants into helpers, you'll be able to experiment with > these funcs in the Panthor driver. correction: that's patch #10 > In general, using atomic_t or kref should be a good thing to do, but > AFAICS it shouldn't bring benefits to the today's drm-shmem users. I'd > want to understand what you're trying to achieve in the Panthor driver. > -- Best regards, Dmitry
