Re: [PATCH i-g-t 5/8] igt_dmem_driver: add amdgpu support

[Kamil Konieczny <kamil.konieczny@linux.intel.com>]

Hi Thadeu,
On 2026-05-12 at 18:51:52 -0300, Thadeu Lima de Souza Cascardo wrote:
> Allocate a BO from VRAM domain. That will try to place BOs on VRAM, but may
> fallback to GTT. That can still be tracked with dmem.current.
> 
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
> ---
>  lib/amdgpu/amd_dmem.c    | 94 ++++++++++++++++++++++++++++++++++++++++

Why amd_dmem.c? Why this all cannot reside in core_cgroups.c?

+cc Vitaly
Cc: Vitaly Prosyak <vitaly.prosyak@amd.com>

>  lib/igt_dmem_driver.h    |  1 +
>  lib/meson.build          |  1 +
>  tests/drv_dmem_cgroups.c |  6 ++-
>  4 files changed, 100 insertions(+), 2 deletions(-)
>  create mode 100644 lib/amdgpu/amd_dmem.c
> 
> diff --git a/lib/amdgpu/amd_dmem.c b/lib/amdgpu/amd_dmem.c
> new file mode 100644
> index 000000000000..1c0825af43b6
> --- /dev/null
> +++ b/lib/amdgpu/amd_dmem.c
> @@ -0,0 +1,94 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright 2026 Valve Corporation
> + * Authors:
> + *  Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
> + */
> +
> +#include "igt_dmem_driver.h"
> +
> +#include <errno.h>

Please make system includes first, then follow with igt ones, so:

#include <errno.h>

#include "igt.h"
#include "igt_cgroup.h"
#include "igt_dmem_driver.h"
#include "lib/amdgpu/amd_memory.h"

Regards,
Kamil

> +
> +#include "igt.h"
> +#include "igt_cgroup.h"
> +#include "lib/amdgpu/amd_memory.h"
> +
> +struct amdgpu_dmem_ctx {
> +	int fd;
> +	amdgpu_device_handle device;
> +	amdgpu_bo_handle *handles;
> +};
> +
> +static int amdgpu_dmem_init(void **ctx, int fd, int max_bo)
> +{
> +	struct amdgpu_dmem_ctx *actx;
> +	uint32_t major, minor;
> +	int err = -ENOMEM;
> +
> +	actx = malloc(sizeof(*actx));
> +	if (!actx)
> +		return -ENOMEM;
> +
> +	actx->handles = calloc(max_bo, sizeof(actx->handles[0]));
> +	if (!actx->handles)
> +		goto out;
> +
> +	err = amdgpu_device_initialize(fd, &major, &minor, &actx->device);
> +	if (err)
> +		goto out;
> +
> +	*ctx = actx;
> +
> +	return 0;
> +
> +out:
> +	if (actx->handles)
> +		free(actx->handles);
> +	free(actx);
> +
> +	return err;
> +}
> +
> +static void amdgpu_dmem_deinit(void *ctx)
> +{
> +	struct amdgpu_dmem_ctx *actx = ctx;
> +
> +	amdgpu_device_deinitialize(actx->device);
> +	free(actx->handles);
> +	free(actx);
> +}
> +
> +static int amdgpu_dmem_allocate_vram(void *ctx, int n_bo, size_t len)
> +{
> +	struct amdgpu_dmem_ctx *actx = ctx;
> +	amdgpu_bo_handle handle;
> +	int err;
> +
> +	err = amdgpu_bo_alloc_wrap(actx->device, len, 4096,
> +				   AMDGPU_GEM_DOMAIN_VRAM, 0, &handle);
> +	if (err)
> +		return err;
> +
> +	actx->handles[n_bo] = handle;
> +
> +	return 0;
> +}
> +
> +static void amdgpu_dmem_free_vram(void *ctx, int n_bo, size_t len)
> +{
> +	struct amdgpu_dmem_ctx *actx = ctx;
> +	int i;
> +	for (i = 0; i < n_bo; i++) {
> +		if (actx->handles[i])
> +			amdgpu_bo_free(actx->handles[i]);
> +	}
> +}
> +
> +const struct igt_dmem_driver amdgpu_dmem_driver = {
> +	.name = "amdgpu",
> +	.get_region_name = amdgpu_cgroup_region_name,
> +	.init = amdgpu_dmem_init,
> +	.deinit = amdgpu_dmem_deinit,
> +	.allocate_vram = amdgpu_dmem_allocate_vram,
> +	.free_vram = amdgpu_dmem_free_vram,
> +};
> diff --git a/lib/igt_dmem_driver.h b/lib/igt_dmem_driver.h
> index 869356fbf2c2..d43e5140d4f6 100644
> --- a/lib/igt_dmem_driver.h
> +++ b/lib/igt_dmem_driver.h
> @@ -20,5 +20,6 @@ struct igt_dmem_driver {
>  };
>  
>  extern const struct igt_dmem_driver xe_dmem_driver;
> +extern const struct igt_dmem_driver amdgpu_dmem_driver;
>  
>  #endif
> diff --git a/lib/meson.build b/lib/meson.build
> index 269f3b9f0af8..dd02d087875b 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -188,6 +188,7 @@ if libdrm_amdgpu.found()
>  		'amdgpu/amd_mmd_shared.c',
>  		'amdgpu/amd_jpeg_shared.c',
>  		'amdgpu/amd_utils.c',
> +		'amdgpu/amd_dmem.c',
>  		'amdgpu/amd_vcn_shared.c'
>  	]
>  	if libdrm_amdgpu.version().version_compare('> 2.4.99')
> diff --git a/tests/drv_dmem_cgroups.c b/tests/drv_dmem_cgroups.c
> index 6f4f779f3c2c..0e26b7e2bb9a 100644
> --- a/tests/drv_dmem_cgroups.c
> +++ b/tests/drv_dmem_cgroups.c
> @@ -116,8 +116,9 @@ static int fill_vram(const struct igt_dmem_driver *drv, void *ctx, int fd, int m
>  			break;
>  	}
>  
> -	igt_assert_f(err == -ENOMEM || err == -ENOSPC,
> -		     "Expected -ENOMEM or -ENOSPC, got %d (%s)\n",
> +	/* amdgpu will fallback to GTT if it cannot allocate on VRAM */
> +	igt_assert_f(err == -ENOMEM || err == -ENOSPC || err == 0,
> +		     "Expected -ENOMEM,-ENOSPC or success, got %d (%s)\n",
>  		     err, strerror(-err));
>  
>  	return n_bo;
> @@ -236,6 +237,7 @@ static const struct {
>  	const struct igt_dmem_driver *driver;
>  } drivers[] = {
>  	{ DRIVER_XE, &xe_dmem_driver },
> +	{ DRIVER_AMDGPU, &amdgpu_dmem_driver },
>  	{ },
>  };
>  
> -- 
> 2.47.3
>