|
[Public]
This bug previously existed, and we have a solution in place for it.
The solution we picked was to force a stall through reading back the memory. You'll see this implemented in dmub_srv.c and the dmub_cmd.h header - through use of a volatile read over the region written. We do this for both the initial allocation for the cache
windows and on every command submission to ensure DMCUB doesn't wakeup before the writes are in VRAM.
The issue on dGPU is the latency through the HDP path, but on APU the issue is out of order writes. We saw this problem on both DCN30/DCN21 when DMCUB was first introduced.
The writes we do happen within dmub_hw_init and on every command execution, but this patch adds the flush before HW init. I think the only issue this potentially fixes is the initial writeout in the SW PSP code to VRAM, but they already have flushes in place
for that. The signature validation would cause firmware to fail to load if it wasn't at least.
So from a correctness perspective I don't think this patch causes any issue, but from a performance perspective this probably adds at least 100us to boot, if not more. My recommendation is to leave things as-is for now.
Regards,
Nicholas Kazlauskas
From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> on behalf of Alex Deucher <alexander.deucher@xxxxxxx>
Sent: Thursday, April 28, 2022 6:13 PM To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx> Subject: [PATCH] drm/amdgpu/display: flush the HDP when setting up DMCUB firmware When data is written to VRAM via the PCI BAR, the data goes
through a block called HDP which has a write queue and a read cache. When the driver writes to VRAM, it needs to flush the HDP write queue to make sure all the data written has actually hit VRAM. When we write the DMCUB firmware to vram, we never flushed the HDP. In theory this could cause DMCUB errors if we try and start the DMCUB firmware without making sure the data has hit memory. This doesn't fix any known issues, but is the right thing to do. Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx> --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index a6c3e1d74124..5c1fd3a91cd5 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -1133,6 +1133,10 @@ static int dm_dmub_hw_init(struct amdgpu_device *adev) break; } + /* flush HDP */ + mb(); + amdgpu_device_flush_hdp(adev, NULL); + status = dmub_srv_hw_init(dmub_srv, &hw_params); if (status != DMUB_STATUS_OK) { DRM_ERROR("Error initializing DMUB HW: %d\n", status); -- 2.35.1 |
