[05/30] drm/i915/slpc: Sanitize GuC version

[Fiedorowicz, Lukasz]

On Mon, 2017-08-14 at 11:24 +0530, Sagar Arun Kamble wrote:
> From: Tom O'Rourke <Tom.O'Rourke at intel.com>
> 
> The SLPC interface is dependent on GuC version.
> Only GuC versions known to be compatible are supported here.
> 
> SLPC with GuC firmware v9 is supported with this series.
> 
> v1: Updated with modified sanitize_slpc_option in earlier patch.
> 
> v2-v3: Rebase.
> 
> v4: Updated support for GuC firmware v9.
> 
> v5: Commit subject updated.
> 
> v6: Commit subject and message update. Add support condition as >=v9.
> 
> v7: Sanitizing GuC version in intel_uc_init_fw for SLPC
> compatibility.
>     Added info. print for needed version and pointer to 01.org.
> 
> v8: s/FIRMWARE_URL/I915_FIRMWARE_URL, Macro added for SLPC required
> GuC
>     Major version and rearrangement for sanitization. (MichalW,
> Joonas)
> 
> Signed-off-by: Tom O'Rourke <Tom.O'Rourke at intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_csr.c        | 15 ++++++---------
>  drivers/gpu/drm/i915/intel_guc_loader.c | 12 ++++++++++++
>  drivers/gpu/drm/i915/intel_uc.h         |  2 ++
>  3 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_csr.c
> b/drivers/gpu/drm/i915/intel_csr.c
> index 965988f..56c56f5 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -52,11 +52,6 @@
>  MODULE_FIRMWARE(I915_CSR_BXT);
>  #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
>  
> -#define FIRMWARE_URL  "https://01.org/linuxgraphics/downloads/firmwa
> re"
> -
> -
> -
> -
>  #define CSR_MAX_FW_SIZE			0x2FFF
>  #define CSR_DEFAULT_FW_OFFSET		0xFFFFFFFF
>  
> @@ -309,11 +304,12 @@ static uint32_t *parse_csr_fw(struct
> drm_i915_private *dev_priv,
>  
>  	if (csr->version != required_version) {
>  		DRM_INFO("Refusing to load DMC firmware v%u.%u,"
> -			 " please use v%u.%u [" FIRMWARE_URL "].\n",
> +			 " please use v%u.%u [%s].\n",
>  			 CSR_VERSION_MAJOR(csr->version),
>  			 CSR_VERSION_MINOR(csr->version),
>  			 CSR_VERSION_MAJOR(required_version),
> -			 CSR_VERSION_MINOR(required_version));
> +			 CSR_VERSION_MINOR(required_version),
> +			 I915_FIRMWARE_URL);
>  		return NULL;
>  	}
>  
> @@ -420,8 +416,9 @@ static void csr_load_work_fn(struct work_struct
> *work)
>  	} else {
>  		dev_notice(dev_priv->drm.dev,
>  			   "Failed to load DMC firmware"
> -			   " [" FIRMWARE_URL "],"
> -			   " disabling runtime power
> management.\n");
> +			   " [%s],"
> +			   " disabling runtime power management.\n",
> +			   I915_FIRMWARE_URL);
>  	}
>  
>  	release_firmware(fw);
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 8b0ae7f..8f960a8 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -64,6 +64,8 @@
>  #define GLK_FW_MAJOR 10
>  #define GLK_FW_MINOR 56
>  
> +#define I915_SLPC_REQUIRED_GUC_MAJOR 9
> +
>  #define GUC_FW_PATH(platform, major, minor) \
>         "i915/" __stringify(platform) "_guc_ver" __stringify(major)
> "_" __stringify(minor) ".bin"
>  
> @@ -415,5 +417,15 @@ int intel_guc_select_fw(struct intel_guc *guc)
>  		return -ENOENT;
>  	}
>  
> +	if (guc->fw.major_ver_wanted <
I think that you should check guc->fw.major_ver_found and not _wanted
in case we load custom GuC firmware. Because when we do load custom
firmware _wanted will be set to 0 but _found might still support SLPC.

> +			I915_SLPC_REQUIRED_GUC_MAJOR) {
> +		DRM_INFO("SLPC not supported with GuC firmware"
> +			 " v%u, please use v%u+ [%s].\n",
> +			 guc->fw.major_ver_found,
> +			 I915_SLPC_REQUIRED_GUC_MAJOR,
> +			 I915_FIRMWARE_URL);
> +		i915.enable_slpc = 0;
> +	}
> +
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_uc.h
> b/drivers/gpu/drm/i915/intel_uc.h
> index 22ae52b..9536dc77 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -32,6 +32,8 @@
>  
>  struct drm_i915_gem_request;
>  
> +#define I915_FIRMWARE_URL  "https://01.org/linuxgraphics/intel-linux
> -graphics-firmwares"
> +
>  /*
>   * This structure primarily describes the GEM object shared with the
> GuC.
>   * The specs sometimes refer to this object as a "GuC context", but
> we use