On Fri, Feb 24, 2017 at 04:40:03PM +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.winiar...@intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
> ---
It looks strange that in one series you're changing function names
or their logic few times ... patch ordering really matters ;)
Please consider reorder/squash.
> drivers/gpu/drm/i915/intel_guc_loader.c | 12 ++----------
> drivers/gpu/drm/i915/intel_huc.c | 14 ++------------
> drivers/gpu/drm/i915/intel_uc.c | 20 ++++++++++++++------
> drivers/gpu/drm/i915/intel_uc.h | 4 ++--
> 4 files changed, 20 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 64f50bd..8ccd832 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -467,15 +467,10 @@ int intel_guc_init_hw(struct drm_i915_private *dev_priv)
> }
>
> /**
> - * intel_guc_init_fw() - select and prepare firmware for loading
> + * intel_guc_select_fw() - selects GuC firmware for loading
> * @guc: intel_guc struct
> - *
> - * Called early during driver load, but after GEM is initialised.
> - *
> - * The firmware will be transferred to the GuC's memory later,
> - * when intel_guc_init_hw() is called.
> */
> -void intel_guc_init_fw(struct intel_guc *guc)
> +void intel_guc_select_fw(struct intel_guc *guc)
> {
> struct drm_i915_private *dev_priv = guc_to_i915(guc);
>
> @@ -498,11 +493,8 @@ void intel_guc_init_fw(struct intel_guc *guc)
> guc->fw.minor_ver_wanted = KBL_FW_MINOR;
> } else {
> DRM_ERROR("No GuC firmware known for platform with GuC!\n");
> - i915.enable_guc_loading = 0;
> return;
> }
> -
> - intel_uc_prepare_fw(dev_priv, &guc->fw);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_huc.c
> b/drivers/gpu/drm/i915/intel_huc.c
> index 757f618..d073a68 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -141,18 +141,10 @@ static int huc_ucode_xfer(struct drm_i915_private
> *dev_priv)
> }
>
> /**
> - * intel_huc_init_fw() - select and prepare firmware for loading
> + * intel_huc_select_fw() - selects HuC firmware for loading
> * @huc: intel_huc struct
> - *
> - * Called early during driver load, but after GEM is initialised. The loading
> - * will continue only when driver explicitly specify firmware name and
> version.
> - * All other cases are considered as INTEL_UC_FIRMWARE_NONE either because HW
> - * is not capable or driver yet support it. And there will be no error
> message
> - * for INTEL_UC_FIRMWARE_NONE cases.
> - *
> - * The DMA-copying to HW is done later when intel_huc_init_hw() is called.
> */
> -void intel_huc_init_fw(struct intel_huc *huc)
> +void intel_huc_select_fw(struct intel_huc *huc)
> {
> struct drm_i915_private *dev_priv = huc_to_i915(huc);
>
> @@ -177,8 +169,6 @@ void intel_huc_init_fw(struct intel_huc *huc)
> DRM_ERROR("No HuC firmware known for platform with HuC!\n");
> return;
> }
> -
> - intel_uc_prepare_fw(dev_priv, &huc->fw);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index ac9ad59..89681b37 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -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;
> }
Code below is only applicable to "else" from the above
> +
> + 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 simpler like this:
i915.enable_guc_loading = intel_guc_select_fw(guc);
and likely it can be done earlier too.
> + }
> }
>
> void intel_uc_init_early(struct drm_i915_private *dev_priv)
> @@ -75,13 +85,11 @@ void intel_uc_init_early(struct drm_i915_private
> *dev_priv)
>
> void intel_uc_init_fw(struct drm_i915_private *dev_priv)
> {
> - if (!i915.enable_guc_loading)
> - return;
> + if (dev_priv->huc.fw.path)
Maybe we can move this common "if" to "intel_uc_prepare_fw()" ?
-Michal
> + intel_uc_prepare_fw(dev_priv, &dev_priv->huc.fw);
>
> - if (HAS_HUC_UCODE(dev_priv))
> - intel_huc_init_fw(&dev_priv->huc);
> -
> - intel_guc_init_fw(&dev_priv->guc);
> + if (dev_priv->guc.fw.path)
> + intel_uc_prepare_fw(dev_priv, &dev_priv->guc.fw);
> }
>
> int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index d19c95e..5f04ea1 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -194,7 +194,7 @@ void intel_uc_prepare_fw(struct drm_i915_private
> *dev_priv,
> struct intel_uc_fw *uc_fw);
>
> /* intel_guc_loader.c */
> -void intel_guc_init_fw(struct intel_guc *guc);
> +void intel_guc_select_fw(struct intel_guc *guc);
> int intel_guc_init_hw(struct drm_i915_private *dev_priv);
> void intel_guc_fini(struct drm_i915_private *dev_priv);
> const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status);
> @@ -230,7 +230,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
> }
>
> /* intel_huc.c */
> -void intel_huc_init_fw(struct intel_huc *huc);
> +void intel_huc_select_fw(struct intel_huc *huc);
> void intel_huc_fini(struct drm_i915_private *dev_priv);
> int intel_huc_init_hw(struct drm_i915_private *dev_priv);
> void intel_guc_auth_huc(struct drm_i915_private *dev_priv);
> --
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
