Re: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers
[Public] If it needs to be in drm-next, please make sure it lands there. Alex From: amd-gfx on behalf of Liu, Shaoyun Sent: Thursday, December 16, 2021 12:51 PM To: Skvortsov, Victor ; Alex Deucher Cc: Ming, Davis ; Chen, JingWen ; amd-gfx list ; Deng, Emily ; Chen, Horace ; Liu, Monk ; Nieto, David M Subject: RE: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers [AMD Official Use Only] Actually I don't know why the change " a35f147621bc drm/amdgpu: get xgmi info at eary_init " not in drm-next , instead it’s in amd-mainline-dkms-5.13. That change is necessary for passthrough XGMI hive to a VM and rely on our driver to do the reset on whole hive when driver is loaded . I checked the code again, it seems we should be ok as long as we get xgmi info at eary_init. So since gfx_v9_0_set_rlc_funcs() already gets called in gfx_v9_0_early_init(), we can move get xgmi info out of gmc_early_init and call it at the last step early_init . Regards Shaoyun.liu -Original Message- From: Skvortsov, Victor Sent: Thursday, December 16, 2021 9:28 AM To: Alex Deucher Cc: amd-gfx list ; Deng, Emily ; Liu, Monk ; Ming, Davis ; Liu, Shaoyun ; Zhou, Peng Ju ; Chen, JingWen ; Chen, Horace ; Nieto, David M Subject: RE: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers [AMD Official Use Only] Gotcha, I will skip this patch for drm-next -Original Message- From: Alex Deucher Sent: Thursday, December 16, 2021 8:53 AM To: Skvortsov, Victor Cc: amd-gfx list ; Deng, Emily ; Liu, Monk ; Ming, Davis ; Liu, Shaoyun ; Zhou, Peng Ju ; Chen, JingWen ; Chen, Horace ; Nieto, David M Subject: Re: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers [CAUTION: External Email] On Wed, Dec 15, 2021 at 6:58 PM Skvortsov, Victor wrote: > > [AMD Official Use Only] > > Hey Alex, > > This change was based on the fact that amd-mainline-dkms-5.13 calls > get_xgmi_info() in gmc_v9_0_early_init(). But I can see that drm-next it's > instead called in gmc_v9_0_sw_init(). So, I'm not sure whats the correct > behavior. But I do agree that the change is kind of ugly. I don't know where > else to put it if we do need to call get_xgmi_info() in early_init. > We could skip this patch for drm-next and just apply it to the dkms branch. There's already a lot of ugly stuff in there to deal with multiple kernel versions. Alex > Thanks, > Victor > > -Original Message- > From: Alex Deucher > Sent: Wednesday, December 15, 2021 4:38 PM > To: Skvortsov, Victor > Cc: amd-gfx list ; Deng, Emily > ; Liu, Monk ; Ming, Davis > ; Liu, Shaoyun ; Zhou, Peng > Ju ; Chen, JingWen ; Chen, > Horace ; Nieto, David M > Subject: Re: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function > pointers > > [CAUTION: External Email] > > On Wed, Dec 15, 2021 at 1:56 PM Victor Skvortsov > wrote: > > > > In SRIOV, RLC function pointers must be initialized early as we rely > > on the RLCG interface for all GC register access. > > > > Signed-off-by: Victor Skvortsov > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 ++ > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 +-- > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h | 2 ++ > > 3 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > > index 65e1f6cc59dd..1bc92a38d124 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > > @@ -844,6 +844,8 @@ static int amdgpu_discovery_set_gc_ip_blocks(struct > > amdgpu_device *adev) > > case IP_VERSION(9, 4, 1): > > case IP_VERSION(9, 4, 2): > > amdgpu_device_ip_block_add(adev, > > _v9_0_ip_block); > > + if (amdgpu_sriov_vf(adev) && adev->ip_versions[GC_HWIP][0] > > == IP_VERSION(9, 4, 2)) > > + gfx_v9_0_set_rlc_funcs(adev); > > amdgpu_discovery.c is IP independent. I'd rather not add random IP specific > function calls. gfx_v9_0_set_rlc_funcs() already gets called in > gfx_v9_0_early_init(). Is that not early enough? In general we shouldn't be > touching the hardware much if at all in early_init. > > Alex > > > break; > > case IP_VERSION(10, 1, 10): > > case IP_VERSION(10, 1, 2): > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > index edb3e3b08eed..d252b06efa43 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > +++ b/drivers/gpu/drm/amd/amdg
RE: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers
[AMD Official Use Only] Actually I don't know why the change " a35f147621bc drm/amdgpu: get xgmi info at eary_init " not in drm-next , instead it’s in amd-mainline-dkms-5.13. That change is necessary for passthrough XGMI hive to a VM and rely on our driver to do the reset on whole hive when driver is loaded . I checked the code again, it seems we should be ok as long as we get xgmi info at eary_init. So since gfx_v9_0_set_rlc_funcs() already gets called in gfx_v9_0_early_init(), we can move get xgmi info out of gmc_early_init and call it at the last step early_init . Regards Shaoyun.liu -Original Message- From: Skvortsov, Victor Sent: Thursday, December 16, 2021 9:28 AM To: Alex Deucher Cc: amd-gfx list ; Deng, Emily ; Liu, Monk ; Ming, Davis ; Liu, Shaoyun ; Zhou, Peng Ju ; Chen, JingWen ; Chen, Horace ; Nieto, David M Subject: RE: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers [AMD Official Use Only] Gotcha, I will skip this patch for drm-next -Original Message- From: Alex Deucher Sent: Thursday, December 16, 2021 8:53 AM To: Skvortsov, Victor Cc: amd-gfx list ; Deng, Emily ; Liu, Monk ; Ming, Davis ; Liu, Shaoyun ; Zhou, Peng Ju ; Chen, JingWen ; Chen, Horace ; Nieto, David M Subject: Re: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers [CAUTION: External Email] On Wed, Dec 15, 2021 at 6:58 PM Skvortsov, Victor wrote: > > [AMD Official Use Only] > > Hey Alex, > > This change was based on the fact that amd-mainline-dkms-5.13 calls > get_xgmi_info() in gmc_v9_0_early_init(). But I can see that drm-next it's > instead called in gmc_v9_0_sw_init(). So, I'm not sure whats the correct > behavior. But I do agree that the change is kind of ugly. I don't know where > else to put it if we do need to call get_xgmi_info() in early_init. > We could skip this patch for drm-next and just apply it to the dkms branch. There's already a lot of ugly stuff in there to deal with multiple kernel versions. Alex > Thanks, > Victor > > -Original Message- > From: Alex Deucher > Sent: Wednesday, December 15, 2021 4:38 PM > To: Skvortsov, Victor > Cc: amd-gfx list ; Deng, Emily > ; Liu, Monk ; Ming, Davis > ; Liu, Shaoyun ; Zhou, Peng > Ju ; Chen, JingWen ; Chen, > Horace ; Nieto, David M > Subject: Re: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function > pointers > > [CAUTION: External Email] > > On Wed, Dec 15, 2021 at 1:56 PM Victor Skvortsov > wrote: > > > > In SRIOV, RLC function pointers must be initialized early as we rely > > on the RLCG interface for all GC register access. > > > > Signed-off-by: Victor Skvortsov > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 ++ > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 +-- > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h | 2 ++ > > 3 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > > index 65e1f6cc59dd..1bc92a38d124 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > > @@ -844,6 +844,8 @@ static int amdgpu_discovery_set_gc_ip_blocks(struct > > amdgpu_device *adev) > > case IP_VERSION(9, 4, 1): > > case IP_VERSION(9, 4, 2): > > amdgpu_device_ip_block_add(adev, > > _v9_0_ip_block); > > + if (amdgpu_sriov_vf(adev) && adev->ip_versions[GC_HWIP][0] > > == IP_VERSION(9, 4, 2)) > > + gfx_v9_0_set_rlc_funcs(adev); > > amdgpu_discovery.c is IP independent. I'd rather not add random IP specific > function calls. gfx_v9_0_set_rlc_funcs() already gets called in > gfx_v9_0_early_init(). Is that not early enough? In general we shouldn't be > touching the hardware much if at all in early_init. > > Alex > > > break; > > case IP_VERSION(10, 1, 10): > > case IP_VERSION(10, 1, 2): > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > index edb3e3b08eed..d252b06efa43 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > @@ -816,7 +816,6 @@ static void gfx_v9_0_sriov_wreg(struct > > amdgpu_device *adev, u32 offset, static void > > gfx_v9_0_set_ring_funcs(struct amdgpu_device *adev); static void > > gfx_v9_0_set_irq_funcs(struct amdgpu_device *adev); static void > > gfx_v9_0_set_gds_init(struct amdgpu_device *adev); -static void > > gfx_v9_0_set_rlc_funcs(struct amdgpu_d
RE: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers
[Public] Hi Victor, calling get_xgmi_info() in gmc_v9_0_early_init is from below patch, which also sent out to amd-gfx for review as a upstream patch. Do you know why it's missed from upstream? drm/amdgpu: get xgmi info at eary_init Driver need to get XGMI info function earlier before ip_init since driver need to check the XGMI setting to determine how to perform reset during init Signed-off-by: shaoyunl Acked-by: Alex Deucher Change-Id: Ic37276bbb6640bb4e9360220fed99494cedd3ef5 Regards, Guchun -Original Message- From: amd-gfx On Behalf Of Skvortsov, Victor Sent: Thursday, December 16, 2021 10:28 PM To: Alex Deucher Cc: Ming, Davis ; Chen, JingWen ; amd-gfx list ; Deng, Emily ; Nieto, David M ; Chen, Horace ; Liu, Monk ; Liu, Shaoyun Subject: RE: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers [AMD Official Use Only] Gotcha, I will skip this patch for drm-next -Original Message- From: Alex Deucher Sent: Thursday, December 16, 2021 8:53 AM To: Skvortsov, Victor Cc: amd-gfx list ; Deng, Emily ; Liu, Monk ; Ming, Davis ; Liu, Shaoyun ; Zhou, Peng Ju ; Chen, JingWen ; Chen, Horace ; Nieto, David M Subject: Re: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers [CAUTION: External Email] On Wed, Dec 15, 2021 at 6:58 PM Skvortsov, Victor wrote: > > [AMD Official Use Only] > > Hey Alex, > > This change was based on the fact that amd-mainline-dkms-5.13 calls > get_xgmi_info() in gmc_v9_0_early_init(). But I can see that drm-next it's > instead called in gmc_v9_0_sw_init(). So, I'm not sure whats the correct > behavior. But I do agree that the change is kind of ugly. I don't know where > else to put it if we do need to call get_xgmi_info() in early_init. > We could skip this patch for drm-next and just apply it to the dkms branch. There's already a lot of ugly stuff in there to deal with multiple kernel versions. Alex > Thanks, > Victor > > -Original Message- > From: Alex Deucher > Sent: Wednesday, December 15, 2021 4:38 PM > To: Skvortsov, Victor > Cc: amd-gfx list ; Deng, Emily > ; Liu, Monk ; Ming, Davis > ; Liu, Shaoyun ; Zhou, Peng > Ju ; Chen, JingWen ; Chen, > Horace ; Nieto, David M > Subject: Re: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function > pointers > > [CAUTION: External Email] > > On Wed, Dec 15, 2021 at 1:56 PM Victor Skvortsov > wrote: > > > > In SRIOV, RLC function pointers must be initialized early as we rely > > on the RLCG interface for all GC register access. > > > > Signed-off-by: Victor Skvortsov > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 ++ > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 +-- > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h | 2 ++ > > 3 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > > index 65e1f6cc59dd..1bc92a38d124 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > > @@ -844,6 +844,8 @@ static int amdgpu_discovery_set_gc_ip_blocks(struct > > amdgpu_device *adev) > > case IP_VERSION(9, 4, 1): > > case IP_VERSION(9, 4, 2): > > amdgpu_device_ip_block_add(adev, > > _v9_0_ip_block); > > + if (amdgpu_sriov_vf(adev) && adev->ip_versions[GC_HWIP][0] > > == IP_VERSION(9, 4, 2)) > > + gfx_v9_0_set_rlc_funcs(adev); > > amdgpu_discovery.c is IP independent. I'd rather not add random IP specific > function calls. gfx_v9_0_set_rlc_funcs() already gets called in > gfx_v9_0_early_init(). Is that not early enough? In general we shouldn't be > touching the hardware much if at all in early_init. > > Alex > > > break; > > case IP_VERSION(10, 1, 10): > > case IP_VERSION(10, 1, 2): > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > index edb3e3b08eed..d252b06efa43 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > @@ -816,7 +816,6 @@ static void gfx_v9_0_sriov_wreg(struct > > amdgpu_device *adev, u32 offset, static void > > gfx_v9_0_set_ring_funcs(struct amdgpu_device *adev); static void > > gfx_v9_0_set_irq_funcs(struct amdgpu_device *adev); static void > > gfx_v9_0_set_gds_init(struct amdgpu_device *adev); -static void > > gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev); static int > > gfx_v9_0_
RE: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers
[AMD Official Use Only]
Gotcha, I will skip this patch for drm-next
-Original Message-
From: Alex Deucher
Sent: Thursday, December 16, 2021 8:53 AM
To: Skvortsov, Victor
Cc: amd-gfx list ; Deng, Emily
; Liu, Monk ; Ming, Davis
; Liu, Shaoyun ; Zhou, Peng Ju
; Chen, JingWen ; Chen, Horace
; Nieto, David M
Subject: Re: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers
[CAUTION: External Email]
On Wed, Dec 15, 2021 at 6:58 PM Skvortsov, Victor
wrote:
>
> [AMD Official Use Only]
>
> Hey Alex,
>
> This change was based on the fact that amd-mainline-dkms-5.13 calls
> get_xgmi_info() in gmc_v9_0_early_init(). But I can see that drm-next it's
> instead called in gmc_v9_0_sw_init(). So, I'm not sure whats the correct
> behavior. But I do agree that the change is kind of ugly. I don't know where
> else to put it if we do need to call get_xgmi_info() in early_init.
>
We could skip this patch for drm-next and just apply it to the dkms branch.
There's already a lot of ugly stuff in there to deal with multiple kernel
versions.
Alex
> Thanks,
> Victor
>
> -Original Message-
> From: Alex Deucher
> Sent: Wednesday, December 15, 2021 4:38 PM
> To: Skvortsov, Victor
> Cc: amd-gfx list ; Deng, Emily
> ; Liu, Monk ; Ming, Davis
> ; Liu, Shaoyun ; Zhou, Peng
> Ju ; Chen, JingWen ; Chen,
> Horace ; Nieto, David M
> Subject: Re: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function
> pointers
>
> [CAUTION: External Email]
>
> On Wed, Dec 15, 2021 at 1:56 PM Victor Skvortsov
> wrote:
> >
> > In SRIOV, RLC function pointers must be initialized early as we rely
> > on the RLCG interface for all GC register access.
> >
> > Signed-off-by: Victor Skvortsov
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 ++
> > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 +--
> > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h | 2 ++
> > 3 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > index 65e1f6cc59dd..1bc92a38d124 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > @@ -844,6 +844,8 @@ static int amdgpu_discovery_set_gc_ip_blocks(struct
> > amdgpu_device *adev)
> > case IP_VERSION(9, 4, 1):
> > case IP_VERSION(9, 4, 2):
> > amdgpu_device_ip_block_add(adev,
> > _v9_0_ip_block);
> > + if (amdgpu_sriov_vf(adev) && adev->ip_versions[GC_HWIP][0]
> > == IP_VERSION(9, 4, 2))
> > + gfx_v9_0_set_rlc_funcs(adev);
>
> amdgpu_discovery.c is IP independent. I'd rather not add random IP specific
> function calls. gfx_v9_0_set_rlc_funcs() already gets called in
> gfx_v9_0_early_init(). Is that not early enough? In general we shouldn't be
> touching the hardware much if at all in early_init.
>
> Alex
>
> > break;
> > case IP_VERSION(10, 1, 10):
> > case IP_VERSION(10, 1, 2):
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > index edb3e3b08eed..d252b06efa43 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > @@ -816,7 +816,6 @@ static void gfx_v9_0_sriov_wreg(struct
> > amdgpu_device *adev, u32 offset, static void
> > gfx_v9_0_set_ring_funcs(struct amdgpu_device *adev); static void
> > gfx_v9_0_set_irq_funcs(struct amdgpu_device *adev); static void
> > gfx_v9_0_set_gds_init(struct amdgpu_device *adev); -static void
> > gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev); static int
> > gfx_v9_0_get_cu_info(struct amdgpu_device *adev,
> > struct amdgpu_cu_info *cu_info);
> > static uint64_t gfx_v9_0_get_gpu_clock_counter(struct amdgpu_device
> > *adev); @@ -7066,7 +7065,7 @@ static void gfx_v9_0_set_irq_funcs(struct
> > amdgpu_device *adev)
> > adev->gfx.cp_ecc_error_irq.funcs =
> > _v9_0_cp_ecc_error_irq_funcs; }
> >
> > -static void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev)
> > +void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev)
> > {
> > switch (adev->ip_versions[GC_HWIP][0]) {
> > case IP_VERSION(9, 0, 1):
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> > index dfe8d4841f58..1817e252354f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> > @@ -29,4 +29,6 @@ extern const struct amdgpu_ip_block_version
> > gfx_v9_0_ip_block; void gfx_v9_0_select_se_sh(struct amdgpu_device *adev,
> > u32 se_num, u32 sh_num,
> >u32 instance);
> >
> > +void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev);
> > +
> > #endif
> > --
> > 2.25.1
> >
Re: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers
On Wed, Dec 15, 2021 at 6:58 PM Skvortsov, Victor
wrote:
>
> [AMD Official Use Only]
>
> Hey Alex,
>
> This change was based on the fact that amd-mainline-dkms-5.13 calls
> get_xgmi_info() in gmc_v9_0_early_init(). But I can see that drm-next it's
> instead called in gmc_v9_0_sw_init(). So, I'm not sure whats the correct
> behavior. But I do agree that the change is kind of ugly. I don't know where
> else to put it if we do need to call get_xgmi_info() in early_init.
>
We could skip this patch for drm-next and just apply it to the dkms
branch. There's already a lot of ugly stuff in there to deal with
multiple kernel versions.
Alex
> Thanks,
> Victor
>
> -Original Message-
> From: Alex Deucher
> Sent: Wednesday, December 15, 2021 4:38 PM
> To: Skvortsov, Victor
> Cc: amd-gfx list ; Deng, Emily
> ; Liu, Monk ; Ming, Davis
> ; Liu, Shaoyun ; Zhou, Peng Ju
> ; Chen, JingWen ; Chen, Horace
> ; Nieto, David M
> Subject: Re: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function
> pointers
>
> [CAUTION: External Email]
>
> On Wed, Dec 15, 2021 at 1:56 PM Victor Skvortsov
> wrote:
> >
> > In SRIOV, RLC function pointers must be initialized early as we rely
> > on the RLCG interface for all GC register access.
> >
> > Signed-off-by: Victor Skvortsov
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 ++
> > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 +--
> > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h | 2 ++
> > 3 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > index 65e1f6cc59dd..1bc92a38d124 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > @@ -844,6 +844,8 @@ static int amdgpu_discovery_set_gc_ip_blocks(struct
> > amdgpu_device *adev)
> > case IP_VERSION(9, 4, 1):
> > case IP_VERSION(9, 4, 2):
> > amdgpu_device_ip_block_add(adev, _v9_0_ip_block);
> > + if (amdgpu_sriov_vf(adev) && adev->ip_versions[GC_HWIP][0]
> > == IP_VERSION(9, 4, 2))
> > + gfx_v9_0_set_rlc_funcs(adev);
>
> amdgpu_discovery.c is IP independent. I'd rather not add random IP specific
> function calls. gfx_v9_0_set_rlc_funcs() already gets called in
> gfx_v9_0_early_init(). Is that not early enough? In general we shouldn't be
> touching the hardware much if at all in early_init.
>
> Alex
>
> > break;
> > case IP_VERSION(10, 1, 10):
> > case IP_VERSION(10, 1, 2):
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > index edb3e3b08eed..d252b06efa43 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > @@ -816,7 +816,6 @@ static void gfx_v9_0_sriov_wreg(struct
> > amdgpu_device *adev, u32 offset, static void
> > gfx_v9_0_set_ring_funcs(struct amdgpu_device *adev); static void
> > gfx_v9_0_set_irq_funcs(struct amdgpu_device *adev); static void
> > gfx_v9_0_set_gds_init(struct amdgpu_device *adev); -static void
> > gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev); static int
> > gfx_v9_0_get_cu_info(struct amdgpu_device *adev,
> > struct amdgpu_cu_info *cu_info);
> > static uint64_t gfx_v9_0_get_gpu_clock_counter(struct amdgpu_device
> > *adev); @@ -7066,7 +7065,7 @@ static void gfx_v9_0_set_irq_funcs(struct
> > amdgpu_device *adev)
> > adev->gfx.cp_ecc_error_irq.funcs =
> > _v9_0_cp_ecc_error_irq_funcs; }
> >
> > -static void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev)
> > +void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev)
> > {
> > switch (adev->ip_versions[GC_HWIP][0]) {
> > case IP_VERSION(9, 0, 1):
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> > index dfe8d4841f58..1817e252354f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> > @@ -29,4 +29,6 @@ extern const struct amdgpu_ip_block_version
> > gfx_v9_0_ip_block; void gfx_v9_0_select_se_sh(struct amdgpu_device *adev,
> > u32 se_num, u32 sh_num,
> >u32 instance);
> >
> > +void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev);
> > +
> > #endif
> > --
> > 2.25.1
> >
RE: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers
[AMD Official Use Only]
Hey Alex,
This change was based on the fact that amd-mainline-dkms-5.13 calls
get_xgmi_info() in gmc_v9_0_early_init(). But I can see that drm-next it's
instead called in gmc_v9_0_sw_init(). So, I'm not sure whats the correct
behavior. But I do agree that the change is kind of ugly. I don't know where
else to put it if we do need to call get_xgmi_info() in early_init.
Thanks,
Victor
-Original Message-
From: Alex Deucher
Sent: Wednesday, December 15, 2021 4:38 PM
To: Skvortsov, Victor
Cc: amd-gfx list ; Deng, Emily
; Liu, Monk ; Ming, Davis
; Liu, Shaoyun ; Zhou, Peng Ju
; Chen, JingWen ; Chen, Horace
; Nieto, David M
Subject: Re: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers
[CAUTION: External Email]
On Wed, Dec 15, 2021 at 1:56 PM Victor Skvortsov
wrote:
>
> In SRIOV, RLC function pointers must be initialized early as we rely
> on the RLCG interface for all GC register access.
>
> Signed-off-by: Victor Skvortsov
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 ++
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 +--
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h | 2 ++
> 3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index 65e1f6cc59dd..1bc92a38d124 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -844,6 +844,8 @@ static int amdgpu_discovery_set_gc_ip_blocks(struct
> amdgpu_device *adev)
> case IP_VERSION(9, 4, 1):
> case IP_VERSION(9, 4, 2):
> amdgpu_device_ip_block_add(adev, _v9_0_ip_block);
> + if (amdgpu_sriov_vf(adev) && adev->ip_versions[GC_HWIP][0] ==
> IP_VERSION(9, 4, 2))
> + gfx_v9_0_set_rlc_funcs(adev);
amdgpu_discovery.c is IP independent. I'd rather not add random IP specific
function calls. gfx_v9_0_set_rlc_funcs() already gets called in
gfx_v9_0_early_init(). Is that not early enough? In general we shouldn't be
touching the hardware much if at all in early_init.
Alex
> break;
> case IP_VERSION(10, 1, 10):
> case IP_VERSION(10, 1, 2):
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index edb3e3b08eed..d252b06efa43 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -816,7 +816,6 @@ static void gfx_v9_0_sriov_wreg(struct
> amdgpu_device *adev, u32 offset, static void
> gfx_v9_0_set_ring_funcs(struct amdgpu_device *adev); static void
> gfx_v9_0_set_irq_funcs(struct amdgpu_device *adev); static void
> gfx_v9_0_set_gds_init(struct amdgpu_device *adev); -static void
> gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev); static int
> gfx_v9_0_get_cu_info(struct amdgpu_device *adev,
> struct amdgpu_cu_info *cu_info);
> static uint64_t gfx_v9_0_get_gpu_clock_counter(struct amdgpu_device
> *adev); @@ -7066,7 +7065,7 @@ static void gfx_v9_0_set_irq_funcs(struct
> amdgpu_device *adev)
> adev->gfx.cp_ecc_error_irq.funcs =
> _v9_0_cp_ecc_error_irq_funcs; }
>
> -static void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev)
> +void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev)
> {
> switch (adev->ip_versions[GC_HWIP][0]) {
> case IP_VERSION(9, 0, 1):
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> index dfe8d4841f58..1817e252354f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> @@ -29,4 +29,6 @@ extern const struct amdgpu_ip_block_version
> gfx_v9_0_ip_block; void gfx_v9_0_select_se_sh(struct amdgpu_device *adev,
> u32 se_num, u32 sh_num,
>u32 instance);
>
> +void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev);
> +
> #endif
> --
> 2.25.1
>
Re: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers
On Wed, Dec 15, 2021 at 1:56 PM Victor Skvortsov
wrote:
>
> In SRIOV, RLC function pointers must be initialized early as
> we rely on the RLCG interface for all GC register access.
>
> Signed-off-by: Victor Skvortsov
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 ++
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 +--
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h | 2 ++
> 3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index 65e1f6cc59dd..1bc92a38d124 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -844,6 +844,8 @@ static int amdgpu_discovery_set_gc_ip_blocks(struct
> amdgpu_device *adev)
> case IP_VERSION(9, 4, 1):
> case IP_VERSION(9, 4, 2):
> amdgpu_device_ip_block_add(adev, _v9_0_ip_block);
> + if (amdgpu_sriov_vf(adev) && adev->ip_versions[GC_HWIP][0] ==
> IP_VERSION(9, 4, 2))
> + gfx_v9_0_set_rlc_funcs(adev);
amdgpu_discovery.c is IP independent. I'd rather not add random IP
specific function calls. gfx_v9_0_set_rlc_funcs() already gets called
in gfx_v9_0_early_init(). Is that not early enough? In general we
shouldn't be touching the hardware much if at all in early_init.
Alex
> break;
> case IP_VERSION(10, 1, 10):
> case IP_VERSION(10, 1, 2):
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index edb3e3b08eed..d252b06efa43 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -816,7 +816,6 @@ static void gfx_v9_0_sriov_wreg(struct amdgpu_device
> *adev, u32 offset,
> static void gfx_v9_0_set_ring_funcs(struct amdgpu_device *adev);
> static void gfx_v9_0_set_irq_funcs(struct amdgpu_device *adev);
> static void gfx_v9_0_set_gds_init(struct amdgpu_device *adev);
> -static void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev);
> static int gfx_v9_0_get_cu_info(struct amdgpu_device *adev,
> struct amdgpu_cu_info *cu_info);
> static uint64_t gfx_v9_0_get_gpu_clock_counter(struct amdgpu_device *adev);
> @@ -7066,7 +7065,7 @@ static void gfx_v9_0_set_irq_funcs(struct amdgpu_device
> *adev)
> adev->gfx.cp_ecc_error_irq.funcs = _v9_0_cp_ecc_error_irq_funcs;
> }
>
> -static void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev)
> +void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev)
> {
> switch (adev->ip_versions[GC_HWIP][0]) {
> case IP_VERSION(9, 0, 1):
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> index dfe8d4841f58..1817e252354f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
> @@ -29,4 +29,6 @@ extern const struct amdgpu_ip_block_version
> gfx_v9_0_ip_block;
> void gfx_v9_0_select_se_sh(struct amdgpu_device *adev, u32 se_num, u32
> sh_num,
>u32 instance);
>
> +void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev);
> +
> #endif
> --
> 2.25.1
>
Re: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers
[AMD Official Use Only]
case IP_VERSION(9, 4, 1):
case IP_VERSION(9, 4, 2):
amdgpu_device_ip_block_add(adev, _v9_0_ip_block);
+ if (amdgpu_sriov_vf(adev) && adev->ip_versions[GC_HWIP][0] ==
IP_VERSION(9, 4, 2))
+ gfx_v9_0_set_rlc_funcs(adev);
break;
case IP_VERSION(10, 1, 10):
I think for the above, it would be more clear just to separate them:
case IP_VERSION(9, 4, 1):
amdgpu_device_ip_block_add(adev, _v9_0_ip_block);
break;
case IP_VERSION(9, 4, 2):
amdgpu_device_ip_block_add(adev, _v9_0_ip_block);
if (amdgpu_sriov_vf(adev))
gfx_v9_0_set_rlc_funcs(adev);
break;
From: Skvortsov, Victor
Sent: Wednesday, December 15, 2021 10:55 AM
To: amd-gfx@lists.freedesktop.org ; Deng, Emily
; Liu, Monk ; Ming, Davis
; Liu, Shaoyun ; Zhou, Peng Ju
; Chen, JingWen ; Chen, Horace
; Nieto, David M
Cc: Skvortsov, Victor
Subject: [PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers
In SRIOV, RLC function pointers must be initialized early as
we rely on the RLCG interface for all GC register access.
Signed-off-by: Victor Skvortsov
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 ++
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 +--
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h | 2 ++
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index 65e1f6cc59dd..1bc92a38d124 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -844,6 +844,8 @@ static int amdgpu_discovery_set_gc_ip_blocks(struct
amdgpu_device *adev)
case IP_VERSION(9, 4, 1):
case IP_VERSION(9, 4, 2):
amdgpu_device_ip_block_add(adev, _v9_0_ip_block);
+ if (amdgpu_sriov_vf(adev) && adev->ip_versions[GC_HWIP][0] ==
IP_VERSION(9, 4, 2))
+ gfx_v9_0_set_rlc_funcs(adev);
break;
case IP_VERSION(10, 1, 10):
case IP_VERSION(10, 1, 2):
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index edb3e3b08eed..d252b06efa43 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -816,7 +816,6 @@ static void gfx_v9_0_sriov_wreg(struct amdgpu_device *adev,
u32 offset,
static void gfx_v9_0_set_ring_funcs(struct amdgpu_device *adev);
static void gfx_v9_0_set_irq_funcs(struct amdgpu_device *adev);
static void gfx_v9_0_set_gds_init(struct amdgpu_device *adev);
-static void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev);
static int gfx_v9_0_get_cu_info(struct amdgpu_device *adev,
struct amdgpu_cu_info *cu_info);
static uint64_t gfx_v9_0_get_gpu_clock_counter(struct amdgpu_device *adev);
@@ -7066,7 +7065,7 @@ static void gfx_v9_0_set_irq_funcs(struct amdgpu_device
*adev)
adev->gfx.cp_ecc_error_irq.funcs = _v9_0_cp_ecc_error_irq_funcs;
}
-static void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev)
+void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev)
{
switch (adev->ip_versions[GC_HWIP][0]) {
case IP_VERSION(9, 0, 1):
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
index dfe8d4841f58..1817e252354f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
@@ -29,4 +29,6 @@ extern const struct amdgpu_ip_block_version gfx_v9_0_ip_block;
void gfx_v9_0_select_se_sh(struct amdgpu_device *adev, u32 se_num, u32 sh_num,
u32 instance);
+void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev);
+
#endif
--
2.25.1
[PATCH 4/5] drm/amdgpu: Initialize Aldebaran RLC function pointers
In SRIOV, RLC function pointers must be initialized early as
we rely on the RLCG interface for all GC register access.
Signed-off-by: Victor Skvortsov
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 ++
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 +--
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h | 2 ++
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index 65e1f6cc59dd..1bc92a38d124 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -844,6 +844,8 @@ static int amdgpu_discovery_set_gc_ip_blocks(struct
amdgpu_device *adev)
case IP_VERSION(9, 4, 1):
case IP_VERSION(9, 4, 2):
amdgpu_device_ip_block_add(adev, _v9_0_ip_block);
+ if (amdgpu_sriov_vf(adev) && adev->ip_versions[GC_HWIP][0] ==
IP_VERSION(9, 4, 2))
+ gfx_v9_0_set_rlc_funcs(adev);
break;
case IP_VERSION(10, 1, 10):
case IP_VERSION(10, 1, 2):
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index edb3e3b08eed..d252b06efa43 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -816,7 +816,6 @@ static void gfx_v9_0_sriov_wreg(struct amdgpu_device *adev,
u32 offset,
static void gfx_v9_0_set_ring_funcs(struct amdgpu_device *adev);
static void gfx_v9_0_set_irq_funcs(struct amdgpu_device *adev);
static void gfx_v9_0_set_gds_init(struct amdgpu_device *adev);
-static void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev);
static int gfx_v9_0_get_cu_info(struct amdgpu_device *adev,
struct amdgpu_cu_info *cu_info);
static uint64_t gfx_v9_0_get_gpu_clock_counter(struct amdgpu_device *adev);
@@ -7066,7 +7065,7 @@ static void gfx_v9_0_set_irq_funcs(struct amdgpu_device
*adev)
adev->gfx.cp_ecc_error_irq.funcs = _v9_0_cp_ecc_error_irq_funcs;
}
-static void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev)
+void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev)
{
switch (adev->ip_versions[GC_HWIP][0]) {
case IP_VERSION(9, 0, 1):
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
index dfe8d4841f58..1817e252354f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.h
@@ -29,4 +29,6 @@ extern const struct amdgpu_ip_block_version gfx_v9_0_ip_block;
void gfx_v9_0_select_se_sh(struct amdgpu_device *adev, u32 se_num, u32 sh_num,
u32 instance);
+void gfx_v9_0_set_rlc_funcs(struct amdgpu_device *adev);
+
#endif
--
2.25.1
