[PATCH 1/4] drm/vmwgfx: Add support for CursorMob and CursorBypass 4
Thomas Zimmermann
tzimmermann at suse.de
Wed Jul 14 07:30:00 UTC 2021
Hi
Am 14.07.21 um 06:14 schrieb Zack Rusin:
> From: Martin Krastev <krastevm at vmware.com>
>
> * Add support for CursorMob
> * Add support for CursorBypass 4
>
> Reviewed-by: Zack Rusin <zackr at vmware.com>
> Signed-off-by: Martin Krastev <krastevm at vmware.com>
> Signed-off-by: Zack Rusin <zackr at 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.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
-------------- 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/20210714/ea59ca04/attachment-0001.sig>
More information about the dri-devel
mailing list