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

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

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@chris-wilson.co.uk>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

A few comments below.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@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
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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx