RE: [PATCH 3/4] drm/ttm: add input parameter force_alloc for ttm_bo_evict_mm

[He, Roger]

I can't think of an use case when we don't want this to succeed.

That is true. seems I can simplify more here.

Thanks
Roger(Hongbo.He)
-Original Message-
From: Koenig, Christian 
Sent: Thursday, February 08, 2018 8:58 PM
To: He, Roger <hongbo...@amd.com>; amd-gfx@lists.freedesktop.org; 
dri-de...@lists.freedesktop.org
Subject: Re: [PATCH 3/4] drm/ttm: add input parameter force_alloc for 
ttm_bo_evict_mm

Am 08.02.2018 um 10:06 schrieb Roger He:
> if true, allocate TTM pages regardless of zone global memory account 
> limit. For suspend, We should avoid TTM memory allocate failure then 
> result in suspend failure.

Why the extra parameter for amdgpu_bo_evict_vram ?

I can't think of an use case when we don't want this to succeed.

Christian.

>
> Signed-off-by: Roger He <hongbo...@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  2 +-
>   drivers/gpu/drm/nouveau/nouveau_drm.c   |  2 +-
>   drivers/gpu/drm/qxl/qxl_object.c|  4 ++--
>   drivers/gpu/drm/radeon/radeon_device.c  |  6 +++---
>   drivers/gpu/drm/radeon/radeon_object.c  |  5 +++--
>   drivers/gpu/drm/radeon/radeon_object.h  |  3 ++-
>   drivers/gpu/drm/ttm/ttm_bo.c| 16 ++--
>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |  6 +++---
>   include/drm/ttm/ttm_bo_api.h|  5 -
>   12 files changed, 34 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index ee76b46..59ee12c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -763,7 +763,7 @@ static int amdgpu_debugfs_evict_vram(struct seq_file *m, 
> void *data)
>   struct drm_device *dev = node->minor->dev;
>   struct amdgpu_device *adev = dev->dev_private;
>   
> - seq_printf(m, "(%d)\n", amdgpu_bo_evict_vram(adev));
> + seq_printf(m, "(%d)\n", amdgpu_bo_evict_vram(adev, true));
>   return 0;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index e3fa3d7..3c5f9ca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2168,7 +2168,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool 
> suspend, bool fbcon)
>   }
>   }
>   /* evict vram memory */
> - amdgpu_bo_evict_vram(adev);
> + amdgpu_bo_evict_vram(adev, true);
>   
>   amdgpu_fence_driver_suspend(adev);
>   
> @@ -2178,7 +2178,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool 
> suspend, bool fbcon)
>* This second call to evict vram is to evict the gart page table
>* using the CPU.
>*/
> - amdgpu_bo_evict_vram(adev);
> + amdgpu_bo_evict_vram(adev, true);
>   
>   pci_save_state(dev->pdev);
>   if (suspend) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 0338ef6..db813f9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -803,14 +803,14 @@ int amdgpu_bo_unpin(struct amdgpu_bo *bo)
>   return r;
>   }
>   
> -int amdgpu_bo_evict_vram(struct amdgpu_device *adev)
> +int amdgpu_bo_evict_vram(struct amdgpu_device *adev, bool 
> +force_alloc)
>   {
>   /* late 2.6.33 fix IGP hibernate - we need pm ops to do this correct */
>   if (0 && (adev->flags & AMD_IS_APU)) {
>   /* Useless to evict on IGP chips */
>   return 0;
>   }
> - return ttm_bo_evict_mm(>mman.bdev, TTM_PL_VRAM);
> + return ttm_bo_evict_mm(>mman.bdev, TTM_PL_VRAM, force_alloc);
>   }
>   
>   static const char *amdgpu_vram_names[] = { diff --git 
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index c2b02f5..6724cdc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -227,7 +227,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 
> domain,
>u64 min_offset, u64 max_offset,
>u64 *gpu_addr);
>   int amdgpu_bo_unpin(struct amdgpu_bo *bo); -int 
> amdgpu_bo_evict_vram(struct amdgpu_device *adev);
> +int amdgpu_bo_evict_vram(struct amdgpu_device *adev, bool 
> +force_alloc);
>   int amdgpu_bo_init(struct amdgpu_device *adev);
>   vo

Re: [PATCH 3/4] drm/ttm: add input parameter force_alloc for ttm_bo_evict_mm

[Christian König]

Am 08.02.2018 um 10:06 schrieb Roger He:

if true, allocate TTM pages regardless of zone global memory
account limit. For suspend, We should avoid TTM memory allocate
failure then result in suspend failure.


Why the extra parameter for amdgpu_bo_evict_vram ?

I can't think of an use case when we don't want this to succeed.

Christian.



Signed-off-by: Roger He 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |  4 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  |  4 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  2 +-
  drivers/gpu/drm/nouveau/nouveau_drm.c   |  2 +-
  drivers/gpu/drm/qxl/qxl_object.c|  4 ++--
  drivers/gpu/drm/radeon/radeon_device.c  |  6 +++---
  drivers/gpu/drm/radeon/radeon_object.c  |  5 +++--
  drivers/gpu/drm/radeon/radeon_object.h  |  3 ++-
  drivers/gpu/drm/ttm/ttm_bo.c| 16 ++--
  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |  6 +++---
  include/drm/ttm/ttm_bo_api.h|  5 -
  12 files changed, 34 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index ee76b46..59ee12c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -763,7 +763,7 @@ static int amdgpu_debugfs_evict_vram(struct seq_file *m, 
void *data)
struct drm_device *dev = node->minor->dev;
struct amdgpu_device *adev = dev->dev_private;
  
-	seq_printf(m, "(%d)\n", amdgpu_bo_evict_vram(adev));

+   seq_printf(m, "(%d)\n", amdgpu_bo_evict_vram(adev, true));
return 0;
  }
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

index e3fa3d7..3c5f9ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2168,7 +2168,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool 
suspend, bool fbcon)
}
}
/* evict vram memory */
-   amdgpu_bo_evict_vram(adev);
+   amdgpu_bo_evict_vram(adev, true);
  
  	amdgpu_fence_driver_suspend(adev);
  
@@ -2178,7 +2178,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool suspend, bool fbcon)

 * This second call to evict vram is to evict the gart page table
 * using the CPU.
 */
-   amdgpu_bo_evict_vram(adev);
+   amdgpu_bo_evict_vram(adev, true);
  
  	pci_save_state(dev->pdev);

if (suspend) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 0338ef6..db813f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -803,14 +803,14 @@ int amdgpu_bo_unpin(struct amdgpu_bo *bo)
return r;
  }
  
-int amdgpu_bo_evict_vram(struct amdgpu_device *adev)

+int amdgpu_bo_evict_vram(struct amdgpu_device *adev, bool force_alloc)
  {
/* late 2.6.33 fix IGP hibernate - we need pm ops to do this correct */
if (0 && (adev->flags & AMD_IS_APU)) {
/* Useless to evict on IGP chips */
return 0;
}
-   return ttm_bo_evict_mm(>mman.bdev, TTM_PL_VRAM);
+   return ttm_bo_evict_mm(>mman.bdev, TTM_PL_VRAM, force_alloc);
  }
  
  static const char *amdgpu_vram_names[] = {

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index c2b02f5..6724cdc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -227,7 +227,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 
domain,
 u64 min_offset, u64 max_offset,
 u64 *gpu_addr);
  int amdgpu_bo_unpin(struct amdgpu_bo *bo);
-int amdgpu_bo_evict_vram(struct amdgpu_device *adev);
+int amdgpu_bo_evict_vram(struct amdgpu_device *adev, bool force_alloc);
  int amdgpu_bo_init(struct amdgpu_device *adev);
  void amdgpu_bo_fini(struct amdgpu_device *adev);
  int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 8d4a5be..c9627ef 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -702,7 +702,7 @@ nouveau_do_suspend(struct drm_device *dev, bool runtime)
}
  
  	NV_DEBUG(drm, "evicting buffers...\n");

-   ttm_bo_evict_mm(>ttm.bdev, TTM_PL_VRAM);
+   ttm_bo_evict_mm(>ttm.bdev, TTM_PL_VRAM, true);
  
  	NV_DEBUG(drm, "waiting for kernel channels to go idle...\n");

if (drm->cechan) {
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index f6b80fe..d8d26c8 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -350,10 +350,10 @@ int qxl_bo_check_id(struct qxl_device *qdev, struct 
qxl_bo *bo)
  
  int