Re: [PATCH v4 11/13] drm/vboxvideo: Use drm_gem_vram_vmap_local() in cursor update

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

 



Hi

Am 12.01.21 um 10:17 schrieb Daniel Vetter:
On Tue, Jan 12, 2021 at 08:54:02AM +0100, Thomas Zimmermann wrote:
Hi

Am 11.01.21 um 18:06 schrieb Daniel Vetter:
On Fri, Jan 08, 2021 at 10:43:38AM +0100, Thomas Zimmermann wrote:
Cursor updates in vboxvideo require a short-term mapping of the
source BO. Use drm_gem_vram_vmap_local() and avoid the pinning
operations.

Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>

All these drivers patches break the dma_resv_lock vs
dma_fence_begin/end_signalling nesting rules, so this doesn't work.

Generally this is what the prepare/cleanup_fb hooks are for, that's where
mappings (including vmaps) are meant to be set up, permanently.

I'm kinda not clear on why we need all these changes, I thought the
locking problem is just in the fb helper paths, because it's outside of
the atomic path and could conflict with an atomic update at the same time?
So only that one should get the vmap_local treatment, everything else
should keep the normal vmap treatment.

Kind of responding to all your comment on the driver changes:

These drivers only require short-term mappings, so using vmap_local would be
the natural choice. For SHMEM helpers, it's mostly a cosmetic thing. For
VRAM helpers, I was hoping to remove the vmap/vunmap helpers entirely. One
cannot really map the BOs for the long-term, so not having the helpers at
all would make sense.

But reading all your comments on the driver patches, I'd rather not update
the drivers here but later convert them to use prepare_fb/cleanup_fb in the
correct way.

Ack from me on this plan. I think I got all the other patches with an r-b
or ack?

The shmem patch needs an update from my side.

Best regards
Thomas

-Daniel


Best regards
Thomas

-Daniel
---
   drivers/gpu/drm/vboxvideo/vbox_mode.c | 15 +++++++++------
   1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
index dbc0dd53c69e..215b37c78c10 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
@@ -381,7 +381,8 @@ 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]);
+	struct drm_gem_object *obj = fb->obj[0];
+	struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(obj);
   	u32 width = plane->state->crtc_w;
   	u32 height = plane->state->crtc_h;
   	size_t data_size, mask_size;
@@ -401,11 +402,12 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
   	vbox_crtc->cursor_enabled = true;
-	ret = drm_gem_vram_vmap(gbo, &map);
+	ret = dma_resv_lock(obj->resv, NULL);
+	if (ret)
+		return;
+	ret = drm_gem_vram_vmap_local(gbo, &map);
   	if (ret) {
-		/*
-		 * BUG: we should have pinned the BO in prepare_fb().
-		 */
+		dma_resv_unlock(obj->resv);
   		mutex_unlock(&vbox->hw_mutex);
   		DRM_WARN("Could not map cursor bo, skipping update\n");
   		return;
@@ -421,7 +423,8 @@ 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);
+	drm_gem_vram_vunmap_local(gbo, &map);
+	dma_resv_unlock(obj->resv);
   	flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE |
   		VBOX_MOUSE_POINTER_ALPHA;
--
2.29.2



--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer






--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

Attachment: OpenPGP_signature
Description: OpenPGP digital signature

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux