RE: [PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from .late_init to .early_init
-Original Message- From: Zhou1, Tao Sent: Wednesday, January 19, 2022 2:11 PM To: Chai, Thomas ; amd-gfx@lists.freedesktop.org Cc: Zhang, Hawking ; Clements, John Subject: RE: [PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from .late_init to .early_init [AMD Official Use Only] > -Original Message- > From: Chai, Thomas > Sent: Wednesday, January 19, 2022 10:56 AM > To: amd-gfx@lists.freedesktop.org > Cc: Chai, Thomas ; Zhang, Hawking > ; Zhou1, Tao ; Clements, > John ; Chai, Thomas > Subject: [PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from > .late_init to .early_init > > Move xgmi ras initialization from .late_init to .early_init, which let > xgmi ras be initialized only once. > > Signed-off-by: yipechai > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 5 - > drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 1 - > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 9 + > 3 files changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > index 3483a82f5734..d83eee1984c8 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > @@ -452,11 +452,6 @@ int amdgpu_gmc_ras_late_init(struct amdgpu_device > *adev) > return r; > } > > - if (!adev->gmc.xgmi.connected_to_cpu) { > - adev->gmc.xgmi.ras = &xgmi_ras; > - amdgpu_ras_register_ras_block(adev, &adev->gmc.xgmi.ras- > >ras_block); > - } > - > if (adev->gmc.xgmi.ras && > adev->gmc.xgmi.ras->ras_block.ras_late_init) > { > r = adev->gmc.xgmi.ras->ras_block.ras_late_init(adev, NULL); > if (r) > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c > b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c > index 4f8d356f8432..5f9f82091000 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c > @@ -716,7 +716,6 @@ static void gmc_v10_0_set_gfxhub_funcs(struct > amdgpu_device *adev) > } > } > > - >[Tao]: Please don't introduce irrelevant change. [Thomas] OK > static int gmc_v10_0_early_init(void *handle) { > struct amdgpu_device *adev = (struct amdgpu_device *)handle; diff > --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > index c76ffd1a70cd..8d1b11368a7b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > @@ -1303,6 +1303,14 @@ static void gmc_v9_0_set_hdp_ras_funcs(struct > amdgpu_device *adev) > amdgpu_ras_register_ras_block(adev, &adev->hdp.ras->ras_block); } > > +static void gmc_v9_0_set_xgmi_ras_funcs(struct amdgpu_device *adev) { > + if (!adev->gmc.xgmi.connected_to_cpu) { > + adev->gmc.xgmi.ras = &xgmi_ras; > + amdgpu_ras_register_ras_block(adev, &adev->gmc.xgmi.ras- > >ras_block); > + } > +} >[Tao]: Since the initialization of xgmi.ras is common for all versions of IP, >I recommend to create a generic ras_early_init func for it. >BTW, I think we can remove the check for block_obj's existence in >register_ras_block now. [Thomas] I will update the patch. > + > static void gmc_v9_0_set_mca_funcs(struct amdgpu_device *adev) { > /* is UMC the right IP to check for MCA? Maybe DF? */ @@ -1339,6 > +1347,7 @@ static int gmc_v9_0_early_init(void *handle) > gmc_v9_0_set_gfxhub_funcs(adev); > gmc_v9_0_set_hdp_ras_funcs(adev); > gmc_v9_0_set_mca_funcs(adev); > + gmc_v9_0_set_xgmi_ras_funcs(adev); > > adev->gmc.shared_aperture_start = 0x2000ULL; > adev->gmc.shared_aperture_end = > -- > 2.25.1
RE: [PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from .late_init to .early_init
[AMD Official Use Only] Patch is Reviewed-by: Hawking Zhang Regards, Hawking -Original Message- From: Chai, Thomas Sent: Wednesday, January 19, 2022 10:56 To: amd-gfx@lists.freedesktop.org Cc: Chai, Thomas ; Zhang, Hawking ; Zhou1, Tao ; Clements, John ; Chai, Thomas Subject: [PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from .late_init to .early_init Move xgmi ras initialization from .late_init to .early_init, which let xgmi ras be initialized only once. Signed-off-by: yipechai --- drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 5 - drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 1 - drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 9 + 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c index 3483a82f5734..d83eee1984c8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c @@ -452,11 +452,6 @@ int amdgpu_gmc_ras_late_init(struct amdgpu_device *adev) return r; } - if (!adev->gmc.xgmi.connected_to_cpu) { - adev->gmc.xgmi.ras = &xgmi_ras; - amdgpu_ras_register_ras_block(adev, &adev->gmc.xgmi.ras->ras_block); - } - if (adev->gmc.xgmi.ras && adev->gmc.xgmi.ras->ras_block.ras_late_init) { r = adev->gmc.xgmi.ras->ras_block.ras_late_init(adev, NULL); if (r) diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c index 4f8d356f8432..5f9f82091000 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c @@ -716,7 +716,6 @@ static void gmc_v10_0_set_gfxhub_funcs(struct amdgpu_device *adev) } } - static int gmc_v10_0_early_init(void *handle) { struct amdgpu_device *adev = (struct amdgpu_device *)handle; diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c index c76ffd1a70cd..8d1b11368a7b 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c @@ -1303,6 +1303,14 @@ static void gmc_v9_0_set_hdp_ras_funcs(struct amdgpu_device *adev) amdgpu_ras_register_ras_block(adev, &adev->hdp.ras->ras_block); } +static void gmc_v9_0_set_xgmi_ras_funcs(struct amdgpu_device *adev) { + if (!adev->gmc.xgmi.connected_to_cpu) { + adev->gmc.xgmi.ras = &xgmi_ras; + amdgpu_ras_register_ras_block(adev, &adev->gmc.xgmi.ras->ras_block); + } +} + static void gmc_v9_0_set_mca_funcs(struct amdgpu_device *adev) { /* is UMC the right IP to check for MCA? Maybe DF? */ @@ -1339,6 +1347,7 @@ static int gmc_v9_0_early_init(void *handle) gmc_v9_0_set_gfxhub_funcs(adev); gmc_v9_0_set_hdp_ras_funcs(adev); gmc_v9_0_set_mca_funcs(adev); + gmc_v9_0_set_xgmi_ras_funcs(adev); adev->gmc.shared_aperture_start = 0x2000ULL; adev->gmc.shared_aperture_end = -- 2.25.1
RE: [PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from .late_init to .early_init
[AMD Official Use Only] > -Original Message- > From: Chai, Thomas > Sent: Wednesday, January 19, 2022 10:56 AM > To: amd-gfx@lists.freedesktop.org > Cc: Chai, Thomas ; Zhang, Hawking > ; Zhou1, Tao ; Clements, > John ; Chai, Thomas > Subject: [PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from .late_init > to .early_init > > Move xgmi ras initialization from .late_init to .early_init, which let xgmi > ras be > initialized only once. > > Signed-off-by: yipechai > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 5 - > drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 1 - > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 9 + > 3 files changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > index 3483a82f5734..d83eee1984c8 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > @@ -452,11 +452,6 @@ int amdgpu_gmc_ras_late_init(struct amdgpu_device > *adev) > return r; > } > > - if (!adev->gmc.xgmi.connected_to_cpu) { > - adev->gmc.xgmi.ras = &xgmi_ras; > - amdgpu_ras_register_ras_block(adev, &adev->gmc.xgmi.ras- > >ras_block); > - } > - > if (adev->gmc.xgmi.ras && adev->gmc.xgmi.ras->ras_block.ras_late_init) > { > r = adev->gmc.xgmi.ras->ras_block.ras_late_init(adev, NULL); > if (r) > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c > b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c > index 4f8d356f8432..5f9f82091000 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c > @@ -716,7 +716,6 @@ static void gmc_v10_0_set_gfxhub_funcs(struct > amdgpu_device *adev) > } > } > > - [Tao]: Please don't introduce irrelevant change. > static int gmc_v10_0_early_init(void *handle) { > struct amdgpu_device *adev = (struct amdgpu_device *)handle; diff --git > a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > index c76ffd1a70cd..8d1b11368a7b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > @@ -1303,6 +1303,14 @@ static void gmc_v9_0_set_hdp_ras_funcs(struct > amdgpu_device *adev) > amdgpu_ras_register_ras_block(adev, &adev->hdp.ras->ras_block); } > > +static void gmc_v9_0_set_xgmi_ras_funcs(struct amdgpu_device *adev) { > + if (!adev->gmc.xgmi.connected_to_cpu) { > + adev->gmc.xgmi.ras = &xgmi_ras; > + amdgpu_ras_register_ras_block(adev, &adev->gmc.xgmi.ras- > >ras_block); > + } > +} [Tao]: Since the initialization of xgmi.ras is common for all versions of IP, I recommend to create a generic ras_early_init func for it. BTW, I think we can remove the check for block_obj's existence in register_ras_block now. > + > static void gmc_v9_0_set_mca_funcs(struct amdgpu_device *adev) { > /* is UMC the right IP to check for MCA? Maybe DF? */ @@ -1339,6 > +1347,7 @@ static int gmc_v9_0_early_init(void *handle) > gmc_v9_0_set_gfxhub_funcs(adev); > gmc_v9_0_set_hdp_ras_funcs(adev); > gmc_v9_0_set_mca_funcs(adev); > + gmc_v9_0_set_xgmi_ras_funcs(adev); > > adev->gmc.shared_aperture_start = 0x2000ULL; > adev->gmc.shared_aperture_end = > -- > 2.25.1
[PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from .late_init to .early_init
Move xgmi ras initialization from .late_init to .early_init, which let xgmi ras be initialized only once. Signed-off-by: yipechai --- drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 5 - drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 1 - drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 9 + 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c index 3483a82f5734..d83eee1984c8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c @@ -452,11 +452,6 @@ int amdgpu_gmc_ras_late_init(struct amdgpu_device *adev) return r; } - if (!adev->gmc.xgmi.connected_to_cpu) { - adev->gmc.xgmi.ras = &xgmi_ras; - amdgpu_ras_register_ras_block(adev, &adev->gmc.xgmi.ras->ras_block); - } - if (adev->gmc.xgmi.ras && adev->gmc.xgmi.ras->ras_block.ras_late_init) { r = adev->gmc.xgmi.ras->ras_block.ras_late_init(adev, NULL); if (r) diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c index 4f8d356f8432..5f9f82091000 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c @@ -716,7 +716,6 @@ static void gmc_v10_0_set_gfxhub_funcs(struct amdgpu_device *adev) } } - static int gmc_v10_0_early_init(void *handle) { struct amdgpu_device *adev = (struct amdgpu_device *)handle; diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c index c76ffd1a70cd..8d1b11368a7b 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c @@ -1303,6 +1303,14 @@ static void gmc_v9_0_set_hdp_ras_funcs(struct amdgpu_device *adev) amdgpu_ras_register_ras_block(adev, &adev->hdp.ras->ras_block); } +static void gmc_v9_0_set_xgmi_ras_funcs(struct amdgpu_device *adev) +{ + if (!adev->gmc.xgmi.connected_to_cpu) { + adev->gmc.xgmi.ras = &xgmi_ras; + amdgpu_ras_register_ras_block(adev, &adev->gmc.xgmi.ras->ras_block); + } +} + static void gmc_v9_0_set_mca_funcs(struct amdgpu_device *adev) { /* is UMC the right IP to check for MCA? Maybe DF? */ @@ -1339,6 +1347,7 @@ static int gmc_v9_0_early_init(void *handle) gmc_v9_0_set_gfxhub_funcs(adev); gmc_v9_0_set_hdp_ras_funcs(adev); gmc_v9_0_set_mca_funcs(adev); + gmc_v9_0_set_xgmi_ras_funcs(adev); adev->gmc.shared_aperture_start = 0x2000ULL; adev->gmc.shared_aperture_end = -- 2.25.1