diff mbox series

[8/8] drm/i915/guc: Update GT/GuC messages in intel_uc.c

Message ID 20230120164050.1765-9-michal.wajdeczko@intel.com (mailing list archive)
State New, archived
Headers show
Series GuC oriented print macros | expand

Commit Message

Michal Wajdeczko Jan. 20, 2023, 4:40 p.m. UTC 
Use new macros to have common prefix that also include GT#.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 74 +++++++++++++--------------
 1 file changed, 36 insertions(+), 38 deletions(-)

Comments

John Harrison Jan. 23, 2023, 11:19 p.m. UTC | #1 
On 1/20/2023 08:40, Michal Wajdeczko wrote:
> Use new macros to have common prefix that also include GT#.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> ---
>   drivers/gpu/drm/i915/gt/uc/intel_uc.c | 74 +++++++++++++--------------
>   1 file changed, 36 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 9a8a1abf71d7..e94f0d7119c4 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -6,11 +6,13 @@
>   #include <linux/string_helpers.h>
>   
>   #include "gt/intel_gt.h"
> +#include "gt/intel_gt_print.h"
>   #include "gt/intel_reset.h"
>   #include "intel_gsc_fw.h"
>   #include "intel_gsc_uc.h"
>   #include "intel_guc.h"
>   #include "intel_guc_ads.h"
> +#include "intel_guc_print.h"
>   #include "intel_guc_submission.h"
>   #include "gt/intel_rps.h"
>   #include "intel_uc.h"
> @@ -67,14 +69,14 @@ static int __intel_uc_reset_hw(struct intel_uc *uc)
>   
>   	ret = intel_reset_guc(gt);
>   	if (ret) {
> -		DRM_ERROR("Failed to reset GuC, ret = %d\n", ret);
> +		gt_err(gt, "Failed to reset GuC, ret = %d\n", ret);
>   		return ret;
>   	}
>   
>   	guc_status = intel_uncore_read(gt->uncore, GUC_STATUS);
> -	WARN(!(guc_status & GS_MIA_IN_RESET),
> -	     "GuC status: 0x%x, MIA core expected to be in reset\n",
> -	     guc_status);
> +	gt_WARN(gt, !(guc_status & GS_MIA_IN_RESET),
> +		"GuC status: 0x%x, MIA core expected to be in reset\n",
> +		guc_status);
>   
>   	return ret;
>   }
> @@ -252,15 +254,13 @@ static int guc_enable_communication(struct intel_guc *guc)
>   	intel_guc_ct_event_handler(&guc->ct);
>   	spin_unlock_irq(gt->irq_lock);
>   
> -	drm_dbg(&i915->drm, "GuC communication enabled\n");
> +	guc_dbg(guc, "communication enabled\n");
>   
>   	return 0;
>   }
>   
>   static void guc_disable_communication(struct intel_guc *guc)
>   {
> -	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
> -
>   	/*
>   	 * Events generated during or after CT disable are logged by guc in
>   	 * via mmio. Make sure the register is clear before disabling CT since
> @@ -280,11 +280,12 @@ static void guc_disable_communication(struct intel_guc *guc)
>   	 */
>   	guc_get_mmio_msg(guc);
>   
> -	drm_dbg(&i915->drm, "GuC communication disabled\n");
> +	guc_dbg(guc, "communication disabled\n");
>   }
>   
>   static void __uc_fetch_firmwares(struct intel_uc *uc)
>   {
> +	struct intel_gt *gt = uc_to_gt(uc);
>   	int err;
>   
>   	GEM_BUG_ON(!intel_uc_wants_guc(uc));
> @@ -293,15 +294,13 @@ static void __uc_fetch_firmwares(struct intel_uc *uc)
>   	if (err) {
>   		/* Make sure we transition out of transient "SELECTED" state */
>   		if (intel_uc_wants_huc(uc)) {
> -			drm_dbg(&uc_to_gt(uc)->i915->drm,
> -				"Failed to fetch GuC: %d disabling HuC\n", err);
> +			gt_dbg(gt, "Failed to fetch GuC fw (%pe) disabling HuC\n", ERR_PTR(err));
>   			intel_uc_fw_change_status(&uc->huc.fw,
>   						  INTEL_UC_FIRMWARE_ERROR);
>   		}
>   
>   		if (intel_uc_wants_gsc_uc(uc)) {
> -			drm_dbg(&uc_to_gt(uc)->i915->drm,
> -				"Failed to fetch GuC: %d disabling GSC\n", err);
> +			gt_dbg(gt, "Failed to fetch GuC fw (%pe) disabling GSC\n", ERR_PTR(err));
>   			intel_uc_fw_change_status(&uc->gsc.fw,
>   						  INTEL_UC_FIRMWARE_ERROR);
>   		}
> @@ -382,7 +381,7 @@ static int uc_init_wopcm(struct intel_uc *uc)
>   	int err;
>   
>   	if (unlikely(!base || !size)) {
> -		i915_probe_error(gt->i915, "Unsuccessful WOPCM partitioning\n");
> +		gt_probe_error(gt, "Unsuccessful WOPCM partitioning\n");
>   		return -E2BIG;
>   	}
>   
> @@ -413,13 +412,13 @@ static int uc_init_wopcm(struct intel_uc *uc)
>   	return 0;
>   
>   err_out:
> -	i915_probe_error(gt->i915, "Failed to init uC WOPCM registers!\n");
> -	i915_probe_error(gt->i915, "%s(%#x)=%#x\n", "DMA_GUC_WOPCM_OFFSET",
> -			 i915_mmio_reg_offset(DMA_GUC_WOPCM_OFFSET),
> -			 intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET));
> -	i915_probe_error(gt->i915, "%s(%#x)=%#x\n", "GUC_WOPCM_SIZE",
> -			 i915_mmio_reg_offset(GUC_WOPCM_SIZE),
> -			 intel_uncore_read(uncore, GUC_WOPCM_SIZE));
> +	gt_probe_error(gt, "Failed to init uC WOPCM registers!\n");
> +	gt_probe_error(gt, "%s(%#x)=%#x\n", "DMA_GUC_WOPCM_OFFSET",
> +		       i915_mmio_reg_offset(DMA_GUC_WOPCM_OFFSET),
> +		       intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET));
> +	gt_probe_error(gt, "%s(%#x)=%#x\n", "GUC_WOPCM_SIZE",
> +		       i915_mmio_reg_offset(GUC_WOPCM_SIZE),
> +		       intel_uncore_read(uncore, GUC_WOPCM_SIZE));
>   
>   	return err;
>   }
> @@ -451,18 +450,17 @@ static int __uc_check_hw(struct intel_uc *uc)
>   
>   static void print_fw_ver(struct intel_uc *uc, struct intel_uc_fw *fw)
>   {
> -	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
> -
> -	drm_info(&i915->drm, "%s firmware %s version %u.%u.%u\n",
> -		 intel_uc_fw_type_repr(fw->type), fw->file_selected.path,
> -		 fw->file_selected.ver.major,
> -		 fw->file_selected.ver.minor,
> -		 fw->file_selected.ver.patch);
> +	gt_info(uc_to_gt(uc), "%s firmware %s version %u.%u.%u\n",
Given that this function does not use 'uc' except to convert to 'gt' and 
the caller already has a local 'gt', could just pass the gt in directly 
and avoid the conversion.

> +		intel_uc_fw_type_repr(fw->type), fw->file_selected.path,
> +		fw->file_selected.ver.major,
> +		fw->file_selected.ver.minor,
> +		fw->file_selected.ver.patch);
>   }
>   
>   static int __uc_init_hw(struct intel_uc *uc)
>   {
> -	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
> +	struct intel_gt *gt = uc_to_gt(uc);
> +	struct drm_i915_private *i915 = gt->i915;
>   	struct intel_guc *guc = &uc->guc;
>   	struct intel_huc *huc = &uc->huc;
>   	int ret, attempts;
> @@ -514,8 +512,8 @@ static int __uc_init_hw(struct intel_uc *uc)
>   		if (ret == 0)
>   			break;
>   
> -		DRM_DEBUG_DRIVER("GuC fw load failed: %d; will reset and "
> -				 "retry %d more time(s)\n", ret, attempts);
> +		gt_dbg(gt, "GuC fw load failed (%pe) will reset and retry %d more time(s)\n",
> +		       ERR_PTR(ret), attempts);
>   	}
>   
>   	/* Did we succeded or run out of retries? */
> @@ -551,10 +549,10 @@ static int __uc_init_hw(struct intel_uc *uc)
>   
>   	intel_gsc_uc_load_start(&uc->gsc);
>   
> -	drm_info(&i915->drm, "GuC submission %s\n",
> -		 str_enabled_disabled(intel_uc_uses_guc_submission(uc)));
> -	drm_info(&i915->drm, "GuC SLPC %s\n",
> -		 str_enabled_disabled(intel_uc_uses_guc_slpc(uc)));
> +	gt_info(gt, "GuC submission %s\n",
> +		str_enabled_disabled(intel_uc_uses_guc_submission(uc)));
> +	gt_info(gt, "GuC SLPC %s\n",
> +		str_enabled_disabled(intel_uc_uses_guc_slpc(uc)));
>   
>   	return 0;
>   
> @@ -572,12 +570,12 @@ static int __uc_init_hw(struct intel_uc *uc)
>   	__uc_sanitize(uc);
>   
>   	if (!ret) {
> -		drm_notice(&i915->drm, "GuC is uninitialized\n");
> +		gt_notice(gt, "GuC is uninitialized\n");
>   		/* We want to run without GuC submission */
>   		return 0;
>   	}
>   
> -	i915_probe_error(i915, "GuC initialization failed %d\n", ret);
> +	gt_probe_error(gt, "GuC initialization failed %pe\n", ERR_PTR(ret));
>   
>   	/* We want to keep KMS alive */
>   	return -EIO;
> @@ -690,7 +688,7 @@ void intel_uc_suspend(struct intel_uc *uc)
>   	with_intel_runtime_pm(&uc_to_gt(uc)->i915->runtime_pm, wakeref) {
>   		err = intel_guc_suspend(guc);
>   		if (err)
> -			DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
> +			gt_dbg(uc_to_gt(uc), "Failed to suspend GuC, %pe", ERR_PTR(err));
May be worth adding a local gt given that the above conversion is now 
being repeated?

>   	}
>   }
>   
> @@ -718,7 +716,7 @@ static int __uc_resume(struct intel_uc *uc, bool enable_communication)
>   
>   	err = intel_guc_resume(guc);
>   	if (err) {
> -		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);
> +		gt_dbg(gt, "Failed to resume GuC, %pe", ERR_PTR(err));
This could be a guc_dbg("Failed to resume: %pe") given that there is 
evidently a 'guc' locally available from the function call just above.

