On 4/18/2023 11:57 PM, Teres Alexis, Alan Previn wrote:
On Wed, 2023-03-29 at 09:56 -0700, Ceraolo Spurio, Daniele wrote:
From: Alexander Usyskin <alexander.usyskin@xxxxxxxxx>
Add GSC proxy driver. It to allows messaging between GSC component
on Intel on board graphics card and CSE device.
alan:nit: isn't "Intel integrated GPU" clearer than "Intel on-board graphics card"?
Same thing for the Kconfig description later (or am i missing something else here).
Will change
alan:snip
+ MEI GSC proxy enables messaging between GSC service on
+ Intel graphics on-board card and services on CSE (MEI)
alan:nit: same as above
diff --git a/drivers/misc/mei/gsc_proxy/mei_gsc_proxy.c b/drivers/misc/mei/gsc_proxy/mei_gsc_proxy.c
new file mode 100644
index 000000000000..953eda1a16fb
--- /dev/null
+++ b/drivers/misc/mei/gsc_proxy/mei_gsc_proxy.c
@@ -0,0 +1,208 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022-2023 Intel Corporation
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/slab.h>
+#include <linux/uuid.h>
+#include <linux/mei_cl_bus.h>
alan: [nit?] i thought we need to have the headers alphabetically ordered? (below too)
Will fix
+#include <linux/component.h>
+#include <drm/drm_connector.h>
+#include <drm/i915_component.h>
+#include <drm/i915_gsc_proxy_mei_interface.h>
+
+/**
+ * mei_gsc_proxy_send - Sends a proxy message to ME FW.
+ * @dev: device corresponding to the mei_cl_device
+ * @buf: a message buffer to send
+ * @size: size of the message
+ * Return: bytes sent on Success, <0 on Failure
+ */
+static int mei_gsc_proxy_send(struct device *dev, const void *buf, size_t size)
+{
+ ssize_t ret;
+
+ if (!dev || !buf)
alan: nit: not sure if we should be checking for !size here - i do see that next patch
is checking for this from i915 side of the interface... but just wasnt sure which is the prefered style
(in terms of where to check for this condition). Either way, its a nit for me since i traced down
all the way to mei_cl_alloc_cb and it looks like mei bus can tolerate zero sized messages.
+ return -EINVAL;
alan:snip
+static int mei_gsc_proxy_recv(struct device *dev, void *buf, size_t size)
+{
+ ssize_t ret;
+
+ if (!dev || !buf)
alan: nit: same as in the 'send' above,.. not sure if we should be checking for !size here...
or perhaps 0 sized recv is supported.
AFAICS the lower level of the mei code do allow for size 0 for both send
and recv. Also, this is the same check as what we do for the PXP component.
+ return -EINVAL;
alan:snip
+static int mei_gsc_proxy_component_match(struct device *dev, int subcomponent,
+ void *data)
+{
+ struct pci_dev *pdev;
+
+ if (!dev_is_pci(dev))
+ return 0;
+
+ pdev = to_pci_dev(dev);
+
+ if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) ||
+ pdev->vendor != PCI_VENDOR_ID_INTEL)
+ return 0;
+
+ if (subcomponent != I915_COMPONENT_GSC_PROXY)
+ return 0;
+
+ return component_compare_dev(dev->parent, ((struct device *)data)->parent);
alan: do we care if both these parents are non-null? i notice in other mei component
drivers match functions we do check that.
Those should always both be non-NULL, since both the mei and the GFX
device have the PCI bus as parent (and the previous check on pdev
ensures those are the 2 devices we're handling at this point).
+}
+
alan:snip
+#define MEI_UUID_GSC_PROXY UUID_LE(0xf73db04, 0x97ab, 0x4125, \
+ 0xb8, 0x93, 0xe9, 0x4, 0xad, 0xd, 0x54, 0x64)
alan: apologies for the newbie question, but why are we using UUID for the gsc_proxy
as opposed to GUID like the other mei components? i am not sure if i read the right
archived patch review but it sounded like GUID is for internal to kernel only whereas
UUID is for external too? (in which case why are we not using GUID for gsc-proxy?)
Consider this a non-blocking inquiry since i assume mei folks have reviewed this
prior and this is an explicit design decision that I'm just not versed on.
AFAICS all other mei components use UUID_LE as well. The code was
updated from GUID to UUID_LE in:
https://lore.kernel.org/all/20221228160558.21311-1-andriy.shevchenko@xxxxxxxxxxxxxxx/
Daniele
alan:snip
