[05/30] drm/i915/slpc: Sanitize GuC version
Kamble, Sagar A
sagar.a.kamble at intel.com
Tue Aug 29 07:41:29 UTC 2017
Agree with your suggestion.
Will move the SLPC option sanitization past fetching of the firmware.
Thanks
Sagar
-----Original Message-----
From: Fiedorowicz, Lukasz
Sent: Monday, August 28, 2017 5:16 PM
To: Intel-gfx-trybot at lists.freedesktop.org; Kamble, Sagar A <sagar.a.kamble at intel.com>
Subject: Re: [05/30] drm/i915/slpc: Sanitize GuC version
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
More information about the Intel-gfx-trybot
mailing list