On 04/13, Alex Deucher wrote: > On Sat, Apr 12, 2025 at 4:22 PM Rodrigo Siqueira <sique...@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 <sique...@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
