On Thu, Apr 28, 2022 at 9:41 PM Kazlauskas, Nicholas
<[email protected]> wrote:
>
> [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.

Thanks for the background.  I'll drop that patch.

Alex


>
> Regards,
> Nicholas Kazlauskas
> ________________________________
> From: amd-gfx <[email protected]> on behalf of Alex 
> Deucher <[email protected]>
> Sent: Thursday, April 28, 2022 6:13 PM
> To: [email protected] <[email protected]>
> Cc: Deucher, Alexander <[email protected]>
> 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 <[email protected]>
> ---
>  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
>

Reply via email to