qxl: No need for NULL check before calling qxl_bo_unref()
Subsystems (3)
PATCH 1/3 qxl: No need for NULL check before calling qxl_bo_unref()
qxl_bo_unref() is already performing a NULL check.
Signed-off-by Christophe Fergeau <[email protected]>
--- drivers/gpu/drm/qxl/qxl_display.c | 4 +--- drivers/gpu/drm/qxl/qxl_draw.c | 3 +-- drivers/gpu/drm/qxl/qxl_kms.c | 6 ++---- 3 files changed, 4 insertions(+), 9 deletions(-)
drivers/gpu/drm/qxl/qxl_display.c +1 -3
@@ -668,9 +668,7 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane,qxl_push_cursor_ring_release (qdev, release,QXL_CMD_CURSOR , false);qxl_release_fence_buffer_objects (release); -if (old_cursor_bo) -qxl_bo_unref (&old_cursor_bo); - +qxl_bo_unref (&old_cursor_bo);qxl_bo_unref (&cursor_bo);return ;
drivers/gpu/drm/qxl/qxl_draw.c +1 -2
@@ -245,8 +245,7 @@ void qxl_draw_opaque_fb(const struct qxl_fb_image *qxl_fb_image,qxl_release_fence_buffer_objects (release); out_free_palette: -if (palette_bo) -qxl_bo_unref (&palette_bo); +qxl_bo_unref (&palette_bo); out_free_image:qxl_image_free_objects (qdev, dimage); out_free_drawable:
drivers/gpu/drm/qxl/qxl_kms.c +2 -4
@@ -313,10 +313,8 @@ int qxl_device_init(struct qxl_device *qdev,void qxl_device_fini (struct qxl_device *qdev) { -if (qdev->current_release_bo[0 ]) -qxl_bo_unref (&qdev->current_release_bo[0 ]); -if (qdev->current_release_bo[1 ]) -qxl_bo_unref (&qdev->current_release_bo[1 ]); +qxl_bo_unref (&qdev->current_release_bo[0 ]); +qxl_bo_unref (&qdev->current_release_bo[1 ]);flush_work (&qdev->gc_work);qxl_ring_free (qdev->command_ring);qxl_ring_free (qdev->cursor_ring);
Signature · 6 lines
2.19.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 2/3 qxl: Remove unused qxl_bo_pin arguments
The 'domain' argument to qxl_bo_pin is redundant with 'bo', and 'gpu_addr' is unused, so we can remove both.
Signed-off-by Christophe Fergeau <[email protected]>
--- drivers/gpu/drm/qxl/qxl_display.c | 4 ++-- drivers/gpu/drm/qxl/qxl_fb.c | 2 +- drivers/gpu/drm/qxl/qxl_object.c | 12 ++++-------- drivers/gpu/drm/qxl/qxl_object.h | 2 +- 4 files changed, 8 insertions(+), 12 deletions(-)
drivers/gpu/drm/qxl/qxl_display.c +2 -2
@@ -753,7 +753,7 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane, } } - ret =qxl_bo_pin (user_bo,QXL_GEM_DOMAIN_CPU ,NULL ); + ret =qxl_bo_pin (user_bo);if (ret)return ret;
@@ -1101,7 +1101,7 @@ int qxl_create_monitors_object(struct qxl_device *qdev) } qdev->monitors_config_bo =gem_to_qxl_bo (gobj); - ret =qxl_bo_pin (qdev->monitors_config_bo,QXL_GEM_DOMAIN_VRAM ,NULL ); + ret =qxl_bo_pin (qdev->monitors_config_bo);if (ret)return ret;
drivers/gpu/drm/qxl/qxl_fb.c +1 -1
@@ -111,7 +111,7 @@ static int qxlfb_create_pinned_object(struct qxl_device *qdev, qbo->surf.stride = mode_cmd->pitches[0 ]; qbo->surf.format = SPICE_SURFACE_FMT_32_xRGB; - ret =qxl_bo_pin (qbo,QXL_GEM_DOMAIN_SURFACE ,NULL ); + ret =qxl_bo_pin (qbo);if (ret) { goto out_unref; }
drivers/gpu/drm/qxl/qxl_object.c +4 -8
@@ -221,7 +221,7 @@ struct qxl_bo *qxl_bo_ref(struct qxl_bo *bo)return bo; } -static int __qxl_bo_pin (struct qxl_bo *bo,u32 domain,u64 *gpu_addr) +static int __qxl_bo_pin (struct qxl_bo *bo) {struct ttm_operation_ctx ctx = { false, false };struct drm_device *ddev = bo->gem_base.dev;
@@ -229,16 +229,12 @@ static int __qxl_bo_pin(struct qxl_bo *bo, u32 domain, u64 *gpu_addr)if (bo->pin_count) { bo->pin_count++; -if (gpu_addr) - *gpu_addr =qxl_bo_gpu_offset (bo);return 0 ; } -qxl_ttm_placement_from_domain (bo, domain, true); +qxl_ttm_placement_from_domain (bo, bo->type, true); r =ttm_bo_validate (&bo->tbo, &bo->placement, &ctx);if (likely (r ==0 )) { bo->pin_count =1 ; -if (gpu_addr !=NULL ) - *gpu_addr =qxl_bo_gpu_offset (bo); }if (unlikely (r !=0 ))dev_err (ddev->dev,"%p pin failed\n" , bo);
@@ -272,7 +268,7 @@ static int __qxl_bo_unpin(struct qxl_bo *bo)* beforehand, use the internal version directly __qxl_bo_pin. * */ -int qxl_bo_pin (struct qxl_bo *bo,u32 domain,u64 *gpu_addr) +int qxl_bo_pin (struct qxl_bo *bo) {int r;
@@ -280,7 +276,7 @@ int qxl_bo_pin(struct qxl_bo *bo, u32 domain, u64 *gpu_addr)if (r)return r; - r =__qxl_bo_pin (bo, bo->type,NULL ); + r =__qxl_bo_pin (bo);qxl_bo_unreserve (bo);return r; }
drivers/gpu/drm/qxl/qxl_object.h +1 -1
@@ -95,7 +95,7 @@ void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo, int pavoid qxl_bo_kunmap_atomic_page (struct qxl_device *qdev,struct qxl_bo *bo,void *map);extern struct qxl_bo *qxl_bo_ref (struct qxl_bo *bo);extern void qxl_bo_unref (struct qxl_bo **bo); -extern int qxl_bo_pin (struct qxl_bo *bo,u32 domain,u64 *gpu_addr); +extern int qxl_bo_pin (struct qxl_bo *bo);extern int qxl_bo_unpin (struct qxl_bo *bo);extern void qxl_ttm_placement_from_domain (struct qxl_bo *qbo,u32 domain,bool pinned);extern bool qxl_ttm_bo_is_qxl_bo (struct ttm_buffer_object *bo);
Signature · 6 lines
2.19.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 3/3 qxl: Make sure qxl_cursor memory is pinned
In reply to Christophe FergeauQEMU keeps a vram reference to the last QXLCursorCmd it received. This QXLCursorCmd command points to a QXLCursor instance (stored in vram too). However, while the QXLCursorCmd memory is pinned, the QXLCursor memory is not.
When booting a recent Fedora to its login screen while monitoring the QXLCursorCmd QEMU holds, it's possible to see the QXLCursor memory becoming invalid shortly after boot. Pinning that memory ensures that that QXLCursor memory is not going to be moved by the guest kernel.
Moving the pin/unpin to qxl_release_list_add()/qxl_release_free_list() would be a more generic fix. However, doing this quickly exhausts QXL video memory, so more fixing would be needed before this is workable.
Signed-off-by Christophe Fergeau <[email protected]>
--- drivers/gpu/drm/qxl/qxl_display.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/gpu/drm/qxl/qxl_display.c +9 -1
@@ -620,10 +620,14 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane,if (ret) goto out_kunmap; - ret =qxl_release_reserve_list (release, true); + ret =qxl_bo_pin (cursor_bo);if (ret) goto out_free_bo; + ret =qxl_release_reserve_list (release, true); +if (ret) + goto out_unpin; + ret =qxl_bo_kmap (cursor_bo, (void **)&cursor);if (ret) goto out_backoff;
@@ -668,6 +672,8 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane,qxl_push_cursor_ring_release (qdev, release,QXL_CMD_CURSOR , false);qxl_release_fence_buffer_objects (release); +if (old_cursor_bo !=NULL ) +qxl_bo_unpin (old_cursor_bo);qxl_bo_unref (&old_cursor_bo);qxl_bo_unref (&cursor_bo);
@@ -675,6 +681,8 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane, out_backoff:qxl_release_backoff_reserve_list (release); +out_unpin: +qxl_bo_unpin (cursor_bo); out_free_bo:qxl_bo_unref (&cursor_bo); out_kunmap:
Signature · 6 lines
2.19.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Gerd Hoffmann
maintainerQuoted 1 line · Christophe Fergeau
qxl_bo_unref()is already performing a NULL check.
All queued up for -next.
thanks,
Gerd
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel