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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 20 Oct 2017 16:50:10 +0200, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:

Quoting Michal Wajdeczko (2017-10-20 15:44:02)
On Fri, 20 Oct 2017 11:41:57 +0200, Joonas Lahtinen
<joonas.lahtinen@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
>> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
>
> A few comments below.
>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
>
> 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)

I still do not understand the insistence on has_guc. no fw is no fw, no
guc is no guc.

I just wanted to cover the case where we have guc but without fw
(explicitly expressed as null path) while skipping fw info for
platforms without guc. No more guesses. No unwanted noise.

Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux