Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH 1/2] drm/xe/guc: Prefer GT oriented asserts in submit code
Date: Thu, 7 Nov 2024 14:15:51 -0800	[thread overview]
Message-ID: <Zy08F1Qfc1TIovZX@lstrano-desk.jf.intel.com> (raw)
In-Reply-To: <20241107194741.2167-2-michal.wajdeczko@intel.com>

On Thu, Nov 07, 2024 at 08:47:40PM +0100, Michal Wajdeczko wrote:
> For better diagnostics, use xe_gt_assert() instead of xe_assert().
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_guc_submit.c | 52 ++++++++++++------------------
>  1 file changed, 21 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 9e0f86f3778b..0311d9e50d06 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -412,12 +412,11 @@ static const int xe_exec_queue_prio_to_guc[] = {
>  static void init_policies(struct xe_guc *guc, struct xe_exec_queue *q)
>  {
>  	struct exec_queue_policy policy;
> -	struct xe_device *xe = guc_to_xe(guc);
>  	enum xe_exec_queue_priority prio = q->sched_props.priority;
>  	u32 timeslice_us = q->sched_props.timeslice_us;
>  	u32 preempt_timeout_us = q->sched_props.preempt_timeout_us;
>  
> -	xe_assert(xe, exec_queue_registered(q));
> +	xe_gt_assert(guc_to_gt(guc), exec_queue_registered(q));
>  
>  	__guc_exec_queue_policy_start_klv(&policy, q->guc->id);
>  	__guc_exec_queue_policy_add_priority(&policy, xe_exec_queue_prio_to_guc[prio]);
> @@ -451,12 +450,11 @@ static void __register_mlrc_exec_queue(struct xe_guc *guc,
>  				       struct guc_ctxt_registration_info *info)
>  {
>  #define MAX_MLRC_REG_SIZE      (13 + XE_HW_ENGINE_MAX_INSTANCE * 2)
> -	struct xe_device *xe = guc_to_xe(guc);
>  	u32 action[MAX_MLRC_REG_SIZE];
>  	int len = 0;
>  	int i;
>  
> -	xe_assert(xe, xe_exec_queue_is_parallel(q));
> +	xe_gt_assert(guc_to_gt(guc), xe_exec_queue_is_parallel(q));
>  
>  	action[len++] = XE_GUC_ACTION_REGISTER_CONTEXT_MULTI_LRC;
>  	action[len++] = info->flags;
> @@ -479,7 +477,7 @@ static void __register_mlrc_exec_queue(struct xe_guc *guc,
>  		action[len++] = upper_32_bits(xe_lrc_descriptor(lrc));
>  	}
>  
> -	xe_assert(xe, len <= MAX_MLRC_REG_SIZE);
> +	xe_gt_assert(guc_to_gt(guc), len <= MAX_MLRC_REG_SIZE);
>  #undef MAX_MLRC_REG_SIZE
>  
>  	xe_guc_ct_send(&guc->ct, action, len, 0, 0);
> @@ -513,7 +511,7 @@ static void register_exec_queue(struct xe_exec_queue *q)
>  	struct xe_lrc *lrc = q->lrc[0];
>  	struct guc_ctxt_registration_info info;
>  
> -	xe_assert(xe, !exec_queue_registered(q));
> +	xe_gt_assert(guc_to_gt(guc), !exec_queue_registered(q));
>  
>  	memset(&info, 0, sizeof(info));
>  	info.context_idx = q->guc->id;
> @@ -603,7 +601,7 @@ static int wq_noop_append(struct xe_exec_queue *q)
>  	if (wq_wait_for_space(q, wq_space_until_wrap(q)))
>  		return -ENODEV;
>  
> -	xe_assert(xe, FIELD_FIT(WQ_LEN_MASK, len_dw));
> +	xe_gt_assert(guc_to_gt(guc), FIELD_FIT(WQ_LEN_MASK, len_dw));
>  
>  	parallel_write(xe, map, wq[q->guc->wqi_tail / sizeof(u32)],
>  		       FIELD_PREP(WQ_TYPE_MASK, WQ_TYPE_NOOP) |
> @@ -643,13 +641,13 @@ static void wq_item_append(struct xe_exec_queue *q)
>  		wqi[i++] = lrc->ring.tail / sizeof(u64);
>  	}
>  
> -	xe_assert(xe, i == wqi_size / sizeof(u32));
> +	xe_gt_assert(guc_to_gt(guc), i == wqi_size / sizeof(u32));
>  
>  	iosys_map_incr(&map, offsetof(struct guc_submit_parallel_scratch,
>  				      wq[q->guc->wqi_tail / sizeof(u32)]));
>  	xe_map_memcpy_to(xe, &map, 0, wqi, wqi_size);
>  	q->guc->wqi_tail += wqi_size;
> -	xe_assert(xe, q->guc->wqi_tail <= WQ_SIZE);
> +	xe_gt_assert(guc_to_gt(guc), q->guc->wqi_tail <= WQ_SIZE);
>  
>  	xe_device_wmb(xe);
>  
> @@ -661,7 +659,6 @@ static void wq_item_append(struct xe_exec_queue *q)
>  static void submit_exec_queue(struct xe_exec_queue *q)
>  {
>  	struct xe_guc *guc = exec_queue_to_guc(q);
> -	struct xe_device *xe = guc_to_xe(guc);
>  	struct xe_lrc *lrc = q->lrc[0];
>  	u32 action[3];
>  	u32 g2h_len = 0;
> @@ -669,7 +666,7 @@ static void submit_exec_queue(struct xe_exec_queue *q)
>  	int len = 0;
>  	bool extra_submit = false;
>  
> -	xe_assert(xe, exec_queue_registered(q));
> +	xe_gt_assert(guc_to_gt(guc), exec_queue_registered(q));
>  
>  	if (xe_exec_queue_is_parallel(q))
>  		wq_item_append(q);
> @@ -716,12 +713,11 @@ guc_exec_queue_run_job(struct drm_sched_job *drm_job)
>  	struct xe_sched_job *job = to_xe_sched_job(drm_job);
>  	struct xe_exec_queue *q = job->q;
>  	struct xe_guc *guc = exec_queue_to_guc(q);
> -	struct xe_device *xe = guc_to_xe(guc);
>  	struct dma_fence *fence = NULL;
>  	bool lr = xe_exec_queue_is_lr(q);
>  
> -	xe_assert(xe, !(exec_queue_destroyed(q) || exec_queue_pending_disable(q)) ||
> -		  exec_queue_banned(q) || exec_queue_suspended(q));
> +	xe_gt_assert(guc_to_gt(guc), !(exec_queue_destroyed(q) || exec_queue_pending_disable(q)) ||
> +		     exec_queue_banned(q) || exec_queue_suspended(q));
>  
>  	trace_xe_sched_job_run(job);
>  
> @@ -867,7 +863,7 @@ static void xe_guc_exec_queue_lr_cleanup(struct work_struct *w)
>  	struct xe_gpu_scheduler *sched = &ge->sched;
>  	bool wedged;
>  
> -	xe_assert(xe, xe_exec_queue_is_lr(q));
> +	xe_gt_assert(guc_to_gt(guc), xe_exec_queue_is_lr(q));
>  	trace_xe_exec_queue_lr_cleanup(q);
>  
>  	wedged = guc_submit_hint_wedged(exec_queue_to_guc(q));
> @@ -1274,9 +1270,8 @@ static void __guc_exec_queue_process_msg_cleanup(struct xe_sched_msg *msg)
>  {
>  	struct xe_exec_queue *q = msg->private_data;
>  	struct xe_guc *guc = exec_queue_to_guc(q);
> -	struct xe_device *xe = guc_to_xe(guc);
>  
> -	xe_assert(xe, !(q->flags & EXEC_QUEUE_FLAG_PERMANENT));
> +	xe_gt_assert(guc_to_gt(guc), !(q->flags & EXEC_QUEUE_FLAG_PERMANENT));
>  	trace_xe_exec_queue_cleanup_entity(q);
>  
>  	if (exec_queue_registered(q))
> @@ -1312,11 +1307,10 @@ static void __suspend_fence_signal(struct xe_exec_queue *q)
>  static void suspend_fence_signal(struct xe_exec_queue *q)
>  {
>  	struct xe_guc *guc = exec_queue_to_guc(q);
> -	struct xe_device *xe = guc_to_xe(guc);
>  
> -	xe_assert(xe, exec_queue_suspended(q) || exec_queue_killed(q) ||
> -		  xe_guc_read_stopped(guc));
> -	xe_assert(xe, q->guc->suspend_pending);
> +	xe_gt_assert(guc_to_gt(guc), exec_queue_suspended(q) || exec_queue_killed(q) ||
> +		     xe_guc_read_stopped(guc));
> +	xe_gt_assert(guc_to_gt(guc), q->guc->suspend_pending);
>  
>  	__suspend_fence_signal(q);
>  }
> @@ -1412,12 +1406,11 @@ static int guc_exec_queue_init(struct xe_exec_queue *q)
>  {
>  	struct xe_gpu_scheduler *sched;
>  	struct xe_guc *guc = exec_queue_to_guc(q);
> -	struct xe_device *xe = guc_to_xe(guc);
>  	struct xe_guc_exec_queue *ge;
>  	long timeout;
>  	int err, i;
>  
> -	xe_assert(xe, xe_device_uc_enabled(guc_to_xe(guc)));
> +	xe_gt_assert(guc_to_gt(guc), xe_device_uc_enabled(guc_to_xe(guc)));
>  
>  	ge = kzalloc(sizeof(*ge), GFP_KERNEL);
>  	if (!ge)
> @@ -1630,9 +1623,8 @@ static void guc_exec_queue_resume(struct xe_exec_queue *q)
>  	struct xe_gpu_scheduler *sched = &q->guc->sched;
>  	struct xe_sched_msg *msg = q->guc->static_msgs + STATIC_MSG_RESUME;
>  	struct xe_guc *guc = exec_queue_to_guc(q);
> -	struct xe_device *xe = guc_to_xe(guc);
>  
> -	xe_assert(xe, !q->guc->suspend_pending);
> +	xe_gt_assert(guc_to_gt(guc), !q->guc->suspend_pending);
>  
>  	xe_sched_msg_lock(sched);
>  	guc_exec_queue_try_add_msg(q, msg, RESUME);
> @@ -1744,9 +1736,8 @@ void xe_guc_submit_stop(struct xe_guc *guc)
>  {
>  	struct xe_exec_queue *q;
>  	unsigned long index;
> -	struct xe_device *xe = guc_to_xe(guc);
>  
> -	xe_assert(xe, xe_guc_read_stopped(guc) == 1);
> +	xe_gt_assert(guc_to_gt(guc), xe_guc_read_stopped(guc) == 1);
>  
>  	mutex_lock(&guc->submission_state.lock);
>  
> @@ -1788,9 +1779,8 @@ int xe_guc_submit_start(struct xe_guc *guc)
>  {
>  	struct xe_exec_queue *q;
>  	unsigned long index;
> -	struct xe_device *xe = guc_to_xe(guc);
>  
> -	xe_assert(xe, xe_guc_read_stopped(guc) == 1);
> +	xe_gt_assert(guc_to_gt(guc), xe_guc_read_stopped(guc) == 1);
>  
>  	mutex_lock(&guc->submission_state.lock);
>  	atomic_dec(&guc->submission_state.stopped);
> @@ -1825,8 +1815,8 @@ g2h_exec_queue_lookup(struct xe_guc *guc, u32 guc_id)
>  		return NULL;
>  	}
>  
> -	xe_assert(xe, guc_id >= q->guc->id);
> -	xe_assert(xe, guc_id < (q->guc->id + q->width));
> +	xe_gt_assert(guc_to_gt(guc), guc_id >= q->guc->id);
> +	xe_gt_assert(guc_to_gt(guc), guc_id < (q->guc->id + q->width));
>  
>  	return q;
>  }
> -- 
> 2.43.0
> 

  reply	other threads:[~2024-11-07 22:15 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-07 19:47 [PATCH 0/2] Prefer GT oriented logs/asserts in submit code Michal Wajdeczko
2024-11-07 19:47 ` [PATCH 1/2] drm/xe/guc: Prefer GT oriented asserts " Michal Wajdeczko
2024-11-07 22:15   ` Matthew Brost [this message]
2024-11-07 19:47 ` [PATCH 2/2] drm/xe/guc: Prefer GT oriented logs " Michal Wajdeczko
2024-11-07 22:19   ` Matthew Brost
2024-11-07 19:53 ` ✓ CI.Patch_applied: success for Prefer GT oriented logs/asserts " Patchwork
2024-11-07 19:54 ` ✓ CI.checkpatch: " Patchwork
2024-11-07 19:55 ` ✓ CI.KUnit: " Patchwork
2024-11-07 20:06 ` ✓ CI.Build: " Patchwork
2024-11-07 20:09 ` ✓ CI.Hooks: " Patchwork
2024-11-07 20:10 ` ✓ CI.checksparse: " Patchwork
2024-11-07 20:34 ` ✓ CI.BAT: " Patchwork
2024-11-09  2:16 ` ✗ CI.FULL: failure " Patchwork
2024-11-09 14:14   ` Michal Wajdeczko

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=Zy08F1Qfc1TIovZX@lstrano-desk.jf.intel.com \
    --to=matthew.brost@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