Hi Am 14.07.21 um 06:14 schrieb Zack Rusin:
From: Martin Krastev <krastevm@xxxxxxxxxx>
* Add support for CursorMob
* Add support for CursorBypass 4
Reviewed-by: Zack Rusin <zackr@xxxxxxxxxx>
Signed-off-by: Martin Krastev <krastevm@xxxxxxxxxx>
Signed-off-by: Zack Rusin <zackr@xxxxxxxxxx>
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 45 +++++++++++++++-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 6 +++
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 79 +++++++++++++++++++++++++++--
3 files changed, 125 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 086dc75e7b42..7d8cc2f6b04e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0 OR MIT
/**************************************************************************
*
- * Copyright 2009-2016 VMware, Inc., Palo Alto, CA., USA
+ * Copyright 2009-2021 VMware, Inc., Palo Alto, CA., USA
*
* Permission is hereby granted, free of charge, to any person obtaining a
* copy of this software and associated documentation files (the
@@ -301,8 +301,12 @@ static void vmw_print_capabilities2(uint32_t capabilities2)
DRM_INFO(" Grow oTable.\n");
These macros have been out of fashion for a while. There's drm_info(), drm_warn(), drm_err(), etc as replacements. They also print device information. Applis here and for the rest of the patchset.
if (capabilities2 & SVGA_CAP2_INTRA_SURFACE_COPY) DRM_INFO(" IntraSurface copy.\n"); + if (capabilities2 & SVGA_CAP2_CURSOR_MOB) + DRM_INFO(" Cursor Mob.\n"); if (capabilities2 & SVGA_CAP2_DX3) DRM_INFO(" DX3.\n"); + if (capabilities2 & SVGA_CAP2_EXTRA_REGS) + DRM_INFO(" Extra Regs.\n"); }static void vmw_print_capabilities(uint32_t capabilities)@@ -505,6 +509,7 @@ static int vmw_request_device_late(struct vmw_private *dev_priv) static int vmw_request_device(struct vmw_private *dev_priv) { int ret; + size_t i;ret = vmw_device_init(dev_priv);if (unlikely(ret != 0)) { @@ -526,6 +531,37 @@ static int vmw_request_device(struct vmw_private *dev_priv) if (unlikely(ret != 0)) goto out_no_query_bo;+ /* Set up mobs for cursor updates */+ if (dev_priv->has_mob && dev_priv->capabilities2 & SVGA_CAP2_CURSOR_MOB) { + const uint32_t cursor_max_dim = vmw_read(dev_priv, SVGA_REG_CURSOR_MAX_DIMENSION); + + for (i = 0; i < ARRAY_SIZE(dev_priv->cursor_mob); i++) { + struct ttm_buffer_object **const bo = &dev_priv->cursor_mob[i]; + + ret = vmw_bo_create_kernel(dev_priv, + cursor_max_dim * cursor_max_dim * sizeof(u32) + sizeof(SVGAGBCursorHeader), + &vmw_mob_placement, bo); + + if (ret != 0) { + DRM_ERROR("Unable to create CursorMob array.\n"); + break; + } + + BUG_ON((*bo)->resource->mem_type != VMW_PL_MOB);
BUG_ON() crashes the kernel. The prefered way is to use drm_WARN_*() and return.
+ + /* Fence the mob creation so we are guarateed to have the mob */ + ret = ttm_bo_reserve(*bo, false, true, NULL); + BUG_ON(ret);
I'm not quite sure, but this line is probably a no-go wrt to best practices. See the comment above.
+
+ vmw_bo_fence_single(*bo, NULL);
+
+ ttm_bo_unreserve(*bo);
+
+ DRM_INFO("Using CursorMob mobid %lu, max dimension %u\n",
+ (*bo)->resource->start, cursor_max_dim);
IIRC anything *_info() is just radom info into the log. Most of the time, no one cares. Better use one of the drm_dbg_() calls.
+ } + } + return 0;out_no_query_bo:@@ -556,6 +592,8 @@ static int vmw_request_device(struct vmw_private *dev_priv) */ static void vmw_release_device_early(struct vmw_private *dev_priv) { + size_t i; + /* * Previous destructions should've released * the pinned bo. @@ -570,6 +608,11 @@ static void vmw_release_device_early(struct vmw_private *dev_priv) if (dev_priv->has_mob) { struct ttm_resource_manager *man;+ for (i = 0; i < ARRAY_SIZE(dev_priv->cursor_mob); i++) {+ if (dev_priv->cursor_mob[i] != NULL) + ttm_bo_put(dev_priv->cursor_mob[i]); + } + man = ttm_manager_type(&dev_priv->bdev, VMW_PL_MOB); ttm_resource_manager_evict_all(&dev_priv->bdev, man); vmw_otables_takedown(dev_priv); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h index 356f82c26f59..46bf54f6169a 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h @@ -642,6 +642,12 @@ struct vmw_private { u8 mksstat_kern_top_timer[MKSSTAT_CAPACITY]; atomic_t mksstat_kern_pids[MKSSTAT_CAPACITY]; #endif + + /* + * CursorMob buffer objects + */ + struct ttm_buffer_object *cursor_mob[2]; + atomic_t cursor_mob_idx;
That's something like page-flipping with alternating BO's and shadow buffering?
You really want a cursor plane to hold this information.I briefly looked through vmwgfx and it has all these fail-able code in its atomic-update path. The patches here only make things worse. With cursor planes, you can do all the preparation in atomic_check and prepare_fb, and store the
intermediate state/mappings/etc in the plane state.The ast driver started with a design like this one here and then we moved it to cursor planes. Ast has now none of the mentioned problems. Relevant code is at [1][2].
Best regards Thomas[1] https://elixir.bootlin.com/linux/v5.13.1/source/drivers/gpu/drm/ast/ast_drv.h#L105
[2] https://elixir.bootlin.com/linux/v5.13.1/source/drivers/gpu/drm/ast/ast_mode.c#L652
-- 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
