| Message ID | 1462881556-17972-1-git-send-email-patrik.jakobsson@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes: > [ text/plain ] > Load specific firmware versions for the DMC instead of using symbolic > links. The currently recommended versions are: SKL 1.26, KBL 1.01 and > BXT 1.07. > We should augment the commit message to answer the 'why' part. Otherwise, looks good. -Mika > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++--------------- > 1 file changed, 14 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > index 2b3b428..ea047cd 100644 > --- a/drivers/gpu/drm/i915/intel_csr.c > +++ b/drivers/gpu/drm/i915/intel_csr.c > @@ -41,15 +41,15 @@ > * be moved to FW_FAILED. > */ > > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin" > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin" > MODULE_FIRMWARE(I915_CSR_KBL); > #define KBL_CSR_VERSION_REQUIRED CSR_VERSION(1, 1) > > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin" > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin" > MODULE_FIRMWARE(I915_CSR_SKL); > -#define SKL_CSR_VERSION_REQUIRED CSR_VERSION(1, 23) > +#define SKL_CSR_VERSION_REQUIRED CSR_VERSION(1, 26) > > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin" > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin" > MODULE_FIRMWARE(I915_CSR_BXT); > #define BXT_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) > > @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, > uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes; > uint32_t i; > uint32_t *dmc_payload; > - uint32_t required_min_version; > + uint32_t required_version; > > if (!fw) > return NULL; > @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, > csr->version = css_header->version; > > if (IS_KABYLAKE(dev_priv)) { > - required_min_version = KBL_CSR_VERSION_REQUIRED; > + required_version = KBL_CSR_VERSION_REQUIRED; > } else if (IS_SKYLAKE(dev_priv)) { > - required_min_version = SKL_CSR_VERSION_REQUIRED; > + required_version = SKL_CSR_VERSION_REQUIRED; > } else if (IS_BROXTON(dev_priv)) { > - required_min_version = BXT_CSR_VERSION_REQUIRED; > + required_version = BXT_CSR_VERSION_REQUIRED; > } else { > MISSING_CASE(INTEL_REVID(dev_priv)); > - required_min_version = 0; > + required_version = 0; > } > > - if (csr->version < required_min_version) { > - DRM_INFO("Refusing to load old DMC firmware v%u.%u," > - " please upgrade to v%u.%u or later" > - " [" FIRMWARE_URL "].\n", > + if (csr->version != required_version) { > + DRM_INFO("Refusing to load DMC firmware v%u.%u," > + " please use v%u.%u [" FIRMWARE_URL "].\n", > CSR_VERSION_MAJOR(csr->version), > CSR_VERSION_MINOR(csr->version), > - CSR_VERSION_MAJOR(required_min_version), > - CSR_VERSION_MINOR(required_min_version)); > + CSR_VERSION_MAJOR(required_version), > + CSR_VERSION_MINOR(required_version)); > return NULL; > } > > -- > 2.5.0
On Tue, May 10, 2016 at 03:52:02PM +0300, Mika Kuoppala wrote: > Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes: > > > [ text/plain ] > > Load specific firmware versions for the DMC instead of using symbolic > > links. The currently recommended versions are: SKL 1.26, KBL 1.01 and > > BXT 1.07. > > > > We should augment the commit message to answer the 'why' part. > > Otherwise, looks good. Yes I agree. Where did this discussion take place? I don't believe I was part of it. Rodrigo told me we've reached consensus on the decision but that's all I know. -Patrik > > -Mika > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Cc: Imre Deak <imre.deak@intel.com> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++--------------- > > 1 file changed, 14 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > > index 2b3b428..ea047cd 100644 > > --- a/drivers/gpu/drm/i915/intel_csr.c > > +++ b/drivers/gpu/drm/i915/intel_csr.c > > @@ -41,15 +41,15 @@ > > * be moved to FW_FAILED. > > */ > > > > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin" > > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin" > > MODULE_FIRMWARE(I915_CSR_KBL); > > #define KBL_CSR_VERSION_REQUIRED CSR_VERSION(1, 1) > > > > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin" > > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin" > > MODULE_FIRMWARE(I915_CSR_SKL); > > -#define SKL_CSR_VERSION_REQUIRED CSR_VERSION(1, 23) > > +#define SKL_CSR_VERSION_REQUIRED CSR_VERSION(1, 26) > > > > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin" > > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin" > > MODULE_FIRMWARE(I915_CSR_BXT); > > #define BXT_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) > > > > @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, > > uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes; > > uint32_t i; > > uint32_t *dmc_payload; > > - uint32_t required_min_version; > > + uint32_t required_version; > > > > if (!fw) > > return NULL; > > @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, > > csr->version = css_header->version; > > > > if (IS_KABYLAKE(dev_priv)) { > > - required_min_version = KBL_CSR_VERSION_REQUIRED; > > + required_version = KBL_CSR_VERSION_REQUIRED; > > } else if (IS_SKYLAKE(dev_priv)) { > > - required_min_version = SKL_CSR_VERSION_REQUIRED; > > + required_version = SKL_CSR_VERSION_REQUIRED; > > } else if (IS_BROXTON(dev_priv)) { > > - required_min_version = BXT_CSR_VERSION_REQUIRED; > > + required_version = BXT_CSR_VERSION_REQUIRED; > > } else { > > MISSING_CASE(INTEL_REVID(dev_priv)); > > - required_min_version = 0; > > + required_version = 0; > > } > > > > - if (csr->version < required_min_version) { > > - DRM_INFO("Refusing to load old DMC firmware v%u.%u," > > - " please upgrade to v%u.%u or later" > > - " [" FIRMWARE_URL "].\n", > > + if (csr->version != required_version) { > > + DRM_INFO("Refusing to load DMC firmware v%u.%u," > > + " please use v%u.%u [" FIRMWARE_URL "].\n", > > CSR_VERSION_MAJOR(csr->version), > > CSR_VERSION_MINOR(csr->version), > > - CSR_VERSION_MAJOR(required_min_version), > > - CSR_VERSION_MINOR(required_min_version)); > > + CSR_VERSION_MAJOR(required_version), > > + CSR_VERSION_MINOR(required_version)); > > return NULL; > > } > > > > -- > > 2.5.0
On Tue, 10 May 2016, Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote: > On Tue, May 10, 2016 at 03:52:02PM +0300, Mika Kuoppala wrote: >> Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes: >> >> > [ text/plain ] >> > Load specific firmware versions for the DMC instead of using symbolic >> > links. The currently recommended versions are: SKL 1.26, KBL 1.01 and >> > BXT 1.07. >> > >> >> We should augment the commit message to answer the 'why' part. >> >> Otherwise, looks good. > > Yes I agree. Where did this discussion take place? I don't believe I was > part of it. Rodrigo told me we've reached consensus on the decision but > that's all I know. This captures most of it I think http://mid.gmane.org/87fuu6i0zb.fsf@intel.com BR, Jani. > > -Patrik > >> >> -Mika >> >> >> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >> > Cc: Imre Deak <imre.deak@intel.com> >> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> >> > --- >> > drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++--------------- >> > 1 file changed, 14 insertions(+), 15 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c >> > index 2b3b428..ea047cd 100644 >> > --- a/drivers/gpu/drm/i915/intel_csr.c >> > +++ b/drivers/gpu/drm/i915/intel_csr.c >> > @@ -41,15 +41,15 @@ >> > * be moved to FW_FAILED. >> > */ >> > >> > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin" >> > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin" >> > MODULE_FIRMWARE(I915_CSR_KBL); >> > #define KBL_CSR_VERSION_REQUIRED CSR_VERSION(1, 1) >> > >> > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin" >> > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin" >> > MODULE_FIRMWARE(I915_CSR_SKL); >> > -#define SKL_CSR_VERSION_REQUIRED CSR_VERSION(1, 23) >> > +#define SKL_CSR_VERSION_REQUIRED CSR_VERSION(1, 26) >> > >> > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin" >> > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin" >> > MODULE_FIRMWARE(I915_CSR_BXT); >> > #define BXT_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) >> > >> > @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, >> > uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes; >> > uint32_t i; >> > uint32_t *dmc_payload; >> > - uint32_t required_min_version; >> > + uint32_t required_version; >> > >> > if (!fw) >> > return NULL; >> > @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, >> > csr->version = css_header->version; >> > >> > if (IS_KABYLAKE(dev_priv)) { >> > - required_min_version = KBL_CSR_VERSION_REQUIRED; >> > + required_version = KBL_CSR_VERSION_REQUIRED; >> > } else if (IS_SKYLAKE(dev_priv)) { >> > - required_min_version = SKL_CSR_VERSION_REQUIRED; >> > + required_version = SKL_CSR_VERSION_REQUIRED; >> > } else if (IS_BROXTON(dev_priv)) { >> > - required_min_version = BXT_CSR_VERSION_REQUIRED; >> > + required_version = BXT_CSR_VERSION_REQUIRED; >> > } else { >> > MISSING_CASE(INTEL_REVID(dev_priv)); >> > - required_min_version = 0; >> > + required_version = 0; >> > } >> > >> > - if (csr->version < required_min_version) { >> > - DRM_INFO("Refusing to load old DMC firmware v%u.%u," >> > - " please upgrade to v%u.%u or later" >> > - " [" FIRMWARE_URL "].\n", >> > + if (csr->version != required_version) { >> > + DRM_INFO("Refusing to load DMC firmware v%u.%u," >> > + " please use v%u.%u [" FIRMWARE_URL "].\n", >> > CSR_VERSION_MAJOR(csr->version), >> > CSR_VERSION_MINOR(csr->version), >> > - CSR_VERSION_MAJOR(required_min_version), >> > - CSR_VERSION_MINOR(required_min_version)); >> > + CSR_VERSION_MAJOR(required_version), >> > + CSR_VERSION_MINOR(required_version)); >> > return NULL; >> > } >> > >> > -- >> > 2.5.0
Jani Nikula <jani.nikula@linux.intel.com> writes: > [ text/plain ] > On Tue, 10 May 2016, Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote: >> On Tue, May 10, 2016 at 03:52:02PM +0300, Mika Kuoppala wrote: >>> Patrik Jakobsson <patrik.jakobsson@linux.intel.com> writes: >>> >>> > [ text/plain ] >>> > Load specific firmware versions for the DMC instead of using symbolic >>> > links. The currently recommended versions are: SKL 1.26, KBL 1.01 and >>> > BXT 1.07. >>> > >>> >>> We should augment the commit message to answer the 'why' part. >>> >>> Otherwise, looks good. >> >> Yes I agree. Where did this discussion take place? I don't believe I was >> part of it. Rodrigo told me we've reached consensus on the decision but >> that's all I know. > > This captures most of it I think > http://mid.gmane.org/87fuu6i0zb.fsf@intel.com > Pretty much. In essence we want to offer a combination with somekind of proof (read tested) that it works. And lock that combination. Think about it as a colored seal paint on screw heads on some products: these were tested and delivered together. You can unscrew and dissassemble the parts still with minimal effort and mix the pieces, but they may or may not work as planned if you do so. And if you broke the seal paint and get a black screen, you know who to blame. -Mika > > BR, > Jani. > > >> >> -Patrik >> >>> >>> -Mika >>> >>> >>> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >>> > Cc: Imre Deak <imre.deak@intel.com> >>> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> >>> > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> >>> > --- >>> > drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++--------------- >>> > 1 file changed, 14 insertions(+), 15 deletions(-) >>> > >>> > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c >>> > index 2b3b428..ea047cd 100644 >>> > --- a/drivers/gpu/drm/i915/intel_csr.c >>> > +++ b/drivers/gpu/drm/i915/intel_csr.c >>> > @@ -41,15 +41,15 @@ >>> > * be moved to FW_FAILED. >>> > */ >>> > >>> > -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin" >>> > +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin" >>> > MODULE_FIRMWARE(I915_CSR_KBL); >>> > #define KBL_CSR_VERSION_REQUIRED CSR_VERSION(1, 1) >>> > >>> > -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin" >>> > +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin" >>> > MODULE_FIRMWARE(I915_CSR_SKL); >>> > -#define SKL_CSR_VERSION_REQUIRED CSR_VERSION(1, 23) >>> > +#define SKL_CSR_VERSION_REQUIRED CSR_VERSION(1, 26) >>> > >>> > -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin" >>> > +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin" >>> > MODULE_FIRMWARE(I915_CSR_BXT); >>> > #define BXT_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) >>> > >>> > @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, >>> > uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes; >>> > uint32_t i; >>> > uint32_t *dmc_payload; >>> > - uint32_t required_min_version; >>> > + uint32_t required_version; >>> > >>> > if (!fw) >>> > return NULL; >>> > @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, >>> > csr->version = css_header->version; >>> > >>> > if (IS_KABYLAKE(dev_priv)) { >>> > - required_min_version = KBL_CSR_VERSION_REQUIRED; >>> > + required_version = KBL_CSR_VERSION_REQUIRED; >>> > } else if (IS_SKYLAKE(dev_priv)) { >>> > - required_min_version = SKL_CSR_VERSION_REQUIRED; >>> > + required_version = SKL_CSR_VERSION_REQUIRED; >>> > } else if (IS_BROXTON(dev_priv)) { >>> > - required_min_version = BXT_CSR_VERSION_REQUIRED; >>> > + required_version = BXT_CSR_VERSION_REQUIRED; >>> > } else { >>> > MISSING_CASE(INTEL_REVID(dev_priv)); >>> > - required_min_version = 0; >>> > + required_version = 0; >>> > } >>> > >>> > - if (csr->version < required_min_version) { >>> > - DRM_INFO("Refusing to load old DMC firmware v%u.%u," >>> > - " please upgrade to v%u.%u or later" >>> > - " [" FIRMWARE_URL "].\n", >>> > + if (csr->version != required_version) { >>> > + DRM_INFO("Refusing to load DMC firmware v%u.%u," >>> > + " please use v%u.%u [" FIRMWARE_URL "].\n", >>> > CSR_VERSION_MAJOR(csr->version), >>> > CSR_VERSION_MINOR(csr->version), >>> > - CSR_VERSION_MAJOR(required_min_version), >>> > - CSR_VERSION_MINOR(required_min_version)); >>> > + CSR_VERSION_MAJOR(required_version), >>> > + CSR_VERSION_MINOR(required_version)); >>> > return NULL; >>> > } >>> > >>> > -- >>> > 2.5.0 > > -- > Jani Nikula, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index 2b3b428..ea047cd 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -41,15 +41,15 @@ * be moved to FW_FAILED. */ -#define I915_CSR_KBL "i915/kbl_dmc_ver1.bin" +#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin" MODULE_FIRMWARE(I915_CSR_KBL); #define KBL_CSR_VERSION_REQUIRED CSR_VERSION(1, 1) -#define I915_CSR_SKL "i915/skl_dmc_ver1.bin" +#define I915_CSR_SKL "i915/skl_dmc_ver1_26.bin" MODULE_FIRMWARE(I915_CSR_SKL); -#define SKL_CSR_VERSION_REQUIRED CSR_VERSION(1, 23) +#define SKL_CSR_VERSION_REQUIRED CSR_VERSION(1, 26) -#define I915_CSR_BXT "i915/bxt_dmc_ver1.bin" +#define I915_CSR_BXT "i915/bxt_dmc_ver1_07.bin" MODULE_FIRMWARE(I915_CSR_BXT); #define BXT_CSR_VERSION_REQUIRED CSR_VERSION(1, 7) @@ -286,7 +286,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes; uint32_t i; uint32_t *dmc_payload; - uint32_t required_min_version; + uint32_t required_version; if (!fw) return NULL; @@ -303,24 +303,23 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, csr->version = css_header->version; if (IS_KABYLAKE(dev_priv)) { - required_min_version = KBL_CSR_VERSION_REQUIRED; + required_version = KBL_CSR_VERSION_REQUIRED; } else if (IS_SKYLAKE(dev_priv)) { - required_min_version = SKL_CSR_VERSION_REQUIRED; + required_version = SKL_CSR_VERSION_REQUIRED; } else if (IS_BROXTON(dev_priv)) { - required_min_version = BXT_CSR_VERSION_REQUIRED; + required_version = BXT_CSR_VERSION_REQUIRED; } else { MISSING_CASE(INTEL_REVID(dev_priv)); - required_min_version = 0; + required_version = 0; } - if (csr->version < required_min_version) { - DRM_INFO("Refusing to load old DMC firmware v%u.%u," - " please upgrade to v%u.%u or later" - " [" FIRMWARE_URL "].\n", + if (csr->version != required_version) { + DRM_INFO("Refusing to load DMC firmware v%u.%u," + " please use v%u.%u [" FIRMWARE_URL "].\n", CSR_VERSION_MAJOR(csr->version), CSR_VERSION_MINOR(csr->version), - CSR_VERSION_MAJOR(required_min_version), - CSR_VERSION_MINOR(required_min_version)); + CSR_VERSION_MAJOR(required_version), + CSR_VERSION_MINOR(required_version)); return NULL; }
Load specific firmware versions for the DMC instead of using symbolic links. The currently recommended versions are: SKL 1.26, KBL 1.01 and BXT 1.07. Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Imre Deak <imre.deak@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> --- drivers/gpu/drm/i915/intel_csr.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-)