AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/amd/pm: Add debug prints
@ 2021-11-17 18:55 Luben Tuikov
  2021-11-17 18:55 ` [PATCH 2/2] drm/amd/pm: Print the error on command submission Luben Tuikov
  2021-11-17 21:54 ` [PATCH 1/2] drm/amd/pm: Add debug prints Alex Deucher
  0 siblings, 2 replies; 5+ messages in thread
From: Luben Tuikov @ 2021-11-17 18:55 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Luben Tuikov

Add prints where there are none and none are printed in the callee.

Add a print in sienna_cichlid_run_btc() to help debug and to mirror other
platforms, as no print is present in the caller, smu_smc_hw_setup().

Remove the word "previous" from comment and print to make it shorter and
avoid confusion in various prints.

Cc: Alex Deucher <Alexander.Deucher@amd.com>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
---
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c               | 8 +++++---
 drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 8 +++++++-
 drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c                  | 4 ++--
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 01168b8955bff3..67cc6fb4f422f4 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -1153,6 +1153,8 @@ static int smu_smc_hw_setup(struct smu_context *smu)
 		case IP_VERSION(11, 5, 0):
 		case IP_VERSION(11, 0, 12):
 			ret = smu_system_features_control(smu, true);
+			if (ret)
+				dev_err(adev->dev, "Failed system features control!\n");
 			break;
 		default:
 			break;
@@ -1277,8 +1279,10 @@ static int smu_smc_hw_setup(struct smu_context *smu)
 	}
 
 	ret = smu_notify_display_change(smu);
-	if (ret)
+	if (ret) {
+		dev_err(adev->dev, "Failed to notify display change!\n");
 		return ret;
+	}
 
 	/*
 	 * Set min deep sleep dce fclk with bootup value from vbios via
@@ -1286,8 +1290,6 @@ static int smu_smc_hw_setup(struct smu_context *smu)
 	 */
 	ret = smu_set_min_dcef_deep_sleep(smu,
 					  smu->smu_table.boot_values.dcefclk / 100);
