[Intel-gfx] [PATCH] drm/i915: Shut up GuC errors when it's disabled

[Chris Wilson]

On Fri, Oct 23, 2015 at 10:33:56AM +0200, Daniel Vetter wrote:
> DRM_ERROR an continue without any issues aren't allowed since that
> causes noise in the CI system. But we absolutely want to have the
> DRM_ERROR when we want to run with GuC.
> 
> For simplicity just short-circuit all the loader code when it's not
> needed.

Or even simpler would be only to DRM_ERROR if enabled.

> Cc: Alex Dai <yu.dai at intel.com>
> Cc: Dave Gordon <david.s.gordon at intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c         | 10 +---------
>  drivers/gpu/drm/i915/intel_guc_loader.c |  6 ++++++
>  2 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 01fef54ecb2d..0604d15bde72 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4747,18 +4747,10 @@ i915_gem_init_hw(struct drm_device *dev)
>  	if (HAS_GUC_UCODE(dev)) {
>  		ret = intel_guc_ucode_load(dev);
>  		if (ret) {
> -			/*
> -			 * If we got an error and GuC submission is enabled, map
> -			 * the error to -EIO so the GPU will be declared wedged.
> -			 * OTOH, if we didn't intend to use the GuC anyway, just
> -			 * discard the error and carry on.
> -			 */
>  			DRM_ERROR("Failed to initialize GuC, error %d%s\n", ret,
>  				  i915.enable_guc_submission ? "" :
>  				  " (ignored)");

The error message is now confused.

> -			ret = i915.enable_guc_submission ? -EIO : 0;
> -			if (ret)
> -				goto out;
> +			return -EIO;

You still need to jump to out to undo the rpm.
ret = -EIO;
goto out;


>  		}
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index c0281df52702..bbbf9b3b00eb 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -364,6 +364,9 @@ int intel_guc_ucode_load(struct drm_device *dev)
>  	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
>  	int err = 0;
>  
> +	if (!i915.enable_guc_submission)
> +		return 0;

Further simplification inside intel_guc_ucode_load() now possible, or
the question is why is the firmware being loaded currently if not used?

>  	DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
>  		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
>  		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
> @@ -589,6 +592,9 @@ void intel_guc_ucode_init(struct drm_device *dev)
>  		fw_path = "";	/* unknown device */
>  	}
>  
> +	if (!i915.enable_guc_submission)
> +		return;

Can be moved ahead of this block. Later we can then use
dev_priv->guc.guc_fw->guc_dev (the tautology!) to see if we have setup
the GUC paths.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre