AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 1/3] drm/amdgpu: rework MMIO_REMAP BO creation
@ 2025-12-02 15:12 Christian König
  2025-12-02 15:12 ` [RFC PATCH 2/3] drm/amdgpu: add AMDGPU_GEM_OP_OPEN_GLOBAL Christian König
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Christian König @ 2025-12-02 15:12 UTC (permalink / raw)
  To: alexdeucher, srinivasan.shanmugam, Leo.Liu, Ruijing.Dong, amd-gfx

First of all avoid using AMDGPU_GEM_DOMAIN_MMIO_REMAP in the TTM code.

Then while at it remove some confusing comments, cleanup the comments
who make sense and rename the functions to be a bit more clear what they
do.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 66 ++++++++++++++-----------
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index e553cf411191..3166469d437a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1892,42 +1892,41 @@ static void amdgpu_ttm_pools_fini(struct amdgpu_device *adev)
 }
 
 /**
- * amdgpu_ttm_mmio_remap_bo_init - Allocate the singleton 4K MMIO_REMAP BO
+ * amdgpu_ttm_mmio_remap_bo_init - Allocate the singleton MMIO_REMAP BO
  * @adev: amdgpu device
  *
- * Allocates a one-page (4K) GEM BO in AMDGPU_GEM_DOMAIN_MMIO_REMAP when the
+ * Allocates a global BO with backing AMDGPU_PL_MMIO_REMAP when the
  * hardware exposes a remap base (adev->rmmio_remap.bus_addr) and the host
  * PAGE_SIZE is <= AMDGPU_GPU_PAGE_SIZE (4K). The BO is created as a regular
  * GEM object (amdgpu_bo_create).
  *
- * The BO is created as a normal GEM object via amdgpu_bo_create(), then
- * reserved and pinned at the TTM level (ttm_bo_pin()) so it can never be
- * migrated or evicted. No CPU mapping is established here.
- *
  * Return:
  *  * 0 on success or intentional skip (feature not present/unsupported)
  *  * negative errno on allocation failure
  */
-static int amdgpu_ttm_mmio_remap_bo_init(struct amdgpu_device *adev)
+static int amdgpu_ttm_alloc_mmio_remap_bo(struct amdgpu_device *adev)
 {
+	struct ttm_operation_ctx ctx = { false, false };
+	struct ttm_placement placement;
+	struct ttm_buffer_object *tbo;
+	struct ttm_place placements;
 	struct amdgpu_bo_param bp;
+	struct ttm_resource *tmp;
 	int r;
 
 	/* Skip if HW doesn't expose remap, or if PAGE_SIZE > AMDGPU_GPU_PAGE_SIZE (4K). */
 	if (!adev->rmmio_remap.bus_addr || PAGE_SIZE > AMDGPU_GPU_PAGE_SIZE)
 		return 0;
 
+	/* Allocate an empty BO without backing store */
 	memset(&bp, 0, sizeof(bp));
-
-	/* Create exactly one GEM BO in the MMIO_REMAP domain. */
-	bp.type        = ttm_bo_type_device;          /* userspace-mappable GEM */
-	bp.size        = AMDGPU_GPU_PAGE_SIZE;        /* 4K */
+	bp.type        = ttm_bo_type_device;
+	bp.size        = AMDGPU_GPU_PAGE_SIZE;
 	bp.byte_align  = AMDGPU_GPU_PAGE_SIZE;
-	bp.domain      = AMDGPU_GEM_DOMAIN_MMIO_REMAP;
+	bp.domain      = 0;
 	bp.flags       = 0;
 	bp.resv        = NULL;
 	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
-
 	r = amdgpu_bo_create(adev, &bp, &adev->rmmio_remap.bo);
 	if (r)
 		return r;
@@ -1936,37 +1935,48 @@ static int amdgpu_ttm_mmio_remap_bo_init(struct amdgpu_device *adev)
 	if (r)
 		goto err_unref;
 
+	tbo = &adev->rmmio_remap.bo->tbo;
+
 	/*
 	 * MMIO_REMAP is a fixed I/O placement (AMDGPU_PL_MMIO_REMAP).
-	 * Use TTM-level pin so the BO cannot be evicted/migrated,
-	 * independent of GEM domains. This
-	 * enforces the “fixed I/O window”
 	 */
-	ttm_bo_pin(&adev->rmmio_remap.bo->tbo);
+	placement.num_placement = 1;
+	placement.placement = &placements;
+	placements.fpfn = 0;
+	placements.lpfn = 0;
+	placements.mem_type = AMDGPU_PL_MMIO_REMAP;
+	placements.flags = 0;
+	r = ttm_bo_mem_space(tbo, &placement, &tmp, &ctx);
+	if (unlikely(r))
+		goto err_unlock;
+
+	ttm_resource_free(tbo, &tbo->resource);
+	ttm_bo_assign_mem(tbo, tmp);
+	ttm_bo_pin(tbo);
 
 	amdgpu_bo_unreserve(adev->rmmio_remap.bo);
 	return 0;
 
+err_unlock:
+	amdgpu_bo_unreserve(adev->rmmio_remap.bo);
+
 err_unref:
-	if (adev->rmmio_remap.bo)
-		amdgpu_bo_unref(&adev->rmmio_remap.bo);
+	amdgpu_bo_unref(&adev->rmmio_remap.bo);
 	adev->rmmio_remap.bo = NULL;
 	return r;
 }
 
 /**
- * amdgpu_ttm_mmio_remap_bo_fini - Free the singleton MMIO_REMAP BO
+ * amdgpu_ttm_free_mmio_remap_bo - Free the singleton MMIO_REMAP BO
  * @adev: amdgpu device
  *
  * Frees the kernel-owned MMIO_REMAP BO if it was allocated by
  * amdgpu_ttm_mmio_remap_bo_init().
  */
-static void amdgpu_ttm_mmio_remap_bo_fini(struct amdgpu_device *adev)
+static void amdgpu_ttm_free_mmio_remap_bo(struct amdgpu_device *adev)
 {
-	struct amdgpu_bo *bo = adev->rmmio_remap.bo;
-
-	if (!bo)
-		return;   /* <-- safest early exit */
+	if (!adev->rmmio_remap.bo)
+		return;
 
 	if (!amdgpu_bo_reserve(adev->rmmio_remap.bo, true)) {
 		ttm_bo_unpin(&adev->rmmio_remap.bo->tbo);
@@ -2152,8 +2162,8 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 		return r;
 	}
 
-	/* Allocate the singleton MMIO_REMAP BO (4K) if supported */
-	r = amdgpu_ttm_mmio_remap_bo_init(adev);
+	/* Allocate the singleton MMIO_REMAP BO if supported */
+	r = amdgpu_ttm_alloc_mmio_remap_bo(adev);
 	if (r)
 		return r;
 
@@ -2220,7 +2230,7 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
 	amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
 					&adev->mman.sdma_access_ptr);
 
-	amdgpu_ttm_mmio_remap_bo_fini(adev);
+	amdgpu_ttm_free_mmio_remap_bo(adev);
 	amdgpu_ttm_fw_reserve_vram_fini(adev);
 	amdgpu_ttm_drv_reserve_vram_fini(adev);
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC PATCH 2/3] drm/amdgpu: add AMDGPU_GEM_OP_OPEN_GLOBAL
  2025-12-02 15:12 [RFC PATCH 1/3] drm/amdgpu: rework MMIO_REMAP BO creation Christian König
@ 2025-12-02 15:12 ` Christian König
  2025-12-10  4:11   ` SHANMUGAM, SRINIVASAN
  2025-12-10 22:06   ` Wu, David
  2025-12-02 15:12 ` [RFC PATCH 3/3] drm/amdgpu: remove AMDGPU_GEM_DOMAIN_DOORBELL Christian König
  2025-12-10  3:22 ` [RFC PATCH 1/3] drm/amdgpu: rework MMIO_REMAP BO creation SHANMUGAM, SRINIVASAN
  2 siblings, 2 replies; 7+ messages in thread
From: Christian König @ 2025-12-02 15:12 UTC (permalink / raw)
  To: alexdeucher, srinivasan.shanmugam, Leo.Liu, Ruijing.Dong, amd-gfx

Instead of abusing the create IOCTL to open global BO add a new
AMDGPU_GEM_OP_OPEN_GLOBAL functionality.

The new AMDGPU_GEM_OP_OPEN_GLOBAL functionality expects an enum which tells
it which global BO to open and copies the information about the BO to
userspace similar to the AMDGPU_GEM_OP_GET_GEM_CREATE_INFO operation.

The advantage is that we don't start overloading the create IOCTL with
tons of special cases and opening the global BOs doesn't requires knowing
the exact size and parameters of it in userspace any more.

Heavily WIP and only compile tested.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 26 ++++++++++++++++++++-----
 include/uapi/drm/amdgpu_drm.h           |  5 ++++-
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9b81a6677f90..9e9b94dcb699 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -968,22 +968,34 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *filp)
 {
+	struct amdgpu_fpriv *fpriv = filp->driver_priv;
 	struct drm_amdgpu_gem_op *args = data;
 	struct drm_gem_object *gobj;
 	struct amdgpu_vm_bo_base *base;
 	struct amdgpu_bo *robj;
 	struct drm_exec exec;
-	struct amdgpu_fpriv *fpriv = filp->driver_priv;
 	int r;
 
 	if (args->padding)
 		return -EINVAL;
 
-	gobj = drm_gem_object_lookup(filp, args->handle);
-	if (!gobj)
-		return -ENOENT;
+	if (args->op == AMDGPU_GEM_OP_OPEN_GLOBAL) {
+		switch (args->handle) {
+		case AMDGPU_GEM_GLOBAL_MMIO_REMAP:
+			robj = drm_to_adev(dev)->rmmio_remap.bo;
+			break;
+		default:
+			return -EINVAL;
+		}
+		gobj = &robj->tbo.base;
+		drm_gem_object_get(gobj);
+	} else {
+		gobj = drm_gem_object_lookup(filp, args->handle);
+		if (!gobj)
+			return -ENOENT;
 
-	robj = gem_to_amdgpu_bo(gobj);
+		robj = gem_to_amdgpu_bo(gobj);
+	}
 
 	drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT |
 			  DRM_EXEC_IGNORE_DUPLICATES, 0);
@@ -1002,6 +1014,7 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
 	}
 
 	switch (args->op) {
+	case AMDGPU_GEM_OP_OPEN_GLOBAL:
 	case AMDGPU_GEM_OP_GET_GEM_CREATE_INFO: {
 		struct drm_amdgpu_gem_create_in info;
 		void __user *out = u64_to_user_ptr(args->value);
@@ -1096,6 +1109,9 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
 		r = -EINVAL;
 	}
 
+	if (!r && args->op == AMDGPU_GEM_OP_OPEN_GLOBAL)
+		r = drm_gem_handle_create(filp, gobj, &args->handle);
+
 	drm_gem_object_put(gobj);
 	return r;
 out_exec:
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index c1336ed4ff75..6927c864a6d1 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -807,6 +807,9 @@ union drm_amdgpu_wait_fences {
 #define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO	0
 #define AMDGPU_GEM_OP_SET_PLACEMENT		1
 #define AMDGPU_GEM_OP_GET_MAPPING_INFO		2
+#define AMDGPU_GEM_OP_OPEN_GLOBAL		3
+
+#define AMDGPU_GEM_GLOBAL_MMIO_REMAP		0
 
 struct drm_amdgpu_gem_vm_entry {
 	/* Start of mapping (in bytes) */
@@ -824,7 +827,7 @@ struct drm_amdgpu_gem_vm_entry {
 
 /* Sets or returns a value associated with a buffer. */
 struct drm_amdgpu_gem_op {
-	/** GEM object handle */
+	/** GEM object handle or AMDGPU_GEM_GLOBAL_* */
 	__u32	handle;
 	/** AMDGPU_GEM_OP_* */
 	__u32	op;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC PATCH 3/3] drm/amdgpu: remove AMDGPU_GEM_DOMAIN_DOORBELL
  2025-12-02 15:12 [RFC PATCH 1/3] drm/amdgpu: rework MMIO_REMAP BO creation Christian König
  2025-12-02 15:12 ` [RFC PATCH 2/3] drm/amdgpu: add AMDGPU_GEM_OP_OPEN_GLOBAL Christian König
@ 2025-12-02 15:12 ` Christian König
  2025-12-10  4:22   ` SHANMUGAM, SRINIVASAN
  2025-12-10  3:22 ` [RFC PATCH 1/3] drm/amdgpu: rework MMIO_REMAP BO creation SHANMUGAM, SRINIVASAN
  2 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2025-12-02 15:12 UTC (permalink / raw)
  To: alexdeucher, srinivasan.shanmugam, Leo.Liu, Ruijing.Dong, amd-gfx

Never activated as UAPI and it turned out that this was to inflexible.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    |  3 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 10 ----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 --
 include/uapi/drm/amdgpu_drm.h              |  6 +-----
 4 files changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9e9b94dcb699..4fd8121f9d95 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -458,9 +458,6 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
 	/* always clear VRAM */
 	flags |= AMDGPU_GEM_CREATE_VRAM_CLEARED;
 
-	if (args->in.domains & AMDGPU_GEM_DOMAIN_MMIO_REMAP)
-		return -EINVAL;
-
 	/* create a gem object to contain this object in */
 	if (args->in.domains & (AMDGPU_GEM_DOMAIN_GDS |
 	    AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA)) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index fe51087a54a9..0584c7383c19 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -153,14 +153,6 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain)
 		c++;
 	}
 
-	if (domain & AMDGPU_GEM_DOMAIN_MMIO_REMAP) {
-		places[c].fpfn = 0;
-		places[c].lpfn = 0;
-		places[c].mem_type = AMDGPU_PL_MMIO_REMAP;
-		places[c].flags = 0;
-		c++;
-	}
-
 	if (domain & AMDGPU_GEM_DOMAIN_GTT) {
 		places[c].fpfn = 0;
 		places[c].lpfn = 0;
@@ -1566,8 +1558,6 @@ uint32_t amdgpu_bo_mem_stats_placement(struct amdgpu_bo *bo)
 		return AMDGPU_PL_OA;
 	case AMDGPU_GEM_DOMAIN_DOORBELL:
 		return AMDGPU_PL_DOORBELL;
-	case AMDGPU_GEM_DOMAIN_MMIO_REMAP:
-		return AMDGPU_PL_MMIO_REMAP;
 	default:
 		return TTM_PL_SYSTEM;
 	}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 52c2d1731aab..912c9afaf9e1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -168,8 +168,6 @@ static inline unsigned amdgpu_mem_type_to_domain(u32 mem_type)
 		return AMDGPU_GEM_DOMAIN_OA;
 	case AMDGPU_PL_DOORBELL:
 		return AMDGPU_GEM_DOMAIN_DOORBELL;
-	case AMDGPU_PL_MMIO_REMAP:
-		return AMDGPU_GEM_DOMAIN_MMIO_REMAP;
 	default:
 		break;
 	}
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 6927c864a6d1..0d743fe92146 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -105,8 +105,6 @@ extern "C" {
  *
  * %AMDGPU_GEM_DOMAIN_DOORBELL	Doorbell. It is an MMIO region for
  * signalling user mode queues.
- *
- * %AMDGPU_GEM_DOMAIN_MMIO_REMAP	MMIO remap page (special mapping for HDP flushing).
  */
 #define AMDGPU_GEM_DOMAIN_CPU		0x1
 #define AMDGPU_GEM_DOMAIN_GTT		0x2
@@ -115,15 +113,13 @@ extern "C" {
 #define AMDGPU_GEM_DOMAIN_GWS		0x10
 #define AMDGPU_GEM_DOMAIN_OA		0x20
 #define AMDGPU_GEM_DOMAIN_DOORBELL	0x40
-#define AMDGPU_GEM_DOMAIN_MMIO_REMAP	0x80
 #define AMDGPU_GEM_DOMAIN_MASK		(AMDGPU_GEM_DOMAIN_CPU | \
 					 AMDGPU_GEM_DOMAIN_GTT | \
 					 AMDGPU_GEM_DOMAIN_VRAM | \
 					 AMDGPU_GEM_DOMAIN_GDS | \
 					 AMDGPU_GEM_DOMAIN_GWS | \
 					 AMDGPU_GEM_DOMAIN_OA |	\
-					 AMDGPU_GEM_DOMAIN_DOORBELL | \
-					 AMDGPU_GEM_DOMAIN_MMIO_REMAP)
+					 AMDGPU_GEM_DOMAIN_DOORBELL)
 
 /* Flag that CPU access will be required for the case of VRAM domain */
 #define AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED	(1 << 0)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* RE: [RFC PATCH 1/3] drm/amdgpu: rework MMIO_REMAP BO creation
  2025-12-02 15:12 [RFC PATCH 1/3] drm/amdgpu: rework MMIO_REMAP BO creation Christian König
  2025-12-02 15:12 ` [RFC PATCH 2/3] drm/amdgpu: add AMDGPU_GEM_OP_OPEN_GLOBAL Christian König
  2025-12-02 15:12 ` [RFC PATCH 3/3] drm/amdgpu: remove AMDGPU_GEM_DOMAIN_DOORBELL Christian König
@ 2025-12-10  3:22 ` SHANMUGAM, SRINIVASAN
  2 siblings, 0 replies; 7+ messages in thread
From: SHANMUGAM, SRINIVASAN @ 2025-12-10  3:22 UTC (permalink / raw)
  To: Christian König, alexdeucher@gmail.com, Liu, Leo,
	Dong, Ruijing, amd-gfx@lists.freedesktop.org

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Christian,

> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Tuesday, December 2, 2025 8:43 PM
> To: alexdeucher@gmail.com; SHANMUGAM, SRINIVASAN
> <SRINIVASAN.SHANMUGAM@amd.com>; Liu, Leo <Leo.Liu@amd.com>; Dong,
> Ruijing <Ruijing.Dong@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: [RFC PATCH 1/3] drm/amdgpu: rework MMIO_REMAP BO creation
>
> First of all avoid using AMDGPU_GEM_DOMAIN_MMIO_REMAP in the TTM code.
>
> Then while at it remove some confusing comments, cleanup the comments who
> make sense and rename the functions to be a bit more clear what they do.

Mentioning Fixes-tag: would be helpful for references, I feel here, to get the continuity from where this was derived for all these series of patches.

>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 66 ++++++++++++++-----------
>  1 file changed, 38 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index e553cf411191..3166469d437a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1892,42 +1892,41 @@ static void amdgpu_ttm_pools_fini(struct
> amdgpu_device *adev)  }
>
>  /**
> - * amdgpu_ttm_mmio_remap_bo_init - Allocate the singleton 4K MMIO_REMAP
> BO
> + * amdgpu_ttm_mmio_remap_bo_init - Allocate the singleton MMIO_REMAP BO
>   * @adev: amdgpu device
>   *
> - * Allocates a one-page (4K) GEM BO in
> AMDGPU_GEM_DOMAIN_MMIO_REMAP when the
> + * Allocates a global BO with backing AMDGPU_PL_MMIO_REMAP when the
>   * hardware exposes a remap base (adev->rmmio_remap.bus_addr) and the host
>   * PAGE_SIZE is <= AMDGPU_GPU_PAGE_SIZE (4K). The BO is created as a
> regular
>   * GEM object (amdgpu_bo_create).
>   *
> - * The BO is created as a normal GEM object via amdgpu_bo_create(), then
> - * reserved and pinned at the TTM level (ttm_bo_pin()) so it can never be
> - * migrated or evicted. No CPU mapping is established here.
> - *
>   * Return:
>   *  * 0 on success or intentional skip (feature not present/unsupported)
>   *  * negative errno on allocation failure
>   */
> -static int amdgpu_ttm_mmio_remap_bo_init(struct amdgpu_device *adev)
> +static int amdgpu_ttm_alloc_mmio_remap_bo(struct amdgpu_device *adev)
>  {
> +     struct ttm_operation_ctx ctx = { false, false };
> +     struct ttm_placement placement;
> +     struct ttm_buffer_object *tbo;
> +     struct ttm_place placements;
>       struct amdgpu_bo_param bp;
> +     struct ttm_resource *tmp;
>       int r;
>
>       /* Skip if HW doesn't expose remap, or if PAGE_SIZE >
> AMDGPU_GPU_PAGE_SIZE (4K). */
>       if (!adev->rmmio_remap.bus_addr || PAGE_SIZE >
> AMDGPU_GPU_PAGE_SIZE)
>               return 0;
>
> +     /* Allocate an empty BO without backing store */

the “empty BO without backing store” wording is a bit confusing here since amdgpu_bo_create() still assigns an initial TTM resource which we then replace with a AMDGPU_PL_MMIO_REMAP resource via amdgpu_ttm_alloc_mmio_remap_bo (). Maybe rephrase the comment to something like “allocate a BO and then move it to AMDGPU_PL_MMIO_REMAP” so the comment better matches the actual flow.

>       memset(&bp, 0, sizeof(bp));
> -
> -     /* Create exactly one GEM BO in the MMIO_REMAP domain. */
> -     bp.type        = ttm_bo_type_device;          /* userspace-mappable GEM */
> -     bp.size        = AMDGPU_GPU_PAGE_SIZE;        /* 4K */
> +     bp.type        = ttm_bo_type_device;
> +     bp.size        = AMDGPU_GPU_PAGE_SIZE;
>       bp.byte_align  = AMDGPU_GPU_PAGE_SIZE;
> -     bp.domain      = AMDGPU_GEM_DOMAIN_MMIO_REMAP;
> +     bp.domain      = 0;
>       bp.flags       = 0;
>       bp.resv        = NULL;
>       bp.bo_ptr_size = sizeof(struct amdgpu_bo);
> -
>       r = amdgpu_bo_create(adev, &bp, &adev->rmmio_remap.bo);
>       if (r)
>               return r;
> @@ -1936,37 +1935,48 @@ static int amdgpu_ttm_mmio_remap_bo_init(struct
> amdgpu_device *adev)
>       if (r)
>               goto err_unref;
>
> +     tbo = &adev->rmmio_remap.bo->tbo;
> +
>       /*
>        * MMIO_REMAP is a fixed I/O placement (AMDGPU_PL_MMIO_REMAP).
> -      * Use TTM-level pin so the BO cannot be evicted/migrated,
> -      * independent of GEM domains. This
> -      * enforces the “fixed I/O window”
>        */
> -     ttm_bo_pin(&adev->rmmio_remap.bo->tbo);
> +     placement.num_placement = 1;
> +     placement.placement = &placements;
> +     placements.fpfn = 0;
> +     placements.lpfn = 0;
> +     placements.mem_type = AMDGPU_PL_MMIO_REMAP;
> +     placements.flags = 0;
> +     r = ttm_bo_mem_space(tbo, &placement, &tmp, &ctx);
> +     if (unlikely(r))
> +             goto err_unlock;
> +
> +     ttm_resource_free(tbo, &tbo->resource);
> +     ttm_bo_assign_mem(tbo, tmp);
> +     ttm_bo_pin(tbo);
>
>       amdgpu_bo_unreserve(adev->rmmio_remap.bo);
>       return 0;
>
> +err_unlock:
> +     amdgpu_bo_unreserve(adev->rmmio_remap.bo);
> +
>  err_unref:
> -     if (adev->rmmio_remap.bo)
> -             amdgpu_bo_unref(&adev->rmmio_remap.bo);
> +     amdgpu_bo_unref(&adev->rmmio_remap.bo);
>       adev->rmmio_remap.bo = NULL;
>       return r;
>  }
>
>  /**
> - * amdgpu_ttm_mmio_remap_bo_fini - Free the singleton MMIO_REMAP BO
> + * amdgpu_ttm_free_mmio_remap_bo - Free the singleton MMIO_REMAP BO
>   * @adev: amdgpu device
>   *
>   * Frees the kernel-owned MMIO_REMAP BO if it was allocated by
>   * amdgpu_ttm_mmio_remap_bo_init().
>   */
> -static void amdgpu_ttm_mmio_remap_bo_fini(struct amdgpu_device *adev)
> +static void amdgpu_ttm_free_mmio_remap_bo(struct amdgpu_device *adev)
>  {
> -     struct amdgpu_bo *bo = adev->rmmio_remap.bo;
> -
> -     if (!bo)
> -             return;   /* <-- safest early exit */
> +     if (!adev->rmmio_remap.bo)
> +             return;
>
>       if (!amdgpu_bo_reserve(adev->rmmio_remap.bo, true)) {
>               ttm_bo_unpin(&adev->rmmio_remap.bo->tbo);
> @@ -2152,8 +2162,8 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>               return r;
>       }
>
> -     /* Allocate the singleton MMIO_REMAP BO (4K) if supported */
> -     r = amdgpu_ttm_mmio_remap_bo_init(adev);
> +     /* Allocate the singleton MMIO_REMAP BO if supported */
> +     r = amdgpu_ttm_alloc_mmio_remap_bo(adev);
>       if (r)
>               return r;
>
> @@ -2220,7 +2230,7 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>       amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
>                                       &adev->mman.sdma_access_ptr);
>
> -     amdgpu_ttm_mmio_remap_bo_fini(adev);
> +     amdgpu_ttm_free_mmio_remap_bo(adev);

Might be worth a short comment around amdgpu_ttm_free_mmio_remap_bo() or its call in amdgpu_ttm_fini() that we rely on the usual DRM teardown ordering (no more user ioctls once we start TTM fini), so adev->rmmio_remap.bo can’t be accessed via OPEN_GLOBAL any more at this point. That makes the lifetime assumptions of the global BO explicit.

>       amdgpu_ttm_fw_reserve_vram_fini(adev);
>       amdgpu_ttm_drv_reserve_vram_fini(adev);
>
> --
> 2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [RFC PATCH 2/3] drm/amdgpu: add AMDGPU_GEM_OP_OPEN_GLOBAL
  2025-12-02 15:12 ` [RFC PATCH 2/3] drm/amdgpu: add AMDGPU_GEM_OP_OPEN_GLOBAL Christian König
@ 2025-12-10  4:11   ` SHANMUGAM, SRINIVASAN
  2025-12-10 22:06   ` Wu, David
  1 sibling, 0 replies; 7+ messages in thread
From: SHANMUGAM, SRINIVASAN @ 2025-12-10  4:11 UTC (permalink / raw)
  To: Christian König, alexdeucher@gmail.com, Liu, Leo,
	Dong, Ruijing, amd-gfx@lists.freedesktop.org

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Tuesday, December 2, 2025 8:43 PM
> To: alexdeucher@gmail.com; SHANMUGAM, SRINIVASAN
> <SRINIVASAN.SHANMUGAM@amd.com>; Liu, Leo <Leo.Liu@amd.com>; Dong,
> Ruijing <Ruijing.Dong@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: [RFC PATCH 2/3] drm/amdgpu: add
> AMDGPU_GEM_OP_OPEN_GLOBAL
>
> Instead of abusing the create IOCTL to open global BO add a new
> AMDGPU_GEM_OP_OPEN_GLOBAL functionality.
>
> The new AMDGPU_GEM_OP_OPEN_GLOBAL functionality expects an enum
> which tells it which global BO to open and copies the information about the BO to
> userspace similar to the AMDGPU_GEM_OP_GET_GEM_CREATE_INFO
> operation.
>
> The advantage is that we don't start overloading the create IOCTL with tons of
> special cases and opening the global BOs doesn't requires knowing the exact size
> and parameters of it in userspace any more.
>
> Heavily WIP and only compile tested.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 26 ++++++++++++++++++++-----
>  include/uapi/drm/amdgpu_drm.h           |  5 ++++-
>  2 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 9b81a6677f90..9e9b94dcb699 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -968,22 +968,34 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void
> *data,  int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
>                       struct drm_file *filp)
>  {
> +     struct amdgpu_fpriv *fpriv = filp->driver_priv;
>       struct drm_amdgpu_gem_op *args = data;
>       struct drm_gem_object *gobj;
>       struct amdgpu_vm_bo_base *base;
>       struct amdgpu_bo *robj;
>       struct drm_exec exec;
> -     struct amdgpu_fpriv *fpriv = filp->driver_priv;
>       int r;
>
>       if (args->padding)
>               return -EINVAL;
>
> -     gobj = drm_gem_object_lookup(filp, args->handle);
> -     if (!gobj)
> -             return -ENOENT;
> +     if (args->op == AMDGPU_GEM_OP_OPEN_GLOBAL) {
> +             switch (args->handle) {
> +             case AMDGPU_GEM_GLOBAL_MMIO_REMAP:
> +                     robj = drm_to_adev(dev)->rmmio_remap.bo;
> +                     break;
> +             default:
> +                     return -EINVAL;
> +             }
> +             gobj = &robj->tbo.base;
> +             drm_gem_object_get(gobj);
> +     } else {
> +             gobj = drm_gem_object_lookup(filp, args->handle);
> +             if (!gobj)
> +                     return -ENOENT;
>
> -     robj = gem_to_amdgpu_bo(gobj);
> +             robj = gem_to_amdgpu_bo(gobj);
> +     }
>

For AMDGPU_GEM_OP_OPEN_GLOBAL, I think it would be nice to centralise the ID→BO mapping into a small helper, e.g. amdgpu_get_global_bo(adev, id), and have it return NULL if the global BO is not available on this device. Then the OPEN_GLOBAL path can simply do:

robj = amdgpu_get_global_bo(drm_to_adev(dev), args->handle);
if (!robj)
    return -EOPNOTSUPP;

That avoids the potential NULL deref on adev->rmmio_remap.bo when the remap page isn’t present and also gives us a central place to extend with additional AMDGPU_GEM_GLOBAL_* IDs in future.

Today, OPEN_GLOBAL switches on args->handle inside amdgpu_gem_op_ioctl():

switch (args->handle) {
case AMDGPU_GEM_GLOBAL_MMIO_REMAP:
    robj = drm_to_adev(dev)->rmmio_remap.bo;
    break;
default:
    return -EINVAL;
}

Like adding a small helper

Add a small helper:

static struct amdgpu_bo *
amdgpu_get_global_bo(struct amdgpu_device *adev, u32 id)
{
    switch (id) {
    case AMDGPU_GEM_GLOBAL_MMIO_REMAP:
        return adev->rmmio_remap.bo;
    // future global BOs can be added here
    default:
        return NULL;
    }
}

So, that only one function knows which global ID maps to which BO.

Easy to add new globals in future (DOORBELL, KFD, etc.).

UAPI and internal mapping stay in sync.


>       drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT |
>                         DRM_EXEC_IGNORE_DUPLICATES, 0);
> @@ -1002,6 +1014,7 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void
> *data,
>       }
>
>       switch (args->op) {
> +     case AMDGPU_GEM_OP_OPEN_GLOBAL:

for AMDGPU_GEM_OP_OPEN_GLOBAL we could treat args->value == 0 as “caller only wants a handle, not the create_info”. In that case we can skip the copy_to_user() entirely and just create the handle. Right now the code assumes a non-NULL pointer for both GET_GEM_CREATE_INFO and OPEN_GLOBAL; relaxing that for the latter would make the API a bit easier to use. (Optional not blocker)

case AMDGPU_GEM_OP_GET_GEM_CREATE_INFO: {
    struct drm_amdgpu_gem_create_in info;
    void __user *out = NULL;

    r = amdgpu_gem_get_create_info(robj, &info);
    if (r)
        break;

    /* For OPEN_GLOBAL, allow value == 0 when caller only wants the handle */
    if (args->op == AMDGPU_GEM_OP_OPEN_GLOBAL && !args->value)
        break;

    out = u64_to_user_ptr(args->value);
    if (copy_to_user(out, &info, sizeof(info)))
        r = -EFAULT;

    break;
}

>       case AMDGPU_GEM_OP_GET_GEM_CREATE_INFO: {
>               struct drm_amdgpu_gem_create_in info;
>               void __user *out = u64_to_user_ptr(args->value); @@ -1096,6
> +1109,9 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
>               r = -EINVAL;
>       }
>
> +     if (!r && args->op == AMDGPU_GEM_OP_OPEN_GLOBAL)
> +             r = drm_gem_handle_create(filp, gobj, &args->handle);
> +
>       drm_gem_object_put(gobj);
>       return r;
>  out_exec:
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h index
> c1336ed4ff75..6927c864a6d1 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -807,6 +807,9 @@ union drm_amdgpu_wait_fences {
>  #define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO    0
>  #define AMDGPU_GEM_OP_SET_PLACEMENT          1
>  #define AMDGPU_GEM_OP_GET_MAPPING_INFO               2
> +#define AMDGPU_GEM_OP_OPEN_GLOBAL            3
> +
> +#define AMDGPU_GEM_GLOBAL_MMIO_REMAP         0
>
>  struct drm_amdgpu_gem_vm_entry {
>       /* Start of mapping (in bytes) */
> @@ -824,7 +827,7 @@ struct drm_amdgpu_gem_vm_entry {
>
>  /* Sets or returns a value associated with a buffer. */  struct drm_amdgpu_gem_op
> {
> -     /** GEM object handle */
> +     /** GEM object handle or AMDGPU_GEM_GLOBAL_* */

For the UAPI docs, maybe can we add a short note that for AMDGPU_GEM_OP_OPEN_GLOBAL the handle field is interpreted as an AMDGPU_GEM_GLOBAL_* ID on input and overwritten with a per-file GEM handle on success. That makes the dual use of handle a bit clearer for userspace.

>       __u32   handle;
>       /** AMDGPU_GEM_OP_* */
>       __u32   op;
> --
> 2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [RFC PATCH 3/3] drm/amdgpu: remove AMDGPU_GEM_DOMAIN_DOORBELL
  2025-12-02 15:12 ` [RFC PATCH 3/3] drm/amdgpu: remove AMDGPU_GEM_DOMAIN_DOORBELL Christian König
@ 2025-12-10  4:22   ` SHANMUGAM, SRINIVASAN
  0 siblings, 0 replies; 7+ messages in thread
From: SHANMUGAM, SRINIVASAN @ 2025-12-10  4:22 UTC (permalink / raw)
  To: Christian König, alexdeucher@gmail.com, Liu, Leo,
	Dong, Ruijing, amd-gfx@lists.freedesktop.org

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Christian
> König
> Sent: Tuesday, December 2, 2025 8:43 PM
> To: alexdeucher@gmail.com; SHANMUGAM, SRINIVASAN
> <SRINIVASAN.SHANMUGAM@amd.com>; Liu, Leo <Leo.Liu@amd.com>; Dong,
> Ruijing <Ruijing.Dong@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: [RFC PATCH 3/3] drm/amdgpu: remove
> AMDGPU_GEM_DOMAIN_DOORBELL

Please change this title to " AMDGPU_GEM_DOMAIN_MMIO_REMAP"

And correspondingly please add fixes tag in commit message for future references from where this was derived.

>
> Never activated as UAPI and it turned out that this was to inflexible.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    |  3 ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 10 ----------
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 --
>  include/uapi/drm/amdgpu_drm.h              |  6 +-----
>  4 files changed, 1 insertion(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 9e9b94dcb699..4fd8121f9d95 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -458,9 +458,6 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void
> *data,
>       /* always clear VRAM */
>       flags |= AMDGPU_GEM_CREATE_VRAM_CLEARED;
>
> -     if (args->in.domains & AMDGPU_GEM_DOMAIN_MMIO_REMAP)
> -             return -EINVAL;
> -
>       /* create a gem object to contain this object in */
>       if (args->in.domains & (AMDGPU_GEM_DOMAIN_GDS |
>           AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA)) { diff
> --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index fe51087a54a9..0584c7383c19 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -153,14 +153,6 @@ void amdgpu_bo_placement_from_domain(struct
> amdgpu_bo *abo, u32 domain)
>               c++;
>       }
>
> -     if (domain & AMDGPU_GEM_DOMAIN_MMIO_REMAP) {
> -             places[c].fpfn = 0;
> -             places[c].lpfn = 0;
> -             places[c].mem_type = AMDGPU_PL_MMIO_REMAP;
> -             places[c].flags = 0;
> -             c++;
> -     }
> -
>       if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>               places[c].fpfn = 0;
>               places[c].lpfn = 0;
> @@ -1566,8 +1558,6 @@ uint32_t amdgpu_bo_mem_stats_placement(struct
> amdgpu_bo *bo)
>               return AMDGPU_PL_OA;
>       case AMDGPU_GEM_DOMAIN_DOORBELL:
>               return AMDGPU_PL_DOORBELL;
> -     case AMDGPU_GEM_DOMAIN_MMIO_REMAP:
> -             return AMDGPU_PL_MMIO_REMAP;

Now that the MMIO_REMAP domain bit is gone, nothing maps to AMDGPU_PL_MMIO_REMAP anymore.

If we still want a dedicated mmioremap bucket in fdinfo/mem-stats, could we instead check bo->resource->mem_type == AMDGPU_PL_MMIO_REMAP here.


Thanks!
Srini


>       default:
>               return TTM_PL_SYSTEM;
>       }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 52c2d1731aab..912c9afaf9e1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -168,8 +168,6 @@ static inline unsigned amdgpu_mem_type_to_domain(u32
> mem_type)
>               return AMDGPU_GEM_DOMAIN_OA;
>       case AMDGPU_PL_DOORBELL:
>               return AMDGPU_GEM_DOMAIN_DOORBELL;
> -     case AMDGPU_PL_MMIO_REMAP:
> -             return AMDGPU_GEM_DOMAIN_MMIO_REMAP;
>       default:
>               break;
>       }
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h index
> 6927c864a6d1..0d743fe92146 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -105,8 +105,6 @@ extern "C" {
>   *
>   * %AMDGPU_GEM_DOMAIN_DOORBELL       Doorbell. It is an MMIO region
> for
>   * signalling user mode queues.
> - *
> - * %AMDGPU_GEM_DOMAIN_MMIO_REMAP     MMIO remap page (special
> mapping for HDP flushing).
>   */
>  #define AMDGPU_GEM_DOMAIN_CPU                0x1
>  #define AMDGPU_GEM_DOMAIN_GTT                0x2
> @@ -115,15 +113,13 @@ extern "C" {
>  #define AMDGPU_GEM_DOMAIN_GWS                0x10
>  #define AMDGPU_GEM_DOMAIN_OA         0x20
>  #define AMDGPU_GEM_DOMAIN_DOORBELL   0x40
> -#define AMDGPU_GEM_DOMAIN_MMIO_REMAP 0x80
>  #define AMDGPU_GEM_DOMAIN_MASK
>       (AMDGPU_GEM_DOMAIN_CPU | \
>                                        AMDGPU_GEM_DOMAIN_GTT | \
>                                        AMDGPU_GEM_DOMAIN_VRAM | \
>                                        AMDGPU_GEM_DOMAIN_GDS | \
>                                        AMDGPU_GEM_DOMAIN_GWS | \
>                                        AMDGPU_GEM_DOMAIN_OA | \
> -                                      AMDGPU_GEM_DOMAIN_DOORBELL | \
> -                                      AMDGPU_GEM_DOMAIN_MMIO_REMAP)
> +                                      AMDGPU_GEM_DOMAIN_DOORBELL)
>
>  /* Flag that CPU access will be required for the case of VRAM domain */
>  #define AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED        (1 << 0)
> --
> 2.43.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH 2/3] drm/amdgpu: add AMDGPU_GEM_OP_OPEN_GLOBAL
  2025-12-02 15:12 ` [RFC PATCH 2/3] drm/amdgpu: add AMDGPU_GEM_OP_OPEN_GLOBAL Christian König
  2025-12-10  4:11   ` SHANMUGAM, SRINIVASAN
@ 2025-12-10 22:06   ` Wu, David
  1 sibling, 0 replies; 7+ messages in thread
From: Wu, David @ 2025-12-10 22:06 UTC (permalink / raw)
  To: amd-gfx

On 12/2/2025 10:12 AM, Christian König wrote:
> Instead of abusing the create IOCTL to open global BO add a new
> AMDGPU_GEM_OP_OPEN_GLOBAL functionality.
>
> The new AMDGPU_GEM_OP_OPEN_GLOBAL functionality expects an enum which tells
> it which global BO to open and copies the information about the BO to
> userspace similar to the AMDGPU_GEM_OP_GET_GEM_CREATE_INFO operation.
>
> The advantage is that we don't start overloading the create IOCTL with
> tons of special cases and opening the global BOs doesn't requires knowing
> the exact size and parameters of it in userspace any more.
>
> Heavily WIP and only compile tested.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 26 ++++++++++++++++++++-----
>   include/uapi/drm/amdgpu_drm.h           |  5 ++++-
>   2 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 9b81a6677f90..9e9b94dcb699 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -968,22 +968,34 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
>   int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
>   			struct drm_file *filp)
>   {
> +	struct amdgpu_fpriv *fpriv = filp->driver_priv;
>   	struct drm_amdgpu_gem_op *args = data;
>   	struct drm_gem_object *gobj;
>   	struct amdgpu_vm_bo_base *base;
>   	struct amdgpu_bo *robj;
>   	struct drm_exec exec;
> -	struct amdgpu_fpriv *fpriv = filp->driver_priv;
>   	int r;
>   
>   	if (args->padding)
>   		return -EINVAL;
>   
> -	gobj = drm_gem_object_lookup(filp, args->handle);
> -	if (!gobj)
> -		return -ENOENT;
> +	if (args->op == AMDGPU_GEM_OP_OPEN_GLOBAL) {
> +		switch (args->handle) {
> +		case AMDGPU_GEM_GLOBAL_MMIO_REMAP:
> +			robj = drm_to_adev(dev)->rmmio_remap.bo;

rmmio_remap.bo could be null - so robj should be checked and return error instead.

David

> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		gobj = &robj->tbo.base;
> +		drm_gem_object_get(gobj);
> +	} else {
> +		gobj = drm_gem_object_lookup(filp, args->handle);
> +		if (!gobj)
> +			return -ENOENT;
>   
> -	robj = gem_to_amdgpu_bo(gobj);
> +		robj = gem_to_amdgpu_bo(gobj);
> +	}
>   
>   	drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT |
>   			  DRM_EXEC_IGNORE_DUPLICATES, 0);
> @@ -1002,6 +1014,7 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
>   	}
>   
>   	switch (args->op) {
> +	case AMDGPU_GEM_OP_OPEN_GLOBAL:
>   	case AMDGPU_GEM_OP_GET_GEM_CREATE_INFO: {
>   		struct drm_amdgpu_gem_create_in info;
>   		void __user *out = u64_to_user_ptr(args->value);
> @@ -1096,6 +1109,9 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
>   		r = -EINVAL;
>   	}
>   
> +	if (!r && args->op == AMDGPU_GEM_OP_OPEN_GLOBAL)
> +		r = drm_gem_handle_create(filp, gobj, &args->handle);
> +
>   	drm_gem_object_put(gobj);
>   	return r;
>   out_exec:
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index c1336ed4ff75..6927c864a6d1 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -807,6 +807,9 @@ union drm_amdgpu_wait_fences {
>   #define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO	0
>   #define AMDGPU_GEM_OP_SET_PLACEMENT		1
>   #define AMDGPU_GEM_OP_GET_MAPPING_INFO		2
> +#define AMDGPU_GEM_OP_OPEN_GLOBAL		3
> +
> +#define AMDGPU_GEM_GLOBAL_MMIO_REMAP		0
>   
>   struct drm_amdgpu_gem_vm_entry {
>   	/* Start of mapping (in bytes) */
> @@ -824,7 +827,7 @@ struct drm_amdgpu_gem_vm_entry {
>   
>   /* Sets or returns a value associated with a buffer. */
>   struct drm_amdgpu_gem_op {
> -	/** GEM object handle */
> +	/** GEM object handle or AMDGPU_GEM_GLOBAL_* */
>   	__u32	handle;
>   	/** AMDGPU_GEM_OP_* */
>   	__u32	op;


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-12-10 22:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-02 15:12 [RFC PATCH 1/3] drm/amdgpu: rework MMIO_REMAP BO creation Christian König
2025-12-02 15:12 ` [RFC PATCH 2/3] drm/amdgpu: add AMDGPU_GEM_OP_OPEN_GLOBAL Christian König
2025-12-10  4:11   ` SHANMUGAM, SRINIVASAN
2025-12-10 22:06   ` Wu, David
2025-12-02 15:12 ` [RFC PATCH 3/3] drm/amdgpu: remove AMDGPU_GEM_DOMAIN_DOORBELL Christian König
2025-12-10  4:22   ` SHANMUGAM, SRINIVASAN
2025-12-10  3:22 ` [RFC PATCH 1/3] drm/amdgpu: rework MMIO_REMAP BO creation SHANMUGAM, SRINIVASAN

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox