On Tue, Jan 28, 2025 at 10:36:50AM -0800, Alan Previn wrote:
> xe_devcoredump calls xe_engine_snapshot_capture_for_queue() to allocate
> and populate the xe_hw_engine_snapshot structure. Move that function
> back into xe_hw_engine.c since it doesn't make sense for
> GuC-Err-Capture to allocate a structure it doesn't own.
> 
> Signed-off-by: Alan Previn <alan.previn.teres.ale...@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_capture.c | 30 -----------------------
>  drivers/gpu/drm/xe/xe_guc_capture.h |  1 -
>  drivers/gpu/drm/xe/xe_hw_engine.c   | 38 ++++++++++++++++++++++++++---
>  drivers/gpu/drm/xe/xe_hw_engine.h   |  3 +--
>  4 files changed, 35 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_capture.c 
> b/drivers/gpu/drm/xe/xe_guc_capture.c
> index a7278a01f586..6f40aad7e212 100644
> --- a/drivers/gpu/drm/xe/xe_guc_capture.c
> +++ b/drivers/gpu/drm/xe/xe_guc_capture.c
> @@ -1866,36 +1866,6 @@ xe_guc_capture_get_matching_and_lock(struct 
> xe_exec_queue *q,
>       return NULL;
>  }
>  
> -/**
> - * xe_engine_snapshot_capture_for_queue - Take snapshot of associated engine
> - * @q: The exec queue object
> - *
> - * Take snapshot of associated HW Engine
> - *
> - * Returns: None.
> - */
> -void
> -xe_engine_snapshot_capture_for_queue(struct xe_exec_queue *q)
> -{
> -     struct xe_device *xe = gt_to_xe(q->gt);
> -     struct xe_devcoredump *coredump = &xe->devcoredump;
> -     struct xe_hw_engine *hwe;
> -     enum xe_hw_engine_id id;
> -     u32 adj_logical_mask = q->logical_mask;
> -
> -     if (IS_SRIOV_VF(xe))
> -             return;
> -
> -     for_each_hw_engine(hwe, q->gt, id) {
> -             if (hwe->class != q->hwe->class ||
> -                 !(BIT(hwe->logical_instance) & adj_logical_mask)) {
> -                     coredump->snapshot.hwe[id] = NULL;
> -                     continue;
> -             }
> -             coredump->snapshot.hwe[id] = xe_hw_engine_snapshot_capture(hwe, 
> q);
> -     }
> -}
> -
>  /*
>   * xe_guc_capture_put_matched_nodes - Cleanup matched nodes
>   * @guc: The GuC object
> diff --git a/drivers/gpu/drm/xe/xe_guc_capture.h 
> b/drivers/gpu/drm/xe/xe_guc_capture.h
> index e67589ab4342..77ee35a3f205 100644
> --- a/drivers/gpu/drm/xe/xe_guc_capture.h
> +++ b/drivers/gpu/drm/xe/xe_guc_capture.h
> @@ -56,7 +56,6 @@ xe_guc_capture_get_matching_and_lock(struct xe_exec_queue 
> *q,
>  void xe_guc_capture_snapshot_store_manual_job(struct xe_guc *guc, struct 
> xe_exec_queue *q);
>  void xe_guc_capture_snapshot_print(struct xe_guc *guc, struct 
> xe_guc_capture_snapshot *node,
>                                  struct drm_printer *p);
> -void xe_engine_snapshot_capture_for_queue(struct xe_exec_queue *q);
>  void xe_guc_capture_steered_list_init(struct xe_guc *guc);
>  void xe_guc_capture_put_matched_nodes(struct xe_guc *guc, struct 
> xe_guc_capture_snapshot *n);
>  int xe_guc_capture_init(struct xe_guc *guc);
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c 
> b/drivers/gpu/drm/xe/xe_hw_engine.c
> index d615ebab6e42..40c1f9814177 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> @@ -830,7 +830,7 @@ void xe_hw_engine_handle_irq(struct xe_hw_engine *hwe, 
> u16 intr_vec)
>  }
>  
>  /**
> - * xe_hw_engine_snapshot_capture - Take a quick snapshot of the HW Engine.
> + * hw_engine_snapshot_capture - Take a quick snapshot of the HW Engine.
>   * @hwe: Xe HW Engine.
>   * @q: The exec queue object.
>   *
> @@ -840,8 +840,8 @@ void xe_hw_engine_handle_irq(struct xe_hw_engine *hwe, 
> u16 intr_vec)
>   * 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_exec_queue 
> *q)
> +static struct xe_hw_engine_snapshot *
> +hw_engine_snapshot_capture(struct xe_hw_engine *hwe, struct xe_exec_queue *q)
>  {
>       struct xe_hw_engine_snapshot *snapshot;
>       struct xe_guc_capture_snapshot *node;
> @@ -885,6 +885,36 @@ xe_hw_engine_snapshot_capture(struct xe_hw_engine *hwe, 
> struct xe_exec_queue *q)
>       return snapshot;
>  }
>  
> +/**
> + * xe_engine_snapshot_capture_for_queue - Take snapshot of associated engine
> + * @q: The exec queue object
> + *
> + * Take snapshot of associated HW Engine
> + *
> + * Returns: None.
> + */
> +void
> +xe_engine_snapshot_capture_for_queue(struct xe_exec_queue *q)
> +{
> +     struct xe_device *xe = gt_to_xe(q->gt);
> +     struct xe_devcoredump *coredump = &xe->devcoredump;
> +     struct xe_hw_engine *hwe;
> +     enum xe_hw_engine_id id;
> +     u32 adj_logical_mask = q->logical_mask;
> +
> +     if (IS_SRIOV_VF(xe))
> +             return;
> +
> +     for_each_hw_engine(hwe, q->gt, id) {
> +             if (hwe->class != q->hwe->class ||
> +                 !(BIT(hwe->logical_instance) & adj_logical_mask)) {
> +                     coredump->snapshot.hwe[id] = NULL;
> +                     continue;
> +             }
> +             coredump->snapshot.hwe[id] = hw_engine_snapshot_capture(hwe, q);
> +     }
> +}
> +
>  /**
>   * xe_hw_engine_snapshot_free - Free all allocated objects for a given 
> snapshot.
>   * @snapshot: Xe HW Engine snapshot object.
> @@ -944,7 +974,7 @@ 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, NULL);
> +     snapshot = hw_engine_snapshot_capture(hwe, NULL);
>       xe_engine_snapshot_print(snapshot, p);
>       xe_hw_engine_snapshot_free(snapshot);
>  }
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.h 
> b/drivers/gpu/drm/xe/xe_hw_engine.h
> index fac2e9a421d9..845153fbc149 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.h
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.h
> @@ -55,8 +55,7 @@ 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);
>  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, struct xe_exec_queue 
> *q);
> +void xe_engine_snapshot_capture_for_queue(struct xe_exec_queue *q);

here as well. please respect the name space.

>  void xe_hw_engine_snapshot_free(struct xe_hw_engine_snapshot *snapshot);
>  void xe_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);
> -- 
> 2.34.1
> 

Reply via email to