[Intel-gfx] [PATCH v2] drm/i915/gt: move remaining debugfs interfaces into gt

[Lucas De Marchi]

On Fri, Oct 8, 2021 at 3:14 AM Andi Shyti <andi.shyti at intel.com> wrote:
>
> Hi Lucas,
>
> > > I am reproposing this patch exactly as it was proposed initially
> > > where the original interfaces are kept where they have been
> > > originally placed. It might generate some duplicated code but,
> > > well, it's debugfs and I don't see any issue. In the future we
> > > can transform the upper interfaces to act upon all the GTs and
> > > provide information from all the GTs. This is, for example, how
> > > the sysfs interfaces will act.
> >
> > NACK. We've made this mistake in the past for other debugfs files.
> > We don't want to do it again just to maintain 2 separate places for
> > one year and then finally realize we want to merge them.
>
> In my opinion it's all about what mistake you like the most
> because until we will have multi-gt support in upstream all the
> patches come with the "promise" of a follow-up and maintenance
> cost.

no. If you put the implementation in a single place, later you only have the
decision on what to do with the per-device entrypoint:

- should we remove it once igt is converted?
- should we make it iterate all gts?
- should we make it mean root tile?

Then you take the action that is needed and decide it per interface.
Here you are leaving behind a lot of code that we will need to maintain
until there is support for such a thing.

It already happened once: we needed to maintain that duplicated code
for over a year with multiple patches changing them (or failing to do so).

>
> > > The reason I removed them in V1 is because igt as only user is
> > > not a strong reason to keep duplicated code, but as Chris
> > > suggested offline:
> > >
> > > "It's debugfs, igt is the primary consumer. CI has to be bridged over
> > > changes to the interfaces it is using in any case, as you want
> > > comparable results before/after the patches land.
> >
> > That doesn't mean you have to copy and paste it. It may mean you
> > do the implementation in one of them and the other calls that implementation.
> > See how I did the deduplication in commit d0c560316d6f ("drm/i915:
> > deduplicate frequency dump on debugfs")
>
> In this case, from a user perspective, which gt is the interface
> affecting? is it affecting all the system? or gt 0, 1...? Does
> the user know? The maintenance cost is that later you will need
> to use for_each_gt and make all those interfaces multitile, and
> this would be your "promise".

multi-gt is irrelevant here.  This patch without any "promise" should do
what the commit message says: *move*. The only reason to keep
the old entrypoint around is because it's missing the igt conversion. If
you are going to support a per-device entrypoint and do for_each_gt(),
or do a symlink to the root tile, or whatever, it isn't very relevant
to this patch.
Right now we have just a single directory, gt.

Lucas De Marchi