John.

>   		return err;
>   	}
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 9a8a1abf71d7..e94f0d7119c4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -6,11 +6,13 @@ 
 #include <linux/string_helpers.h>
 
 #include "gt/intel_gt.h"
+#include "gt/intel_gt_print.h"
 #include "gt/intel_reset.h"
 #include "intel_gsc_fw.h"
 #include "intel_gsc_uc.h"
 #include "intel_guc.h"
 #include "intel_guc_ads.h"
+#include "intel_guc_print.h"
 #include "intel_guc_submission.h"
 #include "gt/intel_rps.h"
 #include "intel_uc.h"
@@ -67,14 +69,14 @@  static int __intel_uc_reset_hw(struct intel_uc *uc)
 
 	ret = intel_reset_guc(gt);
 	if (ret) {
-		DRM_ERROR("Failed to reset GuC, ret = %d\n", ret);
+		gt_err(gt, "Failed to reset GuC, ret = %d\n", ret);
 		return ret;
 	}
 
 	guc_status = intel_uncore_read(gt->uncore, GUC_STATUS);
-	WARN(!(guc_status & GS_MIA_IN_RESET),
-	     "GuC status: 0x%x, MIA core expected to be in reset\n",
-	     guc_status);
+	gt_WARN(gt, !(guc_status & GS_MIA_IN_RESET),
+		"GuC status: 0x%x, MIA core expected to be in reset\n",
+		guc_status);
 
 	return ret;
 }
@@ -252,15 +254,13 @@  static int guc_enable_communication(struct intel_guc *guc)
 	intel_guc_ct_event_handler(&guc->ct);
 	spin_unlock_irq(gt->irq_lock);
 
-	drm_dbg(&i915->drm, "GuC communication enabled\n");
+	guc_dbg(guc, "communication enabled\n");
 
 	return 0;
 }
 
 static void guc_disable_communication(struct intel_guc *guc)
 {
-	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
-
 	/*
 	 * Events generated during or after CT disable are logged by guc in
 	 * via mmio. Make sure the register is clear before disabling CT since
@@ -280,11 +280,12 @@  static void guc_disable_communication(struct intel_guc *guc)
 	 */
 	guc_get_mmio_msg(guc);
 
-	drm_dbg(&i915->drm, "GuC communication disabled\n");
+	guc_dbg(guc, "communication disabled\n");
 }
 
 static void __uc_fetch_firmwares(struct intel_uc *uc)
 {
+	struct intel_gt *gt = uc_to_gt(uc);
 	int err;
 
 	GEM_BUG_ON(!intel_uc_wants_guc(uc));
@@ -293,15 +294,13 @@  static void __uc_fetch_firmwares(struct intel_uc *uc)
 	if (err) {
 		/* Make sure we transition out of transient "SELECTED" state */
 		if (intel_uc_wants_huc(uc)) {
-			drm_dbg(&uc_to_gt(uc)->i915->drm,
-				"Failed to fetch GuC: %d disabling HuC\n", err);
+			gt_dbg(gt, "Failed to fetch GuC fw (%pe) disabling HuC\n", ERR_PTR(err));
 			intel_uc_fw_change_status(&uc->huc.fw,
 						  INTEL_UC_FIRMWARE_ERROR);
 		}
 
 		if (intel_uc_wants_gsc_uc(uc)) {
-			drm_dbg(&uc_to_gt(uc)->i915->drm,
-				"Failed to fetch GuC: %d disabling GSC\n", err);
+			gt_dbg(gt, "Failed to fetch GuC fw (%pe) disabling GSC\n", ERR_PTR(err));
 			intel_uc_fw_change_status(&uc->gsc.fw,
 						  INTEL_UC_FIRMWARE_ERROR);
 		}
@@ -382,7 +381,7 @@  static int uc_init_wopcm(struct intel_uc *uc)
 	int err;
 
 	if (unlikely(!base || !size)) {
-		i915_probe_error(gt->i915, "Unsuccessful WOPCM partitioning\n");
+		gt_probe_error(gt, "Unsuccessful WOPCM partitioning\n");
 		return -E2BIG;
 	}
 
@@ -413,13 +412,13 @@  static int uc_init_wopcm(struct intel_uc *uc)
 	return 0;
 
 err_out:
-	i915_probe_error(gt->i915, "Failed to init uC WOPCM registers!\n");
-	i915_probe_error(gt->i915, "%s(%#x)=%#x\n", "DMA_GUC_WOPCM_OFFSET",
-			 i915_mmio_reg_offset(DMA_GUC_WOPCM_OFFSET),
-			 intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET));
-	i915_probe_error(gt->i915, "%s(%#x)=%#x\n", "GUC_WOPCM_SIZE",
-			 i915_mmio_reg_offset(GUC_WOPCM_SIZE),
-			 intel_uncore_read(uncore, GUC_WOPCM_SIZE));
+	gt_probe_error(gt, "Failed to init uC WOPCM registers!\n");
+	gt_probe_error(gt, "%s(%#x)=%#x\n", "DMA_GUC_WOPCM_OFFSET",
+		       i915_mmio_reg_offset(DMA_GUC_WOPCM_OFFSET),
+		       intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET));
+	gt_probe_error(gt, "%s(%#x)=%#x\n", "GUC_WOPCM_SIZE",
+		       i915_mmio_reg_offset(GUC_WOPCM_SIZE),
+		       intel_uncore_read(uncore, GUC_WOPCM_SIZE));
 
 	return err;
 }
