Hi
Am 14.07.21 um 06:14 schrieb Zack Rusin:
From: Martin Krastev krastevm@vmware.com
- Add support for CursorMob
- Add support for CursorBypass 4
Reviewed-by: Zack Rusin zackr@vmware.com Signed-off-by: Martin Krastev krastevm@vmware.com Signed-off-by: Zack Rusin zackr@vmware.com
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....
[2] https://elixir.bootlin.com/linux/v5.13.1/source/drivers/gpu/drm/ast/ast_mode...