[Intel-gfx] [PATCH v2 0/4] Add ops to intel_uc

[Intel-gfx] [PATCH v2 0/4] Add ops to intel_uc

[Michal Wajdeczko]

Instead of spreading multiple conditionals across the uC code
to find out current mode of uC operation, start using predefined
set of function pointers that reflect that mode.

v2: rebased, using macro to generate ops helpers 

Michal Wajdeczko (4):
  drm/i915/uc: Add ops to intel_uc
  drm/i915/uc: Add init_fw/fini_fw to to intel_uc_ops
  drm/i915/uc: Add init/fini to to intel_uc_ops
  drm/i915/uc: Add sanitize to to intel_uc_ops

 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 77 +++++++++++++++++++--------
 drivers/gpu/drm/i915/gt/uc/intel_uc.h | 36 ++++++++++---
 2 files changed, 83 insertions(+), 30 deletions(-)

-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 1/4] drm/i915/uc: Add ops to intel_uc

[Michal Wajdeczko]

Instead of spreading multiple conditionals across the uC code
to find out current mode of uC operation, start using predefined
set of function pointers that reflect that mode.

Begin with pair of init_hw/fini_hw functions that are responsible
for uC hardware initialization and cleanup.

v2: drop ops_none, use macro to generate ops helpers

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 45 ++++++++++++++++++++++-----
 drivers/gpu/drm/i915/gt/uc/intel_uc.h | 21 +++++++++++--
 2 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 3ffc6267f96e..da401e97bba3 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -12,6 +12,19 @@
 
 #include "i915_drv.h"
 
+static int __uc_check_hw(struct intel_uc *uc);
+static int __uc_init_hw(struct intel_uc *uc);
+static void __uc_fini_hw(struct intel_uc *uc);
+
+static const struct intel_uc_ops uc_ops_off = {
+	.init_hw = __uc_check_hw,
+};
+
+static const struct intel_uc_ops uc_ops_on = {
+	.init_hw = __uc_init_hw,
+	.fini_hw = __uc_fini_hw,
+};
+
 /* Reset GuC providing us with fresh state for both GuC and HuC.
  */
 static int __intel_uc_reset_hw(struct intel_uc *uc)
@@ -89,6 +102,11 @@ void intel_uc_init_early(struct intel_uc *uc)
 	intel_huc_init_early(&uc->huc);
 
 	__confirm_options(uc);
+
+	if (intel_uc_uses_guc(uc))
+		uc->ops = &uc_ops_on;
+	else
+		uc->ops = &uc_ops_off;
 }
 
 void intel_uc_driver_late_release(struct intel_uc *uc)
@@ -380,24 +398,37 @@ static bool uc_is_wopcm_locked(struct intel_uc *uc)
 	       (intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET) & GUC_WOPCM_OFFSET_VALID);
 }
 
-int intel_uc_init_hw(struct intel_uc *uc)
+static int __uc_check_hw(struct intel_uc *uc)
+{
+	if (!intel_uc_supports_guc(uc))
+		return 0;
+
+	/*
+	 * We can silently continue without GuC only if it was never enabled
+	 * before on this system after reboot, otherwise we risk GPU hangs.
+	 * To check if GuC was loaded before we look at WOPCM registers.
+	 */
+	if (uc_is_wopcm_locked(uc))
+		return -EIO;
+
+	return 0;
+}
+
+static int __uc_init_hw(struct intel_uc *uc)
 {
 	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
 	struct intel_guc *guc = &uc->guc;
 	struct intel_huc *huc = &uc->huc;
 	int ret, attempts;
 
-	if (!intel_uc_supports_guc(uc))
-		return 0;
+	GEM_BUG_ON(!intel_uc_supports_guc(uc));
+	GEM_BUG_ON(!intel_uc_uses_guc(uc));
 
 	/*
 	 * We can silently continue without GuC only if it was never enabled
 	 * before on this system after reboot, otherwise we risk GPU hangs.
 	 * To check if GuC was loaded before we look at WOPCM registers.
 	 */
-	if (!intel_uc_uses_guc(uc) && !uc_is_wopcm_locked(uc))
-		return 0;
-
 	if (!intel_uc_fw_is_available(&guc->fw)) {
 		ret = uc_is_wopcm_locked(uc) ||
 		      intel_uc_fw_is_overridden(&guc->fw) ||
@@ -495,7 +526,7 @@ int intel_uc_init_hw(struct intel_uc *uc)
 	return -EIO;
 }
 
-void intel_uc_fini_hw(struct intel_uc *uc)
+static void __uc_fini_hw(struct intel_uc *uc)
 {
 	struct intel_guc *guc = &uc->guc;
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index 527995c21196..d5c2d4cf1d38 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -10,7 +10,15 @@
 #include "intel_huc.h"
 #include "i915_params.h"
 
+struct intel_uc;
+
+struct intel_uc_ops {
+	int (*init_hw)(struct intel_uc *uc);
+	void (*fini_hw)(struct intel_uc *uc);
+};
+
 struct intel_uc {
+	struct intel_uc_ops const *ops;
 	struct intel_guc guc;
 	struct intel_huc huc;
 
@@ -25,8 +33,6 @@ void intel_uc_fetch_firmwares(struct intel_uc *uc);
 void intel_uc_cleanup_firmwares(struct intel_uc *uc);
 void intel_uc_sanitize(struct intel_uc *uc);
 void intel_uc_init(struct intel_uc *uc);
-int intel_uc_init_hw(struct intel_uc *uc);
-void intel_uc_fini_hw(struct intel_uc *uc);
 void intel_uc_fini(struct intel_uc *uc);
 void intel_uc_reset_prepare(struct intel_uc *uc);
 void intel_uc_suspend(struct intel_uc *uc);
@@ -64,4 +70,15 @@ static inline bool intel_uc_uses_huc(struct intel_uc *uc)
 	return intel_huc_is_enabled(&uc->huc);
 }
 
+#define intel_uc_ops_function(_NAME, _OPS, _TYPE, _RET) \
+static inline _TYPE intel_uc_##_NAME(struct intel_uc *uc) \
+{ \
+	if (uc->ops->_OPS) \
+		return uc->ops->_OPS(uc); \
+	return _RET; \
+}
+intel_uc_ops_function(init_hw, init_hw, int, 0);
+intel_uc_ops_function(fini_hw, fini_hw, void, );
+#undef intel_uc_ops_function
+
 #endif
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 2/4] drm/i915/uc: Add init_fw/fini_fw to to intel_uc_ops

[Michal Wajdeczko]

Firmware fetching and cleanup steps are only meaningful if we are
running with uC enabled. Make these functions part of the uc_ops.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 12 ++++++++----
 drivers/gpu/drm/i915/gt/uc/intel_uc.h |  6 ++++--
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index da401e97bba3..133cbc360a7d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -12,6 +12,8 @@
 
 #include "i915_drv.h"
 
+static void __uc_fetch_firmwares(struct intel_uc *uc);
+static void __uc_cleanup_firmwares(struct intel_uc *uc);
 static int __uc_check_hw(struct intel_uc *uc);
 static int __uc_init_hw(struct intel_uc *uc);
 static void __uc_fini_hw(struct intel_uc *uc);
@@ -21,6 +23,9 @@ static const struct intel_uc_ops uc_ops_off = {
 };
 
 static const struct intel_uc_ops uc_ops_on = {
+	.init_fw = __uc_fetch_firmwares,
+	.fini_fw = __uc_cleanup_firmwares,
+
 	.init_hw = __uc_init_hw,
 	.fini_hw = __uc_fini_hw,
 };
@@ -263,12 +268,11 @@ static void guc_disable_communication(struct intel_guc *guc)
 	DRM_INFO("GuC communication disabled\n");
 }
 
-void intel_uc_fetch_firmwares(struct intel_uc *uc)
+static void __uc_fetch_firmwares(struct intel_uc *uc)
 {
 	int err;
 
-	if (!intel_uc_uses_guc(uc))
-		return;
+	GEM_BUG_ON(!intel_uc_uses_guc(uc));
 
 	err = intel_uc_fw_fetch(&uc->guc.fw);
 	if (err)
@@ -278,7 +282,7 @@ void intel_uc_fetch_firmwares(struct intel_uc *uc)
 		intel_uc_fw_fetch(&uc->huc.fw);
 }
 
-void intel_uc_cleanup_firmwares(struct intel_uc *uc)
+static void __uc_cleanup_firmwares(struct intel_uc *uc)
 {
 	intel_uc_fw_cleanup_fetch(&uc->huc.fw);
 	intel_uc_fw_cleanup_fetch(&uc->guc.fw);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index d5c2d4cf1d38..47c7c8add451 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -13,6 +13,8 @@
 struct intel_uc;
 
 struct intel_uc_ops {
+	void (*init_fw)(struct intel_uc *uc);
+	void (*fini_fw)(struct intel_uc *uc);
 	int (*init_hw)(struct intel_uc *uc);
 	void (*fini_hw)(struct intel_uc *uc);
 };
@@ -29,8 +31,6 @@ struct intel_uc {
 void intel_uc_init_early(struct intel_uc *uc);
 void intel_uc_driver_late_release(struct intel_uc *uc);
 void intel_uc_init_mmio(struct intel_uc *uc);
-void intel_uc_fetch_firmwares(struct intel_uc *uc);
-void intel_uc_cleanup_firmwares(struct intel_uc *uc);
 void intel_uc_sanitize(struct intel_uc *uc);
 void intel_uc_init(struct intel_uc *uc);
 void intel_uc_fini(struct intel_uc *uc);
@@ -77,6 +77,8 @@ static inline _TYPE intel_uc_##_NAME(struct intel_uc *uc) \
 		return uc->ops->_OPS(uc); \
 	return _RET; \
 }
+intel_uc_ops_function(fetch_firmwares, init_fw, void, );
+intel_uc_ops_function(cleanup_firmwares, fini_fw, void, );
 intel_uc_ops_function(init_hw, init_hw, int, 0);
 intel_uc_ops_function(fini_hw, fini_hw, void, );
 #undef intel_uc_ops_function
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 3/4] drm/i915/uc: Add init/fini to to intel_uc_ops

[Michal Wajdeczko]

uC preparation and cleanup steps are only meaningful if we are
running with uC enabled. Make these functions part of the uc_ops.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 12 +++++++-----
 drivers/gpu/drm/i915/gt/uc/intel_uc.h |  6 ++++--
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 133cbc360a7d..1b07135a8515 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -14,6 +14,8 @@
 
 static void __uc_fetch_firmwares(struct intel_uc *uc);
 static void __uc_cleanup_firmwares(struct intel_uc *uc);
+static void __uc_init(struct intel_uc *uc);
+static void __uc_fini(struct intel_uc *uc);
 static int __uc_check_hw(struct intel_uc *uc);
 static int __uc_init_hw(struct intel_uc *uc);
 static void __uc_fini_hw(struct intel_uc *uc);
@@ -25,7 +27,8 @@ static const struct intel_uc_ops uc_ops_off = {
 static const struct intel_uc_ops uc_ops_on = {
 	.init_fw = __uc_fetch_firmwares,
 	.fini_fw = __uc_cleanup_firmwares,
-
+	.init = __uc_init,
+	.fini = __uc_fini,
 	.init_hw = __uc_init_hw,
 	.fini_hw = __uc_fini_hw,
 };
@@ -288,14 +291,13 @@ static void __uc_cleanup_firmwares(struct intel_uc *uc)
 	intel_uc_fw_cleanup_fetch(&uc->guc.fw);
 }
 
-void intel_uc_init(struct intel_uc *uc)
+static void __uc_init(struct intel_uc *uc)
 {
 	struct intel_guc *guc = &uc->guc;
 	struct intel_huc *huc = &uc->huc;
 	int ret;
 
-	if (!intel_uc_uses_guc(uc))
-		return;
+	GEM_BUG_ON(!intel_uc_uses_guc(uc));
 
 	/* XXX: GuC submission is unavailable for now */
 	GEM_BUG_ON(intel_uc_supports_guc_submission(uc));
@@ -310,7 +312,7 @@ void intel_uc_init(struct intel_uc *uc)
 		intel_huc_init(huc);
 }
 
-void intel_uc_fini(struct intel_uc *uc)
+static void __uc_fini(struct intel_uc *uc)
 {
 	intel_huc_fini(&uc->huc);
 	intel_guc_fini(&uc->guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index 47c7c8add451..8c0ce0d9f190 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -15,6 +15,8 @@ struct intel_uc;
 struct intel_uc_ops {
 	void (*init_fw)(struct intel_uc *uc);
 	void (*fini_fw)(struct intel_uc *uc);
+	void (*init)(struct intel_uc *uc);
+	void (*fini)(struct intel_uc *uc);
 	int (*init_hw)(struct intel_uc *uc);
 	void (*fini_hw)(struct intel_uc *uc);
 };
@@ -32,8 +34,6 @@ void intel_uc_init_early(struct intel_uc *uc);
 void intel_uc_driver_late_release(struct intel_uc *uc);
 void intel_uc_init_mmio(struct intel_uc *uc);
 void intel_uc_sanitize(struct intel_uc *uc);
-void intel_uc_init(struct intel_uc *uc);
-void intel_uc_fini(struct intel_uc *uc);
 void intel_uc_reset_prepare(struct intel_uc *uc);
 void intel_uc_suspend(struct intel_uc *uc);
 void intel_uc_runtime_suspend(struct intel_uc *uc);
@@ -79,6 +79,8 @@ static inline _TYPE intel_uc_##_NAME(struct intel_uc *uc) \
 }
 intel_uc_ops_function(fetch_firmwares, init_fw, void, );
 intel_uc_ops_function(cleanup_firmwares, fini_fw, void, );
+intel_uc_ops_function(init, init, void, );
+intel_uc_ops_function(fini, fini, void, );
 intel_uc_ops_function(init_hw, init_hw, int, 0);
 intel_uc_ops_function(fini_hw, fini_hw, void, );
 #undef intel_uc_ops_function
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 4/4] drm/i915/uc: Add sanitize to to intel_uc_ops

[Michal Wajdeczko]

uC sanitization is only meaningful if we are running with uC present
or enabled. Make this function part of the uc_ops.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 10 ++--------
 drivers/gpu/drm/i915/gt/uc/intel_uc.h |  3 ++-
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 1b07135a8515..c1d5af775713 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -12,6 +12,7 @@
 
 #include "i915_drv.h"
 
+static int __uc_sanitize(struct intel_uc *uc);
 static void __uc_fetch_firmwares(struct intel_uc *uc);
 static void __uc_cleanup_firmwares(struct intel_uc *uc);
 static void __uc_init(struct intel_uc *uc);
@@ -25,6 +26,7 @@ static const struct intel_uc_ops uc_ops_off = {
 };
 
 static const struct intel_uc_ops uc_ops_on = {
+	.sanitize = __uc_sanitize,
 	.init_fw = __uc_fetch_firmwares,
 	.fini_fw = __uc_cleanup_firmwares,
 	.init = __uc_init,
@@ -333,14 +335,6 @@ static int __uc_sanitize(struct intel_uc *uc)
 	return __intel_uc_reset_hw(uc);
 }
 
-void intel_uc_sanitize(struct intel_uc *uc)
-{
-	if (!intel_uc_supports_guc(uc))
-		return;
-
-	__uc_sanitize(uc);
-}
-
 /* Initialize and verify the uC regs related to uC positioning in WOPCM */
 static int uc_init_wopcm(struct intel_uc *uc)
 {
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index 8c0ce0d9f190..49c913524686 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -13,6 +13,7 @@
 struct intel_uc;
 
 struct intel_uc_ops {
+	int (*sanitize)(struct intel_uc *uc);
 	void (*init_fw)(struct intel_uc *uc);
 	void (*fini_fw)(struct intel_uc *uc);
 	void (*init)(struct intel_uc *uc);
@@ -33,7 +34,6 @@ struct intel_uc {
 void intel_uc_init_early(struct intel_uc *uc);
 void intel_uc_driver_late_release(struct intel_uc *uc);
 void intel_uc_init_mmio(struct intel_uc *uc);
-void intel_uc_sanitize(struct intel_uc *uc);
 void intel_uc_reset_prepare(struct intel_uc *uc);
 void intel_uc_suspend(struct intel_uc *uc);
 void intel_uc_runtime_suspend(struct intel_uc *uc);
@@ -77,6 +77,7 @@ static inline _TYPE intel_uc_##_NAME(struct intel_uc *uc) \
 		return uc->ops->_OPS(uc); \
 	return _RET; \
 }
+intel_uc_ops_function(sanitize, sanitize, int, 0);
 intel_uc_ops_function(fetch_firmwares, init_fw, void, );
 intel_uc_ops_function(cleanup_firmwares, fini_fw, void, );
 intel_uc_ops_function(init, init, void, );
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/uc: Add ops to intel_uc

[Chris Wilson]

Quoting Michal Wajdeczko (2020-01-10 16:29:27)
> Instead of spreading multiple conditionals across the uC code
> to find out current mode of uC operation, start using predefined
> set of function pointers that reflect that mode.
> 
> Begin with pair of init_hw/fini_hw functions that are responsible
> for uC hardware initialization and cleanup.
> 
> v2: drop ops_none, use macro to generate ops helpers
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c | 45 ++++++++++++++++++++++-----
>  drivers/gpu/drm/i915/gt/uc/intel_uc.h | 21 +++++++++++--
>  2 files changed, 57 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 3ffc6267f96e..da401e97bba3 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -12,6 +12,19 @@
>  
>  #include "i915_drv.h"
>  
> +static int __uc_check_hw(struct intel_uc *uc);
> +static int __uc_init_hw(struct intel_uc *uc);
> +static void __uc_fini_hw(struct intel_uc *uc);
> +
> +static const struct intel_uc_ops uc_ops_off = {
> +       .init_hw = __uc_check_hw,
> +};
> +
> +static const struct intel_uc_ops uc_ops_on = {
> +       .init_hw = __uc_init_hw,
> +       .fini_hw = __uc_fini_hw,
> +};
> +
>  /* Reset GuC providing us with fresh state for both GuC and HuC.
>   */
>  static int __intel_uc_reset_hw(struct intel_uc *uc)
> @@ -89,6 +102,11 @@ void intel_uc_init_early(struct intel_uc *uc)
>         intel_huc_init_early(&uc->huc);
>  
>         __confirm_options(uc);
> +
> +       if (intel_uc_uses_guc(uc))
> +               uc->ops = &uc_ops_on;
> +       else
> +               uc->ops = &uc_ops_off;
>  }
>  
>  void intel_uc_driver_late_release(struct intel_uc *uc)
> @@ -380,24 +398,37 @@ static bool uc_is_wopcm_locked(struct intel_uc *uc)
>                (intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET) & GUC_WOPCM_OFFSET_VALID);
>  }
>  
> -int intel_uc_init_hw(struct intel_uc *uc)
> +static int __uc_check_hw(struct intel_uc *uc)
> +{
> +       if (!intel_uc_supports_guc(uc))
> +               return 0;
> +
> +       /*
> +        * We can silently continue without GuC only if it was never enabled
> +        * before on this system after reboot, otherwise we risk GPU hangs.
> +        * To check if GuC was loaded before we look at WOPCM registers.
> +        */
> +       if (uc_is_wopcm_locked(uc))
> +               return -EIO;
> +
> +       return 0;
> +}
> +
> +static int __uc_init_hw(struct intel_uc *uc)
>  {
>         struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
>         struct intel_guc *guc = &uc->guc;
>         struct intel_huc *huc = &uc->huc;
>         int ret, attempts;
>  
> -       if (!intel_uc_supports_guc(uc))
> -               return 0;
> +       GEM_BUG_ON(!intel_uc_supports_guc(uc));
> +       GEM_BUG_ON(!intel_uc_uses_guc(uc));
>  
>         /*
>          * We can silently continue without GuC only if it was never enabled
>          * before on this system after reboot, otherwise we risk GPU hangs.
>          * To check if GuC was loaded before we look at WOPCM registers.
>          */

This comment still required?
Shouldn't there be a call here to __uc_check_hw() instead?

> -       if (!intel_uc_uses_guc(uc) && !uc_is_wopcm_locked(uc))
> -               return 0;
> -
>         if (!intel_uc_fw_is_available(&guc->fw)) {
>                 ret = uc_is_wopcm_locked(uc) ||
>                       intel_uc_fw_is_overridden(&guc->fw) ||
> @@ -495,7 +526,7 @@ int intel_uc_init_hw(struct intel_uc *uc)
>         return -EIO;
>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/uc: Add ops to intel_uc

[Michal Wajdeczko]

On Fri, 10 Jan 2020 17:57:22 +0100, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2020-01-10 16:29:27)
>> Instead of spreading multiple conditionals across the uC code
>> to find out current mode of uC operation, start using predefined
>> set of function pointers that reflect that mode.
>>
>> Begin with pair of init_hw/fini_hw functions that are responsible
>> for uC hardware initialization and cleanup.
>>
>> v2: drop ops_none, use macro to generate ops helpers
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> ---
>>  drivers/gpu/drm/i915/gt/uc/intel_uc.c | 45 ++++++++++++++++++++++-----
>>  drivers/gpu/drm/i915/gt/uc/intel_uc.h | 21 +++++++++++--
>>  2 files changed, 57 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c  
>> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> index 3ffc6267f96e..da401e97bba3 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> @@ -12,6 +12,19 @@
>>
>>  #include "i915_drv.h"
>>
>> +static int __uc_check_hw(struct intel_uc *uc);
>> +static int __uc_init_hw(struct intel_uc *uc);
>> +static void __uc_fini_hw(struct intel_uc *uc);
>> +
>> +static const struct intel_uc_ops uc_ops_off = {
>> +       .init_hw = __uc_check_hw,
>> +};
>> +
>> +static const struct intel_uc_ops uc_ops_on = {
>> +       .init_hw = __uc_init_hw,
>> +       .fini_hw = __uc_fini_hw,
>> +};
>> +
>>  /* Reset GuC providing us with fresh state for both GuC and HuC.
>>   */
>>  static int __intel_uc_reset_hw(struct intel_uc *uc)
>> @@ -89,6 +102,11 @@ void intel_uc_init_early(struct intel_uc *uc)
>>         intel_huc_init_early(&uc->huc);
>>
>>         __confirm_options(uc);
>> +
>> +       if (intel_uc_uses_guc(uc))
>> +               uc->ops = &uc_ops_on;
>> +       else
>> +               uc->ops = &uc_ops_off;
>>  }
>>
>>  void intel_uc_driver_late_release(struct intel_uc *uc)
>> @@ -380,24 +398,37 @@ static bool uc_is_wopcm_locked(struct intel_uc  
>> *uc)
>>                (intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET) &  
>> GUC_WOPCM_OFFSET_VALID);
>>  }
>>
>> -int intel_uc_init_hw(struct intel_uc *uc)
>> +static int __uc_check_hw(struct intel_uc *uc)
>> +{
>> +       if (!intel_uc_supports_guc(uc))
>> +               return 0;
>> +
>> +       /*
>> +        * We can silently continue without GuC only if it was never  
>> enabled
>> +        * before on this system after reboot, otherwise we risk GPU  
>> hangs.
>> +        * To check if GuC was loaded before we look at WOPCM registers.
>> +        */
>> +       if (uc_is_wopcm_locked(uc))
>> +               return -EIO;
>> +
>> +       return 0;
>> +}
>> +
>> +static int __uc_init_hw(struct intel_uc *uc)
>>  {
>>         struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
>>         struct intel_guc *guc = &uc->guc;
>>         struct intel_huc *huc = &uc->huc;
>>         int ret, attempts;
>>
>> -       if (!intel_uc_supports_guc(uc))
>> -               return 0;
>> +       GEM_BUG_ON(!intel_uc_supports_guc(uc));
>> +       GEM_BUG_ON(!intel_uc_uses_guc(uc));
>>
>>         /*
>>          * We can silently continue without GuC only if it was never  
>> enabled
>>          * before on this system after reboot, otherwise we risk GPU  
>> hangs.
>>          * To check if GuC was loaded before we look at WOPCM registers.
>>          */
>
> This comment still required?
> Shouldn't there be a call here to __uc_check_hw() instead?

ok, will replace below call to uc_is_wopcm_locked() with __uc_check_hw()
to avoid redundant (but otherwise still valid) comment

>
>> -       if (!intel_uc_uses_guc(uc) && !uc_is_wopcm_locked(uc))
>> -               return 0;
>> -
>>         if (!intel_uc_fw_is_available(&guc->fw)) {
>>                 ret = uc_is_wopcm_locked(uc) ||

                          ^^^^^^^^^^^^^^^^^^

>>                       intel_uc_fw_is_overridden(&guc->fw) ||
>> @@ -495,7 +526,7 @@ int intel_uc_init_hw(struct intel_uc *uc)
>>         return -EIO;
>>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/uc: Add ops to intel_uc

[Chris Wilson]

Quoting Michal Wajdeczko (2020-01-10 18:47:50)
> On Fri, 10 Jan 2020 17:57:22 +0100, Chris Wilson  
> <chris@chris-wilson.co.uk> wrote:
> 
> > Quoting Michal Wajdeczko (2020-01-10 16:29:27)
> >> Instead of spreading multiple conditionals across the uC code
> >> to find out current mode of uC operation, start using predefined
> >> set of function pointers that reflect that mode.
> >>
> >> Begin with pair of init_hw/fini_hw functions that are responsible
> >> for uC hardware initialization and cleanup.
> >>
> >> v2: drop ops_none, use macro to generate ops helpers
> >>
> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/gt/uc/intel_uc.c | 45 ++++++++++++++++++++++-----
> >>  drivers/gpu/drm/i915/gt/uc/intel_uc.h | 21 +++++++++++--
> >>  2 files changed, 57 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c  
> >> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> >> index 3ffc6267f96e..da401e97bba3 100644
> >> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> >> @@ -12,6 +12,19 @@
> >>
> >>  #include "i915_drv.h"
> >>
> >> +static int __uc_check_hw(struct intel_uc *uc);
> >> +static int __uc_init_hw(struct intel_uc *uc);
> >> +static void __uc_fini_hw(struct intel_uc *uc);
> >> +
> >> +static const struct intel_uc_ops uc_ops_off = {
> >> +       .init_hw = __uc_check_hw,
> >> +};
> >> +
> >> +static const struct intel_uc_ops uc_ops_on = {
> >> +       .init_hw = __uc_init_hw,
> >> +       .fini_hw = __uc_fini_hw,
> >> +};
> >> +
> >>  /* Reset GuC providing us with fresh state for both GuC and HuC.
> >>   */
> >>  static int __intel_uc_reset_hw(struct intel_uc *uc)
> >> @@ -89,6 +102,11 @@ void intel_uc_init_early(struct intel_uc *uc)
> >>         intel_huc_init_early(&uc->huc);
> >>
> >>         __confirm_options(uc);
> >> +
> >> +       if (intel_uc_uses_guc(uc))
> >> +               uc->ops = &uc_ops_on;
> >> +       else
> >> +               uc->ops = &uc_ops_off;
> >>  }
> >>
> >>  void intel_uc_driver_late_release(struct intel_uc *uc)
> >> @@ -380,24 +398,37 @@ static bool uc_is_wopcm_locked(struct intel_uc  
> >> *uc)
> >>                (intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET) &  
> >> GUC_WOPCM_OFFSET_VALID);
> >>  }
> >>
> >> -int intel_uc_init_hw(struct intel_uc *uc)
> >> +static int __uc_check_hw(struct intel_uc *uc)
> >> +{
> >> +       if (!intel_uc_supports_guc(uc))
> >> +               return 0;
> >> +
> >> +       /*
> >> +        * We can silently continue without GuC only if it was never  
> >> enabled
> >> +        * before on this system after reboot, otherwise we risk GPU  
> >> hangs.
> >> +        * To check if GuC was loaded before we look at WOPCM registers.
> >> +        */
> >> +       if (uc_is_wopcm_locked(uc))
> >> +               return -EIO;
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static int __uc_init_hw(struct intel_uc *uc)
> >>  {
> >>         struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
> >>         struct intel_guc *guc = &uc->guc;
> >>         struct intel_huc *huc = &uc->huc;
> >>         int ret, attempts;
> >>
> >> -       if (!intel_uc_supports_guc(uc))
> >> -               return 0;
> >> +       GEM_BUG_ON(!intel_uc_supports_guc(uc));
> >> +       GEM_BUG_ON(!intel_uc_uses_guc(uc));
> >>
> >>         /*
> >>          * We can silently continue without GuC only if it was never  
> >> enabled
> >>          * before on this system after reboot, otherwise we risk GPU  
> >> hangs.
> >>          * To check if GuC was loaded before we look at WOPCM registers.
> >>          */
> >
> > This comment still required?
> > Shouldn't there be a call here to __uc_check_hw() instead?
> 
> ok, will replace below call to uc_is_wopcm_locked() with __uc_check_hw()
> to avoid redundant (but otherwise still valid) comment
> 
> >
> >> -       if (!intel_uc_uses_guc(uc) && !uc_is_wopcm_locked(uc))
> >> -               return 0;
> >> -
> >>         if (!intel_uc_fw_is_available(&guc->fw)) {
> >>                 ret = uc_is_wopcm_locked(uc) ||
> 
>                           ^^^^^^^^^^^^^^^^^^

Ah, I see.

I think I still like the chaining up to __uc_check_hw() for the
continuity aspect, and only being scared once by the warning.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v3 1/4] drm/i915/uc: Add ops to intel_uc

[Michal Wajdeczko]

Instead of spreading multiple conditionals across the uC code
to find out current mode of uC operation, start using predefined
set of function pointers that reflect that mode.

Begin with pair of init_hw/fini_hw functions that are responsible
for uC hardware initialization and cleanup.

v2: drop ops_none, use macro to generate ops helpers
v3: reuse __uc_check_hw to avoid redundant comment

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 46 +++++++++++++++++++++------
 drivers/gpu/drm/i915/gt/uc/intel_uc.h | 21 ++++++++++--
 2 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 3ffc6267f96e..5e9b3fa7bc74 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -12,6 +12,19 @@
 
 #include "i915_drv.h"
 
+static int __uc_check_hw(struct intel_uc *uc);
+static int __uc_init_hw(struct intel_uc *uc);
+static void __uc_fini_hw(struct intel_uc *uc);
+
+static const struct intel_uc_ops uc_ops_off = {
+	.init_hw = __uc_check_hw,
+};
+
+static const struct intel_uc_ops uc_ops_on = {
+	.init_hw = __uc_init_hw,
+	.fini_hw = __uc_fini_hw,
+};
+
 /* Reset GuC providing us with fresh state for both GuC and HuC.
  */
 static int __intel_uc_reset_hw(struct intel_uc *uc)
@@ -89,6 +102,11 @@ void intel_uc_init_early(struct intel_uc *uc)
 	intel_huc_init_early(&uc->huc);
 
 	__confirm_options(uc);
+
+	if (intel_uc_uses_guc(uc))
+		uc->ops = &uc_ops_on;
+	else
+		uc->ops = &uc_ops_off;
 }
 
 void intel_uc_driver_late_release(struct intel_uc *uc)
@@ -380,13 +398,8 @@ static bool uc_is_wopcm_locked(struct intel_uc *uc)
 	       (intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET) & GUC_WOPCM_OFFSET_VALID);
 }
 
-int intel_uc_init_hw(struct intel_uc *uc)
+static int __uc_check_hw(struct intel_uc *uc)
 {
-	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
-	struct intel_guc *guc = &uc->guc;
-	struct intel_huc *huc = &uc->huc;
-	int ret, attempts;
-
 	if (!intel_uc_supports_guc(uc))
 		return 0;
 
@@ -395,11 +408,24 @@ int intel_uc_init_hw(struct intel_uc *uc)
 	 * before on this system after reboot, otherwise we risk GPU hangs.
 	 * To check if GuC was loaded before we look at WOPCM registers.
 	 */
-	if (!intel_uc_uses_guc(uc) && !uc_is_wopcm_locked(uc))
-		return 0;
+	if (uc_is_wopcm_locked(uc))
+		return -EIO;
+
+	return 0;
+}
+
+static int __uc_init_hw(struct intel_uc *uc)
+{
+	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
+	struct intel_guc *guc = &uc->guc;
+	struct intel_huc *huc = &uc->huc;
+	int ret, attempts;
+
+	GEM_BUG_ON(!intel_uc_supports_guc(uc));
+	GEM_BUG_ON(!intel_uc_uses_guc(uc));
 
 	if (!intel_uc_fw_is_available(&guc->fw)) {
-		ret = uc_is_wopcm_locked(uc) ||
+		ret = __uc_check_hw(uc) ||
 		      intel_uc_fw_is_overridden(&guc->fw) ||
 		      intel_uc_supports_guc_submission(uc) ?
 		      intel_uc_fw_status_to_error(guc->fw.status) : 0;
@@ -495,7 +521,7 @@ int intel_uc_init_hw(struct intel_uc *uc)
 	return -EIO;
 }
 
-void intel_uc_fini_hw(struct intel_uc *uc)
+static void __uc_fini_hw(struct intel_uc *uc)
 {
 	struct intel_guc *guc = &uc->guc;
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index 527995c21196..d5c2d4cf1d38 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -10,7 +10,15 @@
 #include "intel_huc.h"
 #include "i915_params.h"
 
+struct intel_uc;
+
+struct intel_uc_ops {
+	int (*init_hw)(struct intel_uc *uc);
+	void (*fini_hw)(struct intel_uc *uc);
+};
+
 struct intel_uc {
+	struct intel_uc_ops const *ops;
 	struct intel_guc guc;
 	struct intel_huc huc;
 
@@ -25,8 +33,6 @@ void intel_uc_fetch_firmwares(struct intel_uc *uc);
 void intel_uc_cleanup_firmwares(struct intel_uc *uc);
 void intel_uc_sanitize(struct intel_uc *uc);
 void intel_uc_init(struct intel_uc *uc);
-int intel_uc_init_hw(struct intel_uc *uc);
-void intel_uc_fini_hw(struct intel_uc *uc);
 void intel_uc_fini(struct intel_uc *uc);
 void intel_uc_reset_prepare(struct intel_uc *uc);
 void intel_uc_suspend(struct intel_uc *uc);
@@ -64,4 +70,15 @@ static inline bool intel_uc_uses_huc(struct intel_uc *uc)
 	return intel_huc_is_enabled(&uc->huc);
 }
 
+#define intel_uc_ops_function(_NAME, _OPS, _TYPE, _RET) \
+static inline _TYPE intel_uc_##_NAME(struct intel_uc *uc) \
+{ \
+	if (uc->ops->_OPS) \
+		return uc->ops->_OPS(uc); \
+	return _RET; \
+}
+intel_uc_ops_function(init_hw, init_hw, int, 0);
+intel_uc_ops_function(fini_hw, fini_hw, void, );
+#undef intel_uc_ops_function
+
 #endif
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/uc: Add sanitize to to intel_uc_ops

[Chris Wilson]

Quoting Michal Wajdeczko (2020-01-10 16:29:30)
> uC sanitization is only meaningful if we are running with uC present
> or enabled. Make this function part of the uc_ops.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Series is 
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c | 10 ++--------
>  drivers/gpu/drm/i915/gt/uc/intel_uc.h |  3 ++-
>  2 files changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 1b07135a8515..c1d5af775713 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -12,6 +12,7 @@
>  
>  #include "i915_drv.h"
>  
> +static int __uc_sanitize(struct intel_uc *uc);
>  static void __uc_fetch_firmwares(struct intel_uc *uc);
>  static void __uc_cleanup_firmwares(struct intel_uc *uc);
>  static void __uc_init(struct intel_uc *uc);
> @@ -25,6 +26,7 @@ static const struct intel_uc_ops uc_ops_off = {
>  };
>  
>  static const struct intel_uc_ops uc_ops_on = {
> +       .sanitize = __uc_sanitize,
>         .init_fw = __uc_fetch_firmwares,
>         .fini_fw = __uc_cleanup_firmwares,
>         .init = __uc_init,

The only nit is that I would use whitespace here more consistently here
to break up the phases, and I would suggest making the ops forwards
declared rather than every function.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Add ops to intel_uc (rev3)

[Patchwork]

== Series Details ==

Series: Add ops to intel_uc (rev3)
URL   : https://patchwork.freedesktop.org/series/70716/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
bb5e4aea2877 drm/i915/uc: Add ops to intel_uc
-:142: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#142: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc.h:73:
+#define intel_uc_ops_function(_NAME, _OPS, _TYPE, _RET) \
+static inline _TYPE intel_uc_##_NAME(struct intel_uc *uc) \
+{ \
+	if (uc->ops->_OPS) \
+		return uc->ops->_OPS(uc); \
+	return _RET; \
+}

-:142: CHECK:MACRO_ARG_REUSE: Macro argument reuse '_OPS' - possible side-effects?
#142: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc.h:73:
+#define intel_uc_ops_function(_NAME, _OPS, _TYPE, _RET) \
+static inline _TYPE intel_uc_##_NAME(struct intel_uc *uc) \
+{ \
+	if (uc->ops->_OPS) \
+		return uc->ops->_OPS(uc); \
+	return _RET; \
+}

-:142: CHECK:MACRO_ARG_PRECEDENCE: Macro argument '_OPS' may be better as '(_OPS)' to avoid precedence issues
#142: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc.h:73:
+#define intel_uc_ops_function(_NAME, _OPS, _TYPE, _RET) \
+static inline _TYPE intel_uc_##_NAME(struct intel_uc *uc) \
+{ \
+	if (uc->ops->_OPS) \
+		return uc->ops->_OPS(uc); \
+	return _RET; \
+}

-:149: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#149: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc.h:80:
+}
+intel_uc_ops_function(init_hw, init_hw, int, 0);

-:150: ERROR:SPACING: space prohibited before that close parenthesis ')'
#150: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc.h:81:
+intel_uc_ops_function(fini_hw, fini_hw, void, );

total: 2 errors, 0 warnings, 3 checks, 117 lines checked
def878150533 drm/i915/uc: Add init_fw/fini_fw to to intel_uc_ops
-:87: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#87: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc.h:80:
 }
+intel_uc_ops_function(fetch_firmwares, init_fw, void, );

-:87: ERROR:SPACING: space prohibited before that close parenthesis ')'
#87: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc.h:80:
+intel_uc_ops_function(fetch_firmwares, init_fw, void, );

-:88: ERROR:SPACING: space prohibited before that close parenthesis ')'
#88: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc.h:81:
+intel_uc_ops_function(cleanup_firmwares, fini_fw, void, );

total: 2 errors, 0 warnings, 1 checks, 63 lines checked
71565950a620 drm/i915/uc: Add init/fini to to intel_uc_ops
-:89: ERROR:SPACING: space prohibited before that close parenthesis ')'
#89: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc.h:82:
+intel_uc_ops_function(init, init, void, );

-:90: ERROR:SPACING: space prohibited before that close parenthesis ')'
#90: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc.h:83:
+intel_uc_ops_function(fini, fini, void, );

total: 2 errors, 0 warnings, 0 checks, 65 lines checked
0960b20d5549 drm/i915/uc: Add sanitize to to intel_uc_ops
-:74: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#74: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc.h:80:
 }
