All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] drm/vmwgfx: Make sure bo's are unpinned before putting them back
@ 2021-04-13 20:59 Zack Rusin
  2021-04-14  5:36 ` Thomas Hellström (Intel)
  2021-04-15 18:05 ` Roland Scheidegger
  0 siblings, 2 replies; 3+ messages in thread
From: Zack Rusin @ 2021-04-13 20:59 UTC (permalink / raw
  To: dri-devel; +Cc: Martin Krastev, Roland Scheidegger

During cotable resize we pin the backup buffer to make sure the
trylock doesn't fail. We were never unpinning the backup buffer
resulting in every subsequent cotable resize trying to release a
pinned bo. After we copy the old backup to the new we can release
the pin.
Mob's are always pinned so we just have to make sure we unpin
them before releasing them.

Cc: Martin Krastev <krastevm@vmware.com>
Cc: Roland Scheidegger <sroland@vmware.com>
Fixes: d1a73c641afd ("drm/vmwgfx: Make sure we unpin no longer needed buffers")
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Zack Rusin <zackr@vmware.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c |  4 ++++
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h     |  5 +----
 drivers/gpu/drm/vmwgfx/vmwgfx_mob.c     | 18 ++++++++++++++----
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
index ba658fa9cf6c..183571c387b7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
@@ -481,11 +481,15 @@ static int vmw_cotable_resize(struct vmw_resource *res, size_t new_size)
 	vmw_bo_unreference(&old_buf);
 	res->id = vcotbl->type;
 
+	/* Release the pin acquired in vmw_bo_init */
+	ttm_bo_unpin(bo);
+
 	return 0;
 
 out_map_new:
 	ttm_bo_kunmap(&old_map);
 out_wait:
+	ttm_bo_unpin(bo);
 	ttm_bo_unreserve(bo);
 	vmw_bo_unreference(&buf);
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 8087a9013455..eb76a6b9ebca 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1522,11 +1522,8 @@ static inline void vmw_bo_unreference(struct vmw_buffer_object **buf)
 	struct vmw_buffer_object *tmp_buf = *buf;
 
 	*buf = NULL;
-	if (tmp_buf != NULL) {
-		if (tmp_buf->base.pin_count > 0)
-			ttm_bo_unpin(&tmp_buf->base);
+	if (tmp_buf != NULL)
 		ttm_bo_put(&tmp_buf->base);
-	}
 }
 
 static inline struct vmw_buffer_object *
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
index a0b53141dded..f2d625415458 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
@@ -94,6 +94,16 @@ static void vmw_mob_pt_setup(struct vmw_mob *mob,
 			     struct vmw_piter data_iter,
 			     unsigned long num_data_pages);
 
+
+static inline void vmw_bo_unpin_unlocked(struct ttm_buffer_object *bo)
+{
+	int ret = ttm_bo_reserve(bo, false, true, NULL);
+	BUG_ON(ret != 0);
+	ttm_bo_unpin(bo);
+	ttm_bo_unreserve(bo);
+}
+
+
 /*
  * vmw_setup_otable_base - Issue an object table base setup command to
  * the device
@@ -277,7 +287,7 @@ static int vmw_otable_batch_setup(struct vmw_private *dev_priv,
 						 &batch->otables[i]);
 	}
 
-	ttm_bo_unpin(batch->otable_bo);
+	vmw_bo_unpin_unlocked(batch->otable_bo);
 	ttm_bo_put(batch->otable_bo);
 	batch->otable_bo = NULL;
 	return ret;
@@ -341,9 +351,9 @@ static void vmw_otable_batch_takedown(struct vmw_private *dev_priv,
 	BUG_ON(ret != 0);
 
 	vmw_bo_fence_single(bo, NULL);
+	ttm_bo_unpin(bo);
 	ttm_bo_unreserve(bo);
 
-	ttm_bo_unpin(batch->otable_bo);
 	ttm_bo_put(batch->otable_bo);
 	batch->otable_bo = NULL;
 }
@@ -530,7 +540,7 @@ static void vmw_mob_pt_setup(struct vmw_mob *mob,
 void vmw_mob_destroy(struct vmw_mob *mob)
 {
 	if (mob->pt_bo) {
-		ttm_bo_unpin(mob->pt_bo);
+		vmw_bo_unpin_unlocked(mob->pt_bo);
 		ttm_bo_put(mob->pt_bo);
 		mob->pt_bo = NULL;
 	}
@@ -646,7 +656,7 @@ int vmw_mob_bind(struct vmw_private *dev_priv,
 out_no_cmd_space:
 	vmw_fifo_resource_dec(dev_priv);
 	if (pt_set_up) {
-		ttm_bo_unpin(mob->pt_bo);
+		vmw_bo_unpin_unlocked(mob->pt_bo);
 		ttm_bo_put(mob->pt_bo);
 		mob->pt_bo = NULL;
 	}
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] drm/vmwgfx: Make sure bo's are unpinned before putting them back
  2021-04-13 20:59 [PATCH v3] drm/vmwgfx: Make sure bo's are unpinned before putting them back Zack Rusin
@ 2021-04-14  5:36 ` Thomas Hellström (Intel)
  2021-04-15 18:05 ` Roland Scheidegger
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Hellström (Intel) @ 2021-04-14  5:36 UTC (permalink / raw
  To: Zack Rusin, dri-devel; +Cc: Martin Krastev, Roland Scheidegger

Hi, Zack,

On 4/13/21 10:59 PM, Zack Rusin wrote:
> During cotable resize we pin the backup buffer to make sure the
> trylock doesn't fail. We were never unpinning the backup buffer
> resulting in every subsequent cotable resize trying to release a
> pinned bo. After we copy the old backup to the new we can release
> the pin.
> Mob's are always pinned so we just have to make sure we unpin
> them before releasing them.
>
> Cc: Martin Krastev <krastevm@vmware.com>
> Cc: Roland Scheidegger <sroland@vmware.com>
> Fixes: d1a73c641afd ("drm/vmwgfx: Make sure we unpin no longer needed buffers")
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Zack Rusin <zackr@vmware.com>

LGTM,

Reviewed-by: Thomas Hellström (Intel) <thomas_os@shipmail.org>


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] drm/vmwgfx: Make sure bo's are unpinned before putting them back
  2021-04-13 20:59 [PATCH v3] drm/vmwgfx: Make sure bo's are unpinned before putting them back Zack Rusin
  2021-04-14  5:36 ` Thomas Hellström (Intel)
@ 2021-04-15 18:05 ` Roland Scheidegger
  1 sibling, 0 replies; 3+ messages in thread
From: Roland Scheidegger @ 2021-04-15 18:05 UTC (permalink / raw
  To: Zack Rusin, dri-devel; +Cc: Martin Krastev

The logic is all complex, but looks good to me.

Reviewed-by: Roland Scheidegger <sroland@vmware.com>

On 13.04.21 22:59, Zack Rusin wrote:
> During cotable resize we pin the backup buffer to make sure the
> trylock doesn't fail. We were never unpinning the backup buffer
> resulting in every subsequent cotable resize trying to release a
> pinned bo. After we copy the old backup to the new we can release
> the pin.
> Mob's are always pinned so we just have to make sure we unpin
> them before releasing them.
> 
> Cc: Martin Krastev <krastevm@vmware.com>
> Cc: Roland Scheidegger <sroland@vmware.com>
> Fixes: d1a73c641afd ("drm/vmwgfx: Make sure we unpin no longer needed buffers")
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Zack Rusin <zackr@vmware.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c |  4 ++++
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h     |  5 +----
>  drivers/gpu/drm/vmwgfx/vmwgfx_mob.c     | 18 ++++++++++++++----
>  3 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
> index ba658fa9cf6c..183571c387b7 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
> @@ -481,11 +481,15 @@ static int vmw_cotable_resize(struct vmw_resource *res, size_t new_size)
>  	vmw_bo_unreference(&old_buf);
>  	res->id = vcotbl->type;
>  
> +	/* Release the pin acquired in vmw_bo_init */
> +	ttm_bo_unpin(bo);
> +
>  	return 0;
>  
>  out_map_new:
>  	ttm_bo_kunmap(&old_map);
>  out_wait:
> +	ttm_bo_unpin(bo);
>  	ttm_bo_unreserve(bo);
>  	vmw_bo_unreference(&buf);
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index 8087a9013455..eb76a6b9ebca 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -1522,11 +1522,8 @@ static inline void vmw_bo_unreference(struct vmw_buffer_object **buf)
>  	struct vmw_buffer_object *tmp_buf = *buf;
>  
>  	*buf = NULL;
> -	if (tmp_buf != NULL) {
> -		if (tmp_buf->base.pin_count > 0)
> -			ttm_bo_unpin(&tmp_buf->base);
> +	if (tmp_buf != NULL)
>  		ttm_bo_put(&tmp_buf->base);
> -	}
>  }
>  
>  static inline struct vmw_buffer_object *
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
> index a0b53141dded..f2d625415458 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
> @@ -94,6 +94,16 @@ static void vmw_mob_pt_setup(struct vmw_mob *mob,
>  			     struct vmw_piter data_iter,
>  			     unsigned long num_data_pages);
>  
> +
> +static inline void vmw_bo_unpin_unlocked(struct ttm_buffer_object *bo)
> +{
> +	int ret = ttm_bo_reserve(bo, false, true, NULL);
> +	BUG_ON(ret != 0);
> +	ttm_bo_unpin(bo);
> +	ttm_bo_unreserve(bo);
> +}
> +
> +
>  /*
>   * vmw_setup_otable_base - Issue an object table base setup command to
>   * the device
> @@ -277,7 +287,7 @@ static int vmw_otable_batch_setup(struct vmw_private *dev_priv,
>  						 &batch->otables[i]);
>  	}
>  
> -	ttm_bo_unpin(batch->otable_bo);
> +	vmw_bo_unpin_unlocked(batch->otable_bo);
>  	ttm_bo_put(batch->otable_bo);
>  	batch->otable_bo = NULL;
>  	return ret;
> @@ -341,9 +351,9 @@ static void vmw_otable_batch_takedown(struct vmw_private *dev_priv,
>  	BUG_ON(ret != 0);
>  
>  	vmw_bo_fence_single(bo, NULL);
> +	ttm_bo_unpin(bo);
>  	ttm_bo_unreserve(bo);
>  
> -	ttm_bo_unpin(batch->otable_bo);
>  	ttm_bo_put(batch->otable_bo);
>  	batch->otable_bo = NULL;
>  }
> @@ -530,7 +540,7 @@ static void vmw_mob_pt_setup(struct vmw_mob *mob,
>  void vmw_mob_destroy(struct vmw_mob *mob)
>  {
>  	if (mob->pt_bo) {
> -		ttm_bo_unpin(mob->pt_bo);
> +		vmw_bo_unpin_unlocked(mob->pt_bo);
>  		ttm_bo_put(mob->pt_bo);
>  		mob->pt_bo = NULL;
>  	}
> @@ -646,7 +656,7 @@ int vmw_mob_bind(struct vmw_private *dev_priv,
>  out_no_cmd_space:
>  	vmw_fifo_resource_dec(dev_priv);
>  	if (pt_set_up) {
> -		ttm_bo_unpin(mob->pt_bo);
> +		vmw_bo_unpin_unlocked(mob->pt_bo);
>  		ttm_bo_put(mob->pt_bo);
>  		mob->pt_bo = NULL;
>  	}
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-04-15 18:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-04-13 20:59 [PATCH v3] drm/vmwgfx: Make sure bo's are unpinned before putting them back Zack Rusin
2021-04-14  5:36 ` Thomas Hellström (Intel)
2021-04-15 18:05 ` Roland Scheidegger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.