From b4b0193e83cb987143583e2b4011b35331f429bd Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Tue, 27 Feb 2024 11:14:57 +0100 Subject: drm/fbdev-generic: Fix locking with drm_client_buffer_vmap_local() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Christian König Reviewed-by: Dmitry Osipenko Tested-by: Dmitry Osipenko # virtio-gpu Acked-by: Zack Rusin Link: https://patchwork.freedesktop.org/patch/msgid/20240227113853.8464-11-tzimmermann@suse.de --- drivers/gpu/drm/drm_client.c | 68 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 60 insertions(+), 8 deletions(-) (limited to 'drivers/gpu/drm/drm_client.c') 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 @@ -304,6 +304,66 @@ err_delete: return ERR_PTR(ret); } +/** + * 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 @@ -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; -- cgit v1.2.3-59-g8ed1b From 1709474ba04179bee919f920c4da877aa1552b41 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Tue, 27 Feb 2024 11:14:58 +0100 Subject: drm/client: Pin vmap'ed GEM buffers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function drm_client_buffer_vmap() establishes a long-term mapping of the client's buffer object into the kernel address space. Make sure that buffer does not move by pinning it to its current location. Same for vunmap with unpin. The only caller of drm_client_buffer_vmap() is fbdev-dma, which uses gem-dma. As DMA-backed GEM buffers do not move, this change is for correctness with little impact in practice. Signed-off-by: Thomas Zimmermann Reviewed-by: Christian König Reviewed-by: Dmitry Osipenko Tested-by: Dmitry Osipenko # virtio-gpu Acked-by: Zack Rusin Link: https://patchwork.freedesktop.org/patch/msgid/20240227113853.8464-12-tzimmermann@suse.de --- drivers/gpu/drm/drm_client.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/drm_client.c') diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index 2cc81831236b..77fe217aeaf3 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -388,16 +388,30 @@ int drm_client_buffer_vmap(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; - ret = drm_gem_vmap_unlocked(buffer->gem, map); + drm_gem_lock(gem); + + ret = drm_gem_pin_locked(gem); if (ret) - return ret; + goto err_drm_gem_pin_locked; + ret = drm_gem_vmap(gem, map); + if (ret) + goto err_drm_gem_vmap; + + drm_gem_unlock(gem); *map_copy = *map; return 0; + +err_drm_gem_vmap: + drm_gem_unpin_locked(buffer->gem); +err_drm_gem_pin_locked: + drm_gem_unlock(gem); + return ret; } EXPORT_SYMBOL(drm_client_buffer_vmap); @@ -411,9 +425,13 @@ EXPORT_SYMBOL(drm_client_buffer_vmap); */ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer) { + struct drm_gem_object *gem = buffer->gem; struct iosys_map *map = &buffer->map; - drm_gem_vunmap_unlocked(buffer->gem, map); + drm_gem_lock(gem); + drm_gem_vunmap(gem, map); + drm_gem_unpin_locked(gem); + drm_gem_unlock(gem); } EXPORT_SYMBOL(drm_client_buffer_vunmap); -- cgit v1.2.3-59-g8ed1b From acc29d5095b01c0eda6a7b4948a805ce699523e3 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Tue, 9 Apr 2024 10:04:23 +0200 Subject: drm/client: Export drm_client_dev_unregister() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Export drm_client_dev_unregister() for use by the i915 driver. The driver does not use drm_dev_unregister(), so it has to clean up the in-kernel DRM clients by itself. Signed-off-by: Thomas Zimmermann Reviewed-by: Jouni Högander Acked-by: Lucas De Marchi Acked-by: Thomas Zimmermann Link: https://patchwork.freedesktop.org/patch/msgid/20240409081029.17843-2-tzimmermann@suse.de Signed-off-by: Jani Nikula --- drivers/gpu/drm/drm_client.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'drivers/gpu/drm/drm_client.c') diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index 9403b3f576f7..3d4f8b77d078 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -172,6 +172,18 @@ void drm_client_release(struct drm_client_dev *client) } EXPORT_SYMBOL(drm_client_release); +/** + * drm_client_dev_unregister - Unregister clients + * @dev: DRM device + * + * This function releases all clients by calling each client's + * &drm_client_funcs.unregister callback. The callback function + * is responsibe for releaseing all resources including the client + * itself. + * + * The helper drm_dev_unregister() calls this function. Drivers + * that use it don't need to call this function themselves. + */ void drm_client_dev_unregister(struct drm_device *dev) { struct drm_client_dev *client, *tmp; @@ -191,6 +203,7 @@ void drm_client_dev_unregister(struct drm_device *dev) } mutex_unlock(&dev->clientlist_mutex); } +EXPORT_SYMBOL(drm_client_dev_unregister); /** * drm_client_dev_hotplug - Send hotplug event to clients -- cgit v1.2.3-59-g8ed1b