From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH 2/2] drm/xe/debugfs: Improve .show() helper for GT-based attributes
Date: Fri, 19 Sep 2025 17:27:05 -0400 [thread overview]
Message-ID: <aM3KqUnWloi38riK@intel.com> (raw)
In-Reply-To: <20250919160430.573-3-michal.wajdeczko@intel.com>
On Fri, Sep 19, 2025 at 06:04:30PM +0200, Michal Wajdeczko wrote:
> Like we did for tile-based attributes, introduce separate show()
> helper that implicitly takes an RPM reference prior to the call
> to the actual print() function. This translates into some savings.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> Here is a report from the scripts/bloat-o-meter:
>
> add/remove: 2/2 grow/shrink: 0/14 up/down: 114/-581 (-467)
> Function old new delta
> xe_gt_debugfs_show_with_rpm - 98 +98
> __pfx_xe_gt_debugfs_show_with_rpm - 16 +16
> __pfx_powergate_info 16 - -16
> register_save_restore 317 288 -29
> vecs_default_lrc 73 42 -31
> vcs_default_lrc 73 42 -31
> rcs_default_lrc 70 39 -31
> ccs_default_lrc 73 42 -31
> bcs_default_lrc 73 42 -31
> hw_engines 179 145 -34
> hwconfig 70 33 -37
> workarounds 66 26 -40
> tunings 66 26 -40
> topology 66 26 -40
> steering 66 26 -40
> pat 66 26 -40
> mocs 66 26 -40
> powergate_info 70 - -70
> Total: Before=2870434, After=2869967, chg -0.02%
> ---
> drivers/gpu/drm/xe/xe_gt_debugfs.c | 116 +++++++++++------------------
> drivers/gpu/drm/xe/xe_gt_debugfs.h | 1 +
> 2 files changed, 46 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> index e4eba91cb83d..b9176d4398e1 100644
> --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> @@ -35,6 +35,11 @@
> #include "xe_uc_debugfs.h"
> #include "xe_wa.h"
>
> +static struct xe_gt *node_to_gt(struct drm_info_node *node)
> +{
> + return node->dent->d_parent->d_inode->i_private;
> +}
> +
> /**
> * xe_gt_debugfs_simple_show - A show callback for struct drm_info_list
> * @m: the &seq_file
> @@ -77,8 +82,7 @@ int xe_gt_debugfs_simple_show(struct seq_file *m, void *data)
> {
> struct drm_printer p = drm_seq_file_printer(m);
> struct drm_info_node *node = m->private;
> - struct dentry *parent = node->dent->d_parent;
> - struct xe_gt *gt = parent->d_inode->i_private;
> + struct xe_gt *gt = node_to_gt(node);
> int (*print)(struct xe_gt *, struct drm_printer *) = node->info_ent->data;
>
> if (WARN_ON(!print))
> @@ -87,15 +91,36 @@ int xe_gt_debugfs_simple_show(struct seq_file *m, void *data)
> return print(gt, &p);
> }
>
> +/**
> + * xe_gt_debugfs_show_with_rpm - A show callback for struct drm_info_list
> + * @m: the &seq_file
> + * @data: data used by the drm debugfs helpers
> + *
> + * Similar to xe_gt_debugfs_simple_show() but implicitly takes a RPM ref.
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int xe_gt_debugfs_show_with_rpm(struct seq_file *m, void *data)
> +{
> + struct drm_info_node *node = m->private;
> + struct xe_gt *gt = node_to_gt(node);
> + struct xe_device *xe = gt_to_xe(gt);
> + int ret;
> +
> + xe_pm_runtime_get(xe);
> + ret = xe_gt_debugfs_simple_show(m, data);
> + xe_pm_runtime_put(xe);
> +
> + return ret;
> +}
> +
> static int hw_engines(struct xe_gt *gt, struct drm_printer *p)
> {
> - struct xe_device *xe = gt_to_xe(gt);
> struct xe_hw_engine *hwe;
> enum xe_hw_engine_id id;
> unsigned int fw_ref;
> int ret = 0;
>
> - xe_pm_runtime_get(xe);
> fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) {
> ret = -ETIMEDOUT;
> @@ -107,37 +132,19 @@ static int hw_engines(struct xe_gt *gt, struct drm_printer *p)
>
> fw_put:
> xe_force_wake_put(gt_to_fw(gt), fw_ref);
> - xe_pm_runtime_put(xe);
> -
> - return ret;
> -}
> -
> -static int powergate_info(struct xe_gt *gt, struct drm_printer *p)
I believe this should be removed in a separate patch.
It confused me when the number of rpm removals and the number of
function changed below didn't match.
> -{
> - int ret;
> -
> - xe_pm_runtime_get(gt_to_xe(gt));
> - ret = xe_gt_idle_pg_print(gt, p);
> - xe_pm_runtime_put(gt_to_xe(gt));
>
> return ret;
> }
>
> static int topology(struct xe_gt *gt, struct drm_printer *p)
> {
> - xe_pm_runtime_get(gt_to_xe(gt));
> xe_gt_topology_dump(gt, p);
> - xe_pm_runtime_put(gt_to_xe(gt));
> -
> return 0;
> }
>
> static int steering(struct xe_gt *gt, struct drm_printer *p)
> {
> - xe_pm_runtime_get(gt_to_xe(gt));
> xe_gt_mcr_steering_dump(gt, p);
> - xe_pm_runtime_put(gt_to_xe(gt));
> -
> return 0;
> }
>
> @@ -146,8 +153,6 @@ static int register_save_restore(struct xe_gt *gt, struct drm_printer *p)
> struct xe_hw_engine *hwe;
> enum xe_hw_engine_id id;
>
> - xe_pm_runtime_get(gt_to_xe(gt));
> -
> xe_reg_sr_dump(>->reg_sr, p);
> drm_printf(p, "\n");
>
> @@ -165,98 +170,66 @@ static int register_save_restore(struct xe_gt *gt, struct drm_printer *p)
> for_each_hw_engine(hwe, gt, id)
> xe_reg_whitelist_dump(&hwe->reg_whitelist, p);
>
> - xe_pm_runtime_put(gt_to_xe(gt));
> -
> return 0;
> }
>
> static int workarounds(struct xe_gt *gt, struct drm_printer *p)
> {
> - xe_pm_runtime_get(gt_to_xe(gt));
> xe_wa_dump(gt, p);
> - xe_pm_runtime_put(gt_to_xe(gt));
> -
> return 0;
> }
>
> static int tunings(struct xe_gt *gt, struct drm_printer *p)
> {
> - xe_pm_runtime_get(gt_to_xe(gt));
> xe_tuning_dump(gt, p);
> - xe_pm_runtime_put(gt_to_xe(gt));
> -
> return 0;
> }
>
> static int pat(struct xe_gt *gt, struct drm_printer *p)
> {
> - xe_pm_runtime_get(gt_to_xe(gt));
> xe_pat_dump(gt, p);
> - xe_pm_runtime_put(gt_to_xe(gt));
> -
> return 0;
> }
>
> static int mocs(struct xe_gt *gt, struct drm_printer *p)
> {
> - xe_pm_runtime_get(gt_to_xe(gt));
> xe_mocs_dump(gt, p);
> - xe_pm_runtime_put(gt_to_xe(gt));
> -
> return 0;
> }
>
> static int rcs_default_lrc(struct xe_gt *gt, struct drm_printer *p)
> {
> - xe_pm_runtime_get(gt_to_xe(gt));
> xe_lrc_dump_default(p, gt, XE_ENGINE_CLASS_RENDER);
> - xe_pm_runtime_put(gt_to_xe(gt));
> -
> return 0;
> }
>
> static int ccs_default_lrc(struct xe_gt *gt, struct drm_printer *p)
> {
> - xe_pm_runtime_get(gt_to_xe(gt));
> xe_lrc_dump_default(p, gt, XE_ENGINE_CLASS_COMPUTE);
> - xe_pm_runtime_put(gt_to_xe(gt));
> -
> return 0;
> }
>
> static int bcs_default_lrc(struct xe_gt *gt, struct drm_printer *p)
> {
> - xe_pm_runtime_get(gt_to_xe(gt));
> xe_lrc_dump_default(p, gt, XE_ENGINE_CLASS_COPY);
> - xe_pm_runtime_put(gt_to_xe(gt));
> -
> return 0;
> }
>
> static int vcs_default_lrc(struct xe_gt *gt, struct drm_printer *p)
> {
> - xe_pm_runtime_get(gt_to_xe(gt));
> xe_lrc_dump_default(p, gt, XE_ENGINE_CLASS_VIDEO_DECODE);
> - xe_pm_runtime_put(gt_to_xe(gt));
> -
> return 0;
> }
>
> static int vecs_default_lrc(struct xe_gt *gt, struct drm_printer *p)
> {
> - xe_pm_runtime_get(gt_to_xe(gt));
> xe_lrc_dump_default(p, gt, XE_ENGINE_CLASS_VIDEO_ENHANCE);
> - xe_pm_runtime_put(gt_to_xe(gt));
> -
> return 0;
> }
>
> static int hwconfig(struct xe_gt *gt, struct drm_printer *p)
> {
> - xe_pm_runtime_get(gt_to_xe(gt));
> xe_guc_hwconfig_dump(>->uc.guc, p);
> - xe_pm_runtime_put(gt_to_xe(gt));
> -
> return 0;
> }
>
> @@ -266,25 +239,26 @@ static int hwconfig(struct xe_gt *gt, struct drm_printer *p)
> * - without access to the PF specific data
> */
> static const struct drm_info_list vf_safe_debugfs_list[] = {
> - {"topology", .show = xe_gt_debugfs_simple_show, .data = topology},
> - {"register-save-restore", .show = xe_gt_debugfs_simple_show, .data = register_save_restore},
> - {"workarounds", .show = xe_gt_debugfs_simple_show, .data = workarounds},
> - {"tunings", .show = xe_gt_debugfs_simple_show, .data = tunings},
> - {"default_lrc_rcs", .show = xe_gt_debugfs_simple_show, .data = rcs_default_lrc},
> - {"default_lrc_ccs", .show = xe_gt_debugfs_simple_show, .data = ccs_default_lrc},
> - {"default_lrc_bcs", .show = xe_gt_debugfs_simple_show, .data = bcs_default_lrc},
> - {"default_lrc_vcs", .show = xe_gt_debugfs_simple_show, .data = vcs_default_lrc},
> - {"default_lrc_vecs", .show = xe_gt_debugfs_simple_show, .data = vecs_default_lrc},
> - {"hwconfig", .show = xe_gt_debugfs_simple_show, .data = hwconfig},
> + { "topology", .show = xe_gt_debugfs_show_with_rpm, .data = topology },
> + { "register-save-restore",
> + .show = xe_gt_debugfs_show_with_rpm, .data = register_save_restore },
> + { "workarounds", .show = xe_gt_debugfs_show_with_rpm, .data = workarounds },
> + { "tunings", .show = xe_gt_debugfs_show_with_rpm, .data = tunings },
> + { "default_lrc_rcs", .show = xe_gt_debugfs_show_with_rpm, .data = rcs_default_lrc },
> + { "default_lrc_ccs", .show = xe_gt_debugfs_show_with_rpm, .data = ccs_default_lrc },
> + { "default_lrc_bcs", .show = xe_gt_debugfs_show_with_rpm, .data = bcs_default_lrc },
> + { "default_lrc_vcs", .show = xe_gt_debugfs_show_with_rpm, .data = vcs_default_lrc },
> + { "default_lrc_vecs", .show = xe_gt_debugfs_show_with_rpm, .data = vecs_default_lrc },
> + { "hwconfig", .show = xe_gt_debugfs_show_with_rpm, .data = hwconfig },
> };
>
> /* everything else should be added here */
> static const struct drm_info_list pf_only_debugfs_list[] = {
> - {"hw_engines", .show = xe_gt_debugfs_simple_show, .data = hw_engines},
> - {"mocs", .show = xe_gt_debugfs_simple_show, .data = mocs},
> - {"pat", .show = xe_gt_debugfs_simple_show, .data = pat},
> - {"powergate_info", .show = xe_gt_debugfs_simple_show, .data = powergate_info},
> - {"steering", .show = xe_gt_debugfs_simple_show, .data = steering},
> + { "hw_engines", .show = xe_gt_debugfs_show_with_rpm, .data = hw_engines },
> + { "mocs", .show = xe_gt_debugfs_show_with_rpm, .data = mocs },
> + { "pat", .show = xe_gt_debugfs_show_with_rpm, .data = pat },
> + { "powergate_info", .show = xe_gt_debugfs_show_with_rpm, .data = xe_gt_idle_pg_print },
> + { "steering", .show = xe_gt_debugfs_show_with_rpm, .data = steering },
> };
>
> static ssize_t write_to_gt_call(const char __user *userbuf, size_t count, loff_t *ppos,
> diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.h b/drivers/gpu/drm/xe/xe_gt_debugfs.h
> index 05a6cc93c78c..32ee3264051b 100644
> --- a/drivers/gpu/drm/xe/xe_gt_debugfs.h
> +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.h
> @@ -11,5 +11,6 @@ struct xe_gt;
>
> void xe_gt_debugfs_register(struct xe_gt *gt);
> int xe_gt_debugfs_simple_show(struct seq_file *m, void *data);
> +int xe_gt_debugfs_show_with_rpm(struct seq_file *m, void *data);
>
> #endif
> --
> 2.47.1
>
next prev parent reply other threads:[~2025-09-19 21:27 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-19 16:04 [PATCH 0/2] drm/xe/debugfs: Small improvements Michal Wajdeczko
2025-09-19 16:04 ` [PATCH 1/2] drm/xe/debugfs: Make ggtt file per-tile Michal Wajdeczko
2025-09-19 21:22 ` Rodrigo Vivi
2025-09-19 16:04 ` [PATCH 2/2] drm/xe/debugfs: Improve .show() helper for GT-based attributes Michal Wajdeczko
2025-09-19 21:27 ` Rodrigo Vivi [this message]
2025-09-23 12:49 ` Rodrigo Vivi
2025-09-19 16:11 ` ✓ CI.KUnit: success for drm/xe/debugfs: Small improvements Patchwork
2025-09-19 16:54 ` ✓ Xe.CI.BAT: " Patchwork
2025-09-20 0:52 ` ✗ Xe.CI.Full: failure " Patchwork
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=aM3KqUnWloi38riK@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=michal.wajdeczko@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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox