On pe, 2017-03-10 at 12:46 +0100, Arkadiusz Hiler wrote:
> Current version of intel_guc_init_hw() does a lot:
> - cares about submission
> - loads huc
> - implement WA
>
> This change offloads some of the logic to intel_uc_init_hw(), which now
> cares about the above.
>
> v2: rename guc_hw_reset and fix typo in define name (M. Wajdeczko)
> v3: rename once again
> v4: remove spurious comments and add some style (J. Lahtinen)
> v5: flow changes, got rid of dead checks (M. Wajdeczko)
> v6: rebase
>
> Cc: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
> Cc: Michal Winiarski <michal.winiarski@xxxxxxxxx>
> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx>
<SNIP>
> +int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> +{
> + int ret, attempts;
> + const int guc_wa_hash_check_not_set_attempts = 3;
I wouldn't bother with this const, the Wa comment makes it pretty clear
already.
Extra newline here, too.
<SNIP>
> +fail:
> + /*
> + * We've failed to load the firmware :(
> + *
> + * Decide whether to disable GuC submission and fall back to
> + * execlist mode, and whether to hide the error by returning
> + * zero or to return -EIO, which the caller will treat as a
> + * nonfatal error (i.e. it doesn't prevent driver load, but
> + * marks the GPU as wedged until reset).
> + */
> + DRM_ERROR("GuC init failed\n");
> + if (i915.enable_guc_loading > 1 || i915.enable_guc_submission > 1)
> + ret = -EIO;
> + else
> + ret = 0;
> +
> + if (i915.enable_guc_submission) {
> + i915.enable_guc_submission = 0;
> + DRM_NOTE("Falling back from GuC submission to execlist mode\n");
> + }
> +
I'd drop the above code to the bottom.
> + i915_ggtt_disable_guc(dev_priv);
> + intel_guc_release_interrupts(dev_priv);
> + i915_guc_submission_disable(dev_priv);
> + i915_guc_submission_fini(dev_priv);
And make this teardown oniony with some more labels.
With those changes, looks good to me.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
