* Re: [PATCH v3 02/10] mei: late_bind: add late binding component driver
@ 2025-06-24 3:14 kernel test robot
0 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2025-06-24 3:14 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20250618190007.2932322-3-badal.nilawar@intel.com>
References: <20250618190007.2932322-3-badal.nilawar@intel.com>
TO: Badal Nilawar <badal.nilawar@intel.com>
Hi Badal,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-xe/drm-xe-next]
[also build test WARNING on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus drm-i915/for-linux-next drm-i915/for-linux-next-fixes linus/master v6.16-rc3 next-20250623]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Badal-Nilawar/mei-bus-add-mei_cldev_mtu-interface/20250619-025825
base: https://gitlab.freedesktop.org/drm/xe/kernel.git drm-xe-next
patch link: https://lore.kernel.org/r/20250618190007.2932322-3-badal.nilawar%40intel.com
patch subject: [PATCH v3 02/10] mei: late_bind: add late binding component driver
:::::: branch date: 5 days ago
:::::: commit date: 5 days ago
config: i386-randconfig-141-20250623 (https://download.01.org/0day-ci/archive/20250624/202506241103.XiG3WA7g-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202506241103.XiG3WA7g-lkp@intel.com/
smatch warnings:
drivers/misc/mei/late_bind/mei_late_bind.c:203 mei_late_bind_component_match() warn: was && intended here instead of ||?
vim +203 drivers/misc/mei/late_bind/mei_late_bind.c
b4fe758cb831cd Alexander Usyskin 2025-06-19 170
b4fe758cb831cd Alexander Usyskin 2025-06-19 171 /**
b4fe758cb831cd Alexander Usyskin 2025-06-19 172 * mei_late_bind_component_match - compare function for matching mei late bind.
b4fe758cb831cd Alexander Usyskin 2025-06-19 173 *
b4fe758cb831cd Alexander Usyskin 2025-06-19 174 * The function checks if requested is Intel VGA device
b4fe758cb831cd Alexander Usyskin 2025-06-19 175 * and the parent of requester and the grand parent of mei_if are the same
b4fe758cb831cd Alexander Usyskin 2025-06-19 176 * device.
b4fe758cb831cd Alexander Usyskin 2025-06-19 177 *
b4fe758cb831cd Alexander Usyskin 2025-06-19 178 * @dev: master device
b4fe758cb831cd Alexander Usyskin 2025-06-19 179 * @subcomponent: subcomponent to match (I915_COMPONENT_LATE_BIND)
b4fe758cb831cd Alexander Usyskin 2025-06-19 180 * @data: compare data (mei late-bind bus device)
b4fe758cb831cd Alexander Usyskin 2025-06-19 181 *
b4fe758cb831cd Alexander Usyskin 2025-06-19 182 * Return:
b4fe758cb831cd Alexander Usyskin 2025-06-19 183 * * 1 - if components match
b4fe758cb831cd Alexander Usyskin 2025-06-19 184 * * 0 - otherwise
b4fe758cb831cd Alexander Usyskin 2025-06-19 185 */
b4fe758cb831cd Alexander Usyskin 2025-06-19 186 static int mei_late_bind_component_match(struct device *dev, int subcomponent,
b4fe758cb831cd Alexander Usyskin 2025-06-19 187 void *data)
b4fe758cb831cd Alexander Usyskin 2025-06-19 188 {
b4fe758cb831cd Alexander Usyskin 2025-06-19 189 struct device *base = data;
b4fe758cb831cd Alexander Usyskin 2025-06-19 190 struct pci_dev *pdev;
b4fe758cb831cd Alexander Usyskin 2025-06-19 191
b4fe758cb831cd Alexander Usyskin 2025-06-19 192 if (!dev)
b4fe758cb831cd Alexander Usyskin 2025-06-19 193 return 0;
b4fe758cb831cd Alexander Usyskin 2025-06-19 194
b4fe758cb831cd Alexander Usyskin 2025-06-19 195 if (!dev_is_pci(dev))
b4fe758cb831cd Alexander Usyskin 2025-06-19 196 return 0;
b4fe758cb831cd Alexander Usyskin 2025-06-19 197
b4fe758cb831cd Alexander Usyskin 2025-06-19 198 pdev = to_pci_dev(dev);
b4fe758cb831cd Alexander Usyskin 2025-06-19 199
b4fe758cb831cd Alexander Usyskin 2025-06-19 200 if (pdev->vendor != PCI_VENDOR_ID_INTEL)
b4fe758cb831cd Alexander Usyskin 2025-06-19 201 return 0;
b4fe758cb831cd Alexander Usyskin 2025-06-19 202
b4fe758cb831cd Alexander Usyskin 2025-06-19 @203 if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) ||
b4fe758cb831cd Alexander Usyskin 2025-06-19 204 pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8))
b4fe758cb831cd Alexander Usyskin 2025-06-19 205 return 0;
b4fe758cb831cd Alexander Usyskin 2025-06-19 206
b4fe758cb831cd Alexander Usyskin 2025-06-19 207 if (subcomponent != I915_COMPONENT_LATE_BIND)
b4fe758cb831cd Alexander Usyskin 2025-06-19 208 return 0;
b4fe758cb831cd Alexander Usyskin 2025-06-19 209
b4fe758cb831cd Alexander Usyskin 2025-06-19 210 base = base->parent;
b4fe758cb831cd Alexander Usyskin 2025-06-19 211 if (!base) /* mei device */
b4fe758cb831cd Alexander Usyskin 2025-06-19 212 return 0;
b4fe758cb831cd Alexander Usyskin 2025-06-19 213
b4fe758cb831cd Alexander Usyskin 2025-06-19 214 base = base->parent; /* pci device */
b4fe758cb831cd Alexander Usyskin 2025-06-19 215
b4fe758cb831cd Alexander Usyskin 2025-06-19 216 return !!base && dev == base;
b4fe758cb831cd Alexander Usyskin 2025-06-19 217 }
b4fe758cb831cd Alexander Usyskin 2025-06-19 218
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 00/10] Introducing firmware late binding
@ 2025-06-18 18:59 Badal Nilawar
2025-06-18 18:59 ` [PATCH v3 02/10] mei: late_bind: add late binding component driver Badal Nilawar
0 siblings, 1 reply; 6+ messages in thread
From: Badal Nilawar @ 2025-06-18 18:59 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
Introducing firmware late binding feature to enable firmware loading
for the devices, such as the fan controller and voltage regulator,
during the driver probe.
Typically, firmware for these devices are part of IFWI flash image but
can be replaced at probe after OEM tuning.
v2:
- Dropped voltage regulator specific code as binaries for it will not
be available for upstreaming as of now.
- Address review comments
v3:
- Dropped fwctl patch for now
- Added new patch to extract binary version
- Address v2 review comments
Alexander Usyskin (2):
mei: bus: add mei_cldev_mtu interface
mei: late_bind: add late binding component driver
Badal Nilawar (8):
drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw
drm/xe/xe_late_bind_fw: Initialize late binding firmware
drm/xe/xe_late_bind_fw: Load late binding firmware
drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume
drm/xe/xe_late_bind_fw: Reload late binding fw in S2Idle/S3 resume
drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late
binding
drm/xe/xe_late_bind_fw: Extract and print version info
[CI]drm/xe/xe_late_bind_fw: Select INTEL_MEI_LATE_BIND for CI
drivers/gpu/drm/xe/Kconfig | 1 +
drivers/gpu/drm/xe/Makefile | 1 +
drivers/gpu/drm/xe/xe_debugfs.c | 41 ++
drivers/gpu/drm/xe/xe_device.c | 5 +
drivers/gpu/drm/xe/xe_device_types.h | 6 +
drivers/gpu/drm/xe/xe_late_bind_fw.c | 432 ++++++++++++++++++++
drivers/gpu/drm/xe/xe_late_bind_fw.h | 17 +
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 82 ++++
drivers/gpu/drm/xe/xe_pci.c | 3 +
drivers/gpu/drm/xe/xe_pm.c | 9 +
drivers/gpu/drm/xe/xe_uc_fw_abi.h | 69 ++++
drivers/misc/mei/Kconfig | 1 +
drivers/misc/mei/Makefile | 1 +
drivers/misc/mei/bus.c | 13 +
drivers/misc/mei/late_bind/Kconfig | 13 +
drivers/misc/mei/late_bind/Makefile | 9 +
drivers/misc/mei/late_bind/mei_late_bind.c | 263 ++++++++++++
include/drm/intel/i915_component.h | 1 +
include/drm/intel/late_bind_mei_interface.h | 50 +++
include/linux/mei_cl_bus.h | 1 +
20 files changed, 1018 insertions(+)
create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.c
create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.h
create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw_types.h
create mode 100644 drivers/misc/mei/late_bind/Kconfig
create mode 100644 drivers/misc/mei/late_bind/Makefile
create mode 100644 drivers/misc/mei/late_bind/mei_late_bind.c
create mode 100644 include/drm/intel/late_bind_mei_interface.h
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 02/10] mei: late_bind: add late binding component driver 2025-06-18 18:59 [PATCH v3 00/10] Introducing firmware late binding Badal Nilawar @ 2025-06-18 18:59 ` Badal Nilawar 2025-06-19 7:32 ` Gupta, Anshuman 2025-06-24 13:37 ` Dan Carpenter 0 siblings, 2 replies; 6+ messages in thread From: Badal Nilawar @ 2025-06-18 18:59 UTC (permalink / raw) To: intel-xe, dri-devel, linux-kernel Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, daniele.ceraolospurio, jgg From: Alexander Usyskin <alexander.usyskin@intel.com> Add late binding component driver. It allows pushing the late binding configuration from, for example, the Xe graphics driver to the Intel discrete graphics card's CSE device. Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com> --- v2: - Use generic naming (Jani) - Drop xe_late_bind_component struct to move to xe code (Daniele/Sasha) v3: - Updated kconfig description - Move CSC late binding specific flags/defines to late_bind_mei_interface.h (Daniele) v4: - Add match for PCI_CLASS_DISPLAY_OTHER to support headless cards (Anshuman) --- drivers/misc/mei/Kconfig | 1 + drivers/misc/mei/Makefile | 1 + drivers/misc/mei/late_bind/Kconfig | 13 + drivers/misc/mei/late_bind/Makefile | 9 + drivers/misc/mei/late_bind/mei_late_bind.c | 264 ++++++++++++++++++++ include/drm/intel/i915_component.h | 1 + include/drm/intel/late_bind_mei_interface.h | 50 ++++ 7 files changed, 339 insertions(+) create mode 100644 drivers/misc/mei/late_bind/Kconfig create mode 100644 drivers/misc/mei/late_bind/Makefile create mode 100644 drivers/misc/mei/late_bind/mei_late_bind.c create mode 100644 include/drm/intel/late_bind_mei_interface.h diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig index 7575fee96cc6..771becc68095 100644 --- a/drivers/misc/mei/Kconfig +++ b/drivers/misc/mei/Kconfig @@ -84,5 +84,6 @@ config INTEL_MEI_VSC source "drivers/misc/mei/hdcp/Kconfig" source "drivers/misc/mei/pxp/Kconfig" source "drivers/misc/mei/gsc_proxy/Kconfig" +source "drivers/misc/mei/late_bind/Kconfig" endif diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile index 6f9fdbf1a495..84bfde888d81 100644 --- a/drivers/misc/mei/Makefile +++ b/drivers/misc/mei/Makefile @@ -31,6 +31,7 @@ CFLAGS_mei-trace.o = -I$(src) obj-$(CONFIG_INTEL_MEI_HDCP) += hdcp/ obj-$(CONFIG_INTEL_MEI_PXP) += pxp/ obj-$(CONFIG_INTEL_MEI_GSC_PROXY) += gsc_proxy/ +obj-$(CONFIG_INTEL_MEI_LATE_BIND) += late_bind/ obj-$(CONFIG_INTEL_MEI_VSC_HW) += mei-vsc-hw.o mei-vsc-hw-y := vsc-tp.o diff --git a/drivers/misc/mei/late_bind/Kconfig b/drivers/misc/mei/late_bind/Kconfig new file mode 100644 index 000000000000..65c7180c5678 --- /dev/null +++ b/drivers/misc/mei/late_bind/Kconfig @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2025, Intel Corporation. All rights reserved. +# +config INTEL_MEI_LATE_BIND + tristate "Intel late binding support on ME Interface" + select INTEL_MEI_ME + depends on DRM_XE + help + MEI Support for Late Binding for Intel graphics card. + + Enables the ME FW interfaces for Late Binding feature, + allowing loading of firmware for the devices like Fan + Controller during by Intel Xe driver. diff --git a/drivers/misc/mei/late_bind/Makefile b/drivers/misc/mei/late_bind/Makefile new file mode 100644 index 000000000000..a0aeda5853f0 --- /dev/null +++ b/drivers/misc/mei/late_bind/Makefile @@ -0,0 +1,9 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# Copyright (c) 2025, Intel Corporation. All rights reserved. +# +# Makefile - Late Binding client driver for Intel MEI Bus Driver. + +subdir-ccflags-y += -I$(srctree)/drivers/misc/mei/ + +obj-$(CONFIG_INTEL_MEI_LATE_BIND) += mei_late_bind.o diff --git a/drivers/misc/mei/late_bind/mei_late_bind.c b/drivers/misc/mei/late_bind/mei_late_bind.c new file mode 100644 index 000000000000..cb985f32309e --- /dev/null +++ b/drivers/misc/mei/late_bind/mei_late_bind.c @@ -0,0 +1,264 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2025 Intel Corporation + */ +#include <drm/drm_connector.h> +#include <drm/intel/i915_component.h> +#include <drm/intel/late_bind_mei_interface.h> +#include <linux/component.h> +#include <linux/pci.h> +#include <linux/mei_cl_bus.h> +#include <linux/module.h> +#include <linux/overflow.h> +#include <linux/slab.h> +#include <linux/uuid.h> + +#include "mkhi.h" + +#define GFX_SRV_MKHI_LATE_BINDING_CMD 0x12 +#define GFX_SRV_MKHI_LATE_BINDING_RSP (GFX_SRV_MKHI_LATE_BINDING_CMD | 0x80) + +#define LATE_BIND_SEND_TIMEOUT_MSEC 3000 +#define LATE_BIND_RECV_TIMEOUT_MSEC 3000 + +/** + * struct csc_heci_late_bind_req - late binding request + * @header: @ref mkhi_msg_hdr + * @type: type of the late binding payload + * @flags: flags to be passed to the firmware + * @reserved: reserved field + * @payload_size: size of the payload data in bytes + * @payload: data to be sent to the firmware + */ +struct csc_heci_late_bind_req { + struct mkhi_msg_hdr header; + u32 type; + u32 flags; + u32 reserved[2]; + u32 payload_size; + u8 payload[] __counted_by(payload_size); +} __packed; + +/** + * struct csc_heci_late_bind_rsp - late binding response + * @header: @ref mkhi_msg_hdr + * @type: type of the late binding payload + * @reserved: reserved field + * @status: status of the late binding command execution by firmware + */ +struct csc_heci_late_bind_rsp { + struct mkhi_msg_hdr header; + u32 type; + u32 reserved[2]; + u32 status; +} __packed; + +static int mei_late_bind_check_response(const struct device *dev, const struct mkhi_msg_hdr *hdr) +{ + if (hdr->group_id != MKHI_GROUP_ID_GFX) { + dev_err(dev, "Mismatch group id: 0x%x instead of 0x%x\n", + hdr->group_id, MKHI_GROUP_ID_GFX); + return -EINVAL; + } + + if (hdr->command != GFX_SRV_MKHI_LATE_BINDING_RSP) { + dev_err(dev, "Mismatch command: 0x%x instead of 0x%x\n", + hdr->command, GFX_SRV_MKHI_LATE_BINDING_RSP); + return -EINVAL; + } + + return 0; +} + +/** + * mei_late_bind_push_config - Sends a config to the firmware. + * @dev: device struct corresponding to the mei device + * @type: payload type + * @flags: payload flags + * @payload: payload buffer + * @payload_size: payload buffer size + * + * Return: 0 success, negative errno value on transport failure, + * positive status returned by FW + */ +static int mei_late_bind_push_config(struct device *dev, u32 type, u32 flags, + const void *payload, size_t payload_size) +{ + struct mei_cl_device *cldev; + struct csc_heci_late_bind_req *req = NULL; + struct csc_heci_late_bind_rsp rsp; + size_t req_size; + int ret; + + if (!dev || !payload || !payload_size) + return -EINVAL; + + cldev = to_mei_cl_device(dev); + + ret = mei_cldev_enable(cldev); + if (ret < 0) { + dev_dbg(dev, "mei_cldev_enable failed. %d\n", ret); + return ret; + } + + req_size = struct_size(req, payload, payload_size); + if (req_size > mei_cldev_mtu(cldev)) { + dev_err(dev, "Payload is too big %zu\n", payload_size); + ret = -EMSGSIZE; + goto end; + } + + req = kmalloc(req_size, GFP_KERNEL); + if (!req) { + ret = -ENOMEM; + goto end; + } + + req->header.group_id = MKHI_GROUP_ID_GFX; + req->header.command = GFX_SRV_MKHI_LATE_BINDING_CMD; + req->type = type; + req->flags = flags; + req->reserved[0] = 0; + req->reserved[1] = 0; + req->payload_size = payload_size; + memcpy(req->payload, payload, payload_size); + + ret = mei_cldev_send_timeout(cldev, (void *)req, req_size, LATE_BIND_SEND_TIMEOUT_MSEC); + if (ret < 0) { + dev_err(dev, "mei_cldev_send failed. %d\n", ret); + goto end; + } + ret = mei_cldev_recv_timeout(cldev, (void *)&rsp, sizeof(rsp), LATE_BIND_RECV_TIMEOUT_MSEC); + if (ret < 0) { + dev_err(dev, "mei_cldev_recv failed. %d\n", ret); + goto end; + } + ret = mei_late_bind_check_response(dev, &rsp.header); + if (ret) { + dev_err(dev, "bad result response from the firmware: 0x%x\n", + *(uint32_t *)&rsp.header); + goto end; + } + ret = (int)rsp.status; + dev_dbg(dev, "%s status = %d\n", __func__, ret); + +end: + mei_cldev_disable(cldev); + kfree(req); + return ret; +} + +static const struct late_bind_component_ops mei_late_bind_ops = { + .owner = THIS_MODULE, + .push_config = mei_late_bind_push_config, +}; + +static int mei_component_master_bind(struct device *dev) +{ + return component_bind_all(dev, (void *)&mei_late_bind_ops); +} + +static void mei_component_master_unbind(struct device *dev) +{ + component_unbind_all(dev, (void *)&mei_late_bind_ops); +} + +static const struct component_master_ops mei_component_master_ops = { + .bind = mei_component_master_bind, + .unbind = mei_component_master_unbind, +}; + +/** + * mei_late_bind_component_match - compare function for matching mei late bind. + * + * The function checks if requested is Intel VGA device + * and the parent of requester and the grand parent of mei_if are the same + * device. + * + * @dev: master device + * @subcomponent: subcomponent to match (I915_COMPONENT_LATE_BIND) + * @data: compare data (mei late-bind bus device) + * + * Return: + * * 1 - if components match + * * 0 - otherwise + */ +static int mei_late_bind_component_match(struct device *dev, int subcomponent, + void *data) +{ + struct device *base = data; + struct pci_dev *pdev; + + if (!dev) + return 0; + + if (!dev_is_pci(dev)) + return 0; + + pdev = to_pci_dev(dev); + + if (pdev->vendor != PCI_VENDOR_ID_INTEL) + return 0; + + if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) || + pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8)) + return 0; + + if (subcomponent != I915_COMPONENT_LATE_BIND) + return 0; + + base = base->parent; + if (!base) /* mei device */ + return 0; + + base = base->parent; /* pci device */ + + return !!base && dev == base; +} + +static int mei_late_bind_probe(struct mei_cl_device *cldev, + const struct mei_cl_device_id *id) +{ + struct component_match *master_match = NULL; + int ret; + + component_match_add_typed(&cldev->dev, &master_match, + mei_late_bind_component_match, &cldev->dev); + if (IS_ERR_OR_NULL(master_match)) + return -ENOMEM; + + ret = component_master_add_with_match(&cldev->dev, + &mei_component_master_ops, + master_match); + if (ret < 0) + dev_err(&cldev->dev, "Master comp add failed %d\n", ret); + + return ret; +} + +static void mei_late_bind_remove(struct mei_cl_device *cldev) +{ + component_master_del(&cldev->dev, &mei_component_master_ops); +} + +#define MEI_GUID_MKHI UUID_LE(0xe2c2afa2, 0x3817, 0x4d19, \ + 0x9d, 0x95, 0x6, 0xb1, 0x6b, 0x58, 0x8a, 0x5d) + +static struct mei_cl_device_id mei_late_bind_tbl[] = { + { .uuid = MEI_GUID_MKHI, .version = MEI_CL_VERSION_ANY }, + { } +}; +MODULE_DEVICE_TABLE(mei, mei_late_bind_tbl); + +static struct mei_cl_driver mei_late_bind_driver = { + .id_table = mei_late_bind_tbl, + .name = KBUILD_MODNAME, + .probe = mei_late_bind_probe, + .remove = mei_late_bind_remove, +}; + +module_mei_cl_driver(mei_late_bind_driver); + +MODULE_AUTHOR("Intel Corporation"); +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("MEI Late Binding"); diff --git a/include/drm/intel/i915_component.h b/include/drm/intel/i915_component.h index 4ea3b17aa143..4945044d41e6 100644 --- a/include/drm/intel/i915_component.h +++ b/include/drm/intel/i915_component.h @@ -31,6 +31,7 @@ enum i915_component_type { I915_COMPONENT_HDCP, I915_COMPONENT_PXP, I915_COMPONENT_GSC_PROXY, + I915_COMPONENT_LATE_BIND, }; /* MAX_PORT is the number of port diff --git a/include/drm/intel/late_bind_mei_interface.h b/include/drm/intel/late_bind_mei_interface.h new file mode 100644 index 000000000000..2c53657ce91b --- /dev/null +++ b/include/drm/intel/late_bind_mei_interface.h @@ -0,0 +1,50 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright (c) 2025 Intel Corporation + */ + +#ifndef _LATE_BIND_MEI_INTERFACE_H_ +#define _LATE_BIND_MEI_INTERFACE_H_ + +#include <linux/types.h> + +struct device; +struct module; + +/** + * Late Binding flags + * Persistent across warm reset + */ +#define CSC_LATE_BINDING_FLAGS_IS_PERSISTENT BIT(0) + +/** + * xe_late_bind_fw_type - enum to determine late binding fw type + */ +enum late_bind_type { + CSC_LATE_BINDING_TYPE_FAN_CONTROL = 1, +}; + +/** + * struct late_bind_component_ops - ops for Late Binding services. + * @owner: Module providing the ops + * @push_config: Sends a config to FW. + */ +struct late_bind_component_ops { + struct module *owner; + + /** + * @push_config: Sends a config to FW. + * @dev: device struct corresponding to the mei device + * @type: payload type + * @flags: payload flags + * @payload: payload buffer + * @payload_size: payload buffer size + * + * Return: 0 success, negative errno value on transport failure, + * positive status returned by FW + */ + int (*push_config)(struct device *dev, u32 type, u32 flags, + const void *payload, size_t payload_size); +}; + +#endif /* _LATE_BIND_MEI_INTERFACE_H_ */ -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH v3 02/10] mei: late_bind: add late binding component driver 2025-06-18 18:59 ` [PATCH v3 02/10] mei: late_bind: add late binding component driver Badal Nilawar @ 2025-06-19 7:32 ` Gupta, Anshuman 2025-06-19 8:11 ` Jani Nikula 2025-06-19 9:06 ` Nilawar, Badal 2025-06-24 13:37 ` Dan Carpenter 1 sibling, 2 replies; 6+ messages in thread From: Gupta, Anshuman @ 2025-06-19 7:32 UTC (permalink / raw) To: Vivi, Rodrigo, Nilawar, Badal, intel-xe@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Jani Nikula, Usyskin, Alexander Cc: gregkh@linuxfoundation.org, Ceraolo Spurio, Daniele, jgg@nvidia.com > -----Original Message----- > From: Nilawar, Badal <badal.nilawar@intel.com> > Sent: Thursday, June 19, 2025 12:30 AM > To: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- > kernel@vger.kernel.org > Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Vivi, Rodrigo > <rodrigo.vivi@intel.com>; Usyskin, Alexander <alexander.usyskin@intel.com>; > gregkh@linuxfoundation.org; Ceraolo Spurio, Daniele > <daniele.ceraolospurio@intel.com>; jgg@nvidia.com > Subject: [PATCH v3 02/10] mei: late_bind: add late binding component driver > > From: Alexander Usyskin <alexander.usyskin@intel.com> > > Add late binding component driver. > It allows pushing the late binding configuration from, for example, the Xe graphics > driver to the Intel discrete graphics card's CSE device. > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com> > --- > v2: > - Use generic naming (Jani) This patch still wrong naming I915_COMPONENT_LATE_BIND. LATE_BIND will never be supported by i915, it is a wrong prefix. @Nikula, Jani @Vivi, Rodrigo is it ok use the i915 naming prefix here ? We can use INTEL_COMPONENT_LATE_BIND here ? This header include/drm/intel/i915_component.h is used by both XE and i915. May be a separate series later requires refactoring this header. > - Drop xe_late_bind_component struct to move to xe code (Daniele/Sasha) > v3: > - Updated kconfig description > - Move CSC late binding specific flags/defines to late_bind_mei_interface.h > (Daniele) > v4: > - Add match for PCI_CLASS_DISPLAY_OTHER to support headless cards > (Anshuman) > --- > drivers/misc/mei/Kconfig | 1 + > drivers/misc/mei/Makefile | 1 + > drivers/misc/mei/late_bind/Kconfig | 13 + > drivers/misc/mei/late_bind/Makefile | 9 + > drivers/misc/mei/late_bind/mei_late_bind.c | 264 ++++++++++++++++++++ > include/drm/intel/i915_component.h | 1 + > include/drm/intel/late_bind_mei_interface.h | 50 ++++ > 7 files changed, 339 insertions(+) > create mode 100644 drivers/misc/mei/late_bind/Kconfig > create mode 100644 drivers/misc/mei/late_bind/Makefile > create mode 100644 drivers/misc/mei/late_bind/mei_late_bind.c > create mode 100644 include/drm/intel/late_bind_mei_interface.h > > diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig index > 7575fee96cc6..771becc68095 100644 > --- a/drivers/misc/mei/Kconfig > +++ b/drivers/misc/mei/Kconfig > @@ -84,5 +84,6 @@ config INTEL_MEI_VSC > source "drivers/misc/mei/hdcp/Kconfig" > source "drivers/misc/mei/pxp/Kconfig" > source "drivers/misc/mei/gsc_proxy/Kconfig" > +source "drivers/misc/mei/late_bind/Kconfig" > > endif > diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile index > 6f9fdbf1a495..84bfde888d81 100644 > --- a/drivers/misc/mei/Makefile > +++ b/drivers/misc/mei/Makefile > @@ -31,6 +31,7 @@ CFLAGS_mei-trace.o = -I$(src) > obj-$(CONFIG_INTEL_MEI_HDCP) += hdcp/ > obj-$(CONFIG_INTEL_MEI_PXP) += pxp/ > obj-$(CONFIG_INTEL_MEI_GSC_PROXY) += gsc_proxy/ > +obj-$(CONFIG_INTEL_MEI_LATE_BIND) += late_bind/ > > obj-$(CONFIG_INTEL_MEI_VSC_HW) += mei-vsc-hw.o mei-vsc-hw-y := vsc-tp.o > diff --git a/drivers/misc/mei/late_bind/Kconfig > b/drivers/misc/mei/late_bind/Kconfig > new file mode 100644 > index 000000000000..65c7180c5678 > --- /dev/null > +++ b/drivers/misc/mei/late_bind/Kconfig > @@ -0,0 +1,13 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2025, Intel Corporation. All rights reserved. > +# > +config INTEL_MEI_LATE_BIND > + tristate "Intel late binding support on ME Interface" > + select INTEL_MEI_ME > + depends on DRM_XE > + help > + MEI Support for Late Binding for Intel graphics card. > + > + Enables the ME FW interfaces for Late Binding feature, > + allowing loading of firmware for the devices like Fan > + Controller during by Intel Xe driver. > diff --git a/drivers/misc/mei/late_bind/Makefile > b/drivers/misc/mei/late_bind/Makefile > new file mode 100644 > index 000000000000..a0aeda5853f0 > --- /dev/null > +++ b/drivers/misc/mei/late_bind/Makefile > @@ -0,0 +1,9 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# > +# Copyright (c) 2025, Intel Corporation. All rights reserved. > +# > +# Makefile - Late Binding client driver for Intel MEI Bus Driver. > + > +subdir-ccflags-y += -I$(srctree)/drivers/misc/mei/ > + > +obj-$(CONFIG_INTEL_MEI_LATE_BIND) += mei_late_bind.o > diff --git a/drivers/misc/mei/late_bind/mei_late_bind.c > b/drivers/misc/mei/late_bind/mei_late_bind.c > new file mode 100644 > index 000000000000..cb985f32309e > --- /dev/null > +++ b/drivers/misc/mei/late_bind/mei_late_bind.c > @@ -0,0 +1,264 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2025 Intel Corporation */ #include > +<drm/drm_connector.h> #include <drm/intel/i915_component.h> #include > +<drm/intel/late_bind_mei_interface.h> > +#include <linux/component.h> > +#include <linux/pci.h> > +#include <linux/mei_cl_bus.h> > +#include <linux/module.h> > +#include <linux/overflow.h> > +#include <linux/slab.h> > +#include <linux/uuid.h> > + > +#include "mkhi.h" > + > +#define GFX_SRV_MKHI_LATE_BINDING_CMD 0x12 #define > +GFX_SRV_MKHI_LATE_BINDING_RSP (GFX_SRV_MKHI_LATE_BINDING_CMD | > 0x80) > + > +#define LATE_BIND_SEND_TIMEOUT_MSEC 3000 #define > +LATE_BIND_RECV_TIMEOUT_MSEC 3000 I commented earlier in V2 series as well, is this timeout specific only to LATE BINDING ? If this is generic timeout for mei_cldev_{send,recv}_timeout(), then this marco should be part of standard MEI headers not late binding. Other consumers of mei_cldev_{send,recv}_timeout() send the timeout input by component-ops callback . @Shahsa could you please explained that. > + > +/** > + * struct csc_heci_late_bind_req - late binding request > + * @header: @ref mkhi_msg_hdr > + * @type: type of the late binding payload > + * @flags: flags to be passed to the firmware > + * @reserved: reserved field > + * @payload_size: size of the payload data in bytes > + * @payload: data to be sent to the firmware */ struct > +csc_heci_late_bind_req { > + struct mkhi_msg_hdr header; > + u32 type; > + u32 flags; > + u32 reserved[2]; > + u32 payload_size; > + u8 payload[] __counted_by(payload_size); } __packed; > + > +/** > + * struct csc_heci_late_bind_rsp - late binding response > + * @header: @ref mkhi_msg_hdr > + * @type: type of the late binding payload > + * @reserved: reserved field > + * @status: status of the late binding command execution by firmware > +*/ struct csc_heci_late_bind_rsp { > + struct mkhi_msg_hdr header; > + u32 type; > + u32 reserved[2]; > + u32 status; > +} __packed; > + > +static int mei_late_bind_check_response(const struct device *dev, const > +struct mkhi_msg_hdr *hdr) { > + if (hdr->group_id != MKHI_GROUP_ID_GFX) { > + dev_err(dev, "Mismatch group id: 0x%x instead of 0x%x\n", > + hdr->group_id, MKHI_GROUP_ID_GFX); > + return -EINVAL; > + } > + > + if (hdr->command != GFX_SRV_MKHI_LATE_BINDING_RSP) { > + dev_err(dev, "Mismatch command: 0x%x instead of 0x%x\n", > + hdr->command, GFX_SRV_MKHI_LATE_BINDING_RSP); > + return -EINVAL; > + } Why are we not checking mkhi_msg_hdr hdr->result here ? > + > + return 0; > +} > + > +/** > + * mei_late_bind_push_config - Sends a config to the firmware. > + * @dev: device struct corresponding to the mei device > + * @type: payload type > + * @flags: payload flags > + * @payload: payload buffer > + * @payload_size: payload buffer size > + * > + * Return: 0 success, negative errno value on transport failure, > + * positive status returned by FW > + */ > +static int mei_late_bind_push_config(struct device *dev, u32 type, u32 flags, > + const void *payload, size_t payload_size) { > + struct mei_cl_device *cldev; > + struct csc_heci_late_bind_req *req = NULL; > + struct csc_heci_late_bind_rsp rsp; > + size_t req_size; > + int ret; > + > + if (!dev || !payload || !payload_size) > + return -EINVAL; > + > + cldev = to_mei_cl_device(dev); > + > + ret = mei_cldev_enable(cldev); > + if (ret < 0) { > + dev_dbg(dev, "mei_cldev_enable failed. %d\n", ret); > + return ret; > + } > + > + req_size = struct_size(req, payload, payload_size); > + if (req_size > mei_cldev_mtu(cldev)) { > + dev_err(dev, "Payload is too big %zu\n", payload_size); > + ret = -EMSGSIZE; > + goto end; > + } > + > + req = kmalloc(req_size, GFP_KERNEL); > + if (!req) { > + ret = -ENOMEM; > + goto end; > + } Use Kzalloc here, to make sure reserved filed of header is zeroed. > + > + req->header.group_id = MKHI_GROUP_ID_GFX; > + req->header.command = GFX_SRV_MKHI_LATE_BINDING_CMD; > + req->type = type; > + req->flags = flags; > + req->reserved[0] = 0; > + req->reserved[1] = 0; > + req->payload_size = payload_size; > + memcpy(req->payload, payload, payload_size); > + > + ret = mei_cldev_send_timeout(cldev, (void *)req, req_size, > LATE_BIND_SEND_TIMEOUT_MSEC); > + if (ret < 0) { > + dev_err(dev, "mei_cldev_send failed. %d\n", ret); > + goto end; > + } > + ret = mei_cldev_recv_timeout(cldev, (void *)&rsp, sizeof(rsp), > LATE_BIND_RECV_TIMEOUT_MSEC); > + if (ret < 0) { > + dev_err(dev, "mei_cldev_recv failed. %d\n", ret); > + goto end; > + } > + ret = mei_late_bind_check_response(dev, &rsp.header); > + if (ret) { > + dev_err(dev, "bad result response from the firmware: 0x%x\n", > + *(uint32_t *)&rsp.header); > + goto end; > + } > + ret = (int)rsp.status; > + dev_dbg(dev, "%s status = %d\n", __func__, ret); AFAIU It would be useful to add the status enum in late_bind_mei_interface.h. > + > +end: > + mei_cldev_disable(cldev); > + kfree(req); > + return ret; > +} > + > +static const struct late_bind_component_ops mei_late_bind_ops = { > + .owner = THIS_MODULE, > + .push_config = mei_late_bind_push_config, }; > + > +static int mei_component_master_bind(struct device *dev) { > + return component_bind_all(dev, (void *)&mei_late_bind_ops); } > + > +static void mei_component_master_unbind(struct device *dev) { > + component_unbind_all(dev, (void *)&mei_late_bind_ops); } > + > +static const struct component_master_ops mei_component_master_ops = { > + .bind = mei_component_master_bind, > + .unbind = mei_component_master_unbind, }; > + > +/** > + * mei_late_bind_component_match - compare function for matching mei late > bind. > + * > + * The function checks if requested is Intel VGA device Please modify the Kenel Doc comment here, as per the function. > + * and the parent of requester and the grand parent of mei_if are the same We are matching against the requester not parent of requester. Modify the Kernel Doc comment properly. > + * device. > + * > + * @dev: master device > + * @subcomponent: subcomponent to match (I915_COMPONENT_LATE_BIND) > + * @data: compare data (mei late-bind bus device) AFAIK It is mei client device not mei bus device. > + * > + * Return: > + * * 1 - if components match > + * * 0 - otherwise > + */ > +static int mei_late_bind_component_match(struct device *dev, int > subcomponent, > + void *data) > +{ > + struct device *base = data; > + struct pci_dev *pdev; > + > + if (!dev) > + return 0; > + > + if (!dev_is_pci(dev)) > + return 0; > + > + pdev = to_pci_dev(dev); > + > + if (pdev->vendor != PCI_VENDOR_ID_INTEL) > + return 0; > + > + if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) || > + pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8)) > + return 0; This condition should be like below, if I am not missing anything. if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) && pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8)) Thanks, Anshuman. > + > + if (subcomponent != I915_COMPONENT_LATE_BIND) > + return 0; > + > + base = base->parent; > + if (!base) /* mei device */ > + return 0; > + > + base = base->parent; /* pci device */ > + > + return !!base && dev == base; > +} > + > +static int mei_late_bind_probe(struct mei_cl_device *cldev, > + const struct mei_cl_device_id *id) { > + struct component_match *master_match = NULL; > + int ret; > + > + component_match_add_typed(&cldev->dev, &master_match, > + mei_late_bind_component_match, &cldev- > >dev); > + if (IS_ERR_OR_NULL(master_match)) > + return -ENOMEM; > + > + ret = component_master_add_with_match(&cldev->dev, > + &mei_component_master_ops, > + master_match); > + if (ret < 0) > + dev_err(&cldev->dev, "Master comp add failed %d\n", ret); > + > + return ret; > +} > + > +static void mei_late_bind_remove(struct mei_cl_device *cldev) { > + component_master_del(&cldev->dev, &mei_component_master_ops); } > + > +#define MEI_GUID_MKHI UUID_LE(0xe2c2afa2, 0x3817, 0x4d19, \ > + 0x9d, 0x95, 0x6, 0xb1, 0x6b, 0x58, 0x8a, 0x5d) > + > +static struct mei_cl_device_id mei_late_bind_tbl[] = { > + { .uuid = MEI_GUID_MKHI, .version = MEI_CL_VERSION_ANY }, > + { } > +}; > +MODULE_DEVICE_TABLE(mei, mei_late_bind_tbl); > + > +static struct mei_cl_driver mei_late_bind_driver = { > + .id_table = mei_late_bind_tbl, > + .name = KBUILD_MODNAME, > + .probe = mei_late_bind_probe, > + .remove = mei_late_bind_remove, > +}; > + > +module_mei_cl_driver(mei_late_bind_driver); > + > +MODULE_AUTHOR("Intel Corporation"); > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("MEI Late Binding"); > diff --git a/include/drm/intel/i915_component.h > b/include/drm/intel/i915_component.h > index 4ea3b17aa143..4945044d41e6 100644 > --- a/include/drm/intel/i915_component.h > +++ b/include/drm/intel/i915_component.h > @@ -31,6 +31,7 @@ enum i915_component_type { > I915_COMPONENT_HDCP, > I915_COMPONENT_PXP, > I915_COMPONENT_GSC_PROXY, > + I915_COMPONENT_LATE_BIND, > }; > > /* MAX_PORT is the number of port > diff --git a/include/drm/intel/late_bind_mei_interface.h > b/include/drm/intel/late_bind_mei_interface.h > new file mode 100644 > index 000000000000..2c53657ce91b > --- /dev/null > +++ b/include/drm/intel/late_bind_mei_interface.h > @@ -0,0 +1,50 @@ > +/* SPDX-License-Identifier: MIT */ > +/* > + * Copyright (c) 2025 Intel Corporation */ > + > +#ifndef _LATE_BIND_MEI_INTERFACE_H_ > +#define _LATE_BIND_MEI_INTERFACE_H_ > + > +#include <linux/types.h> > + > +struct device; > +struct module; > + > +/** > + * Late Binding flags > + * Persistent across warm reset > + */ > +#define CSC_LATE_BINDING_FLAGS_IS_PERSISTENT BIT(0) > + > +/** > + * xe_late_bind_fw_type - enum to determine late binding fw type */ > +enum late_bind_type { > + CSC_LATE_BINDING_TYPE_FAN_CONTROL = 1, }; > + > +/** > + * struct late_bind_component_ops - ops for Late Binding services. > + * @owner: Module providing the ops > + * @push_config: Sends a config to FW. > + */ > +struct late_bind_component_ops { > + struct module *owner; > + > + /** > + * @push_config: Sends a config to FW. > + * @dev: device struct corresponding to the mei device > + * @type: payload type > + * @flags: payload flags > + * @payload: payload buffer > + * @payload_size: payload buffer size > + * > + * Return: 0 success, negative errno value on transport failure, > + * positive status returned by FW > + */ > + int (*push_config)(struct device *dev, u32 type, u32 flags, > + const void *payload, size_t payload_size); }; > + > +#endif /* _LATE_BIND_MEI_INTERFACE_H_ */ > -- > 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v3 02/10] mei: late_bind: add late binding component driver 2025-06-19 7:32 ` Gupta, Anshuman @ 2025-06-19 8:11 ` Jani Nikula 2025-06-19 9:06 ` Nilawar, Badal 1 sibling, 0 replies; 6+ messages in thread From: Jani Nikula @ 2025-06-19 8:11 UTC (permalink / raw) To: Gupta, Anshuman, Vivi, Rodrigo, Nilawar, Badal, intel-xe@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Usyskin, Alexander Cc: gregkh@linuxfoundation.org, Ceraolo Spurio, Daniele, jgg@nvidia.com On Thu, 19 Jun 2025, "Gupta, Anshuman" <anshuman.gupta@intel.com> wrote: >> -----Original Message----- >> From: Nilawar, Badal <badal.nilawar@intel.com> >> Sent: Thursday, June 19, 2025 12:30 AM >> To: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- >> kernel@vger.kernel.org >> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Vivi, Rodrigo >> <rodrigo.vivi@intel.com>; Usyskin, Alexander <alexander.usyskin@intel.com>; >> gregkh@linuxfoundation.org; Ceraolo Spurio, Daniele >> <daniele.ceraolospurio@intel.com>; jgg@nvidia.com >> Subject: [PATCH v3 02/10] mei: late_bind: add late binding component driver >> >> From: Alexander Usyskin <alexander.usyskin@intel.com> >> >> Add late binding component driver. >> It allows pushing the late binding configuration from, for example, the Xe graphics >> driver to the Intel discrete graphics card's CSE device. >> >> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com> >> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com> >> --- >> v2: >> - Use generic naming (Jani) > This patch still wrong naming I915_COMPONENT_LATE_BIND. > LATE_BIND will never be supported by i915, it is a wrong prefix. > @Nikula, Jani @Vivi, Rodrigo is it ok use the i915 naming prefix here ? > We can use INTEL_COMPONENT_LATE_BIND here ? > > This header include/drm/intel/i915_component.h is used by both XE and i915. > May be a separate series later requires refactoring this header. Yeah the goal is that everything under include/drm/intel would be independent of xe and i915, both in naming and implementation. BR, Jani. > > >> - Drop xe_late_bind_component struct to move to xe code (Daniele/Sasha) >> v3: >> - Updated kconfig description >> - Move CSC late binding specific flags/defines to late_bind_mei_interface.h >> (Daniele) >> v4: >> - Add match for PCI_CLASS_DISPLAY_OTHER to support headless cards >> (Anshuman) >> --- >> drivers/misc/mei/Kconfig | 1 + >> drivers/misc/mei/Makefile | 1 + >> drivers/misc/mei/late_bind/Kconfig | 13 + >> drivers/misc/mei/late_bind/Makefile | 9 + >> drivers/misc/mei/late_bind/mei_late_bind.c | 264 ++++++++++++++++++++ >> include/drm/intel/i915_component.h | 1 + >> include/drm/intel/late_bind_mei_interface.h | 50 ++++ >> 7 files changed, 339 insertions(+) >> create mode 100644 drivers/misc/mei/late_bind/Kconfig >> create mode 100644 drivers/misc/mei/late_bind/Makefile >> create mode 100644 drivers/misc/mei/late_bind/mei_late_bind.c >> create mode 100644 include/drm/intel/late_bind_mei_interface.h >> >> diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig index >> 7575fee96cc6..771becc68095 100644 >> --- a/drivers/misc/mei/Kconfig >> +++ b/drivers/misc/mei/Kconfig >> @@ -84,5 +84,6 @@ config INTEL_MEI_VSC >> source "drivers/misc/mei/hdcp/Kconfig" >> source "drivers/misc/mei/pxp/Kconfig" >> source "drivers/misc/mei/gsc_proxy/Kconfig" >> +source "drivers/misc/mei/late_bind/Kconfig" >> >> endif >> diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile index >> 6f9fdbf1a495..84bfde888d81 100644 >> --- a/drivers/misc/mei/Makefile >> +++ b/drivers/misc/mei/Makefile >> @@ -31,6 +31,7 @@ CFLAGS_mei-trace.o = -I$(src) >> obj-$(CONFIG_INTEL_MEI_HDCP) += hdcp/ >> obj-$(CONFIG_INTEL_MEI_PXP) += pxp/ >> obj-$(CONFIG_INTEL_MEI_GSC_PROXY) += gsc_proxy/ >> +obj-$(CONFIG_INTEL_MEI_LATE_BIND) += late_bind/ >> >> obj-$(CONFIG_INTEL_MEI_VSC_HW) += mei-vsc-hw.o mei-vsc-hw-y := vsc-tp.o >> diff --git a/drivers/misc/mei/late_bind/Kconfig >> b/drivers/misc/mei/late_bind/Kconfig >> new file mode 100644 >> index 000000000000..65c7180c5678 >> --- /dev/null >> +++ b/drivers/misc/mei/late_bind/Kconfig >> @@ -0,0 +1,13 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +# Copyright (c) 2025, Intel Corporation. All rights reserved. >> +# >> +config INTEL_MEI_LATE_BIND >> + tristate "Intel late binding support on ME Interface" >> + select INTEL_MEI_ME >> + depends on DRM_XE >> + help >> + MEI Support for Late Binding for Intel graphics card. >> + >> + Enables the ME FW interfaces for Late Binding feature, >> + allowing loading of firmware for the devices like Fan >> + Controller during by Intel Xe driver. >> diff --git a/drivers/misc/mei/late_bind/Makefile >> b/drivers/misc/mei/late_bind/Makefile >> new file mode 100644 >> index 000000000000..a0aeda5853f0 >> --- /dev/null >> +++ b/drivers/misc/mei/late_bind/Makefile >> @@ -0,0 +1,9 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +# >> +# Copyright (c) 2025, Intel Corporation. All rights reserved. >> +# >> +# Makefile - Late Binding client driver for Intel MEI Bus Driver. >> + >> +subdir-ccflags-y += -I$(srctree)/drivers/misc/mei/ >> + >> +obj-$(CONFIG_INTEL_MEI_LATE_BIND) += mei_late_bind.o >> diff --git a/drivers/misc/mei/late_bind/mei_late_bind.c >> b/drivers/misc/mei/late_bind/mei_late_bind.c >> new file mode 100644 >> index 000000000000..cb985f32309e >> --- /dev/null >> +++ b/drivers/misc/mei/late_bind/mei_late_bind.c >> @@ -0,0 +1,264 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2025 Intel Corporation */ #include >> +<drm/drm_connector.h> #include <drm/intel/i915_component.h> #include >> +<drm/intel/late_bind_mei_interface.h> >> +#include <linux/component.h> >> +#include <linux/pci.h> >> +#include <linux/mei_cl_bus.h> >> +#include <linux/module.h> >> +#include <linux/overflow.h> >> +#include <linux/slab.h> >> +#include <linux/uuid.h> >> + >> +#include "mkhi.h" >> + >> +#define GFX_SRV_MKHI_LATE_BINDING_CMD 0x12 #define >> +GFX_SRV_MKHI_LATE_BINDING_RSP (GFX_SRV_MKHI_LATE_BINDING_CMD | >> 0x80) >> + >> +#define LATE_BIND_SEND_TIMEOUT_MSEC 3000 #define >> +LATE_BIND_RECV_TIMEOUT_MSEC 3000 > I commented earlier in V2 series as well, is this timeout specific only to LATE BINDING ? > If this is generic timeout for mei_cldev_{send,recv}_timeout(), > then this marco should be part of standard MEI headers not late binding. > Other consumers of mei_cldev_{send,recv}_timeout() send the timeout input by component-ops callback . > > @Shahsa could you please explained that. >> + >> +/** >> + * struct csc_heci_late_bind_req - late binding request >> + * @header: @ref mkhi_msg_hdr >> + * @type: type of the late binding payload >> + * @flags: flags to be passed to the firmware >> + * @reserved: reserved field >> + * @payload_size: size of the payload data in bytes >> + * @payload: data to be sent to the firmware */ struct >> +csc_heci_late_bind_req { >> + struct mkhi_msg_hdr header; >> + u32 type; >> + u32 flags; >> + u32 reserved[2]; >> + u32 payload_size; >> + u8 payload[] __counted_by(payload_size); } __packed; >> + >> +/** >> + * struct csc_heci_late_bind_rsp - late binding response >> + * @header: @ref mkhi_msg_hdr >> + * @type: type of the late binding payload >> + * @reserved: reserved field >> + * @status: status of the late binding command execution by firmware >> +*/ struct csc_heci_late_bind_rsp { >> + struct mkhi_msg_hdr header; >> + u32 type; >> + u32 reserved[2]; >> + u32 status; >> +} __packed; >> + >> +static int mei_late_bind_check_response(const struct device *dev, const >> +struct mkhi_msg_hdr *hdr) { >> + if (hdr->group_id != MKHI_GROUP_ID_GFX) { >> + dev_err(dev, "Mismatch group id: 0x%x instead of 0x%x\n", >> + hdr->group_id, MKHI_GROUP_ID_GFX); >> + return -EINVAL; >> + } >> + >> + if (hdr->command != GFX_SRV_MKHI_LATE_BINDING_RSP) { >> + dev_err(dev, "Mismatch command: 0x%x instead of 0x%x\n", >> + hdr->command, GFX_SRV_MKHI_LATE_BINDING_RSP); >> + return -EINVAL; >> + } > Why are we not checking mkhi_msg_hdr hdr->result here ? >> + >> + return 0; >> +} >> + >> +/** >> + * mei_late_bind_push_config - Sends a config to the firmware. >> + * @dev: device struct corresponding to the mei device >> + * @type: payload type >> + * @flags: payload flags >> + * @payload: payload buffer >> + * @payload_size: payload buffer size >> + * >> + * Return: 0 success, negative errno value on transport failure, >> + * positive status returned by FW >> + */ >> +static int mei_late_bind_push_config(struct device *dev, u32 type, u32 flags, >> + const void *payload, size_t payload_size) { >> + struct mei_cl_device *cldev; >> + struct csc_heci_late_bind_req *req = NULL; >> + struct csc_heci_late_bind_rsp rsp; >> + size_t req_size; >> + int ret; >> + >> + if (!dev || !payload || !payload_size) >> + return -EINVAL; >> + >> + cldev = to_mei_cl_device(dev); >> + >> + ret = mei_cldev_enable(cldev); >> + if (ret < 0) { >> + dev_dbg(dev, "mei_cldev_enable failed. %d\n", ret); >> + return ret; >> + } >> + >> + req_size = struct_size(req, payload, payload_size); >> + if (req_size > mei_cldev_mtu(cldev)) { >> + dev_err(dev, "Payload is too big %zu\n", payload_size); >> + ret = -EMSGSIZE; >> + goto end; >> + } >> + >> + req = kmalloc(req_size, GFP_KERNEL); >> + if (!req) { >> + ret = -ENOMEM; >> + goto end; >> + } > Use Kzalloc here, to make sure reserved filed of header is zeroed. >> + >> + req->header.group_id = MKHI_GROUP_ID_GFX; >> + req->header.command = GFX_SRV_MKHI_LATE_BINDING_CMD; >> + req->type = type; >> + req->flags = flags; >> + req->reserved[0] = 0; >> + req->reserved[1] = 0; >> + req->payload_size = payload_size; >> + memcpy(req->payload, payload, payload_size); >> + >> + ret = mei_cldev_send_timeout(cldev, (void *)req, req_size, >> LATE_BIND_SEND_TIMEOUT_MSEC); >> + if (ret < 0) { >> + dev_err(dev, "mei_cldev_send failed. %d\n", ret); >> + goto end; >> + } >> + ret = mei_cldev_recv_timeout(cldev, (void *)&rsp, sizeof(rsp), >> LATE_BIND_RECV_TIMEOUT_MSEC); >> + if (ret < 0) { >> + dev_err(dev, "mei_cldev_recv failed. %d\n", ret); >> + goto end; >> + } >> + ret = mei_late_bind_check_response(dev, &rsp.header); >> + if (ret) { >> + dev_err(dev, "bad result response from the firmware: 0x%x\n", >> + *(uint32_t *)&rsp.header); > >> + goto end; >> + } >> + ret = (int)rsp.status; >> + dev_dbg(dev, "%s status = %d\n", __func__, ret); > AFAIU It would be useful to add the status enum in late_bind_mei_interface.h. >> + >> +end: >> + mei_cldev_disable(cldev); >> + kfree(req); >> + return ret; >> +} >> + >> +static const struct late_bind_component_ops mei_late_bind_ops = { >> + .owner = THIS_MODULE, >> + .push_config = mei_late_bind_push_config, }; >> + >> +static int mei_component_master_bind(struct device *dev) { >> + return component_bind_all(dev, (void *)&mei_late_bind_ops); } >> + >> +static void mei_component_master_unbind(struct device *dev) { >> + component_unbind_all(dev, (void *)&mei_late_bind_ops); } >> + >> +static const struct component_master_ops mei_component_master_ops = { >> + .bind = mei_component_master_bind, >> + .unbind = mei_component_master_unbind, }; >> + >> +/** >> + * mei_late_bind_component_match - compare function for matching mei late >> bind. >> + * >> + * The function checks if requested is Intel VGA device > Please modify the Kenel Doc comment here, as per the function. >> + * and the parent of requester and the grand parent of mei_if are the same > We are matching against the requester not parent of requester. > Modify the Kernel Doc comment properly. >> + * device. >> + * >> + * @dev: master device >> + * @subcomponent: subcomponent to match (I915_COMPONENT_LATE_BIND) >> + * @data: compare data (mei late-bind bus device) > AFAIK It is mei client device not mei bus device. >> + * >> + * Return: >> + * * 1 - if components match >> + * * 0 - otherwise >> + */ >> +static int mei_late_bind_component_match(struct device *dev, int >> subcomponent, >> + void *data) >> +{ >> + struct device *base = data; >> + struct pci_dev *pdev; >> + >> + if (!dev) >> + return 0; >> + >> + if (!dev_is_pci(dev)) >> + return 0; >> + >> + pdev = to_pci_dev(dev); >> + >> + if (pdev->vendor != PCI_VENDOR_ID_INTEL) >> + return 0; >> + >> + if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) || >> + pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8)) >> + return 0; > This condition should be like below, if I am not missing anything. > if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) && > pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8)) > > Thanks, > Anshuman. >> + >> + if (subcomponent != I915_COMPONENT_LATE_BIND) >> + return 0; >> + >> + base = base->parent; >> + if (!base) /* mei device */ >> + return 0; >> + >> + base = base->parent; /* pci device */ >> + >> + return !!base && dev == base; >> +} >> + >> +static int mei_late_bind_probe(struct mei_cl_device *cldev, >> + const struct mei_cl_device_id *id) { >> + struct component_match *master_match = NULL; >> + int ret; >> + >> + component_match_add_typed(&cldev->dev, &master_match, >> + mei_late_bind_component_match, &cldev- >> >dev); >> + if (IS_ERR_OR_NULL(master_match)) >> + return -ENOMEM; >> + >> + ret = component_master_add_with_match(&cldev->dev, >> + &mei_component_master_ops, >> + master_match); >> + if (ret < 0) >> + dev_err(&cldev->dev, "Master comp add failed %d\n", ret); >> + >> + return ret; >> +} >> + >> +static void mei_late_bind_remove(struct mei_cl_device *cldev) { >> + component_master_del(&cldev->dev, &mei_component_master_ops); } >> + >> +#define MEI_GUID_MKHI UUID_LE(0xe2c2afa2, 0x3817, 0x4d19, \ >> + 0x9d, 0x95, 0x6, 0xb1, 0x6b, 0x58, 0x8a, 0x5d) >> + >> +static struct mei_cl_device_id mei_late_bind_tbl[] = { >> + { .uuid = MEI_GUID_MKHI, .version = MEI_CL_VERSION_ANY }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(mei, mei_late_bind_tbl); >> + >> +static struct mei_cl_driver mei_late_bind_driver = { >> + .id_table = mei_late_bind_tbl, >> + .name = KBUILD_MODNAME, >> + .probe = mei_late_bind_probe, >> + .remove = mei_late_bind_remove, >> +}; >> + >> +module_mei_cl_driver(mei_late_bind_driver); >> + >> +MODULE_AUTHOR("Intel Corporation"); >> +MODULE_LICENSE("GPL"); >> +MODULE_DESCRIPTION("MEI Late Binding"); >> diff --git a/include/drm/intel/i915_component.h >> b/include/drm/intel/i915_component.h >> index 4ea3b17aa143..4945044d41e6 100644 >> --- a/include/drm/intel/i915_component.h >> +++ b/include/drm/intel/i915_component.h >> @@ -31,6 +31,7 @@ enum i915_component_type { >> I915_COMPONENT_HDCP, >> I915_COMPONENT_PXP, >> I915_COMPONENT_GSC_PROXY, >> + I915_COMPONENT_LATE_BIND, >> }; >> >> /* MAX_PORT is the number of port >> diff --git a/include/drm/intel/late_bind_mei_interface.h >> b/include/drm/intel/late_bind_mei_interface.h >> new file mode 100644 >> index 000000000000..2c53657ce91b >> --- /dev/null >> +++ b/include/drm/intel/late_bind_mei_interface.h >> @@ -0,0 +1,50 @@ >> +/* SPDX-License-Identifier: MIT */ >> +/* >> + * Copyright (c) 2025 Intel Corporation */ >> + >> +#ifndef _LATE_BIND_MEI_INTERFACE_H_ >> +#define _LATE_BIND_MEI_INTERFACE_H_ >> + >> +#include <linux/types.h> >> + >> +struct device; >> +struct module; >> + >> +/** >> + * Late Binding flags >> + * Persistent across warm reset >> + */ >> +#define CSC_LATE_BINDING_FLAGS_IS_PERSISTENT BIT(0) >> + >> +/** >> + * xe_late_bind_fw_type - enum to determine late binding fw type */ >> +enum late_bind_type { >> + CSC_LATE_BINDING_TYPE_FAN_CONTROL = 1, }; >> + >> +/** >> + * struct late_bind_component_ops - ops for Late Binding services. >> + * @owner: Module providing the ops >> + * @push_config: Sends a config to FW. >> + */ >> +struct late_bind_component_ops { >> + struct module *owner; >> + >> + /** >> + * @push_config: Sends a config to FW. >> + * @dev: device struct corresponding to the mei device >> + * @type: payload type >> + * @flags: payload flags >> + * @payload: payload buffer >> + * @payload_size: payload buffer size >> + * >> + * Return: 0 success, negative errno value on transport failure, >> + * positive status returned by FW >> + */ >> + int (*push_config)(struct device *dev, u32 type, u32 flags, >> + const void *payload, size_t payload_size); }; >> + >> +#endif /* _LATE_BIND_MEI_INTERFACE_H_ */ >> -- >> 2.34.1 > -- Jani Nikula, Intel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 02/10] mei: late_bind: add late binding component driver 2025-06-19 7:32 ` Gupta, Anshuman 2025-06-19 8:11 ` Jani Nikula @ 2025-06-19 9:06 ` Nilawar, Badal 1 sibling, 0 replies; 6+ messages in thread From: Nilawar, Badal @ 2025-06-19 9:06 UTC (permalink / raw) To: Gupta, Anshuman, Vivi, Rodrigo, intel-xe@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Jani Nikula, Usyskin, Alexander Cc: gregkh@linuxfoundation.org, Ceraolo Spurio, Daniele, jgg@nvidia.com [-- Attachment #1: Type: text/plain, Size: 16492 bytes --] On 19-06-2025 13:02, Gupta, Anshuman wrote: > >> -----Original Message----- >> From: Nilawar, Badal<badal.nilawar@intel.com> >> Sent: Thursday, June 19, 2025 12:30 AM >> To:intel-xe@lists.freedesktop.org;dri-devel@lists.freedesktop.org; linux- >> kernel@vger.kernel.org >> Cc: Gupta, Anshuman<anshuman.gupta@intel.com>; Vivi, Rodrigo >> <rodrigo.vivi@intel.com>; Usyskin, Alexander<alexander.usyskin@intel.com>; >> gregkh@linuxfoundation.org; Ceraolo Spurio, Daniele >> <daniele.ceraolospurio@intel.com>;jgg@nvidia.com >> Subject: [PATCH v3 02/10] mei: late_bind: add late binding component driver >> >> From: Alexander Usyskin<alexander.usyskin@intel.com> >> >> Add late binding component driver. >> It allows pushing the late binding configuration from, for example, the Xe graphics >> driver to the Intel discrete graphics card's CSE device. >> >> Signed-off-by: Alexander Usyskin<alexander.usyskin@intel.com> >> Signed-off-by: Badal Nilawar<badal.nilawar@intel.com> >> --- >> v2: >> - Use generic naming (Jani) > This patch still wrong naming I915_COMPONENT_LATE_BIND. > LATE_BIND will never be supported by i915, it is a wrong prefix. > @Nikula, Jani @Vivi, Rodrigo is it ok use the i915 naming prefix here ? > We can use INTEL_COMPONENT_LATE_BIND here ? Agree, if we use INTEL_ prefix here is it correct to add this file? > > This header include/drm/intel/i915_component.h is used by both XE and i915. > May be a separate series later requires refactoring this header. Agree, better handle naming of all the components while refactoring. > > >> - Drop xe_late_bind_component struct to move to xe code (Daniele/Sasha) >> v3: >> - Updated kconfig description >> - Move CSC late binding specific flags/defines to late_bind_mei_interface.h >> (Daniele) >> v4: >> - Add match for PCI_CLASS_DISPLAY_OTHER to support headless cards >> (Anshuman) >> --- >> drivers/misc/mei/Kconfig | 1 + >> drivers/misc/mei/Makefile | 1 + >> drivers/misc/mei/late_bind/Kconfig | 13 + >> drivers/misc/mei/late_bind/Makefile | 9 + >> drivers/misc/mei/late_bind/mei_late_bind.c | 264 ++++++++++++++++++++ >> include/drm/intel/i915_component.h | 1 + >> include/drm/intel/late_bind_mei_interface.h | 50 ++++ >> 7 files changed, 339 insertions(+) >> create mode 100644 drivers/misc/mei/late_bind/Kconfig >> create mode 100644 drivers/misc/mei/late_bind/Makefile >> create mode 100644 drivers/misc/mei/late_bind/mei_late_bind.c >> create mode 100644 include/drm/intel/late_bind_mei_interface.h >> >> diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig index >> 7575fee96cc6..771becc68095 100644 >> --- a/drivers/misc/mei/Kconfig >> +++ b/drivers/misc/mei/Kconfig >> @@ -84,5 +84,6 @@ config INTEL_MEI_VSC >> source "drivers/misc/mei/hdcp/Kconfig" >> source "drivers/misc/mei/pxp/Kconfig" >> source "drivers/misc/mei/gsc_proxy/Kconfig" >> +source "drivers/misc/mei/late_bind/Kconfig" >> >> endif >> diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile index >> 6f9fdbf1a495..84bfde888d81 100644 >> --- a/drivers/misc/mei/Makefile >> +++ b/drivers/misc/mei/Makefile >> @@ -31,6 +31,7 @@ CFLAGS_mei-trace.o = -I$(src) >> obj-$(CONFIG_INTEL_MEI_HDCP) += hdcp/ >> obj-$(CONFIG_INTEL_MEI_PXP) += pxp/ >> obj-$(CONFIG_INTEL_MEI_GSC_PROXY) += gsc_proxy/ >> +obj-$(CONFIG_INTEL_MEI_LATE_BIND) += late_bind/ >> >> obj-$(CONFIG_INTEL_MEI_VSC_HW) += mei-vsc-hw.o mei-vsc-hw-y := vsc-tp.o >> diff --git a/drivers/misc/mei/late_bind/Kconfig >> b/drivers/misc/mei/late_bind/Kconfig >> new file mode 100644 >> index 000000000000..65c7180c5678 >> --- /dev/null >> +++ b/drivers/misc/mei/late_bind/Kconfig >> @@ -0,0 +1,13 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +# Copyright (c) 2025, Intel Corporation. All rights reserved. >> +# >> +config INTEL_MEI_LATE_BIND >> + tristate "Intel late binding support on ME Interface" >> + select INTEL_MEI_ME >> + depends on DRM_XE >> + help >> + MEI Support for Late Binding for Intel graphics card. >> + >> + Enables the ME FW interfaces for Late Binding feature, >> + allowing loading of firmware for the devices like Fan >> + Controller during by Intel Xe driver. >> diff --git a/drivers/misc/mei/late_bind/Makefile >> b/drivers/misc/mei/late_bind/Makefile >> new file mode 100644 >> index 000000000000..a0aeda5853f0 >> --- /dev/null >> +++ b/drivers/misc/mei/late_bind/Makefile >> @@ -0,0 +1,9 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +# >> +# Copyright (c) 2025, Intel Corporation. All rights reserved. >> +# >> +# Makefile - Late Binding client driver for Intel MEI Bus Driver. >> + >> +subdir-ccflags-y += -I$(srctree)/drivers/misc/mei/ >> + >> +obj-$(CONFIG_INTEL_MEI_LATE_BIND) += mei_late_bind.o >> diff --git a/drivers/misc/mei/late_bind/mei_late_bind.c >> b/drivers/misc/mei/late_bind/mei_late_bind.c >> new file mode 100644 >> index 000000000000..cb985f32309e >> --- /dev/null >> +++ b/drivers/misc/mei/late_bind/mei_late_bind.c >> @@ -0,0 +1,264 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2025 Intel Corporation */ #include >> +<drm/drm_connector.h> #include <drm/intel/i915_component.h> #include >> +<drm/intel/late_bind_mei_interface.h> >> +#include <linux/component.h> >> +#include <linux/pci.h> >> +#include <linux/mei_cl_bus.h> >> +#include <linux/module.h> >> +#include <linux/overflow.h> >> +#include <linux/slab.h> >> +#include <linux/uuid.h> >> + >> +#include "mkhi.h" >> + >> +#define GFX_SRV_MKHI_LATE_BINDING_CMD 0x12 #define >> +GFX_SRV_MKHI_LATE_BINDING_RSP (GFX_SRV_MKHI_LATE_BINDING_CMD | >> 0x80) >> + >> +#define LATE_BIND_SEND_TIMEOUT_MSEC 3000 #define >> +LATE_BIND_RECV_TIMEOUT_MSEC 3000 > I commented earlier in V2 series as well, is this timeout specific only to LATE BINDING ? > If this is generic timeout for mei_cldev_{send,recv}_timeout(), > then this marco should be part of standard MEI headers not late binding. > Other consumers of mei_cldev_{send,recv}_timeout() send the timeout input by component-ops callback . > > @Shahsa could you please explained that. As per Sasha: This is not generic timeout, only for specific client - mkhi as it is general purpose client and many user-space connects there. Timeouts are derived from real word experience, spec say "close connection as fast as you can and retry if busy", no specific timeout as it flow-specific thing. >> + >> +/** >> + * struct csc_heci_late_bind_req - late binding request >> + * @header: @ref mkhi_msg_hdr >> + * @type: type of the late binding payload >> + * @flags: flags to be passed to the firmware >> + * @reserved: reserved field >> + * @payload_size: size of the payload data in bytes >> + * @payload: data to be sent to the firmware */ struct >> +csc_heci_late_bind_req { >> + struct mkhi_msg_hdr header; >> + u32 type; >> + u32 flags; >> + u32 reserved[2]; >> + u32 payload_size; >> + u8 payload[] __counted_by(payload_size); } __packed; >> + >> +/** >> + * struct csc_heci_late_bind_rsp - late binding response >> + * @header: @ref mkhi_msg_hdr >> + * @type: type of the late binding payload >> + * @reserved: reserved field >> + * @status: status of the late binding command execution by firmware >> +*/ struct csc_heci_late_bind_rsp { >> + struct mkhi_msg_hdr header; >> + u32 type; >> + u32 reserved[2]; >> + u32 status; >> +} __packed; >> + >> +static int mei_late_bind_check_response(const struct device *dev, const >> +struct mkhi_msg_hdr *hdr) { >> + if (hdr->group_id != MKHI_GROUP_ID_GFX) { >> + dev_err(dev, "Mismatch group id: 0x%x instead of 0x%x\n", >> + hdr->group_id, MKHI_GROUP_ID_GFX); >> + return -EINVAL; >> + } >> + >> + if (hdr->command != GFX_SRV_MKHI_LATE_BINDING_RSP) { >> + dev_err(dev, "Mismatch command: 0x%x instead of 0x%x\n", >> + hdr->command, GFX_SRV_MKHI_LATE_BINDING_RSP); >> + return -EINVAL; >> + } > Why are we not checking mkhi_msg_hdr hdr->result here ? Sasha has some suggestions around this, will incorporate in next rev. >> + >> + return 0; >> +} >> + >> +/** >> + * mei_late_bind_push_config - Sends a config to the firmware. >> + * @dev: device struct corresponding to the mei device >> + * @type: payload type >> + * @flags: payload flags >> + * @payload: payload buffer >> + * @payload_size: payload buffer size >> + * >> + * Return: 0 success, negative errno value on transport failure, >> + * positive status returned by FW >> + */ >> +static int mei_late_bind_push_config(struct device *dev, u32 type, u32 flags, >> + const void *payload, size_t payload_size) { >> + struct mei_cl_device *cldev; >> + struct csc_heci_late_bind_req *req = NULL; >> + struct csc_heci_late_bind_rsp rsp; >> + size_t req_size; >> + int ret; >> + >> + if (!dev || !payload || !payload_size) >> + return -EINVAL; >> + >> + cldev = to_mei_cl_device(dev); >> + >> + ret = mei_cldev_enable(cldev); >> + if (ret < 0) { >> + dev_dbg(dev, "mei_cldev_enable failed. %d\n", ret); >> + return ret; >> + } >> + >> + req_size = struct_size(req, payload, payload_size); >> + if (req_size > mei_cldev_mtu(cldev)) { >> + dev_err(dev, "Payload is too big %zu\n", payload_size); >> + ret = -EMSGSIZE; >> + goto end; >> + } >> + >> + req = kmalloc(req_size, GFP_KERNEL); >> + if (!req) { >> + ret = -ENOMEM; >> + goto end; >> + } > Use Kzalloc here, to make sure reserved filed of header is zeroed. Sure. >> + >> + req->header.group_id = MKHI_GROUP_ID_GFX; >> + req->header.command = GFX_SRV_MKHI_LATE_BINDING_CMD; >> + req->type = type; >> + req->flags = flags; >> + req->reserved[0] = 0; >> + req->reserved[1] = 0; >> + req->payload_size = payload_size; >> + memcpy(req->payload, payload, payload_size); >> + >> + ret = mei_cldev_send_timeout(cldev, (void *)req, req_size, >> LATE_BIND_SEND_TIMEOUT_MSEC); >> + if (ret < 0) { >> + dev_err(dev, "mei_cldev_send failed. %d\n", ret); >> + goto end; >> + } >> + ret = mei_cldev_recv_timeout(cldev, (void *)&rsp, sizeof(rsp), >> LATE_BIND_RECV_TIMEOUT_MSEC); >> + if (ret < 0) { >> + dev_err(dev, "mei_cldev_recv failed. %d\n", ret); >> + goto end; >> + } >> + ret = mei_late_bind_check_response(dev, &rsp.header); >> + if (ret) { >> + dev_err(dev, "bad result response from the firmware: 0x%x\n", >> + *(uint32_t *)&rsp.header); >> + goto end; >> + } >> + ret = (int)rsp.status; >> + dev_dbg(dev, "%s status = %d\n", __func__, ret); > AFAIU It would be useful to add the status enum in late_bind_mei_interface.h. Will discuss with Sasha and add if he agree. >> + >> +end: >> + mei_cldev_disable(cldev); >> + kfree(req); >> + return ret; >> +} >> + >> +static const struct late_bind_component_ops mei_late_bind_ops = { >> + .owner = THIS_MODULE, >> + .push_config = mei_late_bind_push_config, }; >> + >> +static int mei_component_master_bind(struct device *dev) { >> + return component_bind_all(dev, (void *)&mei_late_bind_ops); } >> + >> +static void mei_component_master_unbind(struct device *dev) { >> + component_unbind_all(dev, (void *)&mei_late_bind_ops); } >> + >> +static const struct component_master_ops mei_component_master_ops = { >> + .bind = mei_component_master_bind, >> + .unbind = mei_component_master_unbind, }; >> + >> +/** >> + * mei_late_bind_component_match - compare function for matching mei late >> bind. >> + * >> + * The function checks if requested is Intel VGA device > Please modify the Kenel Doc comment here, as per the function. >> + * and the parent of requester and the grand parent of mei_if are the same > We are matching against the requester not parent of requester. > Modify the Kernel Doc comment properly. >> + * device. >> + * >> + * @dev: master device >> + * @subcomponent: subcomponent to match (I915_COMPONENT_LATE_BIND) >> + * @data: compare data (mei late-bind bus device) > AFAIK It is mei client device not mei bus device. It is late-bind device on mei bus. xe_device -> mei client aux device -> [mei bus]-> mei late bind device >> + * >> + * Return: >> + * * 1 - if components match >> + * * 0 - otherwise >> + */ >> +static int mei_late_bind_component_match(struct device *dev, int >> subcomponent, >> + void *data) >> +{ >> + struct device *base = data; >> + struct pci_dev *pdev; >> + >> + if (!dev) >> + return 0; >> + >> + if (!dev_is_pci(dev)) >> + return 0; >> + >> + pdev = to_pci_dev(dev); >> + >> + if (pdev->vendor != PCI_VENDOR_ID_INTEL) >> + return 0; >> + >> + if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) || >> + pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8)) >> + return 0; > This condition should be like below, if I am not missing anything. > if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) && > pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8)) Correct, will fix this. Thanks, Badal > > Thanks, > Anshuman. >> + >> + if (subcomponent != I915_COMPONENT_LATE_BIND) >> + return 0; >> + >> + base = base->parent; >> + if (!base) /* mei device */ >> + return 0; >> + >> + base = base->parent; /* pci device */ >> + >> + return !!base && dev == base; >> +} >> + >> +static int mei_late_bind_probe(struct mei_cl_device *cldev, >> + const struct mei_cl_device_id *id) { >> + struct component_match *master_match = NULL; >> + int ret; >> + >> + component_match_add_typed(&cldev->dev, &master_match, >> + mei_late_bind_component_match, &cldev- >>> dev); >> + if (IS_ERR_OR_NULL(master_match)) >> + return -ENOMEM; >> + >> + ret = component_master_add_with_match(&cldev->dev, >> + &mei_component_master_ops, >> + master_match); >> + if (ret < 0) >> + dev_err(&cldev->dev, "Master comp add failed %d\n", ret); >> + >> + return ret; >> +} >> + >> +static void mei_late_bind_remove(struct mei_cl_device *cldev) { >> + component_master_del(&cldev->dev, &mei_component_master_ops); } >> + >> +#define MEI_GUID_MKHI UUID_LE(0xe2c2afa2, 0x3817, 0x4d19, \ >> + 0x9d, 0x95, 0x6, 0xb1, 0x6b, 0x58, 0x8a, 0x5d) >> + >> +static struct mei_cl_device_id mei_late_bind_tbl[] = { >> + { .uuid = MEI_GUID_MKHI, .version = MEI_CL_VERSION_ANY }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(mei, mei_late_bind_tbl); >> + >> +static struct mei_cl_driver mei_late_bind_driver = { >> + .id_table = mei_late_bind_tbl, >> + .name = KBUILD_MODNAME, >> + .probe = mei_late_bind_probe, >> + .remove = mei_late_bind_remove, >> +}; >> + >> +module_mei_cl_driver(mei_late_bind_driver); >> + >> +MODULE_AUTHOR("Intel Corporation"); >> +MODULE_LICENSE("GPL"); >> +MODULE_DESCRIPTION("MEI Late Binding"); >> diff --git a/include/drm/intel/i915_component.h >> b/include/drm/intel/i915_component.h >> index 4ea3b17aa143..4945044d41e6 100644 >> --- a/include/drm/intel/i915_component.h >> +++ b/include/drm/intel/i915_component.h >> @@ -31,6 +31,7 @@ enum i915_component_type { >> I915_COMPONENT_HDCP, >> I915_COMPONENT_PXP, >> I915_COMPONENT_GSC_PROXY, >> + I915_COMPONENT_LATE_BIND, >> }; >> >> /* MAX_PORT is the number of port >> diff --git a/include/drm/intel/late_bind_mei_interface.h >> b/include/drm/intel/late_bind_mei_interface.h >> new file mode 100644 >> index 000000000000..2c53657ce91b >> --- /dev/null >> +++ b/include/drm/intel/late_bind_mei_interface.h >> @@ -0,0 +1,50 @@ >> +/* SPDX-License-Identifier: MIT */ >> +/* >> + * Copyright (c) 2025 Intel Corporation */ >> + >> +#ifndef _LATE_BIND_MEI_INTERFACE_H_ >> +#define _LATE_BIND_MEI_INTERFACE_H_ >> + >> +#include <linux/types.h> >> + >> +struct device; >> +struct module; >> + >> +/** >> + * Late Binding flags >> + * Persistent across warm reset >> + */ >> +#define CSC_LATE_BINDING_FLAGS_IS_PERSISTENT BIT(0) >> + >> +/** >> + * xe_late_bind_fw_type - enum to determine late binding fw type */ >> +enum late_bind_type { >> + CSC_LATE_BINDING_TYPE_FAN_CONTROL = 1, }; >> + >> +/** >> + * struct late_bind_component_ops - ops for Late Binding services. >> + * @owner: Module providing the ops >> + * @push_config: Sends a config to FW. >> + */ >> +struct late_bind_component_ops { >> + struct module *owner; >> + >> + /** >> + * @push_config: Sends a config to FW. >> + * @dev: device struct corresponding to the mei device >> + * @type: payload type >> + * @flags: payload flags >> + * @payload: payload buffer >> + * @payload_size: payload buffer size >> + * >> + * Return: 0 success, negative errno value on transport failure, >> + * positive status returned by FW >> + */ >> + int (*push_config)(struct device *dev, u32 type, u32 flags, >> + const void *payload, size_t payload_size); }; >> + >> +#endif /* _LATE_BIND_MEI_INTERFACE_H_ */ >> -- >> 2.34.1 [-- Attachment #2: Type: text/html, Size: 20882 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 02/10] mei: late_bind: add late binding component driver 2025-06-18 18:59 ` [PATCH v3 02/10] mei: late_bind: add late binding component driver Badal Nilawar 2025-06-19 7:32 ` Gupta, Anshuman @ 2025-06-24 13:37 ` Dan Carpenter 1 sibling, 0 replies; 6+ messages in thread From: Dan Carpenter @ 2025-06-24 13:37 UTC (permalink / raw) To: oe-kbuild, Badal Nilawar; +Cc: lkp, oe-kbuild-all Hi Badal, kernel test robot noticed the following build warnings: https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Badal-Nilawar/mei-bus-add-mei_cldev_mtu-interface/20250619-025825 base: https://gitlab.freedesktop.org/drm/xe/kernel.git drm-xe-next patch link: https://lore.kernel.org/r/20250618190007.2932322-3-badal.nilawar%40intel.com patch subject: [PATCH v3 02/10] mei: late_bind: add late binding component driver config: i386-randconfig-141-20250623 (https://download.01.org/0day-ci/archive/20250624/202506241103.XiG3WA7g-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org> | Closes: https://lore.kernel.org/r/202506241103.XiG3WA7g-lkp@intel.com/ smatch warnings: drivers/misc/mei/late_bind/mei_late_bind.c:203 mei_late_bind_component_match() warn: was && intended here instead of ||? vim +203 drivers/misc/mei/late_bind/mei_late_bind.c b4fe758cb831cd Alexander Usyskin 2025-06-19 186 static int mei_late_bind_component_match(struct device *dev, int subcomponent, b4fe758cb831cd Alexander Usyskin 2025-06-19 187 void *data) b4fe758cb831cd Alexander Usyskin 2025-06-19 188 { b4fe758cb831cd Alexander Usyskin 2025-06-19 189 struct device *base = data; b4fe758cb831cd Alexander Usyskin 2025-06-19 190 struct pci_dev *pdev; b4fe758cb831cd Alexander Usyskin 2025-06-19 191 b4fe758cb831cd Alexander Usyskin 2025-06-19 192 if (!dev) b4fe758cb831cd Alexander Usyskin 2025-06-19 193 return 0; b4fe758cb831cd Alexander Usyskin 2025-06-19 194 b4fe758cb831cd Alexander Usyskin 2025-06-19 195 if (!dev_is_pci(dev)) b4fe758cb831cd Alexander Usyskin 2025-06-19 196 return 0; b4fe758cb831cd Alexander Usyskin 2025-06-19 197 b4fe758cb831cd Alexander Usyskin 2025-06-19 198 pdev = to_pci_dev(dev); b4fe758cb831cd Alexander Usyskin 2025-06-19 199 b4fe758cb831cd Alexander Usyskin 2025-06-19 200 if (pdev->vendor != PCI_VENDOR_ID_INTEL) b4fe758cb831cd Alexander Usyskin 2025-06-19 201 return 0; b4fe758cb831cd Alexander Usyskin 2025-06-19 202 b4fe758cb831cd Alexander Usyskin 2025-06-19 @203 if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) || This || should be &&. Currently the function will always return 0. b4fe758cb831cd Alexander Usyskin 2025-06-19 204 pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8)) b4fe758cb831cd Alexander Usyskin 2025-06-19 205 return 0; b4fe758cb831cd Alexander Usyskin 2025-06-19 206 b4fe758cb831cd Alexander Usyskin 2025-06-19 207 if (subcomponent != I915_COMPONENT_LATE_BIND) b4fe758cb831cd Alexander Usyskin 2025-06-19 208 return 0; b4fe758cb831cd Alexander Usyskin 2025-06-19 209 b4fe758cb831cd Alexander Usyskin 2025-06-19 210 base = base->parent; b4fe758cb831cd Alexander Usyskin 2025-06-19 211 if (!base) /* mei device */ b4fe758cb831cd Alexander Usyskin 2025-06-19 212 return 0; b4fe758cb831cd Alexander Usyskin 2025-06-19 213 b4fe758cb831cd Alexander Usyskin 2025-06-19 214 base = base->parent; /* pci device */ b4fe758cb831cd Alexander Usyskin 2025-06-19 215 b4fe758cb831cd Alexander Usyskin 2025-06-19 216 return !!base && dev == base; b4fe758cb831cd Alexander Usyskin 2025-06-19 217 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-06-24 13:37 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-24 3:14 [PATCH v3 02/10] mei: late_bind: add late binding component driver kernel test robot -- strict thread matches above, loose matches on Subject: below -- 2025-06-18 18:59 [PATCH v3 00/10] Introducing firmware late binding Badal Nilawar 2025-06-18 18:59 ` [PATCH v3 02/10] mei: late_bind: add late binding component driver Badal Nilawar 2025-06-19 7:32 ` Gupta, Anshuman 2025-06-19 8:11 ` Jani Nikula 2025-06-19 9:06 ` Nilawar, Badal 2025-06-24 13:37 ` Dan Carpenter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.