+intel_uc_ops_function(sanitize, sanitize, int, 0);

total: 0 errors, 0 warnings, 1 checks, 49 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for Add ops to intel_uc (rev3)

[Patchwork]

== Series Details ==

Series: Add ops to intel_uc (rev3)
URL   : https://patchwork.freedesktop.org/series/70716/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7721 -> Patchwork_16059
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16059/index.html

Known issues
------------

  Here are the changes found in Patchwork_16059 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-cfl-guc:         [PASS][1] -> [DMESG-WARN][2] ([i915#889])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7721/fi-cfl-guc/igt@i915_module_load@reload-with-fault-injection.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16059/fi-cfl-guc/igt@i915_module_load@reload-with-fault-injection.html

  * igt@i915_selftest@live_execlists:
    - fi-kbl-soraka:      [PASS][3] -> [DMESG-FAIL][4] ([i915#656])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7721/fi-kbl-soraka/igt@i915_selftest@live_execlists.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16059/fi-kbl-soraka/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_gt_engines:
    - fi-cfl-8700k:       [PASS][5] -> [DMESG-FAIL][6] ([i915#889]) +7 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7721/fi-cfl-8700k/igt@i915_selftest@live_gt_engines.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16059/fi-cfl-8700k/igt@i915_selftest@live_gt_engines.html

  * igt@i915_selftest@live_gt_pm:
    - fi-cfl-8700k:       [PASS][7] -> [DMESG-WARN][8] ([i915#889]) +23 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7721/fi-cfl-8700k/igt@i915_selftest@live_gt_pm.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16059/fi-cfl-8700k/igt@i915_selftest@live_gt_pm.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [PASS][9] -> [DMESG-WARN][10] ([i915#44])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7721/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16059/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@gem_exec_gttfill@basic:
    - {fi-ehl-1}:         [INCOMPLETE][11] ([i915#937]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7721/fi-ehl-1/igt@gem_exec_gttfill@basic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16059/fi-ehl-1/igt@gem_exec_gttfill@basic.html

  * igt@gem_render_linear_blits@basic:
    - fi-icl-dsi:         [DMESG-WARN][13] ([i915#109]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7721/fi-icl-dsi/igt@gem_render_linear_blits@basic.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16059/fi-icl-dsi/igt@gem_render_linear_blits@basic.html

  * igt@gem_sync@basic-each:
    - fi-tgl-y:           [INCOMPLETE][15] ([i915#472] / [i915#707]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7721/fi-tgl-y/igt@gem_sync@basic-each.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16059/fi-tgl-y/igt@gem_sync@basic-each.html

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770:        [DMESG-FAIL][17] ([i915#553] / [i915#725]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7721/fi-hsw-4770/igt@i915_selftest@live_blt.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16059/fi-hsw-4770/igt@i915_selftest@live_blt.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#109]: https://gitlab.freedesktop.org/drm/intel/issues/109
  [i915#44]: https://gitlab.freedesktop.org/drm/intel/issues/44
  [i915#472]: https://gitlab.freedesktop.org/drm/intel/issues/472
  [i915#553]: https://gitlab.freedesktop.org/drm/intel/issues/553
  [i915#656]: https://gitlab.freedesktop.org/drm/intel/issues/656
  [i915#707]: https://gitlab.freedesktop.org/drm/intel/issues/707
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
  [i915#889]: https://gitlab.freedesktop.org/drm/intel/issues/889
  [i915#937]: https://gitlab.freedesktop.org/drm/intel/issues/937


Participating hosts (47 -> 40)
------------------------------

  Additional (4): fi-hsw-4770r fi-ilk-650 fi-byt-n2820 fi-snb-2520m 
  Missing    (11): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bwr-2160 fi-kbl-7500u fi-ctg-p8600 fi-gdg-551 fi-ivb-3770 fi-byt-clapper fi-skl-6600u fi-snb-2600 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7721 -> Patchwork_16059

  CI-20190529: 20190529
  CI_DRM_7721: 3a2436c56fcf2d133d701a112eb1e0dfce0b846d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5364: b7cb6ffdb65cbd233f5ddee2f2dabf97b34fa640 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16059: 0960b20d554902ea4e686f61eb736c733cfefb41 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0960b20d5549 drm/i915/uc: Add sanitize to to intel_uc_ops
71565950a620 drm/i915/uc: Add init/fini to to intel_uc_ops
def878150533 drm/i915/uc: Add init_fw/fini_fw to to intel_uc_ops
bb5e4aea2877 drm/i915/uc: Add ops to intel_uc

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16059/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx