Re: [PATCH 09/10] drm/i915/uc: Separate firmware selection and preparation

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

On pe, 2017-02-24 at 16:40 +0100, Arkadiusz Hiler wrote:
> intel_{h,g}uc_init_fw selects correct firmware and then triggers it's
> preparation (fetch + initial parsing).
> 
> This change separates out select steps, so those can be called by
> the sanitize_options().
> 
> Then, during the init_fw() we prepare the firmware if the firmware was
> selected.
> 
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

One comment below.

> @@ -66,6 +66,16 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
>  		if (!i915.enable_guc_loading)
>  			i915.enable_guc_submission = 0;
>  	}
> +
> +	if (i915.enable_guc_loading) {
> +		if (HAS_HUC_UCODE(dev_priv))
> +			intel_huc_select_fw(&dev_priv->huc);
> +
> +		intel_guc_select_fw(&dev_priv->guc);
> +
> +		if (!dev_priv->guc.fw.path)
> +			i915.enable_guc_loading = 0;

Maybe make this (as suggested by Michal):

	if (intel_guc_select_fw(&dev_priv->guc) < 0)
		i915.enable_guc_loading = 0;

I think the select_fw is appropriate in this place once we introduce
the option to override the FW path, it'll part of sanitization.

Regards, Joonas
-- 
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