[Intel-gfx] [PATCH 15/31] drm/i915/slpc: Sanitize GuC version
Michal Wajdeczko
michal.wajdeczko at intel.com
Thu Sep 21 12:52:04 UTC 2017
On Tue, 19 Sep 2017 19:41:51 +0200, Sagar Arun Kamble
<sagar.a.kamble at intel.com> 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)
>
> v9: Checking major_ver_found to sanitize SLPC option enable_slpc post
> fetching the firmware as with Custom firmware loaded through
> guc_firmware_path parameter, major_ver_wanted are cleared. (Lukasz)
>
> v10: Moved the I915_FIRMWARE_URL macro to intel_uc_common.h.
>
> 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.h | 1 +
> drivers/gpu/drm/i915/intel_guc_loader.c | 15 +++++++++++++++
> drivers/gpu/drm/i915/intel_uc.c | 1 +
> drivers/gpu/drm/i915/intel_uc_common.h | 2 ++
> 5 files changed, 25 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/firmware"
> -
> -
> -
> -
> #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);
Hmm, I'm not sure that including URL here is useful.
URL will be repeated in csr_load_work_fn() where we can explain
its purpose ;)
> 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);
Maybe more user friendly message should looks like:
"Failed to load DMC firmware, disabling runtime power management."
"DMC firmware can be downloaded from
https://01.org/linuxgraphics/downloads/firmware"
> }
> release_firmware(fw);
> diff --git a/drivers/gpu/drm/i915/intel_guc.h
> b/drivers/gpu/drm/i915/intel_guc.h
> index a894991..3821bf2 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -159,6 +159,7 @@ static inline void intel_guc_init_early(struct
> intel_guc *guc)
> int intel_guc_select_fw(struct intel_guc *guc);
> int intel_guc_init_hw(struct intel_guc *guc);
> u32 intel_guc_wopcm_size(struct intel_guc *guc);
> +void intel_guc_fetch_sanitize_options(struct intel_guc *guc);
> /* i915_guc_submission.c */
> int i915_guc_submission_init(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 6ee7c16..4550620 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"
> @@ -418,3 +420,16 @@ int intel_guc_select_fw(struct intel_guc *guc)
> return 0;
> }
> +
> +void intel_guc_fetch_sanitize_options(struct intel_guc *guc)
> +{
> + if (guc->fw.major_ver_found <
> + I915_SLPC_REQUIRED_GUC_MAJOR) {
Generally we do not allow Guc fw major "found" to be different than
"wanted"
so this check could be also done against "wanted".
> + 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);
Not sure that URL in this message is helpful.
> + i915.enable_slpc = 0;
> + }
> +}
> diff --git a/drivers/gpu/drm/i915/intel_uc.c
> b/drivers/gpu/drm/i915/intel_uc.c
> index eeec986..350027f 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -194,6 +194,7 @@ static void fetch_uc_fw(struct drm_i915_private
> *dev_priv,
> }
> uc_fw->major_ver_found = css->guc.sw_version >> 16;
> uc_fw->minor_ver_found = css->guc.sw_version & 0xFFFF;
> + intel_guc_fetch_sanitize_options(&dev_priv->guc);
This should be done as part of intel_uc_sanitize_options()
> break;
> case INTEL_UC_FW_TYPE_HUC:
> diff --git a/drivers/gpu/drm/i915/intel_uc_common.h
> b/drivers/gpu/drm/i915/intel_uc_common.h
> index 3de6823..4726511 100644
> --- a/drivers/gpu/drm/i915/intel_uc_common.h
> +++ b/drivers/gpu/drm/i915/intel_uc_common.h
> @@ -27,6 +27,8 @@
> #include "intel_ringbuffer.h"
> #include "i915_vma.h"
Add separation line
> +#define I915_FIRMWARE_URL
> "https://01.org/linuxgraphics/intel-linux-graphics-firmwares"
> +
> enum intel_uc_fw_status {
> INTEL_UC_FIRMWARE_FAIL = -1,
> INTEL_UC_FIRMWARE_NONE = 0,
Michal
More information about the Intel-gfx
mailing list