Re: [PATCH 09/32] drm/amdgpu: add gfx9.4.2 hw debug mode enable and disable calls

[Felix Kuehling <felix.kuehling@amd.com>]

On 2023-01-25 14:53, Jonathan Kim wrote:
> GFX9.4.2 now supports per-VMID debug mode controls registers
> (SPI_GDBG_PER_VMID_CNTL).
>
> Because the KFD lets the HWS handle PASID-VMID mapping, the KFD will
> forward all debug mode setting register writes to the HWS scheduler
> using a new MAP_PROCESS API, so instead of writing to registers, return
> the required register values that the HWS needs to write on debug enable
> and disable.
>
> v2: add commentary on unused restore_dbg_registers for debug enable.
>
> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
> ---
>   .../drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c  | 43 ++++++++++++++++++-
>   1 file changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c
> index 4485bb29bec9..89868f9927ae 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_aldebaran.c
> @@ -23,6 +23,44 @@
>   #include "amdgpu_amdkfd.h"
>   #include "amdgpu_amdkfd_arcturus.h"
>   #include "amdgpu_amdkfd_gfx_v9.h"
> +#include "gc/gc_9_4_2_offset.h"
> +#include "gc/gc_9_4_2_sh_mask.h"
> +
> +/**

Use /* here.


> + * Returns TRAP_EN, EXCP_EN and EXCP_REPLACE.
> + *
> + * restore_dbg_reisters is ignored here but is a general interface requirement

Typo: registers


> + * for devices that support GFXOFF and where the RLC save/restore list
> + * does not support hw registers for debugging i.e. the driver has to manually
> + * initialize the debug mode registers after it has disabled GFX off during the
> + * debug session.
> + */
> +static uint32_t kgd_aldebaran_enable_debug_trap(struct amdgpu_device *adev,
> +					    bool restore_dbg_registers,
> +					    uint32_t vmid)
> +{
> +	uint32_t data = 0;
> +
> +	data = REG_SET_FIELD(data, SPI_GDBG_PER_VMID_CNTL, TRAP_EN, 1);
> +	data = REG_SET_FIELD(data, SPI_GDBG_PER_VMID_CNTL, EXCP_EN, 0);
> +	data = REG_SET_FIELD(data, SPI_GDBG_PER_VMID_CNTL, EXCP_REPLACE, 0);
> +
> +	return data;
> +}
> +
> +/* returns TRAP_EN, EXCP_EN and EXCP_REPLACE. */
> +static uint32_t kgd_aldebaran_disable_debug_trap(struct amdgpu_device *adev,
> +						bool keep_trap_enabled,
> +						uint32_t vmid)
> +{
> +	uint32_t data = 0;
> +
> +	data = REG_SET_FIELD(data, SPI_GDBG_PER_VMID_CNTL, TRAP_EN, keep_trap_enabled);
> +	data = REG_SET_FIELD(data, SPI_GDBG_PER_VMID_CNTL, EXCP_EN, 0);
> +	data = REG_SET_FIELD(data, SPI_GDBG_PER_VMID_CNTL, EXCP_REPLACE, 0);
> +
> +	return data;
> +}
>   
>   const struct kfd2kgd_calls aldebaran_kfd2kgd = {
>   	.program_sh_mem_settings = kgd_gfx_v9_program_sh_mem_settings,
> @@ -41,6 +79,7 @@ const struct kfd2kgd_calls aldebaran_kfd2kgd = {
>   	.get_atc_vmid_pasid_mapping_info =
>   				kgd_gfx_v9_get_atc_vmid_pasid_mapping_info,
>   	.set_vm_context_page_table_base = kgd_gfx_v9_set_vm_context_page_table_base,
> -	.get_cu_occupancy = kgd_gfx_v9_get_cu_occupancy,

I think you're removing get_cu_occupancy accidentally here?

With those issues fixed, the patch is

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> -	.program_trap_handler_settings = kgd_gfx_v9_program_trap_handler_settings
> +	.enable_debug_trap = kgd_aldebaran_enable_debug_trap,
> +	.disable_debug_trap = kgd_aldebaran_disable_debug_trap,
> +	.program_trap_handler_settings = kgd_gfx_v9_program_trap_handler_settings,
>   };