[Intel-gfx] [PATCH 1/1] drm/i915/debugfs: New debugfs for display clock frequencies

[Jani Nikula]

On Fri, 14 Apr 2023, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
> On Wed, Apr 12, 2023 at 12:22:51PM +0300, Jani Nikula wrote:
>>On Wed, 12 Apr 2023, Bhanuprakash Modem <bhanuprakash.modem at intel.com> wrote:
>>> +
>>>  static ssize_t i915_displayport_test_active_write(struct file *file,
>>>  						  const char __user *ubuf,
>>>  						  size_t len, loff_t *offp)
>>> @@ -1065,6 +1080,7 @@ static const struct drm_info_list intel_display_debugfs_list[] = {
>>>  	{"i915_dp_mst_info", i915_dp_mst_info, 0},
>>>  	{"i915_ddb_info", i915_ddb_info, 0},
>>>  	{"i915_lpsp_status", i915_lpsp_status, 0},
>>> +	{"i915_disply_clock_info", i915_display_clock_info, 0},
>>
>>Maybe "i915_cdclk_info"? (Also, disply has a typo there.)
>
> hijacking this a little bit since I saw the new version of this commit
> applied without the display_ part. Should we consider moving all the
> display-related debugfs files to display/ directory?  I think that would
> make it clearer for the xe integration while also cleaning up a little
> bit the various files on i915. Downside would be synchronizing this with
> the tools reading those files. I guess it's only igt and CI that care about
> them?

While I agree in principle, I see potential for inflicting a lot of
paper cuts here.

We've moved individual files in the past, and it's generally been
fine. The pain is manageable. But seems like moving tons of files needs
to have some transition period with symlinks in kernel or igt checking
both places or something. Imagine bisecting a kernel bug using an igt
test, and needing two different igt builds depending on where the file
is.

On the other hand, maybe display/ directory is something that should be
reserved for drm to create. Anythin driver specific should be
prefixed. So either you'd have files named i915_display/* or
display/i915_*. Needs consideration.

I'm just leaning towards avoiding trouble at this time.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center