Animesh Manna <animesh.manna@xxxxxxxxx> 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@xxxxxxxxx>
>> ---
>> 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) */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
