On Mon, Apr 14, 2025 at 3:38 PM Rodrigo Siqueira <[email protected]> wrote: > > On 04/13, Alex Deucher wrote: > > On Sat, Apr 12, 2025 at 4:22 PM Rodrigo Siqueira <[email protected]> > > 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 <[email protected]> > > > --- > > > 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?
RB = Render Backends. These are the render backends (color and depth buffer handling) on the 3D engine (also called ROPs by a lot of people). Bad RBs are fused off and there is a harvest register the driver reads to determine which RB(s) are fused off so that the driver can configure the hardware state so that nothing gets sent to them. There are also user harvest registers that the driver can program to disable additional RBs, etc. for testing purposes. > 2. Why there is raster_config and raster_config_1? Is raster_config_1 > some sort of fallback? The configuration just happens to require two registers to hold all of the state. Alex > > Thanks > > > Alex > > > > > case CHIP_HAWAII: > > > buffer[count++] = cpu_to_le32(0x3a00161a); > > > buffer[count++] = cpu_to_le32(0x0000002e); > > > -- > > > 2.49.0 > > > > > -- > Rodrigo Siqueira
