From: Vitaly Lubart <[email protected]>
Create mei-gscfi auxiliary device and configure interrupts
to be consumed by mei-gsc device driver.
Signed-off-by: Vitaly Lubart <[email protected]>
Signed-off-by: Alexander Usyskin <[email protected]>
---
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
On Thu, Sep 14, 2023 at 11:01:37AM +0300, Alexander Usyskin wrote:
> From: Vitaly Lubart <[email protected]>
>
> Create mei-gscfi auxiliary device and configure interrupts
> to be consumed by mei-gsc device driver.
>
> Signed-off-by: Vitaly Lubart <[email protected]>
> Signed-off-by: Alexander Usyskin <[email protected]>
> ---
> 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
>
> -----Original Message-----
> From: Vivi, Rodrigo <[email protected]>
> Sent: Monday, September 18, 2023 19:25
> To: Usyskin, Alexander <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>; De Marchi, Lucas
> <[email protected]>; Ceraolo Spurio, Daniele
> <[email protected]>; [email protected]; Winkler,
> Tomas <[email protected]>; [email protected]
> 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 <[email protected]>
> >
> > Create mei-gscfi auxiliary device and configure interrupts
> > to be consumed by mei-gsc device driver.
> >
> > Signed-off-by: Vitaly Lubart <[email protected]>
> > Signed-off-by: Alexander Usyskin <[email protected]>
> > ---
> > 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
> > > 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