Re: [PATCH 5/5] drm/amdgpu: use metadata members of struct amdgpu_bo_user

2021-03-08 Thread Felix Kuehling 
Am 2021-03-05 um 10:06 a.m. schrieb Nirmoy Das:
> These members are only needed for BOs created by
> amdgpu_gem_object_create(), so we can remove these from the
> base class.
>
> CC: felix.kuehl...@amd.com
> Signed-off-by: Nirmoy Das 

Acked-by: Felix Kuehling 


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |  2 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 48 +++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 --
>  3 files changed, 34 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index c5343a5eecbe..f8c8cbd8ab59 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -494,8 +494,6 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, 
> int dma_buf_fd,
>   *dma_buf_kgd = (struct kgd_dev *)adev;
>   if (bo_size)
>   *bo_size = amdgpu_bo_size(bo);
> - if (metadata_size)
> - *metadata_size = bo->metadata_size;
>   if (metadata_buffer)
>   r = amdgpu_bo_get_metadata(bo, metadata_buffer, buffer_size,
>  metadata_size, _flags);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index ca60943312dc..7c744d90ed34 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -77,6 +77,7 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>  {
>   struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
>   struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> + struct amdgpu_bo_user *ubo;
>  
>   if (bo->pin_count > 0)
>   amdgpu_bo_subtract_pin_size(bo);
> @@ -94,7 +95,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object 
> *tbo)
>   }
>   amdgpu_bo_unref(>parent);
>  
> - kfree(bo->metadata);
> + if (bo->tbo.type == ttm_bo_type_device) {
> + ubo = container_of((bo), struct amdgpu_bo_user, bo);
> + kfree(ubo->metadata);
> + }
> +
>   kfree(bo);
>  }
>  
> @@ -1236,13 +1241,20 @@ void amdgpu_bo_get_tiling_flags(struct amdgpu_bo *bo, 
> u64 *tiling_flags)
>  int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void *metadata,
>   uint32_t metadata_size, uint64_t flags)
>  {
> + struct amdgpu_bo_user *ubo;
>   void *buffer;
>  
> + if (bo->tbo.type != ttm_bo_type_device) {
> + DRM_ERROR("can not set metadata for a non-amdgpu_bo_user type 
> BO\n");
> + return -EINVAL;
> + }
> +
> + ubo = container_of((bo), struct amdgpu_bo_user, bo);
>   if (!metadata_size) {
> - if (bo->metadata_size) {
> - kfree(bo->metadata);
> - bo->metadata = NULL;
> - bo->metadata_size = 0;
> + if (ubo->metadata_size) {
> + kfree(ubo->metadata);
> + ubo->metadata = NULL;
> + ubo->metadata_size = 0;
>   }
>   return 0;
>   }
> @@ -1254,10 +1266,10 @@ int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, 
> void *metadata,
>   if (buffer == NULL)
>   return -ENOMEM;
>  
> - kfree(bo->metadata);
> - bo->metadata_flags = flags;
> - bo->metadata = buffer;
> - bo->metadata_size = metadata_size;
> + kfree(ubo->metadata);
> + ubo->metadata_flags = flags;
> + ubo->metadata = buffer;
> + ubo->metadata_size = metadata_size;
>  
>   return 0;
>  }
> @@ -1281,21 +1293,29 @@ int amdgpu_bo_get_metadata(struct amdgpu_bo *bo, void 
> *buffer,
>  size_t buffer_size, uint32_t *metadata_size,
>  uint64_t *flags)
>  {
> + struct amdgpu_bo_user *ubo;
> +
>   if (!buffer && !metadata_size)
>   return -EINVAL;
>  
> + if (bo->tbo.type != ttm_bo_type_device) {
> + DRM_ERROR("can not get metadata for a non-amdgpu_bo_user type 
> BO\n");
> + return -EINVAL;
> + }
> +
> + ubo = container_of((bo), struct amdgpu_bo_user, bo);
>   if (buffer) {
> - if (buffer_size < bo->metadata_size)
> + if (buffer_size < ubo->metadata_size)
>   return -EINVAL;
>  
> - if (bo->metadata_size)
> - memcpy(buffer, bo->metadata, bo->metadata_size);
> + if (ubo->metadata_size)
> + memcpy(buffer, ubo->metadata, ubo->metadata_size);
>   }
>  
>   if (metadata_size)
> - *metadata_size = bo->metadata_size;
> + *metadata_size = ubo->metadata_size;
>   if (flags)
> - *flags = bo->metadata_flags;
> + *flags = ubo->metadata_flags;
>  
>   return 0;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index

Re: [PATCH 5/5] drm/amdgpu: use metadata members of struct amdgpu_bo_user

2021-03-05 Thread Christian König 

Am 05.03.21 um 15:35 schrieb Nirmoy Das:

These memebers are only needed for BOs created by amdgpu_gem_object_create(),
so we can remove these from the base class.


This patch must be squashed together with the previous one or we would 
have a broken driver in between.


Christian.



CC:felix.kuehl...@amd.com
Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |  2 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 48 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 --
  3 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index c5343a5eecbe..f8c8cbd8ab59 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -494,8 +494,6 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, int 
dma_buf_fd,
*dma_buf_kgd = (struct kgd_dev *)adev;
if (bo_size)
*bo_size = amdgpu_bo_size(bo);
-   if (metadata_size)
-   *metadata_size = bo->metadata_size;
if (metadata_buffer)
r = amdgpu_bo_get_metadata(bo, metadata_buffer, buffer_size,
   metadata_size, _flags);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 2e5cf46251af..81b2cff42481 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -77,6 +77,7 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
  {
struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+   struct amdgpu_bo_user *ubo;
  
  	if (bo->pin_count > 0)

amdgpu_bo_subtract_pin_size(bo);
@@ -94,7 +95,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
}
amdgpu_bo_unref(>parent);
  
-	kfree(bo->metadata);

+   if (bo->tbo.type == ttm_bo_type_device) {
+   ubo = container_of((bo), struct amdgpu_bo_user, bo);
+   kfree(ubo->metadata);
+   }
+
kfree(bo);
  }
  
@@ -1230,13 +1235,20 @@ void amdgpu_bo_get_tiling_flags(struct amdgpu_bo *bo, u64 *tiling_flags)

  int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void *metadata,
uint32_t metadata_size, uint64_t flags)
  {
+   struct amdgpu_bo_user *ubo;
void *buffer;
  
+	if (bo->tbo.type != ttm_bo_type_device) {

+   DRM_ERROR("can not set metadata for a non-amdgpu_bo_user type 
BO\n");
+   return -EINVAL;
+   }
+
+   ubo = container_of((bo), struct amdgpu_bo_user, bo);
if (!metadata_size) {
-   if (bo->metadata_size) {
-   kfree(bo->metadata);
-   bo->metadata = NULL;
-   bo->metadata_size = 0;
+   if (ubo->metadata_size) {
+   kfree(ubo->metadata);
+   ubo->metadata = NULL;
+   ubo->metadata_size = 0;
}
return 0;
}
@@ -1248,10 +1260,10 @@ int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void 
*metadata,
if (buffer == NULL)
return -ENOMEM;
  
-	kfree(bo->metadata);

-   bo->metadata_flags = flags;
-   bo->metadata = buffer;
-   bo->metadata_size = metadata_size;
+   kfree(ubo->metadata);
+   ubo->metadata_flags = flags;
+   ubo->metadata = buffer;
+   ubo->metadata_size = metadata_size;
  
  	return 0;

  }
@@ -1275,21 +1287,29 @@ int amdgpu_bo_get_metadata(struct amdgpu_bo *bo, void 
*buffer,
   size_t buffer_size, uint32_t *metadata_size,
   uint64_t *flags)
  {
+   struct amdgpu_bo_user *ubo;
+
if (!buffer && !metadata_size)
return -EINVAL;
  
+	if (bo->tbo.type != ttm_bo_type_device) {

+   DRM_ERROR("can not get metadata for a non-amdgpu_bo_user type 
BO\n");
+   return -EINVAL;
+   }
+
+   ubo = container_of((bo), struct amdgpu_bo_user, bo);
if (buffer) {
-   if (buffer_size < bo->metadata_size)
+   if (buffer_size < ubo->metadata_size)
return -EINVAL;
  
-		if (bo->metadata_size)

-   memcpy(buffer, bo->metadata, bo->metadata_size);
+   if (ubo->metadata_size)
+   memcpy(buffer, ubo->metadata, ubo->metadata_size);
}
  
  	if (metadata_size)

-   *metadata_size = bo->metadata_size;
+   *metadata_size = ubo->metadata_size;
if (flags)
-   *flags = bo->metadata_flags;
+   *flags = ubo->metadata_flags;
  
  	return 0;

  }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 6cc38b71f7ca..466081fa2083 100644
---

[PATCH 5/5] drm/amdgpu: use metadata members of struct amdgpu_bo_user

2021-03-05 Thread Nirmoy Das 
These members are only needed for BOs created by
amdgpu_gem_object_create(), so we can remove these from the
base class.

CC: felix.kuehl...@amd.com
Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |  2 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 48 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 --
 3 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index c5343a5eecbe..f8c8cbd8ab59 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -494,8 +494,6 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, int 
dma_buf_fd,
*dma_buf_kgd = (struct kgd_dev *)adev;
if (bo_size)
*bo_size = amdgpu_bo_size(bo);
-   if (metadata_size)
-   *metadata_size = bo->metadata_size;
if (metadata_buffer)
r = amdgpu_bo_get_metadata(bo, metadata_buffer, buffer_size,
   metadata_size, _flags);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index ca60943312dc..7c744d90ed34 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -77,6 +77,7 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
 {
struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+   struct amdgpu_bo_user *ubo;
 
if (bo->pin_count > 0)
amdgpu_bo_subtract_pin_size(bo);
@@ -94,7 +95,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
}
amdgpu_bo_unref(>parent);
 
-   kfree(bo->metadata);
+   if (bo->tbo.type == ttm_bo_type_device) {
+   ubo = container_of((bo), struct amdgpu_bo_user, bo);
+   kfree(ubo->metadata);
+   }
+
kfree(bo);
 }
 
@@ -1236,13 +1241,20 @@ void amdgpu_bo_get_tiling_flags(struct amdgpu_bo *bo, 
u64 *tiling_flags)
 int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void *metadata,
uint32_t metadata_size, uint64_t flags)
 {
+   struct amdgpu_bo_user *ubo;
void *buffer;
 
+   if (bo->tbo.type != ttm_bo_type_device) {
+   DRM_ERROR("can not set metadata for a non-amdgpu_bo_user type 
BO\n");
+   return -EINVAL;
+   }
+
+   ubo = container_of((bo), struct amdgpu_bo_user, bo);
if (!metadata_size) {
-   if (bo->metadata_size) {
-   kfree(bo->metadata);
-   bo->metadata = NULL;
-   bo->metadata_size = 0;
+   if (ubo->metadata_size) {
+   kfree(ubo->metadata);
+   ubo->metadata = NULL;
+   ubo->metadata_size = 0;
}
return 0;
}
@@ -1254,10 +1266,10 @@ int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void 
*metadata,
if (buffer == NULL)
return -ENOMEM;
 
-   kfree(bo->metadata);
-   bo->metadata_flags = flags;
-   bo->metadata = buffer;
-   bo->metadata_size = metadata_size;
+   kfree(ubo->metadata);
+   ubo->metadata_flags = flags;
+   ubo->metadata = buffer;
+   ubo->metadata_size = metadata_size;
 
return 0;
 }
@@ -1281,21 +1293,29 @@ int amdgpu_bo_get_metadata(struct amdgpu_bo *bo, void 
*buffer,
   size_t buffer_size, uint32_t *metadata_size,
   uint64_t *flags)
 {
+   struct amdgpu_bo_user *ubo;
+
if (!buffer && !metadata_size)
return -EINVAL;
 
+   if (bo->tbo.type != ttm_bo_type_device) {
+   DRM_ERROR("can not get metadata for a non-amdgpu_bo_user type 
BO\n");
+   return -EINVAL;
+   }
+
+   ubo = container_of((bo), struct amdgpu_bo_user, bo);
if (buffer) {
-   if (buffer_size < bo->metadata_size)
+   if (buffer_size < ubo->metadata_size)
return -EINVAL;
 
-   if (bo->metadata_size)
-   memcpy(buffer, bo->metadata, bo->metadata_size);
+   if (ubo->metadata_size)
+   memcpy(buffer, ubo->metadata, ubo->metadata_size);
}
 
if (metadata_size)
-   *metadata_size = bo->metadata_size;
+   *metadata_size = ubo->metadata_size;
if (flags)
-   *flags = bo->metadata_flags;
+   *flags = ubo->metadata_flags;
 
return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 67c1634fa8bd..5005bb4480e9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -91,9 +91,6 @@ struct amdgpu_bo {
struct

[PATCH 5/5] drm/amdgpu: use metadata members of struct amdgpu_bo_user

2021-03-05 Thread Nirmoy Das 
These memebers are only needed for BOs created by amdgpu_gem_object_create(),
so we can remove these from the base class.

CC:felix.kuehl...@amd.com
Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |  2 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 48 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 --
 3 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index c5343a5eecbe..f8c8cbd8ab59 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -494,8 +494,6 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, int 
dma_buf_fd,
*dma_buf_kgd = (struct kgd_dev *)adev;
if (bo_size)
*bo_size = amdgpu_bo_size(bo);
-   if (metadata_size)
-   *metadata_size = bo->metadata_size;
if (metadata_buffer)
r = amdgpu_bo_get_metadata(bo, metadata_buffer, buffer_size,
   metadata_size, _flags);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 2e5cf46251af..81b2cff42481 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -77,6 +77,7 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
 {
struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+   struct amdgpu_bo_user *ubo;
 
if (bo->pin_count > 0)
amdgpu_bo_subtract_pin_size(bo);
@@ -94,7 +95,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
}
amdgpu_bo_unref(>parent);
 
-   kfree(bo->metadata);
+   if (bo->tbo.type == ttm_bo_type_device) {
+   ubo = container_of((bo), struct amdgpu_bo_user, bo);
+   kfree(ubo->metadata);
+   }
+
kfree(bo);
 }
 
@@ -1230,13 +1235,20 @@ void amdgpu_bo_get_tiling_flags(struct amdgpu_bo *bo, 
u64 *tiling_flags)
 int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void *metadata,
uint32_t metadata_size, uint64_t flags)
 {
+   struct amdgpu_bo_user *ubo;
void *buffer;
 
+   if (bo->tbo.type != ttm_bo_type_device) {
+   DRM_ERROR("can not set metadata for a non-amdgpu_bo_user type 
BO\n");
+   return -EINVAL;
+   }
+
+   ubo = container_of((bo), struct amdgpu_bo_user, bo);
if (!metadata_size) {
-   if (bo->metadata_size) {
-   kfree(bo->metadata);
-   bo->metadata = NULL;
-   bo->metadata_size = 0;
+   if (ubo->metadata_size) {
+   kfree(ubo->metadata);
+   ubo->metadata = NULL;
+   ubo->metadata_size = 0;
}
return 0;
}
@@ -1248,10 +1260,10 @@ int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void 
*metadata,
if (buffer == NULL)
return -ENOMEM;
 
-   kfree(bo->metadata);
-   bo->metadata_flags = flags;
-   bo->metadata = buffer;
-   bo->metadata_size = metadata_size;
+   kfree(ubo->metadata);
+   ubo->metadata_flags = flags;
+   ubo->metadata = buffer;
+   ubo->metadata_size = metadata_size;
 
return 0;
 }
@@ -1275,21 +1287,29 @@ int amdgpu_bo_get_metadata(struct amdgpu_bo *bo, void 
*buffer,
   size_t buffer_size, uint32_t *metadata_size,
   uint64_t *flags)
 {
+   struct amdgpu_bo_user *ubo;
+
if (!buffer && !metadata_size)
return -EINVAL;
 
+   if (bo->tbo.type != ttm_bo_type_device) {
+   DRM_ERROR("can not get metadata for a non-amdgpu_bo_user type 
BO\n");
+   return -EINVAL;
+   }
+
+   ubo = container_of((bo), struct amdgpu_bo_user, bo);
if (buffer) {
-   if (buffer_size < bo->metadata_size)
+   if (buffer_size < ubo->metadata_size)
return -EINVAL;
 
-   if (bo->metadata_size)
-   memcpy(buffer, bo->metadata, bo->metadata_size);
+   if (ubo->metadata_size)
+   memcpy(buffer, ubo->metadata, ubo->metadata_size);
}
 
if (metadata_size)
-   *metadata_size = bo->metadata_size;
+   *metadata_size = ubo->metadata_size;
if (flags)
-   *flags = bo->metadata_flags;
+   *flags = ubo->metadata_flags;
 
return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 6cc38b71f7ca..466081fa2083 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -91,9 +91,6 @@ struct amdgpu_bo {
struct