| Message ID | 20230512235603.431386-1-vinay.belgaumkar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/guc/slpc: Disable rps_boost debugfs | expand |
On Fri, 12 May 2023 16:56:03 -0700, Vinay Belgaumkar wrote: > Hi Vinay, > rps_boost debugfs shows host turbo related info. This is not valid > when SLPC is enabled. A couple of thoughts about this. It appears people are know only about rps_boost_info and don't know about guc_slpc_info? So: a. Instead of hiding the rps_boost_info file do we need to print there saying "SLPC is enabled, go look at guc_slpc_info"? b. Or, even just call guc_slpc_info_show from rps_boost_show (so the two files will show the same SLPC information)? Ashutosh > guc_slpc_info already shows the number of boosts. Add num_waiters there > as well and disable rps_boost when SLPC is enabled. > > Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/7632 > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
On 5/12/2023 5:39 PM, Dixit, Ashutosh wrote: > On Fri, 12 May 2023 16:56:03 -0700, Vinay Belgaumkar wrote: > Hi Vinay, > >> rps_boost debugfs shows host turbo related info. This is not valid >> when SLPC is enabled. > A couple of thoughts about this. It appears people are know only about > rps_boost_info and don't know about guc_slpc_info? So: > > a. Instead of hiding the rps_boost_info file do we need to print there > saying "SLPC is enabled, go look at guc_slpc_info"? rps_boost_info has an eval() function which disables the interface when RPS is OFF. This is indeed the case here, so shouldn't we just follow that instead of trying to link the two? > > b. Or, even just call guc_slpc_info_show from rps_boost_show (so the two > files will show the same SLPC information)? slpc_info has a lot of other info like the SLPC state, not sure that matches up with the rps_boost_info name. Thanks, Vinay. > > Ashutosh > > >> guc_slpc_info already shows the number of boosts. Add num_waiters there >> as well and disable rps_boost when SLPC is enabled. >> >> Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/7632 >> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
On Mon, 15 May 2023 15:23:58 -0700, Belgaumkar, Vinay wrote: > > > On 5/12/2023 5:39 PM, Dixit, Ashutosh wrote: > > On Fri, 12 May 2023 16:56:03 -0700, Vinay Belgaumkar wrote: > > Hi Vinay, > > > >> rps_boost debugfs shows host turbo related info. This is not valid > >> when SLPC is enabled. > > A couple of thoughts about this. It appears people are know only about > > rps_boost_info and don't know about guc_slpc_info? So: > > > > a. Instead of hiding the rps_boost_info file do we need to print there > > saying "SLPC is enabled, go look at guc_slpc_info"? > rps_boost_info has an eval() function which disables the interface when RPS > is OFF. This is indeed the case here, so shouldn't we just follow that > instead of trying to link the two? > > > > b. Or, even just call guc_slpc_info_show from rps_boost_show (so the two > > files will show the same SLPC information)? > > slpc_info has a lot of other info like the SLPC state, not sure that > matches up with the rps_boost_info name. OK, I have asked in https://gitlab.freedesktop.org/drm/intel/-/issues/7632: @mattst88: is it acceptable to hide the /sys/kernel/debug/dri/0/i915_rps_boost_info file so that it doesn't cause confusion. And then user would have to go look at /sys/kernel/debug/dri/0/i915_guc_slpc_info or some such file when SLPC is being used? That's what the patch above is doing. Let's see what we hear from @mattst88. > > > >> guc_slpc_info already shows the number of boosts. Add num_waiters there > >> as well and disable rps_boost when SLPC is enabled. > >> > >> Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/7632 > >> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> Thanks. -- Ashutosh
On Mon, 15 May 2023 15:58:26 -0700, Dixit, Ashutosh wrote: > > On Mon, 15 May 2023 15:23:58 -0700, Belgaumkar, Vinay wrote: > > > > > > On 5/12/2023 5:39 PM, Dixit, Ashutosh wrote: > > > On Fri, 12 May 2023 16:56:03 -0700, Vinay Belgaumkar wrote: > > > Hi Vinay, > > > > > >> rps_boost debugfs shows host turbo related info. This is not valid > > >> when SLPC is enabled. > > > A couple of thoughts about this. It appears people are know only about > > > rps_boost_info and don't know about guc_slpc_info? So: > > > > > > a. Instead of hiding the rps_boost_info file do we need to print there > > > saying "SLPC is enabled, go look at guc_slpc_info"? > > rps_boost_info has an eval() function which disables the interface when RPS > > is OFF. This is indeed the case here, so shouldn't we just follow that > > instead of trying to link the two? > > > > > > b. Or, even just call guc_slpc_info_show from rps_boost_show (so the two > > > files will show the same SLPC information)? > > > > slpc_info has a lot of other info like the SLPC state, not sure that > > matches up with the rps_boost_info name. > > OK, I have asked in https://gitlab.freedesktop.org/drm/intel/-/issues/7632: > > @mattst88: is it acceptable to hide the > /sys/kernel/debug/dri/0/i915_rps_boost_info file so that it doesn't cause > confusion. And then user would have to go look at > /sys/kernel/debug/dri/0/i915_guc_slpc_info or some such file when SLPC is > being used? That's what the patch above is doing. > > Let's see what we hear from @mattst88. @mattst88 agreed on #intel-gfx IRC, so ok by me: Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com> > > > > > > >> guc_slpc_info already shows the number of boosts. Add num_waiters there > > >> as well and disable rps_boost when SLPC is enabled. > > >> > > >> Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/7632 > > >> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > > Thanks. > -- > Ashutosh
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c index 80dbbef86b1d..357e2f865727 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c @@ -539,7 +539,10 @@ static bool rps_eval(void *data) { struct intel_gt *gt = data; - return HAS_RPS(gt->i915); + if (intel_guc_slpc_is_used(>->uc.guc)) + return false; + else + return HAS_RPS(gt->i915); } DEFINE_INTEL_GT_DEBUGFS_ATTRIBUTE(rps_boost); diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c index 56dbba1ef668..01b75529311c 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c @@ -837,6 +837,8 @@ int intel_guc_slpc_print_info(struct intel_guc_slpc *slpc, struct drm_printer *p slpc_decode_min_freq(slpc)); drm_printf(p, "\twaitboosts: %u\n", slpc->num_boosts); + drm_printf(p, "\tBoosts outstanding: %u\n", + atomic_read(&slpc->num_waiters)); } }
rps_boost debugfs shows host turbo related info. This is not valid when SLPC is enabled. guc_slpc_info already shows the number of boosts. Add num_waiters there as well and disable rps_boost when SLPC is enabled. Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/7632 Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> --- drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 5 ++++- drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-)