Re: [PATCH] drm/amd/pp: Expose new interface to DC to ctrl auto wattman
Thanks Harry for clarify this question thoroughly. >We'll still need a patch to call this interface from DC. Is that on your plate >or is this something we should hook up? No, I just implemented the interface in pp. DC need to call this function to enable/disable the auto wattman feature when Freesync disable/enable. Best Regards Rex From: Wentland, Harry Sent: Thursday, February 8, 2018 11:28 PM To: Zhu, Rex; amd-gfx@lists.freedesktop.org Cc: Koo, Anthony; Cyr, Aric Subject: Re: [PATCH] drm/amd/pp: Expose new interface to DC to ctrl auto wattman On 2018-02-08 10:10 AM, Harry Wentland wrote: > On 2018-02-08 10:07 AM, Zhu, Rex wrote: >> when autowattman enabled,we will update uphyst/downhyst/min-sclk/mclk >> activity value to smu based on the workload. >> > > Why is this incompatible with Freesync? > Just had a chat with the Windows guys. Apparently AutoWattman changes clocks quite aggressively which can make framerates jump significantly and lead to a subpar experience when using Freesync. We'll still need a patch to call this interface from DC. Is that on your plate or is this something we should hook up? Harry > Harry > >> Best Regards >> Rex >> >> -- >> *From:* Wentland, Harry >> *Sent:* Thursday, February 8, 2018 10:22:16 PM >> *To:* Zhu, Rex; amd-gfx@lists.freedesktop.org >> *Subject:* Re: [PATCH] drm/amd/pp: Expose new interface to DC to ctrl auto >> wattman >> >> On 2018-02-08 06:20 AM, Rex Zhu wrote: >>> Disable AutoWattman (if enabled) when FreeSync is enabled. >> >> Do you have a DC change calling this? >> >> What's the use case for this and why do we need to disable AutoWattman when >> Freesync is enabled? >> >> What does AutoWattman do? >> >> Harry >> >>> >>> Change-Id: I9a531321d7913b8b40e60070c569a01c4f202002 >>> Signed-off-by: Rex Zhu <rex@amd.com> >>> --- >>> drivers/gpu/drm/amd/include/kgd_pp_interface.h | 1 + >>> drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 25 >>> + >>> 2 files changed, 26 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h >>> b/drivers/gpu/drm/amd/include/kgd_pp_interface.h >>> index 22c2fa3..f7bb565 100644 >>> --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h >>> +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h >>> @@ -313,6 +313,7 @@ struct amd_pm_funcs { >>> int (*set_power_profile_mode)(void *handle, long *input, uint32_t >>> size); >>> int (*odn_edit_dpm_table)(void *handle, uint32_t type, long *input, >>> uint32_t size); >>> int (*set_mmhub_powergating_by_smu)(void *handle); >>> + int (*notify_free_sync_change)(void *handle, bool en); >>> }; >>> >>> #endif >>> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c >>> b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c >>> index 376ed2d..d0306b6 100644 >>> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c >>> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c >>> @@ -1555,6 +1555,30 @@ static int pp_set_mmhub_powergating_by_smu(void >>> *handle) >>> return hwmgr->hwmgr_func->set_mmhub_powergating_by_smu(hwmgr); >>> } >>> >>> +static int pp_notify_free_sync_change(void *handle, bool en) >>> +{ >>> + struct pp_hwmgr *hwmgr; >>> + struct pp_instance *pp_handle = (struct pp_instance *)handle; >>> + int ret = 0; >>> + >>> + ret = pp_check(pp_handle); >>> + >>> + if (ret) >>>
Re: [PATCH] drm/amd/pp: Expose new interface to DC to ctrl auto wattman
On 2018-02-08 10:10 AM, Harry Wentland wrote: > On 2018-02-08 10:07 AM, Zhu, Rex wrote: >> when autowattman enabled,we will update uphyst/downhyst/min-sclk/mclk >> activity value to smu based on the workload. >> > > Why is this incompatible with Freesync? > Just had a chat with the Windows guys. Apparently AutoWattman changes clocks quite aggressively which can make framerates jump significantly and lead to a subpar experience when using Freesync. We'll still need a patch to call this interface from DC. Is that on your plate or is this something we should hook up? Harry > Harry > >> Best Regards >> Rex >> >> -- >> *From:* Wentland, Harry >> *Sent:* Thursday, February 8, 2018 10:22:16 PM >> *To:* Zhu, Rex; amd-gfx@lists.freedesktop.org >> *Subject:* Re: [PATCH] drm/amd/pp: Expose new interface to DC to ctrl auto >> wattman >> >> On 2018-02-08 06:20 AM, Rex Zhu wrote: >>> Disable AutoWattman (if enabled) when FreeSync is enabled. >> >> Do you have a DC change calling this? >> >> What's the use case for this and why do we need to disable AutoWattman when >> Freesync is enabled? >> >> What does AutoWattman do? >> >> Harry >> >>> >>> Change-Id: I9a531321d7913b8b40e60070c569a01c4f202002 >>> Signed-off-by: Rex Zhu <rex@amd.com> >>> --- >>> drivers/gpu/drm/amd/include/kgd_pp_interface.h | 1 + >>> drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 25 >>> + >>> 2 files changed, 26 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h >>> b/drivers/gpu/drm/amd/include/kgd_pp_interface.h >>> index 22c2fa3..f7bb565 100644 >>> --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h >>> +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h >>> @@ -313,6 +313,7 @@ struct amd_pm_funcs { >>> int (*set_power_profile_mode)(void *handle, long *input, uint32_t >>> size); >>> int (*odn_edit_dpm_table)(void *handle, uint32_t type, long *input, >>> uint32_t size); >>> int (*set_mmhub_powergating_by_smu)(void *handle); >>> + int (*notify_free_sync_change)(void *handle, bool en); >>> }; >>> >>> #endif >>> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c >>> b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c >>> index 376ed2d..d0306b6 100644 >>> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c >>> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c >>> @@ -1555,6 +1555,30 @@ static int pp_set_mmhub_powergating_by_smu(void >>> *handle) >>> return hwmgr->hwmgr_func->set_mmhub_powergating_by_smu(hwmgr); >>> } >>> >>> +static int pp_notify_free_sync_change(void *handle, bool en) >>> +{ >>> + struct pp_hwmgr *hwmgr; >>> + struct pp_instance *pp_handle = (struct pp_instance *)handle; >>> + int ret = 0; >>> + >>> + ret = pp_check(pp_handle); >>> + >>> + if (ret) >>> + return ret; >>> + >>> + hwmgr = pp_handle->hwmgr; >>> + >>> + mutex_lock(_handle->pp_lock); >>> + if (hwmgr->autowattman_enabled) { >>> + if (hwmgr->hwmgr_func->start_auto_wattman != NULL) { >>> + if >>> (!cancel_delayed_work_sync(>wattman_update_work)) >>> + hwmgr->hwmgr_func->start_auto_wattman(hwmgr, >>> en); >>> + } >>> + } >>> + mutex_unl
Re: [PATCH] drm/amd/pp: Expose new interface to DC to ctrl auto wattman
On 2018-02-08 10:07 AM, Zhu, Rex wrote: > when autowattman enabled,we will update uphyst/downhyst/min-sclk/mclk > activity value to smu based on the workload. > Why is this incompatible with Freesync? Harry > Best Regards > Rex > > -- > *From:* Wentland, Harry > *Sent:* Thursday, February 8, 2018 10:22:16 PM > *To:* Zhu, Rex; amd-gfx@lists.freedesktop.org > *Subject:* Re: [PATCH] drm/amd/pp: Expose new interface to DC to ctrl auto > wattman > > On 2018-02-08 06:20 AM, Rex Zhu wrote: >> Disable AutoWattman (if enabled) when FreeSync is enabled. > > Do you have a DC change calling this? > > What's the use case for this and why do we need to disable AutoWattman when > Freesync is enabled? > > What does AutoWattman do? > > Harry > >> >> Change-Id: I9a531321d7913b8b40e60070c569a01c4f202002 >> Signed-off-by: Rex Zhu <rex@amd.com> >> --- >> drivers/gpu/drm/amd/include/kgd_pp_interface.h | 1 + >> drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 25 >>+ >> 2 files changed, 26 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h >> b/drivers/gpu/drm/amd/include/kgd_pp_interface.h >> index 22c2fa3..f7bb565 100644 >> --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h >> +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h >> @@ -313,6 +313,7 @@ struct amd_pm_funcs { >> int (*set_power_profile_mode)(void *handle, long *input, uint32_t >>size); >> int (*odn_edit_dpm_table)(void *handle, uint32_t type, long *input, >>uint32_t size); >> int (*set_mmhub_powergating_by_smu)(void *handle); >> + int (*notify_free_sync_change)(void *handle, bool en); >> }; >> >> #endif >> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c >> b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c >> index 376ed2d..d0306b6 100644 >> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c >> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c >> @@ -1555,6 +1555,30 @@ static int pp_set_mmhub_powergating_by_smu(void >> *handle) >> return hwmgr->hwmgr_func->set_mmhub_powergating_by_smu(hwmgr); >> } >> >> +static int pp_notify_free_sync_change(void *handle, bool en) >> +{ >> + struct pp_hwmgr *hwmgr; >> + struct pp_instance *pp_handle = (struct pp_instance *)handle; >> + int ret = 0; >> + >> + ret = pp_check(pp_handle); >> + >> + if (ret) >> + return ret; >> + >> + hwmgr = pp_handle->hwmgr; >> + >> + mutex_lock(_handle->pp_lock); >> + if (hwmgr->autowattman_enabled) { >> + if (hwmgr->hwmgr_func->start_auto_wattman != NULL) { >> + if >> (!cancel_delayed_work_sync(>wattman_update_work)) >> + hwmgr->hwmgr_func->start_auto_wattman(hwmgr, >> en); >> + } >> + } >> + mutex_unlock(_handle->pp_lock); >> + return 0; >> +} >> + >> const struct amd_pm_funcs pp_dpm_funcs = { >> .load_firmware = pp_dpm_load_fw, >> .wait_for_fw_loading_complete = pp_dpm_fw_loading_complete, >> @@ -1604,4 +1628,5 @@ static int pp_set_mmhub_powergating_by_smu(void >> *handle) >> .display_clock_voltage_request = pp_display_clock_voltage_request, >> .get_display_mode_validation_clocks = >>pp_get_display_mode_validation_clocks, >> .set_mmhub_powergating_by_smu = pp_set_mmhub_powergating_by_smu, >> + .notify_free_sync_change = pp_notify_free_sync_change, >> }; >> ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/pp: Expose new interface to DC to ctrl auto wattman
when autowattman enabled,we will update uphyst/downhyst/min-sclk/mclk activity value to smu based on the workload. Best Regards Rex From: Wentland, Harry Sent: Thursday, February 8, 2018 10:22:16 PM To: Zhu, Rex; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/amd/pp: Expose new interface to DC to ctrl auto wattman On 2018-02-08 06:20 AM, Rex Zhu wrote: > Disable AutoWattman (if enabled) when FreeSync is enabled. Do you have a DC change calling this? What's the use case for this and why do we need to disable AutoWattman when Freesync is enabled? What does AutoWattman do? Harry > > Change-Id: I9a531321d7913b8b40e60070c569a01c4f202002 > Signed-off-by: Rex Zhu <rex@amd.com> > --- > drivers/gpu/drm/amd/include/kgd_pp_interface.h | 1 + > drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 25 + > 2 files changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h > b/drivers/gpu/drm/amd/include/kgd_pp_interface.h > index 22c2fa3..f7bb565 100644 > --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h > +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h > @@ -313,6 +313,7 @@ struct amd_pm_funcs { >int (*set_power_profile_mode)(void *handle, long *input, uint32_t > size); >int (*odn_edit_dpm_table)(void *handle, uint32_t type, long *input, > uint32_t size); >int (*set_mmhub_powergating_by_smu)(void *handle); > + int (*notify_free_sync_change)(void *handle, bool en); > }; > > #endif > diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > index 376ed2d..d0306b6 100644 > --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > @@ -1555,6 +1555,30 @@ static int pp_set_mmhub_powergating_by_smu(void > *handle) >return hwmgr->hwmgr_func->set_mmhub_powergating_by_smu(hwmgr); > } > > +static int pp_notify_free_sync_change(void *handle, bool en) > +{ > + struct pp_hwmgr *hwmgr; > + struct pp_instance *pp_handle = (struct pp_instance *)handle; > + int ret = 0; > + > + ret = pp_check(pp_handle); > + > + if (ret) > + return ret; > + > + hwmgr = pp_handle->hwmgr; > + > + mutex_lock(_handle->pp_lock); > + if (hwmgr->autowattman_enabled) { > + if (hwmgr->hwmgr_func->start_auto_wattman != NULL) { > + if > (!cancel_delayed_work_sync(>wattman_update_work)) > + hwmgr->hwmgr_func->start_auto_wattman(hwmgr, > en); > + } > + } > + mutex_unlock(_handle->pp_lock); > + return 0; > +} > + > const struct amd_pm_funcs pp_dpm_funcs = { >.load_firmware = pp_dpm_load_fw, >.wait_for_fw_loading_complete = pp_dpm_fw_loading_complete, > @@ -1604,4 +1628,5 @@ static int pp_set_mmhub_powergating_by_smu(void *handle) >.display_clock_voltage_request = pp_display_clock_voltage_request, >.get_display_mode_validation_clocks = > pp_get_display_mode_validation_clocks, >.set_mmhub_powergating_by_smu = pp_set_mmhub_powergating_by_smu, > + .notify_free_sync_change = pp_notify_free_sync_change, > }; > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/pp: Expose new interface to DC to ctrl auto wattman
On 2018-02-08 06:20 AM, Rex Zhu wrote: > Disable AutoWattman (if enabled) when FreeSync is enabled. Do you have a DC change calling this? What's the use case for this and why do we need to disable AutoWattman when Freesync is enabled? What does AutoWattman do? Harry > > Change-Id: I9a531321d7913b8b40e60070c569a01c4f202002 > Signed-off-by: Rex Zhu> --- > drivers/gpu/drm/amd/include/kgd_pp_interface.h | 1 + > drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 25 + > 2 files changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h > b/drivers/gpu/drm/amd/include/kgd_pp_interface.h > index 22c2fa3..f7bb565 100644 > --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h > +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h > @@ -313,6 +313,7 @@ struct amd_pm_funcs { > int (*set_power_profile_mode)(void *handle, long *input, uint32_t size); > int (*odn_edit_dpm_table)(void *handle, uint32_t type, long *input, > uint32_t size); > int (*set_mmhub_powergating_by_smu)(void *handle); > + int (*notify_free_sync_change)(void *handle, bool en); > }; > > #endif > diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > index 376ed2d..d0306b6 100644 > --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > @@ -1555,6 +1555,30 @@ static int pp_set_mmhub_powergating_by_smu(void > *handle) > return hwmgr->hwmgr_func->set_mmhub_powergating_by_smu(hwmgr); > } > > +static int pp_notify_free_sync_change(void *handle, bool en) > +{ > + struct pp_hwmgr *hwmgr; > + struct pp_instance *pp_handle = (struct pp_instance *)handle; > + int ret = 0; > + > + ret = pp_check(pp_handle); > + > + if (ret) > + return ret; > + > + hwmgr = pp_handle->hwmgr; > + > + mutex_lock(_handle->pp_lock); > + if (hwmgr->autowattman_enabled) { > + if (hwmgr->hwmgr_func->start_auto_wattman != NULL) { > + if > (!cancel_delayed_work_sync(>wattman_update_work)) > + hwmgr->hwmgr_func->start_auto_wattman(hwmgr, > en); > + } > + } > + mutex_unlock(_handle->pp_lock); > + return 0; > +} > + > const struct amd_pm_funcs pp_dpm_funcs = { > .load_firmware = pp_dpm_load_fw, > .wait_for_fw_loading_complete = pp_dpm_fw_loading_complete, > @@ -1604,4 +1628,5 @@ static int pp_set_mmhub_powergating_by_smu(void *handle) > .display_clock_voltage_request = pp_display_clock_voltage_request, > .get_display_mode_validation_clocks = > pp_get_display_mode_validation_clocks, > .set_mmhub_powergating_by_smu = pp_set_mmhub_powergating_by_smu, > + .notify_free_sync_change = pp_notify_free_sync_change, > }; > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx