[PATCH v2 7/9] drm/amdgpu/gfx: Clean up gfx_v7_0_get_csb_buffer

[Rodrigo Siqueira]

On 04/13, Alex Deucher wrote:
> On Sat, Apr 12, 2025 at 4:22 PM Rodrigo Siqueira <siqueira at igalia.com> wrote:
> >
> > CHIP_KAVERI, CHIP_KABINI, and CHIP_MULLINS have the same buffer
> > manipulation as the default option in the switch case. Remove those
> > specific manipulations and rely on the default behavior for them.
> >
> > Signed-off-by: Rodrigo Siqueira <siqueira at igalia.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 9 ---------
> >  1 file changed, 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > index 16b94ff5a959..4d8d68b737d1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > @@ -3902,15 +3902,6 @@ static void gfx_v7_0_get_csb_buffer(struct amdgpu_device *adev,
> >                 buffer[count++] = cpu_to_le32(0x16000012);
> >                 buffer[count++] = cpu_to_le32(0x00000000);
> >                 break;
> > -       case CHIP_KAVERI:
> > -               buffer[count++] = cpu_to_le32(0x00000000); /* XXX */
> > -               buffer[count++] = cpu_to_le32(0x00000000);
> > -               break;
> > -       case CHIP_KABINI:
> > -       case CHIP_MULLINS:
> > -               buffer[count++] = cpu_to_le32(0x00000000); /* XXX */
> > -               buffer[count++] = cpu_to_le32(0x00000000);
> > -               break;
> 
> These should be fixed rather than removed.  Actually, all of these
> should be fixed up for all chips.  We should use the values calculated
> by the driver similar to what we do in gfx_v8.  E.g.,
>         buffer[count++] =
> cpu_to_le32(adev->gfx.config.rb_config[0][0].raster_config);
>         buffer[count++] =
> cpu_to_le32(adev->gfx.config.rb_config[0][0].raster_config_1);
> 

ok, I checked the code sequence, and fwiu gfx_v7_0_setup_rb() ->
gfx_v7_0_raster_config() is called before gfx_v7_0_get_csb_buffer().
 gfx_v7_0_raster_config() has the setup for all of the ASICs in this
part. I also have a HAWAII and confirmed that using raster_config
directly had the same value as the one hardcoded in this part. Anyway,
I'll send a v3 with this fix.

When I was looking into the code, it was not clear to me these things:

1. What is this config RB? Why is there a field for User RB? Also, what
   RB stands for?
2. Why there is raster_config and raster_config_1? Is raster_config_1
   some sort of fallback?

Thanks

> Alex
> 
> >         case CHIP_HAWAII:
> >                 buffer[count++] = cpu_to_le32(0x3a00161a);
> >                 buffer[count++] = cpu_to_le32(0x0000002e);
> > --
> > 2.49.0
> >

-- 
Rodrigo Siqueira