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 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.