[PATCH 1/5] drm/amdgpu: allow variable BO struct creation

[PATCH 1/5] drm/amdgpu: allow variable BO struct creation

[Nirmoy Das]

Allow allocating BO structures with different structure size
than struct amdgpu_bo.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 +++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index ac1bb5089260..d32379cbad89 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -543,9 +543,10 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
 	if (!amdgpu_bo_validate_size(adev, size, bp->domain))
 		return -ENOMEM;
 
-	*bo_ptr = NULL;
+	BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo));
 
-	bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
+	*bo_ptr = NULL;
+	bo = kzalloc(bp->bo_ptr_size, GFP_KERNEL);
 	if (bo == NULL)
 		return -ENOMEM;
 	drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
@@ -635,6 +636,7 @@ static int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
 		AMDGPU_GEM_CREATE_SHADOW;
 	bp.type = ttm_bo_type_kernel;
 	bp.resv = bo->tbo.base.resv;
+	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
 
 	r = amdgpu_bo_do_create(adev, &bp, &bo->shadow);
 	if (!r) {
@@ -669,6 +671,9 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 	int r;
 
 	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
+	if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
+		bp->bo_ptr_size = sizeof(struct amdgpu_bo);
+
 	r = amdgpu_bo_do_create(adev, bp, bo_ptr);
 	if (r)
 		return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 54ceb065e546..8e2b556f0b7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -40,6 +40,7 @@
 struct amdgpu_bo_param {
 	unsigned long			size;
 	int				byte_align;
+	u32				bo_ptr_size;
 	u32				domain;
 	u32				preferred_domain;
 	u64				flags;
-- 
2.30.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/5] drm/amdgpu: introduce struct amdgpu_bo_user

[Nirmoy Das]

Implement a new struct amdgpu_bo_user as subclass of
struct amdgpu_bo and a function to created amdgpu_bo_user
bo with a flag to identify the owner.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 28 ++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 14 +++++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index d32379cbad89..abfeb8304894 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -695,6 +695,34 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 	return r;
 }
 
+/**
+ * amdgpu_bo_create_user - create an &amdgpu_bo_user buffer object
+ * @adev: amdgpu device object
+ * @bp: parameters to be used for the buffer object
+ * @ubo_ptr: pointer to the buffer object pointer
+ *
+ * Create a BO to be used by user application;
+ *
+ * Returns:
+ * 0 for success or a negative error code on failure.
+ */
+
+int amdgpu_bo_create_user(struct amdgpu_device *adev,
+			  struct amdgpu_bo_param *bp,
+			  struct amdgpu_bo_user **ubo_ptr)
+{
+	struct amdgpu_bo *bo_ptr;
+	int r;
+
+	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
+	bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
+	r = amdgpu_bo_do_create(adev, bp, &bo_ptr);
+	if (r)
+		return r;
+
+	*ubo_ptr = amdgpu_bo_to_amdgpu_bo_user(bo_ptr);
+	return r;
+}
 /**
  * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
  * @bo: pointer to the buffer object
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 8e2b556f0b7b..fd30221266c8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -37,6 +37,8 @@
 #define AMDGPU_BO_INVALID_OFFSET	LONG_MAX
 #define AMDGPU_BO_MAX_PLACEMENTS	3
 
+#define amdgpu_bo_to_amdgpu_bo_user(abo) container_of((abo), struct amdgpu_bo_user, bo)
+
 struct amdgpu_bo_param {
 	unsigned long			size;
 	int				byte_align;
@@ -112,6 +114,15 @@ struct amdgpu_bo {
 	struct kgd_mem                  *kfd_bo;
 };
 
+struct amdgpu_bo_user {
+	struct amdgpu_bo		bo;
+	u64				tiling_flags;
+	u64				metadata_flags;
+	void				*metadata;
+	u32				metadata_size;
+
+};
+
 static inline struct amdgpu_bo *ttm_to_amdgpu_bo(struct ttm_buffer_object *tbo)
 {
 	return container_of(tbo, struct amdgpu_bo, tbo);
@@ -255,6 +266,9 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
 int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
 			       uint64_t offset, uint64_t size, uint32_t domain,
 			       struct amdgpu_bo **bo_ptr, void **cpu_addr);
+int amdgpu_bo_create_user(struct amdgpu_device *adev,
+			  struct amdgpu_bo_param *bp,
+			  struct amdgpu_bo_user **ubo_ptr);
 void amdgpu_bo_free_kernel(struct amdgpu_bo **bo, u64 *gpu_addr,
 			   void **cpu_addr);
 int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr);
-- 
2.30.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/5] drm/amdgpu: fb BO should be ttm_bo_type_device

[Nirmoy Das]

FB BO should not be ttm_bo_type_kernel type and
amdgpufb_create_pinned_object() pins the FB BO anyway.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index 51cd49c6f38f..24010cacf7d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -146,7 +146,7 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
 	size = mode_cmd->pitches[0] * height;
 	aligned_size = ALIGN(size, PAGE_SIZE);
 	ret = amdgpu_gem_object_create(adev, aligned_size, 0, domain, flags,
-				       ttm_bo_type_kernel, NULL, &gobj);
+				       ttm_bo_type_device, NULL, &gobj);
 	if (ret) {
 		pr_err("failed to allocate framebuffer (%d)\n", aligned_size);
 		return -ENOMEM;
-- 
2.30.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 4/5] drm/amdgpu: use amdgpu_bo_create_user() for when possible

[Nirmoy Das]

Use amdgpu_bo_create_user() for all the BO allocations for
ttm_bo_type_device type.

CC: felix.kuehling@amd.com
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 4 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index f44185f512de..00ac5c272f47 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -316,6 +316,7 @@ int amdgpu_amdkfd_alloc_gws(struct kgd_dev *kgd, size_t size,
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)kgd;
 	struct amdgpu_bo *bo = NULL;
+	struct amdgpu_bo_user *ubo;
 	struct amdgpu_bo_param bp;
 	int r;

@@ -327,13 +328,14 @@ int amdgpu_amdkfd_alloc_gws(struct kgd_dev *kgd, size_t size,
 	bp.type = ttm_bo_type_device;
 	bp.resv = NULL;

-	r = amdgpu_bo_create(adev, &bp, &bo);
+	r = amdgpu_bo_create_user(adev, &bp, &ubo);
 	if (r) {
 		dev_err(adev->dev,
 			"failed to allocate gws BO for amdkfd (%d)\n", r);
 		return r;
 	}

+	bo = &ubo->bo;
 	*mem_obj = bo;
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index fb7171e5507c..beff96ddc0b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -58,6 +58,7 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
 			     struct drm_gem_object **obj)
 {
 	struct amdgpu_bo *bo;
+	struct amdgpu_bo_user *ubo;
 	struct amdgpu_bo_param bp;
 	int r;

@@ -71,10 +72,11 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
 	bp.preferred_domain = initial_domain;
 	bp.flags = flags;
 	bp.domain = initial_domain;
-	r = amdgpu_bo_create(adev, &bp, &bo);
+	r = amdgpu_bo_create_user(adev, &bp, &ubo);
 	if (r)
 		return r;

+	bo = &ubo->bo;
 	*obj = &bo->tbo.base;
 	(*obj)->funcs = &amdgpu_gem_object_funcs;

--
2.30.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 5/5] drm/amdgpu: use amdgpu_bo_user bo for metadata and tiling flag

[Nirmoy Das]

Tiling flag and metadata are only needed for BOs created by
amdgpu_gem_object_create(), so we can remove those from the
base class.

CC: felix.kuehling@amd.com
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |  2 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 59 ++++++++++++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  4 --
 3 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 00ac5c272f47..04a19cdc08c2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -496,8 +496,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, &metadata_flags);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index abfeb8304894..c105ba96dd58 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->tbo.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(&bo->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);
 }

@@ -1161,12 +1166,15 @@ int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
 int amdgpu_bo_set_tiling_flags(struct amdgpu_bo *bo, u64 tiling_flags)
 {
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+	struct amdgpu_bo_user *ubo;

+	BUG_ON(bo->tbo.type != ttm_bo_type_device);
 	if (adev->family <= AMDGPU_FAMILY_CZ &&
 	    AMDGPU_TILING_GET(tiling_flags, TILE_SPLIT) > 6)
 		return -EINVAL;

-	bo->tiling_flags = tiling_flags;
+	ubo = container_of((bo), struct amdgpu_bo_user, bo);
+	ubo->tiling_flags = tiling_flags;
 	return 0;
 }

@@ -1180,10 +1188,14 @@ int amdgpu_bo_set_tiling_flags(struct amdgpu_bo *bo, u64 tiling_flags)
  */
 void amdgpu_bo_get_tiling_flags(struct amdgpu_bo *bo, u64 *tiling_flags)
 {
+	struct amdgpu_bo_user *ubo;
+
+	BUG_ON(bo->tbo.type != ttm_bo_type_device);
 	dma_resv_assert_held(bo->tbo.base.resv);
+	ubo = amdgpu_bo_to_amdgpu_bo_user(bo);

 	if (tiling_flags)
-		*tiling_flags = bo->tiling_flags;
+		*tiling_flags = ubo->tiling_flags;
 }

 /**
@@ -1202,13 +1214,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 = amdgpu_bo_to_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;
 	}
@@ -1220,10 +1239,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;
 }
@@ -1247,21 +1266,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 fd30221266c8..a733b6323c0b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -92,10 +92,6 @@ struct amdgpu_bo {
 	struct ttm_buffer_object	tbo;
 	struct ttm_bo_kmap_obj		kmap;
 	u64				flags;
-	u64				tiling_flags;
-	u64				metadata_flags;
-	void				*metadata;
-	u32				metadata_size;
 	unsigned			prime_shared_count;
 	/* per VM structure for page tables and with virtual addresses */
 	struct amdgpu_vm_bo_base	*vm_bo;
--
2.30.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/amdgpu: allow variable BO struct creation

[Christian König]

Am 08.03.21 um 16:37 schrieb Nirmoy Das:
> Allow allocating BO structures with different structure size
> than struct amdgpu_bo.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 +++++++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 +
>   2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index ac1bb5089260..d32379cbad89 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -543,9 +543,10 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
>   	if (!amdgpu_bo_validate_size(adev, size, bp->domain))
>   		return -ENOMEM;
>   
> -	*bo_ptr = NULL;
> +	BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo));
>   
> -	bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
> +	*bo_ptr = NULL;
> +	bo = kzalloc(bp->bo_ptr_size, GFP_KERNEL);
>   	if (bo == NULL)
>   		return -ENOMEM;
>   	drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
> @@ -635,6 +636,7 @@ static int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
>   		AMDGPU_GEM_CREATE_SHADOW;
>   	bp.type = ttm_bo_type_kernel;
>   	bp.resv = bo->tbo.base.resv;
> +	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
>   
>   	r = amdgpu_bo_do_create(adev, &bp, &bo->shadow);
>   	if (!r) {
> @@ -669,6 +671,9 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>   	int r;
>   
>   	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
> +	if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
> +		bp->bo_ptr_size = sizeof(struct amdgpu_bo);
> +

It's not strictly necessary, but I would prefer if you change all 
callers of amdgpu_bo_create() to correctly do this instead of the check 
here.

Christian.

>   	r = amdgpu_bo_do_create(adev, bp, bo_ptr);
>   	if (r)
>   		return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 54ceb065e546..8e2b556f0b7b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -40,6 +40,7 @@
>   struct amdgpu_bo_param {
>   	unsigned long			size;
>   	int				byte_align;
> +	u32				bo_ptr_size;
>   	u32				domain;
>   	u32				preferred_domain;
>   	u64				flags;

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: fb BO should be ttm_bo_type_device

[Christian König]

Am 08.03.21 um 16:37 schrieb Nirmoy Das:
> FB BO should not be ttm_bo_type_kernel type and
> amdgpufb_create_pinned_object() pins the FB BO anyway.

Mhm, why the heck was that a kernel object?

>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>

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

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> index 51cd49c6f38f..24010cacf7d0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> @@ -146,7 +146,7 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
>   	size = mode_cmd->pitches[0] * height;
>   	aligned_size = ALIGN(size, PAGE_SIZE);
>   	ret = amdgpu_gem_object_create(adev, aligned_size, 0, domain, flags,
> -				       ttm_bo_type_kernel, NULL, &gobj);
> +				       ttm_bo_type_device, NULL, &gobj);
>   	if (ret) {
>   		pr_err("failed to allocate framebuffer (%d)\n", aligned_size);
>   		return -ENOMEM;

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm/amdgpu: use amdgpu_bo_user bo for metadata and tiling flag

[Christian König]

Am 08.03.21 um 16:37 schrieb Nirmoy Das:
> Tiling flag and metadata are only needed for BOs created by
> amdgpu_gem_object_create(), so we can remove those from the
> base class.
>
> CC: felix.kuehling@amd.com
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |  2 -
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 59 ++++++++++++++++------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  4 --
>   3 files changed, 43 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 00ac5c272f47..04a19cdc08c2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -496,8 +496,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, &metadata_flags);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index abfeb8304894..c105ba96dd58 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->tbo.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(&bo->parent);
>
> -	kfree(bo->metadata);
> +	if (bo->tbo.type == ttm_bo_type_device) {
> +		ubo = container_of((bo), struct amdgpu_bo_user, bo);

You could use your new casting macro here.

> +		kfree(ubo->metadata);
> +	}
> +
>   	kfree(bo);
>   }
>
> @@ -1161,12 +1166,15 @@ int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
>   int amdgpu_bo_set_tiling_flags(struct amdgpu_bo *bo, u64 tiling_flags)
>   {
>   	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> +	struct amdgpu_bo_user *ubo;
>
> +	BUG_ON(bo->tbo.type != ttm_bo_type_device);
>   	if (adev->family <= AMDGPU_FAMILY_CZ &&
>   	    AMDGPU_TILING_GET(tiling_flags, TILE_SPLIT) > 6)
>   		return -EINVAL;
>
> -	bo->tiling_flags = tiling_flags;
> +	ubo = container_of((bo), struct amdgpu_bo_user, bo);
> +	ubo->tiling_flags = tiling_flags;
>   	return 0;
>   }
>
> @@ -1180,10 +1188,14 @@ int amdgpu_bo_set_tiling_flags(struct amdgpu_bo *bo, u64 tiling_flags)
>    */
>   void amdgpu_bo_get_tiling_flags(struct amdgpu_bo *bo, u64 *tiling_flags)
>   {
> +	struct amdgpu_bo_user *ubo;
> +
> +	BUG_ON(bo->tbo.type != ttm_bo_type_device);
>   	dma_resv_assert_held(bo->tbo.base.resv);
> +	ubo = amdgpu_bo_to_amdgpu_bo_user(bo);
>
>   	if (tiling_flags)
> -		*tiling_flags = bo->tiling_flags;
> +		*tiling_flags = ubo->tiling_flags;
>   }
>
>   /**
> @@ -1202,13 +1214,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;
> +	}

Either BUG_ON or DRM_ERROR, but keep that consistent please.

Christian.

> +
> +	ubo = amdgpu_bo_to_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;
>   	}
> @@ -1220,10 +1239,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;
>   }
> @@ -1247,21 +1266,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 fd30221266c8..a733b6323c0b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -92,10 +92,6 @@ struct amdgpu_bo {
>   	struct ttm_buffer_object	tbo;
>   	struct ttm_bo_kmap_obj		kmap;
>   	u64				flags;
> -	u64				tiling_flags;
> -	u64				metadata_flags;
> -	void				*metadata;
> -	u32				metadata_size;
>   	unsigned			prime_shared_count;
>   	/* per VM structure for page tables and with virtual addresses */
>   	struct amdgpu_vm_bo_base	*vm_bo;
> --
> 2.30.1
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/5] drm/amdgpu: introduce struct amdgpu_bo_user

[Christian König]

Am 08.03.21 um 16:37 schrieb Nirmoy Das:
> Implement a new struct amdgpu_bo_user as subclass of
> struct amdgpu_bo and a function to created amdgpu_bo_user
> bo with a flag to identify the owner.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 28 ++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 14 +++++++++++
>   2 files changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index d32379cbad89..abfeb8304894 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -695,6 +695,34 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>   	return r;
>   }
>   
> +/**
> + * amdgpu_bo_create_user - create an &amdgpu_bo_user buffer object
> + * @adev: amdgpu device object
> + * @bp: parameters to be used for the buffer object
> + * @ubo_ptr: pointer to the buffer object pointer
> + *
> + * Create a BO to be used by user application;
> + *
> + * Returns:
> + * 0 for success or a negative error code on failure.
> + */
> +
> +int amdgpu_bo_create_user(struct amdgpu_device *adev,
> +			  struct amdgpu_bo_param *bp,
> +			  struct amdgpu_bo_user **ubo_ptr)
> +{
> +	struct amdgpu_bo *bo_ptr;
> +	int r;
> +
> +	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
> +	bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
> +	r = amdgpu_bo_do_create(adev, bp, &bo_ptr);
> +	if (r)
> +		return r;
> +
> +	*ubo_ptr = amdgpu_bo_to_amdgpu_bo_user(bo_ptr);
> +	return r;
> +}
>   /**
>    * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
>    * @bo: pointer to the buffer object
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 8e2b556f0b7b..fd30221266c8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -37,6 +37,8 @@
>   #define AMDGPU_BO_INVALID_OFFSET	LONG_MAX
>   #define AMDGPU_BO_MAX_PLACEMENTS	3
>   
> +#define amdgpu_bo_to_amdgpu_bo_user(abo) container_of((abo), struct amdgpu_bo_user, bo)

Mhm, the name could be improved, but apart from that the patch looks 
good to me.

Christian.

> +
>   struct amdgpu_bo_param {
>   	unsigned long			size;
>   	int				byte_align;
> @@ -112,6 +114,15 @@ struct amdgpu_bo {
>   	struct kgd_mem                  *kfd_bo;
>   };
>   
> +struct amdgpu_bo_user {
> +	struct amdgpu_bo		bo;
> +	u64				tiling_flags;
> +	u64				metadata_flags;
> +	void				*metadata;
> +	u32				metadata_size;
> +
> +};
> +
>   static inline struct amdgpu_bo *ttm_to_amdgpu_bo(struct ttm_buffer_object *tbo)
>   {
>   	return container_of(tbo, struct amdgpu_bo, tbo);
> @@ -255,6 +266,9 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
>   int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
>   			       uint64_t offset, uint64_t size, uint32_t domain,
>   			       struct amdgpu_bo **bo_ptr, void **cpu_addr);
> +int amdgpu_bo_create_user(struct amdgpu_device *adev,
> +			  struct amdgpu_bo_param *bp,
> +			  struct amdgpu_bo_user **ubo_ptr);
>   void amdgpu_bo_free_kernel(struct amdgpu_bo **bo, u64 *gpu_addr,
>   			   void **cpu_addr);
>   int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr);

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: fb BO should be ttm_bo_type_device

[Alex Deucher]

On Mon, Mar 8, 2021 at 3:20 PM Christian König <christian.koenig@amd.com> wrote:
>
> Am 08.03.21 um 16:37 schrieb Nirmoy Das:
> > FB BO should not be ttm_bo_type_kernel type and
> > amdgpufb_create_pinned_object() pins the FB BO anyway.
>
> Mhm, why the heck was that a kernel object?

Maybe because the fbcon was the main user for this historically and
the code was copied from radeon which also still sets it to kernel.

Alex

>
> >
> > Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>
> Acked-by: Christian König <christian.koenig@amd.com>
>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> > index 51cd49c6f38f..24010cacf7d0 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> > @@ -146,7 +146,7 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
> >       size = mode_cmd->pitches[0] * height;
> >       aligned_size = ALIGN(size, PAGE_SIZE);
> >       ret = amdgpu_gem_object_create(adev, aligned_size, 0, domain, flags,
> > -                                    ttm_bo_type_kernel, NULL, &gobj);
> > +                                    ttm_bo_type_device, NULL, &gobj);
> >       if (ret) {
> >               pr_err("failed to allocate framebuffer (%d)\n", aligned_size);
> >               return -ENOMEM;
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: fb BO should be ttm_bo_type_device

[Christian König]

Am 08.03.21 um 21:34 schrieb Alex Deucher:
> On Mon, Mar 8, 2021 at 3:20 PM Christian König <christian.koenig@amd.com> wrote:
>> Am 08.03.21 um 16:37 schrieb Nirmoy Das:
>>> FB BO should not be ttm_bo_type_kernel type and
>>> amdgpufb_create_pinned_object() pins the FB BO anyway.
>> Mhm, why the heck was that a kernel object?
> Maybe because the fbcon was the main user for this historically and
> the code was copied from radeon which also still sets it to kernel.

That's most likely wrong for radeon as well.

All BOs which can be mapped using mmap() into an userspace process 
should be of type device if I'm not completely mistaken.

Going to double check that stuff when I have time.

Thanks for pointing this out Nirmoy.

Christian.

>
> Alex
>
>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> Acked-by: Christian König <christian.koenig@amd.com>
>>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>> index 51cd49c6f38f..24010cacf7d0 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>> @@ -146,7 +146,7 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
>>>        size = mode_cmd->pitches[0] * height;
>>>        aligned_size = ALIGN(size, PAGE_SIZE);
>>>        ret = amdgpu_gem_object_create(adev, aligned_size, 0, domain, flags,
>>> -                                    ttm_bo_type_kernel, NULL, &gobj);
>>> +                                    ttm_bo_type_device, NULL, &gobj);
>>>        if (ret) {
>>>                pr_err("failed to allocate framebuffer (%d)\n", aligned_size);
>>>                return -ENOMEM;
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7Cbe4189aa363c4f65f38208d8e271949e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637508324745711394%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=NYv5BaFhU7hahVnN4e086QFv71GAXEZLeFnn2esK04o%3D&amp;reserved=0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx