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

[Lucas De Marchi]

On Fri, Oct 8, 2021 at 4:59 PM Andi Shyti <andi at etezian.org> wrote:
>
> From: Andi Shyti <andi.shyti at 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 at intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> Changelog:
> ----------
> 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).
>
>  drivers/gpu/drm/i915/gt/intel_gt_debugfs.c    | 42 ++++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_gt_debugfs.h    |  4 ++
>  drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 41 ++++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.h |  4 ++
>  drivers/gpu/drm/i915/i915_debugfs.c           | 43 +++----------------
>  5 files changed, 98 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c
> index 1fe19ccd27942..f712670993b68 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c
> @@ -13,6 +13,46 @@
>  #include "pxp/intel_pxp_debugfs.h"
>  #include "uc/intel_uc_debugfs.h"
>
> +int reset_show(void *data, u64 *val)
> +{
> +       struct intel_gt *gt = data;
> +       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 reset_store(void *data, u64 val)
> +{
> +       struct intel_gt *gt = data;
> +
> +       /* Flush any previous reset before applying for a new one */
> +       wait_event(gt->reset.queue,
> +                  !test_bit(I915_RESET_BACKOFF, &gt->reset.flags));
> +
> +       intel_gt_handle_error(gt, val, I915_ERROR_CAPTURE,
> +                             "Manually reset engine mask to %llx", val);
> +       return 0;
> +}
> +DEFINE_SIMPLE_ATTRIBUTE(reset_fops, reset_show, 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 +64,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 8b6fca09897ce..6bc4f044c23f3 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 reset_show(void *data, u64 *val);
> +int reset_store(void *data, u64 val);

We are trying to make the several parts of the driver self-contained.
Functions exposed by this header should keep the namespace...

So I think this would be intel_gt_debugfs_reset_show() /
intel_gt_debugfs_reset_store()
or something like that.

Also, since you still have a i915_wedged_set() function, here the first
parameter could be struct intel_gt * to make the interface clear.

> +
>  #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 5f84ad6026423..712c91d588eb3 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 __forcewake_user_open(struct intel_gt *gt)
> +{
> +       atomic_inc(&gt->user_wakeref);
> +       intel_gt_pm_get(gt);
> +       if (GRAPHICS_VER(gt->i915) >= 6)
> +               intel_uncore_forcewake_user_get(gt->uncore);
> +
> +       return 0;
> +}
> +
> +int __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(&gt->user_wakeref);
> +
> +       return 0;
> +}
> +
> +static int forcewake_user_open(struct inode *inode, struct file *file)
> +{
> +       struct intel_gt *gt = inode->i_private;
> +
> +       return __forcewake_user_open(gt);
> +}
> +
> +static int forcewake_user_release(struct inode *inode, struct file *file)
> +{
> +       struct intel_gt *gt = inode->i_private;
> +
> +       return __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 2b824289582be..fe306412b996d 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 __forcewake_user_open(struct intel_gt *gt);
> +int __forcewake_user_release(struct intel_gt *gt);

same thing here.

Other than those renames,

Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

thanks
Lucas De Marchi