Christian König
2022-Aug-09 12:22 UTC
[Linaro-mm-sig] [PATCH v2 2/5] drm/gem: Take reservation lock for vmap/vunmap operations
Am 25.07.22 um 17:18 schrieb Dmitry Osipenko:> The new common dma-buf locking convention will require buffer importers > to hold the reservation lock around mapping operations. Make DRM GEM core > to take the lock around the vmapping operations and update QXL and i915 > drivers to use the locked functions for the case where DRM core now holds > the lock. This patch prepares DRM core and drivers to transition to the > common dma-buf locking convention where vmapping of exported GEMs will > be done under the held reservation lock. > > Signed-off-by: Dmitry Osipenko <dmitry.osipenko at collabora.com>Reviewed-by: Christian K?nig <christian.koenig at amd.com>> --- > drivers/gpu/drm/drm_client.c | 4 ++-- > drivers/gpu/drm/drm_gem.c | 24 ++++++++++++++++++++ > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 6 ++--- > drivers/gpu/drm/drm_prime.c | 4 ++-- > drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 2 +- > drivers/gpu/drm/qxl/qxl_object.c | 17 +++++++------- > drivers/gpu/drm/qxl/qxl_prime.c | 4 ++-- > include/drm/drm_gem.h | 3 +++ > 8 files changed, 46 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c > index af3b7395bf69..e9a1cd310352 100644 > --- a/drivers/gpu/drm/drm_client.c > +++ b/drivers/gpu/drm/drm_client.c > @@ -323,7 +323,7 @@ drm_client_buffer_vmap(struct drm_client_buffer *buffer, > * fd_install step out of the driver backend hooks, to make that > * final step optional for internal users. > */ > - ret = drm_gem_vmap(buffer->gem, map); > + ret = drm_gem_vmap_unlocked(buffer->gem, map); > if (ret) > return ret; > > @@ -345,7 +345,7 @@ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer) > { > struct iosys_map *map = &buffer->map; > > - drm_gem_vunmap(buffer->gem, map); > + drm_gem_vunmap_unlocked(buffer->gem, map); > } > EXPORT_SYMBOL(drm_client_buffer_vunmap); > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index eb0c2d041f13..8b92846112ef 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -1171,6 +1171,8 @@ int drm_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map) > { > int ret; > > + dma_resv_assert_held(obj->resv); > + > if (!obj->funcs->vmap) > return -EOPNOTSUPP; > > @@ -1186,6 +1188,8 @@ EXPORT_SYMBOL(drm_gem_vmap); > > void drm_gem_vunmap(struct drm_gem_object *obj, struct iosys_map *map) > { > + dma_resv_assert_held(obj->resv); > + > if (iosys_map_is_null(map)) > return; > > @@ -1197,6 +1201,26 @@ void drm_gem_vunmap(struct drm_gem_object *obj, struct iosys_map *map) > } > EXPORT_SYMBOL(drm_gem_vunmap); > > +int drm_gem_vmap_unlocked(struct drm_gem_object *obj, struct iosys_map *map) > +{ > + int ret; > + > + dma_resv_lock(obj->resv, NULL); > + ret = drm_gem_vmap(obj, map); > + dma_resv_unlock(obj->resv); > + > + return ret; > +} > +EXPORT_SYMBOL(drm_gem_vmap_unlocked); > + > +void drm_gem_vunmap_unlocked(struct drm_gem_object *obj, struct iosys_map *map) > +{ > + dma_resv_lock(obj->resv, NULL); > + drm_gem_vunmap(obj, map); > + dma_resv_unlock(obj->resv); > +} > +EXPORT_SYMBOL(drm_gem_vunmap_unlocked); > + > /** > * drm_gem_lock_reservations - Sets up the ww context and acquires > * the lock on an array of GEM objects. > diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c > index 61339a9cd010..135cd4a96ea9 100644 > --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c > +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c > @@ -354,7 +354,7 @@ int drm_gem_fb_vmap(struct drm_framebuffer *fb, struct iosys_map *map, > ret = -EINVAL; > goto err_drm_gem_vunmap; > } > - ret = drm_gem_vmap(obj, &map[i]); > + ret = drm_gem_vmap_unlocked(obj, &map[i]); > if (ret) > goto err_drm_gem_vunmap; > } > @@ -376,7 +376,7 @@ int drm_gem_fb_vmap(struct drm_framebuffer *fb, struct iosys_map *map, > obj = drm_gem_fb_get_obj(fb, i); > if (!obj) > continue; > - drm_gem_vunmap(obj, &map[i]); > + drm_gem_vunmap_unlocked(obj, &map[i]); > } > return ret; > } > @@ -403,7 +403,7 @@ void drm_gem_fb_vunmap(struct drm_framebuffer *fb, struct iosys_map *map) > continue; > if (iosys_map_is_null(&map[i])) > continue; > - drm_gem_vunmap(obj, &map[i]); > + drm_gem_vunmap_unlocked(obj, &map[i]); > } > } > EXPORT_SYMBOL(drm_gem_fb_vunmap); > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index b75ef1756873..1bd234fd21a5 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -678,7 +678,7 @@ int drm_gem_dmabuf_vmap(struct dma_buf *dma_buf, struct iosys_map *map) > { > struct drm_gem_object *obj = dma_buf->priv; > > - return drm_gem_vmap(obj, map); > + return drm_gem_vmap_unlocked(obj, map); > } > EXPORT_SYMBOL(drm_gem_dmabuf_vmap); > > @@ -694,7 +694,7 @@ void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, struct iosys_map *map) > { > struct drm_gem_object *obj = dma_buf->priv; > > - drm_gem_vunmap(obj, map); > + drm_gem_vunmap_unlocked(obj, map); > } > EXPORT_SYMBOL(drm_gem_dmabuf_vunmap); > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c > index 5ecea7df98b1..cc54a5b1d6ae 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c > @@ -72,7 +72,7 @@ static int i915_gem_dmabuf_vmap(struct dma_buf *dma_buf, > struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf); > void *vaddr; > > - vaddr = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB); > + vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB); > if (IS_ERR(vaddr)) > return PTR_ERR(vaddr); > > diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c > index b42a657e4c2f..a64cd635fbc0 100644 > --- a/drivers/gpu/drm/qxl/qxl_object.c > +++ b/drivers/gpu/drm/qxl/qxl_object.c > @@ -168,9 +168,16 @@ int qxl_bo_vmap_locked(struct qxl_bo *bo, struct iosys_map *map) > bo->map_count++; > goto out; > } > - r = ttm_bo_vmap(&bo->tbo, &bo->map); > + > + r = __qxl_bo_pin(bo); > if (r) > return r; > + > + r = ttm_bo_vmap(&bo->tbo, &bo->map); > + if (r) { > + __qxl_bo_unpin(bo); > + return r; > + } > bo->map_count = 1; > > /* TODO: Remove kptr in favor of map everywhere. */ > @@ -192,12 +199,6 @@ int qxl_bo_vmap(struct qxl_bo *bo, struct iosys_map *map) > if (r) > return r; > > - r = __qxl_bo_pin(bo); > - if (r) { > - qxl_bo_unreserve(bo); > - return r; > - } > - > r = qxl_bo_vmap_locked(bo, map); > qxl_bo_unreserve(bo); > return r; > @@ -247,6 +248,7 @@ void qxl_bo_vunmap_locked(struct qxl_bo *bo) > return; > bo->kptr = NULL; > ttm_bo_vunmap(&bo->tbo, &bo->map); > + __qxl_bo_unpin(bo); > } > > int qxl_bo_vunmap(struct qxl_bo *bo) > @@ -258,7 +260,6 @@ int qxl_bo_vunmap(struct qxl_bo *bo) > return r; > > qxl_bo_vunmap_locked(bo); > - __qxl_bo_unpin(bo); > qxl_bo_unreserve(bo); > return 0; > } > diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c > index 142d01415acb..9169c26357d3 100644 > --- a/drivers/gpu/drm/qxl/qxl_prime.c > +++ b/drivers/gpu/drm/qxl/qxl_prime.c > @@ -59,7 +59,7 @@ int qxl_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map) > struct qxl_bo *bo = gem_to_qxl_bo(obj); > int ret; > > - ret = qxl_bo_vmap(bo, map); > + ret = qxl_bo_vmap_locked(bo, map); > if (ret < 0) > return ret; > > @@ -71,5 +71,5 @@ void qxl_gem_prime_vunmap(struct drm_gem_object *obj, > { > struct qxl_bo *bo = gem_to_qxl_bo(obj); > > - qxl_bo_vunmap(bo); > + qxl_bo_vunmap_locked(bo); > } > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > index 87cffc9efa85..bf3700415229 100644 > --- a/include/drm/drm_gem.h > +++ b/include/drm/drm_gem.h > @@ -420,4 +420,7 @@ void drm_gem_unlock_reservations(struct drm_gem_object **objs, int count, > int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, > u32 handle, u64 *offset); > > +int drm_gem_vmap_unlocked(struct drm_gem_object *obj, struct iosys_map *map); > +void drm_gem_vunmap_unlocked(struct drm_gem_object *obj, struct iosys_map *map); > + > #endif /* __DRM_GEM_H__ */