-	if (ret)
-		return ret;
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index b0bb389185d51c..f3522320df7e58 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -2135,7 +2135,13 @@ static int sienna_cichlid_od_edit_dpm_table(struct smu_context *smu,
 
 static int sienna_cichlid_run_btc(struct smu_context *smu)
 {
-	return smu_cmn_send_smc_msg(smu, SMU_MSG_RunDcBtc, NULL);
+	int res;
+
+	res = smu_cmn_send_smc_msg(smu, SMU_MSG_RunDcBtc, NULL);
+	if (res)
+		dev_err(smu->adev->dev, "RunDcBtc failed!\n");
+
+	return res;
 }
 
 static int sienna_cichlid_baco_enter(struct smu_context *smu)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
index ea6f50c08c5f3b..f9a42a07eeaebf 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
@@ -97,7 +97,7 @@ static void smu_cmn_read_arg(struct smu_context *smu,
  * smu: a pointer to SMU context
  *
  * Returns the status of the SMU, which could be,
- *    0, the SMU is busy with your previous command;
+ *    0, the SMU is busy with your command;
  *    1, execution status: success, execution result: success;
  * 0xFF, execution status: success, execution result: failure;
  * 0xFE, unknown command;
@@ -143,7 +143,7 @@ static void __smu_cmn_reg_print_error(struct smu_context *smu,
 		u32 msg_idx = RREG32_SOC15(MP1, 0, mmMP1_SMN_C2PMSG_66);
 		u32 prm     = RREG32_SOC15(MP1, 0, mmMP1_SMN_C2PMSG_82);
 		dev_err_ratelimited(adev->dev,
-				    "SMU: I'm not done with your previous command: SMN_C2PMSG_66:0x%08X SMN_C2PMSG_82:0x%08X",
+				    "SMU: I'm not done with your command: SMN_C2PMSG_66:0x%08X SMN_C2PMSG_82:0x%08X",
 				    msg_idx, prm);
 	}
 		break;

base-commit: ae2faedcc13fa5ee109ceb9e8cc05d759ad57980
-- 
2.34.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] drm/amd/pm: Print the error on command submission
  2021-11-17 18:55 [PATCH 1/2] drm/amd/pm: Add debug prints Luben Tuikov
@ 2021-11-17 18:55 ` Luben Tuikov
  2021-11-17 21:55   ` Alex Deucher
  2021-11-17 21:54 ` [PATCH 1/2] drm/amd/pm: Add debug prints Alex Deucher
  1 sibling, 1 reply; 5+ messages in thread
From: Luben Tuikov @ 2021-11-17 18:55 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Luben Tuikov

Print the error on command submission immediately after submitting to
the SMU. This is rate-limited. It helps to immediately know there was an
error on command submission, rather than leave it up to clients to report
the error, as sometimes they do not.

Cc: Alex Deucher <Alexander.Deucher@amd.com>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
---
 drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
index f9a42a07eeaebf..048ca16738638f 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
@@ -352,7 +352,7 @@ int smu_cmn_send_smc_msg_with_param(struct smu_context *smu,
 	__smu_cmn_send_msg(smu, (uint16_t) index, param);
 	reg = __smu_cmn_poll_stat(smu);
 	res = __smu_cmn_reg2errno(smu, reg);
-	if (res == -EREMOTEIO)
+	if (res != 0)
 		__smu_cmn_reg_print_error(smu, reg, index, param, msg);
 	if (read_arg)
 		smu_cmn_read_arg(smu, read_arg);
-- 
2.34.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] drm/amd/pm: Add debug prints
  2021-11-17 18:55 [PATCH 1/2] drm/amd/pm: Add debug prints Luben Tuikov
  2021-11-17 18:55 ` [PATCH 2/2] drm/amd/pm: Print the error on command submission Luben Tuikov
@ 2021-11-17 21:54 ` Alex Deucher
  2021-11-22 22:19   ` Luben Tuikov
  1 sibling, 1 reply; 5+ messages in thread
From: Alex Deucher @ 2021-11-17 21:54 UTC (permalink / raw)
  To: Luben Tuikov; +Cc: Alex Deucher, amd-gfx list

On Wed, Nov 17, 2021 at 1:56 PM Luben Tuikov <luben.tuikov@amd.com> wrote:
>
> Add prints where there are none and none are printed in the callee.
>
> Add a print in sienna_cichlid_run_btc() to help debug and to mirror other
> platforms, as no print is present in the caller, smu_smc_hw_setup().
>
> Remove the word "previous" from comment and print to make it shorter and
> avoid confusion in various prints.
>
> Cc: Alex Deucher <Alexander.Deucher@amd.com>
> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
> ---
>  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c               | 8 +++++---
>  drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 8 +++++++-
>  drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c                  | 4 ++--
>  3 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 01168b8955bff3..67cc6fb4f422f4 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -1153,6 +1153,8 @@ static int smu_smc_hw_setup(struct smu_context *smu)
>                 case IP_VERSION(11, 5, 0):
>                 case IP_VERSION(11, 0, 12):
>                         ret = smu_system_features_control(smu, true);
> +                       if (ret)
> +                               dev_err(adev->dev, "Failed system features control!\n");
>                         break;
>                 default:
>                         break;
> @@ -1277,8 +1279,10 @@ static int smu_smc_hw_setup(struct smu_context *smu)
>         }
>
>         ret = smu_notify_display_change(smu);
> -       if (ret)
> +       if (ret) {
> +               dev_err(adev->dev, "Failed to notify display change!\n");
>                 return ret;
> +       }
>
>         /*
>          * Set min deep sleep dce fclk with bootup value from vbios via
> @@ -1286,8 +1290,6 @@ static int smu_smc_hw_setup(struct smu_context *smu)
>          */
>         ret = smu_set_min_dcef_deep_sleep(smu,
>                                           smu->smu_table.boot_values.dcefclk / 100);
> -       if (ret)
> -               return ret;
>
>         return ret;
>  }
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> index b0bb389185d51c..f3522320df7e58 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> @@ -2135,7 +2135,13 @@ static int sienna_cichlid_od_edit_dpm_table(struct smu_context *smu,
>
>  static int sienna_cichlid_run_btc(struct smu_context *smu)
>  {
> -       return smu_cmn_send_smc_msg(smu, SMU_MSG_RunDcBtc, NULL);
> +       int res;
> +
> +       res = smu_cmn_send_smc_msg(smu, SMU_MSG_RunDcBtc, NULL);
> +       if (res)
> +               dev_err(smu->adev->dev, "RunDcBtc failed!\n");
> +
> +       return res;

Maybe better to split this hunk into a separate patch and also fix up
the run_btc functions for other asics.

Alex


>  }
>
>  static int sienna_cichlid_baco_enter(struct smu_context *smu)
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> index ea6f50c08c5f3b..f9a42a07eeaebf 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> @@ -97,7 +97,7 @@ static void smu_cmn_read_arg(struct smu_context *smu,
>   * smu: a pointer to SMU context
>   *
>   * Returns the status of the SMU, which could be,
> - *    0, the SMU is busy with your previous command;
> + *    0, the SMU is busy with your command;
>   *    1, execution status: success, execution result: success;
>   * 0xFF, execution status: success, execution result: failure;
>   * 0xFE, unknown command;
> @@ -143,7 +143,7 @@ static void __smu_cmn_reg_print_error(struct smu_context *smu,
>                 u32 msg_idx = RREG32_SOC15(MP1, 0, mmMP1_SMN_C2PMSG_66);
>                 u32 prm     = RREG32_SOC15(MP1, 0, mmMP1_SMN_C2PMSG_82);
>                 dev_err_ratelimited(adev->dev,
> -                                   "SMU: I'm not done with your previous command: SMN_C2PMSG_66:0x%08X SMN_C2PMSG_82:0x%08X",
> +                                   "SMU: I'm not done with your command: SMN_C2PMSG_66:0x%08X SMN_C2PMSG_82:0x%08X",
>                                     msg_idx, prm);
>         }
>                 break;
>
> base-commit: ae2faedcc13fa5ee109ceb9e8cc05d759ad57980
> --
> 2.34.0
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] drm/amd/pm: Print the error on command submission
  2021-11-17 18:55 ` [PATCH 2/2] drm/amd/pm: Print the error on command submission Luben Tuikov
@ 2021-11-17 21:55   ` Alex Deucher
  0 siblings, 0 replies; 5+ messages in thread
From: Alex Deucher @ 2021-11-17 21:55 UTC (permalink / raw)
  To: Luben Tuikov; +Cc: Alex Deucher, amd-gfx list

Acked-by: Alex Deucher <alexander.deucher@amd.com>

On Wed, Nov 17, 2021 at 1:56 PM Luben Tuikov <luben.tuikov@amd.com> wrote:
>
> Print the error on command submission immediately after submitting to
> the SMU. This is rate-limited. It helps to immediately know there was an
> error on command submission, rather than leave it up to clients to report
> the error, as sometimes they do not.
>
> Cc: Alex Deucher <Alexander.Deucher@amd.com>
> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
> ---
>  drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> index f9a42a07eeaebf..048ca16738638f 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> @@ -352,7 +352,7 @@ int smu_cmn_send_smc_msg_with_param(struct smu_context *smu,
>         __smu_cmn_send_msg(smu, (uint16_t) index, param);
>         reg = __smu_cmn_poll_stat(smu);
>         res = __smu_cmn_reg2errno(smu, reg);
> -       if (res == -EREMOTEIO)
> +       if (res != 0)
>                 __smu_cmn_reg_print_error(smu, reg, index, param, msg);
>         if (read_arg)
>                 smu_cmn_read_arg(smu, read_arg);
> --
> 2.34.0
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] drm/amd/pm: Add debug prints
  2021-11-17 21:54 ` [PATCH 1/2] drm/amd/pm: Add debug prints Alex Deucher
@ 2021-11-22 22:19   ` Luben Tuikov
  0 siblings, 0 replies; 5+ messages in thread
From: Luben Tuikov @ 2021-11-22 22:19 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Alex Deucher, amd-gfx list

On 2021-11-17 16:54, Alex Deucher wrote:
> On Wed, Nov 17, 2021 at 1:56 PM Luben Tuikov <luben.tuikov@amd.com> wrote:
>> Add prints where there are none and none are printed in the callee.
>>
>> Add a print in sienna_cichlid_run_btc() to help debug and to mirror other
>> platforms, as no print is present in the caller, smu_smc_hw_setup().
>>
>> Remove the word "previous" from comment and print to make it shorter and
>> avoid confusion in various prints.
>>
>> Cc: Alex Deucher <Alexander.Deucher@amd.com>
>> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
>> ---
>>  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c               | 8 +++++---
>>  drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 8 +++++++-
>>  drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c                  | 4 ++--
>>  3 files changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> index 01168b8955bff3..67cc6fb4f422f4 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> @@ -1153,6 +1153,8 @@ static int smu_smc_hw_setup(struct smu_context *smu)
>>                 case IP_VERSION(11, 5, 0):
>>                 case IP_VERSION(11, 0, 12):
>>                         ret = smu_system_features_control(smu, true);
>> +                       if (ret)
>> +                               dev_err(adev->dev, "Failed system features control!\n");
>>                         break;
>>                 default:
>>                         break;
>> @@ -1277,8 +1279,10 @@ static int smu_smc_hw_setup(struct smu_context *smu)
>>         }
>>
>>         ret = smu_notify_display_change(smu);
>> -       if (ret)
>> +       if (ret) {
>> +               dev_err(adev->dev, "Failed to notify display change!\n");
>>                 return ret;
>> +       }
>>
>>         /*
>>          * Set min deep sleep dce fclk with bootup value from vbios via
>> @@ -1286,8 +1290,6 @@ static int smu_smc_hw_setup(struct smu_context *smu)
>>          */
>>         ret = smu_set_min_dcef_deep_sleep(smu,
>>                                           smu->smu_table.boot_values.dcefclk / 100);
>> -       if (ret)
>> -               return ret;
>>
>>         return ret;
>>  }
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>> index b0bb389185d51c..f3522320df7e58 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>> @@ -2135,7 +2135,13 @@ static int sienna_cichlid_od_edit_dpm_table(struct smu_context *smu,
>>
>>  static int sienna_cichlid_run_btc(struct smu_context *smu)
>>  {
>> -       return smu_cmn_send_smc_msg(smu, SMU_MSG_RunDcBtc, NULL);
>> +       int res;
>> +
>> +       res = smu_cmn_send_smc_msg(smu, SMU_MSG_RunDcBtc, NULL);
>> +       if (res)
>> +               dev_err(smu->adev->dev, "RunDcBtc failed!\n");
>> +
>> +       return res;
> Maybe better to split this hunk into a separate patch and also fix up
> the run_btc functions for other asics.

No problem. I checked all under amd/pm/swsmu and Sienna is the only one.
I'll send another set of patches.

Regards,
Luben

>
> Alex
>
>
>>  }
>>
>>  static int sienna_cichlid_baco_enter(struct smu_context *smu)
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
>> index ea6f50c08c5f3b..f9a42a07eeaebf 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
>> @@ -97,7 +97,7 @@ static void smu_cmn_read_arg(struct smu_context *smu,
>>   * smu: a pointer to SMU context
>>   *
>>   * Returns the status of the SMU, which could be,
>> - *    0, the SMU is busy with your previous command;
>> + *    0, the SMU is busy with your command;
>>   *    1, execution status: success, execution result: success;
>>   * 0xFF, execution status: success, execution result: failure;
>>   * 0xFE, unknown command;
>> @@ -143,7 +143,7 @@ static void __smu_cmn_reg_print_error(struct smu_context *smu,
>>                 u32 msg_idx = RREG32_SOC15(MP1, 0, mmMP1_SMN_C2PMSG_66);
>>                 u32 prm     = RREG32_SOC15(MP1, 0, mmMP1_SMN_C2PMSG_82);
>>                 dev_err_ratelimited(adev->dev,
>> -                                   "SMU: I'm not done with your previous command: SMN_C2PMSG_66:0x%08X SMN_C2PMSG_82:0x%08X",
>> +                                   "SMU: I'm not done with your command: SMN_C2PMSG_66:0x%08X SMN_C2PMSG_82:0x%08X",
>>                                     msg_idx, prm);
>>         }
>>                 break;
>>
>> base-commit: ae2faedcc13fa5ee109ceb9e8cc05d759ad57980
>> --
>> 2.34.0
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-11-22 22:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-17 18:55 [PATCH 1/2] drm/amd/pm: Add debug prints Luben Tuikov
2021-11-17 18:55 ` [PATCH 2/2] drm/amd/pm: Print the error on command submission Luben Tuikov
2021-11-17 21:55   ` Alex Deucher
2021-11-17 21:54 ` [PATCH 1/2] drm/amd/pm: Add debug prints Alex Deucher
2021-11-22 22:19   ` Luben Tuikov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox