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

[Lucas De Marchi]

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?

thanks
Lucas De Marchi