2023-06-11 16:03:07

by Sui Jingfeng

[permalink] [raw]
Subject: [PATCH v2] drm/amdgpu: Add error reporting code for the display core

[why]

Because the drm/amdgpu drivers do not work with Navi 10 [RX 5700] series
GPUs on non-x86 platforms, this patch helps find out where the drivers fail.
After applying his patch, the following error message can be found:

[drm:dc_create [amdgpu]] *ERROR* dc_construct: failed to create resource pool
[drm:dc_create [amdgpu]] *ERROR* dc_create: dc construct failed
[drm] Display Core failed to initialize with v3.2.230!

Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 29 ++++++++++++++++--------
1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 52564b93f7eb..d33b78aa3e58 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -951,7 +951,7 @@ static bool dc_construct(struct dc *dc,
goto fail;
}

- dc_ctx = dc->ctx;
+ dc_ctx = dc->ctx;

/* Resource should construct all asic specific resources.
* This should be the only place where we need to parse the asic id
@@ -990,16 +990,21 @@ static bool dc_construct(struct dc *dc,
}

dc->res_pool = dc_create_resource_pool(dc, init_params, dc_ctx->dce_version);
- if (!dc->res_pool)
+ if (!dc->res_pool) {
+ dm_error("%s: failed to create resource pool\n", __func__);
goto fail;
+ }

/* set i2c speed if not done by the respective dcnxxx__resource.c */
if (dc->caps.i2c_speed_in_khz_hdcp == 0)
dc->caps.i2c_speed_in_khz_hdcp = dc->caps.i2c_speed_in_khz;

dc->clk_mgr = dc_clk_mgr_create(dc->ctx, dc->res_pool->pp_smu, dc->res_pool->dccg);
- if (!dc->clk_mgr)
+ if (!dc->clk_mgr) {
+ dm_error("%s: failed to create clk manager\n", __func__);
goto fail;
+ }
+
#ifdef CONFIG_DRM_AMD_DC_FP
dc->clk_mgr->force_smu_not_present = init_params->force_smu_not_present;

@@ -1022,14 +1027,18 @@ static bool dc_construct(struct dc *dc,
goto fail;
}

- if (!create_links(dc, init_params->num_virtual_links))
+ if (!create_links(dc, init_params->num_virtual_links)) {
+ dm_error("%s: failed to create links\n", __func__);
goto fail;
+ }

/* Create additional DIG link encoder objects if fewer than the platform
* supports were created during link construction.
*/
- if (!create_link_encoders(dc))
+ if (!create_link_encoders(dc)) {
+ dm_error("%s: failed to create link encoders\n", __func__);
goto fail;
+ }

dc_resource_state_construct(dc, dc->current_state);

@@ -1314,11 +1323,15 @@ struct dc *dc_create(const struct dc_init_data *init_params)
return NULL;

if (init_params->dce_environment == DCE_ENV_VIRTUAL_HW) {
- if (!dc_construct_ctx(dc, init_params))
+ if (!dc_construct_ctx(dc, init_params)) {
+ DC_LOG_ERROR("%s: dc construct failed\n", __func__);
goto destruct_dc;
+ }
} else {
- if (!dc_construct(dc, init_params))
+ if (!dc_construct(dc, init_params)) {
+ DC_LOG_ERROR("%s: dc construct failed\n", __func__);
goto destruct_dc;
+ }

full_pipe_count = dc->res_pool->pipe_count;
if (dc->res_pool->underlay_pipe_index != NO_UNDERLAY_PIPE)
@@ -1349,8 +1362,6 @@ struct dc *dc_create(const struct dc_init_data *init_params)

DC_LOG_DC("Display Core initialized\n");

-
-
return dc;

destruct_dc:
--
2.25.1



2023-06-12 15:01:59

by Deucher, Alexander

[permalink] [raw]
Subject: RE: [PATCH v2] drm/amdgpu: Add error reporting code for the display core

[Public]

> -----Original Message-----
> From: Sui Jingfeng <[email protected]>
> Sent: Sunday, June 11, 2023 11:24 AM
> To: Wentland, Harry <[email protected]>; Li, Sun peng (Leo)
> <[email protected]>; Siqueira, Rodrigo <[email protected]>;
> Deucher, Alexander <[email protected]>; Koenig, Christian
> <[email protected]>; Pan, Xinhui <[email protected]>; David
> Airlie <[email protected]>; Daniel Vetter <[email protected]>; Lee, Alvin
> <[email protected]>; Lei, Jun <[email protected]>; Zhuo, Qingqing (Lillian)
> <[email protected]>; Liu, Wenjing <[email protected]>; Tam,
> Samson <[email protected]>; Varone, Dillon
> <[email protected]>; Liu, HaoPing (Alan) <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]
> Subject: [PATCH v2] drm/amdgpu: Add error reporting code for the display
> core
>
> [why]
>
> Because the drm/amdgpu drivers do not work with Navi 10 [RX 5700] series
> GPUs on non-x86 platforms, this patch helps find out where the drivers fail.
> After applying his patch, the following error message can be found:
>
> [drm:dc_create [amdgpu]] *ERROR* dc_construct: failed to create resource
> pool [drm:dc_create [amdgpu]] *ERROR* dc_create: dc construct failed [drm]
> Display Core failed to initialize with v3.2.230!

Curious what the failure was on your platform. Resource create is mostly just allocating structures.

Alex

>
> Signed-off-by: Sui Jingfeng <[email protected]>
> ---
> drivers/gpu/drm/amd/display/dc/core/dc.c | 29 ++++++++++++++++--------
> 1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 52564b93f7eb..d33b78aa3e58 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -951,7 +951,7 @@ static bool dc_construct(struct dc *dc,
> goto fail;
> }
>
> - dc_ctx = dc->ctx;
> + dc_ctx = dc->ctx;
>
> /* Resource should construct all asic specific resources.
> * This should be the only place where we need to parse the asic id
> @@ -990,16 +990,21 @@ static bool dc_construct(struct dc *dc,
> }
>
> dc->res_pool = dc_create_resource_pool(dc, init_params, dc_ctx-
> >dce_version);
> - if (!dc->res_pool)
> + if (!dc->res_pool) {
> + dm_error("%s: failed to create resource pool\n", __func__);
> goto fail;
> + }
>
> /* set i2c speed if not done by the respective dcnxxx__resource.c */
> if (dc->caps.i2c_speed_in_khz_hdcp == 0)
> dc->caps.i2c_speed_in_khz_hdcp = dc-
> >caps.i2c_speed_in_khz;
>
> dc->clk_mgr = dc_clk_mgr_create(dc->ctx, dc->res_pool->pp_smu, dc-
> >res_pool->dccg);
> - if (!dc->clk_mgr)
> + if (!dc->clk_mgr) {
> + dm_error("%s: failed to create clk manager\n", __func__);
> goto fail;
> + }
> +
> #ifdef CONFIG_DRM_AMD_DC_FP
> dc->clk_mgr->force_smu_not_present = init_params-
> >force_smu_not_present;
>
> @@ -1022,14 +1027,18 @@ static bool dc_construct(struct dc *dc,
> goto fail;
> }
>
> - if (!create_links(dc, init_params->num_virtual_links))
> + if (!create_links(dc, init_params->num_virtual_links)) {
> + dm_error("%s: failed to create links\n", __func__);
> goto fail;
> + }
>
> /* Create additional DIG link encoder objects if fewer than the
> platform
> * supports were created during link construction.
> */
> - if (!create_link_encoders(dc))
> + if (!create_link_encoders(dc)) {
> + dm_error("%s: failed to create link encoders\n", __func__);
> goto fail;
> + }
>
> dc_resource_state_construct(dc, dc->current_state);
>
> @@ -1314,11 +1323,15 @@ struct dc *dc_create(const struct dc_init_data
> *init_params)
> return NULL;
>
> if (init_params->dce_environment == DCE_ENV_VIRTUAL_HW) {
> - if (!dc_construct_ctx(dc, init_params))
> + if (!dc_construct_ctx(dc, init_params)) {
> + DC_LOG_ERROR("%s: dc construct failed\n",
> __func__);
> goto destruct_dc;
> + }
> } else {
> - if (!dc_construct(dc, init_params))
> + if (!dc_construct(dc, init_params)) {
> + DC_LOG_ERROR("%s: dc construct failed\n",
> __func__);
> goto destruct_dc;
> + }
>
> full_pipe_count = dc->res_pool->pipe_count;
> if (dc->res_pool->underlay_pipe_index !=
> NO_UNDERLAY_PIPE) @@ -1349,8 +1362,6 @@ struct dc *dc_create(const
> struct dc_init_data *init_params)
>
> DC_LOG_DC("Display Core initialized\n");
>
> -
> -
> return dc;
>
> destruct_dc:
> --
> 2.25.1


2023-06-12 15:21:38

by Sui Jingfeng

[permalink] [raw]
Subject: Re: [PATCH v2] drm/amdgpu: Add error reporting code for the display core

Hi,

On 2023/6/12 22:41, Deucher, Alexander wrote:
> [Public]
>
>> -----Original Message-----
>> From: Sui Jingfeng <[email protected]>
>> Sent: Sunday, June 11, 2023 11:24 AM
>> To: Wentland, Harry <[email protected]>; Li, Sun peng (Leo)
>> <[email protected]>; Siqueira, Rodrigo <[email protected]>;
>> Deucher, Alexander <[email protected]>; Koenig, Christian
>> <[email protected]>; Pan, Xinhui <[email protected]>; David
>> Airlie <[email protected]>; Daniel Vetter <[email protected]>; Lee, Alvin
>> <[email protected]>; Lei, Jun <[email protected]>; Zhuo, Qingqing (Lillian)
>> <[email protected]>; Liu, Wenjing <[email protected]>; Tam,
>> Samson <[email protected]>; Varone, Dillon
>> <[email protected]>; Liu, HaoPing (Alan) <[email protected]>
>> Cc: [email protected]; [email protected]; linux-
>> [email protected]
>> Subject: [PATCH v2] drm/amdgpu: Add error reporting code for the display
>> core
>>
>> [why]
>>
>> Because the drm/amdgpu drivers do not work with Navi 10 [RX 5700] series
>> GPUs on non-x86 platforms, this patch helps find out where the drivers fail.
>> After applying his patch, the following error message can be found:
>>
>> [drm:dc_create [amdgpu]] *ERROR* dc_construct: failed to create resource
>> pool [drm:dc_create [amdgpu]] *ERROR* dc_create: dc construct failed [drm]
>> Display Core failed to initialize with v3.2.230!
> Curious what the failure was on your platform. Resource create is mostly just allocating structures.
This is because the DRM_AMD_DC_FP configuration does not get selected on
my hardware platform.
>
> Alex
>
>> Signed-off-by: Sui Jingfeng <[email protected]>
>> ---
>> drivers/gpu/drm/amd/display/dc/core/dc.c | 29 ++++++++++++++++--------
>> 1 file changed, 20 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
>> b/drivers/gpu/drm/amd/display/dc/core/dc.c
>> index 52564b93f7eb..d33b78aa3e58 100644
>> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
>> @@ -951,7 +951,7 @@ static bool dc_construct(struct dc *dc,
>> goto fail;
>> }
>>
>> - dc_ctx = dc->ctx;
>> + dc_ctx = dc->ctx;
>>
>> /* Resource should construct all asic specific resources.
>> * This should be the only place where we need to parse the asic id
>> @@ -990,16 +990,21 @@ static bool dc_construct(struct dc *dc,
>> }
>>
>> dc->res_pool = dc_create_resource_pool(dc, init_params, dc_ctx-
>>> dce_version);
>> - if (!dc->res_pool)
>> + if (!dc->res_pool) {
>> + dm_error("%s: failed to create resource pool\n", __func__);
>> goto fail;
>> + }
>>
>> /* set i2c speed if not done by the respective dcnxxx__resource.c */
>> if (dc->caps.i2c_speed_in_khz_hdcp == 0)
>> dc->caps.i2c_speed_in_khz_hdcp = dc-
>>> caps.i2c_speed_in_khz;
>> dc->clk_mgr = dc_clk_mgr_create(dc->ctx, dc->res_pool->pp_smu, dc-
>>> res_pool->dccg);
>> - if (!dc->clk_mgr)
>> + if (!dc->clk_mgr) {
>> + dm_error("%s: failed to create clk manager\n", __func__);
>> goto fail;
>> + }
>> +
>> #ifdef CONFIG_DRM_AMD_DC_FP
>> dc->clk_mgr->force_smu_not_present = init_params-
>>> force_smu_not_present;
>> @@ -1022,14 +1027,18 @@ static bool dc_construct(struct dc *dc,
>> goto fail;
>> }
>>
>> - if (!create_links(dc, init_params->num_virtual_links))
>> + if (!create_links(dc, init_params->num_virtual_links)) {
>> + dm_error("%s: failed to create links\n", __func__);
>> goto fail;
>> + }
>>
>> /* Create additional DIG link encoder objects if fewer than the
>> platform
>> * supports were created during link construction.
>> */
>> - if (!create_link_encoders(dc))
>> + if (!create_link_encoders(dc)) {
>> + dm_error("%s: failed to create link encoders\n", __func__);
>> goto fail;
>> + }
>>
>> dc_resource_state_construct(dc, dc->current_state);
>>
>> @@ -1314,11 +1323,15 @@ struct dc *dc_create(const struct dc_init_data
>> *init_params)
>> return NULL;
>>
>> if (init_params->dce_environment == DCE_ENV_VIRTUAL_HW) {
>> - if (!dc_construct_ctx(dc, init_params))
>> + if (!dc_construct_ctx(dc, init_params)) {
>> + DC_LOG_ERROR("%s: dc construct failed\n",
>> __func__);
>> goto destruct_dc;
>> + }
>> } else {
>> - if (!dc_construct(dc, init_params))
>> + if (!dc_construct(dc, init_params)) {
>> + DC_LOG_ERROR("%s: dc construct failed\n",
>> __func__);
>> goto destruct_dc;
>> + }
>>
>> full_pipe_count = dc->res_pool->pipe_count;
>> if (dc->res_pool->underlay_pipe_index !=
>> NO_UNDERLAY_PIPE) @@ -1349,8 +1362,6 @@ struct dc *dc_create(const
>> struct dc_init_data *init_params)
>>
>> DC_LOG_DC("Display Core initialized\n");
>>
>> -
>> -
>> return dc;
>>
>> destruct_dc:
>> --
>> 2.25.1

--
Jingfeng