[PATCH 1/3] drm/amdgpu: add reset register dump function for gfx_v10_0

[Christian König]

Am 11.01.22 um 08:11 schrieb Somalapuram Amaranath:
> Implementation of register dump function when the AMD GPU resets
>
> Signed-off-by: Somalapuram Amaranath <Amaranath.Somalapuram at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c   | 55 +++++++++++++++++++++++-
>   drivers/gpu/drm/amd/include/amd_shared.h |  2 +
>   2 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index dbe7442fb25c..dbd74204f06d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -7466,7 +7466,6 @@ static int gfx_v10_0_hw_init(void *handle)
>   {
>   	int r;
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> -
>   	if (!amdgpu_emu_mode)
>   		gfx_v10_0_init_golden_registers(adev);
>   
> @@ -7580,6 +7579,59 @@ static int gfx_v10_0_hw_fini(void *handle)
>   	return 0;
>   }
>   
> +static int gfx_v10_0_reset_reg_dumps(void *handle,
> +		uint32_t (**dump)[2], uint32_t *n_regs, char ***r_names)
> +{
> +	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +	uint32_t i = 0;
> +
> +#define N_REGS (17)
> +#define DUMP_REG(addr) do {				\
> +		(*dump)[i][0] = (addr);			\
> +		(*dump)[i++][1] = RREG32(addr);		\
> +	} while (0)

Please no unprefixed local defines like this.

> +
> +	char *reg_names[N_REGS] = {
> +		"mmGRBM_STATUS2", "mmGRBM_STATUS_SE0", "mmGRBM_STATUS_SE1",
> +		"mmGRBM_STATUS_SE2", "mmGRBM_STATUS_SE3", "mmSDMA0_STATUS_REG",
> +		"mmSDMA1_STATUS_REG", "mmCP_STAT", "mmCP_STALLED_STAT1",
> +		"mmCP_STALLED_STAT1", "mmCP_STALLED_STAT3", "mmCP_CPC_STATUS",
> +		"mmCP_CPC_BUSY_STAT", "mmCP_CPC_STALLED_STAT1", "mmCP_CPF_STATUS",
> +		"mmCP_CPF_BUSY_STAT", "mmCP_CPF_STALLED_STAT1"
> +	};

IIRC you can avoid that by stringifying the names in the macro, but in 
general I think the kernel should not have those names.

Only userspace needs to be able to match the register number to the name.

Regards,
Christian.

> +
> +	*dump = kmalloc(N_REGS*2*sizeof(uint32_t), GFP_KERNEL);
> +	if (*dump == NULL)
> +		return -ENOMEM;
> +	DUMP_REG(mmGRBM_STATUS2);
> +	DUMP_REG(mmGRBM_STATUS_SE0);
> +	DUMP_REG(mmGRBM_STATUS_SE1);
> +	DUMP_REG(mmGRBM_STATUS_SE2);
> +	DUMP_REG(mmGRBM_STATUS_SE3);
> +	DUMP_REG(mmSDMA0_STATUS_REG);
> +	DUMP_REG(mmSDMA1_STATUS_REG);
> +	DUMP_REG(mmCP_STAT);
> +	DUMP_REG(mmCP_STALLED_STAT1);
> +	DUMP_REG(mmCP_STALLED_STAT1);
> +	DUMP_REG(mmCP_STALLED_STAT3);
> +	DUMP_REG(mmCP_CPC_STATUS);
> +	DUMP_REG(mmCP_CPC_BUSY_STAT);
> +	DUMP_REG(mmCP_CPC_STALLED_STAT1);
> +	DUMP_REG(mmCP_CPF_STATUS);
> +	DUMP_REG(mmCP_CPF_BUSY_STAT);
> +	DUMP_REG(mmCP_CPF_STALLED_STAT1);

> +
> +	*n_regs = i;
> +
> +	(*r_names) = kcalloc(N_REGS, sizeof(char *), GFP_KERNEL);
> +	for (i = 0; i < N_REGS; i++) {
> +		(*r_names)[i] = kcalloc(30, sizeof(char), GFP_KERNEL);
> +		strcpy((*r_names)[i], reg_names[i]);
> +	}
> +
> +	return 0;
> +}
> +
>   static int gfx_v10_0_suspend(void *handle)
>   {
>   	return gfx_v10_0_hw_fini(handle);
> @@ -9359,6 +9411,7 @@ static const struct amd_ip_funcs gfx_v10_0_ip_funcs = {
>   	.set_clockgating_state = gfx_v10_0_set_clockgating_state,
>   	.set_powergating_state = gfx_v10_0_set_powergating_state,
>   	.get_clockgating_state = gfx_v10_0_get_clockgating_state,
> +	.reset_reg_dumps = gfx_v10_0_reset_reg_dumps,
>   };
>   
>   static const struct amdgpu_ring_funcs gfx_v10_0_ring_funcs_gfx = {
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
> index 4b9e68a79f06..98291a8f57dd 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -298,6 +298,8 @@ struct amd_ip_funcs {
>   				     enum amd_powergating_state state);
>   	void (*get_clockgating_state)(void *handle, u32 *flags);
>   	int (*enable_umd_pstate)(void *handle, enum amd_dpm_forced_level *level);
> +	int (*reset_reg_dumps)(void *handle,
> +			uint32_t (**dump)[2], uint32_t *n_regs, char ***r_names);
>   };
>   
>