On Tue, May 02, 2017 at 10:32:42AM +0000, Michal Wajdeczko wrote:
> It is safer to setup valid send function after successful GuC
> hardware initialization. In addition we prepare placeholder
> where we can setup any alternate GuC communication mechanism.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_uc.c | 27 ++++++++++++++++++++++++++-
> drivers/gpu/drm/i915/intel_uc.h | 1 +
> 2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 900e376..5957a95 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -99,7 +99,7 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
> struct intel_guc *guc = &dev_priv->guc;
>
> mutex_init(&guc->send_mutex);
> - guc->send = intel_guc_send_mmio;
> + guc->send = intel_guc_send_nop;
> }
>
> static void fetch_uc_fw(struct drm_i915_private *dev_priv,
> @@ -252,13 +252,27 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
> __intel_uc_fw_fini(&dev_priv->huc.fw);
> }
>
> +static int guc_enable_communication(struct intel_guc *guc)
> +{
> + /* XXX: placeholder for alternate setup */
> + guc->send = intel_guc_send_mmio;
> + return 0;
> +}
> +
> +static void guc_disable_communication(struct intel_guc *guc)
> +{
> + guc->send = intel_guc_send_nop;
> +}
> +
> int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> {
> + struct intel_guc *guc = &dev_priv->guc;
> int ret, attempts;
>
> if (!i915.enable_guc_loading)
> return 0;
>
> + guc_disable_communication(guc);
> gen9_reset_guc_interrupts(dev_priv);
>
> /* We need to notify the guc whenever we change the GGTT */
> @@ -308,6 +322,10 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> if (ret)
> goto err_submission;
>
> + ret = guc_enable_communication(guc);
> + if (ret)
> + goto err_submission;
> +
> intel_guc_auth_huc(dev_priv);
> if (i915.enable_guc_submission) {
> if (i915.guc_log_level >= 0)
> @@ -330,6 +348,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> * marks the GPU as wedged until reset).
> */
> err_interrupts:
> + guc_disable_communication(guc);
> gen9_disable_guc_interrupts(dev_priv);
> err_submission:
> if (i915.enable_guc_submission)
> @@ -364,6 +383,12 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
> i915_ggtt_disable_guc(dev_priv);
> }
>
> +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
> +{
> + WARN(1, "Unexpected send: action=%#x\n", *action);
> + return -ENOSYS;
> +}
> +
> /*
> * This function implements the MMIO based host to GuC interface.
> */
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 2f0229d..1e0eecd 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -227,6 +227,7 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
> int intel_uc_init_hw(struct drm_i915_private *dev_priv);
> void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
> int intel_guc_sample_forcewake(struct intel_guc *guc);
> +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
Was there a reason for exporting the nop? Its a purely internal BUG_ON()
function that should only be set when disabling the guc. Afaict, it
should be private.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
