Hi Hans Am 03.02.21 um 12:50 schrieb Hans de Goede:
If I understand things correctly, there is some memory corruption introduced by this patch, so there will be a v2 fixing this ? The reason why I'm asking is because I always try to test vboxvideo patches inside a vbox vm, but if a v2 is coming, there is not much use in me testing this version, correct ?
I just sent out v2 of the patchset. [1] It's now build upon DRM helpers, so vboxvideo got slightly smaller. For testing, the latest drm-tip or drm-misc-next is required.
Best regards Thomas[1] https://lore.kernel.org/dri-devel/20210208135044.27827-1-tzimmermann@xxxxxxx/T/#t
Regards, Hans--- drivers/gpu/drm/vboxvideo/vbox_mode.c | 102 +++++++++++++++++++++----- 1 file changed, 82 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c index dbc0dd53c69e..b5625a7d6cef 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c @@ -324,6 +324,19 @@ static void vbox_primary_atomic_disable(struct drm_plane *plane, old_state->src_y >> 16); } +struct vbox_cursor_plane_state { + struct drm_plane_state base; + + /* Transitional state - do not export or duplicate */ + + struct dma_buf_map map; +}; + +static struct vbox_cursor_plane_state *to_vbox_cursor_plane_state(struct drm_plane_state *state) +{ + return container_of(state, struct vbox_cursor_plane_state, base); +} + static int vbox_cursor_atomic_check(struct drm_plane *plane, struct drm_plane_state *new_state) { @@ -381,14 +394,13 @@ 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 vbox_cursor_plane_state *vbox_state = to_vbox_cursor_plane_state(plane->state); + struct dma_buf_map map = vbox_state->map; + 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 +413,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 +422,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; @@ -458,6 +458,43 @@ static void vbox_cursor_atomic_disable(struct drm_plane *plane, mutex_unlock(&vbox->hw_mutex); } +static int vbox_cursor_prepare_fb(struct drm_plane *plane, struct drm_plane_state *new_state) +{ + struct vbox_cursor_plane_state *new_vbox_state = to_vbox_cursor_plane_state(new_state); + struct drm_framebuffer *fb = new_state->fb; + struct drm_gem_vram_object *gbo; + struct dma_buf_map map; + int ret; + + if (!fb) + return 0; + + gbo = drm_gem_vram_of_gem(fb->obj[0]); + + ret = drm_gem_vram_vmap(gbo, &map); + if (ret) + return ret; + + new_vbox_state->map = map; + + return 0; +} + +static void vbox_cursor_cleanup_fb(struct drm_plane *plane, struct drm_plane_state *old_state) +{ + struct vbox_cursor_plane_state *old_vbox_state = to_vbox_cursor_plane_state(old_state); + struct drm_framebuffer *fb = old_state->fb; + struct dma_buf_map map = old_vbox_state->map; + struct drm_gem_vram_object *gbo; + + if (!fb) + return; + + gbo = drm_gem_vram_of_gem(fb->obj[0]); + + drm_gem_vram_vunmap(gbo, &map); +} + static const u32 vbox_cursor_plane_formats[] = { DRM_FORMAT_ARGB8888, }; @@ -466,17 +503,42 @@ 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, + .prepare_fb = vbox_cursor_prepare_fb, + .cleanup_fb = vbox_cursor_cleanup_fb, }; +static struct drm_plane_state *vbox_cursor_atomic_duplicate_state(struct drm_plane *plane) +{ + struct vbox_cursor_plane_state *new_vbox_state; + struct drm_device *dev = plane->dev; + + if (drm_WARN_ON(dev, !plane->state)) + return NULL; + + new_vbox_state = kzalloc(sizeof(*new_vbox_state), GFP_KERNEL); + if (!new_vbox_state) + return NULL; + __drm_atomic_helper_plane_duplicate_state(plane, &new_vbox_state->base); + + return &new_vbox_state->base; +} + +static void vbox_cursor_atomic_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct vbox_cursor_plane_state *vbox_state = to_vbox_cursor_plane_state(state); + + __drm_atomic_helper_plane_destroy_state(&vbox_state->base); + kfree(vbox_state); +} + 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, + .atomic_duplicate_state = vbox_cursor_atomic_duplicate_state, + .atomic_destroy_state = vbox_cursor_atomic_destroy_state, }; static const u32 vbox_primary_plane_formats[] = { base-commit: 3836b7fdfad40e2bac5dc882332f42bed6942cf4 prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d -- 2.30.0-- 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_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- 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
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
