diff options
author | Thomas Zimmermann <tzimmermann@suse.de> | 2024-02-27 11:14:57 +0100 |
---|---|---|
committer | Thomas Zimmermann <tzimmermann@suse.de> | 2024-03-11 13:33:50 +0100 |
commit | b4b0193e83cb987143583e2b4011b35331f429bd (patch) | |
tree | 6db24df3102ca1a5f764fbcacc52b18d0cfba76b | |
parent | a78027847226493ea6f09a00875fa4871fd29e69 (diff) | |
download | linux-b4b0193e83cb987143583e2b4011b35331f429bd.tar.gz linux-b4b0193e83cb987143583e2b4011b35331f429bd.tar.bz2 linux-b4b0193e83cb987143583e2b4011b35331f429bd.zip |
drm/fbdev-generic: Fix locking with drm_client_buffer_vmap_local()
Temporarily lock the fbdev buffer object during updates to prevent
memory managers from evicting/moving the buffer. Moving a buffer
object while update its content results in undefined behaviour.
Fbdev-generic updates its buffer object from a shadow buffer. Gem-shmem
and gem-dma helpers do not move buffer objects, so they are safe to be
used with fbdev-generic. Gem-vram and qxl are based on TTM, but pin
buffer objects are part of the vmap operation. So both are also safe
to be used with fbdev-generic.
Amdgpu and nouveau do not pin or lock the buffer object during an
update. Their TTM-based memory management could move the buffer object
while the update is ongoing.
The new vmap_local and vunmap_local helpers hold the buffer object's
reservation lock during the buffer update. This prevents moving the
buffer object on all memory managers.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> # virtio-gpu
Acked-by: Zack Rusin <zack.rusin@broadcom.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240227113853.8464-11-tzimmermann@suse.de
-rw-r--r-- | drivers/gpu/drm/drm_client.c | 68 | ||||
-rw-r--r-- | drivers/gpu/drm/drm_fbdev_generic.c | 4 | ||||
-rw-r--r-- | drivers/gpu/drm/drm_gem.c | 12 | ||||
-rw-r--r-- | include/drm/drm_client.h | 10 | ||||
-rw-r--r-- | include/drm/drm_gem.h | 3 |
5 files changed, 87 insertions, 10 deletions
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index 9403b3f576f7..2cc81831236b 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -305,6 +305,66 @@ err_delete: } /** + * drm_client_buffer_vmap_local - Map DRM client buffer into address space + * @buffer: DRM client buffer + * @map_copy: Returns the mapped memory's address + * + * This function maps a client buffer into kernel address space. If the + * buffer is already mapped, it returns the existing mapping's address. + * + * Client buffer mappings are not ref'counted. Each call to + * drm_client_buffer_vmap_local() should be closely followed by a call to + * drm_client_buffer_vunmap_local(). See drm_client_buffer_vmap() for + * long-term mappings. + * + * The returned address is a copy of the internal value. In contrast to + * other vmap interfaces, you don't need it for the client's vunmap + * function. So you can modify it at will during blit and draw operations. + * + * Returns: + * 0 on success, or a negative errno code otherwise. + */ +int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer, + struct iosys_map *map_copy) +{ + struct drm_gem_object *gem = buffer->gem; + struct iosys_map *map = &buffer->map; + int ret; + + drm_gem_lock(gem); + + ret = drm_gem_vmap(gem, map); + if (ret) + goto err_drm_gem_vmap_unlocked; + *map_copy = *map; + + return 0; + +err_drm_gem_vmap_unlocked: + drm_gem_unlock(gem); + return 0; +} +EXPORT_SYMBOL(drm_client_buffer_vmap_local); + +/** + * drm_client_buffer_vunmap_local - Unmap DRM client buffer + * @buffer: DRM client buffer + * + * This function removes a client buffer's memory mapping established + * with drm_client_buffer_vunmap_local(). Calling this function is only + * required by clients that manage their buffer mappings by themselves. + */ +void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer) +{ + struct drm_gem_object *gem = buffer->gem; + struct iosys_map *map = &buffer->map; + + drm_gem_vunmap(gem, map); + drm_gem_unlock(gem); +} +EXPORT_SYMBOL(drm_client_buffer_vunmap_local); + +/** * drm_client_buffer_vmap - Map DRM client buffer into address space * @buffer: DRM client buffer * @map_copy: Returns the mapped memory's address @@ -331,14 +391,6 @@ drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct iosys_map *map = &buffer->map; int ret; - /* - * FIXME: The dependency on GEM here isn't required, we could - * convert the driver handle to a dma-buf instead and use the - * backend-agnostic dma-buf vmap support instead. This would - * require that the handle2fd prime ioctl is reworked to pull the - * fd_install step out of the driver backend hooks, to make that - * final step optional for internal users. - */ ret = drm_gem_vmap_unlocked(buffer->gem, map); if (ret) return ret; diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index d647d89764cb..be357f926fae 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -197,14 +197,14 @@ static int drm_fbdev_generic_damage_blit(struct drm_fb_helper *fb_helper, */ mutex_lock(&fb_helper->lock); - ret = drm_client_buffer_vmap(buffer, &map); + ret = drm_client_buffer_vmap_local(buffer, &map); if (ret) goto out; dst = map; drm_fbdev_generic_damage_blit_real(fb_helper, clip, &dst); - drm_client_buffer_vunmap(buffer); + drm_client_buffer_vunmap_local(buffer); out: mutex_unlock(&fb_helper->lock); diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index e0f80c6a7096..d4bbc5d109c8 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1227,6 +1227,18 @@ void drm_gem_vunmap(struct drm_gem_object *obj, struct iosys_map *map) } EXPORT_SYMBOL(drm_gem_vunmap); +void drm_gem_lock(struct drm_gem_object *obj) +{ + dma_resv_lock(obj->resv, NULL); +} +EXPORT_SYMBOL(drm_gem_lock); + +void drm_gem_unlock(struct drm_gem_object *obj) +{ + dma_resv_unlock(obj->resv); +} +EXPORT_SYMBOL(drm_gem_unlock); + int drm_gem_vmap_unlocked(struct drm_gem_object *obj, struct iosys_map *map) { int ret; diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h index d47458ecdac4..bc0e66f9c425 100644 --- a/include/drm/drm_client.h +++ b/include/drm/drm_client.h @@ -141,6 +141,13 @@ struct drm_client_buffer { /** * @gem: GEM object backing this buffer + * + * FIXME: The dependency on GEM here isn't required, we could + * convert the driver handle to a dma-buf instead and use the + * backend-agnostic dma-buf vmap support instead. This would + * require that the handle2fd prime ioctl is reworked to pull the + * fd_install step out of the driver backend hooks, to make that + * final step optional for internal users. */ struct drm_gem_object *gem; @@ -159,6 +166,9 @@ struct drm_client_buffer * drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format); void drm_client_framebuffer_delete(struct drm_client_buffer *buffer); int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect); +int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer, + struct iosys_map *map_copy); +void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer); int drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct iosys_map *map); void drm_client_buffer_vunmap(struct drm_client_buffer *buffer); diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 2ebec3984cd4..bae4865b2101 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -527,6 +527,9 @@ struct page **drm_gem_get_pages(struct drm_gem_object *obj); void drm_gem_put_pages(struct drm_gem_object *obj, struct page **pages, bool dirty, bool accessed); +void drm_gem_lock(struct drm_gem_object *obj); +void drm_gem_unlock(struct drm_gem_object *obj); + 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); |