diff mbox series

ttm: ttm_bo_swapout_all doesn't use it's argument.

Message ID 20200728034254.20114-1-airlied@gmail.com (mailing list archive)
State New, archived
Headers show
Series ttm: ttm_bo_swapout_all doesn't use it's argument. | expand

Commit Message

Dave Airlie July 28, 2020, 3:42 a.m. UTC 
From: Dave Airlie <airlied@redhat.com>

Just drop the argument from this.

This does ask the question if this is the function vmwgfx
should be using or should it be doing an evict all like
the other drivers.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c        | 2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
 include/drm/ttm/ttm_bo_api.h        | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Comments

Christian König July 28, 2020, 7:37 a.m. UTC | #1 
Am 28.07.20 um 05:42 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> Just drop the argument from this.
>
> This does ask the question if this is the function vmwgfx
> should be using or should it be doing an evict all like
> the other drivers.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c        | 2 +-
>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
>   include/drm/ttm/ttm_bo_api.h        | 2 +-
>   3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index b03747717ec7..f297fd5e02d4 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1838,7 +1838,7 @@ int ttm_bo_swapout(struct ttm_bo_global *glob, struct ttm_operation_ctx *ctx)
>   }
>   EXPORT_SYMBOL(ttm_bo_swapout);
>   
> -void ttm_bo_swapout_all(struct ttm_bo_device *bdev)
> +void ttm_bo_swapout_all(void)
>   {
>   	struct ttm_operation_ctx ctx = {
>   		.interruptible = false,
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 470428387878..fb39826f72c1 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1352,7 +1352,7 @@ static int vmw_pm_freeze(struct device *kdev)
>   	vmw_execbuf_release_pinned_bo(dev_priv);
>   	vmw_resource_evict_all(dev_priv);
>   	vmw_release_device_early(dev_priv);
> -	ttm_bo_swapout_all(&dev_priv->bdev);
> +	ttm_bo_swapout_all();
>   	if (dev_priv->enable_fb)
>   		vmw_fifo_resource_dec(dev_priv);
>   	if (atomic_read(&dev_priv->num_fifo_resources) != 0) {
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index b1c705a93517..a9e13b252820 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -692,7 +692,7 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp,
>   
>   int ttm_bo_swapout(struct ttm_bo_global *glob,
>   			struct ttm_operation_ctx *ctx);
> -void ttm_bo_swapout_all(struct ttm_bo_device *bdev);
> +void ttm_bo_swapout_all(void);
>   
>   /**
>    * ttm_bo_uses_embedded_gem_object - check if the given bo uses the
Daniel Vetter July 28, 2020, 9:14 a.m. UTC | #2 
On Tue, Jul 28, 2020 at 01:42:54PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> Just drop the argument from this.
> 
> This does ask the question if this is the function vmwgfx
> should be using or should it be doing an evict all like
> the other drivers.

Yeah this looks a bit like ttm_bo_swapout_all shouldn't even be exported
really, it's part of the internal shrinker stuff.
-Daniel

> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/ttm/ttm_bo.c        | 2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
>  include/drm/ttm/ttm_bo_api.h        | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index b03747717ec7..f297fd5e02d4 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1838,7 +1838,7 @@ int ttm_bo_swapout(struct ttm_bo_global *glob, struct ttm_operation_ctx *ctx)
>  }
>  EXPORT_SYMBOL(ttm_bo_swapout);
>  
> -void ttm_bo_swapout_all(struct ttm_bo_device *bdev)
> +void ttm_bo_swapout_all(void)
>  {
>  	struct ttm_operation_ctx ctx = {
>  		.interruptible = false,
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 470428387878..fb39826f72c1 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1352,7 +1352,7 @@ static int vmw_pm_freeze(struct device *kdev)
>  	vmw_execbuf_release_pinned_bo(dev_priv);
>  	vmw_resource_evict_all(dev_priv);
>  	vmw_release_device_early(dev_priv);
> -	ttm_bo_swapout_all(&dev_priv->bdev);
> +	ttm_bo_swapout_all();
>  	if (dev_priv->enable_fb)
>  		vmw_fifo_resource_dec(dev_priv);
>  	if (atomic_read(&dev_priv->num_fifo_resources) != 0) {
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index b1c705a93517..a9e13b252820 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -692,7 +692,7 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp,
>  
>  int ttm_bo_swapout(struct ttm_bo_global *glob,
>  			struct ttm_operation_ctx *ctx);
> -void ttm_bo_swapout_all(struct ttm_bo_device *bdev);
> +void ttm_bo_swapout_all(void);
>  
>  /**
>   * ttm_bo_uses_embedded_gem_object - check if the given bo uses the
> -- 
> 2.26.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Zack Rusin July 28, 2020, 11:47 p.m. UTC | #3 
> On Jul 27, 2020, at 11:42 PM, Dave Airlie <airlied@gmail.com> wrote:
> 
> From: Dave Airlie <airlied@redhat.com>
> 
> Just drop the argument from this.
> 
> This does ask the question if this is the function vmwgfx
> should be using or should it be doing an evict all like
> the other drivers.

Yea, it really should. This code is really old. Over the next year we’ll spend some time making our driver a little less special so that we can make those symbols private again (we’ll have to reevaulate whether we still need the private ttm_lock code, because ideally we’d bring all of that inline with the other drivers).

For the patch:
Reviewed-by: Zack Rusin <zackr@vmware.com>

z
Roland Scheidegger July 30, 2020, 3:45 p.m. UTC | #4 
Patch looks good to me too.
Reviewed-by: Roland Scheidegger <sroland@vmware.com>

Seems indeed like we should do some cleanup.

Roland

Am 28.07.20 um 09:37 schrieb Christian König:
> Am 28.07.20 um 05:42 schrieb Dave Airlie:
>> From: Dave Airlie <airlied@redhat.com>
>>
>> Just drop the argument from this.
>>
>> This does ask the question if this is the function vmwgfx
>> should be using or should it be doing an evict all like
>> the other drivers.
>>
>> Signed-off-by: Dave Airlie <airlied@redhat.com>
> 
> Reviewed-by: Christian König <christian.koenig@amd.com>
> 
>> ---
>>   drivers/gpu/drm/ttm/ttm_bo.c        | 2 +-
>>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
>>   include/drm/ttm/ttm_bo_api.h        | 2 +-
>>   3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index b03747717ec7..f297fd5e02d4 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -1838,7 +1838,7 @@ int ttm_bo_swapout(struct ttm_bo_global *glob,
>> struct ttm_operation_ctx *ctx)
>>   }
>>   EXPORT_SYMBOL(ttm_bo_swapout);
>>   -void ttm_bo_swapout_all(struct ttm_bo_device *bdev)
>> +void ttm_bo_swapout_all(void)
>>   {
>>       struct ttm_operation_ctx ctx = {
>>           .interruptible = false,
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> index 470428387878..fb39826f72c1 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> @@ -1352,7 +1352,7 @@ static int vmw_pm_freeze(struct device *kdev)
>>       vmw_execbuf_release_pinned_bo(dev_priv);
>>       vmw_resource_evict_all(dev_priv);
>>       vmw_release_device_early(dev_priv);
>> -    ttm_bo_swapout_all(&dev_priv->bdev);
>> +    ttm_bo_swapout_all();
>>       if (dev_priv->enable_fb)
>>           vmw_fifo_resource_dec(dev_priv);
>>       if (atomic_read(&dev_priv->num_fifo_resources) != 0) {
>> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
>> index b1c705a93517..a9e13b252820 100644
>> --- a/include/drm/ttm/ttm_bo_api.h
>> +++ b/include/drm/ttm/ttm_bo_api.h
>> @@ -692,7 +692,7 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev,
>> struct file *filp,
>>     int ttm_bo_swapout(struct ttm_bo_global *glob,
>>               struct ttm_operation_ctx *ctx);
>> -void ttm_bo_swapout_all(struct ttm_bo_device *bdev);
>> +void ttm_bo_swapout_all(void);
>>     /**
>>    * ttm_bo_uses_embedded_gem_object - check if the given bo uses the
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index b03747717ec7..f297fd5e02d4 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1838,7 +1838,7 @@  int ttm_bo_swapout(struct ttm_bo_global *glob, struct ttm_operation_ctx *ctx)
 }
 EXPORT_SYMBOL(ttm_bo_swapout);
 
-void ttm_bo_swapout_all(struct ttm_bo_device *bdev)
+void ttm_bo_swapout_all(void)
 {
 	struct ttm_operation_ctx ctx = {
 		.interruptible = false,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 470428387878..fb39826f72c1 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1352,7 +1352,7 @@  static int vmw_pm_freeze(struct device *kdev)
 	vmw_execbuf_release_pinned_bo(dev_priv);
 	vmw_resource_evict_all(dev_priv);
 	vmw_release_device_early(dev_priv);
-	ttm_bo_swapout_all(&dev_priv->bdev);
+	ttm_bo_swapout_all();
 	if (dev_priv->enable_fb)
 		vmw_fifo_resource_dec(dev_priv);
 	if (atomic_read(&dev_priv->num_fifo_resources) != 0) {
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index b1c705a93517..a9e13b252820 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -692,7 +692,7 @@  ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp,
 
 int ttm_bo_swapout(struct ttm_bo_global *glob,
 			struct ttm_operation_ctx *ctx);
-void ttm_bo_swapout_all(struct ttm_bo_device *bdev);
+void ttm_bo_swapout_all(void);
 
 /**
  * ttm_bo_uses_embedded_gem_object - check if the given bo uses the