[PATCH] drm/amd/display: Align dcn314_smu logging with other DCNs

[Limonciello, Mario]

[Public]

Conceptually makes sense to me, but please see below comments:

> -----Original Message-----
> From: Roman.Li at amd.com <Roman.Li at amd.com>
> Sent: Monday, November 14, 2022 15:07
> To: amd-gfx at lists.freedesktop.org; Deucher, Alexander
> <Alexander.Deucher at amd.com>; Wentland, Harry
> <Harry.Wentland at amd.com>; Limonciello, Mario
> <Mario.Limonciello at amd.com>; Rizvi, Saaem <SyedSaaem.Rizvi at amd.com>
> Cc: Li, Roman <Roman.Li at amd.com>
> Subject: [PATCH] drm/amd/display: Align dcn314_smu logging with other
> DCNs
> 
> From: Roman Li <roman.li at amd.com>
> 
> [Why]
> Assert on non-OK response from SMU is unnecessary.
> It was replaced with respective log message on other asics
> in the past with commit:
> "drm/amd/display: Removing assert statements for Linux"
> 
> [How]
> Remove asert and add dbg logging as on other DCNs.
> 
> CC: Saaem Rizvi <SyedSaaem.Rizvi at amd.com>
> Signed-off-by: Roman Li <roman.li at amd.com>
> ---
>  .../drm/amd/display/dc/clk_mgr/dcn314/dcn314_smu.c    | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn314/dcn314_smu.c
> b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn314/dcn314_smu.c
> index ef0795b14a1f..2db595672a46 100644
> --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn314/dcn314_smu.c
> +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn314/dcn314_smu.c
> @@ -123,9 +123,10 @@ static int
> dcn314_smu_send_msg_with_param(struct clk_mgr_internal *clk_mgr,
>  	uint32_t result;
> 
>  	result = dcn314_smu_wait_for_response(clk_mgr, 10, 200000);
> -	ASSERT(result == VBIOSSMC_Result_OK);

Does this flow actually happen still?  I thought the assertion should have gone
away as a result of 83293f7f3d15fc56e86bd5067a2c88b6b233ac3a.

Maybe we want to also undo the REG_WRITE() call there if pulling this in.

> 
> -	smu_print("SMU response after wait: %d\n", result);
> +	if (result != VBIOSSMC_Result_OK)
> +		smu_print("SMU Response was not OK. SMU response after
> wait received is: %d\n",
> +				result);
> 
>  	if (result == VBIOSSMC_Status_BUSY)
>  		return -1;

I think what is missing to clean up recent asserts is actually a little bit further
in the code than this.  It should be part of the error flow introduced by
03ad3093c7c069d6ab4403730009ebafeea9ee37

> @@ -216,6 +217,12 @@ int dcn314_smu_set_hard_min_dcfclk(struct
> clk_mgr_internal *clk_mgr, int request
>  			VBIOSSMC_MSG_SetHardMinDcfclkByFreq,
>  			khz_to_mhz_ceil(requested_dcfclk_khz));
> 
> +#ifdef DBG
> +	smu_print("actual_dcfclk_set_mhz %d is set to : %d\n",
> +			actual_dcfclk_set_mhz,
> +			actual_dcfclk_set_mhz * 1000);
> +#endif
> +
>  	return actual_dcfclk_set_mhz * 1000;
>  }
> 
> --
> 2.17.1