[PATCH] drm/vboxvideo: Use drm_gem_vram_vmap() interfaces
Hans de Goede
hdegoede at redhat.com
Fri Sep 11 14:07:43 UTC 2020
Hi,
On 9/11/20 9:59 AM, Thomas Zimmermann wrote:
> VRAM helpers support ref counting for pin and vmap operations, no need
> to avoid these operations, by employing the internal kmap interface. Just
> use drm_gem_vram_vmap() and let it handle the details.
>
> Also unexport the kmap interfaces from VRAM helpers. Vboxvideo was the
> last user of these internal functions.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
Nice cleanup.
I've given this a test-run in an actual VirtualBox vm, focussing on
cursor sprite changes and I don't see any regressions, so:
Tested-by: Hans de Goede <hdegoede at redhat.com>
Thomas, I assume that you will push this upstream yourself?
Regards,
Hans
> ---
> drivers/gpu/drm/drm_gem_vram_helper.c | 56 +--------------------------
> drivers/gpu/drm/vboxvideo/vbox_mode.c | 10 +++--
> include/drm/drm_gem_vram_helper.h | 3 --
> 3 files changed, 8 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 07447abb4134..0e3cdc40379c 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -97,8 +97,8 @@ static const struct drm_gem_object_funcs drm_gem_vram_object_funcs;
> * hardware's draing engine.
> *
> * To access a buffer object's memory from the DRM driver, call
> - * drm_gem_vram_kmap(). It (optionally) maps the buffer into kernel address
> - * space and returns the memory address. Use drm_gem_vram_kunmap() to
> + * drm_gem_vram_vmap(). It maps the buffer into kernel address
> + * space and returns the memory address. Use drm_gem_vram_vunmap() to
> * release the mapping.
> */
>
> @@ -436,39 +436,6 @@ static void *drm_gem_vram_kmap_locked(struct drm_gem_vram_object *gbo,
> return kmap->virtual;
> }
>
> -/**
> - * drm_gem_vram_kmap() - Maps a GEM VRAM object into kernel address space
> - * @gbo: the GEM VRAM object
> - * @map: establish a mapping if necessary
> - * @is_iomem: returns true if the mapped memory is I/O memory, or false \
> - otherwise; can be NULL
> - *
> - * This function maps the buffer object into the kernel's address space
> - * or returns the current mapping. If the parameter map is false, the
> - * function only queries the current mapping, but does not establish a
> - * new one.
> - *
> - * Returns:
> - * The buffers virtual address if mapped, or
> - * NULL if not mapped, or
> - * an ERR_PTR()-encoded error code otherwise.
> - */
> -void *drm_gem_vram_kmap(struct drm_gem_vram_object *gbo, bool map,
> - bool *is_iomem)
> -{
> - int ret;
> - void *virtual;
> -
> - ret = ttm_bo_reserve(&gbo->bo, true, false, NULL);
> - if (ret)
> - return ERR_PTR(ret);
> - virtual = drm_gem_vram_kmap_locked(gbo, map, is_iomem);
> - ttm_bo_unreserve(&gbo->bo);
> -
> - return virtual;
> -}
> -EXPORT_SYMBOL(drm_gem_vram_kmap);
> -
> static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
> {
> if (WARN_ON_ONCE(!gbo->kmap_use_count))
> @@ -484,22 +451,6 @@ static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
> */
> }
>
> -/**
> - * drm_gem_vram_kunmap() - Unmaps a GEM VRAM object
> - * @gbo: the GEM VRAM object
> - */
> -void drm_gem_vram_kunmap(struct drm_gem_vram_object *gbo)
> -{
> - int ret;
> -
> - ret = ttm_bo_reserve(&gbo->bo, false, false, NULL);
> - if (WARN_ONCE(ret, "ttm_bo_reserve_failed(): ret=%d\n", ret))
> - return;
> - drm_gem_vram_kunmap_locked(gbo);
> - ttm_bo_unreserve(&gbo->bo);
> -}
> -EXPORT_SYMBOL(drm_gem_vram_kunmap);
> -
> /**
> * drm_gem_vram_vmap() - Pins and maps a GEM VRAM object into kernel address
> * space
> @@ -511,9 +462,6 @@ EXPORT_SYMBOL(drm_gem_vram_kunmap);
> * permanently. Call drm_gem_vram_vunmap() with the returned address to
> * unmap and unpin the GEM VRAM object.
> *
> - * If you have special requirements for the pinning or mapping operations,
> - * call drm_gem_vram_pin() and drm_gem_vram_kmap() directly.
> - *
> * Returns:
> * The buffer's virtual address on success, or
> * an ERR_PTR()-encoded error code otherwise.
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> index d9a5af62af89..4fcc0a542b8a 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> @@ -397,11 +397,13 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
>
> vbox_crtc->cursor_enabled = true;
>
> - /* pinning is done in prepare/cleanup framebuffer */
> - src = drm_gem_vram_kmap(gbo, true, NULL);
> + src = drm_gem_vram_vmap(gbo);
> if (IS_ERR(src)) {
> + /*
> + * BUG: we should have pinned the BO in prepare_fb().
> + */
> mutex_unlock(&vbox->hw_mutex);
> - DRM_WARN("Could not kmap cursor bo, skipping update\n");
> + DRM_WARN("Could not map cursor bo, skipping update\n");
> return;
> }
>
> @@ -414,7 +416,7 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
> data_size = width * height * 4 + mask_size;
>
> copy_cursor_image(src, vbox->cursor_data, width, height, mask_size);
> - drm_gem_vram_kunmap(gbo);
> + drm_gem_vram_vunmap(gbo, src);
>
> flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE |
> VBOX_MOUSE_POINTER_ALPHA;
> diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
> index 035332f3723f..b34f1da89cc7 100644
> --- a/include/drm/drm_gem_vram_helper.h
> +++ b/include/drm/drm_gem_vram_helper.h
> @@ -101,9 +101,6 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo);
> s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo);
> int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag);
> int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo);
> -void *drm_gem_vram_kmap(struct drm_gem_vram_object *gbo, bool map,
> - bool *is_iomem);
> -void drm_gem_vram_kunmap(struct drm_gem_vram_object *gbo);
> void *drm_gem_vram_vmap(struct drm_gem_vram_object *gbo);
> void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo, void *vaddr);
>
>
More information about the dri-devel
mailing list