[PATCH] drm/amd/amdgpu: New debugfs interface for MMIO registers (v2)

Christian König ckoenig.leichtzumerken at gmail.com
Wed Aug 25 06:35:06 UTC 2021



Am 24.08.21 um 15:36 schrieb Tom St Denis:
> This new debugfs interface uses an IOCTL interface in order to pass
> along state information like SRBM and GRBM bank switching.  This
> new interface also allows a full 32-bit MMIO address range which
> the previous didn't.  With this new design we have room to grow
> the flexibility of the file as need be.
>
> (v2): Move read/write to .read/.write, fix style, add comment
>        for IOCTL data structure
>
> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 162 ++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h |  32 ++++
>   2 files changed, 194 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index 277128846dd1..8e8f5743c8f5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -279,6 +279,156 @@ static ssize_t amdgpu_debugfs_regs_write(struct file *f, const char __user *buf,
>   	return amdgpu_debugfs_process_reg_op(false, f, (char __user *)buf, size, pos);
>   }
>   
> +static int amdgpu_debugfs_regs2_open(struct inode *inode, struct file *file)
> +{
> +	struct amdgpu_debugfs_regs2_data *rd;
> +
> +	rd = kzalloc(sizeof *rd, GFP_KERNEL);
> +	if (!rd)
> +		return -ENOMEM;
> +	rd->adev = file_inode(file)->i_private;
> +	file->private_data = rd;
> +
> +	return 0;
> +}
> +
> +static int amdgpu_debugfs_regs2_release(struct inode *inode, struct file *file)
> +{
> +	kfree(file->private_data);
> +	return 0;
> +}
> +
> +static ssize_t amdgpu_debugfs_regs2_op(struct file *f, char __user *buf, size_t size, int write_en)
> +{
> +	struct amdgpu_debugfs_regs2_data *rd = f->private_data;
> +	struct amdgpu_device *adev = rd->adev;
> +	ssize_t result = 0;
> +	int r;
> +	uint32_t value;
> +
> +	if (size & 0x3 || rd->state.offset & 0x3)
> +		return -EINVAL;
> +
> +	if (rd->state.id.use_grbm) {
> +		if (rd->state.id.grbm.se == 0x3FF)
> +			rd->state.id.grbm.se = 0xFFFFFFFF;
> +		if (rd->state.id.grbm.sh == 0x3FF)
> +			rd->state.id.grbm.sh = 0xFFFFFFFF;
> +		if (rd->state.id.grbm.instance == 0x3FF)
> +			rd->state.id.grbm.instance = 0xFFFFFFFF;
> +	}
> +
> +	r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
> +	if (r < 0) {
> +		pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> +		return r;
> +	}
> +
> +	r = amdgpu_virt_enable_access_debugfs(adev);
> +	if (r < 0) {
> +		pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> +		return r;
> +	}
> +
> +	if (rd->state.id.use_grbm) {
> +		if ((rd->state.id.grbm.sh != 0xFFFFFFFF && rd->state.id.grbm.sh >= adev->gfx.config.max_sh_per_se) ||
> +		    (rd->state.id.grbm.se != 0xFFFFFFFF && rd->state.id.grbm.se >= adev->gfx.config.max_shader_engines)) {
> +			pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> +			pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> +			amdgpu_virt_disable_access_debugfs(adev);
> +			return -EINVAL;
> +		}
> +		mutex_lock(&adev->grbm_idx_mutex);
> +		amdgpu_gfx_select_se_sh(adev, rd->state.id.grbm.se,
> +								rd->state.id.grbm.sh,
> +								rd->state.id.grbm.instance);
> +	}
> +
> +	if (rd->state.id.use_srbm) {
> +		mutex_lock(&adev->srbm_mutex);
> +		amdgpu_gfx_select_me_pipe_q(adev, rd->state.id.srbm.me, rd->state.id.srbm.pipe,
> +									rd->state.id.srbm.queue, rd->state.id.srbm.vmid);
> +	}
> +
> +	if (rd->state.id.pg_lock)
> +		mutex_lock(&adev->pm.mutex);
> +
> +	while (size) {
> +		if (!write_en) {
> +			value = RREG32(rd->state.offset >> 2);
> +			r = put_user(value, (uint32_t *)buf);
> +		} else {
> +			r = get_user(value, (uint32_t *)buf);
> +			if (!r)
> +				amdgpu_mm_wreg_mmio_rlc(adev, rd->state.offset >> 2, value);
> +		}
> +		if (r) {
> +			result = r;
> +			goto end;
> +		}
> +		rd->state.offset += 4;
> +		size -= 4;
> +		result += 4;
> +		buf += 4;
> +	}
> +end:
> +	if (rd->state.id.use_grbm) {
> +		amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff);
> +		mutex_unlock(&adev->grbm_idx_mutex);
> +	}
> +
> +	if (rd->state.id.use_srbm) {
> +		amdgpu_gfx_select_me_pipe_q(adev, 0, 0, 0, 0);
> +		mutex_unlock(&adev->srbm_mutex);
> +	}
> +
> +	if (rd->state.id.pg_lock)
> +		mutex_unlock(&adev->pm.mutex);
> +
> +	// in umr (the likely user of this) flags are set per file operation
> +	// which means they're never "unset" explicitly.  To avoid breaking
> +	// this convention we unset the flags after each operation
> +	// flags are for a single call (need to be set for every read/write)
> +	rd->state.id.use_grbm = 0;
> +	rd->state.id.use_srbm = 0;
> +	rd->state.id.pg_lock  = 0;
> +
> +	pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> +	pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> +
> +	amdgpu_virt_disable_access_debugfs(adev);
> +	return result;
> +}
> +
> +static long amdgpu_debugfs_regs2_ioctl(struct file *f, unsigned int cmd, unsigned long data)
> +{
> +	struct amdgpu_debugfs_regs2_data *rd = f->private_data;
> +
> +	switch (cmd) {
> +	case AMDGPU_DEBUGFS_REGS2_IOC_SET_STATE:
> +		if (copy_from_user(&rd->state.id, (struct amdgpu_debugfs_regs2_iocdata *)data, sizeof rd->state.id))
> +			return -EINVAL;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static ssize_t amdgpu_debugfs_regs2_read(struct file *f, char __user *buf, size_t size, loff_t *pos)
> +{
> +	struct amdgpu_debugfs_regs2_data *rd = f->private_data;
> +	rd->state.offset = *pos;
> +	return amdgpu_debugfs_regs2_op(f, buf, size, 0);
> +}
> +
> +static ssize_t amdgpu_debugfs_regs2_write(struct file *f, const char __user *buf, size_t size, loff_t *pos)
> +{
> +	struct amdgpu_debugfs_regs2_data *rd = f->private_data;
> +	rd->state.offset = *pos;
> +	return amdgpu_debugfs_regs2_op(f, (char __user *)buf, size, 1);
> +}
> +
>   
>   /**
>    * amdgpu_debugfs_regs_pcie_read - Read from a PCIE register
> @@ -1091,6 +1241,16 @@ static ssize_t amdgpu_debugfs_gfxoff_read(struct file *f, char __user *buf,
>   	return result;
>   }
>   
> +static const struct file_operations amdgpu_debugfs_regs2_fops = {
> +	.owner = THIS_MODULE,
> +	.unlocked_ioctl = amdgpu_debugfs_regs2_ioctl,
> +	.read = amdgpu_debugfs_regs2_read,
> +	.write = amdgpu_debugfs_regs2_write,
> +	.open = amdgpu_debugfs_regs2_open,
> +	.release = amdgpu_debugfs_regs2_release,
> +	.llseek = default_llseek
> +};
> +
>   static const struct file_operations amdgpu_debugfs_regs_fops = {
>   	.owner = THIS_MODULE,
>   	.read = amdgpu_debugfs_regs_read,
> @@ -1148,6 +1308,7 @@ static const struct file_operations amdgpu_debugfs_gfxoff_fops = {
>   
>   static const struct file_operations *debugfs_regs[] = {
>   	&amdgpu_debugfs_regs_fops,
> +	&amdgpu_debugfs_regs2_fops,
>   	&amdgpu_debugfs_regs_didt_fops,
>   	&amdgpu_debugfs_regs_pcie_fops,
>   	&amdgpu_debugfs_regs_smc_fops,
> @@ -1160,6 +1321,7 @@ static const struct file_operations *debugfs_regs[] = {
>   
>   static const char *debugfs_regs_names[] = {
>   	"amdgpu_regs",
> +	"amdgpu_regs2",
>   	"amdgpu_regs_didt",
>   	"amdgpu_regs_pcie",
>   	"amdgpu_regs_smc",
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h
> index 141a8474e24f..ec044df5d428 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h
> @@ -22,6 +22,8 @@
>    * OTHER DEALINGS IN THE SOFTWARE.
>    *
>    */
> +#include <linux/ioctl.h>
> +#include <uapi/drm/amdgpu_drm.h>
>   
>   /*
>    * Debugfs
> @@ -38,3 +40,33 @@ void amdgpu_debugfs_fence_init(struct amdgpu_device *adev);
>   void amdgpu_debugfs_firmware_init(struct amdgpu_device *adev);
>   void amdgpu_debugfs_gem_init(struct amdgpu_device *adev);
>   int amdgpu_debugfs_wait_dump(struct amdgpu_device *adev);
> +
> +/*
> + * MMIO debugfs IOCTL structure
> + */
> +struct amdgpu_debugfs_regs2_iocdata {
> +	__u8 use_srbm, use_grbm, pg_lock;

You should consider using u32 here as well or add explicitly padding.

> +	struct {
> +		__u32 se, sh, instance;
> +	} grbm;
> +	struct {
> +		__u32 me, pipe, queue, vmid;
> +	} srbm;
> +};
> +
> +/*
> + * MMIO debugfs state data (per file* handle)
> + */
> +struct amdgpu_debugfs_regs2_data {
> +	struct amdgpu_device *adev;
> +	struct {
> +		struct amdgpu_debugfs_regs2_iocdata id;
> +		__u32 offset;

What is the offset good for here?

Regards,
Christian.

> +	} state;
> +};
> +
> +enum AMDGPU_DEBUGFS_REGS2_CMDS {
> +	AMDGPU_DEBUGFS_REGS2_CMD_SET_STATE=0,
> +};
> +
> +#define AMDGPU_DEBUGFS_REGS2_IOC_SET_STATE _IOWR(0x20, AMDGPU_DEBUGFS_REGS2_CMD_SET_STATE, struct amdgpu_debugfs_regs2_iocdata)



More information about the amd-gfx mailing list
  翻译: