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

["Zhang, Hawking" <Hawking.Zhang-5C7GfCeVMHo@public.gmane.org>]

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@amd.com> 
Sent: 2019年9月9日 11:01
To: Zhang, Hawking <Hawking.Zhang@amd.com>; amd-gfx@lists.freedesktop.org; Chen, Guchun <Guchun.Chen@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@amd.com>
> Sent: 2019年9月9日 6:40
> To: Zhang, Hawking <Hawking.Zhang@amd.com>; Zhou1, Tao 
> <Tao.Zhou1@amd.com>; amd-gfx@lists.freedesktop.org; Chen, Guchun 
> <Guchun.Chen@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@lists.freedesktop.org> On Behalf Of 
> Zhang, Hawking
> Sent: 2019年9月9日 6:23
> To: Zhou1, Tao <Tao.Zhou1@amd.com>; amd-gfx@lists.freedesktop.org; 
> Chen, Guchun <Guchun.Chen@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@amd.com>
> Sent: 2019年9月6日 17:01
> To: amd-gfx@lists.freedesktop.org; Zhang, Hawking 
> <Hawking.Zhang@amd.com>; Chen, Guchun <Guchun.Chen@amd.com>
> Cc: Zhou1, Tao <Tao.Zhou1@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@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx