From: Lucas De Marchi <lucas.de.marchi@gmail.com> To: Andi Shyti <andi@etezian.org> Cc: Intel GFX <intel-gfx@lists.freedesktop.org>, DRI Devel <dri-devel@lists.freedesktop.org>, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>, Chris Wilson <chris@chris-wilson.co.uk>, Lucas De Marchi <lucas.demarchi@intel.com>, Andi Shyti <andi.shyti@intel.com>, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Subject: Re: [PATCH v2] drm/i915/gt: move remaining debugfs interfaces into gt Date: Thu, 7 Oct 2021 23:47:00 -0700 [thread overview] Message-ID: <CAKi4VAJ-HUR1=-VKeB0i21Q81i3aC00n2c-gp2zmw3ybeULUbw@mail.gmail.com> (raw) In-Reply-To: <20211007230903.4084-1-andi@etezian.org> On Thu, Oct 7, 2021 at 5:27 PM Andi Shyti <andi@etezian.org> wrote: > > From: Andi Shyti <andi.shyti@intel.com> > > The following interfaces: > > i915_wedged > i915_forcewake_user > i915_gem_interrupt > > are dependent on gt values. Put them inside gt/ and drop the > "i915_" prefix name. This would be the new structure: > > dri/0/gt > | > +-- forcewake_user > | > +-- interrupt_info > | > \-- reset > > For backwards compatibility with existing igt (and the slight > semantic difference between operating on the i915 abi entry > points and the deep gt info): > > dri/0 > | > +-- i915_wedged > | > \-- i915_forcewake_user > > remain at the top level. > > Signed-off-by: Andi Shyti <andi.shyti@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > Hi, > > 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. > > 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") Alternative would be to prepare igt already and then add a Test-with: in this patch series.... But I think it makes more sense to support both locations for some time and then later remove the previous one. Thanks Lucas De Marchi
WARNING: multiple messages have this Message-ID (diff)
From: Lucas De Marchi <lucas.de.marchi@gmail.com> To: Andi Shyti <andi@etezian.org> Cc: Intel GFX <intel-gfx@lists.freedesktop.org>, DRI Devel <dri-devel@lists.freedesktop.org>, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>, Chris Wilson <chris@chris-wilson.co.uk>, Lucas De Marchi <lucas.demarchi@intel.com>, Andi Shyti <andi.shyti@intel.com>, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/gt: move remaining debugfs interfaces into gt Date: Thu, 7 Oct 2021 23:47:00 -0700 [thread overview] Message-ID: <CAKi4VAJ-HUR1=-VKeB0i21Q81i3aC00n2c-gp2zmw3ybeULUbw@mail.gmail.com> (raw) In-Reply-To: <20211007230903.4084-1-andi@etezian.org> On Thu, Oct 7, 2021 at 5:27 PM Andi Shyti <andi@etezian.org> wrote: > > From: Andi Shyti <andi.shyti@intel.com> > > The following interfaces: > > i915_wedged > i915_forcewake_user > i915_gem_interrupt > > are dependent on gt values. Put them inside gt/ and drop the > "i915_" prefix name. This would be the new structure: > > dri/0/gt > | > +-- forcewake_user > | > +-- interrupt_info > | > \-- reset > > For backwards compatibility with existing igt (and the slight > semantic difference between operating on the i915 abi entry > points and the deep gt info): > > dri/0 > | > +-- i915_wedged > | > \-- i915_forcewake_user > > remain at the top level. > > Signed-off-by: Andi Shyti <andi.shyti@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > Hi, > > 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. > > 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") Alternative would be to prepare igt already and then add a Test-with: in this patch series.... But I think it makes more sense to support both locations for some time and then later remove the previous one. Thanks Lucas De Marchi
next prev parent reply other threads:[~2021-10-08 6:47 UTC|newest] Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-10-07 23:09 [PATCH v2] drm/i915/gt: move remaining debugfs interfaces into gt Andi Shyti 2021-10-07 23:09 ` [Intel-gfx] " Andi Shyti 2021-10-08 2:14 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gt: move remaining debugfs interfaces into gt (rev8) Patchwork 2021-10-08 2:47 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2021-10-08 3:57 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork 2021-10-08 6:47 ` Lucas De Marchi [this message] 2021-10-08 6:47 ` [Intel-gfx] [PATCH v2] drm/i915/gt: move remaining debugfs interfaces into gt Lucas De Marchi 2021-10-08 10:14 ` Andi Shyti 2021-10-08 10:14 ` [Intel-gfx] " Andi Shyti 2021-10-08 23:51 ` Lucas De Marchi 2021-10-08 23:51 ` Lucas De Marchi 2021-10-08 11:22 Andi Shyti 2021-10-08 11:27 ` Andi Shyti
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAKi4VAJ-HUR1=-VKeB0i21Q81i3aC00n2c-gp2zmw3ybeULUbw@mail.gmail.com' \ --to=lucas.de.marchi@gmail.com \ --cc=andi.shyti@intel.com \ --cc=andi@etezian.org \ --cc=chris@chris-wilson.co.uk \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=joonas.lahtinen@linux.intel.com \ --cc=lucas.demarchi@intel.com \ --cc=tvrtko.ursulin@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.