Message ID | 20211012221738.16029-1-andi@etezian.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v5] drm/i915/gt: move remaining debugfs interfaces into gt | expand |
Hi, sorry, just forgot to add the changelog On Wed, Oct 13, 2021 at 12:17:38AM +0200, Andi Shyti wrote: > From: Andi Shyti <andi.shyti@linux.intel.com> > > The following interfaces: > > i915_wedged > i915_forcewake_user > > 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 > | > \-- 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@linux.intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- Changelog: ---------- v4 -> v5: https://patchwork.freedesktop.org/patch/458293/ * rename static functions exposed to header files so that they can keep a coherent namespace (thanks Lucas!) * add Lucas r-b. v3 -> v4: https://patchwork.freedesktop.org/patch/458225/ * remove the unnecessary interrupt_info_show() information. They were already removed here by Chris: cf977e18610e6 ("drm/i915/gem: Spring clean debugfs") v2 -> v3: https://patchwork.freedesktop.org/patch/458108/ * keep the original interfaces as they were (thanks Chris) but implement the functionality inside the gt. The upper level files will call the gt functions (thanks Lucas). v1 -> v2: https://patchwork.freedesktop.org/patch/456652/ * keep the original interfaces intact (thanks Chris). Andi
On Wed, Oct 13, 2021 at 12:17:38AM +0200, Andi Shyti wrote: >From: Andi Shyti <andi.shyti@linux.intel.com> > >The following interfaces: > > i915_wedged > i915_forcewake_user > >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 > | > \-- 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@linux.intel.com> >Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >Cc: Chris Wilson <chris@chris-wilson.co.uk> >Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> do you want me to push this? Lucas De Marchi
Hi Lucas, On Wed, Oct 13, 2021 at 05:04:27PM -0700, Lucas De Marchi wrote: > On Wed, Oct 13, 2021 at 12:17:38AM +0200, Andi Shyti wrote: > > From: Andi Shyti <andi.shyti@linux.intel.com> > > > > The following interfaces: > > > > i915_wedged > > i915_forcewake_user > > > > 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 > > | > > \-- 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@linux.intel.com> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> > > do you want me to push this? yes, please. Thanks, Andi
On Thu, Oct 14, 2021 at 02:11:34AM +0200, Andi Shyti wrote: >Hi Lucas, > >On Wed, Oct 13, 2021 at 05:04:27PM -0700, Lucas De Marchi wrote: >> On Wed, Oct 13, 2021 at 12:17:38AM +0200, Andi Shyti wrote: >> > From: Andi Shyti <andi.shyti@linux.intel.com> >> > >> > The following interfaces: >> > >> > i915_wedged >> > i915_forcewake_user >> > >> > 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 >> > | >> > \-- 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@linux.intel.com> >> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> > Cc: Chris Wilson <chris@chris-wilson.co.uk> >> > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> >> >> do you want me to push this? > >yes, please. done, thanks. Now, about igt: eventually we need to update it to use the gt debugfs file. Is this something you have already or is it something we are waiting on multi-gt to land? Lucas De Marchi > >Thanks, >Andi
Hi Lucas, > > > > The following interfaces: > > > > > > > > i915_wedged > > > > i915_forcewake_user > > > > > > > > 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 > > > > | > > > > \-- 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@linux.intel.com> > > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > > > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> > > > > > > do you want me to push this? > > > > yes, please. > > done, thanks. Thanks! > Now, about igt: eventually we need to update it to use the gt > debugfs file. Is this something you have already or is it something > we are waiting on multi-gt to land? There is some work done in igt but it's all around multitile. I think it's better to wait for the multitile to land and then update igt. In any case, at the current state, igt shouldn't be affected. Thanks again, Andi
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c index 1fe19ccd2794..f103664b71d4 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c @@ -13,6 +13,59 @@ #include "pxp/intel_pxp_debugfs.h" #include "uc/intel_uc_debugfs.h" +int intel_gt_debugfs_reset_show(struct intel_gt *gt, u64 *val) +{ + int ret = intel_gt_terminally_wedged(gt); + + switch (ret) { + case -EIO: + *val = 1; + return 0; + case 0: + *val = 0; + return 0; + default: + return ret; + } +} + +int intel_gt_debugfs_reset_store(struct intel_gt *gt, u64 val) +{ + /* Flush any previous reset before applying for a new one */ + wait_event(gt->reset.queue, + !test_bit(I915_RESET_BACKOFF, >->reset.flags)); + + intel_gt_handle_error(gt, val, I915_ERROR_CAPTURE, + "Manually reset engine mask to %llx", val); + return 0; +} + +/* + * keep the interface clean where the first parameter + * is a 'struct intel_gt *' instead of 'void *' + */ +static int __intel_gt_debugfs_reset_show(void *data, u64 *val) +{ + return intel_gt_debugfs_reset_show(data, val); +} + +static int __intel_gt_debugfs_reset_store(void *data, u64 val) +{ + return intel_gt_debugfs_reset_store(data, val); +} + +DEFINE_SIMPLE_ATTRIBUTE(reset_fops, __intel_gt_debugfs_reset_show, + __intel_gt_debugfs_reset_store, "%llu\n"); + +static void gt_debugfs_register(struct intel_gt *gt, struct dentry *root) +{ + static const struct intel_gt_debugfs_file files[] = { + { "reset", &reset_fops, NULL }, + }; + + intel_gt_debugfs_register_files(root, files, ARRAY_SIZE(files), gt); +} + void intel_gt_debugfs_register(struct intel_gt *gt) { struct dentry *root; @@ -24,6 +77,8 @@ void intel_gt_debugfs_register(struct intel_gt *gt) if (IS_ERR(root)) return; + gt_debugfs_register(gt, root); + intel_gt_engines_debugfs_register(gt, root); intel_gt_pm_debugfs_register(gt, root); intel_sseu_debugfs_register(gt, root); diff --git a/drivers/gpu/drm/i915/gt/intel_gt_debugfs.h b/drivers/gpu/drm/i915/gt/intel_gt_debugfs.h index 8b6fca09897c..e307ceb99031 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_debugfs.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_debugfs.h @@ -35,4 +35,8 @@ void intel_gt_debugfs_register_files(struct dentry *root, const struct intel_gt_debugfs_file *files, unsigned long count, void *data); +/* functions that need to be accessed by the upper level non-gt interfaces */ +int intel_gt_debugfs_reset_show(struct intel_gt *gt, u64 *val); +int intel_gt_debugfs_reset_store(struct intel_gt *gt, u64 val); + #endif /* INTEL_GT_DEBUGFS_H */ diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c index 5f84ad602642..0bc1454f38dd 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c @@ -19,6 +19,46 @@ #include "intel_sideband.h" #include "intel_uncore.h" +int intel_gt_pm_debugfs_forcewake_user_open(struct intel_gt *gt) +{ + atomic_inc(>->user_wakeref); + intel_gt_pm_get(gt); + if (GRAPHICS_VER(gt->i915) >= 6) + intel_uncore_forcewake_user_get(gt->uncore); + + return 0; +} + +int intel_gt_pm_debugfs_forcewake_user_release(struct intel_gt *gt) +{ + if (GRAPHICS_VER(gt->i915) >= 6) + intel_uncore_forcewake_user_put(gt->uncore); + intel_gt_pm_put(gt); + atomic_dec(>->user_wakeref); + + return 0; +} + +static int forcewake_user_open(struct inode *inode, struct file *file) +{ + struct intel_gt *gt = inode->i_private; + + return intel_gt_pm_debugfs_forcewake_user_open(gt); +} + +static int forcewake_user_release(struct inode *inode, struct file *file) +{ + struct intel_gt *gt = inode->i_private; + + return intel_gt_pm_debugfs_forcewake_user_release(gt); +} + +static const struct file_operations forcewake_user_fops = { + .owner = THIS_MODULE, + .open = forcewake_user_open, + .release = forcewake_user_release, +}; + static int fw_domains_show(struct seq_file *m, void *data) { struct intel_gt *gt = m->private; @@ -627,6 +667,7 @@ void intel_gt_pm_debugfs_register(struct intel_gt *gt, struct dentry *root) { "drpc", &drpc_fops, NULL }, { "frequency", &frequency_fops, NULL }, { "forcewake", &fw_domains_fops, NULL }, + { "forcewake_user", &forcewake_user_fops, NULL}, { "llc", &llc_fops, llc_eval }, { "rps_boost", &rps_boost_fops, rps_eval }, }; diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.h b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.h index 2b824289582b..a8457887ec65 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.h @@ -13,4 +13,8 @@ struct drm_printer; void intel_gt_pm_debugfs_register(struct intel_gt *gt, struct dentry *root); void intel_gt_pm_frequency_dump(struct intel_gt *gt, struct drm_printer *m); +/* functions that need to be accessed by the upper level non-gt interfaces */ +int intel_gt_pm_debugfs_forcewake_user_open(struct intel_gt *gt); +int intel_gt_pm_debugfs_forcewake_user_release(struct intel_gt *gt); + #endif /* INTEL_GT_PM_DEBUGFS_H */ diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index fdbd46ff59e0..636cc3cf88be 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -35,6 +35,7 @@ #include "gt/intel_gt.h" #include "gt/intel_gt_buffer_pool.h" #include "gt/intel_gt_clock_utils.h" +#include "gt/intel_gt_debugfs.h" #include "gt/intel_gt_pm.h" #include "gt/intel_gt_pm_debugfs.h" #include "gt/intel_gt_requests.h" @@ -554,36 +555,18 @@ static int i915_wa_registers(struct seq_file *m, void *unused) return 0; } -static int -i915_wedged_get(void *data, u64 *val) +static int i915_wedged_get(void *data, u64 *val) { struct drm_i915_private *i915 = data; - int ret = intel_gt_terminally_wedged(&i915->gt); - switch (ret) { - case -EIO: - *val = 1; - return 0; - case 0: - *val = 0; - return 0; - default: - return ret; - } + return intel_gt_debugfs_reset_show(&i915->gt, val); } -static int -i915_wedged_set(void *data, u64 val) +static int i915_wedged_set(void *data, u64 val) { struct drm_i915_private *i915 = data; - /* Flush any previous reset before applying for a new one */ - wait_event(i915->gt.reset.queue, - !test_bit(I915_RESET_BACKOFF, &i915->gt.reset.flags)); - - intel_gt_handle_error(&i915->gt, val, I915_ERROR_CAPTURE, - "Manually set wedged engine mask = %llx", val); - return 0; + return intel_gt_debugfs_reset_store(&i915->gt, val); } DEFINE_SIMPLE_ATTRIBUTE(i915_wedged_fops, @@ -728,27 +711,15 @@ static int i915_sseu_status(struct seq_file *m, void *unused) static int i915_forcewake_open(struct inode *inode, struct file *file) { struct drm_i915_private *i915 = inode->i_private; - struct intel_gt *gt = &i915->gt; - - atomic_inc(>->user_wakeref); - intel_gt_pm_get(gt); - if (GRAPHICS_VER(i915) >= 6) - intel_uncore_forcewake_user_get(gt->uncore); - return 0; + return intel_gt_pm_debugfs_forcewake_user_open(&i915->gt); } static int i915_forcewake_release(struct inode *inode, struct file *file) { struct drm_i915_private *i915 = inode->i_private; - struct intel_gt *gt = &i915->gt; - if (GRAPHICS_VER(i915) >= 6) - intel_uncore_forcewake_user_put(&i915->uncore); - intel_gt_pm_put(gt); - atomic_dec(>->user_wakeref); - - return 0; + return intel_gt_pm_debugfs_forcewake_user_release(&i915->gt); } static const struct file_operations i915_forcewake_fops = {