RE: [PATCH v2] drm/amdgpu/pm: Add notification function for no DC support
[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. Thanks, Lijo From: Zhang, Bokun Sent: Tuesday, August 15, 2023 11:52 PM To: Lazar, Lijo ; Deucher, Alexander ; amd-gfx@lists.freedesktop.org Cc: Quan, Evan 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! From: Lazar, Lijo mailto:lijo.la...@amd.com>> Sent: Tuesday, August 15, 2023 2:03 PM To: Deucher, Alexander mailto:alexander.deuc...@amd.com>>; Zhang, Bokun mailto:bokun.zh...@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> Cc: Zhang, Bokun mailto:bokun.zh...@amd.com>>; Quan, Evan mailto:evan.q...@amd.com>> Subject: Re: [PATCH v2] drm/amdgpu/pm: Add notification function for no DC support [Public] There's already another smu callback - notify_display. This can be accommodated there, no need to add another callback. Thanks, Lijo From: amd-gfx mailto:amd-gfx-boun...@lists.freedesktop.org>> on behalf of Deucher, Alexander mailto:alexander.deuc...@amd.com>> Sent: Tuesday, August 15, 2023 11:13:14 PM To: Zhang, Bokun mailto:bokun.zh...@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> mailto:amd-gfx@lists.freedesktop.org>> Cc: Zhang, Bokun mailto:bokun.zh...@amd.com>>; Quan, Evan mailto:evan.q...@amd.com>> Subject: RE: [PATCH v2] drm/amdgpu/pm: Add notification function for no DC support [Public] [Public] > -Original Message- > From: amd-gfx > mailto:amd-gfx-boun...@lists.freedesktop.org>> > On Behalf Of Bokun > Zhang > Sent: Tuesday, August 15, 2023 11:50 AM > To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> > Cc: Zhang, Bokun mailto:bokun.zh...@amd.com>>; Quan, Evan > mailto:evan.q...@amd.com>> > 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 mailto:evan.q...@amd.com>> > Signed-off-by: Bokun Zhang mailto:bokun.zh...@amd.com>> > --- > 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 6e2069dcb6b
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! From: Lazar, Lijo Sent: Tuesday, August 15, 2023 2:03 PM To: Deucher, Alexander ; Zhang, Bokun ; amd-gfx@lists.freedesktop.org Cc: Zhang, Bokun ; Quan, Evan Subject: Re: [PATCH v2] drm/amdgpu/pm: Add notification function for no DC support [Public] There's already another smu callback - notify_display. This can be accommodated there, no need to add another callback. Thanks, Lijo From: amd-gfx mailto:amd-gfx-boun...@lists.freedesktop.org>> on behalf of Deucher, Alexander mailto:alexander.deuc...@amd.com>> Sent: Tuesday, August 15, 2023 11:13:14 PM To: Zhang, Bokun mailto:bokun.zh...@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> mailto:amd-gfx@lists.freedesktop.org>> Cc: Zhang, Bokun mailto:bokun.zh...@amd.com>>; Quan, Evan mailto:evan.q...@amd.com>> Subject: RE: [PATCH v2] drm/amdgpu/pm: Add notification function for no DC support [Public] [Public] > -Original Message- > From: amd-gfx > mailto:amd-gfx-boun...@lists.freedesktop.org>> > On Behalf Of Bokun > Zhang > Sent: Tuesday, August 15, 2023 11:50 AM > To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> > Cc: Zhang, Bokun mailto:bokun.zh...@amd.com>>; Quan, Evan > mailto:evan.q...@amd.com>> > 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 mailto:evan.q...@amd.com>> > Signed-off-by: Bokun Zhang mailto:bokun.zh...@amd.com>> > --- > 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 mailto:alexander.deuc...@amd.com>> > + */ > + int (*notify_no_dc)(struct smu_context *smu); > }; > > typedef enum { > diff --git > a/drivers/gpu/drm/amd/pm/swsmu/in
Re: [PATCH v2] drm/amdgpu/pm: Add notification function for no DC support
[Public] There's already another smu callback - notify_display. This can be accommodated there, no need to add another callback. Thanks, Lijo From: amd-gfx on behalf of Deucher, Alexander Sent: Tuesday, August 15, 2023 11:13:14 PM To: Zhang, Bokun ; amd-gfx@lists.freedesktop.org Cc: Zhang, Bokun ; Quan, Evan Subject: RE: [PATCH v2] drm/amdgpu/pm: Add notification function for no DC support [Public] [Public] > -Original Message- > From: amd-gfx On Behalf Of Bokun > Zhang > Sent: Tuesday, August 15, 2023 11:50 AM > To: amd-gfx@lists.freedesktop.org > Cc: Zhang, Bokun ; Quan, Evan > > 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 > Signed-off-by: Bokun Zhang > --- > 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 > + */ > + 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_TestMessage0x1 > 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/
RE: [PATCH v2] drm/amdgpu/pm: Add notification function for no DC support
[Public] > -Original Message- > From: amd-gfx On Behalf Of Bokun > Zhang > Sent: Tuesday, August 15, 2023 11:50 AM > To: amd-gfx@lists.freedesktop.org > Cc: Zhang, Bokun ; Quan, Evan > > 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 > Signed-off-by: Bokun Zhang > --- > 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 > + */ > + 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_TestMessage0x1 > 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); > +} > + > stat