[Intel-gfx] [PATCH] drm/i915: deduplicate frequency dump on debugfs

[Lucas De Marchi]

On Wed, Sep 08, 2021 at 07:14:00AM -0700, Lucas De Marchi wrote:
>On Wed, Sep 08, 2021 at 11:54:40AM +0300, Jani Nikula wrote:
>>On Tue, 07 Sep 2021, Lucas De Marchi <lucas.demarchi at intel.com> 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 at intel.com>
>>>---
>>> 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?

Something like the below:

renamed:    drivers/gpu/drm/i915/gt/debugfs_gt.c -> drivers/gpu/drm/i915/gt/intel_gt_debugfs.c
renamed:    drivers/gpu/drm/i915/gt/debugfs_gt.h -> drivers/gpu/drm/i915/gt/intel_gt_debugfs.h
renamed:    drivers/gpu/drm/i915/gt/debugfs_engines.c -> drivers/gpu/drm/i915/gt/intel_gt_engines_debugfs.c
renamed:    drivers/gpu/drm/i915/gt/debugfs_engines.h -> drivers/gpu/drm/i915/gt/intel_gt_engines_debugfs.h
renamed:    drivers/gpu/drm/i915/gt/debugfs_gt_pm.c -> drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
renamed:    drivers/gpu/drm/i915/gt/debugfs_gt_pm.h -> drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.h


and then rename the functions/macros in these files to follow th
filename

Lucas De Marchi

>
>thanks
>Lucas De Marchi