|
[Public]
Hi Bokun,
Originally the call back is meant to enable fast UCLK switching based on display configuration. We can reuse the same interface to notify PMFW for any display related configuration. smu_v13_0_notify_display_change looks to be copied from smuv11, but not really
used now. I think the existing implementation can be dropped altogether.
From: Zhang, Bokun <Bokun.Zhang@xxxxxxx>
Sent: Tuesday, August 15, 2023 11:52 PM
To: Lazar, Lijo <Lijo.Lazar@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Quan, Evan <Evan.Quan@xxxxxxx>
Subject: RE: [PATCH v2] drm/amdgpu/pm: Add notification function for no DC support
[Public]
Hey Lijo,
I have considered to combine the function. However notify_display_change() may serve for another purpose as mentioned in the comment:
/**
* @notify_display_change: Enable fast memory clock switching.
*
* Allows for fine grained memory clock switching but has more stringent
* timing requirements.
*/
This function is implemented as smu_v13_0_notify_display_change() for SMU 13, but
not included in smu_v13_0_0_ppt_funcs struct and therefore not called at the moment.
I am not completely sure about the purpose of smu_v13_0_notify_display_change().
If it makes sense to combine them, I can make the change accordingly. But I would like to know if I should add a new function smu_v13_0_0_notify_display_change, or extend smu_v13_0_notify_display_change()
and add it to smu_v13_0_0_ppt_funcs.
Thanks!
[Public]
There's already another smu callback - notify_display. This can be accommodated there, no need to add another callback.
[Public]
[Public]
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Bokun
> Zhang
> Sent: Tuesday, August 15, 2023 11:50 AM
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Zhang, Bokun <Bokun.Zhang@xxxxxxx>; Quan, Evan
> <Evan.Quan@xxxxxxx>
> Subject: [PATCH v2] drm/amdgpu/pm: Add notification function for no DC
> support
>
> - There is a DPM issue where if DC is not present,
> FCLK will stay at low level.
> We need to send a SMU message to configure the DPM
>
> Reviewed-by: Evan Quan <evan.quan@xxxxxxx>
> Signed-off-by: Bokun Zhang <bokun.zhang@xxxxxxx>
> ---
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 6 ++++++
> drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 5 +++++
> .../gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h | 5
> ++++-
> drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h | 3 ++-
> drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 7
> +++++++
> drivers/gpu/drm/amd/pm/swsmu/smu_internal.h | 1 +
> 6 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index f005a90c35af..c65bebdbec11 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -1410,6 +1410,12 @@ static int smu_smc_hw_setup(struct
> smu_context *smu)
> return ret;
> }
>
> + if (!amdgpu_device_has_dc_support(adev)) {
> + ret = smu_notify_no_dc(smu);
> + if (ret)
> + dev_warn(adev->dev, "Failed to notify no dc support,
> driver may not reach best performance\n");
> + }
> +
> /*
> * Set min deep sleep dce fclk with bootup value from vbios via
> * SetMinDeepSleepDcefclk MSG.
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> index 6e2069dcb6b9..c8fdc3d0aa25 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> @@ -1356,6 +1356,11 @@ struct pptable_funcs {
> * @init_pptable_microcode: Prepare the pptable microcode to upload
> via PSP
> */
> int (*init_pptable_microcode)(struct smu_context *smu);
> +
> + /**
> + * @notify_no_dal: Notify SMU that there is no display and SMU
> should control DPM
Fix the function name in the kernel doc comment here. With that fixed, patch is:
Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
> + */
> + int (*notify_no_dc)(struct smu_context *smu);
> };
>
> typedef enum {
> diff --git
> a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h
> b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h
> index 10cff75b44d5..e2ee855c7748 100644
> ---
> a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h
> +++
> b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h
> @@ -138,7 +138,10 @@
> #define PPSMC_MSG_SetBadMemoryPagesRetiredFlagsPerChannel 0x4A
> #define PPSMC_MSG_SetPriorityDeltaGain 0x4B
> #define PPSMC_MSG_AllowIHHostInterrupt 0x4C
> -#define PPSMC_Message_Count 0x4D
> +
> +#define PPSMC_MSG_DALNotPresent 0x4E
> +
> +#define PPSMC_Message_Count 0x4F
>
> //Debug Dump Message
> #define DEBUGSMC_MSG_TestMessage 0x1
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> index 297b70b9388f..f71fc99447f2 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> @@ -245,7 +245,8 @@
> __SMU_DUMMY_MAP(AllowGpo), \
> __SMU_DUMMY_MAP(Mode2Reset), \
> __SMU_DUMMY_MAP(RequestI2cTransaction), \
> - __SMU_DUMMY_MAP(GetMetricsTable),
> + __SMU_DUMMY_MAP(GetMetricsTable), \
> + __SMU_DUMMY_MAP(DALNotPresent),
>
> #undef __SMU_DUMMY_MAP
> #define __SMU_DUMMY_MAP(type) SMU_MSG_##type
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> index 48b03524a52d..41412cf891a7 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> @@ -160,6 +160,7 @@ static struct cmn2asic_msg_mapping
> smu_v13_0_0_message_map[SMU_MSG_MAX_COUNT] =
> MSG_MAP(AllowGpo, PPSMC_MSG_SetGpoAllow,
> 0),
> MSG_MAP(AllowIHHostInterrupt,
> PPSMC_MSG_AllowIHHostInterrupt, 0),
> MSG_MAP(ReenableAcDcInterrupt,
> PPSMC_MSG_ReenableAcDcInterrupt, 0),
> + MSG_MAP(DALNotPresent,
> PPSMC_MSG_DALNotPresent, 0),
> };
>
> static struct cmn2asic_mapping smu_v13_0_0_clk_map[SMU_CLK_COUNT] =
> {
> @@ -2601,6 +2602,11 @@ static ssize_t smu_v13_0_0_get_ecc_info(struct
> smu_context *smu,
> return ret;
> }
>
> +static int smu_v13_0_0_notify_no_dc(struct smu_context *smu)
> +{
> + return smu_cmn_send_smc_msg(smu, SMU_MSG_DALNotPresent,
> NULL);
> +}
> +
> static const struct pptable_funcs smu_v13_0_0_ppt_funcs = {
> .get_allowed_feature_mask =
> smu_v13_0_0_get_allowed_feature_mask,
> .set_default_dpm_table = smu_v13_0_0_set_default_dpm_table,
> @@ -2680,6 +2686,7 @@ static const struct pptable_funcs
> smu_v13_0_0_ppt_funcs = {
> .send_hbm_bad_channel_flag =
> smu_v13_0_0_send_bad_mem_channel_flag,
> .gpo_control = smu_v13_0_gpo_control,
> .get_ecc_info = smu_v13_0_0_get_ecc_info,
> + .notify_no_dc = smu_v13_0_0_notify_no_dc,
> };
>
> void smu_v13_0_0_set_ppt_funcs(struct smu_context *smu)
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> index bcc42abfc768..9b9a13fdcef8 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> @@ -97,6 +97,7 @@
> #define smu_get_default_config_table_settings(smu, config_table)
> smu_ppt_funcs(get_default_config_table_settings, -EOPNOTSUPP,
> smu, config_table)
> #define smu_set_config_table(smu, config_table)
> smu_ppt_funcs(set_config_table, -EOPNOTSUPP, smu, config_table)
> #define smu_init_pptable_microcode(smu)
> smu_ppt_funcs(init_pptable_microcode, 0, smu)
> +#define smu_notify_no_dc(smu)
> smu_ppt_funcs(notify_no_dc, 0, smu)
>
> #endif
> #endif
> --
> 2.25.1
|
