[PATCH] drm/vboxvideo: Vmap/vunmap cursor BO in prepare_fb and cleanup_fb
Thomas Zimmermann
tzimmermann at suse.de
Mon Feb 8 13:54:14 UTC 2021
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@suse.de/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 at lists.freedesktop.org
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20210208/27e13953/attachment-0001.sig>
More information about the dri-devel
mailing list