@@ -451,18 +450,17 @@  static int __uc_check_hw(struct intel_uc *uc)
 
 static void print_fw_ver(struct intel_uc *uc, struct intel_uc_fw *fw)
 {
-	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
-
-	drm_info(&i915->drm, "%s firmware %s version %u.%u.%u\n",
-		 intel_uc_fw_type_repr(fw->type), fw->file_selected.path,
-		 fw->file_selected.ver.major,
-		 fw->file_selected.ver.minor,
-		 fw->file_selected.ver.patch);
+	gt_info(uc_to_gt(uc), "%s firmware %s version %u.%u.%u\n",
+		intel_uc_fw_type_repr(fw->type), fw->file_selected.path,
+		fw->file_selected.ver.major,
+		fw->file_selected.ver.minor,
+		fw->file_selected.ver.patch);
 }
 
 static int __uc_init_hw(struct intel_uc *uc)
 {
-	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
+	struct intel_gt *gt = uc_to_gt(uc);
+	struct drm_i915_private *i915 = gt->i915;
 	struct intel_guc *guc = &uc->guc;
 	struct intel_huc *huc = &uc->huc;
 	int ret, attempts;
@@ -514,8 +512,8 @@  static int __uc_init_hw(struct intel_uc *uc)
 		if (ret == 0)
 			break;
 
-		DRM_DEBUG_DRIVER("GuC fw load failed: %d; will reset and "
-				 "retry %d more time(s)\n", ret, attempts);
+		gt_dbg(gt, "GuC fw load failed (%pe) will reset and retry %d more time(s)\n",
+		       ERR_PTR(ret), attempts);
 	}
 
 	/* Did we succeded or run out of retries? */
