All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Cc: "Anoop, Vijay" <anoop.c.vijay@intel.com>,
	<intel-xe@lists.freedesktop.org>, <badal.nilawar@intel.com>,
	<aravind.iddamsetty@intel.com>, <riana.tauro@intel.com>,
	<mallesh.koujalagi@intel.com>, <anshuman.gupta@intel.com>,
	<matthew.d.roper@intel.com>, <michal.wajdeczko@intel.com>,
	<michael.j.ruhl@intel.com>, <paul.e.luse@intel.com>,
	<mohamed.mansoor.v@intel.com>, <kam.nasim@intel.com>
Subject: Re: [PATCH v2] drm/xe/sysctrl: Add mailbox control state to error messages
Date: Fri, 9 Oct 2026 21:00:22 -0400	[thread overview]
Message-ID: <asmOJrnBwBSFIKBk@intel.com> (raw)
In-Reply-To: <askw3y5OdyAvYC/Y@soc-5CG1426VCC.clients.intel.com>

On Fri, Oct 09, 2026 at 11:22:23AM -0700, Umesh Nerlige Ramappa wrote:
> On Thu, Oct 08, 2026 at 11:47:55PM -0700, Anoop, Vijay wrote:
> > From: Anoop Vijay <anoop.c.vijay@intel.com>
> > 
> > Include decoded mailbox control register information in mailbox
> > busy, timeout and firmware error paths to provide additional
> > context for failure debugging
> > 
> > Signed-off-by: Anoop Vijay <anoop.c.vijay@intel.com>
> 
> LGTM
> 
> Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

pushed to drm-xe-next. Thanks for the patch and review

> 
> > ---
> > v2 (Umesh):
> > - Remove redundant casts
> > - Align log/format string style
> > - Drop duplicate frame number in frame-ack timeout message
> > ---
> > drivers/gpu/drm/xe/xe_sysctrl_mailbox.c | 56 ++++++++++++++++++-------
> > 1 file changed, 41 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> > index 25425e8bb533..f1a231dfec1e 100644
> > --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> > +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> > @@ -7,6 +7,7 @@
> > #include <linux/cleanup.h>
> > #include <linux/minmax.h>
> > #include <linux/slab.h>
> > +#include <linux/sprintf.h>
> > #include <linux/string.h>
> > 
> > #include "regs/xe_sysctrl_regs.h"
> > @@ -36,17 +37,27 @@ struct xe_sysctrl_mailbox_msg_hdr {
> > 	REG_FIELD_GET(SYSCTRL_HDR_RESULT_MASK, (hdr)->data)
> > 
> > static int sysctrl_wait_bit_clear(struct xe_sysctrl *sc, u32 bit_mask,
> > -				  unsigned int timeout_ms)
> > +				  unsigned int timeout_ms, u32 *out_val)
> > {
> > 	return xe_mmio_wait32_not(sc->mmio, SYSCTRL_MB_CTRL, bit_mask, bit_mask,
> > -				  timeout_ms * 1000, NULL, false);
> > +				  timeout_ms * 1000, out_val, false);
> > }
> > 
> > static int sysctrl_wait_bit_set(struct xe_sysctrl *sc, u32 bit_mask,
> > -				unsigned int timeout_ms)
> > +				unsigned int timeout_ms, u32 *out_val)
> > {
> > 	return xe_mmio_wait32(sc->mmio, SYSCTRL_MB_CTRL, bit_mask, bit_mask,
> > -			      timeout_ms * 1000, NULL, false);
> > +			      timeout_ms * 1000, out_val, false);
> > +}
> > +
> > +static void sysctrl_mb_ctrl_desc(u32 ctrl_reg, char *buf, size_t size)
> > +{
> > +	scnprintf(buf, size, "frame=%u/%u busy=%d resp=%d ctrl=%#010x",
> > +		  REG_FIELD_GET(SYSCTRL_FRAME_CURRENT_MASK, ctrl_reg),
> > +		  REG_FIELD_GET(SYSCTRL_FRAME_TOTAL_MASK, ctrl_reg),
> > +		  !!(ctrl_reg & SYSCTRL_MB_CTRL_RUN_BUSY),
> > +		  !!(ctrl_reg & SYSCTRL_MB_CTRL_RUN_BUSY_OUT),
> > +		  ctrl_reg);
> > }
> > 
> > static void sysctrl_write_frame(struct xe_sysctrl *sc, const void *frame,
> > @@ -128,17 +139,23 @@ static int sysctrl_send_frames(struct xe_sysctrl *sc,
> > 			       const u8 *mbox_cmd,
> > 			       size_t cmd_size, unsigned int timeout_ms)
> > {
> > +	const struct xe_sysctrl_mailbox_msg_hdr *hdr =
> > +		(const struct xe_sysctrl_mailbox_msg_hdr *)mbox_cmd;
> > 	struct xe_device *xe = sc_to_xe(sc);
> > 	u32 ctrl_reg, total_frames, frame;
> > 	size_t bytes_sent, frame_size;
> > +	char ctrl_desc[48];
> > 	bool phase;
> > 	int ret;
> > 
> > 	total_frames = DIV_ROUND_UP(cmd_size, XE_SYSCTRL_MB_FRAME_SIZE);
> > 
> > -	ret = sysctrl_wait_bit_clear(sc, SYSCTRL_MB_CTRL_RUN_BUSY, timeout_ms);
> > +	ret = sysctrl_wait_bit_clear(sc, SYSCTRL_MB_CTRL_RUN_BUSY, timeout_ms, &ctrl_reg);
> > 	if (ret) {
> > -		xe_log_err(xe, SYSCTRL, ret, "Mailbox busy\n");
> > +		sysctrl_mb_ctrl_desc(ctrl_reg, ctrl_desc, sizeof(ctrl_desc));
> > +		xe_log_err(xe, SYSCTRL, ret, "Mailbox busy %#x.%#x %s\n",
> > +			   XE_SYSCTRL_HDR_GROUP_ID(hdr), XE_SYSCTRL_HDR_COMMAND(hdr),
> > +			   ctrl_desc);
> > 		return ret;
> > 	}
> > 
> > @@ -159,9 +176,12 @@ static int sysctrl_send_frames(struct xe_sysctrl *sc,
> > 
> > 		xe_mmio_write32(sc->mmio, SYSCTRL_MB_CTRL, ctrl_reg);
> > 
> > -		ret = sysctrl_wait_bit_clear(sc, SYSCTRL_MB_CTRL_RUN_BUSY, timeout_ms);
> > +		ret = sysctrl_wait_bit_clear(sc, SYSCTRL_MB_CTRL_RUN_BUSY, timeout_ms, &ctrl_reg);
> > 		if (ret) {
> > -			xe_log_err(xe, SYSCTRL, ret, "Frame %u acknowledgment timeout\n", frame);
> > +			sysctrl_mb_ctrl_desc(ctrl_reg, ctrl_desc, sizeof(ctrl_desc));
> > +			xe_log_err(xe, SYSCTRL, ret, "Frame acknowledgment timeout %#x.%#x %s\n",
> > +				   XE_SYSCTRL_HDR_GROUP_ID(hdr), XE_SYSCTRL_HDR_COMMAND(hdr),
> > +				   ctrl_desc);
> > 			return ret;
> > 		}
> > 
> > @@ -171,17 +191,22 @@ static int sysctrl_send_frames(struct xe_sysctrl *sc,
> > 	return 0;
> > }
> > 
> > -static int sysctrl_process_frame(struct xe_sysctrl *sc, void *out,
> > -				 size_t frame_size, unsigned int timeout_ms,
> > +static int sysctrl_process_frame(struct xe_sysctrl *sc,
> > +				 const struct xe_sysctrl_mailbox_msg_hdr *req,
> > +				 void *out, size_t frame_size, unsigned int timeout_ms,
> > 				 bool *done)
> > {
> > 	u32 curr_frame, total_frames, ctrl_reg;
> > 	struct xe_device *xe = sc_to_xe(sc);
> > +	char ctrl_desc[48];
> > 	int ret;
> > 
> > -	ret = sysctrl_wait_bit_set(sc, SYSCTRL_MB_CTRL_RUN_BUSY_OUT, timeout_ms);
> > +	ret = sysctrl_wait_bit_set(sc, SYSCTRL_MB_CTRL_RUN_BUSY_OUT, timeout_ms, &ctrl_reg);
> > 	if (ret) {
> > -		xe_log_err(xe, SYSCTRL, ret, "Response frame timeout\n");
> > +		sysctrl_mb_ctrl_desc(ctrl_reg, ctrl_desc, sizeof(ctrl_desc));
> > +		xe_log_err(xe, SYSCTRL, ret, "Response frame timeout %#x.%#x %s\n",
> > +			   XE_SYSCTRL_HDR_GROUP_ID(req), XE_SYSCTRL_HDR_COMMAND(req),
> > +			   ctrl_desc);
> > 		return ret;
> > 	}
> > 
> > @@ -219,7 +244,7 @@ static int sysctrl_receive_frames(struct xe_sysctrl *sc,
> > 	while (!done && remain) {
> > 		frame_size = min_t(size_t, remain, XE_SYSCTRL_MB_FRAME_SIZE);
> > 
> > -		ret = sysctrl_process_frame(sc, out, frame_size, timeout_ms,
> > +		ret = sysctrl_process_frame(sc, req, out, frame_size, timeout_ms,
> > 					    &done);
> > 		if (ret)
> > 			return ret;
> > @@ -240,8 +265,9 @@ static int sysctrl_receive_frames(struct xe_sysctrl *sc,
> > 	}
> > 
> > 	if (XE_SYSCTRL_HDR_RESULT(hdr) != 0) {
> > -		xe_log_err(xe, SYSCTRL, -EIO, "Firmware error: %#04x\n",
> > -			   XE_SYSCTRL_HDR_RESULT(hdr));
> > +		xe_log_err(xe, SYSCTRL, -EIO, "Firmware error: %#04x %#x.%#x\n",
> > +			   XE_SYSCTRL_HDR_RESULT(hdr),
> > +			   XE_SYSCTRL_HDR_GROUP_ID(req), XE_SYSCTRL_HDR_COMMAND(req));
> > 		return -EIO;
> > 	}
> > 
> > -- 
> > 2.43.0
> > 

      reply	other threads:[~2026-10-10  1:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-09  6:47 [PATCH v2] drm/xe/sysctrl: Add mailbox control state to error messages Anoop, Vijay
2026-10-09  6:56 ` ✓ CI.KUnit: success for " Patchwork
2026-10-09  8:03 ` ✓ Xe.CI.BAT: " Patchwork
2026-10-09  9:29 ` ✓ Xe.CI.FULL: " Patchwork
2026-10-09 18:22 ` [PATCH v2] " Umesh Nerlige Ramappa
2026-10-10  1:00   ` Rodrigo Vivi [this message]

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=asmOJrnBwBSFIKBk@intel.com \
    --to=rodrigo.vivi@intel.com \
    --cc=anoop.c.vijay@intel.com \
    --cc=anshuman.gupta@intel.com \
    --cc=aravind.iddamsetty@intel.com \
    --cc=badal.nilawar@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=kam.nasim@intel.com \
    --cc=mallesh.koujalagi@intel.com \
    --cc=matthew.d.roper@intel.com \
    --cc=michael.j.ruhl@intel.com \
    --cc=michal.wajdeczko@intel.com \
    --cc=mohamed.mansoor.v@intel.com \
    --cc=paul.e.luse@intel.com \
    --cc=riana.tauro@intel.com \
    --cc=umesh.nerlige.ramappa@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.