[PATCH v2 0/4] drm/xe/gsc: add initial gsc support

[PATCH v2 0/4] drm/xe/gsc: add initial gsc support

[Alexander Usyskin]

Add initial GSC support for DG1 and DG2.
Create mei-gscfi auxiliary device and add
support for device created by Xe to the mei-gsc driver.

This series intended to be merged intto Xe tree,
the only patch that touches char-misc is addition
of id and alias in mei-gsc.

V2: Added heci_ to struct and APIs
    heci_gsc moved under xe_device as GSC is not under GT

Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>

Vitaly Lubart (4):
  drm/xe/gsc: add HECI2 register offsets
  drm/xe/gsc: add has_heci_gscfi indication to device
  drm/xe/gsc: add gsc device support
  mei: gsc: add support for auxiliary device created by Xe driver

 drivers/gpu/drm/xe/Kconfig           |   1 +
 drivers/gpu/drm/xe/Makefile          |   1 +
 drivers/gpu/drm/xe/regs/xe_regs.h    |   4 +
 drivers/gpu/drm/xe/xe_device.c       |   4 +
 drivers/gpu/drm/xe/xe_device_types.h |   7 +
 drivers/gpu/drm/xe/xe_heci_gsc.c     | 205 +++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_heci_gsc.h     |  35 +++++
 drivers/gpu/drm/xe/xe_irq.c          |  14 +-
 drivers/gpu/drm/xe/xe_pci.c          |   9 +-
 drivers/misc/mei/gsc-me.c            |   5 +
 10 files changed, 281 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/xe/xe_heci_gsc.c
 create mode 100644 drivers/gpu/drm/xe/xe_heci_gsc.h

-- 
2.34.1

[PATCH v2 1/4] drm/xe/gsc: add HECI2 register offsets

[Alexander Usyskin]

From: Vitaly Lubart <vitaly.lubart@intel.com>

Add HECI2 register offsets for DG1 and DG2 to regs/xe_regs.h

Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/gpu/drm/xe/regs/xe_regs.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/xe/regs/xe_regs.h b/drivers/gpu/drm/xe/regs/xe_regs.h
index 39d7b0740bf0..4cbc3062cb9a 100644
--- a/drivers/gpu/drm/xe/regs/xe_regs.h
+++ b/drivers/gpu/drm/xe/regs/xe_regs.h
@@ -33,6 +33,10 @@
 #define XEHPC_BCS6_RING_BASE			0x3ea000
 #define XEHPC_BCS7_RING_BASE			0x3ec000
 #define XEHPC_BCS8_RING_BASE			0x3ee000
+
+#define DG1_GSC_HECI2_BASE                      0x00259000
+#define DG2_GSC_HECI2_BASE                      0x00374000
+
 #define GSCCS_RING_BASE				0x11a000
 #define   GT_WAIT_SEMAPHORE_INTERRUPT		REG_BIT(11)
 #define   GT_CONTEXT_SWITCH_INTERRUPT		REG_BIT(8)
-- 
2.34.1

Re: [Intel-xe] [PATCH v2 1/4] drm/xe/gsc: add HECI2 register offsets

[Rodrigo Vivi]

On Thu, Sep 14, 2023 at 11:01:35AM +0300, Alexander Usyskin wrote:
> From: Vitaly Lubart <vitaly.lubart@intel.com>
> 
> Add HECI2 register offsets for DG1 and DG2 to regs/xe_regs.h
> 
> Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> ---
>  drivers/gpu/drm/xe/regs/xe_regs.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_regs.h b/drivers/gpu/drm/xe/regs/xe_regs.h
> index 39d7b0740bf0..4cbc3062cb9a 100644
> --- a/drivers/gpu/drm/xe/regs/xe_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_regs.h
> @@ -33,6 +33,10 @@
>  #define XEHPC_BCS6_RING_BASE			0x3ea000
>  #define XEHPC_BCS7_RING_BASE			0x3ec000
>  #define XEHPC_BCS8_RING_BASE			0x3ee000
> +
> +#define DG1_GSC_HECI2_BASE                      0x00259000
> +#define DG2_GSC_HECI2_BASE                      0x00374000

matches the i915 ones, so

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> +
>  #define GSCCS_RING_BASE				0x11a000
>  #define   GT_WAIT_SEMAPHORE_INTERRUPT		REG_BIT(11)
>  #define   GT_CONTEXT_SWITCH_INTERRUPT		REG_BIT(8)
> -- 
> 2.34.1
> 

[PATCH v2 2/4] drm/xe/gsc: add has_heci_gscfi indication to device

[Alexander Usyskin]

From: Vitaly Lubart <vitaly.lubart@intel.com>

Mark support of MEI-GSC interaction per device.
Add has_heci_gscfi indication to xe_device and xe_pci structures.
Mark DG1 and DG2 devices as supported.

Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/gpu/drm/xe/xe_device_types.h | 3 +++
 drivers/gpu/drm/xe/xe_pci.c          | 9 +++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index 750e1f0d3339..1d1fe53fc30d 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -32,6 +32,7 @@ struct xe_ggtt;
 #define GRAPHICS_VERx100(xe) ((xe)->info.graphics_verx100)
 #define MEDIA_VERx100(xe) ((xe)->info.media_verx100)
 #define IS_DGFX(xe) ((xe)->info.is_dgfx)
+#define HAS_HECI_GSCFI(xe) ((xe)->info.has_heci_gscfi)
 
 #define XE_VRAM_FLAGS_NEED64K		BIT(0)
 
@@ -237,6 +238,8 @@ struct xe_device {
 		u8 has_link_copy_engine:1;
 		/** @enable_display: display enabled */
 		u8 enable_display:1;
+		/** @has_heci_gscfi: device has heci gscfi */
+		u8 has_heci_gscfi:1;
 
 #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
 		const struct intel_display_device_info *display;
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index dc233a1226bd..145eaa430d74 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -57,6 +57,7 @@ struct xe_device_desc {
 	u8 require_force_probe:1;
 	u8 is_dgfx:1;
 	u8 has_display:1;
+	u8 has_heci_gscfi:1;
 
 	u8 has_llc:1;
 };
@@ -265,6 +266,7 @@ static const struct xe_device_desc dg1_desc = {
 	PLATFORM(XE_DG1),
 	.has_display = true,
 	.require_force_probe = true,
+	.has_heci_gscfi = 1,
 };
 
 static const u16 dg2_g10_ids[] = { XE_DG2_G10_IDS(NOP), XE_ATS_M150_IDS(NOP), 0 };
@@ -274,6 +276,7 @@ static const u16 dg2_g12_ids[] = { XE_DG2_G12_IDS(NOP), 0 };
 #define DG2_FEATURES \
 	DGFX_FEATURES, \
 	PLATFORM(XE_DG2), \
+	.has_heci_gscfi = 1, \
 	.subplatforms = (const struct xe_subplatform_desc[]) { \
 		{ XE_SUBPLATFORM_DG2_G10, "G10", dg2_g10_ids }, \
 		{ XE_SUBPLATFORM_DG2_G11, "G11", dg2_g11_ids }, \
@@ -562,6 +565,7 @@ static int xe_info_init(struct xe_device *xe,
 		return -ENODEV;
 
 	xe->info.is_dgfx = desc->is_dgfx;
+	xe->info.has_heci_gscfi = desc->has_heci_gscfi;
 	xe->info.graphics_name = graphics_desc->name;
 	xe->info.media_name = media_desc ? media_desc->name : "none";
 	xe->info.has_llc = desc->has_llc;
@@ -703,7 +707,7 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	xe_display_info_init(xe);
 
-	drm_dbg(&xe->drm, "%s %s %04x:%04x dgfx:%d gfx:%s (%d.%02d) media:%s (%d.%02d) display:%s dma_m_s:%d tc:%d",
+	drm_dbg(&xe->drm, "%s %s %04x:%04x dgfx:%d gfx:%s (%d.%02d) media:%s (%d.%02d) display:%s dma_m_s:%d tc:%d gscfi:%d",
 		desc->platform_name,
 		subplatform_desc ? subplatform_desc->name : "",
 		xe->info.devid, xe->info.revid,
@@ -715,7 +719,8 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		xe->info.media_verx100 / 100,
 		xe->info.media_verx100 % 100,
 		str_yes_no(xe->info.enable_display),
-		xe->info.dma_mask_size, xe->info.tile_count);
+		xe->info.dma_mask_size, xe->info.tile_count,
+		xe->info.has_heci_gscfi);
 
 	drm_dbg(&xe->drm, "Stepping = (G:%s, M:%s, D:%s, B:%s)\n",
 		xe_step_name(xe->info.step.graphics),
-- 
2.34.1

Re: [Intel-xe] [PATCH v2 2/4] drm/xe/gsc: add has_heci_gscfi indication to device

[Rodrigo Vivi]

On Thu, Sep 14, 2023 at 11:01:36AM +0300, Alexander Usyskin wrote:
> From: Vitaly Lubart <vitaly.lubart@intel.com>
> 
> Mark support of MEI-GSC interaction per device.
> Add has_heci_gscfi indication to xe_device and xe_pci structures.
> Mark DG1 and DG2 devices as supported.
> 
> Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_device_types.h | 3 +++
>  drivers/gpu/drm/xe/xe_pci.c          | 9 +++++++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 750e1f0d3339..1d1fe53fc30d 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -32,6 +32,7 @@ struct xe_ggtt;
>  #define GRAPHICS_VERx100(xe) ((xe)->info.graphics_verx100)
>  #define MEDIA_VERx100(xe) ((xe)->info.media_verx100)
>  #define IS_DGFX(xe) ((xe)->info.is_dgfx)
> +#define HAS_HECI_GSCFI(xe) ((xe)->info.has_heci_gscfi)
>  
>  #define XE_VRAM_FLAGS_NEED64K		BIT(0)
>  
> @@ -237,6 +238,8 @@ struct xe_device {
>  		u8 has_link_copy_engine:1;
>  		/** @enable_display: display enabled */
>  		u8 enable_display:1;
> +		/** @has_heci_gscfi: device has heci gscfi */
> +		u8 has_heci_gscfi:1;
>  
>  #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
>  		const struct intel_display_device_info *display;
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index dc233a1226bd..145eaa430d74 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -57,6 +57,7 @@ struct xe_device_desc {
>  	u8 require_force_probe:1;
>  	u8 is_dgfx:1;
>  	u8 has_display:1;
> +	u8 has_heci_gscfi:1;
>  
>  	u8 has_llc:1;
>  };
> @@ -265,6 +266,7 @@ static const struct xe_device_desc dg1_desc = {
>  	PLATFORM(XE_DG1),
>  	.has_display = true,
>  	.require_force_probe = true,
> +	.has_heci_gscfi = 1,
>  };
>  
>  static const u16 dg2_g10_ids[] = { XE_DG2_G10_IDS(NOP), XE_ATS_M150_IDS(NOP), 0 };
> @@ -274,6 +276,7 @@ static const u16 dg2_g12_ids[] = { XE_DG2_G12_IDS(NOP), 0 };
>  #define DG2_FEATURES \
>  	DGFX_FEATURES, \
>  	PLATFORM(XE_DG2), \
> +	.has_heci_gscfi = 1, \
>  	.subplatforms = (const struct xe_subplatform_desc[]) { \
>  		{ XE_SUBPLATFORM_DG2_G10, "G10", dg2_g10_ids }, \
>  		{ XE_SUBPLATFORM_DG2_G11, "G11", dg2_g11_ids }, \
> @@ -562,6 +565,7 @@ static int xe_info_init(struct xe_device *xe,
>  		return -ENODEV;
>  
>  	xe->info.is_dgfx = desc->is_dgfx;
> +	xe->info.has_heci_gscfi = desc->has_heci_gscfi;
>  	xe->info.graphics_name = graphics_desc->name;
>  	xe->info.media_name = media_desc ? media_desc->name : "none";
>  	xe->info.has_llc = desc->has_llc;
> @@ -703,7 +707,7 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	xe_display_info_init(xe);
>  
> -	drm_dbg(&xe->drm, "%s %s %04x:%04x dgfx:%d gfx:%s (%d.%02d) media:%s (%d.%02d) display:%s dma_m_s:%d tc:%d",
> +	drm_dbg(&xe->drm, "%s %s %04x:%04x dgfx:%d gfx:%s (%d.%02d) media:%s (%d.%02d) display:%s dma_m_s:%d tc:%d gscfi:%d",
>  		desc->platform_name,
>  		subplatform_desc ? subplatform_desc->name : "",
>  		xe->info.devid, xe->info.revid,
> @@ -715,7 +719,8 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		xe->info.media_verx100 / 100,
>  		xe->info.media_verx100 % 100,
>  		str_yes_no(xe->info.enable_display),
> -		xe->info.dma_mask_size, xe->info.tile_count);
> +		xe->info.dma_mask_size, xe->info.tile_count,
> +		xe->info.has_heci_gscfi);
>  
>  	drm_dbg(&xe->drm, "Stepping = (G:%s, M:%s, D:%s, B:%s)\n",
>  		xe_step_name(xe->info.step.graphics),
> -- 
> 2.34.1
> 

[PATCH v2 3/4] drm/xe/gsc: add gsc device support

[Alexander Usyskin]

From: Vitaly Lubart <vitaly.lubart@intel.com>

Create mei-gscfi auxiliary device and configure interrupts
to be consumed by mei-gsc device driver.

Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/gpu/drm/xe/Kconfig           |   1 +
 drivers/gpu/drm/xe/Makefile          |   1 +
 drivers/gpu/drm/xe/xe_device.c       |   4 +
 drivers/gpu/drm/xe/xe_device_types.h |   4 +
 drivers/gpu/drm/xe/xe_heci_gsc.c     | 205 +++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_heci_gsc.h     |  35 +++++
 drivers/gpu/drm/xe/xe_irq.c          |  14 +-
 7 files changed, 262 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/xe/xe_heci_gsc.c
 create mode 100644 drivers/gpu/drm/xe/xe_heci_gsc.h

diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
index 096bd066afa8..da82084fe236 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -37,6 +37,7 @@ config DRM_XE
 	select DRM_SCHED
 	select MMU_NOTIFIER
 	select WANT_DEV_COREDUMP
+	select AUXILIARY_BUS
 	help
 	  Experimental driver for Intel Xe series GPUs
 
diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 9d2311f8141f..fbdb28fa5ace 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -73,6 +73,7 @@ xe-y += xe_bb.o \
 	xe_guc_log.o \
 	xe_guc_pc.o \
 	xe_guc_submit.o \
+	xe_heci_gsc.o \
 	xe_hw_engine.o \
 	xe_hw_engine_class_sysfs.o \
 	xe_hw_fence.o \
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index d6fc06d4c9dc..4d6e2f2b281f 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -323,6 +323,8 @@ int xe_device_probe(struct xe_device *xe)
 			goto err_irq_shutdown;
 	}
 
+	xe_heci_gsc_init(xe);
+
 	err = xe_display_init(xe);
 	if (err)
 		goto err_irq_shutdown;
@@ -365,6 +367,8 @@ void xe_device_remove(struct xe_device *xe)
 
 	xe_display_fini(xe);
 
+	xe_heci_gsc_fini(xe);
+
 	xe_irq_shutdown(xe);
 }
 
diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index 1d1fe53fc30d..80233c2f0d81 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -13,6 +13,7 @@
 #include <drm/ttm/ttm_device.h>
 
 #include "xe_devcoredump_types.h"
+#include "xe_heci_gsc.h"
 #include "xe_gt_types.h"
 #include "xe_platform_types.h"
 #include "xe_step_types.h"
@@ -364,6 +365,9 @@ struct xe_device {
 	 */
 	struct task_struct *pm_callback_task;
 
+	/** @gsc: graphics security controller */
+	struct xe_heci_gsc heci_gsc;
+
 	/* private: */
 
 #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
diff --git a/drivers/gpu/drm/xe/xe_heci_gsc.c b/drivers/gpu/drm/xe/xe_heci_gsc.c
new file mode 100644
index 000000000000..1eca1c46f257
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_heci_gsc.c
@@ -0,0 +1,205 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright(c) 2023, Intel Corporation. All rights reserved.
+ */
+
+#include <linux/irq.h>
+#include <linux/mei_aux.h>
+#include <linux/pci.h>
+#include <linux/sizes.h>
+
+#include "regs/xe_regs.h"
+#include "xe_device_types.h"
+#include "xe_drv.h"
+#include "xe_heci_gsc.h"
+#include "xe_platform_types.h"
+
+#define GSC_BAR_LENGTH  0x00000FFC
+
+static void heci_gsc_irq_mask(struct irq_data *d)
+{
+	/* generic irq handling */
+}
+
+static void heci_gsc_irq_unmask(struct irq_data *d)
+{
+	/* generic irq handling */
+}
+
+static struct irq_chip heci_gsc_irq_chip = {
+	.name = "gsc_irq_chip",
+	.irq_mask = heci_gsc_irq_mask,
+	.irq_unmask = heci_gsc_irq_unmask,
+};
+
+static int heci_gsc_irq_init(int irq)
+{
+	irq_set_chip_and_handler_name(irq, &heci_gsc_irq_chip,
+				      handle_simple_irq, "heci_gsc_irq_handler");
+
+	return irq_set_chip_data(irq, NULL);
+}
+
+/**
+ * struct heci_gsc_def - graphics security controller heci interface definitions
+ *
+ * @name: name of the heci device
+ * @bar: address of the mmio bar
+ * @bar_size: size of the mmio bar
+ * @use_polling: indication of using polling mode for the device
+ * @slow_firmware: indication of whether the device is slow (needs longer timeouts)
+ */
+struct heci_gsc_def {
+	const char *name;
+	unsigned long bar;
+	size_t bar_size;
+	bool use_polling;
+	bool slow_firmware;
+};
+
+/* gsc resources and definitions */
+static const struct heci_gsc_def heci_gsc_def_dg1 = {
+	.name = "mei-gscfi",
+	.bar = DG1_GSC_HECI2_BASE,
+	.bar_size = GSC_BAR_LENGTH,
+};
+
+static const struct heci_gsc_def heci_gsc_def_dg2 = {
+	.name = "mei-gscfi",
+	.bar = DG2_GSC_HECI2_BASE,
+	.bar_size = GSC_BAR_LENGTH,
+};
+
+static void heci_gsc_release_dev(struct device *dev)
+{
+	struct auxiliary_device *aux_dev = to_auxiliary_dev(dev);
+	struct mei_aux_device *adev = auxiliary_dev_to_mei_aux_dev(aux_dev);
+
+	kfree(adev);
+}
+
+void xe_heci_gsc_fini(struct xe_device *xe)
+{
+	struct xe_heci_gsc *heci_gsc = &xe->heci_gsc;
+
+	if (!HAS_HECI_GSCFI(xe))
+		return;
+
+	if (heci_gsc->adev) {
+		struct auxiliary_device *aux_dev = &heci_gsc->adev->aux_dev;
+
+		auxiliary_device_delete(aux_dev);
+		auxiliary_device_uninit(aux_dev);
+		heci_gsc->adev = NULL;
+	}
+
+	if (heci_gsc->irq >= 0)
+		irq_free_desc(heci_gsc->irq);
+	heci_gsc->irq = -1;
+}
+
+void xe_heci_gsc_init(struct xe_device *xe)
+{
+	struct xe_heci_gsc *heci_gsc = &xe->heci_gsc;
+	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
+	struct mei_aux_device *adev;
+	struct auxiliary_device *aux_dev;
+	const struct heci_gsc_def *def;
+	int ret;
+
+	if (!HAS_HECI_GSCFI(xe))
+		return;
+
+	heci_gsc->irq = -1;
+
+	if (xe->info.platform == XE_DG1) {
+		def = &heci_gsc_def_dg1;
+	} else if (xe->info.platform == XE_DG2) {
+		def = &heci_gsc_def_dg2;
+	} else {
+		drm_warn_once(&xe->drm, "Unknown platform\n");
+		return;
+	}
+
+	if (!def->name) {
+		drm_warn_once(&xe->drm, "HECI is not implemented!\n");
+		return;
+	}
+
+	/* skip irq initialization */
+	if (def->use_polling)
+		goto add_device;
+
+	heci_gsc->irq = irq_alloc_desc(0);
+	if (heci_gsc->irq < 0) {
+		drm_err(&xe->drm, "gsc irq error %d\n", heci_gsc->irq);
+		goto fail;
+	}
+
+	ret = heci_gsc_irq_init(heci_gsc->irq);
+	if (ret < 0) {
+		drm_err(&xe->drm, "gsc irq init failed %d\n", ret);
+		goto fail;
+	}
+
+add_device:
+	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+	if (!adev)
+		goto fail;
+	adev->irq = heci_gsc->irq;
+	adev->bar.parent = &pdev->resource[0];
+	adev->bar.start = def->bar + pdev->resource[0].start;
+	adev->bar.end = adev->bar.start + def->bar_size - 1;
+	adev->bar.flags = IORESOURCE_MEM;
+	adev->bar.desc = IORES_DESC_NONE;
+	adev->slow_firmware = def->slow_firmware;
+
+	aux_dev = &adev->aux_dev;
+	aux_dev->name = def->name;
+	aux_dev->id = (pci_domain_nr(pdev->bus) << 16) |
+		      PCI_DEVID(pdev->bus->number, pdev->devfn);
+	aux_dev->dev.parent = &pdev->dev;
+	aux_dev->dev.release = heci_gsc_release_dev;
+
+	ret = auxiliary_device_init(aux_dev);
+	if (ret < 0) {
+		drm_err(&xe->drm, "gsc aux init failed %d\n", ret);
+		kfree(adev);
+		goto fail;
+	}
+
+	heci_gsc->adev = adev; /* needed by the notifier */
+	ret = auxiliary_device_add(aux_dev);
+	if (ret < 0) {
+		drm_err(&xe->drm, "gsc aux add failed %d\n", ret);
+		heci_gsc->adev = NULL;
+
+		/* adev will be freed with the put_device() and .release sequence */
+		auxiliary_device_uninit(aux_dev);
+		goto fail;
+	}
+
+	return;
+fail:
+	xe_heci_gsc_fini(xe);
+}
+
+void xe_heci_gsc_irq_handler(struct xe_device *xe, u32 iir)
+{
+	int ret;
+
+	if ((iir & GSC_IRQ_INTF(1)) == 0)
+		return;
+
+	if (!HAS_HECI_GSCFI(xe)) {
+		drm_warn_once(&xe->drm, "GSC irq: not supported");
+		return;
+	}
+
+	if (xe->heci_gsc.irq < 0)
+		return;
+
+	ret = generic_handle_irq(xe->heci_gsc.irq);
+	if (ret)
+		drm_err_ratelimited(&xe->drm, "error handling GSC irq: %d\n", ret);
+}
diff --git a/drivers/gpu/drm/xe/xe_heci_gsc.h b/drivers/gpu/drm/xe/xe_heci_gsc.h
new file mode 100644
index 000000000000..9db454478fae
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_heci_gsc.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright(c) 2023, Intel Corporation. All rights reserved.
+ */
+#ifndef __XE_HECI_GSC_DEV_H__
+#define __XE_HECI_GSC_DEV_H__
+
+#include <linux/types.h>
+
+struct xe_device;
+struct mei_aux_device;
+
+/*
+ * The HECI1 bit corresponds to bit15 and HECI2 to bit14.
+ * The reason for this is to allow growth for more interfaces in the future.
+ */
+#define GSC_IRQ_INTF(_x)  BIT(15 - (_x))
+
+/**
+ * struct xe_heci_gsc - graphics security controller for xe, HECI interface
+ *
+ * @adev : pointer to mei auxiliary device structure
+ * @irq : irq number
+ *
+ */
+struct xe_heci_gsc {
+	struct mei_aux_device *adev;
+	int irq;
+};
+
+void xe_heci_gsc_init(struct xe_device *xe);
+void xe_heci_gsc_fini(struct xe_device *xe);
+void xe_heci_gsc_irq_handler(struct xe_device *xe, u32 iir);
+
+#endif /* __XE_HECI_GSC_DEV_H__ */
diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
index 1dee3e832eb5..d297e9b8a3be 100644
--- a/drivers/gpu/drm/xe/xe_irq.c
+++ b/drivers/gpu/drm/xe/xe_irq.c
@@ -128,6 +128,7 @@ void xe_irq_enable_hwe(struct xe_gt *gt)
 	struct xe_device *xe = gt_to_xe(gt);
 	u32 ccs_mask, bcs_mask;
 	u32 irqs, dmask, smask;
+	u32 gsc_mask = GSC_IRQ_INTF(1);
 
 	if (xe_device_guc_submission_enabled(xe)) {
 		irqs = GT_RENDER_USER_INTERRUPT |
@@ -180,6 +181,9 @@ void xe_irq_enable_hwe(struct xe_gt *gt)
 		if (xe_hw_engine_mask_per_class(gt, XE_ENGINE_CLASS_OTHER)) {
 			xe_mmio_write32(gt, GUNIT_GSC_INTR_ENABLE, irqs);
 			xe_mmio_write32(gt, GUNIT_GSC_INTR_MASK, ~irqs);
+		} else if (HAS_HECI_GSCFI(xe)) {
+			xe_mmio_write32(gt, GUNIT_GSC_INTR_ENABLE, gsc_mask);
+			xe_mmio_write32(gt, GUNIT_GSC_INTR_MASK, ~gsc_mask);
 		}
 	}
 }
@@ -284,6 +288,11 @@ static void gt_irq_handler(struct xe_tile *tile,
 			instance = INTR_ENGINE_INSTANCE(identity[bit]);
 			intr_vec = INTR_ENGINE_INTR(identity[bit]);
 
+			if (class == XE_ENGINE_CLASS_OTHER && instance == OTHER_GSC_INSTANCE) {
+				xe_heci_gsc_irq_handler(xe, intr_vec);
+				continue;
+			}
+
 			engine_gt = pick_engine_gt(tile, class, instance);
 
 			hwe = xe_gt_hw_engine(engine_gt, class, instance, false);
@@ -470,8 +479,9 @@ static void gt_irq_reset(struct xe_tile *tile)
 	if (ccs_mask & (BIT(2)|BIT(3)))
 		xe_mmio_write32(mmio,  CCS2_CCS3_INTR_MASK, ~0);
 
-	if (tile->media_gt &&
-	    xe_hw_engine_mask_per_class(tile->media_gt, XE_ENGINE_CLASS_OTHER)) {
+	if ((tile->media_gt &&
+	     xe_hw_engine_mask_per_class(tile->media_gt, XE_ENGINE_CLASS_OTHER)) ||
+	    HAS_HECI_GSCFI(tile_to_xe(tile))) {
 		xe_mmio_write32(mmio, GUNIT_GSC_INTR_ENABLE, 0);
 		xe_mmio_write32(mmio, GUNIT_GSC_INTR_MASK, ~0);
 	}
-- 
2.34.1

Re: [Intel-xe] [PATCH v2 3/4] drm/xe/gsc: add gsc device support

[Rodrigo Vivi]

On Thu, Sep 14, 2023 at 11:01:37AM +0300, Alexander Usyskin wrote:
> From: Vitaly Lubart <vitaly.lubart@intel.com>
> 
> Create mei-gscfi auxiliary device and configure interrupts
> to be consumed by mei-gsc device driver.
> 
> Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> ---
>  drivers/gpu/drm/xe/Kconfig           |   1 +
>  drivers/gpu/drm/xe/Makefile          |   1 +
>  drivers/gpu/drm/xe/xe_device.c       |   4 +
>  drivers/gpu/drm/xe/xe_device_types.h |   4 +
>  drivers/gpu/drm/xe/xe_heci_gsc.c     | 205 +++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_heci_gsc.h     |  35 +++++
>  drivers/gpu/drm/xe/xe_irq.c          |  14 +-
>  7 files changed, 262 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/gpu/drm/xe/xe_heci_gsc.c
>  create mode 100644 drivers/gpu/drm/xe/xe_heci_gsc.h
> 
> diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
> index 096bd066afa8..da82084fe236 100644
> --- a/drivers/gpu/drm/xe/Kconfig
> +++ b/drivers/gpu/drm/xe/Kconfig
> @@ -37,6 +37,7 @@ config DRM_XE
>  	select DRM_SCHED
>  	select MMU_NOTIFIER
>  	select WANT_DEV_COREDUMP
> +	select AUXILIARY_BUS
>  	help
>  	  Experimental driver for Intel Xe series GPUs
>  
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 9d2311f8141f..fbdb28fa5ace 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -73,6 +73,7 @@ xe-y += xe_bb.o \
>  	xe_guc_log.o \
>  	xe_guc_pc.o \
>  	xe_guc_submit.o \
> +	xe_heci_gsc.o \
>  	xe_hw_engine.o \
>  	xe_hw_engine_class_sysfs.o \
>  	xe_hw_fence.o \
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index d6fc06d4c9dc..4d6e2f2b281f 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -323,6 +323,8 @@ int xe_device_probe(struct xe_device *xe)
>  			goto err_irq_shutdown;
>  	}
>  
> +	xe_heci_gsc_init(xe);
> +

could we place this call earlier in the flow? maybe right after setting up mmio?
or maybe after pcode init where we confirmed the boot can proceed on discrete?

>  	err = xe_display_init(xe);
>  	if (err)
>  		goto err_irq_shutdown;
> @@ -365,6 +367,8 @@ void xe_device_remove(struct xe_device *xe)
>  
>  	xe_display_fini(xe);
>  
> +	xe_heci_gsc_fini(xe);
> +
>  	xe_irq_shutdown(xe);
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 1d1fe53fc30d..80233c2f0d81 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -13,6 +13,7 @@
>  #include <drm/ttm/ttm_device.h>
>  
>  #include "xe_devcoredump_types.h"
> +#include "xe_heci_gsc.h"
>  #include "xe_gt_types.h"
>  #include "xe_platform_types.h"
>  #include "xe_step_types.h"
> @@ -364,6 +365,9 @@ struct xe_device {
>  	 */
>  	struct task_struct *pm_callback_task;
>  
> +	/** @gsc: graphics security controller */
> +	struct xe_heci_gsc heci_gsc;

documentation doesn't match variable name.

> +
>  	/* private: */
>  
>  #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
> diff --git a/drivers/gpu/drm/xe/xe_heci_gsc.c b/drivers/gpu/drm/xe/xe_heci_gsc.c
> new file mode 100644
> index 000000000000..1eca1c46f257
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_heci_gsc.c
> @@ -0,0 +1,205 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright(c) 2023, Intel Corporation. All rights reserved.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/mei_aux.h>
> +#include <linux/pci.h>
> +#include <linux/sizes.h>
> +
> +#include "regs/xe_regs.h"
> +#include "xe_device_types.h"
> +#include "xe_drv.h"
> +#include "xe_heci_gsc.h"
> +#include "xe_platform_types.h"
> +
> +#define GSC_BAR_LENGTH  0x00000FFC
> +
> +static void heci_gsc_irq_mask(struct irq_data *d)
> +{
> +	/* generic irq handling */
> +}
> +
> +static void heci_gsc_irq_unmask(struct irq_data *d)
> +{
> +	/* generic irq handling */
> +}
> +
> +static struct irq_chip heci_gsc_irq_chip = {
> +	.name = "gsc_irq_chip",
> +	.irq_mask = heci_gsc_irq_mask,
> +	.irq_unmask = heci_gsc_irq_unmask,
> +};
> +
> +static int heci_gsc_irq_init(int irq)
> +{
> +	irq_set_chip_and_handler_name(irq, &heci_gsc_irq_chip,
> +				      handle_simple_irq, "heci_gsc_irq_handler");
> +
> +	return irq_set_chip_data(irq, NULL);
> +}
> +
> +/**
> + * struct heci_gsc_def - graphics security controller heci interface definitions
> + *
> + * @name: name of the heci device
> + * @bar: address of the mmio bar
> + * @bar_size: size of the mmio bar
> + * @use_polling: indication of using polling mode for the device
> + * @slow_firmware: indication of whether the device is slow (needs longer timeouts)
> + */
> +struct heci_gsc_def {
> +	const char *name;
> +	unsigned long bar;
> +	size_t bar_size;
> +	bool use_polling;
> +	bool slow_firmware;

will we need the lmem_size here?

or what's the difference on the mei-gsc and mei-gscfi exactly?

> +};
> +
> +/* gsc resources and definitions */
> +static const struct heci_gsc_def heci_gsc_def_dg1 = {
> +	.name = "mei-gscfi",
> +	.bar = DG1_GSC_HECI2_BASE,
> +	.bar_size = GSC_BAR_LENGTH,
> +};
> +
> +static const struct heci_gsc_def heci_gsc_def_dg2 = {
> +	.name = "mei-gscfi",
> +	.bar = DG2_GSC_HECI2_BASE,
> +	.bar_size = GSC_BAR_LENGTH,
> +};
> +
> +static void heci_gsc_release_dev(struct device *dev)
> +{
> +	struct auxiliary_device *aux_dev = to_auxiliary_dev(dev);
> +	struct mei_aux_device *adev = auxiliary_dev_to_mei_aux_dev(aux_dev);
> +
> +	kfree(adev);
> +}
> +
> +void xe_heci_gsc_fini(struct xe_device *xe)
> +{
> +	struct xe_heci_gsc *heci_gsc = &xe->heci_gsc;
> +
> +	if (!HAS_HECI_GSCFI(xe))
> +		return;
> +
> +	if (heci_gsc->adev) {
> +		struct auxiliary_device *aux_dev = &heci_gsc->adev->aux_dev;
> +
> +		auxiliary_device_delete(aux_dev);
> +		auxiliary_device_uninit(aux_dev);
> +		heci_gsc->adev = NULL;
> +	}
> +
> +	if (heci_gsc->irq >= 0)
> +		irq_free_desc(heci_gsc->irq);
> +	heci_gsc->irq = -1;
> +}
> +
> +void xe_heci_gsc_init(struct xe_device *xe)
> +{
> +	struct xe_heci_gsc *heci_gsc = &xe->heci_gsc;
> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> +	struct mei_aux_device *adev;
> +	struct auxiliary_device *aux_dev;
> +	const struct heci_gsc_def *def;
> +	int ret;
> +
> +	if (!HAS_HECI_GSCFI(xe))
> +		return;
> +
> +	heci_gsc->irq = -1;
> +
> +	if (xe->info.platform == XE_DG1) {
> +		def = &heci_gsc_def_dg1;
> +	} else if (xe->info.platform == XE_DG2) {
> +		def = &heci_gsc_def_dg2;

in general it is better to add the most recent on top of the oldest.

> +	} else {
> +		drm_warn_once(&xe->drm, "Unknown platform\n");
> +		return;
> +	}
> +
> +	if (!def->name) {
> +		drm_warn_once(&xe->drm, "HECI is not implemented!\n");
> +		return;
> +	}
> +
> +	/* skip irq initialization */
> +	if (def->use_polling)
> +		goto add_device;

what about creating a function
heci_gsc_irq_setup(...)

and then
if (!def->use_polling) {
   ret = heci_gsc_irq_setup(...)
   if (ret)
      goto fail;

> +
> +	heci_gsc->irq = irq_alloc_desc(0);
> +	if (heci_gsc->irq < 0) {
> +		drm_err(&xe->drm, "gsc irq error %d\n", heci_gsc->irq);
> +		goto fail;
> +	}
> +
> +	ret = heci_gsc_irq_init(heci_gsc->irq);
> +	if (ret < 0) {
> +		drm_err(&xe->drm, "gsc irq init failed %d\n", ret);
> +		goto fail;
> +	}
> +
> +add_device:

it looks like this add_device always deserve a dedicated function.

> +	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
> +	if (!adev)
> +		goto fail;
> +	adev->irq = heci_gsc->irq;
> +	adev->bar.parent = &pdev->resource[0];
> +	adev->bar.start = def->bar + pdev->resource[0].start;
> +	adev->bar.end = adev->bar.start + def->bar_size - 1;
> +	adev->bar.flags = IORESOURCE_MEM;
> +	adev->bar.desc = IORES_DESC_NONE;
> +	adev->slow_firmware = def->slow_firmware;
> +
> +	aux_dev = &adev->aux_dev;
> +	aux_dev->name = def->name;
> +	aux_dev->id = (pci_domain_nr(pdev->bus) << 16) |
> +		      PCI_DEVID(pdev->bus->number, pdev->devfn);
> +	aux_dev->dev.parent = &pdev->dev;
> +	aux_dev->dev.release = heci_gsc_release_dev;
> +
> +	ret = auxiliary_device_init(aux_dev);
> +	if (ret < 0) {
> +		drm_err(&xe->drm, "gsc aux init failed %d\n", ret);
> +		kfree(adev);
> +		goto fail;
> +	}
> +
> +	heci_gsc->adev = adev; /* needed by the notifier */
> +	ret = auxiliary_device_add(aux_dev);
> +	if (ret < 0) {
> +		drm_err(&xe->drm, "gsc aux add failed %d\n", ret);
> +		heci_gsc->adev = NULL;
> +
> +		/* adev will be freed with the put_device() and .release sequence */
> +		auxiliary_device_uninit(aux_dev);
> +		goto fail;
> +	}
> +
> +	return;
> +fail:
> +	xe_heci_gsc_fini(xe);
> +}
> +
> +void xe_heci_gsc_irq_handler(struct xe_device *xe, u32 iir)
> +{
> +	int ret;
> +
> +	if ((iir & GSC_IRQ_INTF(1)) == 0)
> +		return;
> +
> +	if (!HAS_HECI_GSCFI(xe)) {
> +		drm_warn_once(&xe->drm, "GSC irq: not supported");
> +		return;
> +	}
> +
> +	if (xe->heci_gsc.irq < 0)
> +		return;
> +
> +	ret = generic_handle_irq(xe->heci_gsc.irq);
> +	if (ret)
> +		drm_err_ratelimited(&xe->drm, "error handling GSC irq: %d\n", ret);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_heci_gsc.h b/drivers/gpu/drm/xe/xe_heci_gsc.h
> new file mode 100644
> index 000000000000..9db454478fae
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_heci_gsc.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright(c) 2023, Intel Corporation. All rights reserved.
> + */
> +#ifndef __XE_HECI_GSC_DEV_H__
> +#define __XE_HECI_GSC_DEV_H__
> +
> +#include <linux/types.h>
> +
> +struct xe_device;
> +struct mei_aux_device;
> +
> +/*
> + * The HECI1 bit corresponds to bit15 and HECI2 to bit14.
> + * The reason for this is to allow growth for more interfaces in the future.
> + */
> +#define GSC_IRQ_INTF(_x)  BIT(15 - (_x))
> +
> +/**
> + * struct xe_heci_gsc - graphics security controller for xe, HECI interface
> + *
> + * @adev : pointer to mei auxiliary device structure
> + * @irq : irq number
> + *
> + */
> +struct xe_heci_gsc {
> +	struct mei_aux_device *adev;
> +	int irq;
> +};
> +
> +void xe_heci_gsc_init(struct xe_device *xe);
> +void xe_heci_gsc_fini(struct xe_device *xe);
> +void xe_heci_gsc_irq_handler(struct xe_device *xe, u32 iir);
> +
> +#endif /* __XE_HECI_GSC_DEV_H__ */
> diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
> index 1dee3e832eb5..d297e9b8a3be 100644
> --- a/drivers/gpu/drm/xe/xe_irq.c
> +++ b/drivers/gpu/drm/xe/xe_irq.c
> @@ -128,6 +128,7 @@ void xe_irq_enable_hwe(struct xe_gt *gt)
>  	struct xe_device *xe = gt_to_xe(gt);
>  	u32 ccs_mask, bcs_mask;
>  	u32 irqs, dmask, smask;
> +	u32 gsc_mask = GSC_IRQ_INTF(1);
>  
>  	if (xe_device_guc_submission_enabled(xe)) {
>  		irqs = GT_RENDER_USER_INTERRUPT |
> @@ -180,6 +181,9 @@ void xe_irq_enable_hwe(struct xe_gt *gt)
>  		if (xe_hw_engine_mask_per_class(gt, XE_ENGINE_CLASS_OTHER)) {
>  			xe_mmio_write32(gt, GUNIT_GSC_INTR_ENABLE, irqs);
>  			xe_mmio_write32(gt, GUNIT_GSC_INTR_MASK, ~irqs);
> +		} else if (HAS_HECI_GSCFI(xe)) {
> +			xe_mmio_write32(gt, GUNIT_GSC_INTR_ENABLE, gsc_mask);
> +			xe_mmio_write32(gt, GUNIT_GSC_INTR_MASK, ~gsc_mask);

is there any way we could combine this with the upper calls to the same register?
I believe i915 is combining them for instance...

>  		}
>  	}
>  }
> @@ -284,6 +288,11 @@ static void gt_irq_handler(struct xe_tile *tile,
>  			instance = INTR_ENGINE_INSTANCE(identity[bit]);
>  			intr_vec = INTR_ENGINE_INTR(identity[bit]);
>  
> +			if (class == XE_ENGINE_CLASS_OTHER && instance == OTHER_GSC_INSTANCE) {
> +				xe_heci_gsc_irq_handler(xe, intr_vec);
> +				continue;
> +			}
> +
>  			engine_gt = pick_engine_gt(tile, class, instance);
>  
>  			hwe = xe_gt_hw_engine(engine_gt, class, instance, false);
> @@ -470,8 +479,9 @@ static void gt_irq_reset(struct xe_tile *tile)
>  	if (ccs_mask & (BIT(2)|BIT(3)))
>  		xe_mmio_write32(mmio,  CCS2_CCS3_INTR_MASK, ~0);
>  
> -	if (tile->media_gt &&
> -	    xe_hw_engine_mask_per_class(tile->media_gt, XE_ENGINE_CLASS_OTHER)) {
> +	if ((tile->media_gt &&
> +	     xe_hw_engine_mask_per_class(tile->media_gt, XE_ENGINE_CLASS_OTHER)) ||
> +	    HAS_HECI_GSCFI(tile_to_xe(tile))) {
>  		xe_mmio_write32(mmio, GUNIT_GSC_INTR_ENABLE, 0);
>  		xe_mmio_write32(mmio, GUNIT_GSC_INTR_MASK, ~0);
>  	}
> -- 
> 2.34.1
> 

RE: [Intel-xe] [PATCH v2 3/4] drm/xe/gsc: add gsc device support

[Usyskin, Alexander]

> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Sent: Monday, September 18, 2023 19:25
> To: Usyskin, Alexander <alexander.usyskin@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; De Marchi, Lucas
> <lucas.demarchi@intel.com>; Ceraolo Spurio, Daniele
> <daniele.ceraolospurio@intel.com>; linux-kernel@vger.kernel.org; Winkler,
> Tomas <tomas.winkler@intel.com>; intel-xe@lists.freedesktop.org
> Subject: Re: [Intel-xe] [PATCH v2 3/4] drm/xe/gsc: add gsc device support
> 
> On Thu, Sep 14, 2023 at 11:01:37AM +0300, Alexander Usyskin wrote:
> > From: Vitaly Lubart <vitaly.lubart@intel.com>
> >
> > Create mei-gscfi auxiliary device and configure interrupts
> > to be consumed by mei-gsc device driver.
> >
> > Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
> > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > ---
> >  drivers/gpu/drm/xe/Kconfig           |   1 +
> >  drivers/gpu/drm/xe/Makefile          |   1 +
> >  drivers/gpu/drm/xe/xe_device.c       |   4 +
> >  drivers/gpu/drm/xe/xe_device_types.h |   4 +
> >  drivers/gpu/drm/xe/xe_heci_gsc.c     | 205
> +++++++++++++++++++++++++++
> >  drivers/gpu/drm/xe/xe_heci_gsc.h     |  35 +++++
> >  drivers/gpu/drm/xe/xe_irq.c          |  14 +-
> >  7 files changed, 262 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/gpu/drm/xe/xe_heci_gsc.c
> >  create mode 100644 drivers/gpu/drm/xe/xe_heci_gsc.h
> >
> > diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
> > index 096bd066afa8..da82084fe236 100644
> > --- a/drivers/gpu/drm/xe/Kconfig
> > +++ b/drivers/gpu/drm/xe/Kconfig
> > @@ -37,6 +37,7 @@ config DRM_XE
> >  	select DRM_SCHED
> >  	select MMU_NOTIFIER
> >  	select WANT_DEV_COREDUMP
> > +	select AUXILIARY_BUS
> >  	help
> >  	  Experimental driver for Intel Xe series GPUs
> >
> > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> > index 9d2311f8141f..fbdb28fa5ace 100644
> > --- a/drivers/gpu/drm/xe/Makefile
> > +++ b/drivers/gpu/drm/xe/Makefile
> > @@ -73,6 +73,7 @@ xe-y += xe_bb.o \
> >  	xe_guc_log.o \
> >  	xe_guc_pc.o \
> >  	xe_guc_submit.o \
> > +	xe_heci_gsc.o \
> >  	xe_hw_engine.o \
> >  	xe_hw_engine_class_sysfs.o \
> >  	xe_hw_fence.o \
> > diff --git a/drivers/gpu/drm/xe/xe_device.c
> b/drivers/gpu/drm/xe/xe_device.c
> > index d6fc06d4c9dc..4d6e2f2b281f 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -323,6 +323,8 @@ int xe_device_probe(struct xe_device *xe)
> >  			goto err_irq_shutdown;
> >  	}
> >
> > +	xe_heci_gsc_init(xe);
> > +
> 
> could we place this call earlier in the flow? maybe right after setting up mmio?
> or maybe after pcode init where we confirmed the boot can proceed on
> discrete?

Right after xe_irq_install will be ok (as we need working irq to setup)?
Will try, if this works.

> 
> >  	err = xe_display_init(xe);
> >  	if (err)
> >  		goto err_irq_shutdown;
> > @@ -365,6 +367,8 @@ void xe_device_remove(struct xe_device *xe)
> >
> >  	xe_display_fini(xe);
> >
> > +	xe_heci_gsc_fini(xe);
> > +
> >  	xe_irq_shutdown(xe);
> >  }
> >
> > diff --git a/drivers/gpu/drm/xe/xe_device_types.h
> b/drivers/gpu/drm/xe/xe_device_types.h
> > index 1d1fe53fc30d..80233c2f0d81 100644
> > --- a/drivers/gpu/drm/xe/xe_device_types.h
> > +++ b/drivers/gpu/drm/xe/xe_device_types.h
> > @@ -13,6 +13,7 @@
> >  #include <drm/ttm/ttm_device.h>
> >
> >  #include "xe_devcoredump_types.h"
> > +#include "xe_heci_gsc.h"
> >  #include "xe_gt_types.h"
> >  #include "xe_platform_types.h"
> >  #include "xe_step_types.h"
> > @@ -364,6 +365,9 @@ struct xe_device {
> >  	 */
> >  	struct task_struct *pm_callback_task;
> >
> > +	/** @gsc: graphics security controller */
> > +	struct xe_heci_gsc heci_gsc;
> 
> documentation doesn't match variable name.

Will fix in V2

> 
> > +
> >  	/* private: */
> >
> >  #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
> > diff --git a/drivers/gpu/drm/xe/xe_heci_gsc.c
> b/drivers/gpu/drm/xe/xe_heci_gsc.c
> > new file mode 100644
> > index 000000000000..1eca1c46f257
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_heci_gsc.c
> > @@ -0,0 +1,205 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright(c) 2023, Intel Corporation. All rights reserved.
> > + */
> > +
> > +#include <linux/irq.h>
> > +#include <linux/mei_aux.h>
> > +#include <linux/pci.h>
> > +#include <linux/sizes.h>
> > +
> > +#include "regs/xe_regs.h"
> > +#include "xe_device_types.h"
> > +#include "xe_drv.h"
> > +#include "xe_heci_gsc.h"
> > +#include "xe_platform_types.h"
> > +
> > +#define GSC_BAR_LENGTH  0x00000FFC
> > +
> > +static void heci_gsc_irq_mask(struct irq_data *d)
> > +{
> > +	/* generic irq handling */
> > +}
> > +
> > +static void heci_gsc_irq_unmask(struct irq_data *d)
> > +{
> > +	/* generic irq handling */
> > +}
> > +
> > +static struct irq_chip heci_gsc_irq_chip = {
> > +	.name = "gsc_irq_chip",
> > +	.irq_mask = heci_gsc_irq_mask,
> > +	.irq_unmask = heci_gsc_irq_unmask,
> > +};
> > +
> > +static int heci_gsc_irq_init(int irq)
> > +{
> > +	irq_set_chip_and_handler_name(irq, &heci_gsc_irq_chip,
> > +				      handle_simple_irq,
> "heci_gsc_irq_handler");
> > +
> > +	return irq_set_chip_data(irq, NULL);
> > +}
> > +
> > +/**
> > + * struct heci_gsc_def - graphics security controller heci interface definitions
> > + *
> > + * @name: name of the heci device
> > + * @bar: address of the mmio bar
> > + * @bar_size: size of the mmio bar
> > + * @use_polling: indication of using polling mode for the device
> > + * @slow_firmware: indication of whether the device is slow (needs longer
> timeouts)
> > + */
> > +struct heci_gsc_def {
> > +	const char *name;
> > +	unsigned long bar;
> > +	size_t bar_size;
> > +	bool use_polling;
> > +	bool slow_firmware;
> 
> will we need the lmem_size here?
> 
PXP is not planned now, so will not need.

> or what's the difference on the mei-gsc and mei-gscfi exactly?
>

GSC sports two HECI heads - one for PXP (we call created device mei-gsc), another for chassis (mei-gscfi)
Here only second head is supported, as PXP is not planned now.

> > +};
> > +
> > +/* gsc resources and definitions */
> > +static const struct heci_gsc_def heci_gsc_def_dg1 = {
> > +	.name = "mei-gscfi",
> > +	.bar = DG1_GSC_HECI2_BASE,
> > +	.bar_size = GSC_BAR_LENGTH,
> > +};
> > +
> > +static const struct heci_gsc_def heci_gsc_def_dg2 = {
> > +	.name = "mei-gscfi",
> > +	.bar = DG2_GSC_HECI2_BASE,
> > +	.bar_size = GSC_BAR_LENGTH,
> > +};
> > +
> > +static void heci_gsc_release_dev(struct device *dev)
> > +{
> > +	struct auxiliary_device *aux_dev = to_auxiliary_dev(dev);
> > +	struct mei_aux_device *adev =
> auxiliary_dev_to_mei_aux_dev(aux_dev);
> > +
> > +	kfree(adev);
> > +}
> > +
> > +void xe_heci_gsc_fini(struct xe_device *xe)
> > +{
> > +	struct xe_heci_gsc *heci_gsc = &xe->heci_gsc;
> > +
> > +	if (!HAS_HECI_GSCFI(xe))
> > +		return;
> > +
> > +	if (heci_gsc->adev) {
> > +		struct auxiliary_device *aux_dev = &heci_gsc->adev->aux_dev;
> > +
> > +		auxiliary_device_delete(aux_dev);
> > +		auxiliary_device_uninit(aux_dev);
> > +		heci_gsc->adev = NULL;
> > +	}
> > +
> > +	if (heci_gsc->irq >= 0)
> > +		irq_free_desc(heci_gsc->irq);
> > +	heci_gsc->irq = -1;
> > +}
> > +
> > +void xe_heci_gsc_init(struct xe_device *xe)
> > +{
> > +	struct xe_heci_gsc *heci_gsc = &xe->heci_gsc;
> > +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> > +	struct mei_aux_device *adev;
> > +	struct auxiliary_device *aux_dev;
> > +	const struct heci_gsc_def *def;
> > +	int ret;
> > +
> > +	if (!HAS_HECI_GSCFI(xe))
> > +		return;
> > +
> > +	heci_gsc->irq = -1;
> > +
> > +	if (xe->info.platform == XE_DG1) {
> > +		def = &heci_gsc_def_dg1;
> > +	} else if (xe->info.platform == XE_DG2) {
> > +		def = &heci_gsc_def_dg2;
> 
> in general it is better to add the most recent on top of the oldest.

Ok, but this is exact copy from i915

> 
> > +	} else {
> > +		drm_warn_once(&xe->drm, "Unknown platform\n");
> > +		return;
> > +	}
> > +
> > +	if (!def->name) {
> > +		drm_warn_once(&xe->drm, "HECI is not implemented!\n");
> > +		return;
> > +	}
> > +
> > +	/* skip irq initialization */
> > +	if (def->use_polling)
> > +		goto add_device;
> 
> what about creating a function
> heci_gsc_irq_setup(...)
> 
> and then
> if (!def->use_polling) {
>    ret = heci_gsc_irq_setup(...)
>    if (ret)
>       goto fail;
> 
Will do, thx.

> > +
> > +	heci_gsc->irq = irq_alloc_desc(0);
> > +	if (heci_gsc->irq < 0) {
> > +		drm_err(&xe->drm, "gsc irq error %d\n", heci_gsc->irq);
> > +		goto fail;
> > +	}
> > +
> > +	ret = heci_gsc_irq_init(heci_gsc->irq);
> > +	if (ret < 0) {
> > +		drm_err(&xe->drm, "gsc irq init failed %d\n", ret);
> > +		goto fail;
> > +	}
> > +
> > +add_device:
> 
> it looks like this add_device always deserve a dedicated function.
> 
Sure, will create one

> > +	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
> > +	if (!adev)
> > +		goto fail;
> > +	adev->irq = heci_gsc->irq;
> > +	adev->bar.parent = &pdev->resource[0];
> > +	adev->bar.start = def->bar + pdev->resource[0].start;
> > +	adev->bar.end = adev->bar.start + def->bar_size - 1;
> > +	adev->bar.flags = IORESOURCE_MEM;
> > +	adev->bar.desc = IORES_DESC_NONE;
> > +	adev->slow_firmware = def->slow_firmware;
> > +
> > +	aux_dev = &adev->aux_dev;
> > +	aux_dev->name = def->name;
> > +	aux_dev->id = (pci_domain_nr(pdev->bus) << 16) |
> > +		      PCI_DEVID(pdev->bus->number, pdev->devfn);
> > +	aux_dev->dev.parent = &pdev->dev;
> > +	aux_dev->dev.release = heci_gsc_release_dev;
> > +
> > +	ret = auxiliary_device_init(aux_dev);
> > +	if (ret < 0) {
> > +		drm_err(&xe->drm, "gsc aux init failed %d\n", ret);
> > +		kfree(adev);
> > +		goto fail;
> > +	}
> > +
> > +	heci_gsc->adev = adev; /* needed by the notifier */
> > +	ret = auxiliary_device_add(aux_dev);
> > +	if (ret < 0) {
> > +		drm_err(&xe->drm, "gsc aux add failed %d\n", ret);
> > +		heci_gsc->adev = NULL;
> > +
> > +		/* adev will be freed with the put_device() and .release
> sequence */
> > +		auxiliary_device_uninit(aux_dev);
> > +		goto fail;
> > +	}
> > +
> > +	return;
> > +fail:
> > +	xe_heci_gsc_fini(xe);
> > +}
> > +
> > +void xe_heci_gsc_irq_handler(struct xe_device *xe, u32 iir)
> > +{
> > +	int ret;
> > +
> > +	if ((iir & GSC_IRQ_INTF(1)) == 0)
> > +		return;
> > +
> > +	if (!HAS_HECI_GSCFI(xe)) {
> > +		drm_warn_once(&xe->drm, "GSC irq: not supported");
> > +		return;
> > +	}
> > +
> > +	if (xe->heci_gsc.irq < 0)
> > +		return;
> > +
> > +	ret = generic_handle_irq(xe->heci_gsc.irq);
> > +	if (ret)
> > +		drm_err_ratelimited(&xe->drm, "error handling GSC irq:
> %d\n", ret);
> > +}
> > diff --git a/drivers/gpu/drm/xe/xe_heci_gsc.h
> b/drivers/gpu/drm/xe/xe_heci_gsc.h
> > new file mode 100644
> > index 000000000000..9db454478fae
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_heci_gsc.h
> > @@ -0,0 +1,35 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright(c) 2023, Intel Corporation. All rights reserved.
> > + */
> > +#ifndef __XE_HECI_GSC_DEV_H__
> > +#define __XE_HECI_GSC_DEV_H__
> > +
> > +#include <linux/types.h>
> > +
> > +struct xe_device;
> > +struct mei_aux_device;
> > +
> > +/*
> > + * The HECI1 bit corresponds to bit15 and HECI2 to bit14.
> > + * The reason for this is to allow growth for more interfaces in the future.
> > + */
> > +#define GSC_IRQ_INTF(_x)  BIT(15 - (_x))
> > +
> > +/**
> > + * struct xe_heci_gsc - graphics security controller for xe, HECI interface
> > + *
> > + * @adev : pointer to mei auxiliary device structure
> > + * @irq : irq number
> > + *
> > + */
> > +struct xe_heci_gsc {
> > +	struct mei_aux_device *adev;
> > +	int irq;
> > +};
> > +
> > +void xe_heci_gsc_init(struct xe_device *xe);
> > +void xe_heci_gsc_fini(struct xe_device *xe);
> > +void xe_heci_gsc_irq_handler(struct xe_device *xe, u32 iir);
> > +
> > +#endif /* __XE_HECI_GSC_DEV_H__ */
> > diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
> > index 1dee3e832eb5..d297e9b8a3be 100644
> > --- a/drivers/gpu/drm/xe/xe_irq.c
> > +++ b/drivers/gpu/drm/xe/xe_irq.c
> > @@ -128,6 +128,7 @@ void xe_irq_enable_hwe(struct xe_gt *gt)
> >  	struct xe_device *xe = gt_to_xe(gt);
> >  	u32 ccs_mask, bcs_mask;
> >  	u32 irqs, dmask, smask;
> > +	u32 gsc_mask = GSC_IRQ_INTF(1);
> >
> >  	if (xe_device_guc_submission_enabled(xe)) {
> >  		irqs = GT_RENDER_USER_INTERRUPT |
> > @@ -180,6 +181,9 @@ void xe_irq_enable_hwe(struct xe_gt *gt)
> >  		if (xe_hw_engine_mask_per_class(gt,
> XE_ENGINE_CLASS_OTHER)) {
> >  			xe_mmio_write32(gt, GUNIT_GSC_INTR_ENABLE,
> irqs);
> >  			xe_mmio_write32(gt, GUNIT_GSC_INTR_MASK,
> ~irqs);
> > +		} else if (HAS_HECI_GSCFI(xe)) {
> > +			xe_mmio_write32(gt, GUNIT_GSC_INTR_ENABLE,
> gsc_mask);
> > +			xe_mmio_write32(gt, GUNIT_GSC_INTR_MASK,
> ~gsc_mask);
> 
> is there any way we could combine this with the upper calls to the same
> register?
> I believe i915 is combining them for instance...
> 

Ok, will do, small gain, but it will be more readable.

> >  		}
> >  	}
> >  }
> > @@ -284,6 +288,11 @@ static void gt_irq_handler(struct xe_tile *tile,
> >  			instance = INTR_ENGINE_INSTANCE(identity[bit]);
> >  			intr_vec = INTR_ENGINE_INTR(identity[bit]);
> >
> > +			if (class == XE_ENGINE_CLASS_OTHER && instance ==
> OTHER_GSC_INSTANCE) {
> > +				xe_heci_gsc_irq_handler(xe, intr_vec);
> > +				continue;
> > +			}
> > +
> >  			engine_gt = pick_engine_gt(tile, class, instance);
> >
> >  			hwe = xe_gt_hw_engine(engine_gt, class, instance,
> false);
> > @@ -470,8 +479,9 @@ static void gt_irq_reset(struct xe_tile *tile)
> >  	if (ccs_mask & (BIT(2)|BIT(3)))
> >  		xe_mmio_write32(mmio,  CCS2_CCS3_INTR_MASK, ~0);
> >
> > -	if (tile->media_gt &&
> > -	    xe_hw_engine_mask_per_class(tile->media_gt,
> XE_ENGINE_CLASS_OTHER)) {
> > +	if ((tile->media_gt &&
> > +	     xe_hw_engine_mask_per_class(tile->media_gt,
> XE_ENGINE_CLASS_OTHER)) ||
> > +	    HAS_HECI_GSCFI(tile_to_xe(tile))) {
> >  		xe_mmio_write32(mmio, GUNIT_GSC_INTR_ENABLE, 0);
> >  		xe_mmio_write32(mmio, GUNIT_GSC_INTR_MASK, ~0);
> >  	}
> > --
> > 2.34.1
> >

-- 
Thanks,
Sasha

RE: [Intel-xe] [PATCH v2 3/4] drm/xe/gsc: add gsc device support

[Usyskin, Alexander]

> > > diff --git a/drivers/gpu/drm/xe/xe_device.c
> > b/drivers/gpu/drm/xe/xe_device.c
> > > index d6fc06d4c9dc..4d6e2f2b281f 100644
> > > --- a/drivers/gpu/drm/xe/xe_device.c
> > > +++ b/drivers/gpu/drm/xe/xe_device.c
> > > @@ -323,6 +323,8 @@ int xe_device_probe(struct xe_device *xe)
> > >  			goto err_irq_shutdown;
> > >  	}
> > >
> > > +	xe_heci_gsc_init(xe);
> > > +
> >
> > could we place this call earlier in the flow? maybe right after setting up
> mmio?
> > or maybe after pcode init where we confirmed the boot can proceed on
> > discrete?
> 
> Right after xe_irq_install will be ok (as we need working irq to setup)?
> Will try, if this works.
> 

Right after xe_irq_install - not working, but after xe_gt_init_early - it works, will move there in V2

> >
> > >  	err = xe_display_init(xe);
> > >  	if (err)
> > >  		goto err_irq_shutdown;

-- 
Thanks,
Sasha

[PATCH v2 4/4] mei: gsc: add support for auxiliary device created by Xe driver

[Alexander Usyskin]

From: Vitaly Lubart <vitaly.lubart@intel.com>

Add support for gsc mei auxiliary device created by Xe driver

Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/misc/mei/gsc-me.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/misc/mei/gsc-me.c b/drivers/misc/mei/gsc-me.c
index e63cabd0818d..63525778a4e0 100644
--- a/drivers/misc/mei/gsc-me.c
+++ b/drivers/misc/mei/gsc-me.c
@@ -292,6 +292,10 @@ static const struct auxiliary_device_id mei_gsc_id_table[] = {
 		.name = "i915.mei-gscfi",
 		.driver_data = MEI_ME_GSCFI_CFG,
 	},
+	{
+		.name = "xe.mei-gscfi",
+		.driver_data = MEI_ME_GSCFI_CFG,
+	},
 	{
 		/* sentinel */
 	}
@@ -312,4 +316,5 @@ module_auxiliary_driver(mei_gsc_driver);
 MODULE_AUTHOR("Intel Corporation");
 MODULE_ALIAS("auxiliary:i915.mei-gsc");
 MODULE_ALIAS("auxiliary:i915.mei-gscfi");
+MODULE_ALIAS("auxiliary:xe.mei-gscfi");
 MODULE_LICENSE("GPL");
-- 
2.34.1

Re: [Intel-xe] [PATCH v2 4/4] mei: gsc: add support for auxiliary device created by Xe driver

[Rodrigo Vivi]

On Thu, Sep 14, 2023 at 11:01:38AM +0300, Alexander Usyskin wrote:
> From: Vitaly Lubart <vitaly.lubart@intel.com>
> 
> Add support for gsc mei auxiliary device created by Xe driver
> 
> Signed-off-by: Vitaly Lubart <vitaly.lubart@intel.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/misc/mei/gsc-me.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/misc/mei/gsc-me.c b/drivers/misc/mei/gsc-me.c
> index e63cabd0818d..63525778a4e0 100644
> --- a/drivers/misc/mei/gsc-me.c
> +++ b/drivers/misc/mei/gsc-me.c
> @@ -292,6 +292,10 @@ static const struct auxiliary_device_id mei_gsc_id_table[] = {
>  		.name = "i915.mei-gscfi",
>  		.driver_data = MEI_ME_GSCFI_CFG,
>  	},
> +	{
> +		.name = "xe.mei-gscfi",
> +		.driver_data = MEI_ME_GSCFI_CFG,
> +	},
>  	{
>  		/* sentinel */
>  	}
> @@ -312,4 +316,5 @@ module_auxiliary_driver(mei_gsc_driver);
>  MODULE_AUTHOR("Intel Corporation");
>  MODULE_ALIAS("auxiliary:i915.mei-gsc");
>  MODULE_ALIAS("auxiliary:i915.mei-gscfi");
> +MODULE_ALIAS("auxiliary:xe.mei-gscfi");
>  MODULE_LICENSE("GPL");
> -- 
> 2.34.1
> 

Re: [Intel-xe] [PATCH v2 0/4] drm/xe/gsc: add initial gsc support

[Rodrigo Vivi]

On Thu, Sep 14, 2023 at 11:01:34AM +0300, Alexander Usyskin wrote:
> Add initial GSC support for DG1 and DG2.
> Create mei-gscfi auxiliary device and add
> support for device created by Xe to the mei-gsc driver.

why are we not adding the GSC already? because dependency on the GSC
engine or something like that?

> 
> This series intended to be merged intto Xe tree,
> the only patch that touches char-misc is addition
> of id and alias in mei-gsc.
> 
> V2: Added heci_ to struct and APIs
>     heci_gsc moved under xe_device as GSC is not under GT
> 
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> 
> Vitaly Lubart (4):
>   drm/xe/gsc: add HECI2 register offsets
>   drm/xe/gsc: add has_heci_gscfi indication to device
>   drm/xe/gsc: add gsc device support
>   mei: gsc: add support for auxiliary device created by Xe driver
> 
>  drivers/gpu/drm/xe/Kconfig           |   1 +
>  drivers/gpu/drm/xe/Makefile          |   1 +
>  drivers/gpu/drm/xe/regs/xe_regs.h    |   4 +
>  drivers/gpu/drm/xe/xe_device.c       |   4 +
>  drivers/gpu/drm/xe/xe_device_types.h |   7 +
>  drivers/gpu/drm/xe/xe_heci_gsc.c     | 205 +++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_heci_gsc.h     |  35 +++++
>  drivers/gpu/drm/xe/xe_irq.c          |  14 +-
>  drivers/gpu/drm/xe/xe_pci.c          |   9 +-
>  drivers/misc/mei/gsc-me.c            |   5 +
>  10 files changed, 281 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/drm/xe/xe_heci_gsc.c
>  create mode 100644 drivers/gpu/drm/xe/xe_heci_gsc.h
> 
> -- 
> 2.34.1
> 

RE: [Intel-xe] [PATCH v2 0/4] drm/xe/gsc: add initial gsc support

[Usyskin, Alexander]

> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Sent: Monday, September 18, 2023 19:07
> To: Usyskin, Alexander <alexander.usyskin@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; De Marchi, Lucas
> <lucas.demarchi@intel.com>; Ceraolo Spurio, Daniele
> <daniele.ceraolospurio@intel.com>; linux-kernel@vger.kernel.org; Winkler,
> Tomas <tomas.winkler@intel.com>; intel-xe@lists.freedesktop.org
> Subject: Re: [Intel-xe] [PATCH v2 0/4] drm/xe/gsc: add initial gsc support
> 
> On Thu, Sep 14, 2023 at 11:01:34AM +0300, Alexander Usyskin wrote:
> > Add initial GSC support for DG1 and DG2.
> > Create mei-gscfi auxiliary device and add
> > support for device created by Xe to the mei-gsc driver.
> 
> why are we not adding the GSC already? because dependency on the GSC
> engine or something like that?
> 

The DG1 and DG2 have only HECI-GSC routed via mei.
The pure GSC without mei is for later projects.

-- 
Thanks,
Sasha


> >
> > This series intended to be merged intto Xe tree,
> > the only patch that touches char-misc is addition
> > of id and alias in mei-gsc.
> >
> > V2: Added heci_ to struct and APIs
> >     heci_gsc moved under xe_device as GSC is not under GT
> >
> > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> >
> > Vitaly Lubart (4):
> >   drm/xe/gsc: add HECI2 register offsets
> >   drm/xe/gsc: add has_heci_gscfi indication to device
> >   drm/xe/gsc: add gsc device support
> >   mei: gsc: add support for auxiliary device created by Xe driver
> >
> >  drivers/gpu/drm/xe/Kconfig           |   1 +
> >  drivers/gpu/drm/xe/Makefile          |   1 +
> >  drivers/gpu/drm/xe/regs/xe_regs.h    |   4 +
> >  drivers/gpu/drm/xe/xe_device.c       |   4 +
> >  drivers/gpu/drm/xe/xe_device_types.h |   7 +
> >  drivers/gpu/drm/xe/xe_heci_gsc.c     | 205
> +++++++++++++++++++++++++++
> >  drivers/gpu/drm/xe/xe_heci_gsc.h     |  35 +++++
> >  drivers/gpu/drm/xe/xe_irq.c          |  14 +-
> >  drivers/gpu/drm/xe/xe_pci.c          |   9 +-
> >  drivers/misc/mei/gsc-me.c            |   5 +
> >  10 files changed, 281 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/gpu/drm/xe/xe_heci_gsc.c
> >  create mode 100644 drivers/gpu/drm/xe/xe_heci_gsc.h
> >
> > --
> > 2.34.1
> >