diff mbox series

[v2,4/6] drm/i915/debugfs: move uC printers and update debugfs file names

Message ID 20200312011631.15262-5-daniele.ceraolospurio@intel.com (mailing list archive)
State New, archived
Headers show
Series Re-org uC debugfs files and move them under GT | expand

Commit Message

Daniele Ceraolo Spurio March 12, 2020, 1:16 a.m. UTC
Move the printers to the respective files for clarity. The
guc_load_status debugfs has been squashed in the guc_info one, has
having separate ones wasn't very useful. The HuC debugfs has been
renamed huc_info to match.

v2: keep printing HUC_STATUS2 (Tony), avoid const->non-const
    container_of (Jani)

Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Tony Ye <tony.ye@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc.c     |  44 +++++++
 drivers/gpu/drm/i915/gt/uc/intel_guc.h     |   2 +
 drivers/gpu/drm/i915/gt/uc/intel_guc_log.c |  92 +++++++++++++++
 drivers/gpu/drm/i915/gt/uc/intel_guc_log.h |   4 +
 drivers/gpu/drm/i915/gt/uc/intel_huc.c     |  29 +++++
 drivers/gpu/drm/i915/gt/uc/intel_huc.h     |   2 +
 drivers/gpu/drm/i915/i915_debugfs.c        | 131 +++------------------
 7 files changed, 189 insertions(+), 115 deletions(-)

Comments

John Harrison March 25, 2020, 5:05 p.m. UTC | #1
On 3/11/2020 18:16, Daniele Ceraolo Spurio wrote:
> Move the printers to the respective files for clarity. The
> guc_load_status debugfs has been squashed in the guc_info one, has
> having separate ones wasn't very useful. The HuC debugfs has been
> renamed huc_info to match.
>
> v2: keep printing HUC_STATUS2 (Tony), avoid const->non-const
>      container_of (Jani)
>
> Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Tony Ye <tony.ye@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/gt/uc/intel_guc.c     |  44 +++++++
>   drivers/gpu/drm/i915/gt/uc/intel_guc.h     |   2 +
>   drivers/gpu/drm/i915/gt/uc/intel_guc_log.c |  92 +++++++++++++++
>   drivers/gpu/drm/i915/gt/uc/intel_guc_log.h |   4 +
>   drivers/gpu/drm/i915/gt/uc/intel_huc.c     |  29 +++++
>   drivers/gpu/drm/i915/gt/uc/intel_huc.h     |   2 +
>   drivers/gpu/drm/i915/i915_debugfs.c        | 131 +++------------------
>   7 files changed, 189 insertions(+), 115 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 827d75073879..861657897c0f 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -723,3 +723,47 @@ int intel_guc_allocate_and_map_vma(struct intel_guc *guc, u32 size,
>   
>   	return 0;
>   }
> +
> +/**
> + * intel_guc_load_status - dump information about GuC load status
> + * @guc: the GuC
> + * @p: the &drm_printer
> + *
> + * Pretty printer for GuC load status.
> + */
> +void intel_guc_load_status(struct intel_guc *guc, struct drm_printer *p)
> +{
> +	struct intel_gt *gt = guc_to_gt(guc);
> +	struct intel_uncore *uncore = gt->uncore;
> +	intel_wakeref_t wakeref;
> +
> +	if (!intel_guc_is_supported(guc)) {
> +		drm_printf(p, "GuC not supported\n");
> +		return;
> +	}
> +
> +	if (!intel_guc_is_wanted(guc)) {
> +		drm_printf(p, "GuC disabled\n");
> +		return;
> +	}
> +
> +	intel_uc_fw_dump(&guc->fw, p);
> +
> +	with_intel_runtime_pm(uncore->rpm, wakeref) {
> +		u32 status = intel_uncore_read(uncore, GUC_STATUS);
> +		u32 i;
> +
> +		drm_printf(p, "\nGuC status 0x%08x:\n", status);
> +		drm_printf(p, "\tBootrom status = 0x%x\n",
> +			   (status & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT);
> +		drm_printf(p, "\tuKernel status = 0x%x\n",
> +			   (status & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT);
> +		drm_printf(p, "\tMIA Core status = 0x%x\n",
> +			   (status & GS_MIA_MASK) >> GS_MIA_SHIFT);
> +		drm_puts(p, "\nScratch registers:\n");
> +		for (i = 0; i < 16; i++) {
> +			drm_printf(p, "\t%2d: \t0x%x\n",
> +				   i, intel_uncore_read(uncore, SOFT_SCRATCH(i)));
> +		}
> +	}
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index 4594ccbeaa34..a5d7a86be4cf 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -190,4 +190,6 @@ static inline void intel_guc_disable_msg(struct intel_guc *guc, u32 mask)
>   int intel_guc_reset_engine(struct intel_guc *guc,
>   			   struct intel_engine_cs *engine);
>   
> +void intel_guc_load_status(struct intel_guc *guc, struct drm_printer *p);
> +
>   #endif
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> index caed0d57e704..8cdd6dc3df58 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> @@ -672,3 +672,95 @@ void intel_guc_log_handle_flush_event(struct intel_guc_log *log)
>   {
>   	queue_work(system_highpri_wq, &log->relay.flush_work);
>   }
> +
> +static const char *
> +stringify_guc_log_type(enum guc_log_buffer_type type)
> +{
> +	switch (type) {
> +	case GUC_ISR_LOG_BUFFER:
> +		return "ISR";
> +	case GUC_DPC_LOG_BUFFER:
> +		return "DPC";
> +	case GUC_CRASH_DUMP_LOG_BUFFER:
> +		return "CRASH";
> +	default:
> +		MISSING_CASE(type);
> +	}
> +
> +	return "";
> +}
> +
> +/**
> + * intel_guc_log_info - dump information about GuC log relay
> + * @guc: the GuC
> + * @p: the &drm_printer
> + *
> + * Pretty printer for GuC log info
> + */
> +void intel_guc_log_info(struct intel_guc_log *log, struct drm_printer *p)
> +{
> +	enum guc_log_buffer_type type;
> +
> +	if (!intel_guc_log_relay_created(log)) {
> +		drm_puts(p, "GuC log relay not created\n");
> +		return;
> +	}
> +
> +	drm_puts(p, "GuC logging stats:\n");
> +
> +	drm_printf(p, "\tRelay full count: %u\n", log->relay.full_count);
> +
> +	for (type = GUC_ISR_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER; type++) {
> +		drm_printf(p, "\t%s:\tflush count %10u, overflow count %10u\n",
> +			   stringify_guc_log_type(type),
> +			   log->stats[type].flush,
> +			   log->stats[type].sampled_overflow);
> +	}
> +}
> +
> +/**
> + * intel_guc_log_dump - dump the contents of the GuC log
> + * @log: the GuC log
> + * @p: the &drm_printer
> + * @dump_load_err: dump the log saved on GuC load error
> + *
> + * Pretty printer for the GuC log
> + */
> +int intel_guc_log_dump(struct intel_guc_log *log, struct drm_printer *p,
> +		       bool dump_load_err)
> +{
> +	struct intel_guc *guc = log_to_guc(log);
> +	struct intel_uc *uc = container_of(guc, struct intel_uc, guc);
> +	struct drm_i915_gem_object *obj = NULL;
> +	u32 *map;
> +	int i = 0;
> +
> +	if (!intel_guc_is_supported(guc))
> +		return -ENODEV;
> +
> +	if (dump_load_err)
> +		obj = uc->load_err_log;
> +	else if (guc->log.vma)
> +		obj = guc->log.vma->obj;
> +
> +	if (!obj)
> +		return 0;
> +
> +	map = i915_gem_object_pin_map(obj, I915_MAP_WC);
> +	if (IS_ERR(map)) {
> +		DRM_DEBUG("Failed to pin object\n");
> +		drm_puts(p, "(log data unaccessible)\n");
> +		return PTR_ERR(map);
> +	}
> +
> +	for (i = 0; i < obj->base.size / sizeof(u32); i += 4)
> +		drm_printf(p, "0x%08x 0x%08x 0x%08x 0x%08x\n",
> +			   *(map + i), *(map + i + 1),
> +			   *(map + i + 2), *(map + i + 3));
> +
> +	drm_puts(p, "\n");
> +
> +	i915_gem_object_unpin_map(obj);
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
> index c252c022c5fc..11fccd0b2294 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
> @@ -79,4 +79,8 @@ static inline u32 intel_guc_log_get_level(struct intel_guc_log *log)
>   	return log->level;
>   }
>   
> +void intel_guc_log_info(struct intel_guc_log *log, struct drm_printer *p);
> +int intel_guc_log_dump(struct intel_guc_log *log, struct drm_printer *p,
> +		       bool dump_load_err);
> +
>   #endif
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index d73dc21686e7..d6097b46600c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -218,3 +218,32 @@ int intel_huc_check_status(struct intel_huc *huc)
>   
>   	return (status & huc->status.mask) == huc->status.value;
>   }
> +
> +/**
> + * intel_huc_load_status - dump information about HuC load status
> + * @huc: the HuC
> + * @p: the &drm_printer
> + *
> + * Pretty printer for HuC load status.
> + */
> +void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p)
> +{
> +	struct intel_gt *gt = huc_to_gt(huc);
> +	intel_wakeref_t wakeref;
> +
> +	if (!intel_huc_is_supported(huc)) {
> +		drm_printf(p, "HuC not supported\n");
> +		return;
> +	}
> +
> +	if (!intel_huc_is_wanted(huc)) {
> +		drm_printf(p, "HuC disabled\n");
> +		return;
> +	}
> +
> +	intel_uc_fw_dump(&huc->fw, p);
> +
> +	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> +		drm_printf(p, "\nHuC status 0x%08x:\n",
> +			   intel_uncore_read(gt->uncore, HUC_STATUS2));
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> index a40b9cfc6c22..daee43b661d4 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> @@ -57,4 +57,6 @@ static inline bool intel_huc_is_authenticated(struct intel_huc *huc)
>   	return intel_uc_fw_is_running(&huc->fw);
>   }
>   
> +void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
> +
>   #endif
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index de94fcd2032b..56504be2a6ec 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1251,105 +1251,32 @@ static int i915_llc(struct seq_file *m, void *data)
>   	return 0;
>   }
>   
> -static int i915_huc_load_status_info(struct seq_file *m, void *data)
> +static int i915_huc_info(struct seq_file *m, void *data)
>   {
>   	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> -	intel_wakeref_t wakeref;
> -	struct drm_printer p;
> -
> -	if (!HAS_GT_UC(dev_priv))
> -		return -ENODEV;
> -
> -	p = drm_seq_file_printer(m);
> -	intel_uc_fw_dump(&dev_priv->gt.uc.huc.fw, &p);
> -
> -	with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref)
> -		seq_printf(m, "\nHuC status 0x%08x:\n", I915_READ(HUC_STATUS2));
> -
> -	return 0;
> -}
> -
> -static int i915_guc_load_status_info(struct seq_file *m, void *data)
> -{
> -	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> -	intel_wakeref_t wakeref;
> -	struct drm_printer p;
> +	struct intel_huc *huc = &dev_priv->gt.uc.huc;
> +	struct drm_printer p = drm_seq_file_printer(m);
>   
> -	if (!HAS_GT_UC(dev_priv))
> +	if (!intel_huc_is_supported(huc))
>   		return -ENODEV;
Isn't this test duplicated inside intel_huc_load_status() with a print 
of 'HuC not supported'? So no need to fail the call here?

>   
> -	p = drm_seq_file_printer(m);
> -	intel_uc_fw_dump(&dev_priv->gt.uc.guc.fw, &p);
> -
> -	with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref) {
> -		u32 tmp = I915_READ(GUC_STATUS);
> -		u32 i;
> -
> -		seq_printf(m, "\nGuC status 0x%08x:\n", tmp);
> -		seq_printf(m, "\tBootrom status = 0x%x\n",
> -			   (tmp & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT);
> -		seq_printf(m, "\tuKernel status = 0x%x\n",
> -			   (tmp & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT);
> -		seq_printf(m, "\tMIA Core status = 0x%x\n",
> -			   (tmp & GS_MIA_MASK) >> GS_MIA_SHIFT);
> -		seq_puts(m, "\nScratch registers:\n");
> -		for (i = 0; i < 16; i++) {
> -			seq_printf(m, "\t%2d: \t0x%x\n",
> -				   i, I915_READ(SOFT_SCRATCH(i)));
> -		}
> -	}
> +	intel_huc_load_status(huc, &p);
>   
>   	return 0;
>   }
>   
> -static const char *
> -stringify_guc_log_type(enum guc_log_buffer_type type)
> -{
> -	switch (type) {
> -	case GUC_ISR_LOG_BUFFER:
> -		return "ISR";
> -	case GUC_DPC_LOG_BUFFER:
> -		return "DPC";
> -	case GUC_CRASH_DUMP_LOG_BUFFER:
> -		return "CRASH";
> -	default:
> -		MISSING_CASE(type);
> -	}
> -
> -	return "";
> -}
> -
> -static void i915_guc_log_info(struct seq_file *m, struct intel_guc_log *log)
> -{
> -	enum guc_log_buffer_type type;
> -
> -	if (!intel_guc_log_relay_created(log)) {
> -		seq_puts(m, "GuC log relay not created\n");
> -		return;
> -	}
> -
> -	seq_puts(m, "GuC logging stats:\n");
> -
> -	seq_printf(m, "\tRelay full count: %u\n",
> -		   log->relay.full_count);
> -
> -	for (type = GUC_ISR_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER; type++) {
> -		seq_printf(m, "\t%s:\tflush count %10u, overflow count %10u\n",
> -			   stringify_guc_log_type(type),
> -			   log->stats[type].flush,
> -			   log->stats[type].sampled_overflow);
> -	}
> -}
> -
>   static int i915_guc_info(struct seq_file *m, void *data)
>   {
>   	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> -	struct intel_uc *uc = &dev_priv->gt.uc;
> +	struct intel_guc *guc = &dev_priv->gt.uc.guc;
> +	struct drm_printer p = drm_seq_file_printer(m);
>   
> -	if (!intel_uc_uses_guc(uc))
> +	if (!intel_guc_is_supported(guc))
>   		return -ENODEV;
>   
As above. No need to bail if the dump call below is trying to return a 
useful message in the not supported case.

John.

> -	i915_guc_log_info(m, &uc->guc.log);
> +	intel_guc_load_status(guc, &p);
> +	drm_puts(&p, "\n");
> +	intel_guc_log_info(&guc->log, &p);
>   
>   	/* Add more as required ... */
>   
> @@ -1360,39 +1287,14 @@ static int i915_guc_log_dump(struct seq_file *m, void *data)
>   {
>   	struct drm_info_node *node = m->private;
>   	struct drm_i915_private *dev_priv = node_to_i915(node);
> +	struct intel_guc *guc = &dev_priv->gt.uc.guc;
>   	bool dump_load_err = !!node->info_ent->data;
> -	struct drm_i915_gem_object *obj = NULL;
> -	u32 *log;
> -	int i = 0;
> +	struct drm_printer p = drm_seq_file_printer(m);
>   
> -	if (!HAS_GT_UC(dev_priv))
> +	if (!intel_guc_is_supported(guc))
>   		return -ENODEV;
>   
> -	if (dump_load_err)
> -		obj = dev_priv->gt.uc.load_err_log;
> -	else if (dev_priv->gt.uc.guc.log.vma)
> -		obj = dev_priv->gt.uc.guc.log.vma->obj;
> -
> -	if (!obj)
> -		return 0;
> -
> -	log = i915_gem_object_pin_map(obj, I915_MAP_WC);
> -	if (IS_ERR(log)) {
> -		DRM_DEBUG("Failed to pin object\n");
> -		seq_puts(m, "(log data unaccessible)\n");
> -		return PTR_ERR(log);
> -	}
> -
> -	for (i = 0; i < obj->base.size / sizeof(u32); i += 4)
> -		seq_printf(m, "0x%08x 0x%08x 0x%08x 0x%08x\n",
> -			   *(log + i), *(log + i + 1),
> -			   *(log + i + 2), *(log + i + 3));
> -
> -	seq_putc(m, '\n');
> -
> -	i915_gem_object_unpin_map(obj);
> -
> -	return 0;
> +	return intel_guc_log_dump(&guc->log, &p, dump_load_err);
>   }
>   
>   static int i915_guc_log_level_get(void *data, u64 *val)
> @@ -2088,10 +1990,9 @@ static const struct drm_info_list i915_debugfs_list[] = {
>   	{"i915_gem_fence_regs", i915_gem_fence_regs_info, 0},
>   	{"i915_gem_interrupt", i915_interrupt_info, 0},
>   	{"i915_guc_info", i915_guc_info, 0},
> -	{"i915_guc_load_status", i915_guc_load_status_info, 0},
>   	{"i915_guc_log_dump", i915_guc_log_dump, 0},
>   	{"i915_guc_load_err_log_dump", i915_guc_log_dump, 0, (void *)1},
> -	{"i915_huc_load_status", i915_huc_load_status_info, 0},
> +	{"i915_huc_info", i915_huc_info, 0},
>   	{"i915_frequency_info", i915_frequency_info, 0},
>   	{"i915_ring_freq_table", i915_ring_freq_table, 0},
>   	{"i915_context_status", i915_context_status, 0},
Daniele Ceraolo Spurio March 25, 2020, 5:14 p.m. UTC | #2
On 3/25/20 10:05 AM, John Harrison wrote:
> On 3/11/2020 18:16, Daniele Ceraolo Spurio wrote:
>> Move the printers to the respective files for clarity. The
>> guc_load_status debugfs has been squashed in the guc_info one, has
>> having separate ones wasn't very useful. The HuC debugfs has been
>> renamed huc_info to match.
>>
>> v2: keep printing HUC_STATUS2 (Tony), avoid const->non-const
>>      container_of (Jani)
>>
>> Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: John Harrison <John.C.Harrison@Intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: Tony Ye <tony.ye@intel.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/uc/intel_guc.c     |  44 +++++++
>>   drivers/gpu/drm/i915/gt/uc/intel_guc.h     |   2 +
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_log.c |  92 +++++++++++++++
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_log.h |   4 +
>>   drivers/gpu/drm/i915/gt/uc/intel_huc.c     |  29 +++++
>>   drivers/gpu/drm/i915/gt/uc/intel_huc.h     |   2 +
>>   drivers/gpu/drm/i915/i915_debugfs.c        | 131 +++------------------
>>   7 files changed, 189 insertions(+), 115 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c 
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> index 827d75073879..861657897c0f 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> @@ -723,3 +723,47 @@ int intel_guc_allocate_and_map_vma(struct 
>> intel_guc *guc, u32 size,
>>       return 0;
>>   }
>> +
>> +/**
>> + * intel_guc_load_status - dump information about GuC load status
>> + * @guc: the GuC
>> + * @p: the &drm_printer
>> + *
>> + * Pretty printer for GuC load status.
>> + */
>> +void intel_guc_load_status(struct intel_guc *guc, struct drm_printer *p)
>> +{
>> +    struct intel_gt *gt = guc_to_gt(guc);
>> +    struct intel_uncore *uncore = gt->uncore;
>> +    intel_wakeref_t wakeref;
>> +
>> +    if (!intel_guc_is_supported(guc)) {
>> +        drm_printf(p, "GuC not supported\n");
>> +        return;
>> +    }
>> +
>> +    if (!intel_guc_is_wanted(guc)) {
>> +        drm_printf(p, "GuC disabled\n");
>> +        return;
>> +    }
>> +
>> +    intel_uc_fw_dump(&guc->fw, p);
>> +
>> +    with_intel_runtime_pm(uncore->rpm, wakeref) {
>> +        u32 status = intel_uncore_read(uncore, GUC_STATUS);
>> +        u32 i;
>> +
>> +        drm_printf(p, "\nGuC status 0x%08x:\n", status);
>> +        drm_printf(p, "\tBootrom status = 0x%x\n",
>> +               (status & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT);
>> +        drm_printf(p, "\tuKernel status = 0x%x\n",
>> +               (status & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT);
>> +        drm_printf(p, "\tMIA Core status = 0x%x\n",
>> +               (status & GS_MIA_MASK) >> GS_MIA_SHIFT);
>> +        drm_puts(p, "\nScratch registers:\n");
>> +        for (i = 0; i < 16; i++) {
>> +            drm_printf(p, "\t%2d: \t0x%x\n",
>> +                   i, intel_uncore_read(uncore, SOFT_SCRATCH(i)));
>> +        }
>> +    }
>> +}
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>> index 4594ccbeaa34..a5d7a86be4cf 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>> @@ -190,4 +190,6 @@ static inline void intel_guc_disable_msg(struct 
>> intel_guc *guc, u32 mask)
>>   int intel_guc_reset_engine(struct intel_guc *guc,
>>                  struct intel_engine_cs *engine);
>> +void intel_guc_load_status(struct intel_guc *guc, struct drm_printer 
>> *p);
>> +
>>   #endif
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c 
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
>> index caed0d57e704..8cdd6dc3df58 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
>> @@ -672,3 +672,95 @@ void intel_guc_log_handle_flush_event(struct 
>> intel_guc_log *log)
>>   {
>>       queue_work(system_highpri_wq, &log->relay.flush_work);
>>   }
>> +
>> +static const char *
>> +stringify_guc_log_type(enum guc_log_buffer_type type)
>> +{
>> +    switch (type) {
>> +    case GUC_ISR_LOG_BUFFER:
>> +        return "ISR";
>> +    case GUC_DPC_LOG_BUFFER:
>> +        return "DPC";
>> +    case GUC_CRASH_DUMP_LOG_BUFFER:
>> +        return "CRASH";
>> +    default:
>> +        MISSING_CASE(type);
>> +    }
>> +
>> +    return "";
>> +}
>> +
>> +/**
>> + * intel_guc_log_info - dump information about GuC log relay
>> + * @guc: the GuC
>> + * @p: the &drm_printer
>> + *
>> + * Pretty printer for GuC log info
>> + */
>> +void intel_guc_log_info(struct intel_guc_log *log, struct drm_printer 
>> *p)
>> +{
>> +    enum guc_log_buffer_type type;
>> +
>> +    if (!intel_guc_log_relay_created(log)) {
>> +        drm_puts(p, "GuC log relay not created\n");
>> +        return;
>> +    }
>> +
>> +    drm_puts(p, "GuC logging stats:\n");
>> +
>> +    drm_printf(p, "\tRelay full count: %u\n", log->relay.full_count);
>> +
>> +    for (type = GUC_ISR_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER; type++) {
>> +        drm_printf(p, "\t%s:\tflush count %10u, overflow count %10u\n",
>> +               stringify_guc_log_type(type),
>> +               log->stats[type].flush,
>> +               log->stats[type].sampled_overflow);
>> +    }
>> +}
>> +
>> +/**
>> + * intel_guc_log_dump - dump the contents of the GuC log
>> + * @log: the GuC log
>> + * @p: the &drm_printer
>> + * @dump_load_err: dump the log saved on GuC load error
>> + *
>> + * Pretty printer for the GuC log
>> + */
>> +int intel_guc_log_dump(struct intel_guc_log *log, struct drm_printer *p,
>> +               bool dump_load_err)
>> +{
>> +    struct intel_guc *guc = log_to_guc(log);
>> +    struct intel_uc *uc = container_of(guc, struct intel_uc, guc);
>> +    struct drm_i915_gem_object *obj = NULL;
>> +    u32 *map;
>> +    int i = 0;
>> +
>> +    if (!intel_guc_is_supported(guc))
>> +        return -ENODEV;
>> +
>> +    if (dump_load_err)
>> +        obj = uc->load_err_log;
>> +    else if (guc->log.vma)
>> +        obj = guc->log.vma->obj;
>> +
>> +    if (!obj)
>> +        return 0;
>> +
>> +    map = i915_gem_object_pin_map(obj, I915_MAP_WC);
>> +    if (IS_ERR(map)) {
>> +        DRM_DEBUG("Failed to pin object\n");
>> +        drm_puts(p, "(log data unaccessible)\n");
>> +        return PTR_ERR(map);
>> +    }
>> +
>> +    for (i = 0; i < obj->base.size / sizeof(u32); i += 4)
>> +        drm_printf(p, "0x%08x 0x%08x 0x%08x 0x%08x\n",
>> +               *(map + i), *(map + i + 1),
>> +               *(map + i + 2), *(map + i + 3));
>> +
>> +    drm_puts(p, "\n");
>> +
>> +    i915_gem_object_unpin_map(obj);
>> +
>> +    return 0;
>> +}
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h 
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
>> index c252c022c5fc..11fccd0b2294 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
>> @@ -79,4 +79,8 @@ static inline u32 intel_guc_log_get_level(struct 
>> intel_guc_log *log)
>>       return log->level;
>>   }
>> +void intel_guc_log_info(struct intel_guc_log *log, struct drm_printer 
>> *p);
>> +int intel_guc_log_dump(struct intel_guc_log *log, struct drm_printer *p,
>> +               bool dump_load_err);
>> +
>>   #endif
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c 
>> b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
>> index d73dc21686e7..d6097b46600c 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
>> @@ -218,3 +218,32 @@ int intel_huc_check_status(struct intel_huc *huc)
>>       return (status & huc->status.mask) == huc->status.value;
>>   }
>> +
>> +/**
>> + * intel_huc_load_status - dump information about HuC load status
>> + * @huc: the HuC
>> + * @p: the &drm_printer
>> + *
>> + * Pretty printer for HuC load status.
>> + */
>> +void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p)
>> +{
>> +    struct intel_gt *gt = huc_to_gt(huc);
>> +    intel_wakeref_t wakeref;
>> +
>> +    if (!intel_huc_is_supported(huc)) {
>> +        drm_printf(p, "HuC not supported\n");
>> +        return;
>> +    }
>> +
>> +    if (!intel_huc_is_wanted(huc)) {
>> +        drm_printf(p, "HuC disabled\n");
>> +        return;
>> +    }
>> +
>> +    intel_uc_fw_dump(&huc->fw, p);
>> +
>> +    with_intel_runtime_pm(gt->uncore->rpm, wakeref)
>> +        drm_printf(p, "\nHuC status 0x%08x:\n",
>> +               intel_uncore_read(gt->uncore, HUC_STATUS2));
>> +}
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h 
>> b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
>> index a40b9cfc6c22..daee43b661d4 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
>> @@ -57,4 +57,6 @@ static inline bool intel_huc_is_authenticated(struct 
>> intel_huc *huc)
>>       return intel_uc_fw_is_running(&huc->fw);
>>   }
>> +void intel_huc_load_status(struct intel_huc *huc, struct drm_printer 
>> *p);
>> +
>>   #endif
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index de94fcd2032b..56504be2a6ec 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -1251,105 +1251,32 @@ static int i915_llc(struct seq_file *m, void 
>> *data)
>>       return 0;
>>   }
>> -static int i915_huc_load_status_info(struct seq_file *m, void *data)
>> +static int i915_huc_info(struct seq_file *m, void *data)
>>   {
>>       struct drm_i915_private *dev_priv = node_to_i915(m->private);
>> -    intel_wakeref_t wakeref;
>> -    struct drm_printer p;
>> -
>> -    if (!HAS_GT_UC(dev_priv))
>> -        return -ENODEV;
>> -
>> -    p = drm_seq_file_printer(m);
>> -    intel_uc_fw_dump(&dev_priv->gt.uc.huc.fw, &p);
>> -
>> -    with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref)
>> -        seq_printf(m, "\nHuC status 0x%08x:\n", I915_READ(HUC_STATUS2));
>> -
>> -    return 0;
>> -}
>> -
>> -static int i915_guc_load_status_info(struct seq_file *m, void *data)
>> -{
>> -    struct drm_i915_private *dev_priv = node_to_i915(m->private);
>> -    intel_wakeref_t wakeref;
>> -    struct drm_printer p;
>> +    struct intel_huc *huc = &dev_priv->gt.uc.huc;
>> +    struct drm_printer p = drm_seq_file_printer(m);
>> -    if (!HAS_GT_UC(dev_priv))
>> +    if (!intel_huc_is_supported(huc))
>>           return -ENODEV;
> Isn't this test duplicated inside intel_huc_load_status() with a print 
> of 'HuC not supported'? So no need to fail the call here?
> 

intel_huc_load_status is now a generic printer which can be called from 
other places, so it needs to print useful messages in all cases. From 
the debugfs POV, I didn't want to change the legacy behavior of 
returning -ENODEV on platforms that don't support the blob, which IMO is 
a clear eough indication of the lack of support. Note that in the next 
patch the code is changed so that the debgufs files are not even created 
if there is no uC support on the platforms (this is in line with what we 
do for other GT features).

Daniele

>> -    p = drm_seq_file_printer(m);
>> -    intel_uc_fw_dump(&dev_priv->gt.uc.guc.fw, &p);
>> -
>> -    with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref) {
>> -        u32 tmp = I915_READ(GUC_STATUS);
>> -        u32 i;
>> -
>> -        seq_printf(m, "\nGuC status 0x%08x:\n", tmp);
>> -        seq_printf(m, "\tBootrom status = 0x%x\n",
>> -               (tmp & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT);
>> -        seq_printf(m, "\tuKernel status = 0x%x\n",
>> -               (tmp & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT);
>> -        seq_printf(m, "\tMIA Core status = 0x%x\n",
>> -               (tmp & GS_MIA_MASK) >> GS_MIA_SHIFT);
>> -        seq_puts(m, "\nScratch registers:\n");
>> -        for (i = 0; i < 16; i++) {
>> -            seq_printf(m, "\t%2d: \t0x%x\n",
>> -                   i, I915_READ(SOFT_SCRATCH(i)));
>> -        }
>> -    }
>> +    intel_huc_load_status(huc, &p);
>>       return 0;
>>   }
>> -static const char *
>> -stringify_guc_log_type(enum guc_log_buffer_type type)
>> -{
>> -    switch (type) {
>> -    case GUC_ISR_LOG_BUFFER:
>> -        return "ISR";
>> -    case GUC_DPC_LOG_BUFFER:
>> -        return "DPC";
>> -    case GUC_CRASH_DUMP_LOG_BUFFER:
>> -        return "CRASH";
>> -    default:
>> -        MISSING_CASE(type);
>> -    }
>> -
>> -    return "";
>> -}
>> -
>> -static void i915_guc_log_info(struct seq_file *m, struct 
>> intel_guc_log *log)
>> -{
>> -    enum guc_log_buffer_type type;
>> -
>> -    if (!intel_guc_log_relay_created(log)) {
>> -        seq_puts(m, "GuC log relay not created\n");
>> -        return;
>> -    }
>> -
>> -    seq_puts(m, "GuC logging stats:\n");
>> -
>> -    seq_printf(m, "\tRelay full count: %u\n",
>> -           log->relay.full_count);
>> -
>> -    for (type = GUC_ISR_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER; type++) {
>> -        seq_printf(m, "\t%s:\tflush count %10u, overflow count %10u\n",
>> -               stringify_guc_log_type(type),
>> -               log->stats[type].flush,
>> -               log->stats[type].sampled_overflow);
>> -    }
>> -}
>> -
>>   static int i915_guc_info(struct seq_file *m, void *data)
>>   {
>>       struct drm_i915_private *dev_priv = node_to_i915(m->private);
>> -    struct intel_uc *uc = &dev_priv->gt.uc;
>> +    struct intel_guc *guc = &dev_priv->gt.uc.guc;
>> +    struct drm_printer p = drm_seq_file_printer(m);
>> -    if (!intel_uc_uses_guc(uc))
>> +    if (!intel_guc_is_supported(guc))
>>           return -ENODEV;
> As above. No need to bail if the dump call below is trying to return a 
> useful message in the not supported case.
> 
> John.
> 
>> -    i915_guc_log_info(m, &uc->guc.log);
>> +    intel_guc_load_status(guc, &p);
>> +    drm_puts(&p, "\n");
>> +    intel_guc_log_info(&guc->log, &p);
>>       /* Add more as required ... */
>> @@ -1360,39 +1287,14 @@ static int i915_guc_log_dump(struct seq_file 
>> *m, void *data)
>>   {
>>       struct drm_info_node *node = m->private;
>>       struct drm_i915_private *dev_priv = node_to_i915(node);
>> +    struct intel_guc *guc = &dev_priv->gt.uc.guc;
>>       bool dump_load_err = !!node->info_ent->data;
>> -    struct drm_i915_gem_object *obj = NULL;
>> -    u32 *log;
>> -    int i = 0;
>> +    struct drm_printer p = drm_seq_file_printer(m);
>> -    if (!HAS_GT_UC(dev_priv))
>> +    if (!intel_guc_is_supported(guc))
>>           return -ENODEV;
>> -    if (dump_load_err)
>> -        obj = dev_priv->gt.uc.load_err_log;
>> -    else if (dev_priv->gt.uc.guc.log.vma)
>> -        obj = dev_priv->gt.uc.guc.log.vma->obj;
>> -
>> -    if (!obj)
>> -        return 0;
>> -
>> -    log = i915_gem_object_pin_map(obj, I915_MAP_WC);
>> -    if (IS_ERR(log)) {
>> -        DRM_DEBUG("Failed to pin object\n");
>> -        seq_puts(m, "(log data unaccessible)\n");
>> -        return PTR_ERR(log);
>> -    }
>> -
>> -    for (i = 0; i < obj->base.size / sizeof(u32); i += 4)
>> -        seq_printf(m, "0x%08x 0x%08x 0x%08x 0x%08x\n",
>> -               *(log + i), *(log + i + 1),
>> -               *(log + i + 2), *(log + i + 3));
>> -
>> -    seq_putc(m, '\n');
>> -
>> -    i915_gem_object_unpin_map(obj);
>> -
>> -    return 0;
>> +    return intel_guc_log_dump(&guc->log, &p, dump_load_err);
>>   }
>>   static int i915_guc_log_level_get(void *data, u64 *val)
>> @@ -2088,10 +1990,9 @@ static const struct drm_info_list 
>> i915_debugfs_list[] = {
>>       {"i915_gem_fence_regs", i915_gem_fence_regs_info, 0},
>>       {"i915_gem_interrupt", i915_interrupt_info, 0},
>>       {"i915_guc_info", i915_guc_info, 0},
>> -    {"i915_guc_load_status", i915_guc_load_status_info, 0},
>>       {"i915_guc_log_dump", i915_guc_log_dump, 0},
>>       {"i915_guc_load_err_log_dump", i915_guc_log_dump, 0, (void *)1},
>> -    {"i915_huc_load_status", i915_huc_load_status_info, 0},
>> +    {"i915_huc_info", i915_huc_info, 0},
>>       {"i915_frequency_info", i915_frequency_info, 0},
>>       {"i915_ring_freq_table", i915_ring_freq_table, 0},
>>       {"i915_context_status", i915_context_status, 0},
>
John Harrison March 25, 2020, 5:35 p.m. UTC | #3
On 3/25/2020 10:14, Daniele Ceraolo Spurio wrote:
> On 3/25/20 10:05 AM, John Harrison wrote:
>> On 3/11/2020 18:16, Daniele Ceraolo Spurio wrote:
>>> Move the printers to the respective files for clarity. The
>>> guc_load_status debugfs has been squashed in the guc_info one, has
>>> having separate ones wasn't very useful. The HuC debugfs has been
>>> renamed huc_info to match.
>>>
>>> v2: keep printing HUC_STATUS2 (Tony), avoid const->non-const
>>>      container_of (Jani)
>>>
>>> Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Cc: John Harrison <John.C.Harrison@Intel.com>
>>> Cc: Matthew Brost <matthew.brost@intel.com>
>>> Cc: Tony Ye <tony.ye@intel.com>
>>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>>> ---
>>> <snip>
>>>
>>> +static int i915_huc_info(struct seq_file *m, void *data)
>>>   {
>>>       struct drm_i915_private *dev_priv = node_to_i915(m->private);
>>> -    intel_wakeref_t wakeref;
>>> -    struct drm_printer p;
>>> -
>>> -    if (!HAS_GT_UC(dev_priv))
>>> -        return -ENODEV;
>>> -
>>> -    p = drm_seq_file_printer(m);
>>> -    intel_uc_fw_dump(&dev_priv->gt.uc.huc.fw, &p);
>>> -
>>> -    with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref)
>>> -        seq_printf(m, "\nHuC status 0x%08x:\n", 
>>> I915_READ(HUC_STATUS2));
>>> -
>>> -    return 0;
>>> -}
>>> -
>>> -static int i915_guc_load_status_info(struct seq_file *m, void *data)
>>> -{
>>> -    struct drm_i915_private *dev_priv = node_to_i915(m->private);
>>> -    intel_wakeref_t wakeref;
>>> -    struct drm_printer p;
>>> +    struct intel_huc *huc = &dev_priv->gt.uc.huc;
>>> +    struct drm_printer p = drm_seq_file_printer(m);
>>> -    if (!HAS_GT_UC(dev_priv))
>>> +    if (!intel_huc_is_supported(huc))
>>>           return -ENODEV;
>> Isn't this test duplicated inside intel_huc_load_status() with a 
>> print of 'HuC not supported'? So no need to fail the call here?
>>
>
> intel_huc_load_status is now a generic printer which can be called 
> from other places, so it needs to print useful messages in all cases. 
> From the debugfs POV, I didn't want to change the legacy behavior of 
> returning -ENODEV on platforms that don't support the blob, which IMO 
> is a clear eough indication of the lack of support. Note that in the 
> next patch the code is changed so that the debgufs files are not even 
> created if there is no uC support on the platforms (this is in line 
> with what we do for other GT features).
>
> Daniele
>
Okay. That makes sense.

Reviewed-by: John Harrison <John.C.Harrison@Intel.com>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 827d75073879..861657897c0f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -723,3 +723,47 @@  int intel_guc_allocate_and_map_vma(struct intel_guc *guc, u32 size,
 
 	return 0;
 }
+
+/**
+ * intel_guc_load_status - dump information about GuC load status
+ * @guc: the GuC
+ * @p: the &drm_printer
+ *
+ * Pretty printer for GuC load status.
+ */
+void intel_guc_load_status(struct intel_guc *guc, struct drm_printer *p)
+{
+	struct intel_gt *gt = guc_to_gt(guc);
+	struct intel_uncore *uncore = gt->uncore;
+	intel_wakeref_t wakeref;
+
+	if (!intel_guc_is_supported(guc)) {
+		drm_printf(p, "GuC not supported\n");
+		return;
+	}
+
+	if (!intel_guc_is_wanted(guc)) {
+		drm_printf(p, "GuC disabled\n");
+		return;
+	}
+
+	intel_uc_fw_dump(&guc->fw, p);
+
+	with_intel_runtime_pm(uncore->rpm, wakeref) {
+		u32 status = intel_uncore_read(uncore, GUC_STATUS);
+		u32 i;
+
+		drm_printf(p, "\nGuC status 0x%08x:\n", status);
+		drm_printf(p, "\tBootrom status = 0x%x\n",
+			   (status & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT);
+		drm_printf(p, "\tuKernel status = 0x%x\n",
+			   (status & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT);
+		drm_printf(p, "\tMIA Core status = 0x%x\n",
+			   (status & GS_MIA_MASK) >> GS_MIA_SHIFT);
+		drm_puts(p, "\nScratch registers:\n");
+		for (i = 0; i < 16; i++) {
+			drm_printf(p, "\t%2d: \t0x%x\n",
+				   i, intel_uncore_read(uncore, SOFT_SCRATCH(i)));
+		}
+	}
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 4594ccbeaa34..a5d7a86be4cf 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -190,4 +190,6 @@  static inline void intel_guc_disable_msg(struct intel_guc *guc, u32 mask)
 int intel_guc_reset_engine(struct intel_guc *guc,
 			   struct intel_engine_cs *engine);
 
+void intel_guc_load_status(struct intel_guc *guc, struct drm_printer *p);
+
 #endif
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
index caed0d57e704..8cdd6dc3df58 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
@@ -672,3 +672,95 @@  void intel_guc_log_handle_flush_event(struct intel_guc_log *log)
 {
 	queue_work(system_highpri_wq, &log->relay.flush_work);
 }
+
+static const char *
+stringify_guc_log_type(enum guc_log_buffer_type type)
+{
+	switch (type) {
+	case GUC_ISR_LOG_BUFFER:
+		return "ISR";
+	case GUC_DPC_LOG_BUFFER:
+		return "DPC";
+	case GUC_CRASH_DUMP_LOG_BUFFER:
+		return "CRASH";
+	default:
+		MISSING_CASE(type);
+	}
+
+	return "";
+}
+
+/**
+ * intel_guc_log_info - dump information about GuC log relay
+ * @guc: the GuC
+ * @p: the &drm_printer
+ *
+ * Pretty printer for GuC log info
+ */
+void intel_guc_log_info(struct intel_guc_log *log, struct drm_printer *p)
+{
+	enum guc_log_buffer_type type;
+
+	if (!intel_guc_log_relay_created(log)) {
+		drm_puts(p, "GuC log relay not created\n");
+		return;
+	}
+
+	drm_puts(p, "GuC logging stats:\n");
+
+	drm_printf(p, "\tRelay full count: %u\n", log->relay.full_count);
+
+	for (type = GUC_ISR_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER; type++) {
+		drm_printf(p, "\t%s:\tflush count %10u, overflow count %10u\n",
+			   stringify_guc_log_type(type),
+			   log->stats[type].flush,
+			   log->stats[type].sampled_overflow);
+	}
+}
+
+/**
+ * intel_guc_log_dump - dump the contents of the GuC log
+ * @log: the GuC log
+ * @p: the &drm_printer
+ * @dump_load_err: dump the log saved on GuC load error
+ *
+ * Pretty printer for the GuC log
+ */
+int intel_guc_log_dump(struct intel_guc_log *log, struct drm_printer *p,
+		       bool dump_load_err)
+{
+	struct intel_guc *guc = log_to_guc(log);
+	struct intel_uc *uc = container_of(guc, struct intel_uc, guc);
+	struct drm_i915_gem_object *obj = NULL;
+	u32 *map;
+	int i = 0;
+
+	if (!intel_guc_is_supported(guc))
+		return -ENODEV;
+
+	if (dump_load_err)
+		obj = uc->load_err_log;
+	else if (guc->log.vma)
+		obj = guc->log.vma->obj;
+
+	if (!obj)
+		return 0;
+
+	map = i915_gem_object_pin_map(obj, I915_MAP_WC);
+	if (IS_ERR(map)) {
+		DRM_DEBUG("Failed to pin object\n");
+		drm_puts(p, "(log data unaccessible)\n");
+		return PTR_ERR(map);
+	}
+
+	for (i = 0; i < obj->base.size / sizeof(u32); i += 4)
+		drm_printf(p, "0x%08x 0x%08x 0x%08x 0x%08x\n",
+			   *(map + i), *(map + i + 1),
+			   *(map + i + 2), *(map + i + 3));
+
+	drm_puts(p, "\n");
+
+	i915_gem_object_unpin_map(obj);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
index c252c022c5fc..11fccd0b2294 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
@@ -79,4 +79,8 @@  static inline u32 intel_guc_log_get_level(struct intel_guc_log *log)
 	return log->level;
 }
 
+void intel_guc_log_info(struct intel_guc_log *log, struct drm_printer *p);
+int intel_guc_log_dump(struct intel_guc_log *log, struct drm_printer *p,
+		       bool dump_load_err);
+
 #endif
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index d73dc21686e7..d6097b46600c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -218,3 +218,32 @@  int intel_huc_check_status(struct intel_huc *huc)
 
 	return (status & huc->status.mask) == huc->status.value;
 }
+
+/**
+ * intel_huc_load_status - dump information about HuC load status
+ * @huc: the HuC
+ * @p: the &drm_printer
+ *
+ * Pretty printer for HuC load status.
+ */
+void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p)
+{
+	struct intel_gt *gt = huc_to_gt(huc);
+	intel_wakeref_t wakeref;
+
+	if (!intel_huc_is_supported(huc)) {
+		drm_printf(p, "HuC not supported\n");
+		return;
+	}
+
+	if (!intel_huc_is_wanted(huc)) {
+		drm_printf(p, "HuC disabled\n");
+		return;
+	}
+
+	intel_uc_fw_dump(&huc->fw, p);
+
+	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
+		drm_printf(p, "\nHuC status 0x%08x:\n",
+			   intel_uncore_read(gt->uncore, HUC_STATUS2));
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index a40b9cfc6c22..daee43b661d4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -57,4 +57,6 @@  static inline bool intel_huc_is_authenticated(struct intel_huc *huc)
 	return intel_uc_fw_is_running(&huc->fw);
 }
 
+void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
+
 #endif
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index de94fcd2032b..56504be2a6ec 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1251,105 +1251,32 @@  static int i915_llc(struct seq_file *m, void *data)
 	return 0;
 }
 
-static int i915_huc_load_status_info(struct seq_file *m, void *data)
+static int i915_huc_info(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	intel_wakeref_t wakeref;
-	struct drm_printer p;
-
-	if (!HAS_GT_UC(dev_priv))
-		return -ENODEV;
-
-	p = drm_seq_file_printer(m);
-	intel_uc_fw_dump(&dev_priv->gt.uc.huc.fw, &p);
-
-	with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref)
-		seq_printf(m, "\nHuC status 0x%08x:\n", I915_READ(HUC_STATUS2));
-
-	return 0;
-}
-
-static int i915_guc_load_status_info(struct seq_file *m, void *data)
-{
-	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	intel_wakeref_t wakeref;
-	struct drm_printer p;
+	struct intel_huc *huc = &dev_priv->gt.uc.huc;
+	struct drm_printer p = drm_seq_file_printer(m);
 
-	if (!HAS_GT_UC(dev_priv))
+	if (!intel_huc_is_supported(huc))
 		return -ENODEV;
 
-	p = drm_seq_file_printer(m);
-	intel_uc_fw_dump(&dev_priv->gt.uc.guc.fw, &p);
-
-	with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref) {
-		u32 tmp = I915_READ(GUC_STATUS);
-		u32 i;
-
-		seq_printf(m, "\nGuC status 0x%08x:\n", tmp);
-		seq_printf(m, "\tBootrom status = 0x%x\n",
-			   (tmp & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT);
-		seq_printf(m, "\tuKernel status = 0x%x\n",
-			   (tmp & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT);
-		seq_printf(m, "\tMIA Core status = 0x%x\n",
-			   (tmp & GS_MIA_MASK) >> GS_MIA_SHIFT);
-		seq_puts(m, "\nScratch registers:\n");
-		for (i = 0; i < 16; i++) {
-			seq_printf(m, "\t%2d: \t0x%x\n",
-				   i, I915_READ(SOFT_SCRATCH(i)));
-		}
-	}
+	intel_huc_load_status(huc, &p);
 
 	return 0;
 }
 
-static const char *
-stringify_guc_log_type(enum guc_log_buffer_type type)
-{
-	switch (type) {
-	case GUC_ISR_LOG_BUFFER:
-		return "ISR";
-	case GUC_DPC_LOG_BUFFER:
-		return "DPC";
-	case GUC_CRASH_DUMP_LOG_BUFFER:
-		return "CRASH";
-	default:
-		MISSING_CASE(type);
-	}
-
-	return "";
-}
-
-static void i915_guc_log_info(struct seq_file *m, struct intel_guc_log *log)
-{
-	enum guc_log_buffer_type type;
-
-	if (!intel_guc_log_relay_created(log)) {
-		seq_puts(m, "GuC log relay not created\n");
-		return;
-	}
-
-	seq_puts(m, "GuC logging stats:\n");
-
-	seq_printf(m, "\tRelay full count: %u\n",
-		   log->relay.full_count);
-
-	for (type = GUC_ISR_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER; type++) {
-		seq_printf(m, "\t%s:\tflush count %10u, overflow count %10u\n",
-			   stringify_guc_log_type(type),
-			   log->stats[type].flush,
-			   log->stats[type].sampled_overflow);
-	}
-}
-
 static int i915_guc_info(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	struct intel_uc *uc = &dev_priv->gt.uc;
+	struct intel_guc *guc = &dev_priv->gt.uc.guc;
+	struct drm_printer p = drm_seq_file_printer(m);
 
-	if (!intel_uc_uses_guc(uc))
+	if (!intel_guc_is_supported(guc))
 		return -ENODEV;
 
-	i915_guc_log_info(m, &uc->guc.log);
+	intel_guc_load_status(guc, &p);
+	drm_puts(&p, "\n");
+	intel_guc_log_info(&guc->log, &p);
 
 	/* Add more as required ... */
 
@@ -1360,39 +1287,14 @@  static int i915_guc_log_dump(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = m->private;
 	struct drm_i915_private *dev_priv = node_to_i915(node);
+	struct intel_guc *guc = &dev_priv->gt.uc.guc;
 	bool dump_load_err = !!node->info_ent->data;
-	struct drm_i915_gem_object *obj = NULL;
-	u32 *log;
-	int i = 0;
+	struct drm_printer p = drm_seq_file_printer(m);
 
-	if (!HAS_GT_UC(dev_priv))
+	if (!intel_guc_is_supported(guc))
 		return -ENODEV;
 
-	if (dump_load_err)
-		obj = dev_priv->gt.uc.load_err_log;
-	else if (dev_priv->gt.uc.guc.log.vma)
-		obj = dev_priv->gt.uc.guc.log.vma->obj;
-
-	if (!obj)
-		return 0;
-
-	log = i915_gem_object_pin_map(obj, I915_MAP_WC);
-	if (IS_ERR(log)) {
-		DRM_DEBUG("Failed to pin object\n");
-		seq_puts(m, "(log data unaccessible)\n");
-		return PTR_ERR(log);
-	}
-
-	for (i = 0; i < obj->base.size / sizeof(u32); i += 4)
-		seq_printf(m, "0x%08x 0x%08x 0x%08x 0x%08x\n",
-			   *(log + i), *(log + i + 1),
-			   *(log + i + 2), *(log + i + 3));
-
-	seq_putc(m, '\n');
-
-	i915_gem_object_unpin_map(obj);
-
-	return 0;
+	return intel_guc_log_dump(&guc->log, &p, dump_load_err);
 }
 
 static int i915_guc_log_level_get(void *data, u64 *val)
@@ -2088,10 +1990,9 @@  static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_gem_fence_regs", i915_gem_fence_regs_info, 0},
 	{"i915_gem_interrupt", i915_interrupt_info, 0},
 	{"i915_guc_info", i915_guc_info, 0},
-	{"i915_guc_load_status", i915_guc_load_status_info, 0},
 	{"i915_guc_log_dump", i915_guc_log_dump, 0},
 	{"i915_guc_load_err_log_dump", i915_guc_log_dump, 0, (void *)1},
-	{"i915_huc_load_status", i915_huc_load_status_info, 0},
+	{"i915_huc_info", i915_huc_info, 0},
 	{"i915_frequency_info", i915_frequency_info, 0},
 	{"i915_ring_freq_table", i915_ring_freq_table, 0},
 	{"i915_context_status", i915_context_status, 0},