@@ -551,10 +549,10 @@  static int __uc_init_hw(struct intel_uc *uc)
 
 	intel_gsc_uc_load_start(&uc->gsc);
 
-	drm_info(&i915->drm, "GuC submission %s\n",
-		 str_enabled_disabled(intel_uc_uses_guc_submission(uc)));
-	drm_info(&i915->drm, "GuC SLPC %s\n",
-		 str_enabled_disabled(intel_uc_uses_guc_slpc(uc)));
+	gt_info(gt, "GuC submission %s\n",
+		str_enabled_disabled(intel_uc_uses_guc_submission(uc)));
+	gt_info(gt, "GuC SLPC %s\n",
+		str_enabled_disabled(intel_uc_uses_guc_slpc(uc)));
 
 	return 0;
 
@@ -572,12 +570,12 @@  static int __uc_init_hw(struct intel_uc *uc)
 	__uc_sanitize(uc);
 
 	if (!ret) {
-		drm_notice(&i915->drm, "GuC is uninitialized\n");
+		gt_notice(gt, "GuC is uninitialized\n");
 		/* We want to run without GuC submission */
 		return 0;
 	}
 
-	i915_probe_error(i915, "GuC initialization failed %d\n", ret);
+	gt_probe_error(gt, "GuC initialization failed %pe\n", ERR_PTR(ret));
 
 	/* We want to keep KMS alive */
 	return -EIO;
@@ -690,7 +688,7 @@  void intel_uc_suspend(struct intel_uc *uc)
 	with_intel_runtime_pm(&uc_to_gt(uc)->i915->runtime_pm, wakeref) {
 		err = intel_guc_suspend(guc);
 		if (err)
-			DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
+			gt_dbg(uc_to_gt(uc), "Failed to suspend GuC, %pe", ERR_PTR(err));
 	}
 }
 
@@ -718,7 +716,7 @@  static int __uc_resume(struct intel_uc *uc, bool enable_communication)
 
 	err = intel_guc_resume(guc);
 	if (err) {
-		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);
+		gt_dbg(gt, "Failed to resume GuC, %pe", ERR_PTR(err));
 		return err;
 	}