dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915: rename intel_gsc to intel_heci_gsc
@ 2022-11-02 21:33 Daniele Ceraolo Spurio
  2022-11-03 10:41 ` Winkler, Tomas
  0 siblings, 1 reply; 4+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-11-02 21:33 UTC (permalink / raw)
  To: intel-gfx
  Cc: Tomas Winkler, Daniele Ceraolo Spurio, Alexander Usyskin, dri-devel

Starting on MTL, the GSC FW is loaded at runtime and will be managed
directly by i915. This means we now have a naming clash around the GSC,
as we have 2 different aspects of it that we need to handle: the HECI
interfaces we export pre-mtl and the new full FW loading and support we
have to introduce starting from MTL. To avoid confusion, rename the
existing intel_gsc structure to intel_heci_gsc, to make it clear it
contains the data related to the HECI interfaces.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Tomas Winkler <tomas.winkler@intel.com>
Cc: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/gpu/drm/i915/Makefile                 |  4 +-
 drivers/gpu/drm/i915/gt/intel_gt.c            |  4 +-
 drivers/gpu/drm/i915/gt/intel_gt.h            |  4 +-
 drivers/gpu/drm/i915/gt/intel_gt_irq.c        |  2 +-
 drivers/gpu/drm/i915/gt/intel_gt_types.h      |  4 +-
 .../i915/gt/{intel_gsc.c => intel_heci_gsc.c} | 43 ++++++++++---------
 .../i915/gt/{intel_gsc.h => intel_heci_gsc.h} | 22 +++++-----
 drivers/gpu/drm/i915/gt/uc/intel_huc.c        | 10 ++---
 drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c     |  2 +-
 9 files changed, 48 insertions(+), 47 deletions(-)
 rename drivers/gpu/drm/i915/gt/{intel_gsc.c => intel_heci_gsc.c} (84%)
 rename drivers/gpu/drm/i915/gt/{intel_gsc.h => intel_heci_gsc.h} (52%)

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 51704b54317c..2fa401dcf087 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -206,8 +206,8 @@ i915-y += gt/uc/intel_uc.o \
 	  gt/uc/intel_huc_debugfs.o \
 	  gt/uc/intel_huc_fw.o
 
-# graphics system controller (GSC) support
-i915-y += gt/intel_gsc.o
+# graphics system controller (GSC) HECI support
+i915-y += gt/intel_heci_gsc.o
 
 # graphics hardware monitoring (HWMON) support
 i915-$(CONFIG_HWMON) += i915_hwmon.o
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 8e914c4066ed..6ca72479c943 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -454,7 +454,7 @@ void intel_gt_chipset_flush(struct intel_gt *gt)
 
 void intel_gt_driver_register(struct intel_gt *gt)
 {
-	intel_gsc_init(&gt->gsc, gt->i915);
+	intel_heci_gsc_init(&gt->heci_gsc, gt->i915);
 
 	intel_rps_driver_register(&gt->rps);
 
@@ -785,7 +785,7 @@ void intel_gt_driver_unregister(struct intel_gt *gt)
 
 	intel_gt_sysfs_unregister(gt);
 	intel_rps_driver_unregister(&gt->rps);
-	intel_gsc_fini(&gt->gsc);
+	intel_heci_gsc_fini(&gt->heci_gsc);
 
 	intel_pxp_fini(&gt->pxp);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
index e0365d556248..43f73239a363 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt.h
@@ -39,9 +39,9 @@ static inline struct intel_gt *huc_to_gt(struct intel_huc *huc)
 	return container_of(huc, struct intel_gt, uc.huc);
 }
 
-static inline struct intel_gt *gsc_to_gt(struct intel_gsc *gsc)
+static inline struct intel_gt *heci_gsc_to_gt(struct intel_heci_gsc *heci_gsc)
 {
-	return container_of(gsc, struct intel_gt, gsc);
+	return container_of(heci_gsc, struct intel_gt, heci_gsc);
 }
 
 void intel_gt_common_init_early(struct intel_gt *gt);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
index f26882fdc24c..3b4bd237659a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -75,7 +75,7 @@ gen11_other_irq_handler(struct intel_gt *gt, const u8 instance,
 		return intel_pxp_irq_handler(&gt->pxp, iir);
 
 	if (instance == OTHER_GSC_INSTANCE)
-		return intel_gsc_irq_handler(gt, iir);
+		return intel_heci_gsc_irq_handler(gt, iir);
 
 	WARN_ONCE(1, "unhandled other interrupt instance=0x%x, iir=0x%x\n",
 		  instance, iir);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index a0cc73b401ef..80a0163dcc01 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -17,7 +17,7 @@
 #include <linux/workqueue.h>
 
 #include "uc/intel_uc.h"
-#include "intel_gsc.h"
+#include "intel_heci_gsc.h"
 
 #include "i915_vma.h"
 #include "i915_perf_types.h"
@@ -100,7 +100,7 @@ struct intel_gt {
 	struct i915_ggtt *ggtt;
 
 	struct intel_uc uc;
-	struct intel_gsc gsc;
+	struct intel_heci_gsc heci_gsc;
 
 	struct {
 		/* Serialize global tlb invalidations */
diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.c b/drivers/gpu/drm/i915/gt/intel_heci_gsc.c
similarity index 84%
rename from drivers/gpu/drm/i915/gt/intel_gsc.c
rename to drivers/gpu/drm/i915/gt/intel_heci_gsc.c
index 976fdf27e790..f57771470dba 100644
--- a/drivers/gpu/drm/i915/gt/intel_gsc.c
+++ b/drivers/gpu/drm/i915/gt/intel_heci_gsc.c
@@ -9,7 +9,7 @@
 #include "i915_reg.h"
 #include "gem/i915_gem_lmem.h"
 #include "gem/i915_gem_region.h"
-#include "gt/intel_gsc.h"
+#include "gt/intel_heci_gsc.h"
 #include "gt/intel_gt.h"
 
 #define GSC_BAR_LENGTH  0x00000FFC
@@ -38,10 +38,11 @@ static int gsc_irq_init(int irq)
 	return irq_set_chip_data(irq, NULL);
 }
 
-static int
-gsc_ext_om_alloc(struct intel_gsc *gsc, struct intel_gsc_intf *intf, size_t size)
+static int gsc_ext_om_alloc(struct intel_heci_gsc *gsc,
+			    struct intel_heci_gsc_intf *intf,
+			    size_t size)
 {
-	struct intel_gt *gt = gsc_to_gt(gsc);
+	struct intel_gt *gt = heci_gsc_to_gt(gsc);
 	struct drm_i915_gem_object *obj;
 	int err;
 
@@ -68,7 +69,7 @@ gsc_ext_om_alloc(struct intel_gsc *gsc, struct intel_gsc_intf *intf, size_t size
 	return err;
 }
 
-static void gsc_ext_om_destroy(struct intel_gsc_intf *intf)
+static void gsc_ext_om_destroy(struct intel_heci_gsc_intf *intf)
 {
 	struct drm_i915_gem_object *obj = fetch_and_zero(&intf->gem_obj);
 
@@ -138,15 +139,15 @@ static void gsc_release_dev(struct device *dev)
 }
 
 static void gsc_destroy_one(struct drm_i915_private *i915,
-			    struct intel_gsc *gsc, unsigned int intf_id)
+			    struct intel_heci_gsc *gsc, unsigned int intf_id)
 {
-	struct intel_gsc_intf *intf = &gsc->intf[intf_id];
+	struct intel_heci_gsc_intf *intf = &gsc->intf[intf_id];
 
 	if (intf->adev) {
 		struct auxiliary_device *aux_dev = &intf->adev->aux_dev;
 
 		if (intf_id == 0)
-			intel_huc_unregister_gsc_notifier(&gsc_to_gt(gsc)->uc.huc,
+			intel_huc_unregister_gsc_notifier(&heci_gsc_to_gt(gsc)->uc.huc,
 							  aux_dev->dev.bus);
 
 		auxiliary_device_delete(aux_dev);
@@ -161,14 +162,14 @@ static void gsc_destroy_one(struct drm_i915_private *i915,
 	gsc_ext_om_destroy(intf);
 }
 
-static void gsc_init_one(struct drm_i915_private *i915, struct intel_gsc *gsc,
+static void gsc_init_one(struct drm_i915_private *i915, struct intel_heci_gsc *gsc,
 			 unsigned int intf_id)
 {
 	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
 	struct mei_aux_device *adev;
 	struct auxiliary_device *aux_dev;
 	const struct gsc_def *def;
-	struct intel_gsc_intf *intf = &gsc->intf[intf_id];
+	struct intel_heci_gsc_intf *intf = &gsc->intf[intf_id];
 	int ret;
 
 	intf->irq = -1;
@@ -252,14 +253,14 @@ static void gsc_init_one(struct drm_i915_private *i915, struct intel_gsc *gsc,
 	intf->adev = adev; /* needed by the notifier */
 
 	if (intf_id == 0)
-		intel_huc_register_gsc_notifier(&gsc_to_gt(gsc)->uc.huc,
+		intel_huc_register_gsc_notifier(&heci_gsc_to_gt(gsc)->uc.huc,
 						aux_dev->dev.bus);
 
 	ret = auxiliary_device_add(aux_dev);
 	if (ret < 0) {
 		drm_err(&i915->drm, "gsc aux add failed %d\n", ret);
 		if (intf_id == 0)
-			intel_huc_unregister_gsc_notifier(&gsc_to_gt(gsc)->uc.huc,
+			intel_huc_unregister_gsc_notifier(&heci_gsc_to_gt(gsc)->uc.huc,
 							  aux_dev->dev.bus);
 		intf->adev = NULL;
 
@@ -277,7 +278,7 @@ static void gsc_irq_handler(struct intel_gt *gt, unsigned int intf_id)
 {
 	int ret;
 
-	if (intf_id >= INTEL_GSC_NUM_INTERFACES) {
+	if (intf_id >= INTEL_HECI_GSC_NUM_INTERFACES) {
 		drm_warn_once(&gt->i915->drm, "GSC irq: intf_id %d is out of range", intf_id);
 		return;
 	}
@@ -287,15 +288,15 @@ static void gsc_irq_handler(struct intel_gt *gt, unsigned int intf_id)
 		return;
 	}
 
-	if (gt->gsc.intf[intf_id].irq < 0)
+	if (gt->heci_gsc.intf[intf_id].irq < 0)
 		return;
 
-	ret = generic_handle_irq(gt->gsc.intf[intf_id].irq);
+	ret = generic_handle_irq(gt->heci_gsc.intf[intf_id].irq);
 	if (ret)
 		drm_err_ratelimited(&gt->i915->drm, "error handling GSC irq: %d\n", ret);
 }
 
-void intel_gsc_irq_handler(struct intel_gt *gt, u32 iir)
+void intel_heci_gsc_irq_handler(struct intel_gt *gt, u32 iir)
 {
 	if (iir & GSC_IRQ_INTF(0))
 		gsc_irq_handler(gt, 0);
@@ -303,25 +304,25 @@ void intel_gsc_irq_handler(struct intel_gt *gt, u32 iir)
 		gsc_irq_handler(gt, 1);
 }
 
-void intel_gsc_init(struct intel_gsc *gsc, struct drm_i915_private *i915)
+void intel_heci_gsc_init(struct intel_heci_gsc *gsc, struct drm_i915_private *i915)
 {
 	unsigned int i;
 
 	if (!HAS_HECI_GSC(i915))
 		return;
 
-	for (i = 0; i < INTEL_GSC_NUM_INTERFACES; i++)
+	for (i = 0; i < INTEL_HECI_GSC_NUM_INTERFACES; i++)
 		gsc_init_one(i915, gsc, i);
 }
 
-void intel_gsc_fini(struct intel_gsc *gsc)
+void intel_heci_gsc_fini(struct intel_heci_gsc *gsc)
 {
-	struct intel_gt *gt = gsc_to_gt(gsc);
+	struct intel_gt *gt = heci_gsc_to_gt(gsc);
 	unsigned int i;
 
 	if (!HAS_HECI_GSC(gt->i915))
 		return;
 
-	for (i = 0; i < INTEL_GSC_NUM_INTERFACES; i++)
+	for (i = 0; i < INTEL_HECI_GSC_NUM_INTERFACES; i++)
 		gsc_destroy_one(gt->i915, gsc, i);
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.h b/drivers/gpu/drm/i915/gt/intel_heci_gsc.h
similarity index 52%
rename from drivers/gpu/drm/i915/gt/intel_gsc.h
rename to drivers/gpu/drm/i915/gt/intel_heci_gsc.h
index fcac1775e9c3..a20632a4316b 100644
--- a/drivers/gpu/drm/i915/gt/intel_gsc.h
+++ b/drivers/gpu/drm/i915/gt/intel_heci_gsc.h
@@ -2,8 +2,8 @@
 /*
  * Copyright(c) 2019-2022, Intel Corporation. All rights reserved.
  */
-#ifndef __INTEL_GSC_DEV_H__
-#define __INTEL_GSC_DEV_H__
+#ifndef __INTEL_HECI_GSC_DEV_H__
+#define __INTEL_HECI_GSC_DEV_H__
 
 #include <linux/types.h>
 
@@ -11,7 +11,7 @@ struct drm_i915_private;
 struct intel_gt;
 struct mei_aux_device;
 
-#define INTEL_GSC_NUM_INTERFACES 2
+#define INTEL_HECI_GSC_NUM_INTERFACES 2
 /*
  * The HECI1 bit corresponds to bit15 and HECI2 to bit14.
  * The reason for this is to allow growth for more interfaces in the future.
@@ -19,22 +19,22 @@ struct mei_aux_device;
 #define GSC_IRQ_INTF(_x)  BIT(15 - (_x))
 
 /**
- * struct intel_gsc - graphics security controller
+ * struct intel_heci_gsc - graphics security controller HECI interfaces
  *
  * @gem_obj: scratch memory GSC operations
  * @intf : gsc interface
  */
-struct intel_gsc {
-	struct intel_gsc_intf {
+struct intel_heci_gsc {
+	struct intel_heci_gsc_intf {
 		struct mei_aux_device *adev;
 		struct drm_i915_gem_object *gem_obj;
 		int irq;
 		unsigned int id;
-	} intf[INTEL_GSC_NUM_INTERFACES];
+	} intf[INTEL_HECI_GSC_NUM_INTERFACES];
 };
 
-void intel_gsc_init(struct intel_gsc *gsc, struct drm_i915_private *dev_priv);
-void intel_gsc_fini(struct intel_gsc *gsc);
-void intel_gsc_irq_handler(struct intel_gt *gt, u32 iir);
+void intel_heci_gsc_init(struct intel_heci_gsc *gsc, struct drm_i915_private *dev_priv);
+void intel_heci_gsc_fini(struct intel_heci_gsc *gsc);
+void intel_heci_gsc_irq_handler(struct intel_gt *gt, u32 iir);
 
-#endif /* __INTEL_GSC_DEV_H__ */
+#endif /* __INTEL_HECI_GSC_DEV_H__ */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index fbc8bae14f76..0fd7baf5b0cf 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -47,10 +47,10 @@
 
 /*
  * MEI-GSC load is an async process. The probing of the exposed aux device
- * (see intel_gsc.c) usually happens a few seconds after i915 probe, depending
- * on when the kernel schedules it. Unless something goes terribly wrong, we're
- * guaranteed for this to happen during boot, so the big timeout is a safety net
- * that we never expect to need.
+ * (see intel_heci_gsc.c) usually happens a few seconds after i915 probe,
+ * depending on when the kernel schedules it. Unless something goes terribly
+ * wrong, we're guaranteed for this to happen during boot, so the big timeout is
+ * a safety net that we never expect to need.
  * MEI-PXP + HuC load usually takes ~300ms, but if the GSC needs to be resumed
  * and/or reset, this can take longer. Note that the kernel might schedule
  * other work between the i915 init/resume and the MEI one, which can add to
@@ -162,7 +162,7 @@ static int gsc_notifier(struct notifier_block *nb, unsigned long action, void *d
 {
 	struct device *dev = data;
 	struct intel_huc *huc = container_of(nb, struct intel_huc, delayed_load.nb);
-	struct intel_gsc_intf *intf = &huc_to_gt(huc)->gsc.intf[0];
+	struct intel_heci_gsc_intf *intf = &huc_to_gt(huc)->heci_gsc.intf[0];
 
 	if (!intf->adev || &intf->adev->aux_dev.dev != dev)
 		return 0;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index 4f246416db17..cced240ef8a4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -3,7 +3,7 @@
  * Copyright © 2014-2019 Intel Corporation
  */
 
-#include "gt/intel_gsc.h"
+#include "gt/intel_heci_gsc.h"
 #include "gt/intel_gt.h"
 #include "intel_huc.h"
 #include "intel_huc_fw.h"
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* RE: [PATCH] drm/i915: rename intel_gsc to intel_heci_gsc
  2022-11-02 21:33 [PATCH] drm/i915: rename intel_gsc to intel_heci_gsc Daniele Ceraolo Spurio
@ 2022-11-03 10:41 ` Winkler, Tomas
  2022-11-17 22:47   ` Ceraolo Spurio, Daniele
  0 siblings, 1 reply; 4+ messages in thread
From: Winkler, Tomas @ 2022-11-03 10:41 UTC (permalink / raw)
  To: Ceraolo Spurio, Daniele, intel-gfx; +Cc: Usyskin, Alexander, dri-devel


> 
> Starting on MTL, the GSC FW is loaded at runtime and will be managed
> directly by i915. This means we now have a naming clash around the GSC, as
> we have 2 different aspects of it that we need to handle: the HECI interfaces
> we export pre-mtl and the new full FW loading and support we have to
> introduce starting from MTL. To avoid confusion, rename the existing
> intel_gsc structure to intel_heci_gsc, to make it clear it contains the data
> related to the HECI interfaces.
> 
Are you sure you want to take this path, it will make backporting quite difficult. 

> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Tomas Winkler <tomas.winkler@intel.com>
> Cc: Alexander Usyskin <alexander.usyskin@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile                 |  4 +-
>  drivers/gpu/drm/i915/gt/intel_gt.c            |  4 +-
>  drivers/gpu/drm/i915/gt/intel_gt.h            |  4 +-
>  drivers/gpu/drm/i915/gt/intel_gt_irq.c        |  2 +-
>  drivers/gpu/drm/i915/gt/intel_gt_types.h      |  4 +-
>  .../i915/gt/{intel_gsc.c => intel_heci_gsc.c} | 43 ++++++++++---------
> .../i915/gt/{intel_gsc.h => intel_heci_gsc.h} | 22 +++++-----
>  drivers/gpu/drm/i915/gt/uc/intel_huc.c        | 10 ++---
>  drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c     |  2 +-
>  9 files changed, 48 insertions(+), 47 deletions(-)  rename
> drivers/gpu/drm/i915/gt/{intel_gsc.c => intel_heci_gsc.c} (84%)  rename
> drivers/gpu/drm/i915/gt/{intel_gsc.h => intel_heci_gsc.h} (52%)
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 51704b54317c..2fa401dcf087 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -206,8 +206,8 @@ i915-y += gt/uc/intel_uc.o \
>  	  gt/uc/intel_huc_debugfs.o \
>  	  gt/uc/intel_huc_fw.o
> 
> -# graphics system controller (GSC) support -i915-y += gt/intel_gsc.o
> +# graphics system controller (GSC) HECI support i915-y +=
> +gt/intel_heci_gsc.o
> 
>  # graphics hardware monitoring (HWMON) support
>  i915-$(CONFIG_HWMON) += i915_hwmon.o
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
> b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 8e914c4066ed..6ca72479c943 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -454,7 +454,7 @@ void intel_gt_chipset_flush(struct intel_gt *gt)
> 
>  void intel_gt_driver_register(struct intel_gt *gt)  {
> -	intel_gsc_init(&gt->gsc, gt->i915);
> +	intel_heci_gsc_init(&gt->heci_gsc, gt->i915);
> 
>  	intel_rps_driver_register(&gt->rps);
> 
> @@ -785,7 +785,7 @@ void intel_gt_driver_unregister(struct intel_gt *gt)
> 
>  	intel_gt_sysfs_unregister(gt);
>  	intel_rps_driver_unregister(&gt->rps);
> -	intel_gsc_fini(&gt->gsc);
> +	intel_heci_gsc_fini(&gt->heci_gsc);
> 
>  	intel_pxp_fini(&gt->pxp);
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h
> b/drivers/gpu/drm/i915/gt/intel_gt.h
> index e0365d556248..43f73239a363 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.h
> @@ -39,9 +39,9 @@ static inline struct intel_gt *huc_to_gt(struct intel_huc
> *huc)
>  	return container_of(huc, struct intel_gt, uc.huc);  }
> 
> -static inline struct intel_gt *gsc_to_gt(struct intel_gsc *gsc)
> +static inline struct intel_gt *heci_gsc_to_gt(struct intel_heci_gsc
> +*heci_gsc)
>  {
> -	return container_of(gsc, struct intel_gt, gsc);
> +	return container_of(heci_gsc, struct intel_gt, heci_gsc);
>  }
> 
>  void intel_gt_common_init_early(struct intel_gt *gt); diff --git
> a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> index f26882fdc24c..3b4bd237659a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> @@ -75,7 +75,7 @@ gen11_other_irq_handler(struct intel_gt *gt, const u8
> instance,
>  		return intel_pxp_irq_handler(&gt->pxp, iir);
> 
>  	if (instance == OTHER_GSC_INSTANCE)
> -		return intel_gsc_irq_handler(gt, iir);
> +		return intel_heci_gsc_irq_handler(gt, iir);
> 
>  	WARN_ONCE(1, "unhandled other interrupt instance=0x%x,
> iir=0x%x\n",
>  		  instance, iir);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> index a0cc73b401ef..80a0163dcc01 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> @@ -17,7 +17,7 @@
>  #include <linux/workqueue.h>
> 
>  #include "uc/intel_uc.h"
> -#include "intel_gsc.h"
> +#include "intel_heci_gsc.h"
> 
>  #include "i915_vma.h"
>  #include "i915_perf_types.h"
> @@ -100,7 +100,7 @@ struct intel_gt {
>  	struct i915_ggtt *ggtt;
> 
>  	struct intel_uc uc;
> -	struct intel_gsc gsc;
> +	struct intel_heci_gsc heci_gsc;
> 
>  	struct {
>  		/* Serialize global tlb invalidations */ diff --git
> a/drivers/gpu/drm/i915/gt/intel_gsc.c
> b/drivers/gpu/drm/i915/gt/intel_heci_gsc.c
> similarity index 84%
> rename from drivers/gpu/drm/i915/gt/intel_gsc.c
> rename to drivers/gpu/drm/i915/gt/intel_heci_gsc.c
> index 976fdf27e790..f57771470dba 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gsc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_heci_gsc.c
> @@ -9,7 +9,7 @@
>  #include "i915_reg.h"
>  #include "gem/i915_gem_lmem.h"
>  #include "gem/i915_gem_region.h"
> -#include "gt/intel_gsc.h"
> +#include "gt/intel_heci_gsc.h"
>  #include "gt/intel_gt.h"
> 
>  #define GSC_BAR_LENGTH  0x00000FFC
> @@ -38,10 +38,11 @@ static int gsc_irq_init(int irq)
>  	return irq_set_chip_data(irq, NULL);
>  }
> 
> -static int
> -gsc_ext_om_alloc(struct intel_gsc *gsc, struct intel_gsc_intf *intf, size_t
> size)
> +static int gsc_ext_om_alloc(struct intel_heci_gsc *gsc,
> +			    struct intel_heci_gsc_intf *intf,
> +			    size_t size)
>  {
> -	struct intel_gt *gt = gsc_to_gt(gsc);
> +	struct intel_gt *gt = heci_gsc_to_gt(gsc);
>  	struct drm_i915_gem_object *obj;
>  	int err;
> 
> @@ -68,7 +69,7 @@ gsc_ext_om_alloc(struct intel_gsc *gsc, struct
> intel_gsc_intf *intf, size_t size
>  	return err;
>  }
> 
> -static void gsc_ext_om_destroy(struct intel_gsc_intf *intf)
> +static void gsc_ext_om_destroy(struct intel_heci_gsc_intf *intf)
>  {
>  	struct drm_i915_gem_object *obj = fetch_and_zero(&intf-
> >gem_obj);
> 
> @@ -138,15 +139,15 @@ static void gsc_release_dev(struct device *dev)  }
> 
>  static void gsc_destroy_one(struct drm_i915_private *i915,
> -			    struct intel_gsc *gsc, unsigned int intf_id)
> +			    struct intel_heci_gsc *gsc, unsigned int intf_id)
>  {
> -	struct intel_gsc_intf *intf = &gsc->intf[intf_id];
> +	struct intel_heci_gsc_intf *intf = &gsc->intf[intf_id];
> 
>  	if (intf->adev) {
>  		struct auxiliary_device *aux_dev = &intf->adev->aux_dev;
> 
>  		if (intf_id == 0)
> -			intel_huc_unregister_gsc_notifier(&gsc_to_gt(gsc)-
> >uc.huc,
> +
> 	intel_huc_unregister_gsc_notifier(&heci_gsc_to_gt(gsc)->uc.huc,
>  							  aux_dev->dev.bus);
> 
>  		auxiliary_device_delete(aux_dev);
> @@ -161,14 +162,14 @@ static void gsc_destroy_one(struct
> drm_i915_private *i915,
>  	gsc_ext_om_destroy(intf);
>  }
> 
> -static void gsc_init_one(struct drm_i915_private *i915, struct intel_gsc *gsc,
> +static void gsc_init_one(struct drm_i915_private *i915, struct
> +intel_heci_gsc *gsc,
>  			 unsigned int intf_id)
>  {
>  	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
>  	struct mei_aux_device *adev;
>  	struct auxiliary_device *aux_dev;
>  	const struct gsc_def *def;
> -	struct intel_gsc_intf *intf = &gsc->intf[intf_id];
> +	struct intel_heci_gsc_intf *intf = &gsc->intf[intf_id];
>  	int ret;
> 
>  	intf->irq = -1;
> @@ -252,14 +253,14 @@ static void gsc_init_one(struct drm_i915_private
> *i915, struct intel_gsc *gsc,
>  	intf->adev = adev; /* needed by the notifier */
> 
>  	if (intf_id == 0)
> -		intel_huc_register_gsc_notifier(&gsc_to_gt(gsc)->uc.huc,
> +		intel_huc_register_gsc_notifier(&heci_gsc_to_gt(gsc)-
> >uc.huc,
>  						aux_dev->dev.bus);
> 
>  	ret = auxiliary_device_add(aux_dev);
>  	if (ret < 0) {
>  		drm_err(&i915->drm, "gsc aux add failed %d\n", ret);
>  		if (intf_id == 0)
> -			intel_huc_unregister_gsc_notifier(&gsc_to_gt(gsc)-
> >uc.huc,
> +
> 	intel_huc_unregister_gsc_notifier(&heci_gsc_to_gt(gsc)->uc.huc,
>  							  aux_dev->dev.bus);
>  		intf->adev = NULL;
> 
> @@ -277,7 +278,7 @@ static void gsc_irq_handler(struct intel_gt *gt,
> unsigned int intf_id)  {
>  	int ret;
> 
> -	if (intf_id >= INTEL_GSC_NUM_INTERFACES) {
> +	if (intf_id >= INTEL_HECI_GSC_NUM_INTERFACES) {
>  		drm_warn_once(&gt->i915->drm, "GSC irq: intf_id %d is out
> of range", intf_id);
>  		return;
>  	}
> @@ -287,15 +288,15 @@ static void gsc_irq_handler(struct intel_gt *gt,
> unsigned int intf_id)
>  		return;
>  	}
> 
> -	if (gt->gsc.intf[intf_id].irq < 0)
> +	if (gt->heci_gsc.intf[intf_id].irq < 0)
>  		return;
> 
> -	ret = generic_handle_irq(gt->gsc.intf[intf_id].irq);
> +	ret = generic_handle_irq(gt->heci_gsc.intf[intf_id].irq);
>  	if (ret)
>  		drm_err_ratelimited(&gt->i915->drm, "error handling GSC
> irq: %d\n", ret);  }
> 
> -void intel_gsc_irq_handler(struct intel_gt *gt, u32 iir)
> +void intel_heci_gsc_irq_handler(struct intel_gt *gt, u32 iir)
>  {
>  	if (iir & GSC_IRQ_INTF(0))
>  		gsc_irq_handler(gt, 0);
> @@ -303,25 +304,25 @@ void intel_gsc_irq_handler(struct intel_gt *gt, u32
> iir)
>  		gsc_irq_handler(gt, 1);
>  }
> 
> -void intel_gsc_init(struct intel_gsc *gsc, struct drm_i915_private *i915)
> +void intel_heci_gsc_init(struct intel_heci_gsc *gsc, struct
> +drm_i915_private *i915)
>  {
>  	unsigned int i;
> 
>  	if (!HAS_HECI_GSC(i915))
>  		return;
> 
> -	for (i = 0; i < INTEL_GSC_NUM_INTERFACES; i++)
> +	for (i = 0; i < INTEL_HECI_GSC_NUM_INTERFACES; i++)
>  		gsc_init_one(i915, gsc, i);
>  }
> 
> -void intel_gsc_fini(struct intel_gsc *gsc)
> +void intel_heci_gsc_fini(struct intel_heci_gsc *gsc)
>  {
> -	struct intel_gt *gt = gsc_to_gt(gsc);
> +	struct intel_gt *gt = heci_gsc_to_gt(gsc);
>  	unsigned int i;
> 
>  	if (!HAS_HECI_GSC(gt->i915))
>  		return;
> 
> -	for (i = 0; i < INTEL_GSC_NUM_INTERFACES; i++)
> +	for (i = 0; i < INTEL_HECI_GSC_NUM_INTERFACES; i++)
>  		gsc_destroy_one(gt->i915, gsc, i);
>  }
> diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.h
> b/drivers/gpu/drm/i915/gt/intel_heci_gsc.h
> similarity index 52%
> rename from drivers/gpu/drm/i915/gt/intel_gsc.h
> rename to drivers/gpu/drm/i915/gt/intel_heci_gsc.h
> index fcac1775e9c3..a20632a4316b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gsc.h
> +++ b/drivers/gpu/drm/i915/gt/intel_heci_gsc.h
> @@ -2,8 +2,8 @@
>  /*
>   * Copyright(c) 2019-2022, Intel Corporation. All rights reserved.
>   */
> -#ifndef __INTEL_GSC_DEV_H__
> -#define __INTEL_GSC_DEV_H__
> +#ifndef __INTEL_HECI_GSC_DEV_H__
> +#define __INTEL_HECI_GSC_DEV_H__
> 
>  #include <linux/types.h>
> 
> @@ -11,7 +11,7 @@ struct drm_i915_private;  struct intel_gt;  struct
> mei_aux_device;
> 
> -#define INTEL_GSC_NUM_INTERFACES 2
> +#define INTEL_HECI_GSC_NUM_INTERFACES 2
>  /*
>   * The HECI1 bit corresponds to bit15 and HECI2 to bit14.
>   * The reason for this is to allow growth for more interfaces in the future.
> @@ -19,22 +19,22 @@ struct mei_aux_device;  #define GSC_IRQ_INTF(_x)
> BIT(15 - (_x))
> 
>  /**
> - * struct intel_gsc - graphics security controller
> + * struct intel_heci_gsc - graphics security controller HECI interfaces
>   *
>   * @gem_obj: scratch memory GSC operations
>   * @intf : gsc interface
>   */
> -struct intel_gsc {
> -	struct intel_gsc_intf {
> +struct intel_heci_gsc {
> +	struct intel_heci_gsc_intf {
>  		struct mei_aux_device *adev;
>  		struct drm_i915_gem_object *gem_obj;
>  		int irq;
>  		unsigned int id;
> -	} intf[INTEL_GSC_NUM_INTERFACES];
> +	} intf[INTEL_HECI_GSC_NUM_INTERFACES];
>  };
> 
> -void intel_gsc_init(struct intel_gsc *gsc, struct drm_i915_private *dev_priv);
> -void intel_gsc_fini(struct intel_gsc *gsc); -void intel_gsc_irq_handler(struct
> intel_gt *gt, u32 iir);
> +void intel_heci_gsc_init(struct intel_heci_gsc *gsc, struct
> +drm_i915_private *dev_priv); void intel_heci_gsc_fini(struct
> +intel_heci_gsc *gsc); void intel_heci_gsc_irq_handler(struct intel_gt
> +*gt, u32 iir);
> 
> -#endif /* __INTEL_GSC_DEV_H__ */
> +#endif /* __INTEL_HECI_GSC_DEV_H__ */
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index fbc8bae14f76..0fd7baf5b0cf 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -47,10 +47,10 @@
> 
>  /*
>   * MEI-GSC load is an async process. The probing of the exposed aux device
> - * (see intel_gsc.c) usually happens a few seconds after i915 probe,
> depending
> - * on when the kernel schedules it. Unless something goes terribly wrong,
> we're
> - * guaranteed for this to happen during boot, so the big timeout is a safety
> net
> - * that we never expect to need.
> + * (see intel_heci_gsc.c) usually happens a few seconds after i915
> + probe,
> + * depending on when the kernel schedules it. Unless something goes
> + terribly
> + * wrong, we're guaranteed for this to happen during boot, so the big
> + timeout is
> + * a safety net that we never expect to need.
>   * MEI-PXP + HuC load usually takes ~300ms, but if the GSC needs to be
> resumed
>   * and/or reset, this can take longer. Note that the kernel might schedule
>   * other work between the i915 init/resume and the MEI one, which can add
> to @@ -162,7 +162,7 @@ static int gsc_notifier(struct notifier_block *nb,
> unsigned long action, void *d  {
>  	struct device *dev = data;
>  	struct intel_huc *huc = container_of(nb, struct intel_huc,
> delayed_load.nb);
> -	struct intel_gsc_intf *intf = &huc_to_gt(huc)->gsc.intf[0];
> +	struct intel_heci_gsc_intf *intf = &huc_to_gt(huc)->heci_gsc.intf[0];
> 
>  	if (!intf->adev || &intf->adev->aux_dev.dev != dev)
>  		return 0;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> index 4f246416db17..cced240ef8a4 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> @@ -3,7 +3,7 @@
>   * Copyright © 2014-2019 Intel Corporation
>   */
> 
> -#include "gt/intel_gsc.h"
> +#include "gt/intel_heci_gsc.h"
>  #include "gt/intel_gt.h"
>  #include "intel_huc.h"
>  #include "intel_huc_fw.h"
> --
> 2.37.3


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/i915: rename intel_gsc to intel_heci_gsc
  2022-11-03 10:41 ` Winkler, Tomas
@ 2022-11-17 22:47   ` Ceraolo Spurio, Daniele
  2022-11-18 15:17     ` Winkler, Tomas
  0 siblings, 1 reply; 4+ messages in thread
From: Ceraolo Spurio, Daniele @ 2022-11-17 22:47 UTC (permalink / raw)
  To: Winkler, Tomas, intel-gfx; +Cc: Usyskin, Alexander, dri-devel



On 11/3/2022 3:41 AM, Winkler, Tomas wrote:
>> Starting on MTL, the GSC FW is loaded at runtime and will be managed
>> directly by i915. This means we now have a naming clash around the GSC, as
>> we have 2 different aspects of it that we need to handle: the HECI interfaces
>> we export pre-mtl and the new full FW loading and support we have to
>> introduce starting from MTL. To avoid confusion, rename the existing
>> intel_gsc structure to intel_heci_gsc, to make it clear it contains the data
>> related to the HECI interfaces.
>>
> Are you sure you want to take this path, it will make backporting quite difficult.

The diff is relatively small (< 50 lines), so it shouldn't be too bad. 
Otherwise, do you have any suggestion on how to avoid name clashing in a 
different way? I really want to avoid confusion around legacy heci gsc 
and new runtime-loaded gsc. My plan was to name them intel_heci_gsc and 
intel_gsc_uc respectively, to make it super clear which is which, but 
I'm open to alternatives.

Daniele

>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Tomas Winkler <tomas.winkler@intel.com>
>> Cc: Alexander Usyskin <alexander.usyskin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/Makefile                 |  4 +-
>>   drivers/gpu/drm/i915/gt/intel_gt.c            |  4 +-
>>   drivers/gpu/drm/i915/gt/intel_gt.h            |  4 +-
>>   drivers/gpu/drm/i915/gt/intel_gt_irq.c        |  2 +-
>>   drivers/gpu/drm/i915/gt/intel_gt_types.h      |  4 +-
>>   .../i915/gt/{intel_gsc.c => intel_heci_gsc.c} | 43 ++++++++++---------
>> .../i915/gt/{intel_gsc.h => intel_heci_gsc.h} | 22 +++++-----
>>   drivers/gpu/drm/i915/gt/uc/intel_huc.c        | 10 ++---
>>   drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c     |  2 +-
>>   9 files changed, 48 insertions(+), 47 deletions(-)  rename
>> drivers/gpu/drm/i915/gt/{intel_gsc.c => intel_heci_gsc.c} (84%)  rename
>> drivers/gpu/drm/i915/gt/{intel_gsc.h => intel_heci_gsc.h} (52%)
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index 51704b54317c..2fa401dcf087 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -206,8 +206,8 @@ i915-y += gt/uc/intel_uc.o \
>>   	  gt/uc/intel_huc_debugfs.o \
>>   	  gt/uc/intel_huc_fw.o
>>
>> -# graphics system controller (GSC) support -i915-y += gt/intel_gsc.o
>> +# graphics system controller (GSC) HECI support i915-y +=
>> +gt/intel_heci_gsc.o
>>
>>   # graphics hardware monitoring (HWMON) support
>>   i915-$(CONFIG_HWMON) += i915_hwmon.o
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
>> b/drivers/gpu/drm/i915/gt/intel_gt.c
>> index 8e914c4066ed..6ca72479c943 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>> @@ -454,7 +454,7 @@ void intel_gt_chipset_flush(struct intel_gt *gt)
>>
>>   void intel_gt_driver_register(struct intel_gt *gt)  {
>> -	intel_gsc_init(&gt->gsc, gt->i915);
>> +	intel_heci_gsc_init(&gt->heci_gsc, gt->i915);
>>
>>   	intel_rps_driver_register(&gt->rps);
>>
>> @@ -785,7 +785,7 @@ void intel_gt_driver_unregister(struct intel_gt *gt)
>>
>>   	intel_gt_sysfs_unregister(gt);
>>   	intel_rps_driver_unregister(&gt->rps);
>> -	intel_gsc_fini(&gt->gsc);
>> +	intel_heci_gsc_fini(&gt->heci_gsc);
>>
>>   	intel_pxp_fini(&gt->pxp);
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h
>> b/drivers/gpu/drm/i915/gt/intel_gt.h
>> index e0365d556248..43f73239a363 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.h
>> @@ -39,9 +39,9 @@ static inline struct intel_gt *huc_to_gt(struct intel_huc
>> *huc)
>>   	return container_of(huc, struct intel_gt, uc.huc);  }
>>
>> -static inline struct intel_gt *gsc_to_gt(struct intel_gsc *gsc)
>> +static inline struct intel_gt *heci_gsc_to_gt(struct intel_heci_gsc
>> +*heci_gsc)
>>   {
>> -	return container_of(gsc, struct intel_gt, gsc);
>> +	return container_of(heci_gsc, struct intel_gt, heci_gsc);
>>   }
>>
>>   void intel_gt_common_init_early(struct intel_gt *gt); diff --git
>> a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
>> b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
>> index f26882fdc24c..3b4bd237659a 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
>> @@ -75,7 +75,7 @@ gen11_other_irq_handler(struct intel_gt *gt, const u8
>> instance,
>>   		return intel_pxp_irq_handler(&gt->pxp, iir);
>>
>>   	if (instance == OTHER_GSC_INSTANCE)
>> -		return intel_gsc_irq_handler(gt, iir);
>> +		return intel_heci_gsc_irq_handler(gt, iir);
>>
>>   	WARN_ONCE(1, "unhandled other interrupt instance=0x%x,
>> iir=0x%x\n",
>>   		  instance, iir);
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> index a0cc73b401ef..80a0163dcc01 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> @@ -17,7 +17,7 @@
>>   #include <linux/workqueue.h>
>>
>>   #include "uc/intel_uc.h"
>> -#include "intel_gsc.h"
>> +#include "intel_heci_gsc.h"
>>
>>   #include "i915_vma.h"
>>   #include "i915_perf_types.h"
>> @@ -100,7 +100,7 @@ struct intel_gt {
>>   	struct i915_ggtt *ggtt;
>>
>>   	struct intel_uc uc;
>> -	struct intel_gsc gsc;
>> +	struct intel_heci_gsc heci_gsc;
>>
>>   	struct {
>>   		/* Serialize global tlb invalidations */ diff --git
>> a/drivers/gpu/drm/i915/gt/intel_gsc.c
>> b/drivers/gpu/drm/i915/gt/intel_heci_gsc.c
>> similarity index 84%
>> rename from drivers/gpu/drm/i915/gt/intel_gsc.c
>> rename to drivers/gpu/drm/i915/gt/intel_heci_gsc.c
>> index 976fdf27e790..f57771470dba 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gsc.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_heci_gsc.c
>> @@ -9,7 +9,7 @@
>>   #include "i915_reg.h"
>>   #include "gem/i915_gem_lmem.h"
>>   #include "gem/i915_gem_region.h"
>> -#include "gt/intel_gsc.h"
>> +#include "gt/intel_heci_gsc.h"
>>   #include "gt/intel_gt.h"
>>
>>   #define GSC_BAR_LENGTH  0x00000FFC
>> @@ -38,10 +38,11 @@ static int gsc_irq_init(int irq)
>>   	return irq_set_chip_data(irq, NULL);
>>   }
>>
>> -static int
>> -gsc_ext_om_alloc(struct intel_gsc *gsc, struct intel_gsc_intf *intf, size_t
>> size)
>> +static int gsc_ext_om_alloc(struct intel_heci_gsc *gsc,
>> +			    struct intel_heci_gsc_intf *intf,
>> +			    size_t size)
>>   {
>> -	struct intel_gt *gt = gsc_to_gt(gsc);
>> +	struct intel_gt *gt = heci_gsc_to_gt(gsc);
>>   	struct drm_i915_gem_object *obj;
>>   	int err;
>>
>> @@ -68,7 +69,7 @@ gsc_ext_om_alloc(struct intel_gsc *gsc, struct
>> intel_gsc_intf *intf, size_t size
>>   	return err;
>>   }
>>
>> -static void gsc_ext_om_destroy(struct intel_gsc_intf *intf)
>> +static void gsc_ext_om_destroy(struct intel_heci_gsc_intf *intf)
>>   {
>>   	struct drm_i915_gem_object *obj = fetch_and_zero(&intf-
>>> gem_obj);
>> @@ -138,15 +139,15 @@ static void gsc_release_dev(struct device *dev)  }
>>
>>   static void gsc_destroy_one(struct drm_i915_private *i915,
>> -			    struct intel_gsc *gsc, unsigned int intf_id)
>> +			    struct intel_heci_gsc *gsc, unsigned int intf_id)
>>   {
>> -	struct intel_gsc_intf *intf = &gsc->intf[intf_id];
>> +	struct intel_heci_gsc_intf *intf = &gsc->intf[intf_id];
>>
>>   	if (intf->adev) {
>>   		struct auxiliary_device *aux_dev = &intf->adev->aux_dev;
>>
>>   		if (intf_id == 0)
>> -			intel_huc_unregister_gsc_notifier(&gsc_to_gt(gsc)-
>>> uc.huc,
>> +
>> 	intel_huc_unregister_gsc_notifier(&heci_gsc_to_gt(gsc)->uc.huc,
>>   							  aux_dev->dev.bus);
>>
>>   		auxiliary_device_delete(aux_dev);
>> @@ -161,14 +162,14 @@ static void gsc_destroy_one(struct
>> drm_i915_private *i915,
>>   	gsc_ext_om_destroy(intf);
>>   }
>>
>> -static void gsc_init_one(struct drm_i915_private *i915, struct intel_gsc *gsc,
>> +static void gsc_init_one(struct drm_i915_private *i915, struct
>> +intel_heci_gsc *gsc,
>>   			 unsigned int intf_id)
>>   {
>>   	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
>>   	struct mei_aux_device *adev;
>>   	struct auxiliary_device *aux_dev;
>>   	const struct gsc_def *def;
>> -	struct intel_gsc_intf *intf = &gsc->intf[intf_id];
>> +	struct intel_heci_gsc_intf *intf = &gsc->intf[intf_id];
>>   	int ret;
>>
>>   	intf->irq = -1;
>> @@ -252,14 +253,14 @@ static void gsc_init_one(struct drm_i915_private
>> *i915, struct intel_gsc *gsc,
>>   	intf->adev = adev; /* needed by the notifier */
>>
>>   	if (intf_id == 0)
>> -		intel_huc_register_gsc_notifier(&gsc_to_gt(gsc)->uc.huc,
>> +		intel_huc_register_gsc_notifier(&heci_gsc_to_gt(gsc)-
>>> uc.huc,
>>   						aux_dev->dev.bus);
>>
>>   	ret = auxiliary_device_add(aux_dev);
>>   	if (ret < 0) {
>>   		drm_err(&i915->drm, "gsc aux add failed %d\n", ret);
>>   		if (intf_id == 0)
>> -			intel_huc_unregister_gsc_notifier(&gsc_to_gt(gsc)-
>>> uc.huc,
>> +
>> 	intel_huc_unregister_gsc_notifier(&heci_gsc_to_gt(gsc)->uc.huc,
>>   							  aux_dev->dev.bus);
>>   		intf->adev = NULL;
>>
>> @@ -277,7 +278,7 @@ static void gsc_irq_handler(struct intel_gt *gt,
>> unsigned int intf_id)  {
>>   	int ret;
>>
>> -	if (intf_id >= INTEL_GSC_NUM_INTERFACES) {
>> +	if (intf_id >= INTEL_HECI_GSC_NUM_INTERFACES) {
>>   		drm_warn_once(&gt->i915->drm, "GSC irq: intf_id %d is out
>> of range", intf_id);
>>   		return;
>>   	}
>> @@ -287,15 +288,15 @@ static void gsc_irq_handler(struct intel_gt *gt,
>> unsigned int intf_id)
>>   		return;
>>   	}
>>
>> -	if (gt->gsc.intf[intf_id].irq < 0)
>> +	if (gt->heci_gsc.intf[intf_id].irq < 0)
>>   		return;
>>
>> -	ret = generic_handle_irq(gt->gsc.intf[intf_id].irq);
>> +	ret = generic_handle_irq(gt->heci_gsc.intf[intf_id].irq);
>>   	if (ret)
>>   		drm_err_ratelimited(&gt->i915->drm, "error handling GSC
>> irq: %d\n", ret);  }
>>
>> -void intel_gsc_irq_handler(struct intel_gt *gt, u32 iir)
>> +void intel_heci_gsc_irq_handler(struct intel_gt *gt, u32 iir)
>>   {
>>   	if (iir & GSC_IRQ_INTF(0))
>>   		gsc_irq_handler(gt, 0);
>> @@ -303,25 +304,25 @@ void intel_gsc_irq_handler(struct intel_gt *gt, u32
>> iir)
>>   		gsc_irq_handler(gt, 1);
>>   }
>>
>> -void intel_gsc_init(struct intel_gsc *gsc, struct drm_i915_private *i915)
>> +void intel_heci_gsc_init(struct intel_heci_gsc *gsc, struct
>> +drm_i915_private *i915)
>>   {
>>   	unsigned int i;
>>
>>   	if (!HAS_HECI_GSC(i915))
>>   		return;
>>
>> -	for (i = 0; i < INTEL_GSC_NUM_INTERFACES; i++)
>> +	for (i = 0; i < INTEL_HECI_GSC_NUM_INTERFACES; i++)
>>   		gsc_init_one(i915, gsc, i);
>>   }
>>
>> -void intel_gsc_fini(struct intel_gsc *gsc)
>> +void intel_heci_gsc_fini(struct intel_heci_gsc *gsc)
>>   {
>> -	struct intel_gt *gt = gsc_to_gt(gsc);
>> +	struct intel_gt *gt = heci_gsc_to_gt(gsc);
>>   	unsigned int i;
>>
>>   	if (!HAS_HECI_GSC(gt->i915))
>>   		return;
>>
>> -	for (i = 0; i < INTEL_GSC_NUM_INTERFACES; i++)
>> +	for (i = 0; i < INTEL_HECI_GSC_NUM_INTERFACES; i++)
>>   		gsc_destroy_one(gt->i915, gsc, i);
>>   }
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.h
>> b/drivers/gpu/drm/i915/gt/intel_heci_gsc.h
>> similarity index 52%
>> rename from drivers/gpu/drm/i915/gt/intel_gsc.h
>> rename to drivers/gpu/drm/i915/gt/intel_heci_gsc.h
>> index fcac1775e9c3..a20632a4316b 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gsc.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_heci_gsc.h
>> @@ -2,8 +2,8 @@
>>   /*
>>    * Copyright(c) 2019-2022, Intel Corporation. All rights reserved.
>>    */
>> -#ifndef __INTEL_GSC_DEV_H__
>> -#define __INTEL_GSC_DEV_H__
>> +#ifndef __INTEL_HECI_GSC_DEV_H__
>> +#define __INTEL_HECI_GSC_DEV_H__
>>
>>   #include <linux/types.h>
>>
>> @@ -11,7 +11,7 @@ struct drm_i915_private;  struct intel_gt;  struct
>> mei_aux_device;
>>
>> -#define INTEL_GSC_NUM_INTERFACES 2
>> +#define INTEL_HECI_GSC_NUM_INTERFACES 2
>>   /*
>>    * The HECI1 bit corresponds to bit15 and HECI2 to bit14.
>>    * The reason for this is to allow growth for more interfaces in the future.
>> @@ -19,22 +19,22 @@ struct mei_aux_device;  #define GSC_IRQ_INTF(_x)
>> BIT(15 - (_x))
>>
>>   /**
>> - * struct intel_gsc - graphics security controller
>> + * struct intel_heci_gsc - graphics security controller HECI interfaces
>>    *
>>    * @gem_obj: scratch memory GSC operations
>>    * @intf : gsc interface
>>    */
>> -struct intel_gsc {
>> -	struct intel_gsc_intf {
>> +struct intel_heci_gsc {
>> +	struct intel_heci_gsc_intf {
>>   		struct mei_aux_device *adev;
>>   		struct drm_i915_gem_object *gem_obj;
>>   		int irq;
>>   		unsigned int id;
>> -	} intf[INTEL_GSC_NUM_INTERFACES];
>> +	} intf[INTEL_HECI_GSC_NUM_INTERFACES];
>>   };
>>
>> -void intel_gsc_init(struct intel_gsc *gsc, struct drm_i915_private *dev_priv);
>> -void intel_gsc_fini(struct intel_gsc *gsc); -void intel_gsc_irq_handler(struct
>> intel_gt *gt, u32 iir);
>> +void intel_heci_gsc_init(struct intel_heci_gsc *gsc, struct
>> +drm_i915_private *dev_priv); void intel_heci_gsc_fini(struct
>> +intel_heci_gsc *gsc); void intel_heci_gsc_irq_handler(struct intel_gt
>> +*gt, u32 iir);
>>
>> -#endif /* __INTEL_GSC_DEV_H__ */
>> +#endif /* __INTEL_HECI_GSC_DEV_H__ */
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
>> index fbc8bae14f76..0fd7baf5b0cf 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
>> @@ -47,10 +47,10 @@
>>
>>   /*
>>    * MEI-GSC load is an async process. The probing of the exposed aux device
>> - * (see intel_gsc.c) usually happens a few seconds after i915 probe,
>> depending
>> - * on when the kernel schedules it. Unless something goes terribly wrong,
>> we're
>> - * guaranteed for this to happen during boot, so the big timeout is a safety
>> net
>> - * that we never expect to need.
>> + * (see intel_heci_gsc.c) usually happens a few seconds after i915
>> + probe,
>> + * depending on when the kernel schedules it. Unless something goes
>> + terribly
>> + * wrong, we're guaranteed for this to happen during boot, so the big
>> + timeout is
>> + * a safety net that we never expect to need.
>>    * MEI-PXP + HuC load usually takes ~300ms, but if the GSC needs to be
>> resumed
>>    * and/or reset, this can take longer. Note that the kernel might schedule
>>    * other work between the i915 init/resume and the MEI one, which can add
>> to @@ -162,7 +162,7 @@ static int gsc_notifier(struct notifier_block *nb,
>> unsigned long action, void *d  {
>>   	struct device *dev = data;
>>   	struct intel_huc *huc = container_of(nb, struct intel_huc,
>> delayed_load.nb);
>> -	struct intel_gsc_intf *intf = &huc_to_gt(huc)->gsc.intf[0];
>> +	struct intel_heci_gsc_intf *intf = &huc_to_gt(huc)->heci_gsc.intf[0];
>>
>>   	if (!intf->adev || &intf->adev->aux_dev.dev != dev)
>>   		return 0;
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
>> index 4f246416db17..cced240ef8a4 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
>> @@ -3,7 +3,7 @@
>>    * Copyright © 2014-2019 Intel Corporation
>>    */
>>
>> -#include "gt/intel_gsc.h"
>> +#include "gt/intel_heci_gsc.h"
>>   #include "gt/intel_gt.h"
>>   #include "intel_huc.h"
>>   #include "intel_huc_fw.h"
>> --
>> 2.37.3


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH] drm/i915: rename intel_gsc to intel_heci_gsc
  2022-11-17 22:47   ` Ceraolo Spurio, Daniele
@ 2022-11-18 15:17     ` Winkler, Tomas
  0 siblings, 0 replies; 4+ messages in thread
From: Winkler, Tomas @ 2022-11-18 15:17 UTC (permalink / raw)
  To: Ceraolo Spurio, Daniele, intel-gfx; +Cc: Usyskin, Alexander, dri-devel


> 
> 
> On 11/3/2022 3:41 AM, Winkler, Tomas wrote:
> >> Starting on MTL, the GSC FW is loaded at runtime and will be managed
> >> directly by i915. This means we now have a naming clash around the
> >> GSC, as we have 2 different aspects of it that we need to handle: the
> >> HECI interfaces we export pre-mtl and the new full FW loading and
> >> support we have to introduce starting from MTL. To avoid confusion,
> >> rename the existing intel_gsc structure to intel_heci_gsc, to make it
> >> clear it contains the data related to the HECI interfaces.
> >>
> > Are you sure you want to take this path, it will make backporting quite
> difficult.
> 
> The diff is relatively small (< 50 lines), so it shouldn't be too bad.
> Otherwise, do you have any suggestion on how to avoid name clashing in a
> different way? I really want to avoid confusion around legacy heci gsc and
> new runtime-loaded gsc. My plan was to name them intel_heci_gsc and
> intel_gsc_uc respectively, to make it super clear which is which, but I'm open
> to alternatives.

I think if you use intel_gsc_uc and leave the old as it is (at least for few kernel release cylces) it would good. There is always spike of issues with thew new hardware that requires backports to stable kernels. 
That's what I think. 

Thanks
Tomas

> 
> Daniele
> 
> >
> >> Signed-off-by: Daniele Ceraolo Spurio
> >> <daniele.ceraolospurio@intel.com>
> >> Cc: Tomas Winkler <tomas.winkler@intel.com>
> >> Cc: Alexander Usyskin <alexander.usyskin@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/Makefile                 |  4 +-
> >>   drivers/gpu/drm/i915/gt/intel_gt.c            |  4 +-
> >>   drivers/gpu/drm/i915/gt/intel_gt.h            |  4 +-
> >>   drivers/gpu/drm/i915/gt/intel_gt_irq.c        |  2 +-
> >>   drivers/gpu/drm/i915/gt/intel_gt_types.h      |  4 +-
> >>   .../i915/gt/{intel_gsc.c => intel_heci_gsc.c} | 43
> >> ++++++++++--------- .../i915/gt/{intel_gsc.h => intel_heci_gsc.h} | 22
> +++++-----
> >>   drivers/gpu/drm/i915/gt/uc/intel_huc.c        | 10 ++---
> >>   drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c     |  2 +-
> >>   9 files changed, 48 insertions(+), 47 deletions(-)  rename
> >> drivers/gpu/drm/i915/gt/{intel_gsc.c => intel_heci_gsc.c} (84%)
> >> rename drivers/gpu/drm/i915/gt/{intel_gsc.h => intel_heci_gsc.h}
> >> (52%)
> >>
> >> diff --git a/drivers/gpu/drm/i915/Makefile
> >> b/drivers/gpu/drm/i915/Makefile index 51704b54317c..2fa401dcf087
> >> 100644
> >> --- a/drivers/gpu/drm/i915/Makefile
> >> +++ b/drivers/gpu/drm/i915/Makefile
> >> @@ -206,8 +206,8 @@ i915-y += gt/uc/intel_uc.o \
> >>   	  gt/uc/intel_huc_debugfs.o \
> >>   	  gt/uc/intel_huc_fw.o
> >>
> >> -# graphics system controller (GSC) support -i915-y += gt/intel_gsc.o
> >> +# graphics system controller (GSC) HECI support i915-y +=
> >> +gt/intel_heci_gsc.o
> >>
> >>   # graphics hardware monitoring (HWMON) support
> >>   i915-$(CONFIG_HWMON) += i915_hwmon.o diff --git
> >> a/drivers/gpu/drm/i915/gt/intel_gt.c
> >> b/drivers/gpu/drm/i915/gt/intel_gt.c
> >> index 8e914c4066ed..6ca72479c943 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> >> @@ -454,7 +454,7 @@ void intel_gt_chipset_flush(struct intel_gt *gt)
> >>
> >>   void intel_gt_driver_register(struct intel_gt *gt)  {
> >> -	intel_gsc_init(&gt->gsc, gt->i915);
> >> +	intel_heci_gsc_init(&gt->heci_gsc, gt->i915);
> >>
> >>   	intel_rps_driver_register(&gt->rps);
> >>
> >> @@ -785,7 +785,7 @@ void intel_gt_driver_unregister(struct intel_gt
> >> *gt)
> >>
> >>   	intel_gt_sysfs_unregister(gt);
> >>   	intel_rps_driver_unregister(&gt->rps);
> >> -	intel_gsc_fini(&gt->gsc);
> >> +	intel_heci_gsc_fini(&gt->heci_gsc);
> >>
> >>   	intel_pxp_fini(&gt->pxp);
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h
> >> b/drivers/gpu/drm/i915/gt/intel_gt.h
> >> index e0365d556248..43f73239a363 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_gt.h
> >> +++ b/drivers/gpu/drm/i915/gt/intel_gt.h
> >> @@ -39,9 +39,9 @@ static inline struct intel_gt *huc_to_gt(struct
> >> intel_huc
> >> *huc)
> >>   	return container_of(huc, struct intel_gt, uc.huc);  }
> >>
> >> -static inline struct intel_gt *gsc_to_gt(struct intel_gsc *gsc)
> >> +static inline struct intel_gt *heci_gsc_to_gt(struct intel_heci_gsc
> >> +*heci_gsc)
> >>   {
> >> -	return container_of(gsc, struct intel_gt, gsc);
> >> +	return container_of(heci_gsc, struct intel_gt, heci_gsc);
> >>   }
> >>
> >>   void intel_gt_common_init_early(struct intel_gt *gt); diff --git
> >> a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> >> b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> >> index f26882fdc24c..3b4bd237659a 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> >> @@ -75,7 +75,7 @@ gen11_other_irq_handler(struct intel_gt *gt, const
> >> u8 instance,
> >>   		return intel_pxp_irq_handler(&gt->pxp, iir);
> >>
> >>   	if (instance == OTHER_GSC_INSTANCE)
> >> -		return intel_gsc_irq_handler(gt, iir);
> >> +		return intel_heci_gsc_irq_handler(gt, iir);
> >>
> >>   	WARN_ONCE(1, "unhandled other interrupt instance=0x%x,
> >> iir=0x%x\n",
> >>   		  instance, iir);
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> >> b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> >> index a0cc73b401ef..80a0163dcc01 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> >> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> >> @@ -17,7 +17,7 @@
> >>   #include <linux/workqueue.h>
> >>
> >>   #include "uc/intel_uc.h"
> >> -#include "intel_gsc.h"
> >> +#include "intel_heci_gsc.h"
> >>
> >>   #include "i915_vma.h"
> >>   #include "i915_perf_types.h"
> >> @@ -100,7 +100,7 @@ struct intel_gt {
> >>   	struct i915_ggtt *ggtt;
> >>
> >>   	struct intel_uc uc;
> >> -	struct intel_gsc gsc;
> >> +	struct intel_heci_gsc heci_gsc;
> >>
> >>   	struct {
> >>   		/* Serialize global tlb invalidations */ diff --git
> >> a/drivers/gpu/drm/i915/gt/intel_gsc.c
> >> b/drivers/gpu/drm/i915/gt/intel_heci_gsc.c
> >> similarity index 84%
> >> rename from drivers/gpu/drm/i915/gt/intel_gsc.c
> >> rename to drivers/gpu/drm/i915/gt/intel_heci_gsc.c
> >> index 976fdf27e790..f57771470dba 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_gsc.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_heci_gsc.c
> >> @@ -9,7 +9,7 @@
> >>   #include "i915_reg.h"
> >>   #include "gem/i915_gem_lmem.h"
> >>   #include "gem/i915_gem_region.h"
> >> -#include "gt/intel_gsc.h"
> >> +#include "gt/intel_heci_gsc.h"
> >>   #include "gt/intel_gt.h"
> >>
> >>   #define GSC_BAR_LENGTH  0x00000FFC
> >> @@ -38,10 +38,11 @@ static int gsc_irq_init(int irq)
> >>   	return irq_set_chip_data(irq, NULL);
> >>   }
> >>
> >> -static int
> >> -gsc_ext_om_alloc(struct intel_gsc *gsc, struct intel_gsc_intf *intf,
> >> size_t
> >> size)
> >> +static int gsc_ext_om_alloc(struct intel_heci_gsc *gsc,
> >> +			    struct intel_heci_gsc_intf *intf,
> >> +			    size_t size)
> >>   {
> >> -	struct intel_gt *gt = gsc_to_gt(gsc);
> >> +	struct intel_gt *gt = heci_gsc_to_gt(gsc);
> >>   	struct drm_i915_gem_object *obj;
> >>   	int err;
> >>
> >> @@ -68,7 +69,7 @@ gsc_ext_om_alloc(struct intel_gsc *gsc, struct
> >> intel_gsc_intf *intf, size_t size
> >>   	return err;
> >>   }
> >>
> >> -static void gsc_ext_om_destroy(struct intel_gsc_intf *intf)
> >> +static void gsc_ext_om_destroy(struct intel_heci_gsc_intf *intf)
> >>   {
> >>   	struct drm_i915_gem_object *obj = fetch_and_zero(&intf-
> >>> gem_obj);
> >> @@ -138,15 +139,15 @@ static void gsc_release_dev(struct device *dev)
> >> }
> >>
> >>   static void gsc_destroy_one(struct drm_i915_private *i915,
> >> -			    struct intel_gsc *gsc, unsigned int intf_id)
> >> +			    struct intel_heci_gsc *gsc, unsigned int intf_id)
> >>   {
> >> -	struct intel_gsc_intf *intf = &gsc->intf[intf_id];
> >> +	struct intel_heci_gsc_intf *intf = &gsc->intf[intf_id];
> >>
> >>   	if (intf->adev) {
> >>   		struct auxiliary_device *aux_dev = &intf->adev->aux_dev;
> >>
> >>   		if (intf_id == 0)
> >> -			intel_huc_unregister_gsc_notifier(&gsc_to_gt(gsc)-
> >>> uc.huc,
> >> +
> >> 	intel_huc_unregister_gsc_notifier(&heci_gsc_to_gt(gsc)->uc.huc,
> >>   							  aux_dev->dev.bus);
> >>
> >>   		auxiliary_device_delete(aux_dev); @@ -161,14 +162,14 @@
> static
> >> void gsc_destroy_one(struct drm_i915_private *i915,
> >>   	gsc_ext_om_destroy(intf);
> >>   }
> >>
> >> -static void gsc_init_one(struct drm_i915_private *i915, struct
> >> intel_gsc *gsc,
> >> +static void gsc_init_one(struct drm_i915_private *i915, struct
> >> +intel_heci_gsc *gsc,
> >>   			 unsigned int intf_id)
> >>   {
> >>   	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> >>   	struct mei_aux_device *adev;
> >>   	struct auxiliary_device *aux_dev;
> >>   	const struct gsc_def *def;
> >> -	struct intel_gsc_intf *intf = &gsc->intf[intf_id];
> >> +	struct intel_heci_gsc_intf *intf = &gsc->intf[intf_id];
> >>   	int ret;
> >>
> >>   	intf->irq = -1;
> >> @@ -252,14 +253,14 @@ static void gsc_init_one(struct
> >> drm_i915_private *i915, struct intel_gsc *gsc,
> >>   	intf->adev = adev; /* needed by the notifier */
> >>
> >>   	if (intf_id == 0)
> >> -		intel_huc_register_gsc_notifier(&gsc_to_gt(gsc)->uc.huc,
> >> +		intel_huc_register_gsc_notifier(&heci_gsc_to_gt(gsc)-
> >>> uc.huc,
> >>   						aux_dev->dev.bus);
> >>
> >>   	ret = auxiliary_device_add(aux_dev);
> >>   	if (ret < 0) {
> >>   		drm_err(&i915->drm, "gsc aux add failed %d\n", ret);
> >>   		if (intf_id == 0)
> >> -			intel_huc_unregister_gsc_notifier(&gsc_to_gt(gsc)-
> >>> uc.huc,
> >> +
> >> 	intel_huc_unregister_gsc_notifier(&heci_gsc_to_gt(gsc)->uc.huc,
> >>   							  aux_dev->dev.bus);
> >>   		intf->adev = NULL;
> >>
> >> @@ -277,7 +278,7 @@ static void gsc_irq_handler(struct intel_gt *gt,
> >> unsigned int intf_id)  {
> >>   	int ret;
> >>
> >> -	if (intf_id >= INTEL_GSC_NUM_INTERFACES) {
> >> +	if (intf_id >= INTEL_HECI_GSC_NUM_INTERFACES) {
> >>   		drm_warn_once(&gt->i915->drm, "GSC irq: intf_id %d is out
> of
> >> range", intf_id);
> >>   		return;
> >>   	}
> >> @@ -287,15 +288,15 @@ static void gsc_irq_handler(struct intel_gt
> >> *gt, unsigned int intf_id)
> >>   		return;
> >>   	}
> >>
> >> -	if (gt->gsc.intf[intf_id].irq < 0)
> >> +	if (gt->heci_gsc.intf[intf_id].irq < 0)
> >>   		return;
> >>
> >> -	ret = generic_handle_irq(gt->gsc.intf[intf_id].irq);
> >> +	ret = generic_handle_irq(gt->heci_gsc.intf[intf_id].irq);
> >>   	if (ret)
> >>   		drm_err_ratelimited(&gt->i915->drm, "error handling GSC
> >> irq: %d\n", ret);  }
> >>
> >> -void intel_gsc_irq_handler(struct intel_gt *gt, u32 iir)
> >> +void intel_heci_gsc_irq_handler(struct intel_gt *gt, u32 iir)
> >>   {
> >>   	if (iir & GSC_IRQ_INTF(0))
> >>   		gsc_irq_handler(gt, 0);
> >> @@ -303,25 +304,25 @@ void intel_gsc_irq_handler(struct intel_gt *gt,
> >> u32
> >> iir)
> >>   		gsc_irq_handler(gt, 1);
> >>   }
> >>
> >> -void intel_gsc_init(struct intel_gsc *gsc, struct drm_i915_private
> >> *i915)
> >> +void intel_heci_gsc_init(struct intel_heci_gsc *gsc, struct
> >> +drm_i915_private *i915)
> >>   {
> >>   	unsigned int i;
> >>
> >>   	if (!HAS_HECI_GSC(i915))
> >>   		return;
> >>
> >> -	for (i = 0; i < INTEL_GSC_NUM_INTERFACES; i++)
> >> +	for (i = 0; i < INTEL_HECI_GSC_NUM_INTERFACES; i++)
> >>   		gsc_init_one(i915, gsc, i);
> >>   }
> >>
> >> -void intel_gsc_fini(struct intel_gsc *gsc)
> >> +void intel_heci_gsc_fini(struct intel_heci_gsc *gsc)
> >>   {
> >> -	struct intel_gt *gt = gsc_to_gt(gsc);
> >> +	struct intel_gt *gt = heci_gsc_to_gt(gsc);
> >>   	unsigned int i;
> >>
> >>   	if (!HAS_HECI_GSC(gt->i915))
> >>   		return;
> >>
> >> -	for (i = 0; i < INTEL_GSC_NUM_INTERFACES; i++)
> >> +	for (i = 0; i < INTEL_HECI_GSC_NUM_INTERFACES; i++)
> >>   		gsc_destroy_one(gt->i915, gsc, i);
> >>   }
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.h
> >> b/drivers/gpu/drm/i915/gt/intel_heci_gsc.h
> >> similarity index 52%
> >> rename from drivers/gpu/drm/i915/gt/intel_gsc.h
> >> rename to drivers/gpu/drm/i915/gt/intel_heci_gsc.h
> >> index fcac1775e9c3..a20632a4316b 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_gsc.h
> >> +++ b/drivers/gpu/drm/i915/gt/intel_heci_gsc.h
> >> @@ -2,8 +2,8 @@
> >>   /*
> >>    * Copyright(c) 2019-2022, Intel Corporation. All rights reserved.
> >>    */
> >> -#ifndef __INTEL_GSC_DEV_H__
> >> -#define __INTEL_GSC_DEV_H__
> >> +#ifndef __INTEL_HECI_GSC_DEV_H__
> >> +#define __INTEL_HECI_GSC_DEV_H__
> >>
> >>   #include <linux/types.h>
> >>
> >> @@ -11,7 +11,7 @@ struct drm_i915_private;  struct intel_gt;  struct
> >> mei_aux_device;
> >>
> >> -#define INTEL_GSC_NUM_INTERFACES 2
> >> +#define INTEL_HECI_GSC_NUM_INTERFACES 2
> >>   /*
> >>    * The HECI1 bit corresponds to bit15 and HECI2 to bit14.
> >>    * The reason for this is to allow growth for more interfaces in the future.
> >> @@ -19,22 +19,22 @@ struct mei_aux_device;  #define
> GSC_IRQ_INTF(_x)
> >> BIT(15 - (_x))
> >>
> >>   /**
> >> - * struct intel_gsc - graphics security controller
> >> + * struct intel_heci_gsc - graphics security controller HECI
> >> + interfaces
> >>    *
> >>    * @gem_obj: scratch memory GSC operations
> >>    * @intf : gsc interface
> >>    */
> >> -struct intel_gsc {
> >> -	struct intel_gsc_intf {
> >> +struct intel_heci_gsc {
> >> +	struct intel_heci_gsc_intf {
> >>   		struct mei_aux_device *adev;
> >>   		struct drm_i915_gem_object *gem_obj;
> >>   		int irq;
> >>   		unsigned int id;
> >> -	} intf[INTEL_GSC_NUM_INTERFACES];
> >> +	} intf[INTEL_HECI_GSC_NUM_INTERFACES];
> >>   };
> >>
> >> -void intel_gsc_init(struct intel_gsc *gsc, struct drm_i915_private
> >> *dev_priv); -void intel_gsc_fini(struct intel_gsc *gsc); -void
> >> intel_gsc_irq_handler(struct intel_gt *gt, u32 iir);
> >> +void intel_heci_gsc_init(struct intel_heci_gsc *gsc, struct
> >> +drm_i915_private *dev_priv); void intel_heci_gsc_fini(struct
> >> +intel_heci_gsc *gsc); void intel_heci_gsc_irq_handler(struct
> >> +intel_gt *gt, u32 iir);
> >>
> >> -#endif /* __INTEL_GSC_DEV_H__ */
> >> +#endif /* __INTEL_HECI_GSC_DEV_H__ */
> >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> >> b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> >> index fbc8bae14f76..0fd7baf5b0cf 100644
> >> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> >> @@ -47,10 +47,10 @@
> >>
> >>   /*
> >>    * MEI-GSC load is an async process. The probing of the exposed aux
> >> device
> >> - * (see intel_gsc.c) usually happens a few seconds after i915 probe,
> >> depending
> >> - * on when the kernel schedules it. Unless something goes terribly
> >> wrong, we're
> >> - * guaranteed for this to happen during boot, so the big timeout is
> >> a safety net
> >> - * that we never expect to need.
> >> + * (see intel_heci_gsc.c) usually happens a few seconds after i915
> >> + probe,
> >> + * depending on when the kernel schedules it. Unless something goes
> >> + terribly
> >> + * wrong, we're guaranteed for this to happen during boot, so the
> >> + big timeout is
> >> + * a safety net that we never expect to need.
> >>    * MEI-PXP + HuC load usually takes ~300ms, but if the GSC needs to
> >> be resumed
> >>    * and/or reset, this can take longer. Note that the kernel might schedule
> >>    * other work between the i915 init/resume and the MEI one, which
> >> can add to @@ -162,7 +162,7 @@ static int gsc_notifier(struct
> >> notifier_block *nb, unsigned long action, void *d  {
> >>   	struct device *dev = data;
> >>   	struct intel_huc *huc = container_of(nb, struct intel_huc,
> >> delayed_load.nb);
> >> -	struct intel_gsc_intf *intf = &huc_to_gt(huc)->gsc.intf[0];
> >> +	struct intel_heci_gsc_intf *intf =
> >> +&huc_to_gt(huc)->heci_gsc.intf[0];
> >>
> >>   	if (!intf->adev || &intf->adev->aux_dev.dev != dev)
> >>   		return 0;
> >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> >> b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> >> index 4f246416db17..cced240ef8a4 100644
> >> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> >> @@ -3,7 +3,7 @@
> >>    * Copyright © 2014-2019 Intel Corporation
> >>    */
> >>
> >> -#include "gt/intel_gsc.h"
> >> +#include "gt/intel_heci_gsc.h"
> >>   #include "gt/intel_gt.h"
> >>   #include "intel_huc.h"
> >>   #include "intel_huc_fw.h"
> >> --
> >> 2.37.3


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-11-18 15:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-02 21:33 [PATCH] drm/i915: rename intel_gsc to intel_heci_gsc Daniele Ceraolo Spurio
2022-11-03 10:41 ` Winkler, Tomas
2022-11-17 22:47   ` Ceraolo Spurio, Daniele
2022-11-18 15:17     ` Winkler, Tomas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).