Re: [PATCH v2 4/8] drm/vmwgfx: Simplify fb pinning
On 1/30/23 19:35, Zack Rusin wrote:
> From: Zack Rusin
>
> Only the legacy display unit requires pinning of the fb memory in vram.
> Both the screen objects and screen targets can present from any buffer.
> That makes the pinning abstraction pointless. Simplify all of the code
> and move it to the legacy display unit, the only place that needs it.
>
> Signed-off-by: Zack Rusin
> Reviewed-by: Martin Krastev
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 8 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 4 --
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 66 -
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 4 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 57 +
> 5 files changed, 54 insertions(+), 85 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index b6dc0baef350..c6dc733f6d45 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -73,10 +73,10 @@ static bool bo_is_vmw(struct ttm_buffer_object *bo)
> * Return: Zero on success, Negative error code on failure. In particular
> * -ERESTARTSYS if interrupted by a signal
> */
> -int vmw_bo_pin_in_placement(struct vmw_private *dev_priv,
> - struct vmw_bo *buf,
> - struct ttm_placement *placement,
> - bool interruptible)
> +static int vmw_bo_pin_in_placement(struct vmw_private *dev_priv,
> +struct vmw_bo *buf,
> +struct ttm_placement *placement,
> +bool interruptible)
> {
> struct ttm_operation_ctx ctx = {interruptible, false };
> struct ttm_buffer_object *bo = >base;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> index 03ef4059c0d2..e2dadd68a16d 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> @@ -82,10 +82,6 @@ int vmw_bo_init(struct vmw_private *dev_priv,
> int vmw_bo_unref_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
>
> -int vmw_bo_pin_in_placement(struct vmw_private *vmw_priv,
> - struct vmw_bo *bo,
> - struct ttm_placement *placement,
> - bool interruptible);
> int vmw_bo_pin_in_vram(struct vmw_private *dev_priv,
> struct vmw_bo *buf,
> bool interruptible);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index ad41396c0a5d..6780391c57ea 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1487,69 +1487,6 @@ static const struct drm_framebuffer_funcs
> vmw_framebuffer_bo_funcs = {
> .dirty = vmw_framebuffer_bo_dirty_ext,
> };
>
> -/*
> - * Pin the bofer in a location suitable for access by the
> - * display system.
> - */
> -static int vmw_framebuffer_pin(struct vmw_framebuffer *vfb)
> -{
> - struct vmw_private *dev_priv = vmw_priv(vfb->base.dev);
> - struct vmw_bo *buf;
> - struct ttm_placement *placement;
> - int ret;
> -
> - buf = vfb->bo ? vmw_framebuffer_to_vfbd(>base)->buffer :
> - vmw_framebuffer_to_vfbs(>base)->surface->res.backup;
> -
> - if (!buf)
> - return 0;
> -
> - switch (dev_priv->active_display_unit) {
> - case vmw_du_legacy:
> - vmw_overlay_pause_all(dev_priv);
> - ret = vmw_bo_pin_in_start_of_vram(dev_priv, buf, false);
> - vmw_overlay_resume_all(dev_priv);
> - break;
> - case vmw_du_screen_object:
> - case vmw_du_screen_target:
> - if (vfb->bo) {
> - if (dev_priv->capabilities & SVGA_CAP_3D) {
> - /*
> - * Use surface DMA to get content to
> - * sreen target surface.
> - */
> - placement = _vram_gmr_placement;
> - } else {
> - /* Use CPU blit. */
> - placement = _sys_placement;
> - }
> - } else {
> - /* Use surface / image update */
> - placement = _mob_placement;
> - }
> -
> - return vmw_bo_pin_in_placement(dev_priv, buf, placement, false);
> - default:
> - return -EINVAL;
> - }
> -
> - return ret;
> -}
> -
> -static int vmw_framebuffer_unpin(struct vmw_framebuffer *vfb)
> -{
> - struct vmw_private *dev_priv = vmw_priv(vfb->base.dev);
> - struct vmw_bo *buf;
> -
> - buf = vfb->bo ? vmw_framebuffer_to_vfbd(>base)->buffer :
> - vmw_framebuffer_to_vfbs(>base)->surface->res.backup;
> -
> - if (WARN_ON(!buf))
> - return 0;
> -
> - return
[PATCH v2 4/8] drm/vmwgfx: Simplify fb pinning
From: Zack Rusin
Only the legacy display unit requires pinning of the fb memory in vram.
Both the screen objects and screen targets can present from any buffer.
That makes the pinning abstraction pointless. Simplify all of the code
and move it to the legacy display unit, the only place that needs it.
Signed-off-by: Zack Rusin
Reviewed-by: Martin Krastev
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 8 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 4 --
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 66 -
drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 4 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 57 +
5 files changed, 54 insertions(+), 85 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index b6dc0baef350..c6dc733f6d45 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -73,10 +73,10 @@ static bool bo_is_vmw(struct ttm_buffer_object *bo)
* Return: Zero on success, Negative error code on failure. In particular
* -ERESTARTSYS if interrupted by a signal
*/
-int vmw_bo_pin_in_placement(struct vmw_private *dev_priv,
- struct vmw_bo *buf,
- struct ttm_placement *placement,
- bool interruptible)
+static int vmw_bo_pin_in_placement(struct vmw_private *dev_priv,
+ struct vmw_bo *buf,
+ struct ttm_placement *placement,
+ bool interruptible)
{
struct ttm_operation_ctx ctx = {interruptible, false };
struct ttm_buffer_object *bo = >base;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
index 03ef4059c0d2..e2dadd68a16d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
@@ -82,10 +82,6 @@ int vmw_bo_init(struct vmw_private *dev_priv,
int vmw_bo_unref_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
-int vmw_bo_pin_in_placement(struct vmw_private *vmw_priv,
- struct vmw_bo *bo,
- struct ttm_placement *placement,
- bool interruptible);
int vmw_bo_pin_in_vram(struct vmw_private *dev_priv,
struct vmw_bo *buf,
bool interruptible);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index ad41396c0a5d..6780391c57ea 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1487,69 +1487,6 @@ static const struct drm_framebuffer_funcs
vmw_framebuffer_bo_funcs = {
.dirty = vmw_framebuffer_bo_dirty_ext,
};
-/*
- * Pin the bofer in a location suitable for access by the
- * display system.
- */
-static int vmw_framebuffer_pin(struct vmw_framebuffer *vfb)
-{
- struct vmw_private *dev_priv = vmw_priv(vfb->base.dev);
- struct vmw_bo *buf;
- struct ttm_placement *placement;
- int ret;
-
- buf = vfb->bo ? vmw_framebuffer_to_vfbd(>base)->buffer :
- vmw_framebuffer_to_vfbs(>base)->surface->res.backup;
-
- if (!buf)
- return 0;
-
- switch (dev_priv->active_display_unit) {
- case vmw_du_legacy:
- vmw_overlay_pause_all(dev_priv);
- ret = vmw_bo_pin_in_start_of_vram(dev_priv, buf, false);
- vmw_overlay_resume_all(dev_priv);
- break;
- case vmw_du_screen_object:
- case vmw_du_screen_target:
- if (vfb->bo) {
- if (dev_priv->capabilities & SVGA_CAP_3D) {
- /*
-* Use surface DMA to get content to
-* sreen target surface.
-*/
- placement = _vram_gmr_placement;
- } else {
- /* Use CPU blit. */
- placement = _sys_placement;
- }
- } else {
- /* Use surface / image update */
- placement = _mob_placement;
- }
-
- return vmw_bo_pin_in_placement(dev_priv, buf, placement, false);
- default:
- return -EINVAL;
- }
-
- return ret;
-}
-
-static int vmw_framebuffer_unpin(struct vmw_framebuffer *vfb)
-{
- struct vmw_private *dev_priv = vmw_priv(vfb->base.dev);
- struct vmw_bo *buf;
-
- buf = vfb->bo ? vmw_framebuffer_to_vfbd(>base)->buffer :
- vmw_framebuffer_to_vfbs(>base)->surface->res.backup;
-
- if (WARN_ON(!buf))
- return 0;
-
- return vmw_bo_unpin(dev_priv, buf, false);
-}
-
/**
* vmw_create_bo_proxy - create a proxy surface for the buffer object
*
@@ -1766,9 +1703,6 @@ vmw_kms_new_framebuffer(struct
