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
> >
prev parent 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox