Re: [PATCH] drm/vboxvideo: Use drm_gem_vram_vmap() interfaces

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx>

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@xxxxxxxxxx>

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);

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux