[RFC PATCH 7/7] drm/amdgpu: Return Handle to MMIO_REMAP Singleton for GEM Create

[Christian König]

On 20.08.25 13:32, Srinivasan Shanmugam wrote:
> When userspace requests a GEM in AMDGPU_GEM_DOMAIN_MMIO_REMAP, return a
> handle to the kernel-owned singleton BO instead of allocating a new one.
> 
> Validate inputs (exact PAGE_SIZE, alignment PAGE_SIZE, no extra flags)
> and zero the ioctl out-struct on success for a clean echo.
> 
> This puts the userspace-visible behavior last, after all internal kernel
> plumbing and initialization are in place.
> 
> Cc: Christian König <christian.koenig at amd.com>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 56 +++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index e3f65977eeee..1345e81214e8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -424,6 +424,26 @@ const struct drm_gem_object_funcs amdgpu_gem_object_funcs = {
>  	.vm_ops = &amdgpu_gem_vm_ops,
>  };
>  
> +/* ========= MMIO remap (HDP flush) GEM handle helper ========= */
> +static int amdgpu_gem_get_mmio_remap_handle(struct drm_file *file_priv,
> +					    struct amdgpu_device *adev,
> +					    u32 *handle)
> +{
> +	struct amdgpu_bo *bo = adev->mmio_remap_bo;
> +	struct drm_gem_object *gobj;
> +	int r;
> +
> +	if (!bo)
> +		return -ENODEV;
> +
> +	/* Take a temporary ref; the handle creation will hold its own ref. */
> +	bo = amdgpu_bo_ref(bo);

That is superflous, you can just call drm_gem_handle_create().

> +	gobj = &bo->tbo.base;
> +	r = drm_gem_handle_create(file_priv, gobj, handle);
> +	amdgpu_bo_unref(&bo);  /* drops our temporary ref */
> +	return r;
> +}
> +
>  /*
>   * GEM ioctls.
>   */
> @@ -465,6 +485,42 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>  	/* always clear VRAM */
>  	flags |= AMDGPU_GEM_CREATE_VRAM_CLEARED;
>  
> +	/*
> +	 * === MMIO remap (HDP flush) fast-path ===
> +	 * If userspace asks for the MMIO_REMAP domain, don't allocate a new BO.
> +	 * Return a handle to the singleton BO created at device init.
> +	 */
> +	if (args->in.domains & AMDGPU_GEM_DOMAIN_MMIO_REMAP) {
> +		u32 mmio_handle;
> +		/* Enforce fixed size & alignment (exactly one page). */
> +		if (size && size != PAGE_SIZE)
> +			return -EINVAL;
> +		if (args->in.alignment && args->in.alignment != PAGE_SIZE)
> +			return -EINVAL;
> +		/* No extra domain flags for this special object. */
> +		if (args->in.domain_flags)
> +			return -EINVAL;
> +		/* Disallow flags that don't make sense for a fixed I/O page. */
> +		if (flags & (AMDGPU_GEM_CREATE_CPU_GTT_USWC |
> +			     AMDGPU_GEM_CREATE_ENCRYPTED |
> +			     AMDGPU_GEM_CREATE_DISCARDABLE))
> +			return -EINVAL;
> +
> +		/* Normalize inputs (optional, for user-visible echo/debug). */

> +		args->in.bo_size     = PAGE_SIZE;
> +		args->in.alignment   = PAGE_SIZE;
> +		args->in.domains     = AMDGPU_GEM_DOMAIN_MMIO_REMAP;
> +		args->in.domain_flags = 0;
> +

That makes not much sense, the in parameters are meaningless at this point.

Regards,
Christian.

> +		r = amdgpu_gem_get_mmio_remap_handle(filp, adev, &mmio_handle);
> +		if (r)
> +			return r;
> +
> +		memset(args, 0, sizeof(*args));
> +		args->out.handle = mmio_handle;
> +		return 0;
> +	}
> +
>  	/* 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)) {