[v3,2/2] drm/vboxvideo: Implement cursor plane with struct drm_shadow_plane_state

Commit Message

Thomas Zimmermann Feb. 9, 2021, 12:10 p.m. UTC

Functions in the atomic commit tail are not allowed to acquire the
dmabuf's reservation lock. So we cannot legally call the GEM object's
vmap functionality in atomic_update.

Instead use struct drm_shadow_plane_state and friends. It vmaps the
framebuffer BOs in prepare_fb and vunmaps them in cleanup_fb. The
cursor plane state stores the mapping's address. The pinning of the
BO is implicitly done by vmap.

As an extra benefit, there's no source of runtime errors left in
atomic_update.

v2:
	* rebase patch onto struct drm_shadow_plane_state

Signed-off-by: Thomas Zimmermann <[e-mail]>
Reviewed-by: Hans de Goede <[e-mail]>
Tested-by: Hans de Goede <[e-mail]>
---
 drivers/gpu/drm/vboxvideo/vbox_mode.c | 28 +++++++--------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

Patch

diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
index dbc0dd53c69e..6e4ad966be71 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
@@ -17,6 +17,7 @@ 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_fourcc.h>
+#include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
@@ -381,14 +382,14 @@  static void vbox_cursor_atomic_update(struct drm_plane *plane,
 		container_of(plane->dev, struct vbox_private, ddev);
 	struct vbox_crtc *vbox_crtc = to_vbox_crtc(plane->state->crtc);
 	struct drm_framebuffer *fb = plane->state->fb;
-	struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(fb->obj[0]);
 	u32 width = plane->state->crtc_w;
 	u32 height = plane->state->crtc_h;
+	struct drm_shadow_plane_state *shadow_plane_state =
+		to_drm_shadow_plane_state(plane->state);
+	struct dma_buf_map map = shadow_plane_state->map[0];
+	u8 *src = map.vaddr; /* TODO: Use mapping abstraction properly */
 	size_t data_size, mask_size;
 	u32 flags;
-	struct dma_buf_map map;
-	int ret;
-	u8 *src;
 
 	/*
 	 * VirtualBox uses the host windowing system to draw the cursor so
@@ -401,17 +402,6 @@  static void vbox_cursor_atomic_update(struct drm_plane *plane,
 
 	vbox_crtc->cursor_enabled = true;
 
-	ret = drm_gem_vram_vmap(gbo, &map);
-	if (ret) {
-		/*
-		 * BUG: we should have pinned the BO in prepare_fb().
-		 */
-		mutex_unlock(&vbox->hw_mutex);
-		DRM_WARN("Could not map cursor bo, skipping update\n");
-		return;
-	}
-	src = map.vaddr; /* TODO: Use mapping abstraction properly */
-
 	/*
 	 * The mask must be calculated based on the alpha
 	 * channel, one bit per ARGB word, and must be 32-bit
@@ -421,7 +411,6 @@  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_vunmap(gbo, &map);
 
 	flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE |
 		VBOX_MOUSE_POINTER_ALPHA;
@@ -466,17 +455,14 @@  static const struct drm_plane_helper_funcs vbox_cursor_helper_funcs = {
 	.atomic_check	= vbox_cursor_atomic_check,
 	.atomic_update	= vbox_cursor_atomic_update,
 	.atomic_disable	= vbox_cursor_atomic_disable,
-	.prepare_fb	= drm_gem_vram_plane_helper_prepare_fb,
-	.cleanup_fb	= drm_gem_vram_plane_helper_cleanup_fb,
+	DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
 };
 
 static const struct drm_plane_funcs vbox_cursor_plane_funcs = {
 	.update_plane	= drm_atomic_helper_update_plane,
 	.disable_plane	= drm_atomic_helper_disable_plane,
 	.destroy	= drm_primary_helper_destroy,
-	.reset		= drm_atomic_helper_plane_reset,
-	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
+	DRM_GEM_SHADOW_PLANE_FUNCS,
 };
 
 static const u32 vbox_primary_plane_formats[] = {