[Intel-gfx] [PATCH v3] drm/i915: Add Guc/HuC firmware details to error state

[Michal Wajdeczko]

On Fri, 20 Oct 2017 11:41:57 +0200, Joonas Lahtinen  
<joonas.lahtinen at linux.intel.com> wrote:

> On Thu, 2017-10-19 at 20:23 +0000, Michal Wajdeczko wrote:
>> Include GuC and HuC firmware details in captured error state
>> to provide additional debug information. To reuse existing
>> uc firmware pretty printer, introduce new drm-printer variant
>> that works with our i915_error_state_buf output. Also update
>> uc firmware pretty printer to accept const input.
>>
>> v2: don't rely on current caps (Chris)
>>     dump correct fw info (Michal)
>> v3: simplify capture of custom paths (Chris)
>>
>> Suggested-by: Chris Wilson <chris at chris-wilson.co.uk>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>
> A few comments below.
>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>
> Regards, Joonas
>
>> @@ -774,6 +793,11 @@ int i915_error_state_to_str(struct  
>> drm_i915_error_state_buf *m,
>>  	err_print_capabilities(m, &error->device_info);
>>  	err_print_params(m, &error->params);
>>
>> +	if (error->device_info.has_guc) {
>> +		intel_uc_fw_dump(&error->guc_fw, &p);
>> +		intel_uc_fw_dump(&error->huc_fw, &p);
>> +	}
>
> I might use "error->{g,h}uc_fw.path" as the condition, for both

If we use "uc_fw->path" as condition then we have to assume that
lack of uc firmware info is for this reason, and not from corrupted
or incomplete or legacy format error dump ;)

I would prefer to stay with "has_guc" condition here, and trim
output from uc fw printer if path is not set. This way we will
cover all possible scenario (no guc = no fw, no fw = no fw info)

> individually. We will have DMC here in the future, too.

Hmm, while not here, info about DMC fw is already included in
error dump. But in their case reported DMC fw state (loaded flag,
version) is taken directly from dev_priv instead of captured error
state.

Michal