[PATCH 3/3] drm/amdgpu: rename umc ras_init to ras_asic_init

Zhang, Hawking Hawking.Zhang at amd.com
Tue Sep 10 02:22:43 UTC 2019


RE - err_cnt_init is good for current implementation but may be not enough for the future, how about rename it to umc.funcs->ras_hw_init?

I think it's better that one callback function map to one specific hw sequence. Going forward, if there is additional programming needed for another generation of IP, we can add it as a new callback function. But for now, err_cnt_init is exactly what we did. 

Regards,
Hawking

-----Original Message-----
From: Zhou1, Tao <Tao.Zhou1 at amd.com> 
Sent: 2019年9月9日 11:01
To: Zhang, Hawking <Hawking.Zhang at amd.com>; amd-gfx at lists.freedesktop.org; Chen, Guchun <Guchun.Chen at amd.com>
Subject: RE: [PATCH 3/3] drm/amdgpu: rename umc ras_init to ras_asic_init

umc.funcs->ras_late_init is common for all versions of umc, so it's implemented in amdgpu_umc.c, but ras_asic_init is specific to each version of umc and is placed in umc_vx_x.c.
err_cnt_init is good for current implementation but may be not enough for the future, how about rename it to umc.funcs->ras_hw_init?

Regards,
Tao

> -----Original Message-----
> From: Zhang, Hawking <Hawking.Zhang at amd.com>
> Sent: 2019年9月9日 6:40
> To: Zhang, Hawking <Hawking.Zhang at amd.com>; Zhou1, Tao 
> <Tao.Zhou1 at amd.com>; amd-gfx at lists.freedesktop.org; Chen, Guchun 
> <Guchun.Chen at amd.com>
> Subject: RE: [PATCH 3/3] drm/amdgpu: rename umc ras_init to 
> ras_asic_init
> 
> Never mind. I was confused by the name "ras_asic_init". The 
> programming sequence is exactly what we discussed before. I think we 
> can rename this function to "err_cnt_init".
> 
> Regards,
> Hawking
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of 
> Zhang, Hawking
> Sent: 2019年9月9日 6:23
> To: Zhou1, Tao <Tao.Zhou1 at amd.com>; amd-gfx at lists.freedesktop.org; 
> Chen, Guchun <Guchun.Chen at amd.com>
> Subject: RE: [PATCH 3/3] drm/amdgpu: rename umc ras_init to 
> ras_asic_init
> 
> The ras init (or the new asic_init) seems not necessary as last time 
> we discussed. Any UMC RAS register initialization is safe enough to be 
> centralized to ras_late_init interface. I would suggest to reduce such 
> kind of un-necessary interface.
> 
> Regards,
> Hawking
> -----Original Message-----
> From: Zhou1, Tao <Tao.Zhou1 at amd.com>
> Sent: 2019年9月6日 17:01
> To: amd-gfx at lists.freedesktop.org; Zhang, Hawking 
> <Hawking.Zhang at amd.com>; Chen, Guchun <Guchun.Chen at amd.com>
> Cc: Zhou1, Tao <Tao.Zhou1 at amd.com>
> Subject: [PATCH 3/3] drm/amdgpu: rename umc ras_init to ras_asic_init
> 
> this interface is related to specific version of umc, distinguish it 
> from ras_late_init
> 
> Signed-off-by: Tao Zhou <tao.zhou1 at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c | 4 ++-- 
> drivers/gpu/drm/amd/amdgpu/amdgpu_umc.h | 2 +-
>  drivers/gpu/drm/amd/amdgpu/umc_v6_1.c   | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c
> index 5683c51710aa..b1c7f643f198 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c
> @@ -63,8 +63,8 @@ int amdgpu_umc_ras_late_init(struct amdgpu_device 
> *adev, void *ras_ih_info)
>  	}
> 
>  	/* ras init of specific umc version */
> -	if (adev->umc.funcs && adev->umc.funcs->ras_init)
> -		adev->umc.funcs->ras_init(adev);
> +	if (adev->umc.funcs && adev->umc.funcs->ras_asic_init)
> +		adev->umc.funcs->ras_asic_init(adev);
> 
>  	return 0;
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.h
> index 6f22c9704555..a5e4df2440be 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.h
> @@ -54,7 +54,7 @@
>  	adev->umc.funcs->disable_umc_index_mode(adev);
> 
>  struct amdgpu_umc_funcs {
> -	void (*ras_init)(struct amdgpu_device *adev);
> +	void (*ras_asic_init)(struct amdgpu_device *adev);
>  	int (*ras_late_init)(struct amdgpu_device *adev, void *ras_ih_info);
>  	void (*query_ras_error_count)(struct amdgpu_device *adev,
>  					void *ras_error_status);
> diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v6_1.c
> b/drivers/gpu/drm/amd/amdgpu/umc_v6_1.c
> index 4cdb5c04cd17..92f3b148e181 100644
> --- a/drivers/gpu/drm/amd/amdgpu/umc_v6_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/umc_v6_1.c
> @@ -272,7 +272,7 @@ static void umc_v6_1_ras_init(struct amdgpu_device
> *adev)  }
> 
>  const struct amdgpu_umc_funcs umc_v6_1_funcs = {
> -	.ras_init = umc_v6_1_ras_init,
> +	.ras_asic_init = umc_v6_1_ras_init,
>  	.ras_late_init = amdgpu_umc_ras_late_init,
>  	.query_ras_error_count = umc_v6_1_query_ras_error_count,
>  	.query_ras_error_address = umc_v6_1_query_ras_error_address,
> --
> 2.17.1
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


More information about the amd-gfx mailing list