[Intel-gfx] [PATCH 2/7] drm/i915/skl: Refuse to load outdated dmc firmware

[Imre Deak]

On ti, 2015-10-27 at 14:47 +0200, Mika Kuoppala wrote:
> There is known issue on GT interrupt delivery with DC6 and
> firmwares <1.21. There is a suspicion that this causes
> spurious gpu hangs on driver init and with some workloads,
> as upgrading the firmware to 1.21 makes these problems
> disappear.
> 
> As of now the current version included in distribution
> firmware packages is very like to be 1.19. Play it safe and
> refuse to load a firmware version that may affect gpu
> side stability.
> 
> With < 1.23 there is a palette and dmc ram corruption issue
> so blacklist anything below that.
> 
> v2: Refuse to load fw instead of notifying the user
> v3: Rebase on header version changes
> v4: Refuse to load anything less than 1.23
> v5: Give enough information for user for finding correct fw (Chris)
> v6: better url and formatting (Chris)
> v7: move error log for each fail path (Mika)
>     bail out earlier in load path (Imre)
> 
> Cc: Animesh Manna <animesh.manna at intel.com>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Dave Gordon <david.s.gordon at intel.com>
> Cc: Arun Siluvery <arun.siluvery at linux.intel.com>
> Cc: Imre Deak <imre.deak at intel.com>
> Cc: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at gmail.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> References: https://01.org/linuxgraphics/downloads/skldmcver121
> References: https://01.org/linuxgraphics/downloads/skylake-dmc-1.23
> Testcase: igt/gem_exec_nop
> Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_csr.c | 37 +++++++++++++++++++++++++++----------
>  1 file changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index e620e85..701c685 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -47,6 +47,9 @@
>  MODULE_FIRMWARE(I915_CSR_SKL);
>  MODULE_FIRMWARE(I915_CSR_BXT);
>  
> +#define SKL_REQUIRED_FW_MAJOR	1
> +#define SKL_REQUIRED_FW_MINOR	23
> +
>  /*
>  * SKL CSR registers for DC5 and DC6
>  */
> @@ -303,10 +306,8 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>  	uint32_t *dmc_payload;
>  	bool fw_loaded = false;
>  
> -	if (!fw) {
> -		i915_firmware_load_error_print(csr->fw_path, 0);
> +	if (!fw)
>  		goto out;
> -	}
>  
>  	if ((stepping == -ENODATA) || (substepping == -ENODATA)) {
>  		DRM_ERROR("Unknown stepping info, firmware loading failed\n");
> @@ -324,6 +325,19 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>  
>  	csr->version = css_header->version;
>  
> +	if (IS_SKYLAKE(dev) &&
> +	    (CSR_VERSION_MAJOR(csr->version) < SKL_REQUIRED_FW_MAJOR ||
> +	     CSR_VERSION_MINOR(csr->version) < SKL_REQUIRED_FW_MINOR)) {

This would also reject 2.22 too for example, isn't that a problem?
If so, after fixing that:
Reviewed-by: Imre Deak <imre.deak at intel.com>

> +		DRM_INFO("Refusing to load old Skylake DMC firmware v%u.%u,"
> +			 " please upgrade to v%u.%u or later"
> +			 " [https://01.org/linuxgraphics/intel-linux-graphics-firmwares].\n",
> +			 CSR_VERSION_MAJOR(csr->version),
> +			 CSR_VERSION_MINOR(csr->version),
> +			 SKL_REQUIRED_FW_MAJOR,
> +			 SKL_REQUIRED_FW_MINOR);
> +		goto out;
> +	}
> +
>  	readcount += sizeof(struct intel_css_header);
>  
>  	/* Extract Package Header information*/
> @@ -405,17 +419,20 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>  	intel_csr_load_program(dev);
>  	fw_loaded = true;
>  
> -	DRM_INFO("Finished loading %s (v%u.%u)\n",
> -		 dev_priv->csr.fw_path,
> -		 CSR_VERSION_MAJOR(csr->version),
> -		 CSR_VERSION_MINOR(csr->version));
> -
>  out:
> -	if (fw_loaded)
> +	if (fw_loaded) {
>  		intel_runtime_pm_put(dev_priv);
> -	else
> +
> +		DRM_INFO("Finished loading %s (v%u.%u)\n",
> +			 dev_priv->csr.fw_path,
> +			 CSR_VERSION_MAJOR(csr->version),
> +			 CSR_VERSION_MINOR(csr->version));
> +	} else {
>  		intel_csr_load_status_set(dev_priv, FW_FAILED);
>  
> +		i915_firmware_load_error_print(csr->fw_path, 0);
> +	}
> +
>  	release_firmware(fw);
>  }
>