On Wed, Sep 08, 2021 at 11:54:40AM +0300, Jani Nikula wrote:
On Tue, 07 Sep 2021, Lucas De Marchi <lucas.demarchi@xxxxxxxxx> wrote:Although commit 9dd4b065446a ("drm/i915/gt: Move pm debug files into a gt aware debugfs") says it was moving debug files to gt/, the i915_frequency_info file was left behind and its implementation copied into drivers/gpu/drm/i915/gt/debugfs_gt_pm.c. Over time we had several patches having to change both places to keep them in sync (and some patches failing to do so). The initial idea was to remove i915_frequency_info, but there are user space tools using it. From a quick code search there are other scripts and test tools besides igt, so it's not simply updating igt to get rid of the older file. Here we export a function using drm_printer as parameter and make both show() implementations to call this same function. Aside from a few variable name differences, for i915_frequency_info this brings a few lines that were not previously printed: RP UP EI, RP UP THRESHOLD, RP DOWN THRESHOLD and RP DOWN EI. These came in as part of commit 9c878557b1eb ("drm/i915/gt: Use the RPM config register to determine clk frequencies"), which didn't change both places. Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> --- drivers/gpu/drm/i915/gt/debugfs_gt_pm.c | 127 ++++++------- drivers/gpu/drm/i915/gt/debugfs_gt_pm.h | 2 + drivers/gpu/drm/i915/i915_debugfs.c | 227 +----------------------- 3 files changed, 74 insertions(+), 282 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c b/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c index f6733f279890..6a27c011d0ff 100644 --- a/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c @@ -240,9 +240,8 @@ static int drpc_show(struct seq_file *m, void *unused) } DEFINE_GT_DEBUGFS_ATTRIBUTE(drpc); -static int frequency_show(struct seq_file *m, void *unused) +void debugfs_gt_pm_frequency_dump(struct intel_gt *gt, struct drm_printer *p)The debugfs prefix belongs to debugfs, and I don't think we should have non-static functions with that prefix. I know it's in line with what's currently in the file, and I've complained about it before, but apparently that hasn't been enough.
I was surprised by the prefix too. intel_gt_pm_debugfs.[hc] - would that be better or do you have another suggestion? thanks Lucas De Marchi
