Re: [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 07/06/16 09:14, Dave Gordon wrote:
> The last stage of the GuC loader also sanitises the GuC submission
> settings, so should be called unconditionally (even on platforms
> without a GuC) to ensure consistent settings; in particular, this
> prevents any attempt to use GuC submission on GuCless platforms!
>
> Also fix error path handling and clarify DRM_INFO fallback message.
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem.c         |  8 +++-----
>   drivers/gpu/drm/i915/intel_guc_loader.c | 12 ++++++++----
>   2 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1bfc260..eae8d7a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4930,11 +4930,9 @@ int i915_gem_init_engines(struct drm_device *dev)
>   	intel_mocs_init_l3cc_table(dev);
>
>   	/* We can't enable contexts until all firmware is loaded */
> -	if (HAS_GUC(dev)) {
> -		ret = intel_guc_setup(dev);
> -		if (ret)
> -			goto out;
> -	}
> +	ret = intel_guc_setup(dev);
> +	if (ret)
> +		goto out;
>
>   	/*
>   	 * Increment the next seqno by 0x100 so we have a visible break
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index f2b88c7..4e34c2e 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -425,9 +425,13 @@ int intel_guc_setup(struct drm_device *dev)
>   	if (!i915.enable_guc_loading) {
>   		err = 0;
>   		goto fail;
> -	} else if (fw_path == NULL || *fw_path == '\0') {
> -		if (*fw_path == '\0')

Ops. I can only assume I meant !fw_path.

> -			DRM_INFO("No GuC firmware known for this platform\n");
> +	} else if (fw_path == NULL) {
> +		/* Device is known to have no uCode (e.g. no GuC) */
> +		err = -ENXIO;
> +		goto fail;
> +	} else if (*fw_path == '\0') {
> +		/* Device has a GuC but we don't know what f/w to load? */
> +		DRM_INFO("No GuC firmware known for this platform\n");
>   		err = -ENODEV;
>   		goto fail;
>   	}
> @@ -535,7 +539,7 @@ int intel_guc_setup(struct drm_device *dev)
>   		if (fw_path == NULL)
>   			DRM_INFO("GuC submission without firmware not supported\n");
>   		if (ret == 0)
> -			DRM_INFO("Falling back to execlist mode\n");
> +			DRM_INFO("Falling back from GuC submission to execlist mode\n");
>   		else
>   			DRM_ERROR("GuC init failed: %d\n", ret);
>   	}
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx