[PATCH] drm/amd/amdgpu: Merge the smc/pcie/didt debugfs read/write methods

[Christian König]

Am 04.04.2018 um 15:35 schrieb Tom St Denis:
> Fold the read/write methods of the various register access
> debugfs entries into op functions.
>
> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>

I would rather split out a dword helper. E.g. something like:

amdgpu_debugfs_regs_dword_op(struct file *f, char __user *buf, size_t 
size, loff_t *pos,
                                                         int 
(*op)(struct amdgpu_device *adev, char __user *buf))
{
...
}

And then you have op callbacks for PCIE, didt, SMC each in a read/write 
variant.

Would save quite a bunch more of loc if I'm not completely mistaken.

Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 140 +++++++++++-----------------
>   1 file changed, 57 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index c98e59721444..b80e18469d25 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -177,8 +177,8 @@ 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 ssize_t amdgpu_debugfs_regs_pcie_read(struct file *f, char __user *buf,
> -					size_t size, loff_t *pos)
> +static ssize_t amdgpu_debugfs_regs_pcie_op(bool read, struct file *f,
> +					char __user *buf, size_t size, loff_t *pos)
>   {
>   	struct amdgpu_device *adev = file_inode(f)->i_private;
>   	ssize_t result = 0;
> @@ -190,11 +190,18 @@ static ssize_t amdgpu_debugfs_regs_pcie_read(struct file *f, char __user *buf,
>   	while (size) {
>   		uint32_t value;
>   
> -		value = RREG32_PCIE(*pos >> 2);
> -		r = put_user(value, (uint32_t *)buf);
> -		if (r)
> -			return r;
> +		if (read) {
> +			value = RREG32_PCIE(*pos >> 2);
> +			r = put_user(value, (uint32_t *)buf);
> +			if (r)
> +				return r;
> +		} else {
> +			r = get_user(value, (uint32_t *)buf);
> +			if (r)
> +				return r;
>   
> +			WREG32_PCIE(*pos >> 2, value);
> +		}
>   		result += 4;
>   		buf += 4;
>   		*pos += 4;
> @@ -204,8 +211,20 @@ static ssize_t amdgpu_debugfs_regs_pcie_read(struct file *f, char __user *buf,
>   	return result;
>   }
>   
> +static ssize_t amdgpu_debugfs_regs_pcie_read(struct file *f, char __user *buf,
> +					size_t size, loff_t *pos)
> +{
> +	return amdgpu_debugfs_regs_pcie_op(true, f, buf, size, pos);
> +}
> +
>   static ssize_t amdgpu_debugfs_regs_pcie_write(struct file *f, const char __user *buf,
>   					 size_t size, loff_t *pos)
> +{
> +	return amdgpu_debugfs_regs_pcie_op(false, f, (char __user *)buf, size, pos);
> +}
> +
> +static ssize_t amdgpu_debugfs_regs_didt_op(bool read, struct file *f,
> +					char __user *buf, size_t size, loff_t *pos)
>   {
>   	struct amdgpu_device *adev = file_inode(f)->i_private;
>   	ssize_t result = 0;
> @@ -217,12 +236,18 @@ static ssize_t amdgpu_debugfs_regs_pcie_write(struct file *f, const char __user
>   	while (size) {
>   		uint32_t value;
>   
> -		r = get_user(value, (uint32_t *)buf);
> -		if (r)
> -			return r;
> -
> -		WREG32_PCIE(*pos >> 2, value);
> +		if (read) {
> +			value = RREG32_DIDT(*pos >> 2);
> +			r = put_user(value, (uint32_t *)buf);
> +			if (r)
> +				return r;
> +		} else {
> +			r = get_user(value, (uint32_t *)buf);
> +			if (r)
> +				return r;
>   
> +			WREG32_DIDT(*pos >> 2, value);
> +		}
>   		result += 4;
>   		buf += 4;
>   		*pos += 4;
> @@ -235,32 +260,18 @@ static ssize_t amdgpu_debugfs_regs_pcie_write(struct file *f, const char __user
>   static ssize_t amdgpu_debugfs_regs_didt_read(struct file *f, char __user *buf,
>   					size_t size, loff_t *pos)
>   {
> -	struct amdgpu_device *adev = file_inode(f)->i_private;
> -	ssize_t result = 0;
> -	int r;
> -
> -	if (size & 0x3 || *pos & 0x3)
> -		return -EINVAL;
> -
> -	while (size) {
> -		uint32_t value;
> -
> -		value = RREG32_DIDT(*pos >> 2);
> -		r = put_user(value, (uint32_t *)buf);
> -		if (r)
> -			return r;
> -
> -		result += 4;
> -		buf += 4;
> -		*pos += 4;
> -		size -= 4;
> -	}
> -
> -	return result;
> +	return amdgpu_debugfs_regs_didt_op(true, f, buf, size, pos);
>   }
>   
>   static ssize_t amdgpu_debugfs_regs_didt_write(struct file *f, const char __user *buf,
>   					 size_t size, loff_t *pos)
> +{
> +	return amdgpu_debugfs_regs_didt_op(false, f, (char __user *)buf, size, pos);
> +}
> +
> +static ssize_t amdgpu_debugfs_regs_smc_op(bool read,
> +					struct file *f, char __user *buf,
> +					size_t size, loff_t *pos)
>   {
>   	struct amdgpu_device *adev = file_inode(f)->i_private;
>   	ssize_t result = 0;
> @@ -272,11 +283,17 @@ static ssize_t amdgpu_debugfs_regs_didt_write(struct file *f, const char __user
>   	while (size) {
>   		uint32_t value;
>   
> -		r = get_user(value, (uint32_t *)buf);
> -		if (r)
> -			return r;
> -
> -		WREG32_DIDT(*pos >> 2, value);
> +		if (read) {
> +			value = RREG32_SMC(*pos);
> +			r = put_user(value, (uint32_t *)buf);
> +			if (r)
> +				return r;
> +		} else {
> +			r = get_user(value, (uint32_t *)buf);
> +			if (r)
> +				return r;
> +			WREG32_SMC(*pos, value);
> +		}
>   
>   		result += 4;
>   		buf += 4;
> @@ -290,56 +307,13 @@ static ssize_t amdgpu_debugfs_regs_didt_write(struct file *f, const char __user
>   static ssize_t amdgpu_debugfs_regs_smc_read(struct file *f, char __user *buf,
>   					size_t size, loff_t *pos)
>   {
> -	struct amdgpu_device *adev = file_inode(f)->i_private;
> -	ssize_t result = 0;
> -	int r;
> -
> -	if (size & 0x3 || *pos & 0x3)
> -		return -EINVAL;
> -
> -	while (size) {
> -		uint32_t value;
> -
> -		value = RREG32_SMC(*pos);
> -		r = put_user(value, (uint32_t *)buf);
> -		if (r)
> -			return r;
> -
> -		result += 4;
> -		buf += 4;
> -		*pos += 4;
> -		size -= 4;
> -	}
> -
> -	return result;
> +	return amdgpu_debugfs_regs_smc_op(true, f, buf, size, pos);
>   }
>   
>   static ssize_t amdgpu_debugfs_regs_smc_write(struct file *f, const char __user *buf,
>   					 size_t size, loff_t *pos)
>   {
> -	struct amdgpu_device *adev = file_inode(f)->i_private;
> -	ssize_t result = 0;
> -	int r;
> -
> -	if (size & 0x3 || *pos & 0x3)
> -		return -EINVAL;
> -
> -	while (size) {
> -		uint32_t value;
> -
> -		r = get_user(value, (uint32_t *)buf);
> -		if (r)
> -			return r;
> -
> -		WREG32_SMC(*pos, value);
> -
> -		result += 4;
> -		buf += 4;
> -		*pos += 4;
> -		size -= 4;
> -	}
> -
> -	return result;
> +	return amdgpu_debugfs_regs_smc_op(false, f, (char __user *)buf, size, pos);
>   }
>   
>   static ssize_t amdgpu_debugfs_gca_config_read(struct file *f, char __user *buf,