Re: [Intel-xe] [PATCH 10/14] drm/xe: Convert Xe HW Engine print to snapshot capture and print.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 26, 2023 at 04:57:09PM -0400, Rodrigo Vivi wrote:
> The goal is to allow for a snapshot capture to be taken at the time
> of the crash, while the print out can happen at a later time through
> the exposed devcoredump virtual device.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
>  drivers/gpu/drm/xe/xe_gt_debugfs.c      |   2 +-
>  drivers/gpu/drm/xe/xe_guc_submit.c      |   2 +-
>  drivers/gpu/drm/xe/xe_hw_engine.c       | 210 +++++++++++++++++-------
>  drivers/gpu/drm/xe/xe_hw_engine.h       |   8 +-
>  drivers/gpu/drm/xe/xe_hw_engine_types.h |  78 +++++++++
>  5 files changed, 241 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> index c45486c2015a..8bf441e850a0 100644
> --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> @@ -42,7 +42,7 @@ static int hw_engines(struct seq_file *m, void *data)
>  	}
>  
>  	for_each_hw_engine(hwe, gt, id)
> -		xe_hw_engine_print_state(hwe, &p);
> +		xe_hw_engine_print(hwe, &p);
>  
>  	xe_device_mem_access_put(xe);
>  	err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 9c06411f857f..74659d0a69b3 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -751,7 +751,7 @@ static void simple_error_capture(struct xe_engine *e)
>  			if (hwe->class != e->hwe->class ||
>  			    !(BIT(hwe->logical_instance) & adj_logical_mask))
>  				continue;
> -			xe_hw_engine_print_state(hwe, &p);
> +			xe_hw_engine_print(hwe, &p);
>  		}
>  		xe_analyze_vm(&p, e->vm, e->gt->info.id);
>  		xe_force_wake_put(gt_to_fw(guc_to_gt(guc)), XE_FORCEWAKE_ALL);
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c b/drivers/gpu/drm/xe/xe_hw_engine.c
> index 23b9f120c258..eda0666bfa2f 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> @@ -505,77 +505,175 @@ void xe_hw_engine_handle_irq(struct xe_hw_engine *hwe, u16 intr_vec)
>  		xe_hw_fence_irq_run(hwe->fence_irq);
>  }
>  
> -void xe_hw_engine_print_state(struct xe_hw_engine *hwe, struct drm_printer *p)
> +/**
> + * xe_hw_engine_snapshot_capture - Take a quick snapshot of the HW Engine.
> + * @hwe: Xe HW Engine.
> + *
> + * This can be printed out in a later stage like during dev_coredump
> + * analysis.
> + *
> + * Returns: a Xe HW Engine snapshot object that must be freed by the
> + * 	    caller, using `xe_hw_engine_snapshot_free`.
> + */
> +struct xe_hw_engine_snapshot *
> +xe_hw_engine_snapshot_capture(struct xe_hw_engine *hwe)
>  {
> +	struct xe_hw_engine_snapshot *snapshot;
> +	int len;
> +
>  	if (!xe_hw_engine_is_valid(hwe))
> -		return;
> +		return NULL;
> +
> +	snapshot = kzalloc(sizeof(struct xe_hw_engine_snapshot), GFP_ATOMIC);

Same alloc problem as previous files.

> +
> +	len = strlen(hwe->name) + 1;
> +	snapshot->name = kzalloc(len, GFP_ATOMIC);
> +	strscpy(snapshot->name, hwe->name, len);
> +	snapshot->class = hwe->class;
> +	snapshot->logical_instance = hwe->logical_instance;
> +	snapshot->forcewake.domain = hwe->domain;
> +	snapshot->forcewake.ref = xe_force_wake_ref(gt_to_fw(hwe->gt),
> +						    hwe->domain);
> +	snapshot->mmio_base = hwe->mmio_base;
> +
> +	snapshot->reg.ring_hwstam = hw_engine_mmio_read32(hwe,
> +							  RING_HWSTAM(0).reg);
> +	snapshot->reg.ring_hws_pga = hw_engine_mmio_read32(hwe,
> +							   RING_HWS_PGA(0).reg);
> +	snapshot->reg.ring_execlist_status_lo =
> +		hw_engine_mmio_read32(hwe, RING_EXECLIST_STATUS_LO(0).reg);
> +	snapshot->reg.ring_execlist_status_hi =
> +		hw_engine_mmio_read32(hwe, RING_EXECLIST_STATUS_HI(0).reg);
> +	snapshot->reg.ring_execlist_sq_contents_lo =
> +		hw_engine_mmio_read32(hwe,
> +				      RING_EXECLIST_SQ_CONTENTS_LO(0).reg);
> +	snapshot->reg.ring_execlist_sq_contents_hi =
> +		hw_engine_mmio_read32(hwe,
> +				      RING_EXECLIST_SQ_CONTENTS_HI(0).reg);
> +	snapshot->reg.ring_execlist_control =
> +		hw_engine_mmio_read32(hwe, RING_EXECLIST_CONTROL(0).reg);
> +	snapshot->reg.ring_start = hw_engine_mmio_read32(hwe,
> +							 RING_START(0).reg);
> +	snapshot->reg.ring_head =
> +		hw_engine_mmio_read32(hwe, RING_HEAD(0).reg) & HEAD_ADDR;
> +	snapshot->reg.ring_tail =
> +		hw_engine_mmio_read32(hwe, RING_TAIL(0).reg) & TAIL_ADDR;
> +	snapshot->reg.ring_ctl = hw_engine_mmio_read32(hwe, RING_CTL(0).reg);
> +	snapshot->reg.ring_mi_mode =
> +		hw_engine_mmio_read32(hwe, RING_MI_MODE(0).reg);
> +	snapshot->reg.ring_mode_gen7 =
> +		hw_engine_mmio_read32(hwe, RING_MODE_GEN7(0).reg);
> +	snapshot->reg.ring_imr = hw_engine_mmio_read32(hwe, RING_IMR(0).reg);
> +	snapshot->reg.ring_esr = hw_engine_mmio_read32(hwe, RING_ESR(0).reg);
> +	snapshot->reg.ring_emr = hw_engine_mmio_read32(hwe, RING_EMR(0).reg);
> +	snapshot->reg.ring_eir = hw_engine_mmio_read32(hwe, RING_EIR(0).reg);
> +	snapshot->reg.ring_acthd_udw =
> +		hw_engine_mmio_read32(hwe, RING_ACTHD_UDW(0).reg);
> +	snapshot->reg.ring_acthd = hw_engine_mmio_read32(hwe,
> +							 RING_ACTHD(0).reg);
> +	snapshot->reg.ring_bbaddr_udw =
> +		hw_engine_mmio_read32(hwe, RING_BBADDR_UDW(0).reg);
> +	snapshot->reg.ring_bbaddr = hw_engine_mmio_read32(hwe, RING_BBADDR(0).reg);
> +	snapshot->reg.ring_dma_fadd_udw =
> +		hw_engine_mmio_read32(hwe, RING_DMA_FADD_UDW(0).reg),
> +	snapshot->reg.ring_dma_fadd =
> +		hw_engine_mmio_read32(hwe, RING_DMA_FADD(0).reg);
> +	snapshot->reg.ipeir = hw_engine_mmio_read32(hwe, IPEIR(0).reg);
> +	snapshot->reg.ipehr = hw_engine_mmio_read32(hwe, IPEHR(0).reg);
>  
> -	drm_printf(p, "%s (physical), logical instance=%d\n", hwe->name,
> -		hwe->logical_instance);
> -	drm_printf(p, "\tForcewake: domain 0x%x, ref %d\n",
> -		hwe->domain,
> -		xe_force_wake_ref(gt_to_fw(hwe->gt), hwe->domain));
> -	drm_printf(p, "\tMMIO base: 0x%08x\n", hwe->mmio_base);
> +	if (snapshot->class == XE_ENGINE_CLASS_COMPUTE)
> +		snapshot->reg.rcu_mode = xe_mmio_read32(hwe->gt,
> +							GEN12_RCU_MODE.reg);
>  
> -	drm_printf(p, "\tHWSTAM: 0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_HWSTAM(0).reg));
> -	drm_printf(p, "\tRING_HWS_PGA: 0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_HWS_PGA(0).reg));
> +	return snapshot;
> +}
>  
> +/**
> + * xe_hw_engine_snapshot_print - Print out a given Xe HW Engine snapshot.
> + * @snapshot: Xe HW Engine snapshot object.
> + * @p: drm_printer where it will be printed out.
> + *
> + * This function prints out a given Xe HW Engine snapshot object.
> + */
> +void xe_hw_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot,
> +				 struct drm_printer *p)
> +{
> +	if (!snapshot)
> +		return;
> +
> +	drm_printf(p, "%s (physical), logical instance=%d\n", snapshot->name,
> +		snapshot->logical_instance);
> +	drm_printf(p, "\tForcewake: domain 0x%x, ref %d\n",
> +		snapshot->forcewake.domain, snapshot->forcewake.ref);
> +	drm_printf(p, "\tMMIO base: 0x%08x\n", snapshot->mmio_base);
> +	drm_printf(p, "\tHWSTAM: 0x%08x\n", snapshot->reg.ring_hwstam);
> +	drm_printf(p, "\tRING_HWS_PGA: 0x%08x\n", snapshot->reg.ring_hws_pga);
>  	drm_printf(p, "\tRING_EXECLIST_STATUS_LO: 0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_EXECLIST_STATUS_LO(0).reg));
> +		   snapshot->reg.ring_execlist_status_lo);
>  	drm_printf(p, "\tRING_EXECLIST_STATUS_HI: 0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_EXECLIST_STATUS_HI(0).reg));
> +		   snapshot->reg.ring_execlist_status_hi);
>  	drm_printf(p, "\tRING_EXECLIST_SQ_CONTENTS_LO: 0x%08x\n",
> -		hw_engine_mmio_read32(hwe,
> -					 RING_EXECLIST_SQ_CONTENTS_LO(0).reg));
> +		   snapshot->reg.ring_execlist_sq_contents_lo);
>  	drm_printf(p, "\tRING_EXECLIST_SQ_CONTENTS_HI: 0x%08x\n",
> -		hw_engine_mmio_read32(hwe,
> -					 RING_EXECLIST_SQ_CONTENTS_HI(0).reg));
> +		   snapshot->reg.ring_execlist_sq_contents_hi);
>  	drm_printf(p, "\tRING_EXECLIST_CONTROL: 0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_EXECLIST_CONTROL(0).reg));
> -
> -	drm_printf(p, "\tRING_START: 0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_START(0).reg));
> -	drm_printf(p, "\tRING_HEAD:  0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_HEAD(0).reg) & HEAD_ADDR);
> -	drm_printf(p, "\tRING_TAIL:  0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_TAIL(0).reg) & TAIL_ADDR);
> -	drm_printf(p, "\tRING_CTL: 0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_CTL(0).reg));
> -	drm_printf(p, "\tRING_MODE: 0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_MI_MODE(0).reg));
> +		   snapshot->reg.ring_execlist_control);
> +	drm_printf(p, "\tRING_START: 0x%08x\n", snapshot->reg.ring_start);
> +	drm_printf(p, "\tRING_HEAD:  0x%08x\n", snapshot->reg.ring_head);
> +	drm_printf(p, "\tRING_TAIL:  0x%08x\n", snapshot->reg.ring_tail);
> +	drm_printf(p, "\tRING_CTL: 0x%08x\n", snapshot->reg.ring_ctl);
> +	drm_printf(p, "\tRING_MODE: 0x%08x\n", snapshot->reg.ring_mi_mode);
>  	drm_printf(p, "\tRING_MODE_GEN7: 0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_MODE_GEN7(0).reg));
> -
> -	drm_printf(p, "\tRING_IMR:   0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_IMR(0).reg));
> -	drm_printf(p, "\tRING_ESR:   0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_ESR(0).reg));
> -	drm_printf(p, "\tRING_EMR:   0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_EMR(0).reg));
> -	drm_printf(p, "\tRING_EIR:   0x%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_EIR(0).reg));
> -
> -        drm_printf(p, "\tACTHD:  0x%08x_%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_ACTHD_UDW(0).reg),
> -		hw_engine_mmio_read32(hwe, RING_ACTHD(0).reg));
> -        drm_printf(p, "\tBBADDR: 0x%08x_%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_BBADDR_UDW(0).reg),
> -		hw_engine_mmio_read32(hwe, RING_BBADDR(0).reg));
> +		   snapshot->reg.ring_mode_gen7);
> +	drm_printf(p, "\tRING_IMR:   0x%08x\n", snapshot->reg.ring_imr);
> +	drm_printf(p, "\tRING_ESR:   0x%08x\n", snapshot->reg.ring_esr);
> +	drm_printf(p, "\tRING_EMR:   0x%08x\n", snapshot->reg.ring_emr);
> +	drm_printf(p, "\tRING_EIR:   0x%08x\n", snapshot->reg.ring_eir);
> +        drm_printf(p, "\tACTHD:  0x%08x_%08x\n", snapshot->reg.ring_acthd_udw,

The alignment looks off here, looks like spaces rather than tabs.

Also I hate 0x%08x_%08x, can we just do 0x%016llx?

> +		   snapshot->reg.ring_acthd);
> +        drm_printf(p, "\tBBADDR: 0x%08x_%08x\n", snapshot->reg.ring_bbaddr_udw,
> +		   snapshot->reg.ring_bbaddr);
>          drm_printf(p, "\tDMA_FADDR: 0x%08x_%08x\n",
> -		hw_engine_mmio_read32(hwe, RING_DMA_FADD_UDW(0).reg),
> -		hw_engine_mmio_read32(hwe, RING_DMA_FADD(0).reg));
> +		   snapshot->reg.ring_dma_fadd_udw,
> +		   snapshot->reg.ring_dma_fadd);
> +	drm_printf(p, "\tIPEIR: 0x%08x\n", snapshot->reg.ipeir);
> +	drm_printf(p, "\tIPEHR: 0x%08x\n\n", snapshot->reg.ipehr);
>  
> -	drm_printf(p, "\tIPEIR: 0x%08x\n",
> -		hw_engine_mmio_read32(hwe, IPEIR(0).reg));
> -	drm_printf(p, "\tIPEHR: 0x%08x\n\n",
> -		hw_engine_mmio_read32(hwe, IPEHR(0).reg));
> -
> -	if (hwe->class == XE_ENGINE_CLASS_COMPUTE)
> +	if (snapshot->class == XE_ENGINE_CLASS_COMPUTE)
>  		drm_printf(p, "\tGEN12_RCU_MODE: 0x%08x\n",
> -			xe_mmio_read32(hwe->gt, GEN12_RCU_MODE.reg));
> +			   snapshot->reg.rcu_mode);
> +}
> +
> +/**
> + * xe_hw_engine_snapshot_free - Free all allocated objects for a given snapshot.
> + * @snapshot: Xe HW Engine snapshot object.
> + *
> + * This function free all the memory that needed to be allocated at capture
> + * time.
> + */
> +void xe_hw_engine_snapshot_free(struct xe_hw_engine_snapshot *snapshot)
> +{
> +	if (!snapshot)
> +		return;
> +
> +	kfree(snapshot->name);
> +	kfree(snapshot);
> +}
> +
> +/**
> + * xe_hw_engine_print - Xe HW Engine Print.
> + * @hwe: Hardware Engine.
> + * @p: drm_printer.
> + *
> + * This function quickly capture a snapshot and immediately print it out.
> + */
> +void xe_hw_engine_print(struct xe_hw_engine *hwe, struct drm_printer *p)
> +{
> +	struct xe_hw_engine_snapshot *snapshot;
>  
> +	snapshot = xe_hw_engine_snapshot_capture(hwe);
> +	xe_hw_engine_snapshot_print(snapshot, p);
> +	xe_hw_engine_snapshot_free(snapshot);
>  }
>  
>  u32 xe_hw_engine_mask_per_class(struct xe_gt *gt,
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.h b/drivers/gpu/drm/xe/xe_hw_engine.h
> index ceab65397256..a0514bf859c6 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.h
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.h
> @@ -14,10 +14,16 @@ int xe_hw_engines_init_early(struct xe_gt *gt);
>  int xe_hw_engines_init(struct xe_gt *gt);
>  void xe_hw_engine_handle_irq(struct xe_hw_engine *hwe, u16 intr_vec);
>  void xe_hw_engine_enable_ring(struct xe_hw_engine *hwe);
> -void xe_hw_engine_print_state(struct xe_hw_engine *hwe, struct drm_printer *p);
>  u32 xe_hw_engine_mask_per_class(struct xe_gt *gt,
>  				enum xe_engine_class engine_class);
>  
> +struct xe_hw_engine_snapshot *
> +xe_hw_engine_snapshot_capture(struct xe_hw_engine *hwe);
> +void xe_hw_engine_snapshot_free(struct xe_hw_engine_snapshot *snapshot);
> +void xe_hw_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot,
> +				 struct drm_printer *p);
> +void xe_hw_engine_print(struct xe_hw_engine *hwe, struct drm_printer *p);
> +
>  bool xe_hw_engine_is_reserved(struct xe_hw_engine *hwe);
>  static inline bool xe_hw_engine_is_valid(struct xe_hw_engine *hwe)
>  {
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine_types.h b/drivers/gpu/drm/xe/xe_hw_engine_types.h
> index 2c40384957da..c15d6c671fbb 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine_types.h
> +++ b/drivers/gpu/drm/xe/xe_hw_engine_types.h
> @@ -109,4 +109,82 @@ struct xe_hw_engine {
>  	enum xe_hw_engine_id engine_id;
>  };
>  
> +/**
> + * struct xe_hw_engine_snapshot - Hardware engine snapshot
> + *
> + * Contains the snapshot of usefull hardware engine info and registers.
> + */
> +struct xe_hw_engine_snapshot {
> +	/** @name: name of the hw engine */
> +	char *name;
> +	/** @class: class of this hw engine */
> +	enum xe_engine_class class;
> +	/** @logical_instance: logical instance of this hw engine */
> +	u16 logical_instance;
> +	/** @forcewake: Force Wake information snapshot */
> +	struct {
> +		/** @domain: force wake domain of this hw engine */
> +		enum xe_force_wake_domains domain;
> +		/** @ref: Forcewake ref for the above domain */
> +		int ref;
> +	} forcewake;
> +	/** @reg: Useful MMIO register snapshot */

Should be down 1 more line.

Matt

> +	/** @mmio_base: MMIO base address of this hw engine*/
> +	u32 mmio_base;
> +	struct {
> +		/** @ring_hwstam: RING_HWSTAM */
> +		u32 ring_hwstam;
> +		/** @ring_hws_pga: RING_HWS_PGA */
> +		u32 ring_hws_pga;
> +		/** @ring_execlist_status_lo: RING_EXECLIST_STATUS_LO */
> +		u32 ring_execlist_status_lo;
> +		/** @ring_execlist_status_hi: RING_EXECLIST_STATUS_HI */
> +		u32 ring_execlist_status_hi;
> +		/** @ring_execlist_sq_contents_lo: RING_EXECLIST_SQ_CONTENTS */
> +		u32 ring_execlist_sq_contents_lo;
> +		/** @ring_execlist_sq_contents_hi: RING_EXECLIST_SQ_CONTENTS + 4 */
> +		u32 ring_execlist_sq_contents_hi;
> +		/** @ring_execlist_control: RING_EXECLIST_CONTROL */
> +		u32 ring_execlist_control;
> +		/** @ring_start: RING_START */
> +		u32 ring_start;
> +		/** @ring_head: RING_HEAD */
> +		u32 ring_head;
> +		/** @ring_tail: RING_TAIL */
> +		u32 ring_tail;
> +		/** @ring_ctl: RING_CTL */
> +		u32 ring_ctl;
> +		/** @ring_mi_mode: RING_MI_MODE */
> +		u32 ring_mi_mode;
> +		/** @ring_mode_gen7: RING_MODE_GEN7 */
> +		u32 ring_mode_gen7;
> +		/** @ring_imr: RING_IMR */
> +		u32 ring_imr;
> +		/** @ring_esr: RING_ESR */
> +		u32 ring_esr;
> +		/** @ring_emr: RING_EMR */
> +		u32 ring_emr;
> +		/** @ring_eir: RING_EIR */
> +		u32 ring_eir;
> +		/** @ring_acthd_udw: RING_ACTHD_UDW */
> +		u32 ring_acthd_udw;
> +		/** @ring_acthd: RING_ACTHD */
> +		u32 ring_acthd;
> +		/** @ring_bbaddr_udw: RING_BBADDR_UDW */
> +		u32 ring_bbaddr_udw;
> +		/** @ring_bbaddr: RING_BBADDR */
> +		u32 ring_bbaddr;
> +		/** @ring_dma_fadd_udw: RING_DMA_FADD_UDW */
> +		u32 ring_dma_fadd_udw;
> +		/** @ring_dma_fadd: RING_DMA_FADD */
> +		u32 ring_dma_fadd;
> +		/** @ipeir: IPEIR */
> +		u32 ipeir;
> +		/** @ipehr: IPEHR */
> +		u32 ipehr;
> +		/** @rcu_mode: GEN12_RCU_MODE */
> +		u32 rcu_mode;
> +	} reg;
> +};
> +
>  #endif
> -- 
> 2.39.2
> 



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux