[PATCH v2 2/3] drm/xe: Move LRC snapshot capture to HW engine.
Souza, Jose
jose.souza at intel.com
Mon Feb 26 18:48:42 UTC 2024
On Mon, 2024-02-26 at 16:59 +0100, Maarten Lankhorst wrote:
> From: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>
> We want to start dumping status pages. Since we created some functions
> to dump LRC, I think it makes sense to move it to HW engine section.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>
> --
> XXX: I've used a linear mapping for HW Engine to LRC.
> LRC[0] belonging to lowest HW engine instance, etc..
> Is this correct for parallel submissions?
> ---
> drivers/gpu/drm/xe/xe_devcoredump.c | 19 +++++++++++++++++--
> drivers/gpu/drm/xe/xe_devcoredump_types.h | 6 ++++++
> drivers/gpu/drm/xe/xe_guc_submit.c | 21 ---------------------
> drivers/gpu/drm/xe/xe_guc_submit_types.h | 3 ---
> 4 files changed, 23 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
> index 68d3d623a05b..7002e3b698f2 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump.c
> +++ b/drivers/gpu/drm/xe/xe_devcoredump.c
> @@ -16,6 +16,7 @@
> #include "xe_guc_ct.h"
> #include "xe_guc_submit.h"
> #include "xe_hw_engine.h"
> +#include "xe_lrc.h"
> #include "xe_sched_job.h"
> #include "xe_vm.h"
>
> @@ -113,10 +114,20 @@ static ssize_t xe_devcoredump_read(char *buffer, loff_t offset,
> xe_sched_job_snapshot_print(coredump->snapshot.job, &p);
>
> drm_printf(&p, "\n**** HW Engines ****\n");
> - for (i = 0; i < XE_NUM_HW_ENGINES; i++)
> + for (i = 0; i < XE_NUM_HW_ENGINES; i++) {
> if (coredump->snapshot.hwe[i])
> xe_hw_engine_snapshot_print(coredump->snapshot.hwe[i],
> &p);
> + }
> +
> + drm_puts(&p, "\n**** LRC ****\n");
> + for (i = 0; i < ARRAY_SIZE(coredump->snapshot.lrc); i++) {
> + if (!coredump->snapshot.lrc[i])
> + continue;
> + drm_printf(&p, "LRC %i\n", i);
> + xe_lrc_snapshot_print(coredump->snapshot.lrc[i], &p);
We would need information about what class and width this LRC is so UMD can do the mapping.
Taking the options I guess it would be better keep it inside where it was(**** GuC CT **** -> GuC ID), sorry for asking it to be moved.
If in future the is a replacement for GuC or something like that the LRC functions can be called from the section that prints the information about
this new section.
> + }
> +
> if (coredump->snapshot.vm) {
> drm_printf(&p, "\n**** VM state ****\n");
> xe_vm_snapshot_print(coredump->snapshot.vm, &p);
> @@ -139,9 +150,11 @@ static void xe_devcoredump_free(void *data)
> xe_guc_ct_snapshot_free(coredump->snapshot.ct);
> xe_guc_exec_queue_snapshot_free(coredump->snapshot.ge);
> xe_sched_job_snapshot_free(coredump->snapshot.job);
> - for (i = 0; i < XE_NUM_HW_ENGINES; i++)
> + for (i = 0; i < XE_NUM_HW_ENGINES; i++) {
> + xe_lrc_snapshot_free(coredump->snapshot.lrc[i]);
> if (coredump->snapshot.hwe[i])
> xe_hw_engine_snapshot_free(coredump->snapshot.hwe[i]);
> + }
> xe_vm_snapshot_free(coredump->snapshot.vm);
>
> /* To prevent stale data on next snapshot, clear everything */
> @@ -195,6 +208,8 @@ static void devcoredump_snapshot(struct xe_devcoredump *coredump,
> }
> coredump->snapshot.hwe[id] = xe_hw_engine_snapshot_capture(hwe);
> }
> + for (i = 0; i < q->width; i++)
> + coredump->snapshot.lrc[i] = xe_lrc_snapshot_capture(&q->lrc[i]);
You are printing lrc with 'for (i = 0; i < ARRAY_SIZE(coredump->snapshot.lrc); i++) {', freeing with 'for (i = 0; i < XE_NUM_HW_ENGINES; i++) {' and
capturing with 'for (i = 0; i < q->width; i++)', it should be a similar method not 3 different.
But keeping it the previous section it would not be a issue.
>
> if (ss->vm)
> queue_work(system_unbound_wq, &ss->work);
> diff --git a/drivers/gpu/drm/xe/xe_devcoredump_types.h b/drivers/gpu/drm/xe/xe_devcoredump_types.h
> index 6f654b63c7f1..d56ac20e2ca9 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump_types.h
> +++ b/drivers/gpu/drm/xe/xe_devcoredump_types.h
> @@ -14,6 +14,8 @@
> struct xe_device;
> struct xe_gt;
>
> +struct xe_lrc_snapshot;
> +
> /**
> * struct xe_devcoredump_snapshot - Crash snapshot
> *
> @@ -40,6 +42,10 @@ struct xe_devcoredump_snapshot {
>
> /** @hwe: HW Engine snapshot array */
> struct xe_hw_engine_snapshot *hwe[XE_NUM_HW_ENGINES];
> +
> + /** @lrc: Snapshot of each LRC */
> + struct xe_lrc_snapshot *lrc[XE_NUM_HW_ENGINES];
> +
> /** @job: Snapshot of job state */
> struct xe_sched_job_snapshot *job;
> /** @vm: Snapshot of VM state */
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 7348689ec5ae..66dc561336df 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -1814,17 +1814,6 @@ xe_guc_exec_queue_snapshot_capture(struct xe_sched_job *job)
> snapshot->sched_props.preempt_timeout_us =
> q->sched_props.preempt_timeout_us;
>
> - snapshot->lrc = kmalloc_array(q->width, sizeof(struct xe_lrc_snapshot *),
> - GFP_ATOMIC);
> -
> - if (snapshot->lrc) {
> - for (i = 0; i < q->width; ++i) {
> - struct xe_lrc *lrc = q->lrc + i;
> -
> - snapshot->lrc[i] = xe_lrc_snapshot_capture(lrc);
> - }
> - }
> -
> snapshot->schedule_state = atomic_read(&q->guc->state);
> snapshot->exec_queue_flags = q->flags;
>
> @@ -1886,10 +1875,6 @@ xe_guc_exec_queue_snapshot_print(struct xe_guc_submit_exec_queue_snapshot *snaps
> snapshot->sched_props.timeslice_us);
> drm_printf(p, "\tPreempt timeout: %u (us)\n",
> snapshot->sched_props.preempt_timeout_us);
> -
> - for (i = 0; snapshot->lrc && i < snapshot->width; ++i)
> - xe_lrc_snapshot_print(snapshot->lrc[i], p);
> -
> drm_printf(p, "\tSchedule State: 0x%x\n", snapshot->schedule_state);
> drm_printf(p, "\tFlags: 0x%lx\n", snapshot->exec_queue_flags);
>
> @@ -1914,15 +1899,9 @@ xe_guc_exec_queue_snapshot_print(struct xe_guc_submit_exec_queue_snapshot *snaps
> */
> void xe_guc_exec_queue_snapshot_free(struct xe_guc_submit_exec_queue_snapshot *snapshot)
> {
> - int i;
> if (!snapshot)
> return;
>
> - if (snapshot->lrc) {
> - for (i = 0; i < snapshot->width; i++)
> - xe_lrc_snapshot_free(snapshot->lrc[i]);
> - kfree(snapshot->lrc);
> - }
> kfree(snapshot->pending_list);
> kfree(snapshot);
> }
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit_types.h b/drivers/gpu/drm/xe/xe_guc_submit_types.h
> index dc7456c34583..92a7b7bcb899 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit_types.h
> +++ b/drivers/gpu/drm/xe/xe_guc_submit_types.h
> @@ -97,9 +97,6 @@ struct xe_guc_submit_exec_queue_snapshot {
> u32 preempt_timeout_us;
> } sched_props;
>
> - /** @lrc: LRC Snapshot */
> - struct xe_lrc_snapshot **lrc;
> -
> /** @schedule_state: Schedule State at the moment of Crash */
> u32 schedule_state;
> /** @exec_queue_flags: Flags of the faulty exec_queue */
More information about the Intel-xe
mailing list