[Public]
Thanks Lijo and Christian for your review, this patch has been pushed with Alex's RB:(
Regards,
Guchun
-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
Sent: Thursday, November 11, 2021 3:22 PM
To: Chen, Guchun <Guchun.Chen@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, Christian <Christian.Koenig@xxxxxxx>; Pan, Xinhui <Xinhui.Pan@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: add error print when failing to add IP block(v2)
On 11/11/2021 9:41 AM, Guchun Chen wrote:
> Driver initialization is driven by IP version from IP discovery table.
> So add error print when failing to add ip block during driver
> initialization, this will be more friendly to user to know which IP
> version is not correct.
>
> [ 40.467361] [drm] host supports REQ_INIT_DATA handshake
> [ 40.474076] [drm] add ip block number 0 <nv_common>
> [ 40.474090] [drm] add ip block number 1 <gmc_v10_0>
> [ 40.474101] [drm] add ip block number 2 <psp>
> [ 40.474103] [drm] add ip block number 3 <navi10_ih>
> [ 40.474114] [drm] add ip block number 4 <smu>
> [ 40.474119] [drm] add ip block number 5 <amdgpu_vkms>
> [ 40.474134] [drm] add ip block number 6 <gfx_v10_0>
> [ 40.474143] [drm] add ip block number 7 <sdma_v5_2>
> [ 40.474147] amdgpu 0000:00:08.0: amdgpu: Fatal error during GPU init
> [ 40.474545] amdgpu 0000:00:08.0: amdgpu: amdgpu: finishing device.
>
> v2: use dev_err to multi-GPU system
>
> Signed-off-by: Guchun Chen <guchun.chen@xxxxxxx>
> Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 36 +++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index ff70bc233489..4e3669407518 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -587,6 +587,9 @@ static int amdgpu_discovery_set_common_ip_blocks(struct amdgpu_device *adev)
> amdgpu_device_ip_block_add(adev, &nv_common_ip_block);
> break;
> default:
> + dev_err(adev->dev,
> + "Failed to add common ip block(GC_HWIP:0x%x)\n",
> + adev->ip_versions[GC_HWIP][0]);
If not submitted, a minor modification to message (if that sounds
appropriate)- "Found unsupported IP version" or "IP version is not supported yet". No need for v3.
Thanks,
Lijo
> return -EINVAL;
> }
> return 0;
> @@ -619,6 +622,9 @@ static int amdgpu_discovery_set_gmc_ip_blocks(struct amdgpu_device *adev)
> amdgpu_device_ip_block_add(adev, &gmc_v10_0_ip_block);
> break;
> default:
> + dev_err(adev->dev,
> + "Failed to add gmc ip block(GC_HWIP:0x%x)\n",
> + adev->ip_versions[GC_HWIP][0]);
> return -EINVAL;
> }
> return 0;
> @@ -648,6 +654,9 @@ static int amdgpu_discovery_set_ih_ip_blocks(struct amdgpu_device *adev)
> amdgpu_device_ip_block_add(adev, &navi10_ih_ip_block);
> break;
> default:
> + dev_err(adev->dev,
> + "Failed to add ih ip block(OSSSYS_HWIP:0x%x)\n",
> + adev->ip_versions[OSSSYS_HWIP][0]);
> return -EINVAL;
> }
> return 0;
> @@ -688,6 +697,9 @@ static int amdgpu_discovery_set_psp_ip_blocks(struct amdgpu_device *adev)
> amdgpu_device_ip_block_add(adev, &psp_v13_0_ip_block);
> break;
> default:
> + dev_err(adev->dev,
> + "Failed to add psp ip block(MP0_HWIP:0x%x)\n",
> + adev->ip_versions[MP0_HWIP][0]);
> return -EINVAL;
> }
> return 0;
> @@ -726,6 +738,9 @@ static int amdgpu_discovery_set_smu_ip_blocks(struct amdgpu_device *adev)
> amdgpu_device_ip_block_add(adev, &smu_v13_0_ip_block);
> break;
> default:
> + dev_err(adev->dev,
> + "Failed to add smu ip block(MP1_HWIP:0x%x)\n",
> + adev->ip_versions[MP1_HWIP][0]);
> return -EINVAL;
> }
> return 0;
> @@ -753,6 +768,9 @@ static int amdgpu_discovery_set_display_ip_blocks(struct amdgpu_device *adev)
> amdgpu_device_ip_block_add(adev, &dm_ip_block);
> break;
> default:
> + dev_err(adev->dev,
> + "Failed to add dm ip block(DCE_HWIP:0x%x)\n",
> + adev->ip_versions[DCE_HWIP][0]);
> return -EINVAL;
> }
> } else if (adev->ip_versions[DCI_HWIP][0]) { @@ -763,6 +781,9 @@
> static int amdgpu_discovery_set_display_ip_blocks(struct amdgpu_device *adev)
> amdgpu_device_ip_block_add(adev, &dm_ip_block);
> break;
> default:
> + dev_err(adev->dev,
> + "Failed to add dm ip block(DCI_HWIP:0x%x)\n",
> + adev->ip_versions[DCI_HWIP][0]);
> return -EINVAL;
> }
> #endif
> @@ -796,6 +817,9 @@ static int amdgpu_discovery_set_gc_ip_blocks(struct amdgpu_device *adev)
> amdgpu_device_ip_block_add(adev, &gfx_v10_0_ip_block);
> break;
> default:
> + dev_err(adev->dev,
> + "Failed to add gfx ip block(GC_HWIP:0x%x)\n",
> + adev->ip_versions[GC_HWIP][0]);
> return -EINVAL;
> }
> return 0;
> @@ -829,6 +853,9 @@ static int amdgpu_discovery_set_sdma_ip_blocks(struct amdgpu_device *adev)
> amdgpu_device_ip_block_add(adev, &sdma_v5_2_ip_block);
> break;
> default:
> + dev_err(adev->dev,
> + "Failed to add sdma ip block(SDMA0_HWIP:0x%x)\n",
> + adev->ip_versions[SDMA0_HWIP][0]);
> return -EINVAL;
> }
> return 0;
> @@ -845,6 +872,9 @@ static int amdgpu_discovery_set_mm_ip_blocks(struct amdgpu_device *adev)
> amdgpu_device_ip_block_add(adev, &uvd_v7_0_ip_block);
> break;
> default:
> + dev_err(adev->dev,
> + "Failed to add uvd v7 ip block(UVD_HWIP:0x%x)\n",
> + adev->ip_versions[UVD_HWIP][0]);
> return -EINVAL;
> }
> switch (adev->ip_versions[VCE_HWIP][0]) { @@ -855,6 +885,9 @@
> static int amdgpu_discovery_set_mm_ip_blocks(struct amdgpu_device *adev)
> amdgpu_device_ip_block_add(adev, &vce_v4_0_ip_block);
> break;
> default:
> + dev_err(adev->dev,
> + "Failed to add VCE v4 ip block(VCE_HWIP:0x%x)\n",
> + adev->ip_versions[VCE_HWIP][0]);
> return -EINVAL;
> }
> } else {
> @@ -893,6 +926,9 @@ static int amdgpu_discovery_set_mm_ip_blocks(struct amdgpu_device *adev)
> amdgpu_device_ip_block_add(adev, &vcn_v3_0_ip_block);
> break;
> default:
> + dev_err(adev->dev,
> + "Failed to add vcn/jpeg ip block(UVD_HWIP:0x%x)\n",
> + adev->ip_versions[UVD_HWIP][0]);
> return -EINVAL;
> }
> }
>
