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 <[email protected]> > Signed-off-by: Michal Wajdeczko <[email protected]> > Cc: Chris Wilson <[email protected]> > Cc: Joonas Lahtinen <[email protected]>
A few comments below. Reviewed-by: Joonas Lahtinen <[email protected]> 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 individually. We will have DMC here in the future, too. > +static void i915_capture_uc_state(struct drm_i915_private *dev_priv, > + struct i915_gpu_state *error) > +{ > + error->guc_fw = dev_priv->guc.fw; > + error->huc_fw = dev_priv->huc.fw; > + > + /* Make sure to capture custom firmware paths */ This is again a "what" comment, not "why". And they're not necessarily the custom ones. The "why" is generic to all error capture funcs. > + error->guc_fw.path = kstrdup(error->guc_fw.path, GFP_ATOMIC); > + error->huc_fw.path = kstrdup(error->huc_fw.path, GFP_ATOMIC); > +} -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
