Re: [Intel-gfx] [PATCH 1/6] drm/ttm: rename and cleanup ttm_bo_init_reserved

2022-07-08 Thread Ruhl, Michael J 
>-Original Message-
>From: dri-devel  On Behalf Of
>Christian König
>Sent: Thursday, July 7, 2022 6:25 AM
>To: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org;
>nouv...@lists.freedesktop.org; amd-...@lists.freedesktop.org
>Cc: Christian König 
>Subject: [PATCH 1/6] drm/ttm: rename and cleanup ttm_bo_init_reserved
>
>Rename ttm_bo_init_reserved to ttm_bo_init_validate since that better
>matches what the function is actually doing.
>
>Remove the unused size parameter, move the function's kerneldoc to the
>implementation and cleanup the whole error handling.
>
>Signed-off-by: Christian König 
>---
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |   2 +-
> drivers/gpu/drm/drm_gem_vram_helper.c  |   6 +-
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c|   5 +-
> drivers/gpu/drm/nouveau/nouveau_bo.c   |   6 +-
> drivers/gpu/drm/qxl/qxl_object.c   |   2 +-
> drivers/gpu/drm/radeon/radeon_object.c |   6 +-
> drivers/gpu/drm/ttm/ttm_bo.c   | 147 +++--
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c |  12 +-
> include/drm/ttm/ttm_bo_api.h   |  93 ++---
> 9 files changed, 129 insertions(+), 150 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>index 2c82b1d5a0d7..d9cfe259f2a9 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>@@ -591,7 +591,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>   if (!bp->destroy)
>   bp->destroy = _bo_destroy;
>
>-  r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, bp-
>>type,
>+  r = ttm_bo_init_reserved(>mman.bdev, >tbo, bp->type,
>>placement, page_align, ,  NULL,
>bp->resv, bp->destroy);
>   if (unlikely(r != 0))
>diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
>b/drivers/gpu/drm/drm_gem_vram_helper.c
>index d607043716d3..125160b534be 100644
>--- a/drivers/gpu/drm/drm_gem_vram_helper.c
>+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>@@ -226,9 +226,9 @@ struct drm_gem_vram_object
>*drm_gem_vram_create(struct drm_device *dev,
>* A failing ttm_bo_init will call ttm_buffer_object_destroy
>* to release gbo->bo.base and kfree gbo.
>*/
>-  ret = ttm_bo_init(bdev, >bo, size, ttm_bo_type_device,
>->placement, pg_align, false, NULL, NULL,
>-ttm_buffer_object_destroy);
>+  ret = ttm_bo_init_validate(bdev, >bo, ttm_bo_type_device,
>+ >placement, pg_align, false, NULL,
>NULL,
>+ ttm_buffer_object_destroy);
>   if (ret)
>   return ERR_PTR(ret);
>
>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>index 4c25d9b2f138..70e2ed4e99df 100644
>--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>@@ -1229,9 +1229,8 @@ int __i915_gem_ttm_object_init(struct
>intel_memory_region *mem,
>* Similarly, in delayed_destroy, we can't call ttm_bo_put()
>* until successful initialization.
>*/
>-  ret = ttm_bo_init_reserved(>bdev, i915_gem_to_ttm(obj),
>size,
>- bo_type, _sys_placement,
>- page_size >> PAGE_SHIFT,
>+  ret = ttm_bo_init_reserved(>bdev, i915_gem_to_ttm(obj),
>bo_type,
>+ _sys_placement, page_size >>
>PAGE_SHIFT,
>  , NULL, NULL, i915_ttm_bo_destroy);
>   if (ret)
>   return i915_ttm_err_to_gem(ret);
>diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
>b/drivers/gpu/drm/nouveau/nouveau_bo.c
>index 05076e530e7d..92cd19021084 100644
>--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
>+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
>@@ -307,9 +307,9 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size,
>int align, u32 domain,
>   nouveau_bo_placement_set(nvbo, domain, 0);
>   INIT_LIST_HEAD(>io_reserve_lru);
>
>-  ret = ttm_bo_init(nvbo->bo.bdev, >bo, size, type,
>->placement, align >> PAGE_SHIFT, false, sg,
>-robj, nouveau_bo_del_ttm);
>+  ret = ttm_bo_init_validate(nvbo->bo.bdev, >bo, type,
>+ >placement, align >> PAGE_SHIFT,
>false,
>+ sg, robj, nouveau_bo_del_ttm);
>   if (ret) {
>   /* ttm will call nouveau_bo_del_ttm if it fails.. */
>   return ret;
>diff --git a/drivers/gpu/drm/qxl/qxl_object.c
>b/drivers/gpu/drm/qxl/qxl_object.c
>index b42a657e4c2f..695d9308d1f0 100644
>--- a/drivers/gpu/drm/qxl/qxl_object.c
>+++ b/drivers/gpu/drm/qxl/qxl_object.c
>@@ -141,7 +141,7 @@ int qxl_bo_create(struct qxl_device *qdev, unsigned
>long size,
>   qxl_ttm_placement_from_domain(bo, domain);
>
>   bo->tbo.priority = priority;
>-  r =

Re: [Intel-gfx] [PATCH 1/6] drm/ttm: rename and cleanup ttm_bo_init_reserved

2022-07-07 Thread kernel test robot 
Hi "Christian,

I love your patch! Perhaps something to improve:

[auto build test WARNING on drm-tip/drm-tip]

url:
https://github.com/intel-lab-lkp/linux/commits/Christian-K-nig/drm-ttm-rename-and-cleanup-ttm_bo_init_reserved/20220707-192538
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-randconfig-a011 
(https://download.01.org/0day-ci/archive/20220708/202207080122.b4dlodd2-...@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
# 
https://github.com/intel-lab-lkp/linux/commit/607b24f78509a1fb0e55c8f9f610113bb0421706
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Christian-K-nig/drm-ttm-rename-and-cleanup-ttm_bo_init_reserved/20220707-192538
git checkout 607b24f78509a1fb0e55c8f9f610113bb0421706
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/ttm/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/ttm/ttm_bo.c:99: warning: Function parameter or member 
'bulk' not described in 'ttm_bo_set_bulk_move'
>> drivers/gpu/drm/ttm/ttm_bo.c:962: warning: expecting prototype for 
>> ttm_bo_init_validate(). Prototype was for ttm_bo_init_reserved() instead


vim +962 drivers/gpu/drm/ttm/ttm_bo.c

ba4e7d973dd09b6 Thomas Hellstrom  2009-06-10   923  
607b24f78509a1f Christian König   2022-07-07   924  /**
607b24f78509a1f Christian König   2022-07-07   925   * ttm_bo_init_validate
607b24f78509a1f Christian König   2022-07-07   926   *
607b24f78509a1f Christian König   2022-07-07   927   * @bdev: Pointer to a 
ttm_device struct.
607b24f78509a1f Christian König   2022-07-07   928   * @bo: Pointer to a 
ttm_buffer_object to be initialized.
607b24f78509a1f Christian König   2022-07-07   929   * @type: Requested type of 
buffer object.
607b24f78509a1f Christian König   2022-07-07   930   * @placement: Initial 
placement for buffer object.
607b24f78509a1f Christian König   2022-07-07   931   * @alignment: Data 
alignment in pages.
607b24f78509a1f Christian König   2022-07-07   932   * @ctx: TTM operation 
context for memory allocation.
607b24f78509a1f Christian König   2022-07-07   933   * @sg: Scatter-gather 
table.
607b24f78509a1f Christian König   2022-07-07   934   * @resv: Pointer to a 
dma_resv, or NULL to let ttm allocate one.
607b24f78509a1f Christian König   2022-07-07   935   * @destroy: Destroy 
function. Use NULL for kfree().
607b24f78509a1f Christian König   2022-07-07   936   *
607b24f78509a1f Christian König   2022-07-07   937   * This function 
initializes a pre-allocated struct ttm_buffer_object.
607b24f78509a1f Christian König   2022-07-07   938   * As this object may be 
part of a larger structure, this function,
607b24f78509a1f Christian König   2022-07-07   939   * together with the 
@destroy function, enables driver-specific objects
607b24f78509a1f Christian König   2022-07-07   940   * derived from a 
ttm_buffer_object.
607b24f78509a1f Christian König   2022-07-07   941   *
607b24f78509a1f Christian König   2022-07-07   942   * On successful return, 
the caller owns an object kref to @bo. The kref and
607b24f78509a1f Christian König   2022-07-07   943   * list_kref are usually 
set to 1, but note that in some situations, other
607b24f78509a1f Christian König   2022-07-07   944   * tasks may already be 
holding references to @bo as well.
607b24f78509a1f Christian König   2022-07-07   945   * Furthermore, if resv == 
NULL, the buffer's reservation lock will be held,
607b24f78509a1f Christian König   2022-07-07   946   * and it is the caller's 
responsibility to call ttm_bo_unreserve.
607b24f78509a1f Christian König   2022-07-07   947   *
607b24f78509a1f Christian König   2022-07-07   948   * If a failure occurs, the 
function will call the @destroy function. Thus,
607b24f78509a1f Christian König   2022-07-07   949   * after a failure, 
dereferencing @bo is illegal and will likely cause memory
607b24f78509a1f Christian König   2022-07-07   950   * corruption.
607b24f78509a1f Christian König   2022-07-07   951   *
607b24f78509a1f Christian König   2022-07-07   952   * Returns
607b24f78509a1f Christian König   2022-07-07   953   * -ENOMEM: Out of memory.
607b24f78509a1f Christian König   2022-07-07   954   * -EINVAL: Invalid 
placement flags.
607b24f78509a1f Christian König   2022-07-07   955   * -ERESTARTSYS: 
Interrupted by signal while sleeping waiting for resources.
607b24f78509a1f Christian König   2022-07-07   956   */
607b24f78509a1f Christian König   2022-07-07   957  int 
ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo,
607b24f78509a1f Christian König   2022-07-07   958   enum 
ttm_bo_type type, struct ttm_placement *placement,
607b24f78509a1f Christian König   2022-07-07   959

[Intel-gfx] [PATCH 1/6] drm/ttm: rename and cleanup ttm_bo_init_reserved

2022-07-07 Thread Christian König 
Rename ttm_bo_init_reserved to ttm_bo_init_validate since that better
matches what the function is actually doing.

Remove the unused size parameter, move the function's kerneldoc to the
implementation and cleanup the whole error handling.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |   2 +-
 drivers/gpu/drm/drm_gem_vram_helper.c  |   6 +-
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c|   5 +-
 drivers/gpu/drm/nouveau/nouveau_bo.c   |   6 +-
 drivers/gpu/drm/qxl/qxl_object.c   |   2 +-
 drivers/gpu/drm/radeon/radeon_object.c |   6 +-
 drivers/gpu/drm/ttm/ttm_bo.c   | 147 +++--
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c |  12 +-
 include/drm/ttm/ttm_bo_api.h   |  93 ++---
 9 files changed, 129 insertions(+), 150 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 2c82b1d5a0d7..d9cfe259f2a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -591,7 +591,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (!bp->destroy)
bp->destroy = _bo_destroy;
 
-   r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, bp->type,
+   r = ttm_bo_init_reserved(>mman.bdev, >tbo, bp->type,
 >placement, page_align, ,  NULL,
 bp->resv, bp->destroy);
if (unlikely(r != 0))
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
b/drivers/gpu/drm/drm_gem_vram_helper.c
index d607043716d3..125160b534be 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -226,9 +226,9 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct 
drm_device *dev,
 * A failing ttm_bo_init will call ttm_buffer_object_destroy
 * to release gbo->bo.base and kfree gbo.
 */
-   ret = ttm_bo_init(bdev, >bo, size, ttm_bo_type_device,
- >placement, pg_align, false, NULL, NULL,
- ttm_buffer_object_destroy);
+   ret = ttm_bo_init_validate(bdev, >bo, ttm_bo_type_device,
+  >placement, pg_align, false, NULL, NULL,
+  ttm_buffer_object_destroy);
if (ret)
return ERR_PTR(ret);
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c 
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 4c25d9b2f138..70e2ed4e99df 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -1229,9 +1229,8 @@ int __i915_gem_ttm_object_init(struct intel_memory_region 
*mem,
 * Similarly, in delayed_destroy, we can't call ttm_bo_put()
 * until successful initialization.
 */
-   ret = ttm_bo_init_reserved(>bdev, i915_gem_to_ttm(obj), size,
-  bo_type, _sys_placement,
-  page_size >> PAGE_SHIFT,
+   ret = ttm_bo_init_reserved(>bdev, i915_gem_to_ttm(obj), bo_type,
+  _sys_placement, page_size >> PAGE_SHIFT,
   , NULL, NULL, i915_ttm_bo_destroy);
if (ret)
return i915_ttm_err_to_gem(ret);
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 05076e530e7d..92cd19021084 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -307,9 +307,9 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int 
align, u32 domain,
nouveau_bo_placement_set(nvbo, domain, 0);
INIT_LIST_HEAD(>io_reserve_lru);
 
-   ret = ttm_bo_init(nvbo->bo.bdev, >bo, size, type,
- >placement, align >> PAGE_SHIFT, false, sg,
- robj, nouveau_bo_del_ttm);
+   ret = ttm_bo_init_validate(nvbo->bo.bdev, >bo, type,
+  >placement, align >> PAGE_SHIFT, false,
+  sg, robj, nouveau_bo_del_ttm);
if (ret) {
/* ttm will call nouveau_bo_del_ttm if it fails.. */
return ret;
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index b42a657e4c2f..695d9308d1f0 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -141,7 +141,7 @@ int qxl_bo_create(struct qxl_device *qdev, unsigned long 
size,
qxl_ttm_placement_from_domain(bo, domain);
 
bo->tbo.priority = priority;
-   r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, type,
+   r = ttm_bo_init_reserved(>mman.bdev, >tbo, type,
 >placement, 0, , NULL, NULL,
 _ttm_bo_destroy);
if (unlikely(r != 0)) {
diff --git a/drivers/gpu/drm/radeon/radeon_object.c 
b/drivers/gpu/drm/radeon/radeon_object.c
index 6c4a6802ca96..00c33b24d5d3 100644
---