| Message ID | 1490177034-6138-6-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 22, 2017 at 03:33:38PM +0530, Sagar Arun Kamble wrote: > From: Tom O'Rourke <Tom.O'Rourke@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. > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > --- > drivers/gpu/drm/i915/intel_csr.c | 5 ----- > drivers/gpu/drm/i915/intel_uc.c | 14 ++++++++++++-- > drivers/gpu/drm/i915/intel_uc.h | 2 ++ > 3 files changed, 14 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > index 3683225..b784b7b 100644 > --- a/drivers/gpu/drm/i915/intel_csr.c > +++ b/drivers/gpu/drm/i915/intel_csr.c > @@ -49,11 +49,6 @@ > MODULE_FIRMWARE(I915_CSR_BXT); > #define BXT_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) > > -#define FIRMWARE_URL "https://01.org/linuxgraphics/intel-linux-graphics-firmwares" > - > - > - > - > #define CSR_MAX_FW_SIZE 0x2FFF > #define CSR_DEFAULT_FW_OFFSET 0xFFFFFFFF > > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c > index c0a1d14..8502ead 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -101,11 +101,21 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv) > > void intel_uc_init_fw(struct drm_i915_private *dev_priv) > { > + struct intel_uc_fw *guc_fw = &dev_priv->guc.fw; > + > if (dev_priv->huc.fw.path) > intel_uc_prepare_fw(dev_priv, &dev_priv->huc.fw); > > - if (dev_priv->guc.fw.path) > - intel_uc_prepare_fw(dev_priv, &dev_priv->guc.fw); > + if (guc_fw->path) { > + intel_uc_prepare_fw(dev_priv, guc_fw); > + > + if (guc_fw->major_ver_found < 9) { In intel_uc_prepare_fw() there is explicit check that 'found' major version match the 'wanted' major version, thus I think you can do this slpc check in intel_uc_sanitize_options() right after sucessful call to intel_guc_select_fw() > + DRM_INFO("SLPC not supported with GuC firmware v%u," > + " please use v9+ [" FIRMWARE_URL "].\n", Hmm, as "9" is used here for the second time, maybe there shall be a definition #define I915_SLPC_REQUIRED_GUC_MAJOR 9 Also, if FIRMWARE_URL is used more than once in the driver, maybe to save some driver footprint it would be better to use %s ? > + guc_fw->major_ver_found); > + i915.enable_slpc = 0; > + } > + } > } > > int intel_uc_init_hw(struct drm_i915_private *dev_priv) > diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h > index a35eded..928c214 100644 > --- a/drivers/gpu/drm/i915/intel_uc.h > +++ b/drivers/gpu/drm/i915/intel_uc.h > @@ -130,6 +130,8 @@ struct intel_uc_fw { > uint32_t ucode_offset; > }; > > +#define FIRMWARE_URL "https://01.org/linuxgraphics/intel-linux-graphics-firmwares" Is it ok that we use such generic macro name without any intel/i915 prefix? Regards, -Michal
On ke, 2017-03-22 at 16:18 +0100, Michal Wajdeczko wrote: > On Wed, Mar 22, 2017 at 03:33:38PM +0530, Sagar Arun Kamble wrote: > > @@ -130,6 +130,8 @@ struct intel_uc_fw { > > uint32_t ucode_offset; > > }; > > > > +#define FIRMWARE_URL "https://01.org/linuxgraphics/intel-linux-gr > > aphics-firmwares" > > Is it ok that we use such generic macro name without any intel/i915 > prefix? Definitely not, this is result of moving from .c to .h. Need to add I915_ prefix. Regards, Joonas
Thanks Joonas, Michal. Will update this patch. On 3/22/2017 9:00 PM, Joonas Lahtinen wrote: > On ke, 2017-03-22 at 16:18 +0100, Michal Wajdeczko wrote: >> On Wed, Mar 22, 2017 at 03:33:38PM +0530, Sagar Arun Kamble wrote: >>> @@ -130,6 +130,8 @@ struct intel_uc_fw { >>> uint32_t ucode_offset; >>> }; >>> >>> +#define FIRMWARE_URL "https://01.org/linuxgraphics/intel-linux-gr >>> aphics-firmwares" >> Is it ok that we use such generic macro name without any intel/i915 >> prefix? > Definitely not, this is result of moving from .c to .h. Need to add > I915_ prefix. > > Regards, Joonas
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index 3683225..b784b7b 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -49,11 +49,6 @@ MODULE_FIRMWARE(I915_CSR_BXT); #define BXT_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) -#define FIRMWARE_URL "https://01.org/linuxgraphics/intel-linux-graphics-firmwares" - - - - #define CSR_MAX_FW_SIZE 0x2FFF #define CSR_DEFAULT_FW_OFFSET 0xFFFFFFFF diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index c0a1d14..8502ead 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -101,11 +101,21 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv) void intel_uc_init_fw(struct drm_i915_private *dev_priv) { + struct intel_uc_fw *guc_fw = &dev_priv->guc.fw; + if (dev_priv->huc.fw.path) intel_uc_prepare_fw(dev_priv, &dev_priv->huc.fw); - if (dev_priv->guc.fw.path) - intel_uc_prepare_fw(dev_priv, &dev_priv->guc.fw); + if (guc_fw->path) { + intel_uc_prepare_fw(dev_priv, guc_fw); + + if (guc_fw->major_ver_found < 9) { + DRM_INFO("SLPC not supported with GuC firmware v%u," + " please use v9+ [" FIRMWARE_URL "].\n", + guc_fw->major_ver_found); + i915.enable_slpc = 0; + } + } } int intel_uc_init_hw(struct drm_i915_private *dev_priv) diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index a35eded..928c214 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -130,6 +130,8 @@ struct intel_uc_fw { uint32_t ucode_offset; }; +#define FIRMWARE_URL "https://01.org/linuxgraphics/intel-linux-graphics-firmwares" + struct intel_guc_log { uint32_t flags; struct i915_vma *vma;