[AMD Official Use Only]


> -----Original Message-----
> From: Chai, Thomas <yipeng.c...@amd.com>
> Sent: Wednesday, February 9, 2022 1:57 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Chai, Thomas <yipeng.c...@amd.com>; Zhang, Hawking
> <hawking.zh...@amd.com>; Zhou1, Tao <tao.zh...@amd.com>; Clements,
> John <john.cleme...@amd.com>; Chai, Thomas <yipeng.c...@amd.com>
> Subject: [PATCH 01/11] drm/amdgpu: Optimize
> xxx_ras_late_init/xxx_ras_late_fini for each ras block
> 
> 1. Define amdgpu_ras_block_late_init to create sysfs nodes
>    and interrupt handles.
> 2. Define amdgpu_ras_block_late_fini to remove sysfs nodes
>    and interrupt handles.
> 3. Replace ras block variable members in struct
>    amdgpu_ras_block_object with struct ras_common_if, which
>    can makes it easy to associate each ras block instance
[Tao]: makes -> make

>    with each ras block functional interface.
> 4. Add .ras_cb to struct amdgpu_ras_block_object.
> 5. Change each ras block to fit for the changement of struct
>    amdgpu_ras_block_object.
> 
> Signed-off-by: yipechai <yipeng.c...@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c  |  7 +++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c  | 35
> +++++++++++++++++++++++-  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h  |
> 15 ++++++----  drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c |  6 ++--
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c    |  4 +--
>  drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c   |  4 +--
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c    |  8 +++---
>  drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c    |  6 ++--
>  drivers/gpu/drm/amd/amdgpu/mca_v3_0.c    | 28 +++++++++++--------
>  drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c   |  6 ++--
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c   |  4 +--
>  11 files changed, 86 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c
> index 52a60c2316a2..ad057d6b2c77 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mca.c
> @@ -83,14 +83,15 @@ int amdgpu_mca_ras_late_init(struct amdgpu_device
> *adev,
>               .sysfs_name = sysfs_name,
>       };
> 
> -     snprintf(sysfs_name, sizeof(sysfs_name), "%s_err_count", mca_dev-
> >ras->ras_block.name);
> +     snprintf(sysfs_name, sizeof(sysfs_name), "%s_err_count",
> +             mca_dev->ras->ras_block.ras_comm.name);
> 
>       if (!mca_dev->ras_if) {
>               mca_dev->ras_if = kmalloc(sizeof(struct ras_common_if),
> GFP_KERNEL);
>               if (!mca_dev->ras_if)
>                       return -ENOMEM;
> -             mca_dev->ras_if->block = mca_dev->ras->ras_block.block;
> -             mca_dev->ras_if->sub_block_index = mca_dev->ras-
> >ras_block.sub_block_index;
> +             mca_dev->ras_if->block = mca_dev->ras-
> >ras_block.ras_comm.block;
> +             mca_dev->ras_if->sub_block_index =
> +mca_dev->ras->ras_block.ras_comm.sub_block_index;
>               mca_dev->ras_if->type =
> AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE;
>       }
>       ih_info.head = fs_info.head = *mca_dev->ras_if; diff --git
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index 5934326b9db3..b7aed19db7e9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -877,7 +877,7 @@ static int amdgpu_ras_block_match_default(struct
> amdgpu_ras_block_object *block_
>       if (!block_obj)
>               return -EINVAL;
> 
> -     if (block_obj->block == block)
> +     if (block_obj->ras_comm.block == block)
>               return 0;
> 
>       return -EINVAL;
> @@ -2457,6 +2457,23 @@ int amdgpu_ras_late_init(struct amdgpu_device
> *adev,
>       return r;
>  }
> 
> +int amdgpu_ras_block_late_init(struct amdgpu_device *adev,
> +                     struct ras_common_if *ras_block)
> +{
> +     char sysfs_name[32];
> +     struct ras_ih_if ih_info;
> +     struct ras_fs_if fs_info;
> +     struct amdgpu_ras_block_object *obj;
> +
> +     obj = container_of(ras_block, struct amdgpu_ras_block_object,
> ras_comm);
> +     ih_info.cb = obj->ras_cb;
> +     ih_info.head = *ras_block;
> +     snprintf(sysfs_name, sizeof(sysfs_name), "%s_err_count", ras_block-
> >name);
> +     fs_info.sysfs_name = (const char *)sysfs_name;
> +     fs_info.head = *ras_block;
> +     return amdgpu_ras_late_init(adev, ras_block, &fs_info, &ih_info); }
> +
>  /* helper function to remove ras fs node and interrupt handler */  void
> amdgpu_ras_late_fini(struct amdgpu_device *adev,
>                         struct ras_common_if *ras_block,
> @@ -2470,6 +2487,22 @@ void amdgpu_ras_late_fini(struct amdgpu_device
> *adev,
>               amdgpu_ras_interrupt_remove_handler(adev, ih_info);  }
> 
> +void amdgpu_ras_block_late_fini(struct amdgpu_device *adev,
> +                       struct ras_common_if *ras_block)
> +{
> +     struct ras_ih_if ih_info;
> +     struct amdgpu_ras_block_object *obj;
> +
> +     if (!ras_block)
> +             return;
> +
> +     obj = container_of(ras_block, struct amdgpu_ras_block_object,
> ras_comm);
> +     ih_info.head = *ras_block;
> +     ih_info.cb = obj->ras_cb;
> +
> +     amdgpu_ras_late_fini(adev, ras_block, &ih_info); }
> +
>  /* do some init work after IP late init as dependence.
>   * and it runs in resume/gpu reset/booting up cases.
>   */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> index a55743b12d57..8b94b556baf6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> @@ -486,17 +486,13 @@ struct ras_debug_if {  };
> 
>  struct amdgpu_ras_block_object {
> -     /* block name */
> -     char name[32];
> -
> -     enum amdgpu_ras_block block;
> -
> -     uint32_t sub_block_index;
> +     struct ras_common_if  ras_comm;
> 
>       int (*ras_block_match)(struct amdgpu_ras_block_object *block_obj,
>                               enum amdgpu_ras_block block, uint32_t
> sub_block_index);
>       int (*ras_late_init)(struct amdgpu_device *adev, void *ras_info);
>       void (*ras_fini)(struct amdgpu_device *adev);
> +     ras_ih_cb ras_cb;
>       const struct amdgpu_ras_block_hw_ops *hw_ops;  };
> 
> @@ -605,10 +601,17 @@ int amdgpu_ras_late_init(struct amdgpu_device
> *adev,
>                        struct ras_common_if *ras_block,
>                        struct ras_fs_if *fs_info,
>                        struct ras_ih_if *ih_info);
> +
> +int amdgpu_ras_block_late_init(struct amdgpu_device *adev,
> +                     struct ras_common_if *ras_block);
> +
>  void amdgpu_ras_late_fini(struct amdgpu_device *adev,
>                         struct ras_common_if *ras_block,
>                         struct ras_ih_if *ih_info);
> 
> +void amdgpu_ras_block_late_fini(struct amdgpu_device *adev,
> +                       struct ras_common_if *ras_block);
> +
>  int amdgpu_ras_feature_enable(struct amdgpu_device *adev,
>               struct ras_common_if *head, bool enable);
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index 5929d6f528c9..15707af89212 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -981,8 +981,10 @@ struct amdgpu_ras_block_hw_ops  xgmi_ras_hw_ops
> = {
> 
>  struct amdgpu_xgmi_ras xgmi_ras = {
>       .ras_block = {
> -             .name = "xgmi",
> -             .block = AMDGPU_RAS_BLOCK__XGMI_WAFL,
> +             .ras_comm = {
> +                     .name = "xgmi",
> +                     .block = AMDGPU_RAS_BLOCK__XGMI_WAFL,
> +             },
>               .hw_ops = &xgmi_ras_hw_ops,
>               .ras_late_init = amdgpu_xgmi_ras_late_init,
>               .ras_fini = amdgpu_xgmi_ras_fini,
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index ca7b886c6ce6..0a291d2e5f91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -2198,8 +2198,8 @@ static int gfx_v9_0_gpu_early_init(struct
> amdgpu_device *adev)
>                       return err;
>               }
> 
> -             strcpy(adev->gfx.ras->ras_block.name,"gfx");
> -             adev->gfx.ras->ras_block.block = AMDGPU_RAS_BLOCK__GFX;
> +             strcpy(adev->gfx.ras->ras_block.ras_comm.name, "gfx");
> +             adev->gfx.ras->ras_block.ras_comm.block =
> AMDGPU_RAS_BLOCK__GFX;
> 
>               /* If not define special ras_late_init function, use gfx default
> ras_late_init */
>               if (!adev->gfx.ras->ras_block.ras_late_init)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> index bddaf2417344..2a362c570346 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> @@ -672,8 +672,8 @@ static void gmc_v10_0_set_umc_funcs(struct
> amdgpu_device *adev)
>       if (adev->umc.ras) {
>               amdgpu_ras_register_ras_block(adev, &adev->umc.ras-
> >ras_block);
> 
> -             strcpy(adev->umc.ras->ras_block.name, "umc");
> -             adev->umc.ras->ras_block.block =
> AMDGPU_RAS_BLOCK__UMC;
> +             strcpy(adev->umc.ras->ras_block.ras_comm.name, "umc");
> +             adev->umc.ras->ras_block.ras_comm.block =
> AMDGPU_RAS_BLOCK__UMC;
> 
>               /* If don't define special ras_late_init function, use default
> ras_late_init */
>               if (!adev->umc.ras->ras_block.ras_late_init)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 4595027a8c63..af873c99d5e4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1232,8 +1232,8 @@ static void gmc_v9_0_set_umc_funcs(struct
> amdgpu_device *adev)
>       if (adev->umc.ras) {
>               amdgpu_ras_register_ras_block(adev, &adev->umc.ras-
> >ras_block);
> 
> -             strcpy(adev->umc.ras->ras_block.name, "umc");
> -             adev->umc.ras->ras_block.block =
> AMDGPU_RAS_BLOCK__UMC;
> +             strcpy(adev->umc.ras->ras_block.ras_comm.name, "umc");
> +             adev->umc.ras->ras_block.ras_comm.block =
> AMDGPU_RAS_BLOCK__UMC;
> 
>               /* If don't define special ras_late_init function, use default
> ras_late_init */
>               if (!adev->umc.ras->ras_block.ras_late_init)
> @@ -1280,8 +1280,8 @@ static void gmc_v9_0_set_mmhub_ras_funcs(struct
> amdgpu_device *adev)
>       if (adev->mmhub.ras) {
>               amdgpu_ras_register_ras_block(adev, &adev->mmhub.ras-
> >ras_block);
> 
> -             strcpy(adev->mmhub.ras->ras_block.name,"mmhub");
> -             adev->mmhub.ras->ras_block.block =
> AMDGPU_RAS_BLOCK__MMHUB;
> +             strcpy(adev->mmhub.ras->ras_block.ras_comm.name,
> "mmhub");
> +             adev->mmhub.ras->ras_block.ras_comm.block =
> AMDGPU_RAS_BLOCK__MMHUB;
> 
>               /* If don't define special ras_late_init function, use default
> ras_late_init */
>               if (!adev->mmhub.ras->ras_block.ras_late_init)
> diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> index 6b41fcbf4875..503c292b321e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> @@ -157,8 +157,10 @@ struct amdgpu_ras_block_hw_ops
> hdp_v4_0_ras_hw_ops = {
> 
>  struct amdgpu_hdp_ras hdp_v4_0_ras = {
>       .ras_block = {
> -             .name = "hdp",
> -             .block = AMDGPU_RAS_BLOCK__HDP,
> +             .ras_comm = {
> +                     .name = "hdp",
> +                     .block = AMDGPU_RAS_BLOCK__HDP,
> +             },
>               .hw_ops = &hdp_v4_0_ras_hw_ops,
>               .ras_late_init = amdgpu_hdp_ras_late_init,
>               .ras_fini = amdgpu_hdp_ras_fini,
> diff --git a/drivers/gpu/drm/amd/amdgpu/mca_v3_0.c
> b/drivers/gpu/drm/amd/amdgpu/mca_v3_0.c
> index 68565262af9c..386416378a82 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mca_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mca_v3_0.c
> @@ -53,8 +53,8 @@ static int mca_v3_0_ras_block_match(struct
> amdgpu_ras_block_object *block_obj,
>       if (!block_obj)
>               return -EINVAL;
> 
> -     if ((block_obj->block == block) &&
> -             (block_obj->sub_block_index == sub_block_index)) {
> +     if ((block_obj->ras_comm.block == block) &&
> +             (block_obj->ras_comm.sub_block_index == sub_block_index)) {
>               return 0;
>       }
> 
> @@ -68,9 +68,11 @@ const struct amdgpu_ras_block_hw_ops
> mca_v3_0_mp0_hw_ops = {
> 
>  struct amdgpu_mca_ras_block mca_v3_0_mp0_ras = {
>       .ras_block = {
> -             .block = AMDGPU_RAS_BLOCK__MCA,
> -             .sub_block_index = AMDGPU_RAS_MCA_BLOCK__MP0,
> -             .name = "mp0",
> +             .ras_comm = {
> +                     .block = AMDGPU_RAS_BLOCK__MCA,
> +                     .sub_block_index = AMDGPU_RAS_MCA_BLOCK__MP0,
> +                     .name = "mp0",
> +             },
>               .hw_ops = &mca_v3_0_mp0_hw_ops,
>               .ras_block_match = mca_v3_0_ras_block_match,
>               .ras_late_init = mca_v3_0_mp0_ras_late_init, @@ -103,9
> +105,11 @@ const struct amdgpu_ras_block_hw_ops mca_v3_0_mp1_hw_ops
> = {
> 
>  struct amdgpu_mca_ras_block mca_v3_0_mp1_ras = {
>       .ras_block = {
> -             .block = AMDGPU_RAS_BLOCK__MCA,
> -             .sub_block_index = AMDGPU_RAS_MCA_BLOCK__MP1,
> -             .name = "mp1",
> +             .ras_comm = {
> +                     .block = AMDGPU_RAS_BLOCK__MCA,
> +                     .sub_block_index = AMDGPU_RAS_MCA_BLOCK__MP1,
> +                     .name = "mp1",
> +             },
>               .hw_ops = &mca_v3_0_mp1_hw_ops,
>               .ras_block_match = mca_v3_0_ras_block_match,
>               .ras_late_init = mca_v3_0_mp1_ras_late_init, @@ -138,9
> +142,11 @@ const struct amdgpu_ras_block_hw_ops mca_v3_0_mpio_hw_ops
> = {
> 
>  struct amdgpu_mca_ras_block mca_v3_0_mpio_ras = {
>       .ras_block = {
> -             .block = AMDGPU_RAS_BLOCK__MCA,
> -             .sub_block_index = AMDGPU_RAS_MCA_BLOCK__MPIO,
> -             .name = "mpio",
> +             .ras_comm = {
> +                     .block = AMDGPU_RAS_BLOCK__MCA,
> +                     .sub_block_index =
> AMDGPU_RAS_MCA_BLOCK__MPIO,
> +                     .name = "mpio",
> +             },
>               .hw_ops = &mca_v3_0_mpio_hw_ops,
>               .ras_block_match = mca_v3_0_ras_block_match,
>               .ras_late_init = mca_v3_0_mpio_ras_late_init, diff --git
> a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> index 39974b449341..c7cca87f1647 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> @@ -664,8 +664,10 @@ const struct amdgpu_ras_block_hw_ops
> nbio_v7_4_ras_hw_ops = {
> 
>  struct amdgpu_nbio_ras nbio_v7_4_ras = {
>       .ras_block = {
> -             .name = "pcie_bif",
> -             .block = AMDGPU_RAS_BLOCK__PCIE_BIF,
> +             .ras_comm = {
> +                     .name = "pcie_bif",
> +                     .block = AMDGPU_RAS_BLOCK__PCIE_BIF,
> +             },
>               .hw_ops = &nbio_v7_4_ras_hw_ops,
>               .ras_late_init = amdgpu_nbio_ras_late_init,
>               .ras_fini = amdgpu_nbio_ras_fini,
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 06a7ceda4c87..8b0a8587dd36 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -2814,8 +2814,8 @@ static void sdma_v4_0_set_ras_funcs(struct
> amdgpu_device *adev)
>       if (adev->sdma.ras) {
>               amdgpu_ras_register_ras_block(adev, &adev->sdma.ras-
> >ras_block);
> 
> -             strcpy(adev->sdma.ras->ras_block.name, "sdma");
> -             adev->sdma.ras->ras_block.block =
> AMDGPU_RAS_BLOCK__SDMA;
> +             strcpy(adev->sdma.ras->ras_block.ras_comm.name, "sdma");
> +             adev->sdma.ras->ras_block.ras_comm.block =
> AMDGPU_RAS_BLOCK__SDMA;
> 
>               /* If don't define special ras_late_init function, use default
> ras_late_init */
>               if (!adev->sdma.ras->ras_block.ras_late_init)
> --
> 2.25.1

Reply via email to