Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
To: "Anoop, Vijay" <anoop.c.vijay@intel.com>
Cc: <intel-xe@lists.freedesktop.org>, <badal.nilawar@intel.com>,
	<rodrigo.vivi@intel.com>, <aravind.iddamsetty@intel.com>,
	<riana.tauro@intel.com>, <anshuman.gupta@intel.com>,
	<matthew.d.roper@intel.com>, <michael.j.ruhl@intel.com>,
	<paul.e.luse@intel.com>, <mohamed.mansoor.v@intel.com>,
	<kam.nasim@intel.com>
Subject: Re: [PATCH v4 1/4] drm/xe/sysctrl: Add System Controller get application status
Date: Thu, 3 Sep 2026 12:17:53 -0700	[thread overview]
Message-ID: <apnH4aA3m1B7UAkM@soc-5CG1426VCC.clients.intel.com> (raw)
In-Reply-To: <20260903140340.83500-7-anoop.c.vijay@intel.com>

On Thu, Sep 03, 2026 at 07:03:41AM -0700, Anoop, Vijay wrote:
>From: Anoop Vijay <anoop.c.vijay@intel.com>
>
>Add support for querying System Controller firmware application
>status. This allows callers to determine whether a given firmware
>application has completed boot and post-boot initialization.
>
>This patch is carried here only as a build dependency for this
>series; it is authored and under review as part of a separate
>get-app-status series and should be dropped/rebased away once that
>series lands.
>
>Link: https://lore.kernel.org/intel-xe/20260814095306.2482289-5-anoop.c.vijay@intel.com/
>Link: https://lore.kernel.org/intel-xe/20260814095306.2482289-6-anoop.c.vijay@intel.com/
>
>Signed-off-by: Anoop Vijay <anoop.c.vijay@intel.com>
>---
>v4 (Rodrigo):
>- Added lore links to the parent application-status series
>---
> drivers/gpu/drm/xe/xe_sysctrl.c               | 82 +++++++++++++++++++
> drivers/gpu/drm/xe/xe_sysctrl.h               |  2 +
> drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h | 65 +++++++++++++++
> 3 files changed, 149 insertions(+)
>
>diff --git a/drivers/gpu/drm/xe/xe_sysctrl.c b/drivers/gpu/drm/xe/xe_sysctrl.c
>index 62ccc9be71b4..5da591ade4e4 100644
>--- a/drivers/gpu/drm/xe/xe_sysctrl.c
>+++ b/drivers/gpu/drm/xe/xe_sysctrl.c
>@@ -13,9 +13,11 @@
> #include "xe_device.h"
> #include "xe_mmio.h"
> #include "xe_pm.h"
>+#include "xe_printk.h"
> #include "xe_soc_remapper.h"
> #include "xe_sysctrl.h"
> #include "xe_sysctrl_mailbox.h"
>+#include "xe_sysctrl_mailbox_types.h"
> #include "xe_sysctrl_types.h"
>
> /**
>@@ -125,3 +127,83 @@ void xe_sysctrl_pm_resume(struct xe_device *xe)
>
> 	xe->soc_remapper.set_sysctrl_region(xe, SYSCTRL_MAILBOX_INDEX);
> }
>+
>+static enum xe_sysctrl_fw_status
>+xe_sysctrl_check_app_status(struct xe_device *xe, enum xe_sysctrl_app_id app_id)
>+{
>+	struct xe_sysctrl_app_status_req req = {};
>+	struct xe_sysctrl_app_status_resp resp = {};
>+	struct xe_sysctrl_mailbox_command cmd = {};
>+	size_t out_len = 0;
>+	u32 flags;
>+	int ret;
>+
>+	if (!xe->info.has_sysctrl)
>+		return XE_SYSCTRL_FIRMWARE_APP_NOTSUPP;

Reiterating previous review comment - based on the users that you had 
listed earlier who query app status, we will never get to the above 
condition, so it can be safely dropped. Once you do that, you can drop 
the status == XE_SYSCTRL_FIRMWARE_APP_NOTSUPP checks in the below 
(_ready) helpers.

Instead, if you want to retain this check here, then each of the below 
helpers must be split into _ready() and _supported(). The split versions 
should check for the specific case. It's the caller's responsibility to 
handle the cases as needed.

>+
>+	req.app_id = (u8)app_id;
>+
>+	xe_sysctrl_create_command(&cmd, XE_SYSCTRL_GROUP_CORE, XE_SYSCTRL_CMD_GET_APP_STATUS_BY_ID,
>+				  &req, sizeof(req), &resp, sizeof(resp));
>+
>+	ret = xe_sysctrl_send_command(&xe->sc, &cmd, &out_len);
>+	if (ret)
>+		return XE_SYSCTRL_FIRMWARE_COMM_FAILURE;
>+
>+	if (out_len != sizeof(resp)) {
>+		xe_err(xe, "sysctrl: unexpected get app status response length %zu (expected %zu)\n",
>+		       out_len, sizeof(resp));
>+		return XE_SYSCTRL_FIRMWARE_COMM_FAILURE;
>+	}
>+
>+	flags = resp.flags;
>+
>+	if (!(flags & XE_SYSCTRL_APP_RESP_VALID))
>+		return XE_SYSCTRL_FIRMWARE_APP_INVALID;
>+
>+	if (!(flags & XE_SYSCTRL_APP_RESP_BOOTED))
>+		return XE_SYSCTRL_FIRMWARE_APP_NOT_LOADED;
>+
>+	if (!(flags & XE_SYSCTRL_APP_RESP_INITIALIZED))
>+		return XE_SYSCTRL_FIRMWARE_APP_BOOTED;
>+
>+	return XE_SYSCTRL_FIRMWARE_APP_INITIALIZED;
>+}
>+
>+/**
>+ * xe_sysctrl_is_oobmsm_fw_ready() - Check if oCode firmware is fully initialized
>+ * @xe: xe device instance
>+ *
>+ * Returns true if oCode firmware has reached the initialized state, indicating
>+ * it is ready to handle requests. Returns true also on platforms without System
>+ * Controller support, as there is no firmware gate to wait on.
>+ *
>+ * Return: true if oCode is initialized or sysctrl is not present, false otherwise
>+ */
>+bool xe_sysctrl_is_oobmsm_fw_ready(struct xe_device *xe)
>+{
>+	enum xe_sysctrl_fw_status status =
>+		xe_sysctrl_check_app_status(xe, XE_SYSCTRL_APP_OCODE);
>+
>+	return status == XE_SYSCTRL_FIRMWARE_APP_INITIALIZED ||
>+	       status == XE_SYSCTRL_FIRMWARE_APP_NOTSUPP;

See above comment.

>+}
>+
>+/**
>+ * xe_sysctrl_is_diag_fw_ready() - Check if diag firmware is fully initialized
>+ * @xe: xe device instance
>+ *
>+ * Returns true if diag firmware has reached the initialized state, indicating
>+ * it is ready to handle requests. Returns true also on platforms without System
>+ * Controller support, as there is no firmware gate to wait on.
>+ *
>+ * Return: true if diag firmware is initialized or sysctrl is not present, false otherwise
>+ */
>+bool xe_sysctrl_is_diag_fw_ready(struct xe_device *xe)
>+{
>+	enum xe_sysctrl_fw_status status =
>+		xe_sysctrl_check_app_status(xe, XE_SYSCTRL_APP_DIAG);
>+
>+	return status == XE_SYSCTRL_FIRMWARE_APP_INITIALIZED ||
>+	       status == XE_SYSCTRL_FIRMWARE_APP_NOTSUPP;

See above comment.

Thanks,
Umesh
>+}
>diff --git a/drivers/gpu/drm/xe/xe_sysctrl.h b/drivers/gpu/drm/xe/xe_sysctrl.h
>index 090dffb6d55f..b69a3f474236 100644
>--- a/drivers/gpu/drm/xe/xe_sysctrl.h
>+++ b/drivers/gpu/drm/xe/xe_sysctrl.h
>@@ -20,5 +20,7 @@ void xe_sysctrl_event(struct xe_sysctrl *sc);
> int xe_sysctrl_init(struct xe_device *xe);
> void xe_sysctrl_irq_handler(struct xe_device *xe, u32 master_ctl);
> void xe_sysctrl_pm_resume(struct xe_device *xe);
>+bool xe_sysctrl_is_oobmsm_fw_ready(struct xe_device *xe);
>+bool xe_sysctrl_is_diag_fw_ready(struct xe_device *xe);
>
> #endif
>diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
>index 66e7cbcc3f91..d756b50cde6b 100644
>--- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
>+++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
>@@ -14,9 +14,11 @@
>  * enum xe_sysctrl_group - System Controller command groups
>  *
>  * @XE_SYSCTRL_GROUP_GFSP: GFSP group
>+ * @XE_SYSCTRL_GROUP_CORE: Core group
>  */
> enum xe_sysctrl_group {
> 	XE_SYSCTRL_GROUP_GFSP			= 0x01,
>+	XE_SYSCTRL_GROUP_CORE			= 0xFF,
> };
>
> /**
>@@ -42,6 +44,69 @@ enum xe_sysctrl_gfsp_cmd {
> 	XE_SYSCTRL_CMD_SET_HEALTH		= 0x0C,
> };
>
>+/**
>+ * enum xe_sysctrl_core_cmd - Commands supported by Core group
>+ *
>+ * @XE_SYSCTRL_CMD_GET_APP_STATUS_BY_ID: Retrieve application status by ID
>+ */
>+enum xe_sysctrl_core_cmd {
>+	XE_SYSCTRL_CMD_GET_APP_STATUS_BY_ID		= 0x05,
>+};
>+
>+/**
>+ * struct xe_sysctrl_app_status_req - Get application status request
>+ *
>+ * @app_id: Application ID for which to retrieve status
>+ */
>+struct xe_sysctrl_app_status_req {
>+	u8 app_id;
>+} __packed;
>+
>+/**
>+ * struct xe_sysctrl_app_status_resp - Get application status response
>+ * @flags: Application status flags (see XE_SYSCTRL_APP_RESP_* definitions)
>+ */
>+struct xe_sysctrl_app_status_resp {
>+	u32 flags;
>+} __packed;
>+
>+/** XE_SYSCTRL_APP_RESP_VALID - app_id is recognized by System Controller firmware */
>+#define XE_SYSCTRL_APP_RESP_VALID		BIT(0)
>+/** XE_SYSCTRL_APP_RESP_BOOTED - application has completed its boot sequence */
>+#define XE_SYSCTRL_APP_RESP_BOOTED		BIT(1)
>+/** XE_SYSCTRL_APP_RESP_INITIALIZED - application has completed all post-boot initialization */
>+#define XE_SYSCTRL_APP_RESP_INITIALIZED		BIT(2)
>+
>+/**
>+ * enum xe_sysctrl_fw_status - System Controller firmware application lifecycle states
>+ *
>+ * @XE_SYSCTRL_FIRMWARE_APP_INVALID: app_id is not recognized by firmware
>+ * @XE_SYSCTRL_FIRMWARE_APP_NOT_LOADED: application is known but has not yet booted
>+ * @XE_SYSCTRL_FIRMWARE_APP_BOOTED: boot sequence completed, post-boot init pending
>+ * @XE_SYSCTRL_FIRMWARE_APP_INITIALIZED: application fully operational
>+ * @XE_SYSCTRL_FIRMWARE_APP_NOTSUPP: System Controller not available on this device
>+ * @XE_SYSCTRL_FIRMWARE_COMM_FAILURE: communication with System Controller firmware failed
>+ */
>+enum xe_sysctrl_fw_status {
>+	XE_SYSCTRL_FIRMWARE_APP_INVALID,
>+	XE_SYSCTRL_FIRMWARE_APP_NOT_LOADED,
>+	XE_SYSCTRL_FIRMWARE_APP_BOOTED,
>+	XE_SYSCTRL_FIRMWARE_APP_INITIALIZED,
>+	XE_SYSCTRL_FIRMWARE_APP_NOTSUPP,
>+	XE_SYSCTRL_FIRMWARE_COMM_FAILURE,
>+};
>+
>+/**
>+ * enum xe_sysctrl_app_id - Known System Controller application identifiers
>+ *
>+ * @XE_SYSCTRL_APP_OCODE: oCode application (firmware ID 12)
>+ * @XE_SYSCTRL_APP_DIAG: diag application (firmware ID 13)
>+ */
>+enum xe_sysctrl_app_id {
>+	XE_SYSCTRL_APP_OCODE	= 0x0C,
>+	XE_SYSCTRL_APP_DIAG	= 0x0D,
>+};
>+
> /**
>  * struct xe_sysctrl_mailbox_command - System Controller mailbox command
>  */
>-- 
>2.43.0
>

  parent reply	other threads:[~2026-09-03 19:18 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-03 14:03 [PATCH v4 0/4] drm/xe/sysctrl: Add System Controller debugfs Anoop, Vijay
2026-09-03 14:03 ` [PATCH v4 1/4] drm/xe/sysctrl: Add System Controller get application status Anoop, Vijay
2026-09-03 14:16   ` sashiko-bot
2026-09-03 19:17   ` Umesh Nerlige Ramappa [this message]
2026-09-03 20:12     ` Anoop Vijay
2026-09-03 20:42       ` Ruhl, Michael J
2026-09-04  3:13         ` Anoop Vijay
2026-09-03 14:03 ` [PATCH v4 2/4] drm/xe/sysctrl: Add sysctrl debugfs infrastructure and loopback test interface Anoop, Vijay
2026-09-03 14:13   ` sashiko-bot
2026-09-03 14:03 ` [PATCH v4 3/4] drm/xe/sysctrl: Add RAS error injection debugfs interface Anoop, Vijay
2026-09-03 14:13   ` sashiko-bot
2026-09-03 14:03 ` [PATCH v4 4/4] drm/xe/sysctrl: Add generic mailbox passthrough debugfs entry Anoop, Vijay
2026-09-03 14:12   ` sashiko-bot
2026-09-03 14:10 ` ✗ CI.checkpatch: warning for drm/xe/sysctrl: Add System Controller debugfs (rev4) Patchwork
2026-09-03 14:12 ` ✓ CI.KUnit: success " Patchwork
2026-09-03 15:02 ` ✓ Xe.CI.BAT: " Patchwork
2026-09-04  1:27 ` ✓ Xe.CI.FULL: " Patchwork

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=apnH4aA3m1B7UAkM@soc-5CG1426VCC.clients.intel.com \
    --to=umesh.nerlige.ramappa@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=matthew.d.roper@intel.com \
    --cc=michael.j.ruhl@intel.com \
    --cc=mohamed.mansoor.v@intel.com \
    --cc=paul.e.luse@intel.com \
    --cc=riana.tauro@intel.com \
    --cc=rodrigo.vivi@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