RE: [PATCH 03/11] drm/amdgpu: Optimize amdgpu_hdp_ras_late_init/amdgpu_hdp_ras_fini function code
[AMD Official Use Only] OK, if there is further refinement, the series is: Reviewed-by: Tao Zhou > -Original Message- > From: Chai, Thomas > Sent: Thursday, February 10, 2022 10:59 AM > To: Zhou1, Tao ; amd-gfx@lists.freedesktop.org > Cc: Zhang, Hawking ; Clements, John > > Subject: RE: [PATCH 03/11] drm/amdgpu: Optimize > amdgpu_hdp_ras_late_init/amdgpu_hdp_ras_fini function code > > [AMD Official Use Only] > > > > -Original Message- > From: Zhou1, Tao > Sent: Wednesday, February 9, 2022 4:54 PM > To: Chai, Thomas ; amd-gfx@lists.freedesktop.org > Cc: Zhang, Hawking ; Clements, John > > Subject: RE: [PATCH 03/11] drm/amdgpu: Optimize > amdgpu_hdp_ras_late_init/amdgpu_hdp_ras_fini function code > > [AMD Official Use Only] > > > > > -Original Message- > > From: Chai, Thomas > > Sent: Wednesday, February 9, 2022 1:57 PM > > To: amd-gfx@lists.freedesktop.org > > Cc: Chai, Thomas ; Zhang, Hawking > > ; Zhou1, Tao ; Clements, > > John ; Chai, Thomas > > Subject: [PATCH 03/11] drm/amdgpu: Optimize > > amdgpu_hdp_ras_late_init/amdgpu_hdp_ras_fini function code > > > > Optimize amdgpu_hdp_ras_late_init/amdgpu_hdp_ras_fini function code. > > > > Signed-off-by: yipechai > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c | 37 ++--- > > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 1 + > > drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c | 1 + > > 3 files changed, 5 insertions(+), 34 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c > > index 518966a26130..21a5f884dd2a 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c > > @@ -26,43 +26,12 @@ > > > > int amdgpu_hdp_ras_late_init(struct amdgpu_device *adev, void *ras_info) { > > - int r; > > - struct ras_ih_if ih_info = { > > - .cb = NULL, > > - }; > > - struct ras_fs_if fs_info = { > > - .sysfs_name = "hdp_err_count", > > - }; > > - > > - if (!adev->hdp.ras_if) { > > - adev->hdp.ras_if = kmalloc(sizeof(struct ras_common_if), > > GFP_KERNEL); > > - if (!adev->hdp.ras_if) > > - return -ENOMEM; > > - adev->hdp.ras_if->block = AMDGPU_RAS_BLOCK__HDP; > > - adev->hdp.ras_if->type = > > AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE; > > - adev->hdp.ras_if->sub_block_index = 0; > > - } > > - ih_info.head = fs_info.head = *adev->hdp.ras_if; > > - r = amdgpu_ras_late_init(adev, adev->hdp.ras_if, > > -_info, _info); > > - if (r || !amdgpu_ras_is_supported(adev, adev->hdp.ras_if->block)) { > > - kfree(adev->hdp.ras_if); > > - adev->hdp.ras_if = NULL; > > - } > > - > > - return r; > > + return amdgpu_ras_block_late_init(adev, adev->hdp.ras_if); > > } > > > > void amdgpu_hdp_ras_fini(struct amdgpu_device *adev) { > > if (amdgpu_ras_is_supported(adev, AMDGPU_RAS_BLOCK__HDP) && > > - adev->hdp.ras_if) { > > - struct ras_common_if *ras_if = adev->hdp.ras_if; > > - struct ras_ih_if ih_info = { > > - .cb = NULL, > > - }; > > - > > - amdgpu_ras_late_fini(adev, ras_if, _info); > > - kfree(ras_if); > > - } > > + adev->hdp.ras_if) > > + amdgpu_ras_block_late_fini(adev, adev->hdp.ras_if); > > } > >[Tao]: Since hdp_ras_late_init/fini are simple wrapper, can we remove them > and call amdgpu_ras_block_late_init/fini directly? > >The same comment to other blocks. > > [Thomas] Compared with amdgpu_ras_block_late_init/fin, > hdp_ras_late_init/fini have different function interface parameters. > But can do it as a new ticket later. > > > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > > b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > > index af873c99d5e4..b12fe6703f02 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > > @@ -1302,6 +1302,7 @@ static void gmc_v9_0_set_hdp_ras_funcs(struct > > amdgpu_device *adev) { > > adev->hdp.ras = _v4_0_ras; > > amdgpu_ras_register_ras_block(adev, >hdp.ras->ras_block); > > + adev->hdp.ras_if = >hdp.ras->ras_block.ras_comm; > > } > > > > static void gmc_v9_0_set_mca_funcs(struct amdgpu_device *adev) diff > > --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c > > b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c > > index 503c292b321e..a9ed4232cdeb 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c > > @@ -160,6 +160,7 @@ struct amdgpu_hdp_ras hdp_v4_0_ras = { > > .ras_comm = { > > .name = "hdp", > > .block = AMDGPU_RAS_BLOCK__HDP, > > + .type = > > AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE, > > }, > > .hw_ops = _v4_0_ras_hw_ops, > > .ras_late_init = amdgpu_hdp_ras_late_init, > > -- > > 2.25.1
RE: [PATCH 03/11] drm/amdgpu: Optimize amdgpu_hdp_ras_late_init/amdgpu_hdp_ras_fini function code
[AMD Official Use Only] -Original Message- From: Zhou1, Tao Sent: Wednesday, February 9, 2022 4:54 PM To: Chai, Thomas ; amd-gfx@lists.freedesktop.org Cc: Zhang, Hawking ; Clements, John Subject: RE: [PATCH 03/11] drm/amdgpu: Optimize amdgpu_hdp_ras_late_init/amdgpu_hdp_ras_fini function code [AMD Official Use Only] > -Original Message- > From: Chai, Thomas > Sent: Wednesday, February 9, 2022 1:57 PM > To: amd-gfx@lists.freedesktop.org > Cc: Chai, Thomas ; Zhang, Hawking > ; Zhou1, Tao ; Clements, > John ; Chai, Thomas > Subject: [PATCH 03/11] drm/amdgpu: Optimize > amdgpu_hdp_ras_late_init/amdgpu_hdp_ras_fini function code > > Optimize amdgpu_hdp_ras_late_init/amdgpu_hdp_ras_fini function code. > > Signed-off-by: yipechai > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c | 37 ++--- > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 1 + > drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c | 1 + > 3 files changed, 5 insertions(+), 34 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c > index 518966a26130..21a5f884dd2a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c > @@ -26,43 +26,12 @@ > > int amdgpu_hdp_ras_late_init(struct amdgpu_device *adev, void *ras_info) { > - int r; > - struct ras_ih_if ih_info = { > - .cb = NULL, > - }; > - struct ras_fs_if fs_info = { > - .sysfs_name = "hdp_err_count", > - }; > - > - if (!adev->hdp.ras_if) { > - adev->hdp.ras_if = kmalloc(sizeof(struct ras_common_if), > GFP_KERNEL); > - if (!adev->hdp.ras_if) > - return -ENOMEM; > - adev->hdp.ras_if->block = AMDGPU_RAS_BLOCK__HDP; > - adev->hdp.ras_if->type = > AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE; > - adev->hdp.ras_if->sub_block_index = 0; > - } > - ih_info.head = fs_info.head = *adev->hdp.ras_if; > - r = amdgpu_ras_late_init(adev, adev->hdp.ras_if, > - _info, _info); > - if (r || !amdgpu_ras_is_supported(adev, adev->hdp.ras_if->block)) { > - kfree(adev->hdp.ras_if); > - adev->hdp.ras_if = NULL; > - } > - > - return r; > + return amdgpu_ras_block_late_init(adev, adev->hdp.ras_if); > } > > void amdgpu_hdp_ras_fini(struct amdgpu_device *adev) { > if (amdgpu_ras_is_supported(adev, AMDGPU_RAS_BLOCK__HDP) && > - adev->hdp.ras_if) { > - struct ras_common_if *ras_if = adev->hdp.ras_if; > - struct ras_ih_if ih_info = { > - .cb = NULL, > - }; > - > - amdgpu_ras_late_fini(adev, ras_if, _info); > - kfree(ras_if); > - } > + adev->hdp.ras_if) > + amdgpu_ras_block_late_fini(adev, adev->hdp.ras_if); > } >[Tao]: Since hdp_ras_late_init/fini are simple wrapper, can we remove them and >call amdgpu_ras_block_late_init/fini directly? >The same comment to other blocks. [Thomas] Compared with amdgpu_ras_block_late_init/fin, hdp_ras_late_init/fini have different function interface parameters. But can do it as a new ticket later. > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > index af873c99d5e4..b12fe6703f02 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > @@ -1302,6 +1302,7 @@ static void gmc_v9_0_set_hdp_ras_funcs(struct > amdgpu_device *adev) { > adev->hdp.ras = _v4_0_ras; > amdgpu_ras_register_ras_block(adev, >hdp.ras->ras_block); > + adev->hdp.ras_if = >hdp.ras->ras_block.ras_comm; > } > > static void gmc_v9_0_set_mca_funcs(struct amdgpu_device *adev) diff > --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c > b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c > index 503c292b321e..a9ed4232cdeb 100644 > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c > @@ -160,6 +160,7 @@ struct amdgpu_hdp_ras hdp_v4_0_ras = { > .ras_comm = { > .name = "hdp", > .block = AMDGPU_RAS_BLOCK__HDP, > + .type = > AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE, > }, > .hw_ops = _v4_0_ras_hw_ops, > .ras_late_init = amdgpu_hdp_ras_late_init, > -- > 2.25.1
RE: [PATCH 03/11] drm/amdgpu: Optimize amdgpu_hdp_ras_late_init/amdgpu_hdp_ras_fini function code
[AMD Official Use Only] > -Original Message- > From: Chai, Thomas > Sent: Wednesday, February 9, 2022 1:57 PM > To: amd-gfx@lists.freedesktop.org > Cc: Chai, Thomas ; Zhang, Hawking > ; Zhou1, Tao ; Clements, > John ; Chai, Thomas > Subject: [PATCH 03/11] drm/amdgpu: Optimize > amdgpu_hdp_ras_late_init/amdgpu_hdp_ras_fini function code > > Optimize amdgpu_hdp_ras_late_init/amdgpu_hdp_ras_fini function code. > > Signed-off-by: yipechai > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c | 37 ++--- > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 1 + > drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c | 1 + > 3 files changed, 5 insertions(+), 34 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c > index 518966a26130..21a5f884dd2a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c > @@ -26,43 +26,12 @@ > > int amdgpu_hdp_ras_late_init(struct amdgpu_device *adev, void *ras_info) { > - int r; > - struct ras_ih_if ih_info = { > - .cb = NULL, > - }; > - struct ras_fs_if fs_info = { > - .sysfs_name = "hdp_err_count", > - }; > - > - if (!adev->hdp.ras_if) { > - adev->hdp.ras_if = kmalloc(sizeof(struct ras_common_if), > GFP_KERNEL); > - if (!adev->hdp.ras_if) > - return -ENOMEM; > - adev->hdp.ras_if->block = AMDGPU_RAS_BLOCK__HDP; > - adev->hdp.ras_if->type = > AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE; > - adev->hdp.ras_if->sub_block_index = 0; > - } > - ih_info.head = fs_info.head = *adev->hdp.ras_if; > - r = amdgpu_ras_late_init(adev, adev->hdp.ras_if, > - _info, _info); > - if (r || !amdgpu_ras_is_supported(adev, adev->hdp.ras_if->block)) { > - kfree(adev->hdp.ras_if); > - adev->hdp.ras_if = NULL; > - } > - > - return r; > + return amdgpu_ras_block_late_init(adev, adev->hdp.ras_if); > } > > void amdgpu_hdp_ras_fini(struct amdgpu_device *adev) { > if (amdgpu_ras_is_supported(adev, AMDGPU_RAS_BLOCK__HDP) && > - adev->hdp.ras_if) { > - struct ras_common_if *ras_if = adev->hdp.ras_if; > - struct ras_ih_if ih_info = { > - .cb = NULL, > - }; > - > - amdgpu_ras_late_fini(adev, ras_if, _info); > - kfree(ras_if); > - } > + adev->hdp.ras_if) > + amdgpu_ras_block_late_fini(adev, adev->hdp.ras_if); > } [Tao]: Since hdp_ras_late_init/fini are simple wrapper, can we remove them and call amdgpu_ras_block_late_init/fini directly? The same comment to other blocks. > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > index af873c99d5e4..b12fe6703f02 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > @@ -1302,6 +1302,7 @@ static void gmc_v9_0_set_hdp_ras_funcs(struct > amdgpu_device *adev) { > adev->hdp.ras = _v4_0_ras; > amdgpu_ras_register_ras_block(adev, >hdp.ras->ras_block); > + adev->hdp.ras_if = >hdp.ras->ras_block.ras_comm; > } > > static void gmc_v9_0_set_mca_funcs(struct amdgpu_device *adev) diff --git > a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c > b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c > index 503c292b321e..a9ed4232cdeb 100644 > --- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c > @@ -160,6 +160,7 @@ struct amdgpu_hdp_ras hdp_v4_0_ras = { > .ras_comm = { > .name = "hdp", > .block = AMDGPU_RAS_BLOCK__HDP, > + .type = > AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE, > }, > .hw_ops = _v4_0_ras_hw_ops, > .ras_late_init = amdgpu_hdp_ras_late_init, > -- > 2.25.1
[PATCH 03/11] drm/amdgpu: Optimize amdgpu_hdp_ras_late_init/amdgpu_hdp_ras_fini function code
Optimize amdgpu_hdp_ras_late_init/amdgpu_hdp_ras_fini function code. Signed-off-by: yipechai --- drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c | 37 ++--- drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 1 + drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c | 1 + 3 files changed, 5 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c index 518966a26130..21a5f884dd2a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c @@ -26,43 +26,12 @@ int amdgpu_hdp_ras_late_init(struct amdgpu_device *adev, void *ras_info) { - int r; - struct ras_ih_if ih_info = { - .cb = NULL, - }; - struct ras_fs_if fs_info = { - .sysfs_name = "hdp_err_count", - }; - - if (!adev->hdp.ras_if) { - adev->hdp.ras_if = kmalloc(sizeof(struct ras_common_if), GFP_KERNEL); - if (!adev->hdp.ras_if) - return -ENOMEM; - adev->hdp.ras_if->block = AMDGPU_RAS_BLOCK__HDP; - adev->hdp.ras_if->type = AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE; - adev->hdp.ras_if->sub_block_index = 0; - } - ih_info.head = fs_info.head = *adev->hdp.ras_if; - r = amdgpu_ras_late_init(adev, adev->hdp.ras_if, -_info, _info); - if (r || !amdgpu_ras_is_supported(adev, adev->hdp.ras_if->block)) { - kfree(adev->hdp.ras_if); - adev->hdp.ras_if = NULL; - } - - return r; + return amdgpu_ras_block_late_init(adev, adev->hdp.ras_if); } void amdgpu_hdp_ras_fini(struct amdgpu_device *adev) { if (amdgpu_ras_is_supported(adev, AMDGPU_RAS_BLOCK__HDP) && - adev->hdp.ras_if) { - struct ras_common_if *ras_if = adev->hdp.ras_if; - struct ras_ih_if ih_info = { - .cb = NULL, - }; - - amdgpu_ras_late_fini(adev, ras_if, _info); - kfree(ras_if); - } + adev->hdp.ras_if) + amdgpu_ras_block_late_fini(adev, adev->hdp.ras_if); } diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c index af873c99d5e4..b12fe6703f02 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c @@ -1302,6 +1302,7 @@ static void gmc_v9_0_set_hdp_ras_funcs(struct amdgpu_device *adev) { adev->hdp.ras = _v4_0_ras; amdgpu_ras_register_ras_block(adev, >hdp.ras->ras_block); + adev->hdp.ras_if = >hdp.ras->ras_block.ras_comm; } static void gmc_v9_0_set_mca_funcs(struct amdgpu_device *adev) diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c index 503c292b321e..a9ed4232cdeb 100644 --- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c @@ -160,6 +160,7 @@ struct amdgpu_hdp_ras hdp_v4_0_ras = { .ras_comm = { .name = "hdp", .block = AMDGPU_RAS_BLOCK__HDP, + .type = AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE, }, .hw_ops = _v4_0_ras_hw_ops, .ras_late_init = amdgpu_hdp_ras_late_init, -- 2.25.1