Re: [PATCH 3/5] drm/amdgpu: add vmhub funcs helper

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

Am 21.07.20 um 12:29 schrieb Huang Rui:
> This patch is to introduce vmhub funcs helper to add following callback
> (print_l2_protection_fault_status). Each GC/MMHUB register specific programming
> should be in gfxhub/mmhub level.
>
> Signed-off-by: Huang Rui <ray.huang@xxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h  |  7 +++++++
>   drivers/gpu/drm/amd/amdgpu/gfxhub_v2_0.c | 34 ++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c | 34 ++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c   | 25 ++---------------------
>   drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c  | 34 ++++++++++++++++++++++++++++++++
>   5 files changed, 111 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 1785a0e..bbecd87 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -74,6 +74,11 @@ struct amdgpu_gmc_fault {
>   /*
>    * VMHUB structures, functions & helpers
>    */
> +struct amdgpu_vmhub_funcs {
> +	void (*print_l2_protection_fault_status)(struct amdgpu_device *adev,
> +						 uint32_t status);
> +};
> +
>   struct amdgpu_vmhub {
>   	uint32_t	ctx0_ptb_addr_lo32;
>   	uint32_t	ctx0_ptb_addr_hi32;
> @@ -94,6 +99,8 @@ struct amdgpu_vmhub {
>   	uint32_t	eng_addr_distance; /* include LO32/HI32 */
>   
>   	uint32_t	vm_cntx_cntl_vm_fault;
> +
> +	const struct amdgpu_vmhub_funcs *vmhub_funcs;
>   };
>   
>   /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_0.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_0.c
> index 993185f..14268ea 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_0.c
> @@ -31,6 +31,33 @@
>   
>   #include "soc15_common.h"
>   
> +static void
> +gfxhub_v2_0_print_l2_protection_fault_status(struct amdgpu_device *adev,
> +					     uint32_t status)
> +{
> +	dev_err(adev->dev,
> +		"GCVM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
> +		status);
> +	dev_err(adev->dev, "\t Faulty UTCL2 client ID: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		GCVM_L2_PROTECTION_FAULT_STATUS, CID));
> +	dev_err(adev->dev, "\t MORE_FAULTS: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		GCVM_L2_PROTECTION_FAULT_STATUS, MORE_FAULTS));
> +	dev_err(adev->dev, "\t WALKER_ERROR: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		GCVM_L2_PROTECTION_FAULT_STATUS, WALKER_ERROR));
> +	dev_err(adev->dev, "\t PERMISSION_FAULTS: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		GCVM_L2_PROTECTION_FAULT_STATUS, PERMISSION_FAULTS));
> +	dev_err(adev->dev, "\t MAPPING_ERROR: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		GCVM_L2_PROTECTION_FAULT_STATUS, MAPPING_ERROR));
> +	dev_err(adev->dev, "\t RW: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		GCVM_L2_PROTECTION_FAULT_STATUS, RW));
> +}
> +
>   u64 gfxhub_v2_0_get_fb_location(struct amdgpu_device *adev)
>   {
>   	u64 base = RREG32_SOC15(GC, 0, mmGCMC_VM_FB_LOCATION_BASE);
> @@ -360,6 +387,10 @@ void gfxhub_v2_0_set_fault_enable_default(struct amdgpu_device *adev,
>   	WREG32_SOC15(GC, 0, mmGCVM_L2_PROTECTION_FAULT_CNTL, tmp);
>   }
>   
> +static const struct amdgpu_vmhub_funcs gfxhub_v2_0_vmhub_funcs = {
> +	.print_l2_protection_fault_status = gfxhub_v2_0_print_l2_protection_fault_status,
> +};
> +
>   void gfxhub_v2_0_init(struct amdgpu_device *adev)
>   {
>   	struct amdgpu_vmhub *hub = &adev->vmhub[AMDGPU_GFXHUB_0];
> @@ -398,4 +429,7 @@ void gfxhub_v2_0_init(struct amdgpu_device *adev)
>   		GCVM_CONTEXT1_CNTL__READ_PROTECTION_FAULT_ENABLE_INTERRUPT_MASK |
>   		GCVM_CONTEXT1_CNTL__WRITE_PROTECTION_FAULT_ENABLE_INTERRUPT_MASK |
>   		GCVM_CONTEXT1_CNTL__EXECUTE_PROTECTION_FAULT_ENABLE_INTERRUPT_MASK;
> +
> +	if (hub->vmhub_funcs == NULL)
> +		hub->vmhub_funcs = &gfxhub_v2_0_vmhub_funcs;
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
> index 07cae64..45fbce7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
> @@ -31,6 +31,33 @@
>   
>   #include "soc15_common.h"
>   
> +static void
> +gfxhub_v2_1_print_l2_protection_fault_status(struct amdgpu_device *adev,
> +					     uint32_t status)
> +{
> +	dev_err(adev->dev,
> +		"GCVM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
> +		status);
> +	dev_err(adev->dev, "\t Faulty UTCL2 client ID: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		GCVM_L2_PROTECTION_FAULT_STATUS, CID));
> +	dev_err(adev->dev, "\t MORE_FAULTS: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		GCVM_L2_PROTECTION_FAULT_STATUS, MORE_FAULTS));
> +	dev_err(adev->dev, "\t WALKER_ERROR: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		GCVM_L2_PROTECTION_FAULT_STATUS, WALKER_ERROR));
> +	dev_err(adev->dev, "\t PERMISSION_FAULTS: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		GCVM_L2_PROTECTION_FAULT_STATUS, PERMISSION_FAULTS));
> +	dev_err(adev->dev, "\t MAPPING_ERROR: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		GCVM_L2_PROTECTION_FAULT_STATUS, MAPPING_ERROR));
> +	dev_err(adev->dev, "\t RW: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		GCVM_L2_PROTECTION_FAULT_STATUS, RW));
> +}
> +
>   u64 gfxhub_v2_1_get_fb_location(struct amdgpu_device *adev)
>   {
>   	u64 base = RREG32_SOC15(GC, 0, mmGCMC_VM_FB_LOCATION_BASE);
> @@ -359,6 +386,10 @@ void gfxhub_v2_1_set_fault_enable_default(struct amdgpu_device *adev,
>   	WREG32_SOC15(GC, 0, mmGCVM_L2_PROTECTION_FAULT_CNTL, tmp);
>   }
>   
> +static const struct amdgpu_vmhub_funcs gfxhub_v2_1_vmhub_funcs = {
> +	.print_l2_protection_fault_status = gfxhub_v2_1_print_l2_protection_fault_status,
> +};
> +
>   void gfxhub_v2_1_init(struct amdgpu_device *adev)
>   {
>   	struct amdgpu_vmhub *hub = &adev->vmhub[AMDGPU_GFXHUB_0];
> @@ -397,6 +428,9 @@ void gfxhub_v2_1_init(struct amdgpu_device *adev)
>   		GCVM_CONTEXT1_CNTL__READ_PROTECTION_FAULT_ENABLE_INTERRUPT_MASK |
>   		GCVM_CONTEXT1_CNTL__WRITE_PROTECTION_FAULT_ENABLE_INTERRUPT_MASK |
>   		GCVM_CONTEXT1_CNTL__EXECUTE_PROTECTION_FAULT_ENABLE_INTERRUPT_MASK;
> +
> +	if (hub->vmhub_funcs == NULL)
> +		hub->vmhub_funcs = &gfxhub_v2_1_vmhub_funcs;
>   }
>   
>   int gfxhub_v2_1_get_xgmi_info(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> index e6c8526..8f35e13 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> @@ -121,29 +121,8 @@ static int gmc_v10_0_process_interrupt(struct amdgpu_device *adev,
>   			task_info.task_name, task_info.pid);
>   		dev_err(adev->dev, "  in page starting at address 0x%016llx from client %d\n",
>   			addr, entry->client_id);
> -		if (!amdgpu_sriov_vf(adev)) {
> -			dev_err(adev->dev,
> -				"GCVM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
> -				status);
> -			dev_err(adev->dev, "\t Faulty UTCL2 client ID: 0x%lx\n",
> -				REG_GET_FIELD(status,
> -				GCVM_L2_PROTECTION_FAULT_STATUS, CID));
> -			dev_err(adev->dev, "\t MORE_FAULTS: 0x%lx\n",
> -				REG_GET_FIELD(status,
> -				GCVM_L2_PROTECTION_FAULT_STATUS, MORE_FAULTS));
> -			dev_err(adev->dev, "\t WALKER_ERROR: 0x%lx\n",
> -				REG_GET_FIELD(status,
> -				GCVM_L2_PROTECTION_FAULT_STATUS, WALKER_ERROR));
> -			dev_err(adev->dev, "\t PERMISSION_FAULTS: 0x%lx\n",
> -				REG_GET_FIELD(status,
> -				GCVM_L2_PROTECTION_FAULT_STATUS, PERMISSION_FAULTS));
> -			dev_err(adev->dev, "\t MAPPING_ERROR: 0x%lx\n",
> -				REG_GET_FIELD(status,
> -				GCVM_L2_PROTECTION_FAULT_STATUS, MAPPING_ERROR));
> -			dev_err(adev->dev, "\t RW: 0x%lx\n",
> -				REG_GET_FIELD(status,
> -				GCVM_L2_PROTECTION_FAULT_STATUS, RW));
> -		}
> +		if (!amdgpu_sriov_vf(adev))
> +			hub->vmhub_funcs->print_l2_protection_fault_status(adev, status);
>   	}
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
> index 48134b9..fb634c1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
> @@ -36,6 +36,33 @@
>   #define mmDAGB0_CNTL_MISC2_Sienna_Cichlid                       0x0070
>   #define mmDAGB0_CNTL_MISC2_Sienna_Cichlid_BASE_IDX              0
>   
> +static void
> +mmhub_v2_0_print_l2_protection_fault_status(struct amdgpu_device *adev,
> +					     uint32_t status)
> +{
> +	dev_err(adev->dev,
> +		"MMVM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
> +		status);
> +	dev_err(adev->dev, "\t Faulty UTCL2 client ID: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		MMVM_L2_PROTECTION_FAULT_STATUS, CID));
> +	dev_err(adev->dev, "\t MORE_FAULTS: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		MMVM_L2_PROTECTION_FAULT_STATUS, MORE_FAULTS));
> +	dev_err(adev->dev, "\t WALKER_ERROR: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		MMVM_L2_PROTECTION_FAULT_STATUS, WALKER_ERROR));
> +	dev_err(adev->dev, "\t PERMISSION_FAULTS: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		MMVM_L2_PROTECTION_FAULT_STATUS, PERMISSION_FAULTS));
> +	dev_err(adev->dev, "\t MAPPING_ERROR: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		MMVM_L2_PROTECTION_FAULT_STATUS, MAPPING_ERROR));
> +	dev_err(adev->dev, "\t RW: 0x%lx\n",
> +		REG_GET_FIELD(status,
> +		MMVM_L2_PROTECTION_FAULT_STATUS, RW));
> +}
> +
>   void mmhub_v2_0_setup_vm_pt_regs(struct amdgpu_device *adev, uint32_t vmid,
>   				uint64_t page_table_base)
>   {
> @@ -351,6 +378,10 @@ void mmhub_v2_0_set_fault_enable_default(struct amdgpu_device *adev, bool value)
>   	WREG32_SOC15(MMHUB, 0, mmMMVM_L2_PROTECTION_FAULT_CNTL, tmp);
>   }
>   
> +static const struct amdgpu_vmhub_funcs mmhub_v2_0_vmhub_funcs = {
> +	.print_l2_protection_fault_status = mmhub_v2_0_print_l2_protection_fault_status,
> +};
> +
>   void mmhub_v2_0_init(struct amdgpu_device *adev)
>   {
>   	struct amdgpu_vmhub *hub = &adev->vmhub[AMDGPU_MMHUB_0];
> @@ -389,6 +420,9 @@ void mmhub_v2_0_init(struct amdgpu_device *adev)
>   		MMVM_CONTEXT1_CNTL__READ_PROTECTION_FAULT_ENABLE_INTERRUPT_MASK |
>   		MMVM_CONTEXT1_CNTL__WRITE_PROTECTION_FAULT_ENABLE_INTERRUPT_MASK |
>   		MMVM_CONTEXT1_CNTL__EXECUTE_PROTECTION_FAULT_ENABLE_INTERRUPT_MASK;
> +
> +	if (hub->vmhub_funcs == NULL)
> +		hub->vmhub_funcs = &mmhub_v2_0_vmhub_funcs;

Please assign that unconditionally here.

Apart from that the series looks good to me as well.

Christian.

>   }
>   
>   static void mmhub_v2_0_update_medium_grain_clock_gating(struct amdgpu_device *adev,

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx