On 04-09-2026 02:12, Ruhl, Michael J wrote: > > Anoop, > > The XE VSEC code will only use this API if the device is CRI, so yes, > removing the NOTSUPP > > will work. > > Mike > Thanks for confirming. I will drop NOTSUPP handling and the corresponding checks in the _ready() helpers. Anoop > *From:*Vijay, Anoop C > *Sent:* Thursday, September 3, 2026 4:13 PM > *To:* Nerlige Ramappa, Umesh > *Cc:* intel-xe@lists.freedesktop.org; Nilawar, Badal > ; Vivi, Rodrigo ; > Iddamsetty, Aravind ; Tauro, Riana > ; Gupta, Anshuman ; > Roper, Matthew D ; Ruhl, Michael J > ; Luse, Paul E ; V, > Mohamed Mansoor ; Nasim, Kam > > *Subject:* Re: [PATCH v4 1/4] drm/xe/sysctrl: Add System Controller > get application status > > On 04-09-2026 00:47, Umesh Nerlige Ramappa wrote: > > On Thu, Sep 03, 2026 at 07:03:41AM -0700, Anoop, Vijay wrote: > > From: Anoop Vijay > > > 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 > > --- > 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. > > APP_NOTSUPP was added based on Mike's earlier feedback to handle > non-SysCtrl platforms. > > @Ruhl, Michael J , can xe_vsec > perform platform check and only call _fw_ready() on SysCtrl-capable > platforms? If so, I'll remove APP_NOTSUPP. Otherwise, do you need > separate _supported() and _ready() helpers? > > > > + > +    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 >