RE: [PATCH] drm/amd/amdgpu: Remove static from variable in RLCG Reg RW.
[AMD Official Use Only] Dear Paul, Thanks for you comments. Let me do it. Thanks, Gavin -Original Message- From: Paul Menzel Sent: Thursday, April 14, 2022 1:59 PM To: Wan, Gavin Cc: amd-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/amd/amdgpu: Remove static from variable in RLCG Reg RW. Dear Gavin, Thank you for your patch. Am 13.04.22 um 17:26 schrieb Gavin Wan: Should you re-roll your patch (v2), please remove the dot/period from the end of the git commit message summary (subject). > [why] These static variables saves the RLC Scratch registers address. s/saves/save/ >When we installed multiple GPUs (for example: XGMI setting) and s/installed/install/ >multiple GPUs call the function at same time. The RLC Scratch … same time, the RLC … >registers address are changed each other. Then it caused s/caused/causes/ >reading/writing to wrong GPU. I see from other patches posted here, that [why] is put on a separate line, so you do not need to indent the text. [why] These static … > > [fix] Removed the static from the variables. The variables are >in stack. Same here, though *how* instead of *fix* seems more common. s/Removed/Remove/ s/in stack/on the stack/ > > Signed-off-by: Gavin Wan > Change-Id: Iee78849291d4f7a9688ecc5165bec70ee85cdfbe Without the Gerrit URL that line is useless. Kind regards. Paul > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > index d5eea031c3e3..d18a05a20566 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > @@ -868,11 +868,11 @@ static u32 amdgpu_virt_rlcg_reg_rw(struct amdgpu_device > *adev, u32 offset, u32 v > uint32_t timeout = 5; > uint32_t i, tmp; > uint32_t ret = 0; > - static void *scratch_reg0; > - static void *scratch_reg1; > - static void *scratch_reg2; > - static void *scratch_reg3; > - static void *spare_int; > + void *scratch_reg0; > + void *scratch_reg1; > + void *scratch_reg2; > + void *scratch_reg3; > + void *spare_int; > > if (!adev->gfx.rlc.rlcg_reg_access_supported) { > dev_err(adev->dev,
Re: [PATCH] drm/amd/amdgpu: Remove static from variable in RLCG Reg RW.
Dear Gavin, Thank you for your patch. Am 13.04.22 um 17:26 schrieb Gavin Wan: Should you re-roll your patch (v2), please remove the dot/period from the end of the git commit message summary (subject). [why] These static variables saves the RLC Scratch registers address. s/saves/save/ When we installed multiple GPUs (for example: XGMI setting) and s/installed/install/ multiple GPUs call the function at same time. The RLC Scratch … same time, the RLC … registers address are changed each other. Then it caused s/caused/causes/ reading/writing to wrong GPU. I see from other patches posted here, that [why] is put on a separate line, so you do not need to indent the text. [why] These static … [fix] Removed the static from the variables. The variables are in stack. Same here, though *how* instead of *fix* seems more common. s/Removed/Remove/ s/in stack/on the stack/ Signed-off-by: Gavin Wan Change-Id: Iee78849291d4f7a9688ecc5165bec70ee85cdfbe Without the Gerrit URL that line is useless. Kind regards. Paul --- drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c index d5eea031c3e3..d18a05a20566 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c @@ -868,11 +868,11 @@ static u32 amdgpu_virt_rlcg_reg_rw(struct amdgpu_device *adev, u32 offset, u32 v uint32_t timeout = 5; uint32_t i, tmp; uint32_t ret = 0; - static void *scratch_reg0; - static void *scratch_reg1; - static void *scratch_reg2; - static void *scratch_reg3; - static void *spare_int; + void *scratch_reg0; + void *scratch_reg1; + void *scratch_reg2; + void *scratch_reg3; + void *spare_int; if (!adev->gfx.rlc.rlcg_reg_access_supported) { dev_err(adev->dev,
Re: [PATCH] drm/amd/amdgpu: Remove static from variable in RLCG Reg RW.
On Wed, Apr 13, 2022 at 11:27 AM Gavin Wan wrote: > > [why] These static variables saves the RLC Scratch registers address. > When we installed multiple GPUs (for example: XGMI setting) and > multiple GPUs call the function at same time. The RLC Scratch > registers address are changed each other. Then it caused > reading/writing to wrong GPU. > > [fix] Removed the static from the variables. The variables are > in stack. Please add: Fixes: 5d447e29670148 ("drm/amdgpu: add helper for rlcg indirect reg access") With that added. Reviewed-by: Alex Deucher > > Signed-off-by: Gavin Wan > Change-Id: Iee78849291d4f7a9688ecc5165bec70ee85cdfbe > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > index d5eea031c3e3..d18a05a20566 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > @@ -868,11 +868,11 @@ static u32 amdgpu_virt_rlcg_reg_rw(struct amdgpu_device > *adev, u32 offset, u32 v > uint32_t timeout = 5; > uint32_t i, tmp; > uint32_t ret = 0; > - static void *scratch_reg0; > - static void *scratch_reg1; > - static void *scratch_reg2; > - static void *scratch_reg3; > - static void *spare_int; > + void *scratch_reg0; > + void *scratch_reg1; > + void *scratch_reg2; > + void *scratch_reg3; > + void *spare_int; > > if (!adev->gfx.rlc.rlcg_reg_access_supported) { > dev_err(adev->dev, > -- > 2.32.0 >