2022-04-06 13:00:07

by Grigory Vasilyev

Subject: [PATCH] drm/amdgpu: Unnecessary code in gfx_v7_0.c

The code is useless and doesn't change the value.

(0 << GB_ADDR_CONFIG__ROW_SIZE__SHIFT) = 0
gb_addr_config | 0 = gb_addr_config

Perhaps there could be 1 instead of 0, but this does not correspond with
the logic of the switch.

Signed-off-by: Grigory Vasilyev <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 1cb5db17d2b9..5ed84a6467ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -4409,16 +4409,14 @@ static void gfx_v7_0_gpu_early_init(struct amdgpu_device *adev)
/* fix up row size */
gb_addr_config &= ~GB_ADDR_CONFIG__ROW_SIZE_MASK;
switch (adev->gfx.config.mem_row_size_in_kb) {
- case 1:
- default:
- gb_addr_config |= (0 << GB_ADDR_CONFIG__ROW_SIZE__SHIFT);
- break;
case 2:
gb_addr_config |= (1 << GB_ADDR_CONFIG__ROW_SIZE__SHIFT);
break;
case 4:
gb_addr_config |= (2 << GB_ADDR_CONFIG__ROW_SIZE__SHIFT);
break;
+ default:
+ break;
}
adev->gfx.config.gb_addr_config = gb_addr_config;
}
--
2.35.1

2022-04-06 16:43:34

by Alex Deucher

Subject: Re: [PATCH] drm/amdgpu: Unnecessary code in gfx_v7_0.c

On Wed, Apr 6, 2022 at 4:00 AM Grigory Vasilyev <[email protected]> wrote:
>
> The code is useless and doesn't change the value.
>
> (0 << GB_ADDR_CONFIG__ROW_SIZE__SHIFT) = 0
> gb_addr_config | 0 = gb_addr_config
>
> Perhaps there could be 1 instead of 0, but this does not correspond with
> the logic of the switch.

It doesn't do anything, but it helps the developer to understand how
the driver state maps to hardware state.

Alex

>
> Signed-off-by: Grigory Vasilyev <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index 1cb5db17d2b9..5ed84a6467ee 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -4409,16 +4409,14 @@ static void gfx_v7_0_gpu_early_init(struct amdgpu_device *adev)
> /* fix up row size */
> gb_addr_config &= ~GB_ADDR_CONFIG__ROW_SIZE_MASK;
> switch (adev->gfx.config.mem_row_size_in_kb) {
> - case 1:
> - default:
> - gb_addr_config |= (0 << GB_ADDR_CONFIG__ROW_SIZE__SHIFT);
> - break;
> case 2:
> gb_addr_config |= (1 << GB_ADDR_CONFIG__ROW_SIZE__SHIFT);
> break;
> case 4:
> gb_addr_config |= (2 << GB_ADDR_CONFIG__ROW_SIZE__SHIFT);
> break;
> + default:
> + break;
> }
> adev->gfx.config.gb_addr_config = gb_addr_config;
> }
> --
> 2.35.1
>

2022-04-06 16:55:12

by Grigory Vasilyev

Subject: Re: [PATCH] drm/amdgpu: Unnecessary code in gfx_v7_0.c

Alex Deucher, Thanks, that's what I thought too. But this code is
uninformative, and without it is clear what this code is doing. If you
need to provide additional information on how the code works, then for
me it's better to leave a comment.
This is actually more like a bug than an explanation of what the code is doing.

ср, 6 апр. 2022 г. в 16:20, Alex Deucher <[email protected]>:
>
> On Wed, Apr 6, 2022 at 4:00 AM Grigory Vasilyev <[email protected]> wrote:
> >
> > The code is useless and doesn't change the value.
> >
> > (0 << GB_ADDR_CONFIG__ROW_SIZE__SHIFT) = 0
> > gb_addr_config | 0 = gb_addr_config
> >
> > Perhaps there could be 1 instead of 0, but this does not correspond with
> > the logic of the switch.
>
> It doesn't do anything, but it helps the developer to understand how
> the driver state maps to hardware state.
>
> Alex
>
> >
> > Signed-off-by: Grigory Vasilyev <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > index 1cb5db17d2b9..5ed84a6467ee 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > @@ -4409,16 +4409,14 @@ static void gfx_v7_0_gpu_early_init(struct amdgpu_device *adev)
> > /* fix up row size */
> > gb_addr_config &= ~GB_ADDR_CONFIG__ROW_SIZE_MASK;
> > switch (adev->gfx.config.mem_row_size_in_kb) {
> > - case 1:
> > - default:
> > - gb_addr_config |= (0 << GB_ADDR_CONFIG__ROW_SIZE__SHIFT);
> > - break;
> > case 2:
> > gb_addr_config |= (1 << GB_ADDR_CONFIG__ROW_SIZE__SHIFT);
> > break;
> > case 4:
> > gb_addr_config |= (2 << GB_ADDR_CONFIG__ROW_SIZE__SHIFT);
> > break;
> > + default:
> > + break;
> > }
> > adev->gfx.config.gb_addr_config = gb_addr_config;
> > }
> > --
> > 2.35.1
> >