[Intel-gfx] [PATCH 5/5] drm/i915: Add dmc firmware debugfs status entry

[Mika Kuoppala]

Animesh Manna <animesh.manna at intel.com> writes:

> On 9/18/2015 8:47 PM, Mika Kuoppala wrote:
>> Add debugfs entry for csr/dmc fw to inspect firmware
>> loading status and version.
>>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_debugfs.c | 32 ++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/i915_reg.h     |  5 +++++
>>   drivers/gpu/drm/i915/intel_csr.c    |  3 ---
>>   3 files changed, 37 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 72ae347..4a798a6 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -2509,6 +2509,37 @@ static int i915_guc_log_dump(struct seq_file *m, void *data)
>>   	return 0;
>>   }
>>   
>> +static int i915_dmc_load_status_info(struct seq_file *m, void *data)
>> +{
>> +	struct drm_info_node *node = m->private;
>> +	struct drm_i915_private *dev_priv = node->minor->dev->dev_private;
>> +	struct intel_csr *csr = &dev_priv->csr;
>> +	uint32_t state;
>> +	const char * const state_str[] = { "uninitialized",
>> +					   "loaded",
>> +					   "failed",
>> +					   "unknown" };
>> +
>> +	seq_puts(m, "DMC firmware status:\n");
>> +
>> +	mutex_lock(&dev_priv->csr_lock);
>
> DMC-redesign series is floated for review where csr_lock is removed completely, not sure do we need mutex lock here.
>
>> +
>> +	seq_printf(m, "\tpath: %s\n", csr->fw_path);
>> +	seq_printf(m, "\tfw_ver: %u.%u\n", csr->dmc_ver_major,
>> +		   csr->dmc_ver_minor);
>> +	seq_printf(m, "\tsize: %u bytes\n", csr->dmc_fw_size * 4);
>> +	state = (uint32_t)csr->state <= 3 ? csr->state : 3;
>> +	seq_printf(m, "\tstate: %s\n", state_str[state]);
>> +
>> +	seq_printf(m, "\tprogram base: 0x%08x\n", I915_READ(CSR_PROGRAM_BASE));
>> +	seq_printf(m, "\tssp base: 0x%08x\n", I915_READ(CSR_SSP_BASE));
>> +	seq_printf(m, "\thtp: 0x%08x\n", I915_READ(CSR_HTP_SKL));
>
> Better print dmc firmware loading related info if firmware is loaded successfully.
> In failure case, only firmware not loaded msg with path I feel is sufficient.
>

The loader peeks these registers to determine if the loading was
succesful. If we toggle the register output based on the final state,
we lose debuggability in a case where the loading fails. As we wont
get to read the registers through this debugfs entry.

If the firmware has been loaded successfully, there is
little value in these registers as they are asserted
into known values in the code. 

The debugfs entry was made to sched some light if
we encounter a case where firmware didn't load succesfully.
I guess the real question is should we keep this or drop it
as unnecessary?

-Mika

> -Animesh
>
>> +
>> +	mutex_unlock(&dev_priv->csr_lock);
>> +
>> +	return 0;
>> +}
>> +
>>   static int i915_edp_psr_status(struct seq_file *m, void *data)
>>   {
>>   	struct drm_info_node *node = m->private;
>> @@ -5173,6 +5204,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
>>   	{"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_dmc_load_status", i915_dmc_load_status_info, 0},
>>   	{"i915_frequency_info", i915_frequency_info, 0},
>>   	{"i915_hangcheck_info", i915_hangcheck_info, 0},
>>   	{"i915_drpc_info", i915_drpc_info, 0},
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 67bf205..cd040ff 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7977,4 +7977,9 @@ enum skl_disp_power_wells {
>>   #define GEN9_VEBOX_MOCS_0	0xcb00	/* Video MOCS base register*/
>>   #define GEN9_BLT_MOCS_0		0xcc00	/* Blitter MOCS base register*/
>>   
>> +/* DMC/CSR firmware */
>> +#define CSR_PROGRAM_BASE		0x80000
>> +#define CSR_SSP_BASE			0x8F074
>> +#define CSR_HTP_SKL			0x8F004
>> +
>>   #endif /* _I915_REG_H_ */
>> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>> index 73807c3..876c839 100644
>> --- a/drivers/gpu/drm/i915/intel_csr.c
>> +++ b/drivers/gpu/drm/i915/intel_csr.c
>> @@ -51,11 +51,8 @@ MODULE_FIRMWARE(I915_CSR_SKL);
>>   /*
>>   * SKL CSR registers for DC5 and DC6
>>   */
>> -#define CSR_PROGRAM_BASE		0x80000
>>   #define CSR_SSP_BASE_ADDR_GEN9		0x00002FC0
>>   #define CSR_HTP_ADDR_SKL		0x00500034
>> -#define CSR_SSP_BASE			0x8F074
>> -#define CSR_HTP_SKL			0x8F004
>>   #define CSR_LAST_WRITE			0x8F034
>>   #define CSR_LAST_WRITE_VALUE		0xc003b400
>>   /* MMIO address range for CSR program (0x80000 - 0x82FFF) */