* Re: [PATCH v2 1/1] drm/xe/sysctrl: Add system controller component for Xe3p dGPU platforms
2026-01-02 16:54 ` [PATCH v2 1/1] " Anoop, Vijay
@ 2026-01-02 23:15 ` Matthew Brost
2026-01-05 6:29 ` Riana Tauro
2026-01-06 18:37 ` Umesh Nerlige Ramappa
2 siblings, 0 replies; 8+ messages in thread
From: Matthew Brost @ 2026-01-02 23:15 UTC (permalink / raw)
To: Anoop, Vijay
Cc: intel-xe, umesh.nerlige.ramappa, badal.nilawar, rodrigo.vivi,
aravind.iddamsetty, riana.tauro, ravi.kishore.koppuravuri,
anshuman.gupta, matthew.d.roper, michael.j.ruhl, paul.e.luse,
mohamed.mansoor.v, kam.nasim
On Fri, Jan 02, 2026 at 08:54:50AM -0800, Anoop, Vijay wrote:
> From: Anoop Vijay <anoop.c.vijay@intel.com>
>
> Add a new system controller (sysctrl) component for Intel Xe3p dGPU
> platforms.
>
> This component provides the foundational infrastructure for communication
> with the System Controller firmware using MKHI protocol over a mailbox
> interface.
>
> Key features introduced:
> - Detection and initialization of System Controller interface on Xe3p
> dGPU platforms
> - Mailbox communication with System Controller firmware
> - Fragmented message transfer for large command payloads
>
> This implementation establishes the base for future System Controller
> feature enablement and firmware command handling.
>
> Signed-off-by: Anoop Vijay <anoop.c.vijay@intel.com>
Drive by comment - this patch needs to be split into a series of smaller
patches for review. A single large patch is highly unlikely to get
reviewed or pulled by a maintainer of Xe or DRM.
Off the top of my head, a sample split could be:
- Define xe_sysctrl_regs.h
- Add has_sysctrl to xe_device / xe_pci
- Implement xe_sysctrl_mailbox
- Implement xe_sysctrl
- Call xe_sysctrl_init during device probe.
Matt
> ---
> drivers/gpu/drm/xe/Makefile | 2 +
> drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h | 44 +++
> drivers/gpu/drm/xe/xe_device.c | 5 +
> drivers/gpu/drm/xe/xe_device_types.h | 6 +
> drivers/gpu/drm/xe/xe_pci.c | 2 +
> drivers/gpu/drm/xe/xe_pci_types.h | 1 +
> drivers/gpu/drm/xe/xe_sysctrl.c | 62 ++++
> drivers/gpu/drm/xe/xe_sysctrl.h | 18 +
> drivers/gpu/drm/xe/xe_sysctrl_mailbox.c | 409 ++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_sysctrl_mailbox.h | 77 ++++
> drivers/gpu/drm/xe/xe_sysctrl_types.h | 23 ++
> 11 files changed, 649 insertions(+)
> create mode 100644 drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl.c
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl.h
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl_mailbox.h
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl_types.h
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 2b20c79d7ec9..947fbcac65d5 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -121,6 +121,8 @@ xe-y += xe_bb.o \
> xe_step.o \
> xe_survivability_mode.o \
> xe_sync.o \
> + xe_sysctrl.o \
> + xe_sysctrl_mailbox.o \
> xe_tile.o \
> xe_tile_sysfs.o \
> xe_tlb_inval.o \
> diff --git a/drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h b/drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h
> new file mode 100644
> index 000000000000..6627a9c32c4f
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#ifndef _XE_SYSCTRL_REGS_H_
> +#define _XE_SYSCTRL_REGS_H_
> +
> +#include "xe_regs.h"
> +
> +#define SYSCTRL_BASE_OFFSET 0xDB000
> +#define SYSCTRL_BASE (SOC_BASE + SYSCTRL_BASE_OFFSET)
> +#define SYSCTRL_MAILBOX_INDEX 0x03
> +#define SC_BAR_LENGTH 0x1000
> +
> +#define SC_MB_CTRL XE_REG(SYSCTRL_BASE + 0x10)
> +#define SC_MB_CTRL_RUN_BUSY REG_BIT(31)
> +#define SC_MB_CTRL_IRQ REG_BIT(30)
> +#define SC_MB_CTRL_RUN_BUSY_OUT REG_BIT(29)
> +#define SC_MB_CTRL_PARAM3_MASK REG_GENMASK(28, 24)
> +#define SC_MB_CTRL_PARAM2_MASK REG_GENMASK(23, 16)
> +#define SC_MB_CTRL_PARAM1_MASK REG_GENMASK(15, 8)
> +#define SC_MB_CTRL_COMMAND_MASK REG_GENMASK(7, 0)
> +
> +#define SC_MB_DATA0 XE_REG(SYSCTRL_BASE + 0x14)
> +#define SC_MB_DATA1 XE_REG(SYSCTRL_BASE + 0x18)
> +#define SC_MB_DATA2 XE_REG(SYSCTRL_BASE + 0x1C)
> +#define SC_MB_DATA3 XE_REG(SYSCTRL_BASE + 0x20)
> +
> +#define MKHI_FRAME_PHASE REG_BIT(24)
> +#define MKHI_FRAME_CURRENT_MASK REG_GENMASK(21, 16)
> +#define MKHI_FRAME_TOTAL_MASK REG_GENMASK(13, 8)
> +#define MKHI_FRAME_COMMAND_MASK REG_GENMASK(7, 0)
> +
> +#define SC_MB_FRAME_SIZE 16
> +#define SC_MB_MAX_FRAMES 64
> +#define SC_MB_MAX_MESSAGE_SIZE (SC_MB_FRAME_SIZE * SC_MB_MAX_FRAMES)
> +#define SC_MKHI_COMMAND 5
> +
> +#define SC_MB_DEFAULT_TIMEOUT_MS 500
> +#define SC_MB_RETRY_TIMEOUT_MS 20
> +#define SC_MB_POLL_INTERVAL_US 100
> +
> +#endif /* _XE_SYSCTRL_REGS_H_ */
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index e101d290b2a6..805d48dd954d 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -66,6 +66,7 @@
> #include "xe_survivability_mode.h"
> #include "xe_sriov.h"
> #include "xe_svm.h"
> +#include "xe_sysctrl.h"
> #include "xe_tile.h"
> #include "xe_ttm_stolen_mgr.h"
> #include "xe_ttm_sys_mgr.h"
> @@ -1032,6 +1033,10 @@ int xe_device_probe(struct xe_device *xe)
> if (err)
> goto err_unregister_display;
>
> + err = xe_sysctrl_init(xe);
> + if (err)
> + goto err_unregister_display;
> +
> err = xe_device_sysfs_init(xe);
> if (err)
> goto err_unregister_display;
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index a85be9ba175e..6295b2c35d4a 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -29,6 +29,7 @@
> #include "xe_sriov_vf_ccs_types.h"
> #include "xe_step_types.h"
> #include "xe_survivability_mode_types.h"
> +#include "xe_sysctrl_types.h"
> #include "xe_tile_sriov_vf_types.h"
> #include "xe_validation.h"
>
> @@ -340,6 +341,8 @@ struct xe_device {
> u8 has_soc_remapper_telem:1;
> /** @info.has_sriov: Supports SR-IOV */
> u8 has_sriov:1;
> + /** @info.has_sysctrl: Supports System Controller */
> + u8 has_sysctrl:1;
> /** @info.has_usm: Device has unified shared memory support */
> u8 has_usm:1;
> /** @info.has_64bit_timestamp: Device supports 64-bit timestamps */
> @@ -606,6 +609,9 @@ struct xe_device {
> /** @heci_gsc: graphics security controller */
> struct xe_heci_gsc heci_gsc;
>
> + /** @sc: System Controller */
> + struct xe_sysctrl sc;
> +
> /** @nvm: discrete graphics non-volatile memory */
> struct intel_dg_nvm_dev *nvm;
>
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 91e0553a8163..b6dc3030b673 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -426,6 +426,7 @@ static const struct xe_device_desc cri_desc = {
> .has_soc_remapper_sysctrl = true,
> .has_soc_remapper_telem = true,
> .has_sriov = true,
> + .has_sysctrl = true,
> .max_gt_per_tile = 2,
> .require_force_probe = true,
> .va_bits = 57,
> @@ -701,6 +702,7 @@ static int xe_info_init_early(struct xe_device *xe,
> xe->info.has_soc_remapper_telem = desc->has_soc_remapper_telem;
> xe->info.has_sriov = xe_configfs_primary_gt_allowed(to_pci_dev(xe->drm.dev)) &&
> desc->has_sriov;
> + xe->info.has_sysctrl = desc->has_sysctrl;
> xe->info.has_mem_copy_instr = desc->has_mem_copy_instr;
> xe->info.skip_guc_pc = desc->skip_guc_pc;
> xe->info.skip_mtcfg = desc->skip_mtcfg;
> diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
> index 5f20f56571d1..53e44a32883d 100644
> --- a/drivers/gpu/drm/xe/xe_pci_types.h
> +++ b/drivers/gpu/drm/xe/xe_pci_types.h
> @@ -56,6 +56,7 @@ struct xe_device_desc {
> u8 has_soc_remapper_sysctrl:1;
> u8 has_soc_remapper_telem:1;
> u8 has_sriov:1;
> + u8 has_sysctrl:1;
> u8 needs_scratch:1;
> u8 skip_guc_pc:1;
> u8 skip_mtcfg:1;
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl.c b/drivers/gpu/drm/xe/xe_sysctrl.c
> new file mode 100644
> index 000000000000..e0e7b0ecf2bf
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_sysctrl.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#include <drm/drm_managed.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +
> +#include "regs/xe_sysctrl_regs.h"
> +#include "xe_device.h"
> +#include "xe_printk.h"
> +#include "xe_soc_remapper.h"
> +#include "xe_sysctrl.h"
> +#include "xe_sysctrl_mailbox.h"
> +#include "xe_sysctrl_types.h"
> +
> +static void xe_sysctrl_fini(void *arg)
> +{
> + struct xe_sysctrl *sc = arg;
> + struct xe_device *xe = sc_to_xe(sc);
> +
> + if (!xe->soc_remapper.set_sysctrl_region)
> + return;
> +
> + xe->soc_remapper.set_sysctrl_region(xe, 0);
> +}
> +
> +/**
> + * xe_sysctrl_init - Initialize SC subsystem
> + * @xe: xe device instance
> + *
> + * Entry point for SC initialization, called from xe_device_probe().
> + * This function checks platform support and initializes the system controller.
> + *
> + * Return: 0 on success, error code on failure
> + */
> +int xe_sysctrl_init(struct xe_device *xe)
> +{
> + struct xe_sysctrl *sc = &xe->sc;
> + int ret;
> +
> + if (!xe->info.has_sysctrl)
> + return 0;
> +
> + ret = devm_add_action_or_reset(xe->drm.dev, xe_sysctrl_fini, sc);
> + if (ret)
> + return ret;
> +
> + if (!xe->soc_remapper.set_sysctrl_region)
> + return -ENODEV;
> +
> + xe->soc_remapper.set_sysctrl_region(xe, SYSCTRL_MAILBOX_INDEX);
> +
> + ret = drmm_mutex_init(&xe->drm, &sc->cmd_lock);
> + if (ret)
> + return ret;
> +
> + xe_sysctrl_mailbox_init(sc);
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl.h b/drivers/gpu/drm/xe/xe_sysctrl.h
> new file mode 100644
> index 000000000000..fe90d6577d54
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_sysctrl.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#ifndef _XE_SYSCTRL_H_
> +#define _XE_SYSCTRL_H_
> +
> +struct xe_device;
> +
> +static inline struct xe_device *sc_to_xe(struct xe_sysctrl *sc)
> +{
> + return container_of(sc, struct xe_device, sc);
> +}
> +
> +int xe_sysctrl_init(struct xe_device *xe);
> +
> +#endif /* _XE_SYSCTRL_H_ */
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> new file mode 100644
> index 000000000000..940ea535da2e
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> @@ -0,0 +1,409 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/errno.h>
> +#include <linux/minmax.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +
> +#include "regs/xe_sysctrl_regs.h"
> +#include "xe_device.h"
> +#include "xe_mmio.h"
> +#include "xe_pm.h"
> +#include "xe_printk.h"
> +#include "xe_sysctrl.h"
> +#include "xe_sysctrl_mailbox.h"
> +#include "xe_sysctrl_types.h"
> +
> +static bool xe_sysctrl_mailbox_wait_bit_clear(struct xe_sysctrl *sc, u32 bit_mask,
> + unsigned int timeout_ms)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> + int ret;
> +
> + ret = xe_mmio_wait32_not(mmio, SC_MB_CTRL, bit_mask, bit_mask,
> + timeout_ms * 1000, NULL, false);
> +
> + return ret == 0;
> +}
> +
> +static bool xe_sysctrl_mailbox_wait_bit_set(struct xe_sysctrl *sc, u32 bit_mask,
> + unsigned int timeout_ms)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> + int ret;
> +
> + ret = xe_mmio_wait32(mmio, SC_MB_CTRL, bit_mask, bit_mask,
> + timeout_ms * 1000, NULL, false);
> +
> + return ret == 0;
> +}
> +
> +static int xe_sysctrl_mailbox_write_frame(struct xe_sysctrl *sc, const void *frame,
> + size_t len)
> +{
> + static const struct xe_reg regs[] = {
> + SC_MB_DATA0, SC_MB_DATA1, SC_MB_DATA2, SC_MB_DATA3
> + };
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> + u32 val[SC_MB_FRAME_SIZE / sizeof(u32)] = {0};
> + u32 dw = DIV_ROUND_UP(len, sizeof(u32));
> + u32 i;
> +
> + memcpy(val, frame, len);
> +
> + for (i = 0; i < dw; i++)
> + xe_mmio_write32(mmio, regs[i], val[i]);
> +
> + return 0;
> +}
> +
> +static int xe_sysctrl_mailbox_read_frame(struct xe_sysctrl *sc, void *frame,
> + size_t len)
> +{
> + static const struct xe_reg regs[] = {
> + SC_MB_DATA0, SC_MB_DATA1, SC_MB_DATA2, SC_MB_DATA3
> + };
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> + u32 val[SC_MB_FRAME_SIZE / sizeof(u32)] = {0};
> + u32 dw = DIV_ROUND_UP(len, sizeof(u32));
> + u32 i;
> +
> + for (i = 0; i < dw; i++)
> + val[i] = xe_mmio_read32(mmio, regs[i]);
> +
> + memcpy(frame, val, len);
> +
> + return 0;
> +}
> +
> +static void xe_sysctrl_mailbox_clear_response(struct xe_sysctrl *sc)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> +
> + xe_mmio_rmw32(mmio, SC_MB_CTRL, SC_MB_CTRL_RUN_BUSY_OUT, 0);
> +}
> +
> +static int xe_sysctrl_mailbox_prepare_command(struct xe_sysctrl *sc,
> + u8 group_id, u8 command,
> + const void *data_in, size_t data_in_len,
> + u8 **mbox_cmd, size_t *cmd_size)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_sysctrl_mailbox_mkhi_msg_hdr *mkhi_hdr;
> + size_t size;
> + u8 *buffer;
> +
> + size = sizeof(*mkhi_hdr) + data_in_len;
> + if (size > SC_MB_MAX_MESSAGE_SIZE) {
> + xe_err(xe, "sysctrl: Message too large: %zu bytes\n", size);
> + return -EINVAL;
> + }
> +
> + buffer = kmalloc(size, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
> +
> + mkhi_hdr = (struct xe_sysctrl_mailbox_mkhi_msg_hdr *)buffer;
> + mkhi_hdr->data = cpu_to_le32(FIELD_PREP(MKHI_HDR_GROUP_ID_MASK, group_id) |
> + FIELD_PREP(MKHI_HDR_COMMAND_MASK, command & 0x7F) |
> + FIELD_PREP(MKHI_HDR_IS_RESPONSE, 0) |
> + FIELD_PREP(MKHI_HDR_RESERVED_MASK, 0) |
> + FIELD_PREP(MKHI_HDR_RESULT_MASK, 0));
> +
> + if (data_in && data_in_len)
> + memcpy(buffer + sizeof(*mkhi_hdr), data_in, data_in_len);
> +
> + *mbox_cmd = buffer;
> + *cmd_size = size;
> +
> + return 0;
> +}
> +
> +static int xe_sysctrl_mailbox_send_frames(struct xe_sysctrl *sc, const u8 *mbox_cmd,
> + size_t cmd_size, unsigned int timeout_ms)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> + u32 ctrl_reg, total_frames, frame;
> + size_t bytes_sent, frame_size;
> +
> + total_frames = DIV_ROUND_UP(cmd_size, SC_MB_FRAME_SIZE);
> +
> + if (!xe_sysctrl_mailbox_wait_bit_clear(sc, SC_MB_CTRL_RUN_BUSY, timeout_ms)) {
> + xe_err(xe, "sysctrl: Mailbox busy\n");
> + return -EBUSY;
> + }
> +
> + sc->phase_bit ^= 1;
> + bytes_sent = 0;
> +
> + for (frame = 0; frame < total_frames; frame++) {
> + frame_size = min(cmd_size - bytes_sent, (size_t)SC_MB_FRAME_SIZE);
> +
> + if (xe_sysctrl_mailbox_write_frame(sc, mbox_cmd + bytes_sent, frame_size)) {
> + xe_err(xe, "sysctrl: Failed to write frame %u\n", frame);
> + sc->phase_bit ^= 1;
> + return -EIO;
> + }
> +
> + ctrl_reg = SC_MB_CTRL_RUN_BUSY |
> + FIELD_PREP(MKHI_FRAME_CURRENT_MASK, frame) |
> + FIELD_PREP(MKHI_FRAME_TOTAL_MASK, total_frames - 1) |
> + FIELD_PREP(MKHI_FRAME_COMMAND_MASK, SC_MKHI_COMMAND) |
> + (sc->phase_bit ? MKHI_FRAME_PHASE : 0);
> +
> + xe_mmio_write32(mmio, SC_MB_CTRL, ctrl_reg);
> +
> + if (!xe_sysctrl_mailbox_wait_bit_clear(sc, SC_MB_CTRL_RUN_BUSY, timeout_ms)) {
> + xe_err(xe, "sysctrl: Frame %u acknowledgment timeout\n", frame);
> + return -ETIMEDOUT;
> + }
> +
> + bytes_sent += frame_size;
> + }
> +
> + return 0;
> +}
> +
> +static int xe_sysctrl_mailbox_process_first_frame(struct xe_sysctrl *sc,
> + const struct xe_sysctrl_mailbox_mkhi_msg_hdr *req,
> + void *out,
> + size_t frame_size,
> + size_t *payload_bytes)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + u32 frame_data[4];
> + struct xe_sysctrl_mailbox_mkhi_msg_hdr *resp_hdr;
> + size_t hdr_size = sizeof(*resp_hdr);
> + size_t payload_size;
> + int ret;
> +
> + ret = xe_sysctrl_mailbox_read_frame(sc, frame_data, frame_size);
> + if (ret)
> + return ret;
> +
> + resp_hdr = (struct xe_sysctrl_mailbox_mkhi_msg_hdr *)frame_data;
> +
> + if (!XE_SYSCTRL_MKHI_HDR_IS_RESPONSE(resp_hdr) ||
> + XE_SYSCTRL_MKHI_HDR_GROUP_ID(resp_hdr) != XE_SYSCTRL_MKHI_HDR_GROUP_ID(req) ||
> + XE_SYSCTRL_MKHI_HDR_COMMAND(resp_hdr) != XE_SYSCTRL_MKHI_HDR_COMMAND(req)) {
> + xe_err(xe, "SC: Response header mismatch\n");
> + return -EPROTO;
> + }
> +
> + if (XE_SYSCTRL_MKHI_HDR_RESULT(resp_hdr) != 0) {
> + xe_err(xe, "SC: Firmware error: 0x%02lx\n",
> + XE_SYSCTRL_MKHI_HDR_RESULT(resp_hdr));
> + return -EIO;
> + }
> +
> + payload_size = frame_size - hdr_size;
> + if (payload_size > 0)
> + memcpy(out, (u8 *)frame_data + hdr_size, payload_size);
> +
> + *payload_bytes = payload_size;
> +
> + xe_sysctrl_mailbox_clear_response(sc);
> +
> + return 0;
> +}
> +
> +static int xe_sysctrl_mailbox_process_frame(struct xe_sysctrl *sc,
> + void *out, size_t frame_size,
> + unsigned int timeout_ms)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + int ret;
> +
> + if (!xe_sysctrl_mailbox_wait_bit_set(sc, SC_MB_CTRL_RUN_BUSY_OUT, timeout_ms)) {
> + xe_err(xe, "sysctrl: Response frame timeout\n");
> + return -ETIMEDOUT;
> + }
> +
> + ret = xe_sysctrl_mailbox_read_frame(sc, out, frame_size);
> + if (ret)
> + return ret;
> +
> + xe_sysctrl_mailbox_clear_response(sc);
> +
> + return 0;
> +}
> +
> +static int xe_sysctrl_mailbox_receive_frames(struct xe_sysctrl *sc,
> + const struct xe_sysctrl_mailbox_mkhi_msg_hdr *req,
> + void *data_out, size_t data_out_len,
> + size_t *rdata_len, unsigned int timeout_ms)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> + struct xe_sysctrl_mailbox_mkhi_msg_hdr *mkhi_hdr;
> + u32 ctrl_reg, total_frames, frame;
> + size_t hdr_size = sizeof(*mkhi_hdr);
> + u8 *out = data_out;
> + size_t received = 0;
> + size_t frame_size;
> + int ret = 0;
> +
> + if (!xe_sysctrl_mailbox_wait_bit_set(sc, SC_MB_CTRL_RUN_BUSY_OUT, timeout_ms)) {
> + xe_err(xe, "sysctrl: Response frame 0 timeout\n");
> + return -ETIMEDOUT;
> + }
> +
> + ctrl_reg = xe_mmio_read32(mmio, SC_MB_CTRL);
> + total_frames = FIELD_GET(MKHI_FRAME_TOTAL_MASK, ctrl_reg) + 1;
> +
> + if (total_frames == 1)
> + frame_size = min(hdr_size + data_out_len, (size_t)SC_MB_FRAME_SIZE);
> + else
> + frame_size = SC_MB_FRAME_SIZE;
> +
> + ret = xe_sysctrl_mailbox_process_first_frame(sc, req, out, frame_size, &received);
> + if (ret)
> + return ret;
> +
> + out += received;
> +
> + for (frame = 1; frame < total_frames; frame++) {
> + size_t remaining = data_out_len - received;
> +
> + frame_size = min_t(size_t, remaining, SC_MB_FRAME_SIZE);
> +
> + ret = xe_sysctrl_mailbox_process_frame(sc, out, frame_size, timeout_ms);
> + if (ret)
> + break;
> +
> + received += frame_size;
> + out += frame_size;
> + }
> +
> + *rdata_len = received;
> +
> + return ret;
> +}
> +
> +static int xe_sysctrl_mailbox_send_command(struct xe_sysctrl *sc,
> + const u8 *mbox_cmd, size_t cmd_size,
> + void *data_out, size_t data_out_len,
> + size_t *rdata_len, unsigned int timeout_ms)
> +{
> + const struct xe_sysctrl_mailbox_mkhi_msg_hdr *mkhi_hdr;
> + size_t received;
> + int ret;
> +
> + ret = xe_sysctrl_mailbox_send_frames(sc, mbox_cmd, cmd_size, timeout_ms);
> + if (ret)
> + return ret;
> +
> + if (!data_out || !rdata_len)
> + return 0;
> +
> + mkhi_hdr = (const struct xe_sysctrl_mailbox_mkhi_msg_hdr *)mbox_cmd;
> +
> + ret = xe_sysctrl_mailbox_receive_frames(sc, mkhi_hdr, data_out, data_out_len,
> + &received, timeout_ms);
> + if (ret)
> + return ret;
> +
> + *rdata_len = received;
> +
> + return 0;
> +}
> +
> +/**
> + * xe_sysctrl_send_command - Send command to System Controller via mailbox
> + * @handle: XE device handle
> + * @cmd_buffer: Pointer to xe_sysctrl_mailbox_command structure
> + * @rdata_len: Pointer to store actual response data size (can be NULL)
> + *
> + * Send a command to the System Controller using MKHI protocol. Handles
> + * command preparation, fragmentation, transmission, and response reception.
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +int xe_sysctrl_send_command(void *handle, void *cmd_buffer, size_t *rdata_len)
> +{
> + struct xe_device *xe = handle;
> + struct xe_sysctrl *sc = &xe->sc;
> + struct xe_sysctrl_mailbox_command *cmd = cmd_buffer;
> + u8 *mbox_cmd = NULL;
> + size_t cmd_size = 0;
> + u8 group_id, command_code;
> + int ret = 0;
> +
> + if (!xe) {
> + pr_err("sysctrl: Invalid device handle\n");
> + return -EINVAL;
> + }
> +
> + if (!cmd) {
> + xe_err(xe, "sysctrl: Invalid command buffer\n");
> + return -EINVAL;
> + }
> +
> + if (!xe->info.has_sysctrl)
> + return -ENODEV;
> +
> + group_id = XE_SYSCTRL_APP_HDR_GROUP_ID(&cmd->header);
> + command_code = XE_SYSCTRL_APP_HDR_COMMAND(&cmd->header);
> +
> + if (!cmd->data_in && cmd->data_in_len) {
> + xe_err(xe, "sysctrl: Invalid input parameters\n");
> + return -EINVAL;
> + }
> +
> + if (!cmd->data_out && cmd->data_out_len) {
> + xe_err(xe, "sysctrl: Invalid output parameters\n");
> + return -EINVAL;
> + }
> +
> + might_sleep();
> +
> + ret = xe_sysctrl_mailbox_prepare_command(sc, group_id, command_code,
> + cmd->data_in, cmd->data_in_len,
> + &mbox_cmd, &cmd_size);
> + if (ret) {
> + xe_err(xe, "sysctrl: Failed to prepare command: %d\n", ret);
> + return ret;
> + }
> +
> + guard(xe_pm_runtime)(xe);
> +
> + guard(mutex)(&sc->cmd_lock);
> +
> + ret = xe_sysctrl_mailbox_send_command(sc, mbox_cmd, cmd_size,
> + cmd->data_out, cmd->data_out_len, rdata_len,
> + SC_MB_DEFAULT_TIMEOUT_MS);
> + if (ret)
> + xe_err(xe, "sysctrl: Mailbox command failed: %d\n", ret);
> +
> + kfree(mbox_cmd);
> +
> + return ret;
> +}
> +
> +/**
> + * xe_sysctrl_mailbox_init - Initialize the System Controller mailbox state
> + * @sc: System controller structure
> + */
> +void xe_sysctrl_mailbox_init(struct xe_sysctrl *sc)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> + u32 ctrl_reg;
> +
> + ctrl_reg = xe_mmio_read32(mmio, SC_MB_CTRL);
> + sc->phase_bit = (ctrl_reg & MKHI_FRAME_PHASE) ? 1 : 0;
> +
> + xe_mmio_rmw32(mmio, SC_MB_CTRL, MKHI_FRAME_PHASE, 0);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.h b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.h
> new file mode 100644
> index 000000000000..3e472418ebd0
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.h
> @@ -0,0 +1,77 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#ifndef __XE_SYSCTRL_MAILBOX_H__
> +#define __XE_SYSCTRL_MAILBOX_H__
> +
> +#include <linux/bitfield.h>
> +#include <linux/types.h>
> +
> +struct xe_sysctrl;
> +
> +#define MKHI_HDR_GROUP_ID_MASK GENMASK(7, 0)
> +#define MKHI_HDR_COMMAND_MASK GENMASK(14, 8)
> +#define MKHI_HDR_IS_RESPONSE BIT(15)
> +#define MKHI_HDR_RESERVED_MASK GENMASK(23, 16)
> +#define MKHI_HDR_RESULT_MASK GENMASK(31, 24)
> +
> +struct xe_sysctrl_mailbox_mkhi_msg_hdr {
> + __le32 data;
> +} __packed;
> +
> +#define APP_HDR_GROUP_ID_MASK GENMASK(7, 0)
> +#define APP_HDR_COMMAND_MASK GENMASK(15, 8)
> +#define APP_HDR_VERSION_MASK GENMASK(23, 16)
> +#define APP_HDR_RESERVED_MASK GENMASK(31, 24)
> +
> +struct xe_sysctrl_mailbox_app_msg_hdr {
> + __le32 data;
> +} __packed;
> +
> +#define XE_SYSCTRL_APP_HDR_GROUP_ID(hdr) \
> + FIELD_GET(APP_HDR_GROUP_ID_MASK, le32_to_cpu((hdr)->data))
> +
> +#define XE_SYSCTRL_APP_HDR_COMMAND(hdr) \
> + FIELD_GET(APP_HDR_COMMAND_MASK, le32_to_cpu((hdr)->data))
> +
> +#define XE_SYSCTRL_APP_HDR_VERSION(hdr) \
> + FIELD_GET(APP_HDR_VERSION_MASK, le32_to_cpu((hdr)->data))
> +
> +#define XE_SYSCTRL_MKHI_HDR_GROUP_ID(hdr) \
> + FIELD_GET(MKHI_HDR_GROUP_ID_MASK, le32_to_cpu((hdr)->data))
> +
> +#define XE_SYSCTRL_MKHI_HDR_COMMAND(hdr) \
> + FIELD_GET(MKHI_HDR_COMMAND_MASK, le32_to_cpu((hdr)->data))
> +
> +#define XE_SYSCTRL_MKHI_HDR_IS_RESPONSE(hdr) \
> + FIELD_GET(MKHI_HDR_IS_RESPONSE, le32_to_cpu((hdr)->data))
> +
> +#define XE_SYSCTRL_MKHI_HDR_RESULT(hdr) \
> + FIELD_GET(MKHI_HDR_RESULT_MASK, le32_to_cpu((hdr)->data))
> +
> +/**
> + * struct xe_sysctrl_mailbox_command - System Controller mailbox command structure
> + */
> +struct xe_sysctrl_mailbox_command {
> + /** @header: Application message header containing command information */
> + struct xe_sysctrl_mailbox_app_msg_hdr header;
> +
> + /** @data_in: Pointer to input payload data (can be NULL if no input data) */
> + void *data_in;
> +
> + /** @data_in_len: Size of input payload in bytes (0 if no input data) */
> + size_t data_in_len;
> +
> + /** @data_out: Pointer to output buffer for response data (can be NULL if no response) */
> + void *data_out;
> +
> + /** @data_out_len: Size of output buffer in bytes (0 if no response expected) */
> + size_t data_out_len;
> +};
> +
> +void xe_sysctrl_mailbox_init(struct xe_sysctrl *sc);
> +int xe_sysctrl_send_command(void *handle, void *cmd_buffer, size_t *rdata_len);
> +
> +#endif /* __XE_SYSCTRL_MAILBOX_H__ */
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_types.h b/drivers/gpu/drm/xe/xe_sysctrl_types.h
> new file mode 100644
> index 000000000000..88a34967688b
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_types.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#ifndef _XE_SYSCTRL_TYPES_H_
> +#define _XE_SYSCTRL_TYPES_H_
> +
> +#include <linux/mutex.h>
> +#include <linux/types.h>
> +
> +/**
> + * struct xe_sysctrl - System Controller driver context
> + */
> +struct xe_sysctrl {
> + /** @cmd_lock: Mutex protecting mailbox command operations */
> + struct mutex cmd_lock;
> +
> + /** @phase_bit: MKHI message boundary phase toggle bit */
> + u32 phase_bit;
> +};
> +
> +#endif /* _XE_SYSCTRL_TYPES_H_ */
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/1] drm/xe/sysctrl: Add system controller component for Xe3p dGPU platforms
2026-01-02 16:54 ` [PATCH v2 1/1] " Anoop, Vijay
2026-01-02 23:15 ` Matthew Brost
@ 2026-01-05 6:29 ` Riana Tauro
2026-01-06 18:37 ` Umesh Nerlige Ramappa
2 siblings, 0 replies; 8+ messages in thread
From: Riana Tauro @ 2026-01-05 6:29 UTC (permalink / raw)
To: Anoop, Vijay, intel-xe
Cc: umesh.nerlige.ramappa, badal.nilawar, rodrigo.vivi,
aravind.iddamsetty, ravi.kishore.koppuravuri, anshuman.gupta,
matthew.d.roper, michael.j.ruhl, paul.e.luse, mohamed.mansoor.v,
kam.nasim
Hi Anoop
On 1/2/2026 10:24 PM, Anoop, Vijay wrote:
> From: Anoop Vijay <anoop.c.vijay@intel.com>
>
> Add a new system controller (sysctrl) component for Intel Xe3p dGPU
> platforms.
>
> This component provides the foundational infrastructure for communication
> with the System Controller firmware using MKHI protocol over a mailbox
> interface.
>
> Key features introduced:
> - Detection and initialization of System Controller interface on Xe3p
> dGPU platforms
> - Mailbox communication with System Controller firmware
> - Fragmented message transfer for large command payloads
>
> This implementation establishes the base for future System Controller
> feature enablement and firmware command handling.
Cover letter is not required for a single patch.
Also had version diff so that it's easier for the reviewers to know what
changed between versions since this is a big patch.
>
> Signed-off-by: Anoop Vijay <anoop.c.vijay@intel.com>
> ---
> drivers/gpu/drm/xe/Makefile | 2 +
> drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h | 44 +++
> drivers/gpu/drm/xe/xe_device.c | 5 +
> drivers/gpu/drm/xe/xe_device_types.h | 6 +
> drivers/gpu/drm/xe/xe_pci.c | 2 +
> drivers/gpu/drm/xe/xe_pci_types.h | 1 +
> drivers/gpu/drm/xe/xe_sysctrl.c | 62 ++++
> drivers/gpu/drm/xe/xe_sysctrl.h | 18 +
> drivers/gpu/drm/xe/xe_sysctrl_mailbox.c | 409 ++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_sysctrl_mailbox.h | 77 ++++
> drivers/gpu/drm/xe/xe_sysctrl_types.h | 23 ++
> 11 files changed, 649 insertions(+)
> create mode 100644 drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl.c
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl.h
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl_mailbox.h
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl_types.h
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 2b20c79d7ec9..947fbcac65d5 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -121,6 +121,8 @@ xe-y += xe_bb.o \
> xe_step.o \
> xe_survivability_mode.o \
> xe_sync.o \
> + xe_sysctrl.o \
> + xe_sysctrl_mailbox.o \
> xe_tile.o \
> xe_tile_sysfs.o \
> xe_tlb_inval.o \
> diff --git a/drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h b/drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h
> new file mode 100644
> index 000000000000..6627a9c32c4f
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#ifndef _XE_SYSCTRL_REGS_H_
> +#define _XE_SYSCTRL_REGS_H_
> +
> +#include "xe_regs.h"
> +
> +#define SYSCTRL_BASE_OFFSET 0xDB000
> +#define SYSCTRL_BASE (SOC_BASE + SYSCTRL_BASE_OFFSET)
> +#define SYSCTRL_MAILBOX_INDEX 0x03
> +#define SC_BAR_LENGTH 0x1000
Maintain consistency. Either SYSCTRL or SC
> +
> +#define SC_MB_CTRL XE_REG(SYSCTRL_BASE + 0x10)
> +#define SC_MB_CTRL_RUN_BUSY REG_BIT(31)
> +#define SC_MB_CTRL_IRQ REG_BIT(30)
> +#define SC_MB_CTRL_RUN_BUSY_OUT REG_BIT(29)
> +#define SC_MB_CTRL_PARAM3_MASK REG_GENMASK(28, 24)
> +#define SC_MB_CTRL_PARAM2_MASK REG_GENMASK(23, 16)
> +#define SC_MB_CTRL_PARAM1_MASK REG_GENMASK(15, 8)
> +#define SC_MB_CTRL_COMMAND_MASK REG_GENMASK(7, 0)
> +
> +#define SC_MB_DATA0 XE_REG(SYSCTRL_BASE + 0x14)
> +#define SC_MB_DATA1 XE_REG(SYSCTRL_BASE + 0x18)
> +#define SC_MB_DATA2 XE_REG(SYSCTRL_BASE + 0x1C)
> +#define SC_MB_DATA3 XE_REG(SYSCTRL_BASE + 0x20)
> +
> +#define MKHI_FRAME_PHASE REG_BIT(24)
> +#define MKHI_FRAME_CURRENT_MASK REG_GENMASK(21, 16)
> +#define MKHI_FRAME_TOTAL_MASK REG_GENMASK(13, 8)
> +#define MKHI_FRAME_COMMAND_MASK REG_GENMASK(7, 0)
> +
> +#define SC_MB_FRAME_SIZE 16
> +#define SC_MB_MAX_FRAMES 64
> +#define SC_MB_MAX_MESSAGE_SIZE (SC_MB_FRAME_SIZE * SC_MB_MAX_FRAMES)
> +#define SC_MKHI_COMMAND 5
> +
> +#define SC_MB_DEFAULT_TIMEOUT_MS 500
> +#define SC_MB_RETRY_TIMEOUT_MS 20
> +#define SC_MB_POLL_INTERVAL_US 100
> +
> +#endif /* _XE_SYSCTRL_REGS_H_ */
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index e101d290b2a6..805d48dd954d 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -66,6 +66,7 @@
> #include "xe_survivability_mode.h"
> #include "xe_sriov.h"
> #include "xe_svm.h"
> +#include "xe_sysctrl.h"
> #include "xe_tile.h"
> #include "xe_ttm_stolen_mgr.h"
> #include "xe_ttm_sys_mgr.h"
> @@ -1032,6 +1033,10 @@ int xe_device_probe(struct xe_device *xe)
> if (err)
> goto err_unregister_display;
>
> + err = xe_sysctrl_init(xe);
> + if (err)
> + goto err_unregister_display;
> +
> err = xe_device_sysfs_init(xe);
> if (err)
> goto err_unregister_display;
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index a85be9ba175e..6295b2c35d4a 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -29,6 +29,7 @@
> #include "xe_sriov_vf_ccs_types.h"
> #include "xe_step_types.h"
> #include "xe_survivability_mode_types.h"
> +#include "xe_sysctrl_types.h"
> #include "xe_tile_sriov_vf_types.h"
> #include "xe_validation.h"
>
> @@ -340,6 +341,8 @@ struct xe_device {
> u8 has_soc_remapper_telem:1;
> /** @info.has_sriov: Supports SR-IOV */
> u8 has_sriov:1;
> + /** @info.has_sysctrl: Supports System Controller */
> + u8 has_sysctrl:1;
> /** @info.has_usm: Device has unified shared memory support */
> u8 has_usm:1;
> /** @info.has_64bit_timestamp: Device supports 64-bit timestamps */
> @@ -606,6 +609,9 @@ struct xe_device {
> /** @heci_gsc: graphics security controller */
> struct xe_heci_gsc heci_gsc;
>
> + /** @sc: System Controller */
> + struct xe_sysctrl sc;
> +
> /** @nvm: discrete graphics non-volatile memory */
> struct intel_dg_nvm_dev *nvm;
>
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 91e0553a8163..b6dc3030b673 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -426,6 +426,7 @@ static const struct xe_device_desc cri_desc = {
> .has_soc_remapper_sysctrl = true,
> .has_soc_remapper_telem = true,
> .has_sriov = true,
> + .has_sysctrl = true,
> .max_gt_per_tile = 2,
> .require_force_probe = true,
> .va_bits = 57,
> @@ -701,6 +702,7 @@ static int xe_info_init_early(struct xe_device *xe,
> xe->info.has_soc_remapper_telem = desc->has_soc_remapper_telem;
> xe->info.has_sriov = xe_configfs_primary_gt_allowed(to_pci_dev(xe->drm.dev)) &&
> desc->has_sriov;
> + xe->info.has_sysctrl = desc->has_sysctrl;
> xe->info.has_mem_copy_instr = desc->has_mem_copy_instr;
> xe->info.skip_guc_pc = desc->skip_guc_pc;
> xe->info.skip_mtcfg = desc->skip_mtcfg;
> diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
> index 5f20f56571d1..53e44a32883d 100644
> --- a/drivers/gpu/drm/xe/xe_pci_types.h
> +++ b/drivers/gpu/drm/xe/xe_pci_types.h
> @@ -56,6 +56,7 @@ struct xe_device_desc {
> u8 has_soc_remapper_sysctrl:1;
> u8 has_soc_remapper_telem:1;
> u8 has_sriov:1;
> + u8 has_sysctrl:1;
> u8 needs_scratch:1;
> u8 skip_guc_pc:1;
> u8 skip_mtcfg:1;
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl.c b/drivers/gpu/drm/xe/xe_sysctrl.c
> new file mode 100644
> index 000000000000..e0e7b0ecf2bf
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_sysctrl.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#include <drm/drm_managed.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +
> +#include "regs/xe_sysctrl_regs.h"
> +#include "xe_device.h"
> +#include "xe_printk.h"
> +#include "xe_soc_remapper.h"
> +#include "xe_sysctrl.h"
> +#include "xe_sysctrl_mailbox.h"
> +#include "xe_sysctrl_types.h"
A short documentation of System controller would be helpful here.
> +
> +static void xe_sysctrl_fini(void *arg)
> +{
> + struct xe_sysctrl *sc = arg;
> + struct xe_device *xe = sc_to_xe(sc);
> +
> + if (!xe->soc_remapper.set_sysctrl_region)
> + return;
> +> + xe->soc_remapper.set_sysctrl_region(xe, 0);
> +}
> +
> +/**
> + * xe_sysctrl_init - Initialize SC subsystem
> + * @xe: xe device instance
> + *
> + * Entry point for SC initialization, called from xe_device_probe().
> + * This function checks platform support and initializes the system controller.
> + *
> + * Return: 0 on success, error code on failure
> + */
> +int xe_sysctrl_init(struct xe_device *xe)
> +{
> + struct xe_sysctrl *sc = &xe->sc;
> + int ret;
> +
> + if (!xe->info.has_sysctrl)
> + return 0;
> +
> + ret = devm_add_action_or_reset(xe->drm.dev, xe_sysctrl_fini, sc);
Add this after set_sysctrl region.
> + if (ret)
> + return ret;
> +
> + if (!xe->soc_remapper.set_sysctrl_region)
> + return -ENODEV;
> +
> + xe->soc_remapper.set_sysctrl_region(xe, SYSCTRL_MAILBOX_INDEX
> +
> + ret = drmm_mutex_init(&xe->drm, &sc->cmd_lock);
> + if (ret)
> + return ret;
> +
> + xe_sysctrl_mailbox_init(sc);
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl.h b/drivers/gpu/drm/xe/xe_sysctrl.h
> new file mode 100644
> index 000000000000..fe90d6577d54
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_sysctrl.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#ifndef _XE_SYSCTRL_H_
> +#define _XE_SYSCTRL_H_
> +
> +struct xe_device;
> +
> +static inline struct xe_device *sc_to_xe(struct xe_sysctrl *sc)
> +{
> + return container_of(sc, struct xe_device, sc);
> +}
> +
> +int xe_sysctrl_init(struct xe_device *xe);
> +
> +#endif /* _XE_SYSCTRL_H_ */
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> new file mode 100644
> index 000000000000..940ea535da2e
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> @@ -0,0 +1,409 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/errno.h>
> +#include <linux/minmax.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +
> +#include "regs/xe_sysctrl_regs.h"
> +#include "xe_device.h"
> +#include "xe_mmio.h"
> +#include "xe_pm.h"
> +#include "xe_printk.h"
> +#include "xe_sysctrl.h"
> +#include "xe_sysctrl_mailbox.h"
> +#include "xe_sysctrl_types.h"
> +
> +static bool xe_sysctrl_mailbox_wait_bit_clear(struct xe_sysctrl *sc, u32 bit_mask,
> + unsigned int timeout_ms)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> + int ret;
> +
> + ret = xe_mmio_wait32_not(mmio, SC_MB_CTRL, bit_mask, bit_mask,
> + timeout_ms * 1000, NULL, false);
> +
> + return ret == 0;
> +}
> +
> +static bool xe_sysctrl_mailbox_wait_bit_set(struct xe_sysctrl *sc, u32 bit_mask,
> + unsigned int timeout_ms)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> + int ret;
> +
> + ret = xe_mmio_wait32(mmio, SC_MB_CTRL, bit_mask, bit_mask,
> + timeout_ms * 1000, NULL, false);
> +
> + return ret == 0;
> +}
> +
> +static int xe_sysctrl_mailbox_write_frame(struct xe_sysctrl *sc, const void *frame,
> + size_t len)
> +{
> + static const struct xe_reg regs[] = {
> + SC_MB_DATA0, SC_MB_DATA1, SC_MB_DATA2, SC_MB_DATA3
> + };
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> + u32 val[SC_MB_FRAME_SIZE / sizeof(u32)] = {0};
> + u32 dw = DIV_ROUND_UP(len, sizeof(u32));
> + u32 i;
> +
> + memcpy(val, frame, len);
> +
> + for (i = 0; i < dw; i++)
> + xe_mmio_write32(mmio, regs[i], val[i]);
> +
> + return 0;
> +}
> +
> +static int xe_sysctrl_mailbox_read_frame(struct xe_sysctrl *sc, void *frame,
> + size_t len)
> +{
> + static const struct xe_reg regs[] = {
> + SC_MB_DATA0, SC_MB_DATA1, SC_MB_DATA2, SC_MB_DATA3
> + };
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> + u32 val[SC_MB_FRAME_SIZE / sizeof(u32)] = {0};
> + u32 dw = DIV_ROUND_UP(len, sizeof(u32));
> + u32 i;
> +
> + for (i = 0; i < dw; i++)
> + val[i] = xe_mmio_read32(mmio, regs[i]);
> +
> + memcpy(frame, val, len);
> +
> + return 0;
> +}
> +
> +static void xe_sysctrl_mailbox_clear_response(struct xe_sysctrl *sc)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> +
> + xe_mmio_rmw32(mmio, SC_MB_CTRL, SC_MB_CTRL_RUN_BUSY_OUT, 0);
> +}
> +
> +static int xe_sysctrl_mailbox_prepare_command(struct xe_sysctrl *sc,
> + u8 group_id, u8 command,
> + const void *data_in, size_t data_in_len,
> + u8 **mbox_cmd, size_t *cmd_size)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_sysctrl_mailbox_mkhi_msg_hdr *mkhi_hdr;
> + size_t size;
> + u8 *buffer;
> +
> + size = sizeof(*mkhi_hdr) + data_in_len;
> + if (size > SC_MB_MAX_MESSAGE_SIZE) {
> + xe_err(xe, "sysctrl: Message too large: %zu bytes\n", size);
> + return -EINVAL;
> + }
> +
> + buffer = kmalloc(size, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
> +
> + mkhi_hdr = (struct xe_sysctrl_mailbox_mkhi_msg_hdr *)buffer;
> + mkhi_hdr->data = cpu_to_le32(FIELD_PREP(MKHI_HDR_GROUP_ID_MASK, group_id) |
> + FIELD_PREP(MKHI_HDR_COMMAND_MASK, command & 0x7F) |
> + FIELD_PREP(MKHI_HDR_IS_RESPONSE, 0) |
> + FIELD_PREP(MKHI_HDR_RESERVED_MASK, 0) |
> + FIELD_PREP(MKHI_HDR_RESULT_MASK, 0));
> +
> + if (data_in && data_in_len)
> + memcpy(buffer + sizeof(*mkhi_hdr), data_in, data_in_len);
> +
> + *mbox_cmd = buffer;
> + *cmd_size = size;
> +
> + return 0;
> +}
> +
> +static int xe_sysctrl_mailbox_send_frames(struct xe_sysctrl *sc, const u8 *mbox_cmd,
> + size_t cmd_size, unsigned int timeout_ms)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> + u32 ctrl_reg, total_frames, frame;
> + size_t bytes_sent, frame_size;
> +
> + total_frames = DIV_ROUND_UP(cmd_size, SC_MB_FRAME_SIZE);
> +
> + if (!xe_sysctrl_mailbox_wait_bit_clear(sc, SC_MB_CTRL_RUN_BUSY, timeout_ms)) {
> + xe_err(xe, "sysctrl: Mailbox busy\n");
> + return -EBUSY;
> + }
> +
> + sc->phase_bit ^= 1;
> + bytes_sent = 0;
> +
> + for (frame = 0; frame < total_frames; frame++) {
> + frame_size = min(cmd_size - bytes_sent, (size_t)SC_MB_FRAME_SIZE);
> +
> + if (xe_sysctrl_mailbox_write_frame(sc, mbox_cmd + bytes_sent, frame_size)) {
> + xe_err(xe, "sysctrl: Failed to write frame %u\n", frame);
> + sc->phase_bit ^= 1;
> + return -EIO;
> + }
> +
> + ctrl_reg = SC_MB_CTRL_RUN_BUSY |
> + FIELD_PREP(MKHI_FRAME_CURRENT_MASK, frame) |
> + FIELD_PREP(MKHI_FRAME_TOTAL_MASK, total_frames - 1) |
> + FIELD_PREP(MKHI_FRAME_COMMAND_MASK, SC_MKHI_COMMAND) |
> + (sc->phase_bit ? MKHI_FRAME_PHASE : 0);
> +
> + xe_mmio_write32(mmio, SC_MB_CTRL, ctrl_reg);
> +
> + if (!xe_sysctrl_mailbox_wait_bit_clear(sc, SC_MB_CTRL_RUN_BUSY, timeout_ms)) {
> + xe_err(xe, "sysctrl: Frame %u acknowledgment timeout\n", frame);
> + return -ETIMEDOUT;
> + }
> +
> + bytes_sent += frame_size;
> + }
> +
> + return 0;
> +}
> +
> +static int xe_sysctrl_mailbox_process_first_frame(struct xe_sysctrl *sc,
> + const struct xe_sysctrl_mailbox_mkhi_msg_hdr *req,
> + void *out,
> + size_t frame_size,
> + size_t *payload_bytes)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + u32 frame_data[4];
> + struct xe_sysctrl_mailbox_mkhi_msg_hdr *resp_hdr;
> + size_t hdr_size = sizeof(*resp_hdr);
> + size_t payload_size;
> + int ret;
> +
> + ret = xe_sysctrl_mailbox_read_frame(sc, frame_data, frame_size);
> + if (ret)
> + return ret;
> +
> + resp_hdr = (struct xe_sysctrl_mailbox_mkhi_msg_hdr *)frame_data;
> +
> + if (!XE_SYSCTRL_MKHI_HDR_IS_RESPONSE(resp_hdr) ||
> + XE_SYSCTRL_MKHI_HDR_GROUP_ID(resp_hdr) != XE_SYSCTRL_MKHI_HDR_GROUP_ID(req) ||
> + XE_SYSCTRL_MKHI_HDR_COMMAND(resp_hdr) != XE_SYSCTRL_MKHI_HDR_COMMAND(req)) {
> + xe_err(xe, "SC: Response header mismatch\n");
> + return -EPROTO;
> + }
> +
> + if (XE_SYSCTRL_MKHI_HDR_RESULT(resp_hdr) != 0) {
> + xe_err(xe, "SC: Firmware error: 0x%02lx\n",
> + XE_SYSCTRL_MKHI_HDR_RESULT(resp_hdr));
> + return -EIO;
> + }
> +
> + payload_size = frame_size - hdr_size;
> + if (payload_size > 0)
> + memcpy(out, (u8 *)frame_data + hdr_size, payload_size);
> +
> + *payload_bytes = payload_size;
> +
> + xe_sysctrl_mailbox_clear_response(sc);
> +
> + return 0;
> +}
> +
> +static int xe_sysctrl_mailbox_process_frame(struct xe_sysctrl *sc,
> + void *out, size_t frame_size,
> + unsigned int timeout_ms)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + int ret;
> +
> + if (!xe_sysctrl_mailbox_wait_bit_set(sc, SC_MB_CTRL_RUN_BUSY_OUT, timeout_ms)) {
> + xe_err(xe, "sysctrl: Response frame timeout\n");
> + return -ETIMEDOUT;
> + }
> +
> + ret = xe_sysctrl_mailbox_read_frame(sc, out, frame_size);
> + if (ret)
> + return ret;
> +
> + xe_sysctrl_mailbox_clear_response(sc);
> +
> + return 0;
> +}
> +
> +static int xe_sysctrl_mailbox_receive_frames(struct xe_sysctrl *sc,
> + const struct xe_sysctrl_mailbox_mkhi_msg_hdr *req,
> + void *data_out, size_t data_out_len,
> + size_t *rdata_len, unsigned int timeout_ms)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> + struct xe_sysctrl_mailbox_mkhi_msg_hdr *mkhi_hdr;
> + u32 ctrl_reg, total_frames, frame;
> + size_t hdr_size = sizeof(*mkhi_hdr);
> + u8 *out = data_out;
> + size_t received = 0;
> + size_t frame_size;
> + int ret = 0;
> +
> + if (!xe_sysctrl_mailbox_wait_bit_set(sc, SC_MB_CTRL_RUN_BUSY_OUT, timeout_ms)) {
> + xe_err(xe, "sysctrl: Response frame 0 timeout\n");
> + return -ETIMEDOUT;
> + }
> +
> + ctrl_reg = xe_mmio_read32(mmio, SC_MB_CTRL);
> + total_frames = FIELD_GET(MKHI_FRAME_TOTAL_MASK, ctrl_reg) + 1;
> +
> + if (total_frames == 1)
> + frame_size = min(hdr_size + data_out_len, (size_t)SC_MB_FRAME_SIZE);
> + else
> + frame_size = SC_MB_FRAME_SIZE;
> +
> + ret = xe_sysctrl_mailbox_process_first_frame(sc, req, out, frame_size, &received);
> + if (ret)
> + return ret;
> +
> + out += received;
> +
> + for (frame = 1; frame < total_frames; frame++) {
> + size_t remaining = data_out_len - received;
> +
> + frame_size = min_t(size_t, remaining, SC_MB_FRAME_SIZE);
> +
> + ret = xe_sysctrl_mailbox_process_frame(sc, out, frame_size, timeout_ms);
> + if (ret)
> + break;
> +
> + received += frame_size;
> + out += frame_size;
> + }
> +
> + *rdata_len = received;
> +
> + return ret;
> +}
> +
> +static int xe_sysctrl_mailbox_send_command(struct xe_sysctrl *sc,
> + const u8 *mbox_cmd, size_t cmd_size,
> + void *data_out, size_t data_out_len,
> + size_t *rdata_len, unsigned int timeout_ms)
> +{
> + const struct xe_sysctrl_mailbox_mkhi_msg_hdr *mkhi_hdr;
> + size_t received;
> + int ret;
> +
> + ret = xe_sysctrl_mailbox_send_frames(sc, mbox_cmd, cmd_size, timeout_ms);
> + if (ret)
> + return ret;
> +
> + if (!data_out || !rdata_len)
> + return 0;
> +
> + mkhi_hdr = (const struct xe_sysctrl_mailbox_mkhi_msg_hdr *)mbox_cmd;
> +
> + ret = xe_sysctrl_mailbox_receive_frames(sc, mkhi_hdr, data_out, data_out_len,
> + &received, timeout_ms);
> + if (ret)
> + return ret;
> +
> + *rdata_len = received;
> +
> + return 0;
> +}
> +
> +/**
> + * xe_sysctrl_send_command - Send command to System Controller via mailbox
> + * @handle: XE device handle
> + * @cmd_buffer: Pointer to xe_sysctrl_mailbox_command structure
> + * @rdata_len: Pointer to store actual response data size (can be NULL)
> + *
> + * Send a command to the System Controller using MKHI protocol. Handles
> + * command preparation, fragmentation, transmission, and response reception.
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +int xe_sysctrl_send_command(void *handle, void *cmd_buffer, size_t *rdata_len)
> +{
> + struct xe_device *xe = handle;
> + struct xe_sysctrl *sc = &xe->sc;
> + struct xe_sysctrl_mailbox_command *cmd = cmd_buffer;
> + u8 *mbox_cmd = NULL;
> + size_t cmd_size = 0;
> + u8 group_id, command_code;
> + int ret = 0;
> +
> + if (!xe) {
> + pr_err("sysctrl: Invalid device handle\n");
> + return -EINVAL;
> + }
> +
> + if (!cmd) {
> + xe_err(xe, "sysctrl: Invalid command buffer\n");
> + return -EINVAL;
> + }
> +
> + if (!xe->info.has_sysctrl)
> + return -ENODEV;
> +
> + group_id = XE_SYSCTRL_APP_HDR_GROUP_ID(&cmd->header);
> + command_code = XE_SYSCTRL_APP_HDR_COMMAND(&cmd->header);
> +
> + if (!cmd->data_in && cmd->data_in_len) {
> + xe_err(xe, "sysctrl: Invalid input parameters\n");
> + return -EINVAL;
> + }
> +
> + if (!cmd->data_out && cmd->data_out_len) {
> + xe_err(xe, "sysctrl: Invalid output parameters\n");
> + return -EINVAL;
> + }
> +
> + might_sleep();
> +
> + ret = xe_sysctrl_mailbox_prepare_command(sc, group_id, command_code,
> + cmd->data_in, cmd->data_in_len,
> + &mbox_cmd, &cmd_size);
> + if (ret) {
> + xe_err(xe, "sysctrl: Failed to prepare command: %d\n", ret);
> + return ret;
> + }
> +
> + guard(xe_pm_runtime)(xe);
> +
> + guard(mutex)(&sc->cmd_lock);
> +
> + ret = xe_sysctrl_mailbox_send_command(sc, mbox_cmd, cmd_size,
> + cmd->data_out, cmd->data_out_len, rdata_len,
> + SC_MB_DEFAULT_TIMEOUT_MS);
> + if (ret)
> + xe_err(xe, "sysctrl: Mailbox command failed: %d\n", ret);
> +
> + kfree(mbox_cmd);
> +
> + return ret;
> +}
> +
> +/**
> + * xe_sysctrl_mailbox_init - Initialize the System Controller mailbox state
> + * @sc: System controller structure
> + */
Missing description https://docs.kernel.org/doc-guide/kernel-doc.html
> +void xe_sysctrl_mailbox_init(struct xe_sysctrl *sc)
> +{
> + struct xe_device *xe = sc_to_xe(sc);
> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> + u32 ctrl_reg;
> +
> + ctrl_reg = xe_mmio_read32(mmio, SC_MB_CTRL);
> + sc->phase_bit = (ctrl_reg & MKHI_FRAME_PHASE) ? 1 : 0;
> +
> + xe_mmio_rmw32(mmio, SC_MB_CTRL, MKHI_FRAME_PHASE, 0);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.h b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.h
> new file mode 100644
> index 000000000000..3e472418ebd0
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.h
> @@ -0,0 +1,77 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#ifndef __XE_SYSCTRL_MAILBOX_H__
> +#define __XE_SYSCTRL_MAILBOX_H__
> +
> +#include <linux/bitfield.h>
> +#include <linux/types.h>
> +
> +struct xe_sysctrl;
> +
> +#define MKHI_HDR_GROUP_ID_MASK GENMASK(7, 0)
> +#define MKHI_HDR_COMMAND_MASK GENMASK(14, 8)
> +#define MKHI_HDR_IS_RESPONSE BIT(15)
> +#define MKHI_HDR_RESERVED_MASK GENMASK(23, 16)
> +#define MKHI_HDR_RESULT_MASK GENMASK(31, 24)
> +
> +struct xe_sysctrl_mailbox_mkhi_msg_hdr {
> + __le32 data;
> +} __packed;
> +
> +#define APP_HDR_GROUP_ID_MASK GENMASK(7, 0)
> +#define APP_HDR_COMMAND_MASK GENMASK(15, 8)
> +#define APP_HDR_VERSION_MASK GENMASK(23, 16)
> +#define APP_HDR_RESERVED_MASK GENMASK(31, 24)
> +
> +struct xe_sysctrl_mailbox_app_msg_hdr {
> + __le32 data;
> +} __packed;
> +
> +#define XE_SYSCTRL_APP_HDR_GROUP_ID(hdr) \
> + FIELD_GET(APP_HDR_GROUP_ID_MASK, le32_to_cpu((hdr)->data))
> +
> +#define XE_SYSCTRL_APP_HDR_COMMAND(hdr) \
> + FIELD_GET(APP_HDR_COMMAND_MASK, le32_to_cpu((hdr)->data))
> +
> +#define XE_SYSCTRL_APP_HDR_VERSION(hdr) \
> + FIELD_GET(APP_HDR_VERSION_MASK, le32_to_cpu((hdr)->data))
> +
> +#define XE_SYSCTRL_MKHI_HDR_GROUP_ID(hdr) \
> + FIELD_GET(MKHI_HDR_GROUP_ID_MASK, le32_to_cpu((hdr)->data))
> +
> +#define XE_SYSCTRL_MKHI_HDR_COMMAND(hdr) \
> + FIELD_GET(MKHI_HDR_COMMAND_MASK, le32_to_cpu((hdr)->data))
> +
> +#define XE_SYSCTRL_MKHI_HDR_IS_RESPONSE(hdr) \
> + FIELD_GET(MKHI_HDR_IS_RESPONSE, le32_to_cpu((hdr)->data))
> +
> +#define XE_SYSCTRL_MKHI_HDR_RESULT(hdr) \
> + FIELD_GET(MKHI_HDR_RESULT_MASK, le32_to_cpu((hdr)->data))
> +
> +/**
> + * struct xe_sysctrl_mailbox_command - System Controller mailbox command structure
> + */
> +struct xe_sysctrl_mailbox_command {
> + /** @header: Application message header containing command information */
> + struct xe_sysctrl_mailbox_app_msg_hdr header;
> +
> + /** @data_in: Pointer to input payload data (can be NULL if no input data) */
> + void *data_in;
> +
> + /** @data_in_len: Size of input payload in bytes (0 if no input data) */
> + size_t data_in_len;
> +
> + /** @data_out: Pointer to output buffer for response data (can be NULL if no response) */
> + void *data_out;
> +
> + /** @data_out_len: Size of output buffer in bytes (0 if no response expected) */
> + size_t data_out_len;
> +};
structures are defined in _types.h
Thanks
Riana
> +
> +void xe_sysctrl_mailbox_init(struct xe_sysctrl *sc);
> +int xe_sysctrl_send_command(void *handle, void *cmd_buffer, size_t *rdata_len);
> +
> +#endif /* __XE_SYSCTRL_MAILBOX_H__ */
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_types.h b/drivers/gpu/drm/xe/xe_sysctrl_types.h
> new file mode 100644
> index 000000000000..88a34967688b
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_types.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#ifndef _XE_SYSCTRL_TYPES_H_
> +#define _XE_SYSCTRL_TYPES_H_
> +
> +#include <linux/mutex.h>
> +#include <linux/types.h>
> +
> +/**
> + * struct xe_sysctrl - System Controller driver context
> + */
> +struct xe_sysctrl {
> + /** @cmd_lock: Mutex protecting mailbox command operations */
> + struct mutex cmd_lock;
> +
> + /** @phase_bit: MKHI message boundary phase toggle bit */
> + u32 phase_bit;
> +};
> +
> +#endif /* _XE_SYSCTRL_TYPES_H_ */
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/1] drm/xe/sysctrl: Add system controller component for Xe3p dGPU platforms
2026-01-02 16:54 ` [PATCH v2 1/1] " Anoop, Vijay
2026-01-02 23:15 ` Matthew Brost
2026-01-05 6:29 ` Riana Tauro
@ 2026-01-06 18:37 ` Umesh Nerlige Ramappa
2026-01-07 1:24 ` Matthew Brost
2 siblings, 1 reply; 8+ messages in thread
From: Umesh Nerlige Ramappa @ 2026-01-06 18:37 UTC (permalink / raw)
To: Anoop, Vijay
Cc: intel-xe, badal.nilawar, rodrigo.vivi, aravind.iddamsetty,
riana.tauro, ravi.kishore.koppuravuri, anshuman.gupta,
matthew.d.roper, michael.j.ruhl, paul.e.luse, mohamed.mansoor.v,
kam.nasim
Hi Anoop,
Thanks for incorporating most of the comments here. I have a few more in
this revision.
On Fri, Jan 02, 2026 at 08:54:50AM -0800, Anoop, Vijay wrote:
>From: Anoop Vijay <anoop.c.vijay@intel.com>
>
>Add a new system controller (sysctrl) component for Intel Xe3p dGPU
>platforms.
>
>This component provides the foundational infrastructure for communication
>with the System Controller firmware using MKHI protocol over a mailbox
>interface.
>
>Key features introduced:
> - Detection and initialization of System Controller interface on Xe3p
> dGPU platforms
> - Mailbox communication with System Controller firmware
> - Fragmented message transfer for large command payloads
Let's break down the patches as Matt suggested.
>
>This implementation establishes the base for future System Controller
>feature enablement and firmware command handling.
>
>Signed-off-by: Anoop Vijay <anoop.c.vijay@intel.com>
>---
> drivers/gpu/drm/xe/Makefile | 2 +
> drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h | 44 +++
> drivers/gpu/drm/xe/xe_device.c | 5 +
> drivers/gpu/drm/xe/xe_device_types.h | 6 +
> drivers/gpu/drm/xe/xe_pci.c | 2 +
> drivers/gpu/drm/xe/xe_pci_types.h | 1 +
> drivers/gpu/drm/xe/xe_sysctrl.c | 62 ++++
> drivers/gpu/drm/xe/xe_sysctrl.h | 18 +
> drivers/gpu/drm/xe/xe_sysctrl_mailbox.c | 409 ++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_sysctrl_mailbox.h | 77 ++++
> drivers/gpu/drm/xe/xe_sysctrl_types.h | 23 ++
> 11 files changed, 649 insertions(+)
> create mode 100644 drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl.c
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl.h
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl_mailbox.h
> create mode 100644 drivers/gpu/drm/xe/xe_sysctrl_types.h
>
>diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>index 2b20c79d7ec9..947fbcac65d5 100644
>--- a/drivers/gpu/drm/xe/Makefile
>+++ b/drivers/gpu/drm/xe/Makefile
>@@ -121,6 +121,8 @@ xe-y += xe_bb.o \
> xe_step.o \
> xe_survivability_mode.o \
> xe_sync.o \
>+ xe_sysctrl.o \
>+ xe_sysctrl_mailbox.o \
> xe_tile.o \
> xe_tile_sysfs.o \
> xe_tlb_inval.o \
>diff --git a/drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h b/drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h
>new file mode 100644
>index 000000000000..6627a9c32c4f
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h
>@@ -0,0 +1,44 @@
>+/* SPDX-License-Identifier: MIT */
>+/*
>+ * Copyright © 2026 Intel Corporation
>+ */
>+
>+#ifndef _XE_SYSCTRL_REGS_H_
>+#define _XE_SYSCTRL_REGS_H_
>+
>+#include "xe_regs.h"
>+
>+#define SYSCTRL_BASE_OFFSET 0xDB000
>+#define SYSCTRL_BASE (SOC_BASE + SYSCTRL_BASE_OFFSET)
>+#define SYSCTRL_MAILBOX_INDEX 0x03
>+#define SC_BAR_LENGTH 0x1000
>+
>+#define SC_MB_CTRL XE_REG(SYSCTRL_BASE + 0x10)
>+#define SC_MB_CTRL_RUN_BUSY REG_BIT(31)
>+#define SC_MB_CTRL_IRQ REG_BIT(30)
>+#define SC_MB_CTRL_RUN_BUSY_OUT REG_BIT(29)
>+#define SC_MB_CTRL_PARAM3_MASK REG_GENMASK(28, 24)
>+#define SC_MB_CTRL_PARAM2_MASK REG_GENMASK(23, 16)
>+#define SC_MB_CTRL_PARAM1_MASK REG_GENMASK(15, 8)
>+#define SC_MB_CTRL_COMMAND_MASK REG_GENMASK(7, 0)
>+
>+#define SC_MB_DATA0 XE_REG(SYSCTRL_BASE + 0x14)
>+#define SC_MB_DATA1 XE_REG(SYSCTRL_BASE + 0x18)
>+#define SC_MB_DATA2 XE_REG(SYSCTRL_BASE + 0x1C)
>+#define SC_MB_DATA3 XE_REG(SYSCTRL_BASE + 0x20)
>+
>+#define MKHI_FRAME_PHASE REG_BIT(24)
>+#define MKHI_FRAME_CURRENT_MASK REG_GENMASK(21, 16)
>+#define MKHI_FRAME_TOTAL_MASK REG_GENMASK(13, 8)
>+#define MKHI_FRAME_COMMAND_MASK REG_GENMASK(7, 0)
>+
>+#define SC_MB_FRAME_SIZE 16
>+#define SC_MB_MAX_FRAMES 64
>+#define SC_MB_MAX_MESSAGE_SIZE (SC_MB_FRAME_SIZE * SC_MB_MAX_FRAMES)
>+#define SC_MKHI_COMMAND 5
>+
>+#define SC_MB_DEFAULT_TIMEOUT_MS 500
>+#define SC_MB_RETRY_TIMEOUT_MS 20
>+#define SC_MB_POLL_INTERVAL_US 100
1)
I missed this the last time. Can we change SC to SYSCTRL here for
everything? I see that for the mailbox, you are using SC_MB, but at the end
of the day SC still means SYSCTRL and consistently using the same name helps
readability.
2)
The MKHI_FRAME definitions can move to the xe_sysctrl_mailbox.c since that's
the only code accessing it. See more comments below.
>+
>+#endif /* _XE_SYSCTRL_REGS_H_ */
>diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>index e101d290b2a6..805d48dd954d 100644
>--- a/drivers/gpu/drm/xe/xe_device.c
>+++ b/drivers/gpu/drm/xe/xe_device.c
>@@ -66,6 +66,7 @@
> #include "xe_survivability_mode.h"
> #include "xe_sriov.h"
> #include "xe_svm.h"
>+#include "xe_sysctrl.h"
> #include "xe_tile.h"
> #include "xe_ttm_stolen_mgr.h"
> #include "xe_ttm_sys_mgr.h"
>@@ -1032,6 +1033,10 @@ int xe_device_probe(struct xe_device *xe)
> if (err)
> goto err_unregister_display;
>
>+ err = xe_sysctrl_init(xe);
>+ if (err)
>+ goto err_unregister_display;
>+
> err = xe_device_sysfs_init(xe);
> if (err)
> goto err_unregister_display;
>diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>index a85be9ba175e..6295b2c35d4a 100644
>--- a/drivers/gpu/drm/xe/xe_device_types.h
>+++ b/drivers/gpu/drm/xe/xe_device_types.h
>@@ -29,6 +29,7 @@
> #include "xe_sriov_vf_ccs_types.h"
> #include "xe_step_types.h"
> #include "xe_survivability_mode_types.h"
>+#include "xe_sysctrl_types.h"
> #include "xe_tile_sriov_vf_types.h"
> #include "xe_validation.h"
>
>@@ -340,6 +341,8 @@ struct xe_device {
> u8 has_soc_remapper_telem:1;
> /** @info.has_sriov: Supports SR-IOV */
> u8 has_sriov:1;
>+ /** @info.has_sysctrl: Supports System Controller */
>+ u8 has_sysctrl:1;
> /** @info.has_usm: Device has unified shared memory support */
> u8 has_usm:1;
> /** @info.has_64bit_timestamp: Device supports 64-bit timestamps */
>@@ -606,6 +609,9 @@ struct xe_device {
> /** @heci_gsc: graphics security controller */
> struct xe_heci_gsc heci_gsc;
>
>+ /** @sc: System Controller */
>+ struct xe_sysctrl sc;
>+
> /** @nvm: discrete graphics non-volatile memory */
> struct intel_dg_nvm_dev *nvm;
>
>diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>index 91e0553a8163..b6dc3030b673 100644
>--- a/drivers/gpu/drm/xe/xe_pci.c
>+++ b/drivers/gpu/drm/xe/xe_pci.c
>@@ -426,6 +426,7 @@ static const struct xe_device_desc cri_desc = {
> .has_soc_remapper_sysctrl = true,
> .has_soc_remapper_telem = true,
> .has_sriov = true,
>+ .has_sysctrl = true,
> .max_gt_per_tile = 2,
> .require_force_probe = true,
> .va_bits = 57,
>@@ -701,6 +702,7 @@ static int xe_info_init_early(struct xe_device *xe,
> xe->info.has_soc_remapper_telem = desc->has_soc_remapper_telem;
> xe->info.has_sriov = xe_configfs_primary_gt_allowed(to_pci_dev(xe->drm.dev)) &&
> desc->has_sriov;
>+ xe->info.has_sysctrl = desc->has_sysctrl;
> xe->info.has_mem_copy_instr = desc->has_mem_copy_instr;
> xe->info.skip_guc_pc = desc->skip_guc_pc;
> xe->info.skip_mtcfg = desc->skip_mtcfg;
>diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
>index 5f20f56571d1..53e44a32883d 100644
>--- a/drivers/gpu/drm/xe/xe_pci_types.h
>+++ b/drivers/gpu/drm/xe/xe_pci_types.h
>@@ -56,6 +56,7 @@ struct xe_device_desc {
> u8 has_soc_remapper_sysctrl:1;
> u8 has_soc_remapper_telem:1;
> u8 has_sriov:1;
>+ u8 has_sysctrl:1;
> u8 needs_scratch:1;
> u8 skip_guc_pc:1;
> u8 skip_mtcfg:1;
>diff --git a/drivers/gpu/drm/xe/xe_sysctrl.c b/drivers/gpu/drm/xe/xe_sysctrl.c
>new file mode 100644
>index 000000000000..e0e7b0ecf2bf
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/xe_sysctrl.c
>@@ -0,0 +1,62 @@
>+// SPDX-License-Identifier: MIT
>+/*
>+ * Copyright © 2026 Intel Corporation
>+ */
>+
>+#include <drm/drm_managed.h>
>+#include <linux/device.h>
>+#include <linux/mutex.h>
>+
>+#include "regs/xe_sysctrl_regs.h"
>+#include "xe_device.h"
>+#include "xe_printk.h"
>+#include "xe_soc_remapper.h"
>+#include "xe_sysctrl.h"
>+#include "xe_sysctrl_mailbox.h"
>+#include "xe_sysctrl_types.h"
>+
>+static void xe_sysctrl_fini(void *arg)
>+{
>+ struct xe_sysctrl *sc = arg;
>+ struct xe_device *xe = sc_to_xe(sc);
Instead of using sc_to_xe here, just pass the xe as arg.
>+
>+ if (!xe->soc_remapper.set_sysctrl_region)
>+ return;
>+
>+ xe->soc_remapper.set_sysctrl_region(xe, 0);
>+}
>+
>+/**
>+ * xe_sysctrl_init - Initialize SC subsystem
>+ * @xe: xe device instance
>+ *
>+ * Entry point for SC initialization, called from xe_device_probe().
>+ * This function checks platform support and initializes the system controller.
>+ *
>+ * Return: 0 on success, error code on failure
>+ */
>+int xe_sysctrl_init(struct xe_device *xe)
>+{
>+ struct xe_sysctrl *sc = &xe->sc;
>+ int ret;
>+
>+ if (!xe->info.has_sysctrl)
>+ return 0;
>+
>+ ret = devm_add_action_or_reset(xe->drm.dev, xe_sysctrl_fini, sc);
>+ if (ret)
>+ return ret;
>+
>+ if (!xe->soc_remapper.set_sysctrl_region)
>+ return -ENODEV;
>+
>+ xe->soc_remapper.set_sysctrl_region(xe, SYSCTRL_MAILBOX_INDEX);
>+
>+ ret = drmm_mutex_init(&xe->drm, &sc->cmd_lock);
>+ if (ret)
>+ return ret;
>+
>+ xe_sysctrl_mailbox_init(sc);
>+
>+ return 0;
>+}
>diff --git a/drivers/gpu/drm/xe/xe_sysctrl.h b/drivers/gpu/drm/xe/xe_sysctrl.h
>new file mode 100644
>index 000000000000..fe90d6577d54
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/xe_sysctrl.h
>@@ -0,0 +1,18 @@
>+/* SPDX-License-Identifier: MIT */
>+/*
>+ * Copyright © 2026 Intel Corporation
>+ */
>+
>+#ifndef _XE_SYSCTRL_H_
>+#define _XE_SYSCTRL_H_
>+
>+struct xe_device;
>+
>+static inline struct xe_device *sc_to_xe(struct xe_sysctrl *sc)
>+{
>+ return container_of(sc, struct xe_device, sc);
>+}
I would move this to the xe_sysctrl_mailbox.c. Also compile fails here due
to missing struct xe_sysctrl declaration.
>+
>+int xe_sysctrl_init(struct xe_device *xe);
>+
>+#endif /* _XE_SYSCTRL_H_ */
>diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
>new file mode 100644
>index 000000000000..940ea535da2e
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
>@@ -0,0 +1,409 @@
>+// SPDX-License-Identifier: MIT
>+/*
>+ * Copyright © 2026 Intel Corporation
>+ */
>+
>+#include <linux/bitfield.h>
>+#include <linux/errno.h>
>+#include <linux/minmax.h>
>+#include <linux/mutex.h>
>+#include <linux/slab.h>
>+#include <linux/string.h>
>+#include <linux/types.h>
>+
>+#include "regs/xe_sysctrl_regs.h"
>+#include "xe_device.h"
>+#include "xe_mmio.h"
>+#include "xe_pm.h"
>+#include "xe_printk.h"
>+#include "xe_sysctrl.h"
>+#include "xe_sysctrl_mailbox.h"
>+#include "xe_sysctrl_types.h"
>+
>+static bool xe_sysctrl_mailbox_wait_bit_clear(struct xe_sysctrl *sc, u32 bit_mask,
>+ unsigned int timeout_ms)
>+{
>+ struct xe_device *xe = sc_to_xe(sc);
>+ struct xe_mmio *mmio = xe_root_tile_mmio(xe);
>+ int ret;
>+
>+ ret = xe_mmio_wait32_not(mmio, SC_MB_CTRL, bit_mask, bit_mask,
>+ timeout_ms * 1000, NULL, false);
>+
>+ return ret == 0;
>+}
>+
>+static bool xe_sysctrl_mailbox_wait_bit_set(struct xe_sysctrl *sc, u32 bit_mask,
>+ unsigned int timeout_ms)
>+{
>+ struct xe_device *xe = sc_to_xe(sc);
>+ struct xe_mmio *mmio = xe_root_tile_mmio(xe);
>+ int ret;
>+
>+ ret = xe_mmio_wait32(mmio, SC_MB_CTRL, bit_mask, bit_mask,
>+ timeout_ms * 1000, NULL, false);
>+
>+ return ret == 0;
>+}
>+
>+static int xe_sysctrl_mailbox_write_frame(struct xe_sysctrl *sc, const void *frame,
>+ size_t len)
>+{
>+ static const struct xe_reg regs[] = {
>+ SC_MB_DATA0, SC_MB_DATA1, SC_MB_DATA2, SC_MB_DATA3
>+ };
>+ struct xe_device *xe = sc_to_xe(sc);
>+ struct xe_mmio *mmio = xe_root_tile_mmio(xe);
>+ u32 val[SC_MB_FRAME_SIZE / sizeof(u32)] = {0};
>+ u32 dw = DIV_ROUND_UP(len, sizeof(u32));
>+ u32 i;
>+
>+ memcpy(val, frame, len);
>+
>+ for (i = 0; i < dw; i++)
>+ xe_mmio_write32(mmio, regs[i], val[i]);
>+
>+ return 0;
>+}
>+
>+static int xe_sysctrl_mailbox_read_frame(struct xe_sysctrl *sc, void *frame,
>+ size_t len)
>+{
>+ static const struct xe_reg regs[] = {
>+ SC_MB_DATA0, SC_MB_DATA1, SC_MB_DATA2, SC_MB_DATA3
>+ };
>+ struct xe_device *xe = sc_to_xe(sc);
>+ struct xe_mmio *mmio = xe_root_tile_mmio(xe);
>+ u32 val[SC_MB_FRAME_SIZE / sizeof(u32)] = {0};
>+ u32 dw = DIV_ROUND_UP(len, sizeof(u32));
>+ u32 i;
>+
>+ for (i = 0; i < dw; i++)
>+ val[i] = xe_mmio_read32(mmio, regs[i]);
>+
>+ memcpy(frame, val, len);
>+
>+ return 0;
>+}
>+
>+static void xe_sysctrl_mailbox_clear_response(struct xe_sysctrl *sc)
>+{
>+ struct xe_device *xe = sc_to_xe(sc);
>+ struct xe_mmio *mmio = xe_root_tile_mmio(xe);
>+
>+ xe_mmio_rmw32(mmio, SC_MB_CTRL, SC_MB_CTRL_RUN_BUSY_OUT, 0);
>+}
>+
>+static int xe_sysctrl_mailbox_prepare_command(struct xe_sysctrl *sc,
>+ u8 group_id, u8 command,
>+ const void *data_in, size_t data_in_len,
>+ u8 **mbox_cmd, size_t *cmd_size)
>+{
>+ struct xe_device *xe = sc_to_xe(sc);
>+ struct xe_sysctrl_mailbox_mkhi_msg_hdr *mkhi_hdr;
>+ size_t size;
>+ u8 *buffer;
>+
>+ size = sizeof(*mkhi_hdr) + data_in_len;
>+ if (size > SC_MB_MAX_MESSAGE_SIZE) {
>+ xe_err(xe, "sysctrl: Message too large: %zu bytes\n", size);
>+ return -EINVAL;
>+ }
>+
>+ buffer = kmalloc(size, GFP_KERNEL);
>+ if (!buffer)
>+ return -ENOMEM;
>+
>+ mkhi_hdr = (struct xe_sysctrl_mailbox_mkhi_msg_hdr *)buffer;
>+ mkhi_hdr->data = cpu_to_le32(FIELD_PREP(MKHI_HDR_GROUP_ID_MASK, group_id) |
>+ FIELD_PREP(MKHI_HDR_COMMAND_MASK, command & 0x7F) |
>+ FIELD_PREP(MKHI_HDR_IS_RESPONSE, 0) |
>+ FIELD_PREP(MKHI_HDR_RESERVED_MASK, 0) |
>+ FIELD_PREP(MKHI_HDR_RESULT_MASK, 0));
>+
>+ if (data_in && data_in_len)
>+ memcpy(buffer + sizeof(*mkhi_hdr), data_in, data_in_len);
>+
>+ *mbox_cmd = buffer;
>+ *cmd_size = size;
>+
>+ return 0;
>+}
In all the functions above, you do not use the sc pointer at all, so why
not just pass xe to the functions? If you do that, then in most functions
below you will not use sc pointer and you can do away with the sc_to_xe
conversion altogether and pass xe everywhere. Note that you only need the sc
object in
xe_sysctrl_init()
xe_sysctrl_mailbox_send_frames()
xe_sysctrl_mailbox_init()
>+
>+static int xe_sysctrl_mailbox_send_frames(struct xe_sysctrl *sc, const u8 *mbox_cmd,
>+ size_t cmd_size, unsigned int timeout_ms)
>+{
>+ struct xe_device *xe = sc_to_xe(sc);
>+ struct xe_mmio *mmio = xe_root_tile_mmio(xe);
>+ u32 ctrl_reg, total_frames, frame;
>+ size_t bytes_sent, frame_size;
>+
>+ total_frames = DIV_ROUND_UP(cmd_size, SC_MB_FRAME_SIZE);
>+
>+ if (!xe_sysctrl_mailbox_wait_bit_clear(sc, SC_MB_CTRL_RUN_BUSY, timeout_ms)) {
>+ xe_err(xe, "sysctrl: Mailbox busy\n");
>+ return -EBUSY;
>+ }
>+
>+ sc->phase_bit ^= 1;
>+ bytes_sent = 0;
>+
>+ for (frame = 0; frame < total_frames; frame++) {
>+ frame_size = min(cmd_size - bytes_sent, (size_t)SC_MB_FRAME_SIZE);
>+
>+ if (xe_sysctrl_mailbox_write_frame(sc, mbox_cmd + bytes_sent, frame_size)) {
>+ xe_err(xe, "sysctrl: Failed to write frame %u\n", frame);
>+ sc->phase_bit ^= 1;
>+ return -EIO;
>+ }
>+
>+ ctrl_reg = SC_MB_CTRL_RUN_BUSY |
>+ FIELD_PREP(MKHI_FRAME_CURRENT_MASK, frame) |
>+ FIELD_PREP(MKHI_FRAME_TOTAL_MASK, total_frames - 1) |
>+ FIELD_PREP(MKHI_FRAME_COMMAND_MASK, SC_MKHI_COMMAND) |
>+ (sc->phase_bit ? MKHI_FRAME_PHASE : 0);
>+
>+ xe_mmio_write32(mmio, SC_MB_CTRL, ctrl_reg);
>+
>+ if (!xe_sysctrl_mailbox_wait_bit_clear(sc, SC_MB_CTRL_RUN_BUSY, timeout_ms)) {
>+ xe_err(xe, "sysctrl: Frame %u acknowledgment timeout\n", frame);
>+ return -ETIMEDOUT;
>+ }
>+
>+ bytes_sent += frame_size;
>+ }
>+
>+ return 0;
>+}
>+
>+static int xe_sysctrl_mailbox_process_first_frame(struct xe_sysctrl *sc,
>+ const struct xe_sysctrl_mailbox_mkhi_msg_hdr *req,
>+ void *out,
>+ size_t frame_size,
>+ size_t *payload_bytes)
>+{
>+ struct xe_device *xe = sc_to_xe(sc);
>+ u32 frame_data[4];
>+ struct xe_sysctrl_mailbox_mkhi_msg_hdr *resp_hdr;
>+ size_t hdr_size = sizeof(*resp_hdr);
>+ size_t payload_size;
>+ int ret;
I would rearrange the variables in decreasing line length. Not a coding
guideline, but is generally advised to do so.
struct xe_sysctrl_mailbox_mkhi_msg_hdr *resp_hdr;
struct xe_device *xe = sc_to_xe(sc);
size_t hdr_size = sizeof(*resp_hdr);
size_t payload_size;
u32 frame_data[4];
int ret;
Also, please avoid hardcoded 4 in frame_data dimension. It could be
frame_data[SC_MB_FRAME_SIZE / sizeof(u32)]
>+
>+ ret = xe_sysctrl_mailbox_read_frame(sc, frame_data, frame_size);
>+ if (ret)
>+ return ret;
>+
>+ resp_hdr = (struct xe_sysctrl_mailbox_mkhi_msg_hdr *)frame_data;
>+
>+ if (!XE_SYSCTRL_MKHI_HDR_IS_RESPONSE(resp_hdr) ||
>+ XE_SYSCTRL_MKHI_HDR_GROUP_ID(resp_hdr) != XE_SYSCTRL_MKHI_HDR_GROUP_ID(req) ||
>+ XE_SYSCTRL_MKHI_HDR_COMMAND(resp_hdr) != XE_SYSCTRL_MKHI_HDR_COMMAND(req)) {
>+ xe_err(xe, "SC: Response header mismatch\n");
>+ return -EPROTO;
>+ }
>+
>+ if (XE_SYSCTRL_MKHI_HDR_RESULT(resp_hdr) != 0) {
>+ xe_err(xe, "SC: Firmware error: 0x%02lx\n",
>+ XE_SYSCTRL_MKHI_HDR_RESULT(resp_hdr));
>+ return -EIO;
>+ }
>+
>+ payload_size = frame_size - hdr_size;
>+ if (payload_size > 0)
>+ memcpy(out, (u8 *)frame_data + hdr_size, payload_size);
>+
>+ *payload_bytes = payload_size;
>+
>+ xe_sysctrl_mailbox_clear_response(sc);
>+
>+ return 0;
>+}
>+
>+static int xe_sysctrl_mailbox_process_frame(struct xe_sysctrl *sc,
>+ void *out, size_t frame_size,
>+ unsigned int timeout_ms)
>+{
>+ struct xe_device *xe = sc_to_xe(sc);
>+ int ret;
>+
>+ if (!xe_sysctrl_mailbox_wait_bit_set(sc, SC_MB_CTRL_RUN_BUSY_OUT, timeout_ms)) {
>+ xe_err(xe, "sysctrl: Response frame timeout\n");
>+ return -ETIMEDOUT;
>+ }
>+
>+ ret = xe_sysctrl_mailbox_read_frame(sc, out, frame_size);
>+ if (ret)
>+ return ret;
>+
>+ xe_sysctrl_mailbox_clear_response(sc);
>+
>+ return 0;
>+}
>+
>+static int xe_sysctrl_mailbox_receive_frames(struct xe_sysctrl *sc,
>+ const struct xe_sysctrl_mailbox_mkhi_msg_hdr *req,
>+ void *data_out, size_t data_out_len,
>+ size_t *rdata_len, unsigned int timeout_ms)
>+{
>+ struct xe_device *xe = sc_to_xe(sc);
>+ struct xe_mmio *mmio = xe_root_tile_mmio(xe);
>+ struct xe_sysctrl_mailbox_mkhi_msg_hdr *mkhi_hdr;
>+ u32 ctrl_reg, total_frames, frame;
>+ size_t hdr_size = sizeof(*mkhi_hdr);
>+ u8 *out = data_out;
>+ size_t received = 0;
>+ size_t frame_size;
>+ int ret = 0;
>+
>+ if (!xe_sysctrl_mailbox_wait_bit_set(sc, SC_MB_CTRL_RUN_BUSY_OUT, timeout_ms)) {
>+ xe_err(xe, "sysctrl: Response frame 0 timeout\n");
>+ return -ETIMEDOUT;
>+ }
>+
>+ ctrl_reg = xe_mmio_read32(mmio, SC_MB_CTRL);
>+ total_frames = FIELD_GET(MKHI_FRAME_TOTAL_MASK, ctrl_reg) + 1;
>+
>+ if (total_frames == 1)
>+ frame_size = min(hdr_size + data_out_len, (size_t)SC_MB_FRAME_SIZE);
>+ else
>+ frame_size = SC_MB_FRAME_SIZE;
>+
>+ ret = xe_sysctrl_mailbox_process_first_frame(sc, req, out, frame_size, &received);
>+ if (ret)
>+ return ret;
>+
>+ out += received;
>+
>+ for (frame = 1; frame < total_frames; frame++) {
>+ size_t remaining = data_out_len - received;
>+
>+ frame_size = min_t(size_t, remaining, SC_MB_FRAME_SIZE);
>+
>+ ret = xe_sysctrl_mailbox_process_frame(sc, out, frame_size, timeout_ms);
>+ if (ret)
>+ break;
>+
>+ received += frame_size;
>+ out += frame_size;
>+ }
I fell there should be a simpler way to implement this function. I will get
back if I can think of anything.
>+
>+ *rdata_len = received;
>+
>+ return ret;
>+}
>+
>+static int xe_sysctrl_mailbox_send_command(struct xe_sysctrl *sc,
>+ const u8 *mbox_cmd, size_t cmd_size,
>+ void *data_out, size_t data_out_len,
>+ size_t *rdata_len, unsigned int timeout_ms)
>+{
>+ const struct xe_sysctrl_mailbox_mkhi_msg_hdr *mkhi_hdr;
>+ size_t received;
>+ int ret;
>+
>+ ret = xe_sysctrl_mailbox_send_frames(sc, mbox_cmd, cmd_size, timeout_ms);
>+ if (ret)
>+ return ret;
>+
>+ if (!data_out || !rdata_len)
>+ return 0;
>+
>+ mkhi_hdr = (const struct xe_sysctrl_mailbox_mkhi_msg_hdr *)mbox_cmd;
>+
>+ ret = xe_sysctrl_mailbox_receive_frames(sc, mkhi_hdr, data_out, data_out_len,
>+ &received, timeout_ms);
>+ if (ret)
>+ return ret;
>+
>+ *rdata_len = received;
>+
>+ return 0;
>+}
>+
>+/**
>+ * xe_sysctrl_send_command - Send command to System Controller via mailbox
>+ * @handle: XE device handle
>+ * @cmd_buffer: Pointer to xe_sysctrl_mailbox_command structure
>+ * @rdata_len: Pointer to store actual response data size (can be NULL)
>+ *
>+ * Send a command to the System Controller using MKHI protocol. Handles
>+ * command preparation, fragmentation, transmission, and response reception.
>+ *
>+ * Return: 0 on success, negative error code on failure
>+ */
>+int xe_sysctrl_send_command(void *handle, void *cmd_buffer, size_t *rdata_len)
>+{
>+ struct xe_device *xe = handle;
>+ struct xe_sysctrl *sc = &xe->sc;
>+ struct xe_sysctrl_mailbox_command *cmd = cmd_buffer;
>+ u8 *mbox_cmd = NULL;
>+ size_t cmd_size = 0;
>+ u8 group_id, command_code;
>+ int ret = 0;
>+
>+ if (!xe) {
>+ pr_err("sysctrl: Invalid device handle\n");
>+ return -EINVAL;
>+ }
>+
>+ if (!cmd) {
>+ xe_err(xe, "sysctrl: Invalid command buffer\n");
>+ return -EINVAL;
>+ }
>+
>+ if (!xe->info.has_sysctrl)
>+ return -ENODEV;
>+
>+ group_id = XE_SYSCTRL_APP_HDR_GROUP_ID(&cmd->header);
>+ command_code = XE_SYSCTRL_APP_HDR_COMMAND(&cmd->header);
>+
>+ if (!cmd->data_in && cmd->data_in_len) {
>+ xe_err(xe, "sysctrl: Invalid input parameters\n");
>+ return -EINVAL;
>+ }
>+
>+ if (!cmd->data_out && cmd->data_out_len) {
>+ xe_err(xe, "sysctrl: Invalid output parameters\n");
>+ return -EINVAL;
>+ }
>+
>+ might_sleep();
>+
>+ ret = xe_sysctrl_mailbox_prepare_command(sc, group_id, command_code,
>+ cmd->data_in, cmd->data_in_len,
>+ &mbox_cmd, &cmd_size);
>+ if (ret) {
>+ xe_err(xe, "sysctrl: Failed to prepare command: %d\n", ret);
>+ return ret;
>+ }
>+
>+ guard(xe_pm_runtime)(xe);
>+
>+ guard(mutex)(&sc->cmd_lock);
>+
>+ ret = xe_sysctrl_mailbox_send_command(sc, mbox_cmd, cmd_size,
>+ cmd->data_out, cmd->data_out_len, rdata_len,
>+ SC_MB_DEFAULT_TIMEOUT_MS);
>+ if (ret)
>+ xe_err(xe, "sysctrl: Mailbox command failed: %d\n", ret);
>+
>+ kfree(mbox_cmd);
>+
>+ return ret;
>+}
>+
>+/**
>+ * xe_sysctrl_mailbox_init - Initialize the System Controller mailbox state
>+ * @sc: System controller structure
>+ */
>+void xe_sysctrl_mailbox_init(struct xe_sysctrl *sc)
>+{
>+ struct xe_device *xe = sc_to_xe(sc);
>+ struct xe_mmio *mmio = xe_root_tile_mmio(xe);
>+ u32 ctrl_reg;
>+
>+ ctrl_reg = xe_mmio_read32(mmio, SC_MB_CTRL);
>+ sc->phase_bit = (ctrl_reg & MKHI_FRAME_PHASE) ? 1 : 0;
>+
>+ xe_mmio_rmw32(mmio, SC_MB_CTRL, MKHI_FRAME_PHASE, 0);
>+}
>diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.h b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.h
>new file mode 100644
>index 000000000000..3e472418ebd0
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.h
>@@ -0,0 +1,77 @@
>+/* SPDX-License-Identifier: MIT */
>+/*
>+ * Copyright © 2026 Intel Corporation
>+ */
>+
>+#ifndef __XE_SYSCTRL_MAILBOX_H__
>+#define __XE_SYSCTRL_MAILBOX_H__
>+
>+#include <linux/bitfield.h>
>+#include <linux/types.h>
>+
>+struct xe_sysctrl;
>+
>+#define MKHI_HDR_GROUP_ID_MASK GENMASK(7, 0)
>+#define MKHI_HDR_COMMAND_MASK GENMASK(14, 8)
>+#define MKHI_HDR_IS_RESPONSE BIT(15)
>+#define MKHI_HDR_RESERVED_MASK GENMASK(23, 16)
>+#define MKHI_HDR_RESULT_MASK GENMASK(31, 24)
Same comment as (2) above. If the MKHI defines are solely used by
xe_sysctrl_mailbox.c, then you can just move them there. I assume that the
app is only using the APP_HDR and is not concerned with the underlying
protocol/transport.
>+
>+struct xe_sysctrl_mailbox_mkhi_msg_hdr {
>+ __le32 data;
>+} __packed;
>+
>+#define APP_HDR_GROUP_ID_MASK GENMASK(7, 0)
>+#define APP_HDR_COMMAND_MASK GENMASK(15, 8)
>+#define APP_HDR_VERSION_MASK GENMASK(23, 16)
>+#define APP_HDR_RESERVED_MASK GENMASK(31, 24)
>+
>+struct xe_sysctrl_mailbox_app_msg_hdr {
>+ __le32 data;
>+} __packed;
>+
>+#define XE_SYSCTRL_APP_HDR_GROUP_ID(hdr) \
>+ FIELD_GET(APP_HDR_GROUP_ID_MASK, le32_to_cpu((hdr)->data))
>+
>+#define XE_SYSCTRL_APP_HDR_COMMAND(hdr) \
>+ FIELD_GET(APP_HDR_COMMAND_MASK, le32_to_cpu((hdr)->data))
>+
>+#define XE_SYSCTRL_APP_HDR_VERSION(hdr) \
>+ FIELD_GET(APP_HDR_VERSION_MASK, le32_to_cpu((hdr)->data))
>+
>+#define XE_SYSCTRL_MKHI_HDR_GROUP_ID(hdr) \
>+ FIELD_GET(MKHI_HDR_GROUP_ID_MASK, le32_to_cpu((hdr)->data))
>+
>+#define XE_SYSCTRL_MKHI_HDR_COMMAND(hdr) \
>+ FIELD_GET(MKHI_HDR_COMMAND_MASK, le32_to_cpu((hdr)->data))
>+
>+#define XE_SYSCTRL_MKHI_HDR_IS_RESPONSE(hdr) \
>+ FIELD_GET(MKHI_HDR_IS_RESPONSE, le32_to_cpu((hdr)->data))
>+
>+#define XE_SYSCTRL_MKHI_HDR_RESULT(hdr) \
>+ FIELD_GET(MKHI_HDR_RESULT_MASK, le32_to_cpu((hdr)->data))
Same here. MKHI defines can move to the C file. Either that or the APP api
can be moved to a separate header - xe_sysctrl_mailbox_api.h.
Regards,
Umesh
>+
>+/**
>+ * struct xe_sysctrl_mailbox_command - System Controller mailbox command structure
>+ */
>+struct xe_sysctrl_mailbox_command {
>+ /** @header: Application message header containing command information */
>+ struct xe_sysctrl_mailbox_app_msg_hdr header;
>+
>+ /** @data_in: Pointer to input payload data (can be NULL if no input data) */
>+ void *data_in;
>+
>+ /** @data_in_len: Size of input payload in bytes (0 if no input data) */
>+ size_t data_in_len;
>+
>+ /** @data_out: Pointer to output buffer for response data (can be NULL if no response) */
>+ void *data_out;
>+
>+ /** @data_out_len: Size of output buffer in bytes (0 if no response expected) */
>+ size_t data_out_len;
>+};
>+
>+void xe_sysctrl_mailbox_init(struct xe_sysctrl *sc);
>+int xe_sysctrl_send_command(void *handle, void *cmd_buffer, size_t *rdata_len);
>+
>+#endif /* __XE_SYSCTRL_MAILBOX_H__ */
>diff --git a/drivers/gpu/drm/xe/xe_sysctrl_types.h b/drivers/gpu/drm/xe/xe_sysctrl_types.h
>new file mode 100644
>index 000000000000..88a34967688b
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/xe_sysctrl_types.h
>@@ -0,0 +1,23 @@
>+/* SPDX-License-Identifier: MIT */
>+/*
>+ * Copyright © 2026 Intel Corporation
>+ */
>+
>+#ifndef _XE_SYSCTRL_TYPES_H_
>+#define _XE_SYSCTRL_TYPES_H_
>+
>+#include <linux/mutex.h>
>+#include <linux/types.h>
>+
>+/**
>+ * struct xe_sysctrl - System Controller driver context
>+ */
>+struct xe_sysctrl {
>+ /** @cmd_lock: Mutex protecting mailbox command operations */
>+ struct mutex cmd_lock;
>+
>+ /** @phase_bit: MKHI message boundary phase toggle bit */
>+ u32 phase_bit;
>+};
>+
>+#endif /* _XE_SYSCTRL_TYPES_H_ */
>--
>2.43.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/1] drm/xe/sysctrl: Add system controller component for Xe3p dGPU platforms
2026-01-06 18:37 ` Umesh Nerlige Ramappa
@ 2026-01-07 1:24 ` Matthew Brost
0 siblings, 0 replies; 8+ messages in thread
From: Matthew Brost @ 2026-01-07 1:24 UTC (permalink / raw)
To: Umesh Nerlige Ramappa
Cc: Anoop, Vijay, intel-xe, badal.nilawar, rodrigo.vivi,
aravind.iddamsetty, riana.tauro, ravi.kishore.koppuravuri,
anshuman.gupta, matthew.d.roper, michael.j.ruhl, paul.e.luse,
mohamed.mansoor.v, kam.nasim
On Tue, Jan 06, 2026 at 10:37:46AM -0800, Umesh Nerlige Ramappa wrote:
> Hi Anoop,
>
> Thanks for incorporating most of the comments here. I have a few more in
> this revision.
>
> On Fri, Jan 02, 2026 at 08:54:50AM -0800, Anoop, Vijay wrote:
> > From: Anoop Vijay <anoop.c.vijay@intel.com>
> >
> > Add a new system controller (sysctrl) component for Intel Xe3p dGPU
> > platforms.
> >
> > This component provides the foundational infrastructure for communication
> > with the System Controller firmware using MKHI protocol over a mailbox
> > interface.
> >
> > Key features introduced:
> > - Detection and initialization of System Controller interface on Xe3p
> > dGPU platforms
> > - Mailbox communication with System Controller firmware
> > - Fragmented message transfer for large command payloads
>
> Let's break down the patches as Matt suggested.
>
> >
> > This implementation establishes the base for future System Controller
> > feature enablement and firmware command handling.
> >
> > Signed-off-by: Anoop Vijay <anoop.c.vijay@intel.com>
> > ---
> > drivers/gpu/drm/xe/Makefile | 2 +
> > drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h | 44 +++
> > drivers/gpu/drm/xe/xe_device.c | 5 +
> > drivers/gpu/drm/xe/xe_device_types.h | 6 +
> > drivers/gpu/drm/xe/xe_pci.c | 2 +
> > drivers/gpu/drm/xe/xe_pci_types.h | 1 +
> > drivers/gpu/drm/xe/xe_sysctrl.c | 62 ++++
> > drivers/gpu/drm/xe/xe_sysctrl.h | 18 +
> > drivers/gpu/drm/xe/xe_sysctrl_mailbox.c | 409 ++++++++++++++++++++++
> > drivers/gpu/drm/xe/xe_sysctrl_mailbox.h | 77 ++++
> > drivers/gpu/drm/xe/xe_sysctrl_types.h | 23 ++
> > 11 files changed, 649 insertions(+)
> > create mode 100644 drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h
> > create mode 100644 drivers/gpu/drm/xe/xe_sysctrl.c
> > create mode 100644 drivers/gpu/drm/xe/xe_sysctrl.h
> > create mode 100644 drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> > create mode 100644 drivers/gpu/drm/xe/xe_sysctrl_mailbox.h
> > create mode 100644 drivers/gpu/drm/xe/xe_sysctrl_types.h
> >
> > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> > index 2b20c79d7ec9..947fbcac65d5 100644
> > --- a/drivers/gpu/drm/xe/Makefile
> > +++ b/drivers/gpu/drm/xe/Makefile
> > @@ -121,6 +121,8 @@ xe-y += xe_bb.o \
> > xe_step.o \
> > xe_survivability_mode.o \
> > xe_sync.o \
> > + xe_sysctrl.o \
> > + xe_sysctrl_mailbox.o \
> > xe_tile.o \
> > xe_tile_sysfs.o \
> > xe_tlb_inval.o \
> > diff --git a/drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h b/drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h
> > new file mode 100644
> > index 000000000000..6627a9c32c4f
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/regs/xe_sysctrl_regs.h
> > @@ -0,0 +1,44 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2026 Intel Corporation
> > + */
> > +
> > +#ifndef _XE_SYSCTRL_REGS_H_
> > +#define _XE_SYSCTRL_REGS_H_
> > +
> > +#include "xe_regs.h"
> > +
> > +#define SYSCTRL_BASE_OFFSET 0xDB000
> > +#define SYSCTRL_BASE (SOC_BASE + SYSCTRL_BASE_OFFSET)
> > +#define SYSCTRL_MAILBOX_INDEX 0x03
> > +#define SC_BAR_LENGTH 0x1000
> > +
> > +#define SC_MB_CTRL XE_REG(SYSCTRL_BASE + 0x10)
> > +#define SC_MB_CTRL_RUN_BUSY REG_BIT(31)
> > +#define SC_MB_CTRL_IRQ REG_BIT(30)
> > +#define SC_MB_CTRL_RUN_BUSY_OUT REG_BIT(29)
> > +#define SC_MB_CTRL_PARAM3_MASK REG_GENMASK(28, 24)
> > +#define SC_MB_CTRL_PARAM2_MASK REG_GENMASK(23, 16)
> > +#define SC_MB_CTRL_PARAM1_MASK REG_GENMASK(15, 8)
> > +#define SC_MB_CTRL_COMMAND_MASK REG_GENMASK(7, 0)
> > +
> > +#define SC_MB_DATA0 XE_REG(SYSCTRL_BASE + 0x14)
> > +#define SC_MB_DATA1 XE_REG(SYSCTRL_BASE + 0x18)
> > +#define SC_MB_DATA2 XE_REG(SYSCTRL_BASE + 0x1C)
> > +#define SC_MB_DATA3 XE_REG(SYSCTRL_BASE + 0x20)
> > +
> > +#define MKHI_FRAME_PHASE REG_BIT(24)
> > +#define MKHI_FRAME_CURRENT_MASK REG_GENMASK(21, 16)
> > +#define MKHI_FRAME_TOTAL_MASK REG_GENMASK(13, 8)
> > +#define MKHI_FRAME_COMMAND_MASK REG_GENMASK(7, 0)
> > +
> > +#define SC_MB_FRAME_SIZE 16
> > +#define SC_MB_MAX_FRAMES 64
> > +#define SC_MB_MAX_MESSAGE_SIZE (SC_MB_FRAME_SIZE * SC_MB_MAX_FRAMES)
> > +#define SC_MKHI_COMMAND 5
> > +
> > +#define SC_MB_DEFAULT_TIMEOUT_MS 500
> > +#define SC_MB_RETRY_TIMEOUT_MS 20
> > +#define SC_MB_POLL_INTERVAL_US 100
>
> 1)
> I missed this the last time. Can we change SC to SYSCTRL here for
> everything? I see that for the mailbox, you are using SC_MB, but at the end
> of the day SC still means SYSCTRL and consistently using the same name helps
> readability.
>
> 2)
> The MKHI_FRAME definitions can move to the xe_sysctrl_mailbox.c since that's
> the only code accessing it. See more comments below.
>
>
> > +
> > +#endif /* _XE_SYSCTRL_REGS_H_ */
> > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> > index e101d290b2a6..805d48dd954d 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -66,6 +66,7 @@
> > #include "xe_survivability_mode.h"
> > #include "xe_sriov.h"
> > #include "xe_svm.h"
> > +#include "xe_sysctrl.h"
> > #include "xe_tile.h"
> > #include "xe_ttm_stolen_mgr.h"
> > #include "xe_ttm_sys_mgr.h"
> > @@ -1032,6 +1033,10 @@ int xe_device_probe(struct xe_device *xe)
> > if (err)
> > goto err_unregister_display;
> >
> > + err = xe_sysctrl_init(xe);
> > + if (err)
> > + goto err_unregister_display;
> > +
> > err = xe_device_sysfs_init(xe);
> > if (err)
> > goto err_unregister_display;
> > diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> > index a85be9ba175e..6295b2c35d4a 100644
> > --- a/drivers/gpu/drm/xe/xe_device_types.h
> > +++ b/drivers/gpu/drm/xe/xe_device_types.h
> > @@ -29,6 +29,7 @@
> > #include "xe_sriov_vf_ccs_types.h"
> > #include "xe_step_types.h"
> > #include "xe_survivability_mode_types.h"
> > +#include "xe_sysctrl_types.h"
> > #include "xe_tile_sriov_vf_types.h"
> > #include "xe_validation.h"
> >
> > @@ -340,6 +341,8 @@ struct xe_device {
> > u8 has_soc_remapper_telem:1;
> > /** @info.has_sriov: Supports SR-IOV */
> > u8 has_sriov:1;
> > + /** @info.has_sysctrl: Supports System Controller */
> > + u8 has_sysctrl:1;
> > /** @info.has_usm: Device has unified shared memory support */
> > u8 has_usm:1;
> > /** @info.has_64bit_timestamp: Device supports 64-bit timestamps */
> > @@ -606,6 +609,9 @@ struct xe_device {
> > /** @heci_gsc: graphics security controller */
> > struct xe_heci_gsc heci_gsc;
> >
> > + /** @sc: System Controller */
> > + struct xe_sysctrl sc;
> > +
> > /** @nvm: discrete graphics non-volatile memory */
> > struct intel_dg_nvm_dev *nvm;
> >
> > diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> > index 91e0553a8163..b6dc3030b673 100644
> > --- a/drivers/gpu/drm/xe/xe_pci.c
> > +++ b/drivers/gpu/drm/xe/xe_pci.c
> > @@ -426,6 +426,7 @@ static const struct xe_device_desc cri_desc = {
> > .has_soc_remapper_sysctrl = true,
> > .has_soc_remapper_telem = true,
> > .has_sriov = true,
> > + .has_sysctrl = true,
> > .max_gt_per_tile = 2,
> > .require_force_probe = true,
> > .va_bits = 57,
> > @@ -701,6 +702,7 @@ static int xe_info_init_early(struct xe_device *xe,
> > xe->info.has_soc_remapper_telem = desc->has_soc_remapper_telem;
> > xe->info.has_sriov = xe_configfs_primary_gt_allowed(to_pci_dev(xe->drm.dev)) &&
> > desc->has_sriov;
> > + xe->info.has_sysctrl = desc->has_sysctrl;
> > xe->info.has_mem_copy_instr = desc->has_mem_copy_instr;
> > xe->info.skip_guc_pc = desc->skip_guc_pc;
> > xe->info.skip_mtcfg = desc->skip_mtcfg;
> > diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
> > index 5f20f56571d1..53e44a32883d 100644
> > --- a/drivers/gpu/drm/xe/xe_pci_types.h
> > +++ b/drivers/gpu/drm/xe/xe_pci_types.h
> > @@ -56,6 +56,7 @@ struct xe_device_desc {
> > u8 has_soc_remapper_sysctrl:1;
> > u8 has_soc_remapper_telem:1;
> > u8 has_sriov:1;
> > + u8 has_sysctrl:1;
> > u8 needs_scratch:1;
> > u8 skip_guc_pc:1;
> > u8 skip_mtcfg:1;
> > diff --git a/drivers/gpu/drm/xe/xe_sysctrl.c b/drivers/gpu/drm/xe/xe_sysctrl.c
> > new file mode 100644
> > index 000000000000..e0e7b0ecf2bf
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_sysctrl.c
> > @@ -0,0 +1,62 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2026 Intel Corporation
> > + */
> > +
> > +#include <drm/drm_managed.h>
> > +#include <linux/device.h>
> > +#include <linux/mutex.h>
> > +
> > +#include "regs/xe_sysctrl_regs.h"
> > +#include "xe_device.h"
> > +#include "xe_printk.h"
> > +#include "xe_soc_remapper.h"
> > +#include "xe_sysctrl.h"
> > +#include "xe_sysctrl_mailbox.h"
> > +#include "xe_sysctrl_types.h"
> > +
> > +static void xe_sysctrl_fini(void *arg)
> > +{
> > + struct xe_sysctrl *sc = arg;
> > + struct xe_device *xe = sc_to_xe(sc);
>
> Instead of using sc_to_xe here, just pass the xe as arg.
> > +
> > + if (!xe->soc_remapper.set_sysctrl_region)
> > + return;
> > +
> > + xe->soc_remapper.set_sysctrl_region(xe, 0);
> > +}
> > +
> > +/**
> > + * xe_sysctrl_init - Initialize SC subsystem
> > + * @xe: xe device instance
> > + *
> > + * Entry point for SC initialization, called from xe_device_probe().
> > + * This function checks platform support and initializes the system controller.
> > + *
> > + * Return: 0 on success, error code on failure
> > + */
> > +int xe_sysctrl_init(struct xe_device *xe)
> > +{
> > + struct xe_sysctrl *sc = &xe->sc;
> > + int ret;
> > +
> > + if (!xe->info.has_sysctrl)
> > + return 0;
> > +
> > + ret = devm_add_action_or_reset(xe->drm.dev, xe_sysctrl_fini, sc);
> > + if (ret)
> > + return ret;
> > +
> > + if (!xe->soc_remapper.set_sysctrl_region)
> > + return -ENODEV;
> > +
> > + xe->soc_remapper.set_sysctrl_region(xe, SYSCTRL_MAILBOX_INDEX);
> > +
> > + ret = drmm_mutex_init(&xe->drm, &sc->cmd_lock);
> > + if (ret)
> > + return ret;
> > +
> > + xe_sysctrl_mailbox_init(sc);
> > +
> > + return 0;
> > +}
> > diff --git a/drivers/gpu/drm/xe/xe_sysctrl.h b/drivers/gpu/drm/xe/xe_sysctrl.h
> > new file mode 100644
> > index 000000000000..fe90d6577d54
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_sysctrl.h
> > @@ -0,0 +1,18 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2026 Intel Corporation
> > + */
> > +
> > +#ifndef _XE_SYSCTRL_H_
> > +#define _XE_SYSCTRL_H_
> > +
> > +struct xe_device;
> > +
> > +static inline struct xe_device *sc_to_xe(struct xe_sysctrl *sc)
> > +{
> > + return container_of(sc, struct xe_device, sc);
> > +}
>
> I would move this to the xe_sysctrl_mailbox.c. Also compile fails here due
> to missing struct xe_sysctrl declaration.
>
> > +
> > +int xe_sysctrl_init(struct xe_device *xe);
> > +
> > +#endif /* _XE_SYSCTRL_H_ */
> > diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> > new file mode 100644
> > index 000000000000..940ea535da2e
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> > @@ -0,0 +1,409 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2026 Intel Corporation
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/errno.h>
> > +#include <linux/minmax.h>
> > +#include <linux/mutex.h>
> > +#include <linux/slab.h>
> > +#include <linux/string.h>
> > +#include <linux/types.h>
> > +
> > +#include "regs/xe_sysctrl_regs.h"
> > +#include "xe_device.h"
> > +#include "xe_mmio.h"
> > +#include "xe_pm.h"
> > +#include "xe_printk.h"
> > +#include "xe_sysctrl.h"
> > +#include "xe_sysctrl_mailbox.h"
> > +#include "xe_sysctrl_types.h"
> > +
> > +static bool xe_sysctrl_mailbox_wait_bit_clear(struct xe_sysctrl *sc, u32 bit_mask,
> > + unsigned int timeout_ms)
> > +{
> > + struct xe_device *xe = sc_to_xe(sc);
> > + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> > + int ret;
> > +
> > + ret = xe_mmio_wait32_not(mmio, SC_MB_CTRL, bit_mask, bit_mask,
> > + timeout_ms * 1000, NULL, false);
> > +
> > + return ret == 0;
> > +}
> > +
> > +static bool xe_sysctrl_mailbox_wait_bit_set(struct xe_sysctrl *sc, u32 bit_mask,
> > + unsigned int timeout_ms)
> > +{
> > + struct xe_device *xe = sc_to_xe(sc);
> > + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> > + int ret;
> > +
> > + ret = xe_mmio_wait32(mmio, SC_MB_CTRL, bit_mask, bit_mask,
> > + timeout_ms * 1000, NULL, false);
> > +
> > + return ret == 0;
> > +}
> > +
> > +static int xe_sysctrl_mailbox_write_frame(struct xe_sysctrl *sc, const void *frame,
> > + size_t len)
> > +{
> > + static const struct xe_reg regs[] = {
> > + SC_MB_DATA0, SC_MB_DATA1, SC_MB_DATA2, SC_MB_DATA3
> > + };
> > + struct xe_device *xe = sc_to_xe(sc);
> > + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> > + u32 val[SC_MB_FRAME_SIZE / sizeof(u32)] = {0};
> > + u32 dw = DIV_ROUND_UP(len, sizeof(u32));
> > + u32 i;
> > +
> > + memcpy(val, frame, len);
> > +
> > + for (i = 0; i < dw; i++)
> > + xe_mmio_write32(mmio, regs[i], val[i]);
> > +
> > + return 0;
> > +}
> > +
> > +static int xe_sysctrl_mailbox_read_frame(struct xe_sysctrl *sc, void *frame,
> > + size_t len)
> > +{
> > + static const struct xe_reg regs[] = {
> > + SC_MB_DATA0, SC_MB_DATA1, SC_MB_DATA2, SC_MB_DATA3
> > + };
> > + struct xe_device *xe = sc_to_xe(sc);
> > + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> > + u32 val[SC_MB_FRAME_SIZE / sizeof(u32)] = {0};
> > + u32 dw = DIV_ROUND_UP(len, sizeof(u32));
> > + u32 i;
> > +
> > + for (i = 0; i < dw; i++)
> > + val[i] = xe_mmio_read32(mmio, regs[i]);
> > +
> > + memcpy(frame, val, len);
> > +
> > + return 0;
> > +}
> > +
> > +static void xe_sysctrl_mailbox_clear_response(struct xe_sysctrl *sc)
> > +{
> > + struct xe_device *xe = sc_to_xe(sc);
> > + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> > +
> > + xe_mmio_rmw32(mmio, SC_MB_CTRL, SC_MB_CTRL_RUN_BUSY_OUT, 0);
> > +}
> > +
> > +static int xe_sysctrl_mailbox_prepare_command(struct xe_sysctrl *sc,
> > + u8 group_id, u8 command,
> > + const void *data_in, size_t data_in_len,
> > + u8 **mbox_cmd, size_t *cmd_size)
> > +{
> > + struct xe_device *xe = sc_to_xe(sc);
> > + struct xe_sysctrl_mailbox_mkhi_msg_hdr *mkhi_hdr;
> > + size_t size;
> > + u8 *buffer;
> > +
> > + size = sizeof(*mkhi_hdr) + data_in_len;
> > + if (size > SC_MB_MAX_MESSAGE_SIZE) {
> > + xe_err(xe, "sysctrl: Message too large: %zu bytes\n", size);
> > + return -EINVAL;
> > + }
> > +
> > + buffer = kmalloc(size, GFP_KERNEL);
> > + if (!buffer)
> > + return -ENOMEM;
> > +
> > + mkhi_hdr = (struct xe_sysctrl_mailbox_mkhi_msg_hdr *)buffer;
> > + mkhi_hdr->data = cpu_to_le32(FIELD_PREP(MKHI_HDR_GROUP_ID_MASK, group_id) |
> > + FIELD_PREP(MKHI_HDR_COMMAND_MASK, command & 0x7F) |
> > + FIELD_PREP(MKHI_HDR_IS_RESPONSE, 0) |
> > + FIELD_PREP(MKHI_HDR_RESERVED_MASK, 0) |
> > + FIELD_PREP(MKHI_HDR_RESULT_MASK, 0));
> > +
> > + if (data_in && data_in_len)
> > + memcpy(buffer + sizeof(*mkhi_hdr), data_in, data_in_len);
> > +
> > + *mbox_cmd = buffer;
> > + *cmd_size = size;
> > +
> > + return 0;
> > +}
>
> In all the functions above, you do not use the sc pointer at all, so why
> not just pass xe to the functions? If you do that, then in most functions
> below you will not use sc pointer and you can do away with the sc_to_xe
> conversion altogether and pass xe everywhere. Note that you only need the sc
> object in
>
> xe_sysctrl_init()
> xe_sysctrl_mailbox_send_frames()
> xe_sysctrl_mailbox_init()
>
> > +
> > +static int xe_sysctrl_mailbox_send_frames(struct xe_sysctrl *sc, const u8 *mbox_cmd,
> > + size_t cmd_size, unsigned int timeout_ms)
> > +{
> > + struct xe_device *xe = sc_to_xe(sc);
> > + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> > + u32 ctrl_reg, total_frames, frame;
> > + size_t bytes_sent, frame_size;
> > +
> > + total_frames = DIV_ROUND_UP(cmd_size, SC_MB_FRAME_SIZE);
> > +
> > + if (!xe_sysctrl_mailbox_wait_bit_clear(sc, SC_MB_CTRL_RUN_BUSY, timeout_ms)) {
> > + xe_err(xe, "sysctrl: Mailbox busy\n");
> > + return -EBUSY;
> > + }
> > +
> > + sc->phase_bit ^= 1;
> > + bytes_sent = 0;
> > +
> > + for (frame = 0; frame < total_frames; frame++) {
> > + frame_size = min(cmd_size - bytes_sent, (size_t)SC_MB_FRAME_SIZE);
> > +
> > + if (xe_sysctrl_mailbox_write_frame(sc, mbox_cmd + bytes_sent, frame_size)) {
> > + xe_err(xe, "sysctrl: Failed to write frame %u\n", frame);
> > + sc->phase_bit ^= 1;
> > + return -EIO;
> > + }
> > +
> > + ctrl_reg = SC_MB_CTRL_RUN_BUSY |
> > + FIELD_PREP(MKHI_FRAME_CURRENT_MASK, frame) |
> > + FIELD_PREP(MKHI_FRAME_TOTAL_MASK, total_frames - 1) |
> > + FIELD_PREP(MKHI_FRAME_COMMAND_MASK, SC_MKHI_COMMAND) |
> > + (sc->phase_bit ? MKHI_FRAME_PHASE : 0);
> > +
> > + xe_mmio_write32(mmio, SC_MB_CTRL, ctrl_reg);
> > +
> > + if (!xe_sysctrl_mailbox_wait_bit_clear(sc, SC_MB_CTRL_RUN_BUSY, timeout_ms)) {
> > + xe_err(xe, "sysctrl: Frame %u acknowledgment timeout\n", frame);
> > + return -ETIMEDOUT;
> > + }
> > +
> > + bytes_sent += frame_size;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int xe_sysctrl_mailbox_process_first_frame(struct xe_sysctrl *sc,
> > + const struct xe_sysctrl_mailbox_mkhi_msg_hdr *req,
> > + void *out,
> > + size_t frame_size,
> > + size_t *payload_bytes)
> > +{
> > + struct xe_device *xe = sc_to_xe(sc);
> > + u32 frame_data[4];
> > + struct xe_sysctrl_mailbox_mkhi_msg_hdr *resp_hdr;
> > + size_t hdr_size = sizeof(*resp_hdr);
> > + size_t payload_size;
> > + int ret;
>
> I would rearrange the variables in decreasing line length. Not a coding
> guideline, but is generally advised to do so.
>
> struct xe_sysctrl_mailbox_mkhi_msg_hdr *resp_hdr;
> struct xe_device *xe = sc_to_xe(sc);
> size_t hdr_size = sizeof(*resp_hdr);
> size_t payload_size;
> u32 frame_data[4];
> int ret;
>
> Also, please avoid hardcoded 4 in frame_data dimension. It could be
> frame_data[SC_MB_FRAME_SIZE / sizeof(u32)]
>
> > +
> > + ret = xe_sysctrl_mailbox_read_frame(sc, frame_data, frame_size);
> > + if (ret)
> > + return ret;
> > +
> > + resp_hdr = (struct xe_sysctrl_mailbox_mkhi_msg_hdr *)frame_data;
> > +
> > + if (!XE_SYSCTRL_MKHI_HDR_IS_RESPONSE(resp_hdr) ||
> > + XE_SYSCTRL_MKHI_HDR_GROUP_ID(resp_hdr) != XE_SYSCTRL_MKHI_HDR_GROUP_ID(req) ||
> > + XE_SYSCTRL_MKHI_HDR_COMMAND(resp_hdr) != XE_SYSCTRL_MKHI_HDR_COMMAND(req)) {
> > + xe_err(xe, "SC: Response header mismatch\n");
> > + return -EPROTO;
> > + }
> > +
> > + if (XE_SYSCTRL_MKHI_HDR_RESULT(resp_hdr) != 0) {
> > + xe_err(xe, "SC: Firmware error: 0x%02lx\n",
> > + XE_SYSCTRL_MKHI_HDR_RESULT(resp_hdr));
> > + return -EIO;
> > + }
> > +
> > + payload_size = frame_size - hdr_size;
> > + if (payload_size > 0)
> > + memcpy(out, (u8 *)frame_data + hdr_size, payload_size);
> > +
> > + *payload_bytes = payload_size;
> > +
> > + xe_sysctrl_mailbox_clear_response(sc);
> > +
> > + return 0;
> > +}
> > +
> > +static int xe_sysctrl_mailbox_process_frame(struct xe_sysctrl *sc,
> > + void *out, size_t frame_size,
> > + unsigned int timeout_ms)
> > +{
> > + struct xe_device *xe = sc_to_xe(sc);
> > + int ret;
> > +
> > + if (!xe_sysctrl_mailbox_wait_bit_set(sc, SC_MB_CTRL_RUN_BUSY_OUT, timeout_ms)) {
> > + xe_err(xe, "sysctrl: Response frame timeout\n");
> > + return -ETIMEDOUT;
> > + }
> > +
> > + ret = xe_sysctrl_mailbox_read_frame(sc, out, frame_size);
> > + if (ret)
> > + return ret;
> > +
> > + xe_sysctrl_mailbox_clear_response(sc);
> > +
> > + return 0;
> > +}
> > +
> > +static int xe_sysctrl_mailbox_receive_frames(struct xe_sysctrl *sc,
> > + const struct xe_sysctrl_mailbox_mkhi_msg_hdr *req,
> > + void *data_out, size_t data_out_len,
> > + size_t *rdata_len, unsigned int timeout_ms)
> > +{
> > + struct xe_device *xe = sc_to_xe(sc);
> > + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> > + struct xe_sysctrl_mailbox_mkhi_msg_hdr *mkhi_hdr;
> > + u32 ctrl_reg, total_frames, frame;
> > + size_t hdr_size = sizeof(*mkhi_hdr);
> > + u8 *out = data_out;
> > + size_t received = 0;
> > + size_t frame_size;
> > + int ret = 0;
> > +
> > + if (!xe_sysctrl_mailbox_wait_bit_set(sc, SC_MB_CTRL_RUN_BUSY_OUT, timeout_ms)) {
> > + xe_err(xe, "sysctrl: Response frame 0 timeout\n");
> > + return -ETIMEDOUT;
> > + }
> > +
> > + ctrl_reg = xe_mmio_read32(mmio, SC_MB_CTRL);
> > + total_frames = FIELD_GET(MKHI_FRAME_TOTAL_MASK, ctrl_reg) + 1;
> > +
> > + if (total_frames == 1)
> > + frame_size = min(hdr_size + data_out_len, (size_t)SC_MB_FRAME_SIZE);
> > + else
> > + frame_size = SC_MB_FRAME_SIZE;
> > +
> > + ret = xe_sysctrl_mailbox_process_first_frame(sc, req, out, frame_size, &received);
> > + if (ret)
> > + return ret;
> > +
> > + out += received;
> > +
> > + for (frame = 1; frame < total_frames; frame++) {
> > + size_t remaining = data_out_len - received;
> > +
> > + frame_size = min_t(size_t, remaining, SC_MB_FRAME_SIZE);
> > +
> > + ret = xe_sysctrl_mailbox_process_frame(sc, out, frame_size, timeout_ms);
> > + if (ret)
> > + break;
> > +
> > + received += frame_size;
> > + out += frame_size;
> > + }
>
> I fell there should be a simpler way to implement this function. I will get
> back if I can think of anything.
>
> > +
> > + *rdata_len = received;
> > +
> > + return ret;
> > +}
> > +
> > +static int xe_sysctrl_mailbox_send_command(struct xe_sysctrl *sc,
> > + const u8 *mbox_cmd, size_t cmd_size,
> > + void *data_out, size_t data_out_len,
> > + size_t *rdata_len, unsigned int timeout_ms)
> > +{
> > + const struct xe_sysctrl_mailbox_mkhi_msg_hdr *mkhi_hdr;
> > + size_t received;
> > + int ret;
> > +
> > + ret = xe_sysctrl_mailbox_send_frames(sc, mbox_cmd, cmd_size, timeout_ms);
> > + if (ret)
> > + return ret;
> > +
> > + if (!data_out || !rdata_len)
> > + return 0;
> > +
> > + mkhi_hdr = (const struct xe_sysctrl_mailbox_mkhi_msg_hdr *)mbox_cmd;
> > +
> > + ret = xe_sysctrl_mailbox_receive_frames(sc, mkhi_hdr, data_out, data_out_len,
> > + &received, timeout_ms);
> > + if (ret)
> > + return ret;
> > +
> > + *rdata_len = received;
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xe_sysctrl_send_command - Send command to System Controller via mailbox
> > + * @handle: XE device handle
> > + * @cmd_buffer: Pointer to xe_sysctrl_mailbox_command structure
> > + * @rdata_len: Pointer to store actual response data size (can be NULL)
> > + *
> > + * Send a command to the System Controller using MKHI protocol. Handles
> > + * command preparation, fragmentation, transmission, and response reception.
> > + *
> > + * Return: 0 on success, negative error code on failure
> > + */
> > +int xe_sysctrl_send_command(void *handle, void *cmd_buffer, size_t *rdata_len)
> > +{
> > + struct xe_device *xe = handle;
> > + struct xe_sysctrl *sc = &xe->sc;
> > + struct xe_sysctrl_mailbox_command *cmd = cmd_buffer;
> > + u8 *mbox_cmd = NULL;
> > + size_t cmd_size = 0;
> > + u8 group_id, command_code;
> > + int ret = 0;
> > +
> > + if (!xe) {
> > + pr_err("sysctrl: Invalid device handle\n");
> > + return -EINVAL;
> > + }
> > +
> > + if (!cmd) {
> > + xe_err(xe, "sysctrl: Invalid command buffer\n");
> > + return -EINVAL;
> > + }
> > +
> > + if (!xe->info.has_sysctrl)
> > + return -ENODEV;
> > +
> > + group_id = XE_SYSCTRL_APP_HDR_GROUP_ID(&cmd->header);
> > + command_code = XE_SYSCTRL_APP_HDR_COMMAND(&cmd->header);
> > +
> > + if (!cmd->data_in && cmd->data_in_len) {
> > + xe_err(xe, "sysctrl: Invalid input parameters\n");
> > + return -EINVAL;
> > + }
> > +
> > + if (!cmd->data_out && cmd->data_out_len) {
> > + xe_err(xe, "sysctrl: Invalid output parameters\n");
> > + return -EINVAL;
> > + }
> > +
> > + might_sleep();
> > +
> > + ret = xe_sysctrl_mailbox_prepare_command(sc, group_id, command_code,
> > + cmd->data_in, cmd->data_in_len,
> > + &mbox_cmd, &cmd_size);
> > + if (ret) {
> > + xe_err(xe, "sysctrl: Failed to prepare command: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + guard(xe_pm_runtime)(xe);
> > +
> > + guard(mutex)(&sc->cmd_lock);
> > +
> > + ret = xe_sysctrl_mailbox_send_command(sc, mbox_cmd, cmd_size,
> > + cmd->data_out, cmd->data_out_len, rdata_len,
> > + SC_MB_DEFAULT_TIMEOUT_MS);
> > + if (ret)
> > + xe_err(xe, "sysctrl: Mailbox command failed: %d\n", ret);
> > +
> > + kfree(mbox_cmd);
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * xe_sysctrl_mailbox_init - Initialize the System Controller mailbox state
> > + * @sc: System controller structure
> > + */
> > +void xe_sysctrl_mailbox_init(struct xe_sysctrl *sc)
> > +{
> > + struct xe_device *xe = sc_to_xe(sc);
> > + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> > + u32 ctrl_reg;
> > +
> > + ctrl_reg = xe_mmio_read32(mmio, SC_MB_CTRL);
> > + sc->phase_bit = (ctrl_reg & MKHI_FRAME_PHASE) ? 1 : 0;
> > +
> > + xe_mmio_rmw32(mmio, SC_MB_CTRL, MKHI_FRAME_PHASE, 0);
> > +}
> > diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.h b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.h
> > new file mode 100644
> > index 000000000000..3e472418ebd0
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.h
> > @@ -0,0 +1,77 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2026 Intel Corporation
> > + */
> > +
> > +#ifndef __XE_SYSCTRL_MAILBOX_H__
> > +#define __XE_SYSCTRL_MAILBOX_H__
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/types.h>
> > +
> > +struct xe_sysctrl;
> > +
> > +#define MKHI_HDR_GROUP_ID_MASK GENMASK(7, 0)
> > +#define MKHI_HDR_COMMAND_MASK GENMASK(14, 8)
> > +#define MKHI_HDR_IS_RESPONSE BIT(15)
> > +#define MKHI_HDR_RESERVED_MASK GENMASK(23, 16)
> > +#define MKHI_HDR_RESULT_MASK GENMASK(31, 24)
>
> Same comment as (2) above. If the MKHI defines are solely used by
> xe_sysctrl_mailbox.c, then you can just move them there. I assume that the
> app is only using the APP_HDR and is not concerned with the underlying
> protocol/transport.
>
> > +
> > +struct xe_sysctrl_mailbox_mkhi_msg_hdr {
> > + __le32 data;
> > +} __packed;
> > +
> > +#define APP_HDR_GROUP_ID_MASK GENMASK(7, 0)
> > +#define APP_HDR_COMMAND_MASK GENMASK(15, 8)
> > +#define APP_HDR_VERSION_MASK GENMASK(23, 16)
> > +#define APP_HDR_RESERVED_MASK GENMASK(31, 24)
> > +
> > +struct xe_sysctrl_mailbox_app_msg_hdr {
> > + __le32 data;
> > +} __packed;
> > +
> > +#define XE_SYSCTRL_APP_HDR_GROUP_ID(hdr) \
> > + FIELD_GET(APP_HDR_GROUP_ID_MASK, le32_to_cpu((hdr)->data))
> > +
> > +#define XE_SYSCTRL_APP_HDR_COMMAND(hdr) \
> > + FIELD_GET(APP_HDR_COMMAND_MASK, le32_to_cpu((hdr)->data))
> > +
> > +#define XE_SYSCTRL_APP_HDR_VERSION(hdr) \
> > + FIELD_GET(APP_HDR_VERSION_MASK, le32_to_cpu((hdr)->data))
> > +
> > +#define XE_SYSCTRL_MKHI_HDR_GROUP_ID(hdr) \
> > + FIELD_GET(MKHI_HDR_GROUP_ID_MASK, le32_to_cpu((hdr)->data))
> > +
> > +#define XE_SYSCTRL_MKHI_HDR_COMMAND(hdr) \
> > + FIELD_GET(MKHI_HDR_COMMAND_MASK, le32_to_cpu((hdr)->data))
> > +
> > +#define XE_SYSCTRL_MKHI_HDR_IS_RESPONSE(hdr) \
> > + FIELD_GET(MKHI_HDR_IS_RESPONSE, le32_to_cpu((hdr)->data))
> > +
> > +#define XE_SYSCTRL_MKHI_HDR_RESULT(hdr) \
> > + FIELD_GET(MKHI_HDR_RESULT_MASK, le32_to_cpu((hdr)->data))
>
> Same here. MKHI defines can move to the C file. Either that or the APP api
> can be moved to a separate header - xe_sysctrl_mailbox_api.h.
>
> Regards,
> Umesh
>
> > +
> > +/**
> > + * struct xe_sysctrl_mailbox_command - System Controller mailbox command structure
> > + */
> > +struct xe_sysctrl_mailbox_command {
> > + /** @header: Application message header containing command information */
> > + struct xe_sysctrl_mailbox_app_msg_hdr header;
> > +
> > + /** @data_in: Pointer to input payload data (can be NULL if no input data) */
> > + void *data_in;
> > +
> > + /** @data_in_len: Size of input payload in bytes (0 if no input data) */
> > + size_t data_in_len;
> > +
> > + /** @data_out: Pointer to output buffer for response data (can be NULL if no response) */
> > + void *data_out;
> > +
> > + /** @data_out_len: Size of output buffer in bytes (0 if no response expected) */
> > + size_t data_out_len;
> > +};
> > +
> > +void xe_sysctrl_mailbox_init(struct xe_sysctrl *sc);
> > +int xe_sysctrl_send_command(void *handle, void *cmd_buffer, size_t *rdata_len);
The above function looks to be unused too.
Also why 'void *handle' rather than xe_device or xe_sysctrl?
Matt
> > +
> > +#endif /* __XE_SYSCTRL_MAILBOX_H__ */
> > diff --git a/drivers/gpu/drm/xe/xe_sysctrl_types.h b/drivers/gpu/drm/xe/xe_sysctrl_types.h
> > new file mode 100644
> > index 000000000000..88a34967688b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_sysctrl_types.h
> > @@ -0,0 +1,23 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2026 Intel Corporation
> > + */
> > +
> > +#ifndef _XE_SYSCTRL_TYPES_H_
> > +#define _XE_SYSCTRL_TYPES_H_
> > +
> > +#include <linux/mutex.h>
> > +#include <linux/types.h>
> > +
> > +/**
> > + * struct xe_sysctrl - System Controller driver context
> > + */
> > +struct xe_sysctrl {
> > + /** @cmd_lock: Mutex protecting mailbox command operations */
> > + struct mutex cmd_lock;
> > +
> > + /** @phase_bit: MKHI message boundary phase toggle bit */
> > + u32 phase_bit;
> > +};
> > +
> > +#endif /* _XE_SYSCTRL_TYPES_H_ */
> > --
> > 2.43.0
> >
^ permalink raw reply [flat|nested] 8+ messages in thread