On ke, 2017-02-22 at 13:41 +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_load(), which now
> cares about the above.
>
> v2: rename guc_hw_reset and fix typo in define name (M. Wajdeczko)
> v3: rename once again
>
> 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>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx>
<SNIP>
> @@ -443,147 +425,53 @@ int intel_guc_init_hw(struct drm_i915_private *dev_priv)
> {
> struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
> const char *fw_path = guc_fw->path;
> - int retries, ret, err;
> + int ret;
>
> DRM_DEBUG_DRIVER("GuC fw status: path %s, fetch %s, load %s\n",
> fw_path,
> intel_uc_fw_status_repr(guc_fw->fetch_status),
> intel_uc_fw_status_repr(guc_fw->load_status));
>
> - /* Loading forbidden, or no firmware to load? */
> - if (!i915.enable_guc_loading) {
> - err = 0;
> - goto fail;
> - } else if (fw_path == NULL) {
> + if (fw_path == NULL) {
if (!fw_path)
> /* Device is known to have no uCode (e.g. no GuC) */
Spurious comment.
> - err = -ENXIO;
> - goto fail;
> + return -ENXIO;
> } else if (*fw_path == '\0') {
> /* Device has a GuC but we don't know what f/w to load? */
Even more spurious comment, as there's WARN :)
> WARN(1, "No GuC firmware known for this platform!\n");
> - err = -ENODEV;
> - goto fail;
> + return -ENODEV;
> }
>
> /* Fetch failed, or already fetched but failed to load? */
Code is again self-documenting here.
> if (guc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS) {
> - err = -EIO;
> - goto fail;
> + return -EIO;
> } else if (guc_fw->load_status == INTEL_UC_FIRMWARE_FAIL) {
> - err = -ENOEXEC;
> - goto fail;
> + return -ENOEXEC;
> }
Single line braces can be dropped.
> +int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> +{
This function is still lacking onion tear-down (because it's so
convoluted), but it's in the better direction (and it was previously
so).
+++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -29,6 +29,8 @@
> #include "intel_ringbuffer.h"
>
> #include "i915_vma.h"
> +/* WaEnableGuCBootHashCheckNotSet:skl,bxt */
> +#define GUC_WA_HASH_CHECK_NOT_SET_ATTEMPTS 3
As this is a W/A only, I'd keep the code local to single spot.
With above stylistic nitpicks;
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
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
