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

[Michal Wajdeczko]

On Fri, 20 Oct 2017 14:43:46 +0200, Chris Wilson  
<chris at chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2017-10-20 13:36:59)
>> On Fri, 20 Oct 2017 14:21:16 +0200, Chris Wilson
>> <chris at chris-wilson.co.uk> wrote:
>>
>> > Quoting Michal Wajdeczko (2017-10-20 13:14:45)
>> >> On Fri, 20 Oct 2017 13:44:04 +0200, Chris Wilson
>> >> <chris at chris-wilson.co.uk> wrote:
>> >>
>> >> > Quoting Joonas Lahtinen (2017-10-20 10:41:57)
>> >> >> On Thu, 2017-10-19 at 20:23 +0000, Michal Wajdeczko wrote:
>> >> >> > @@ -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
>> >> >> individually. We will have DMC here in the future, too.
>> >> >
>> >> > That's the type of predicate I was looking for. If we can tell when
>> >> the
>> >> > fw has been loaded just by looking at the uc_fw struct, please do  
>> so.
>> >>
>> >> If you want to include uc fw info only when fw was loaded at capture
>> >> moment,
>> >> then we can look directly at its "load_status" field:
>> >>
>> >>         if (error->guc_fw.load_status == INTEL_UC_FIRMWARE_SUCCESS)
>> >>
>> >> But what if fw was loaded earlier, before error capture is started?
>> >> Don't we want to capture whole driver configuration?
>> >
>> > The goal is to know what the driver/hw was doing at the time of the
>> > capture (which we presume is still identical to the hang).
>> >
>> > At this point, we are just asking ourselves what is the most  
>> unambiguous
>> > way of printing valid data. We already show has_guc in the output, so  
>> it
>> > boils down to what was the uc_fw state. It doesn't need to be
>> > load_status == SUCCESS, as we should show a failed attempt to upload  
>> the
>> > fw as well.
>> >
>> > So intel_uc_fw.size?
>>
>> But then in case of zero size fw you will not get info about load  
>> failure
>> ;)
>
> Heh, I would just go back and make sure we had a clear well of deducing
> when fw was being utilised. Maybe
>
> 	if (uc_fw.load_status != INTEL_UC_FIRMWARE_NONE)
>

If we don't care about earlier fw fetch attempts/errors, then ok.

Michal