diff mbox series

[v3,1/2] drm/i915/dmc: Do not require specific versions

Message ID 20221229190740.45471-2-gustavo.sousa@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/dmc: Make firmware loading backwards-compatible | expand

Commit Message

Gustavo Sousa Dec. 29, 2022, 7:07 p.m. UTC
Currently there is no DMC version requirement with respect to how i915
interacts with it and new versions of the firmware serve as drop-in
replacements of older ones. As such, do not require specific versions.

References: https://lore.kernel.org/r/Y3081s7c5ksutpMW@intel.com
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dmc.c | 35 ------------------------
 drivers/gpu/drm/i915/display/intel_dmc.h |  1 -
 2 files changed, 36 deletions(-)

Comments

Rodrigo Vivi Dec. 30, 2022, 8:32 a.m. UTC | #1
On Thu, Dec 29, 2022 at 04:07:39PM -0300, Gustavo Sousa wrote:
> Currently there is no DMC version requirement with respect to how i915
> interacts with it and new versions of the firmware serve as drop-in
> replacements of older ones. As such, do not require specific versions.
> 
> References: https://lore.kernel.org/r/Y3081s7c5ksutpMW@intel.com

I don't believe this link is a good reference as justification
of this patch.

Probably https://docs.kernel.org/next/driver-api/firmware/firmware-usage-guidelines.html
is a better link?

Also, in the commit message we should be more clear that i915
interacts with the Hardware and not with any FW ABI/API, so the API
is fixed within the platform, hence no need to get this so-tied
version requirement.

with the commit msg changed you can count on my reviewed-by,
the patch looks good to me.

> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dmc.c | 35 ------------------------
>  drivers/gpu/drm/i915/display/intel_dmc.h |  1 -
>  2 files changed, 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
> index 905b5dcdca14..4124b3d37110 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> @@ -53,51 +53,40 @@
>  #define DISPLAY_VER12_DMC_MAX_FW_SIZE	ICL_DMC_MAX_FW_SIZE
>  
>  #define DG2_DMC_PATH			DMC_PATH(dg2, 2, 08)
> -#define DG2_DMC_VERSION_REQUIRED	DMC_VERSION(2, 8)
>  MODULE_FIRMWARE(DG2_DMC_PATH);
>  
>  #define ADLP_DMC_PATH			DMC_PATH(adlp, 2, 16)
> -#define ADLP_DMC_VERSION_REQUIRED	DMC_VERSION(2, 16)
>  MODULE_FIRMWARE(ADLP_DMC_PATH);
>  
>  #define ADLS_DMC_PATH			DMC_PATH(adls, 2, 01)
> -#define ADLS_DMC_VERSION_REQUIRED	DMC_VERSION(2, 1)
>  MODULE_FIRMWARE(ADLS_DMC_PATH);
>  
>  #define DG1_DMC_PATH			DMC_PATH(dg1, 2, 02)
> -#define DG1_DMC_VERSION_REQUIRED	DMC_VERSION(2, 2)
>  MODULE_FIRMWARE(DG1_DMC_PATH);
>  
>  #define RKL_DMC_PATH			DMC_PATH(rkl, 2, 03)
> -#define RKL_DMC_VERSION_REQUIRED	DMC_VERSION(2, 3)
>  MODULE_FIRMWARE(RKL_DMC_PATH);
>  
>  #define TGL_DMC_PATH			DMC_PATH(tgl, 2, 12)
> -#define TGL_DMC_VERSION_REQUIRED	DMC_VERSION(2, 12)
>  MODULE_FIRMWARE(TGL_DMC_PATH);
>  
>  #define ICL_DMC_PATH			DMC_PATH(icl, 1, 09)
> -#define ICL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 9)
>  #define ICL_DMC_MAX_FW_SIZE		0x6000
>  MODULE_FIRMWARE(ICL_DMC_PATH);
>  
>  #define GLK_DMC_PATH			DMC_PATH(glk, 1, 04)
> -#define GLK_DMC_VERSION_REQUIRED	DMC_VERSION(1, 4)
>  #define GLK_DMC_MAX_FW_SIZE		0x4000
>  MODULE_FIRMWARE(GLK_DMC_PATH);
>  
>  #define KBL_DMC_PATH			DMC_PATH(kbl, 1, 04)
> -#define KBL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 4)
>  #define KBL_DMC_MAX_FW_SIZE		BXT_DMC_MAX_FW_SIZE
>  MODULE_FIRMWARE(KBL_DMC_PATH);
>  
>  #define SKL_DMC_PATH			DMC_PATH(skl, 1, 27)
> -#define SKL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 27)
>  #define SKL_DMC_MAX_FW_SIZE		BXT_DMC_MAX_FW_SIZE
>  MODULE_FIRMWARE(SKL_DMC_PATH);
>  
>  #define BXT_DMC_PATH			DMC_PATH(bxt, 1, 07)
> -#define BXT_DMC_VERSION_REQUIRED	DMC_VERSION(1, 7)
>  #define BXT_DMC_MAX_FW_SIZE		0x3000
>  MODULE_FIRMWARE(BXT_DMC_PATH);
>  
> @@ -765,17 +754,6 @@ static u32 parse_dmc_fw_css(struct intel_dmc *dmc,
>  		return 0;
>  	}
>  
> -	if (dmc->required_version &&
> -	    css_header->version != dmc->required_version) {
> -		drm_info(&i915->drm, "Refusing to load DMC firmware v%u.%u,"
> -			 " please use v%u.%u\n",
> -			 DMC_VERSION_MAJOR(css_header->version),
> -			 DMC_VERSION_MINOR(css_header->version),
> -			 DMC_VERSION_MAJOR(dmc->required_version),
> -			 DMC_VERSION_MINOR(dmc->required_version));
> -		return 0;
> -	}
> -
>  	dmc->version = css_header->version;
>  
>  	return sizeof(struct intel_css_header);
> @@ -903,49 +881,38 @@ void intel_dmc_ucode_init(struct drm_i915_private *dev_priv)
>  
>  	if (IS_DG2(dev_priv)) {
>  		dmc->fw_path = DG2_DMC_PATH;
> -		dmc->required_version = DG2_DMC_VERSION_REQUIRED;
>  		dmc->max_fw_size = DISPLAY_VER13_DMC_MAX_FW_SIZE;
>  	} else if (IS_ALDERLAKE_P(dev_priv)) {
>  		dmc->fw_path = ADLP_DMC_PATH;
> -		dmc->required_version = ADLP_DMC_VERSION_REQUIRED;
>  		dmc->max_fw_size = DISPLAY_VER13_DMC_MAX_FW_SIZE;
>  	} else if (IS_ALDERLAKE_S(dev_priv)) {
>  		dmc->fw_path = ADLS_DMC_PATH;
> -		dmc->required_version = ADLS_DMC_VERSION_REQUIRED;
>  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
>  	} else if (IS_DG1(dev_priv)) {
>  		dmc->fw_path = DG1_DMC_PATH;
> -		dmc->required_version = DG1_DMC_VERSION_REQUIRED;
>  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
>  	} else if (IS_ROCKETLAKE(dev_priv)) {
>  		dmc->fw_path = RKL_DMC_PATH;
> -		dmc->required_version = RKL_DMC_VERSION_REQUIRED;
>  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
>  	} else if (IS_TIGERLAKE(dev_priv)) {
>  		dmc->fw_path = TGL_DMC_PATH;
> -		dmc->required_version = TGL_DMC_VERSION_REQUIRED;
>  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
>  	} else if (DISPLAY_VER(dev_priv) == 11) {
>  		dmc->fw_path = ICL_DMC_PATH;
> -		dmc->required_version = ICL_DMC_VERSION_REQUIRED;
>  		dmc->max_fw_size = ICL_DMC_MAX_FW_SIZE;
>  	} else if (IS_GEMINILAKE(dev_priv)) {
>  		dmc->fw_path = GLK_DMC_PATH;
> -		dmc->required_version = GLK_DMC_VERSION_REQUIRED;
>  		dmc->max_fw_size = GLK_DMC_MAX_FW_SIZE;
>  	} else if (IS_KABYLAKE(dev_priv) ||
>  		   IS_COFFEELAKE(dev_priv) ||
>  		   IS_COMETLAKE(dev_priv)) {
>  		dmc->fw_path = KBL_DMC_PATH;
> -		dmc->required_version = KBL_DMC_VERSION_REQUIRED;
>  		dmc->max_fw_size = KBL_DMC_MAX_FW_SIZE;
>  	} else if (IS_SKYLAKE(dev_priv)) {
>  		dmc->fw_path = SKL_DMC_PATH;
> -		dmc->required_version = SKL_DMC_VERSION_REQUIRED;
>  		dmc->max_fw_size = SKL_DMC_MAX_FW_SIZE;
>  	} else if (IS_BROXTON(dev_priv)) {
>  		dmc->fw_path = BXT_DMC_PATH;
> -		dmc->required_version = BXT_DMC_VERSION_REQUIRED;
>  		dmc->max_fw_size = BXT_DMC_MAX_FW_SIZE;
>  	}
>  
> @@ -958,8 +925,6 @@ void intel_dmc_ucode_init(struct drm_i915_private *dev_priv)
>  		}
>  
>  		dmc->fw_path = dev_priv->params.dmc_firmware_path;
> -		/* Bypass version check for firmware override. */
> -		dmc->required_version = 0;
>  	}
>  
>  	if (!dmc->fw_path) {
> diff --git a/drivers/gpu/drm/i915/display/intel_dmc.h b/drivers/gpu/drm/i915/display/intel_dmc.h
> index 67e03315ef99..435eab9b016b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc.h
> +++ b/drivers/gpu/drm/i915/display/intel_dmc.h
> @@ -25,7 +25,6 @@ enum {
>  struct intel_dmc {
>  	struct work_struct work;
>  	const char *fw_path;
> -	u32 required_version;
>  	u32 max_fw_size; /* bytes */
>  	u32 version;
>  	struct dmc_fw_info {
> -- 
> 2.39.0
>
Gustavo Sousa Dec. 30, 2022, 12:42 p.m. UTC | #2
On Fri, Dec 30, 2022 at 03:32:41AM -0500, Rodrigo Vivi wrote:
> On Thu, Dec 29, 2022 at 04:07:39PM -0300, Gustavo Sousa wrote:
> > Currently there is no DMC version requirement with respect to how i915
> > interacts with it and new versions of the firmware serve as drop-in
> > replacements of older ones. As such, do not require specific versions.
> > 
> > References: https://lore.kernel.org/r/Y3081s7c5ksutpMW@intel.com
> 
> I don't believe this link is a good reference as justification
> of this patch.
> 
> Probably https://docs.kernel.org/next/driver-api/firmware/firmware-usage-guidelines.html
> is a better link?

Yep. I agree this would be better. Is there an "archive" version of this page?
Just to make sure we link to the exact version of that guide at the time of
writing this patch.

> 
> Also, in the commit message we should be more clear that i915
> interacts with the Hardware and not with any FW ABI/API, so the API
> is fixed within the platform, hence no need to get this so-tied
> version requirement.

Thanks! I'll grab this paragraph and adapt it into the commit message if you
allow me =)

> 
> with the commit msg changed you can count on my reviewed-by,
> the patch looks good to me.
> 
> > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dmc.c | 35 ------------------------
> >  drivers/gpu/drm/i915/display/intel_dmc.h |  1 -
> >  2 files changed, 36 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
> > index 905b5dcdca14..4124b3d37110 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> > @@ -53,51 +53,40 @@
> >  #define DISPLAY_VER12_DMC_MAX_FW_SIZE	ICL_DMC_MAX_FW_SIZE
> >  
> >  #define DG2_DMC_PATH			DMC_PATH(dg2, 2, 08)
> > -#define DG2_DMC_VERSION_REQUIRED	DMC_VERSION(2, 8)
> >  MODULE_FIRMWARE(DG2_DMC_PATH);
> >  
> >  #define ADLP_DMC_PATH			DMC_PATH(adlp, 2, 16)
> > -#define ADLP_DMC_VERSION_REQUIRED	DMC_VERSION(2, 16)
> >  MODULE_FIRMWARE(ADLP_DMC_PATH);
> >  
> >  #define ADLS_DMC_PATH			DMC_PATH(adls, 2, 01)
> > -#define ADLS_DMC_VERSION_REQUIRED	DMC_VERSION(2, 1)
> >  MODULE_FIRMWARE(ADLS_DMC_PATH);
> >  
> >  #define DG1_DMC_PATH			DMC_PATH(dg1, 2, 02)
> > -#define DG1_DMC_VERSION_REQUIRED	DMC_VERSION(2, 2)
> >  MODULE_FIRMWARE(DG1_DMC_PATH);
> >  
> >  #define RKL_DMC_PATH			DMC_PATH(rkl, 2, 03)
> > -#define RKL_DMC_VERSION_REQUIRED	DMC_VERSION(2, 3)
> >  MODULE_FIRMWARE(RKL_DMC_PATH);
> >  
> >  #define TGL_DMC_PATH			DMC_PATH(tgl, 2, 12)
> > -#define TGL_DMC_VERSION_REQUIRED	DMC_VERSION(2, 12)
> >  MODULE_FIRMWARE(TGL_DMC_PATH);
> >  
> >  #define ICL_DMC_PATH			DMC_PATH(icl, 1, 09)
> > -#define ICL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 9)
> >  #define ICL_DMC_MAX_FW_SIZE		0x6000
> >  MODULE_FIRMWARE(ICL_DMC_PATH);
> >  
> >  #define GLK_DMC_PATH			DMC_PATH(glk, 1, 04)
> > -#define GLK_DMC_VERSION_REQUIRED	DMC_VERSION(1, 4)
> >  #define GLK_DMC_MAX_FW_SIZE		0x4000
> >  MODULE_FIRMWARE(GLK_DMC_PATH);
> >  
> >  #define KBL_DMC_PATH			DMC_PATH(kbl, 1, 04)
> > -#define KBL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 4)
> >  #define KBL_DMC_MAX_FW_SIZE		BXT_DMC_MAX_FW_SIZE
> >  MODULE_FIRMWARE(KBL_DMC_PATH);
> >  
> >  #define SKL_DMC_PATH			DMC_PATH(skl, 1, 27)
> > -#define SKL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 27)
> >  #define SKL_DMC_MAX_FW_SIZE		BXT_DMC_MAX_FW_SIZE
> >  MODULE_FIRMWARE(SKL_DMC_PATH);
> >  
> >  #define BXT_DMC_PATH			DMC_PATH(bxt, 1, 07)
> > -#define BXT_DMC_VERSION_REQUIRED	DMC_VERSION(1, 7)
> >  #define BXT_DMC_MAX_FW_SIZE		0x3000
> >  MODULE_FIRMWARE(BXT_DMC_PATH);
> >  
> > @@ -765,17 +754,6 @@ static u32 parse_dmc_fw_css(struct intel_dmc *dmc,
> >  		return 0;
> >  	}
> >  
> > -	if (dmc->required_version &&
> > -	    css_header->version != dmc->required_version) {
> > -		drm_info(&i915->drm, "Refusing to load DMC firmware v%u.%u,"
> > -			 " please use v%u.%u\n",
> > -			 DMC_VERSION_MAJOR(css_header->version),
> > -			 DMC_VERSION_MINOR(css_header->version),
> > -			 DMC_VERSION_MAJOR(dmc->required_version),
> > -			 DMC_VERSION_MINOR(dmc->required_version));
> > -		return 0;
> > -	}
> > -
> >  	dmc->version = css_header->version;
> >  
> >  	return sizeof(struct intel_css_header);
> > @@ -903,49 +881,38 @@ void intel_dmc_ucode_init(struct drm_i915_private *dev_priv)
> >  
> >  	if (IS_DG2(dev_priv)) {
> >  		dmc->fw_path = DG2_DMC_PATH;
> > -		dmc->required_version = DG2_DMC_VERSION_REQUIRED;
> >  		dmc->max_fw_size = DISPLAY_VER13_DMC_MAX_FW_SIZE;
> >  	} else if (IS_ALDERLAKE_P(dev_priv)) {
> >  		dmc->fw_path = ADLP_DMC_PATH;
> > -		dmc->required_version = ADLP_DMC_VERSION_REQUIRED;
> >  		dmc->max_fw_size = DISPLAY_VER13_DMC_MAX_FW_SIZE;
> >  	} else if (IS_ALDERLAKE_S(dev_priv)) {
> >  		dmc->fw_path = ADLS_DMC_PATH;
> > -		dmc->required_version = ADLS_DMC_VERSION_REQUIRED;
> >  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
> >  	} else if (IS_DG1(dev_priv)) {
> >  		dmc->fw_path = DG1_DMC_PATH;
> > -		dmc->required_version = DG1_DMC_VERSION_REQUIRED;
> >  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
> >  	} else if (IS_ROCKETLAKE(dev_priv)) {
> >  		dmc->fw_path = RKL_DMC_PATH;
> > -		dmc->required_version = RKL_DMC_VERSION_REQUIRED;
> >  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
> >  	} else if (IS_TIGERLAKE(dev_priv)) {
> >  		dmc->fw_path = TGL_DMC_PATH;
> > -		dmc->required_version = TGL_DMC_VERSION_REQUIRED;
> >  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
> >  	} else if (DISPLAY_VER(dev_priv) == 11) {
> >  		dmc->fw_path = ICL_DMC_PATH;
> > -		dmc->required_version = ICL_DMC_VERSION_REQUIRED;
> >  		dmc->max_fw_size = ICL_DMC_MAX_FW_SIZE;
> >  	} else if (IS_GEMINILAKE(dev_priv)) {
> >  		dmc->fw_path = GLK_DMC_PATH;
> > -		dmc->required_version = GLK_DMC_VERSION_REQUIRED;
> >  		dmc->max_fw_size = GLK_DMC_MAX_FW_SIZE;
> >  	} else if (IS_KABYLAKE(dev_priv) ||
> >  		   IS_COFFEELAKE(dev_priv) ||
> >  		   IS_COMETLAKE(dev_priv)) {
> >  		dmc->fw_path = KBL_DMC_PATH;
> > -		dmc->required_version = KBL_DMC_VERSION_REQUIRED;
> >  		dmc->max_fw_size = KBL_DMC_MAX_FW_SIZE;
> >  	} else if (IS_SKYLAKE(dev_priv)) {
> >  		dmc->fw_path = SKL_DMC_PATH;
> > -		dmc->required_version = SKL_DMC_VERSION_REQUIRED;
> >  		dmc->max_fw_size = SKL_DMC_MAX_FW_SIZE;
> >  	} else if (IS_BROXTON(dev_priv)) {
> >  		dmc->fw_path = BXT_DMC_PATH;
> > -		dmc->required_version = BXT_DMC_VERSION_REQUIRED;
> >  		dmc->max_fw_size = BXT_DMC_MAX_FW_SIZE;
> >  	}
> >  
> > @@ -958,8 +925,6 @@ void intel_dmc_ucode_init(struct drm_i915_private *dev_priv)
> >  		}
> >  
> >  		dmc->fw_path = dev_priv->params.dmc_firmware_path;
> > -		/* Bypass version check for firmware override. */
> > -		dmc->required_version = 0;
> >  	}
> >  
> >  	if (!dmc->fw_path) {
> > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.h b/drivers/gpu/drm/i915/display/intel_dmc.h
> > index 67e03315ef99..435eab9b016b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dmc.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dmc.h
> > @@ -25,7 +25,6 @@ enum {
> >  struct intel_dmc {
> >  	struct work_struct work;
> >  	const char *fw_path;
> > -	u32 required_version;
> >  	u32 max_fw_size; /* bytes */
> >  	u32 version;
> >  	struct dmc_fw_info {
> > -- 
> > 2.39.0
> >
Rodrigo Vivi Dec. 30, 2022, 4:52 p.m. UTC | #3
On Fri, Dec 30, 2022 at 09:42:39AM -0300, Gustavo Sousa wrote:
> On Fri, Dec 30, 2022 at 03:32:41AM -0500, Rodrigo Vivi wrote:
> > On Thu, Dec 29, 2022 at 04:07:39PM -0300, Gustavo Sousa wrote:
> > > Currently there is no DMC version requirement with respect to how i915
> > > interacts with it and new versions of the firmware serve as drop-in
> > > replacements of older ones. As such, do not require specific versions.
> > > 
> > > References: https://lore.kernel.org/r/Y3081s7c5ksutpMW@intel.com
> > 
> > I don't believe this link is a good reference as justification
> > of this patch.
> > 
> > Probably https://docs.kernel.org/next/driver-api/firmware/firmware-usage-guidelines.html
> > is a better link?
> 
> Yep. I agree this would be better. Is there an "archive" version of this page?
> Just to make sure we link to the exact version of that guide at the time of
> writing this patch.

This question reminded me that I should had used this link instead:
https://www.kernel.org/doc/html/latest/driver-api/firmware/firmware-usage-guidelines.html

And this is the one you are looking for:
https://www.kernel.org/doc/html/v6.1/driver-api/firmware/firmware-usage-guidelines.html

> 
> > 
> > Also, in the commit message we should be more clear that i915
> > interacts with the Hardware and not with any FW ABI/API, so the API
> > is fixed within the platform, hence no need to get this so-tied
> > version requirement.
> 
> Thanks! I'll grab this paragraph and adapt it into the commit message if you
> allow me =)

sure, thanks!

> 
> > 
> > with the commit msg changed you can count on my reviewed-by,
> > the patch looks good to me.
> > 
> > > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dmc.c | 35 ------------------------
> > >  drivers/gpu/drm/i915/display/intel_dmc.h |  1 -
> > >  2 files changed, 36 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > index 905b5dcdca14..4124b3d37110 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > @@ -53,51 +53,40 @@
> > >  #define DISPLAY_VER12_DMC_MAX_FW_SIZE	ICL_DMC_MAX_FW_SIZE
> > >  
> > >  #define DG2_DMC_PATH			DMC_PATH(dg2, 2, 08)
> > > -#define DG2_DMC_VERSION_REQUIRED	DMC_VERSION(2, 8)
> > >  MODULE_FIRMWARE(DG2_DMC_PATH);
> > >  
> > >  #define ADLP_DMC_PATH			DMC_PATH(adlp, 2, 16)
> > > -#define ADLP_DMC_VERSION_REQUIRED	DMC_VERSION(2, 16)
> > >  MODULE_FIRMWARE(ADLP_DMC_PATH);
> > >  
> > >  #define ADLS_DMC_PATH			DMC_PATH(adls, 2, 01)
> > > -#define ADLS_DMC_VERSION_REQUIRED	DMC_VERSION(2, 1)
> > >  MODULE_FIRMWARE(ADLS_DMC_PATH);
> > >  
> > >  #define DG1_DMC_PATH			DMC_PATH(dg1, 2, 02)
> > > -#define DG1_DMC_VERSION_REQUIRED	DMC_VERSION(2, 2)
> > >  MODULE_FIRMWARE(DG1_DMC_PATH);
> > >  
> > >  #define RKL_DMC_PATH			DMC_PATH(rkl, 2, 03)
> > > -#define RKL_DMC_VERSION_REQUIRED	DMC_VERSION(2, 3)
> > >  MODULE_FIRMWARE(RKL_DMC_PATH);
> > >  
> > >  #define TGL_DMC_PATH			DMC_PATH(tgl, 2, 12)
> > > -#define TGL_DMC_VERSION_REQUIRED	DMC_VERSION(2, 12)
> > >  MODULE_FIRMWARE(TGL_DMC_PATH);
> > >  
> > >  #define ICL_DMC_PATH			DMC_PATH(icl, 1, 09)
> > > -#define ICL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 9)
> > >  #define ICL_DMC_MAX_FW_SIZE		0x6000
> > >  MODULE_FIRMWARE(ICL_DMC_PATH);
> > >  
> > >  #define GLK_DMC_PATH			DMC_PATH(glk, 1, 04)
> > > -#define GLK_DMC_VERSION_REQUIRED	DMC_VERSION(1, 4)
> > >  #define GLK_DMC_MAX_FW_SIZE		0x4000
> > >  MODULE_FIRMWARE(GLK_DMC_PATH);
> > >  
> > >  #define KBL_DMC_PATH			DMC_PATH(kbl, 1, 04)
> > > -#define KBL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 4)
> > >  #define KBL_DMC_MAX_FW_SIZE		BXT_DMC_MAX_FW_SIZE
> > >  MODULE_FIRMWARE(KBL_DMC_PATH);
> > >  
> > >  #define SKL_DMC_PATH			DMC_PATH(skl, 1, 27)
> > > -#define SKL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 27)
> > >  #define SKL_DMC_MAX_FW_SIZE		BXT_DMC_MAX_FW_SIZE
> > >  MODULE_FIRMWARE(SKL_DMC_PATH);
> > >  
> > >  #define BXT_DMC_PATH			DMC_PATH(bxt, 1, 07)
> > > -#define BXT_DMC_VERSION_REQUIRED	DMC_VERSION(1, 7)
> > >  #define BXT_DMC_MAX_FW_SIZE		0x3000
> > >  MODULE_FIRMWARE(BXT_DMC_PATH);
> > >  
> > > @@ -765,17 +754,6 @@ static u32 parse_dmc_fw_css(struct intel_dmc *dmc,
> > >  		return 0;
> > >  	}
> > >  
> > > -	if (dmc->required_version &&
> > > -	    css_header->version != dmc->required_version) {
> > > -		drm_info(&i915->drm, "Refusing to load DMC firmware v%u.%u,"
> > > -			 " please use v%u.%u\n",
> > > -			 DMC_VERSION_MAJOR(css_header->version),
> > > -			 DMC_VERSION_MINOR(css_header->version),
> > > -			 DMC_VERSION_MAJOR(dmc->required_version),
> > > -			 DMC_VERSION_MINOR(dmc->required_version));
> > > -		return 0;
> > > -	}
> > > -
> > >  	dmc->version = css_header->version;
> > >  
> > >  	return sizeof(struct intel_css_header);
> > > @@ -903,49 +881,38 @@ void intel_dmc_ucode_init(struct drm_i915_private *dev_priv)
> > >  
> > >  	if (IS_DG2(dev_priv)) {
> > >  		dmc->fw_path = DG2_DMC_PATH;
> > > -		dmc->required_version = DG2_DMC_VERSION_REQUIRED;
> > >  		dmc->max_fw_size = DISPLAY_VER13_DMC_MAX_FW_SIZE;
> > >  	} else if (IS_ALDERLAKE_P(dev_priv)) {
> > >  		dmc->fw_path = ADLP_DMC_PATH;
> > > -		dmc->required_version = ADLP_DMC_VERSION_REQUIRED;
> > >  		dmc->max_fw_size = DISPLAY_VER13_DMC_MAX_FW_SIZE;
> > >  	} else if (IS_ALDERLAKE_S(dev_priv)) {
> > >  		dmc->fw_path = ADLS_DMC_PATH;
> > > -		dmc->required_version = ADLS_DMC_VERSION_REQUIRED;
> > >  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
> > >  	} else if (IS_DG1(dev_priv)) {
> > >  		dmc->fw_path = DG1_DMC_PATH;
> > > -		dmc->required_version = DG1_DMC_VERSION_REQUIRED;
> > >  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
> > >  	} else if (IS_ROCKETLAKE(dev_priv)) {
> > >  		dmc->fw_path = RKL_DMC_PATH;
> > > -		dmc->required_version = RKL_DMC_VERSION_REQUIRED;
> > >  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
> > >  	} else if (IS_TIGERLAKE(dev_priv)) {
> > >  		dmc->fw_path = TGL_DMC_PATH;
> > > -		dmc->required_version = TGL_DMC_VERSION_REQUIRED;
> > >  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
> > >  	} else if (DISPLAY_VER(dev_priv) == 11) {
> > >  		dmc->fw_path = ICL_DMC_PATH;
> > > -		dmc->required_version = ICL_DMC_VERSION_REQUIRED;
> > >  		dmc->max_fw_size = ICL_DMC_MAX_FW_SIZE;
> > >  	} else if (IS_GEMINILAKE(dev_priv)) {
> > >  		dmc->fw_path = GLK_DMC_PATH;
> > > -		dmc->required_version = GLK_DMC_VERSION_REQUIRED;
> > >  		dmc->max_fw_size = GLK_DMC_MAX_FW_SIZE;
> > >  	} else if (IS_KABYLAKE(dev_priv) ||
> > >  		   IS_COFFEELAKE(dev_priv) ||
> > >  		   IS_COMETLAKE(dev_priv)) {
> > >  		dmc->fw_path = KBL_DMC_PATH;
> > > -		dmc->required_version = KBL_DMC_VERSION_REQUIRED;
> > >  		dmc->max_fw_size = KBL_DMC_MAX_FW_SIZE;
> > >  	} else if (IS_SKYLAKE(dev_priv)) {
> > >  		dmc->fw_path = SKL_DMC_PATH;
> > > -		dmc->required_version = SKL_DMC_VERSION_REQUIRED;
> > >  		dmc->max_fw_size = SKL_DMC_MAX_FW_SIZE;
> > >  	} else if (IS_BROXTON(dev_priv)) {
> > >  		dmc->fw_path = BXT_DMC_PATH;
> > > -		dmc->required_version = BXT_DMC_VERSION_REQUIRED;
> > >  		dmc->max_fw_size = BXT_DMC_MAX_FW_SIZE;
> > >  	}
> > >  
> > > @@ -958,8 +925,6 @@ void intel_dmc_ucode_init(struct drm_i915_private *dev_priv)
> > >  		}
> > >  
> > >  		dmc->fw_path = dev_priv->params.dmc_firmware_path;
> > > -		/* Bypass version check for firmware override. */
> > > -		dmc->required_version = 0;
> > >  	}
> > >  
> > >  	if (!dmc->fw_path) {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.h b/drivers/gpu/drm/i915/display/intel_dmc.h
> > > index 67e03315ef99..435eab9b016b 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dmc.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_dmc.h
> > > @@ -25,7 +25,6 @@ enum {
> > >  struct intel_dmc {
> > >  	struct work_struct work;
> > >  	const char *fw_path;
> > > -	u32 required_version;
> > >  	u32 max_fw_size; /* bytes */
> > >  	u32 version;
> > >  	struct dmc_fw_info {
> > > -- 
> > > 2.39.0
> > >
Gustavo Sousa Dec. 30, 2022, 6:27 p.m. UTC | #4
Thanks for all the feedback.

I have just sent a v4 with only this patch. I'll split the second one as
instructed.

--
Gustavo Sousa

On Fri, Dec 30, 2022 at 11:52:07AM -0500, Rodrigo Vivi wrote:
> On Fri, Dec 30, 2022 at 09:42:39AM -0300, Gustavo Sousa wrote:
> > On Fri, Dec 30, 2022 at 03:32:41AM -0500, Rodrigo Vivi wrote:
> > > On Thu, Dec 29, 2022 at 04:07:39PM -0300, Gustavo Sousa wrote:
> > > > Currently there is no DMC version requirement with respect to how i915
> > > > interacts with it and new versions of the firmware serve as drop-in
> > > > replacements of older ones. As such, do not require specific versions.
> > > > 
> > > > References: https://lore.kernel.org/r/Y3081s7c5ksutpMW@intel.com
> > > 
> > > I don't believe this link is a good reference as justification
> > > of this patch.
> > > 
> > > Probably https://docs.kernel.org/next/driver-api/firmware/firmware-usage-guidelines.html
> > > is a better link?
> > 
> > Yep. I agree this would be better. Is there an "archive" version of this page?
> > Just to make sure we link to the exact version of that guide at the time of
> > writing this patch.
> 
> This question reminded me that I should had used this link instead:
> https://www.kernel.org/doc/html/latest/driver-api/firmware/firmware-usage-guidelines.html
> 
> And this is the one you are looking for:
> https://www.kernel.org/doc/html/v6.1/driver-api/firmware/firmware-usage-guidelines.html
> 
> > 
> > > 
> > > Also, in the commit message we should be more clear that i915
> > > interacts with the Hardware and not with any FW ABI/API, so the API
> > > is fixed within the platform, hence no need to get this so-tied
> > > version requirement.
> > 
> > Thanks! I'll grab this paragraph and adapt it into the commit message if you
> > allow me =)
> 
> sure, thanks!
> 
> > 
> > > 
> > > with the commit msg changed you can count on my reviewed-by,
> > > the patch looks good to me.
> > > 
> > > > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_dmc.c | 35 ------------------------
> > > >  drivers/gpu/drm/i915/display/intel_dmc.h |  1 -
> > > >  2 files changed, 36 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > index 905b5dcdca14..4124b3d37110 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> > > > @@ -53,51 +53,40 @@
> > > >  #define DISPLAY_VER12_DMC_MAX_FW_SIZE	ICL_DMC_MAX_FW_SIZE
> > > >  
> > > >  #define DG2_DMC_PATH			DMC_PATH(dg2, 2, 08)
> > > > -#define DG2_DMC_VERSION_REQUIRED	DMC_VERSION(2, 8)
> > > >  MODULE_FIRMWARE(DG2_DMC_PATH);
> > > >  
> > > >  #define ADLP_DMC_PATH			DMC_PATH(adlp, 2, 16)
> > > > -#define ADLP_DMC_VERSION_REQUIRED	DMC_VERSION(2, 16)
> > > >  MODULE_FIRMWARE(ADLP_DMC_PATH);
> > > >  
> > > >  #define ADLS_DMC_PATH			DMC_PATH(adls, 2, 01)
> > > > -#define ADLS_DMC_VERSION_REQUIRED	DMC_VERSION(2, 1)
> > > >  MODULE_FIRMWARE(ADLS_DMC_PATH);
> > > >  
> > > >  #define DG1_DMC_PATH			DMC_PATH(dg1, 2, 02)
> > > > -#define DG1_DMC_VERSION_REQUIRED	DMC_VERSION(2, 2)
> > > >  MODULE_FIRMWARE(DG1_DMC_PATH);
> > > >  
> > > >  #define RKL_DMC_PATH			DMC_PATH(rkl, 2, 03)
> > > > -#define RKL_DMC_VERSION_REQUIRED	DMC_VERSION(2, 3)
> > > >  MODULE_FIRMWARE(RKL_DMC_PATH);
> > > >  
> > > >  #define TGL_DMC_PATH			DMC_PATH(tgl, 2, 12)
> > > > -#define TGL_DMC_VERSION_REQUIRED	DMC_VERSION(2, 12)
> > > >  MODULE_FIRMWARE(TGL_DMC_PATH);
> > > >  
> > > >  #define ICL_DMC_PATH			DMC_PATH(icl, 1, 09)
> > > > -#define ICL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 9)
> > > >  #define ICL_DMC_MAX_FW_SIZE		0x6000
> > > >  MODULE_FIRMWARE(ICL_DMC_PATH);
> > > >  
> > > >  #define GLK_DMC_PATH			DMC_PATH(glk, 1, 04)
> > > > -#define GLK_DMC_VERSION_REQUIRED	DMC_VERSION(1, 4)
> > > >  #define GLK_DMC_MAX_FW_SIZE		0x4000
> > > >  MODULE_FIRMWARE(GLK_DMC_PATH);
> > > >  
> > > >  #define KBL_DMC_PATH			DMC_PATH(kbl, 1, 04)
> > > > -#define KBL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 4)
> > > >  #define KBL_DMC_MAX_FW_SIZE		BXT_DMC_MAX_FW_SIZE
> > > >  MODULE_FIRMWARE(KBL_DMC_PATH);
> > > >  
> > > >  #define SKL_DMC_PATH			DMC_PATH(skl, 1, 27)
> > > > -#define SKL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 27)
> > > >  #define SKL_DMC_MAX_FW_SIZE		BXT_DMC_MAX_FW_SIZE
> > > >  MODULE_FIRMWARE(SKL_DMC_PATH);
> > > >  
> > > >  #define BXT_DMC_PATH			DMC_PATH(bxt, 1, 07)
> > > > -#define BXT_DMC_VERSION_REQUIRED	DMC_VERSION(1, 7)
> > > >  #define BXT_DMC_MAX_FW_SIZE		0x3000
> > > >  MODULE_FIRMWARE(BXT_DMC_PATH);
> > > >  
> > > > @@ -765,17 +754,6 @@ static u32 parse_dmc_fw_css(struct intel_dmc *dmc,
> > > >  		return 0;
> > > >  	}
> > > >  
> > > > -	if (dmc->required_version &&
> > > > -	    css_header->version != dmc->required_version) {
> > > > -		drm_info(&i915->drm, "Refusing to load DMC firmware v%u.%u,"
> > > > -			 " please use v%u.%u\n",
> > > > -			 DMC_VERSION_MAJOR(css_header->version),
> > > > -			 DMC_VERSION_MINOR(css_header->version),
> > > > -			 DMC_VERSION_MAJOR(dmc->required_version),
> > > > -			 DMC_VERSION_MINOR(dmc->required_version));
> > > > -		return 0;
> > > > -	}
> > > > -
> > > >  	dmc->version = css_header->version;
> > > >  
> > > >  	return sizeof(struct intel_css_header);
> > > > @@ -903,49 +881,38 @@ void intel_dmc_ucode_init(struct drm_i915_private *dev_priv)
> > > >  
> > > >  	if (IS_DG2(dev_priv)) {
> > > >  		dmc->fw_path = DG2_DMC_PATH;
> > > > -		dmc->required_version = DG2_DMC_VERSION_REQUIRED;
> > > >  		dmc->max_fw_size = DISPLAY_VER13_DMC_MAX_FW_SIZE;
> > > >  	} else if (IS_ALDERLAKE_P(dev_priv)) {
> > > >  		dmc->fw_path = ADLP_DMC_PATH;
> > > > -		dmc->required_version = ADLP_DMC_VERSION_REQUIRED;
> > > >  		dmc->max_fw_size = DISPLAY_VER13_DMC_MAX_FW_SIZE;
> > > >  	} else if (IS_ALDERLAKE_S(dev_priv)) {
> > > >  		dmc->fw_path = ADLS_DMC_PATH;
> > > > -		dmc->required_version = ADLS_DMC_VERSION_REQUIRED;
> > > >  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
> > > >  	} else if (IS_DG1(dev_priv)) {
> > > >  		dmc->fw_path = DG1_DMC_PATH;
> > > > -		dmc->required_version = DG1_DMC_VERSION_REQUIRED;
> > > >  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
> > > >  	} else if (IS_ROCKETLAKE(dev_priv)) {
> > > >  		dmc->fw_path = RKL_DMC_PATH;
> > > > -		dmc->required_version = RKL_DMC_VERSION_REQUIRED;
> > > >  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
> > > >  	} else if (IS_TIGERLAKE(dev_priv)) {
> > > >  		dmc->fw_path = TGL_DMC_PATH;
> > > > -		dmc->required_version = TGL_DMC_VERSION_REQUIRED;
> > > >  		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
> > > >  	} else if (DISPLAY_VER(dev_priv) == 11) {
> > > >  		dmc->fw_path = ICL_DMC_PATH;
> > > > -		dmc->required_version = ICL_DMC_VERSION_REQUIRED;
> > > >  		dmc->max_fw_size = ICL_DMC_MAX_FW_SIZE;
> > > >  	} else if (IS_GEMINILAKE(dev_priv)) {
> > > >  		dmc->fw_path = GLK_DMC_PATH;
> > > > -		dmc->required_version = GLK_DMC_VERSION_REQUIRED;
> > > >  		dmc->max_fw_size = GLK_DMC_MAX_FW_SIZE;
> > > >  	} else if (IS_KABYLAKE(dev_priv) ||
> > > >  		   IS_COFFEELAKE(dev_priv) ||
> > > >  		   IS_COMETLAKE(dev_priv)) {
> > > >  		dmc->fw_path = KBL_DMC_PATH;
> > > > -		dmc->required_version = KBL_DMC_VERSION_REQUIRED;
> > > >  		dmc->max_fw_size = KBL_DMC_MAX_FW_SIZE;
> > > >  	} else if (IS_SKYLAKE(dev_priv)) {
> > > >  		dmc->fw_path = SKL_DMC_PATH;
> > > > -		dmc->required_version = SKL_DMC_VERSION_REQUIRED;
> > > >  		dmc->max_fw_size = SKL_DMC_MAX_FW_SIZE;
> > > >  	} else if (IS_BROXTON(dev_priv)) {
> > > >  		dmc->fw_path = BXT_DMC_PATH;
> > > > -		dmc->required_version = BXT_DMC_VERSION_REQUIRED;
> > > >  		dmc->max_fw_size = BXT_DMC_MAX_FW_SIZE;
> > > >  	}
> > > >  
> > > > @@ -958,8 +925,6 @@ void intel_dmc_ucode_init(struct drm_i915_private *dev_priv)
> > > >  		}
> > > >  
> > > >  		dmc->fw_path = dev_priv->params.dmc_firmware_path;
> > > > -		/* Bypass version check for firmware override. */
> > > > -		dmc->required_version = 0;
> > > >  	}
> > > >  
> > > >  	if (!dmc->fw_path) {
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.h b/drivers/gpu/drm/i915/display/intel_dmc.h
> > > > index 67e03315ef99..435eab9b016b 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dmc.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dmc.h
> > > > @@ -25,7 +25,6 @@ enum {
> > > >  struct intel_dmc {
> > > >  	struct work_struct work;
> > > >  	const char *fw_path;
> > > > -	u32 required_version;
> > > >  	u32 max_fw_size; /* bytes */
> > > >  	u32 version;
> > > >  	struct dmc_fw_info {
> > > > -- 
> > > > 2.39.0
> > > >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 905b5dcdca14..4124b3d37110 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -53,51 +53,40 @@ 
 #define DISPLAY_VER12_DMC_MAX_FW_SIZE	ICL_DMC_MAX_FW_SIZE
 
 #define DG2_DMC_PATH			DMC_PATH(dg2, 2, 08)
-#define DG2_DMC_VERSION_REQUIRED	DMC_VERSION(2, 8)
 MODULE_FIRMWARE(DG2_DMC_PATH);
 
 #define ADLP_DMC_PATH			DMC_PATH(adlp, 2, 16)
-#define ADLP_DMC_VERSION_REQUIRED	DMC_VERSION(2, 16)
 MODULE_FIRMWARE(ADLP_DMC_PATH);
 
 #define ADLS_DMC_PATH			DMC_PATH(adls, 2, 01)
-#define ADLS_DMC_VERSION_REQUIRED	DMC_VERSION(2, 1)
 MODULE_FIRMWARE(ADLS_DMC_PATH);
 
 #define DG1_DMC_PATH			DMC_PATH(dg1, 2, 02)
-#define DG1_DMC_VERSION_REQUIRED	DMC_VERSION(2, 2)
 MODULE_FIRMWARE(DG1_DMC_PATH);
 
 #define RKL_DMC_PATH			DMC_PATH(rkl, 2, 03)
-#define RKL_DMC_VERSION_REQUIRED	DMC_VERSION(2, 3)
 MODULE_FIRMWARE(RKL_DMC_PATH);
 
 #define TGL_DMC_PATH			DMC_PATH(tgl, 2, 12)
-#define TGL_DMC_VERSION_REQUIRED	DMC_VERSION(2, 12)
 MODULE_FIRMWARE(TGL_DMC_PATH);
 
 #define ICL_DMC_PATH			DMC_PATH(icl, 1, 09)
-#define ICL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 9)
 #define ICL_DMC_MAX_FW_SIZE		0x6000
 MODULE_FIRMWARE(ICL_DMC_PATH);
 
 #define GLK_DMC_PATH			DMC_PATH(glk, 1, 04)
-#define GLK_DMC_VERSION_REQUIRED	DMC_VERSION(1, 4)
 #define GLK_DMC_MAX_FW_SIZE		0x4000
 MODULE_FIRMWARE(GLK_DMC_PATH);
 
 #define KBL_DMC_PATH			DMC_PATH(kbl, 1, 04)
-#define KBL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 4)
 #define KBL_DMC_MAX_FW_SIZE		BXT_DMC_MAX_FW_SIZE
 MODULE_FIRMWARE(KBL_DMC_PATH);
 
 #define SKL_DMC_PATH			DMC_PATH(skl, 1, 27)
-#define SKL_DMC_VERSION_REQUIRED	DMC_VERSION(1, 27)
 #define SKL_DMC_MAX_FW_SIZE		BXT_DMC_MAX_FW_SIZE
 MODULE_FIRMWARE(SKL_DMC_PATH);
 
 #define BXT_DMC_PATH			DMC_PATH(bxt, 1, 07)
-#define BXT_DMC_VERSION_REQUIRED	DMC_VERSION(1, 7)
 #define BXT_DMC_MAX_FW_SIZE		0x3000
 MODULE_FIRMWARE(BXT_DMC_PATH);
 
@@ -765,17 +754,6 @@  static u32 parse_dmc_fw_css(struct intel_dmc *dmc,
 		return 0;
 	}
 
-	if (dmc->required_version &&
-	    css_header->version != dmc->required_version) {
-		drm_info(&i915->drm, "Refusing to load DMC firmware v%u.%u,"
-			 " please use v%u.%u\n",
-			 DMC_VERSION_MAJOR(css_header->version),
-			 DMC_VERSION_MINOR(css_header->version),
-			 DMC_VERSION_MAJOR(dmc->required_version),
-			 DMC_VERSION_MINOR(dmc->required_version));
-		return 0;
-	}
-
 	dmc->version = css_header->version;
 
 	return sizeof(struct intel_css_header);
@@ -903,49 +881,38 @@  void intel_dmc_ucode_init(struct drm_i915_private *dev_priv)
 
 	if (IS_DG2(dev_priv)) {
 		dmc->fw_path = DG2_DMC_PATH;
-		dmc->required_version = DG2_DMC_VERSION_REQUIRED;
 		dmc->max_fw_size = DISPLAY_VER13_DMC_MAX_FW_SIZE;
 	} else if (IS_ALDERLAKE_P(dev_priv)) {
 		dmc->fw_path = ADLP_DMC_PATH;
-		dmc->required_version = ADLP_DMC_VERSION_REQUIRED;
 		dmc->max_fw_size = DISPLAY_VER13_DMC_MAX_FW_SIZE;
 	} else if (IS_ALDERLAKE_S(dev_priv)) {
 		dmc->fw_path = ADLS_DMC_PATH;
-		dmc->required_version = ADLS_DMC_VERSION_REQUIRED;
 		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
 	} else if (IS_DG1(dev_priv)) {
 		dmc->fw_path = DG1_DMC_PATH;
-		dmc->required_version = DG1_DMC_VERSION_REQUIRED;
 		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
 	} else if (IS_ROCKETLAKE(dev_priv)) {
 		dmc->fw_path = RKL_DMC_PATH;
-		dmc->required_version = RKL_DMC_VERSION_REQUIRED;
 		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
 	} else if (IS_TIGERLAKE(dev_priv)) {
 		dmc->fw_path = TGL_DMC_PATH;
-		dmc->required_version = TGL_DMC_VERSION_REQUIRED;
 		dmc->max_fw_size = DISPLAY_VER12_DMC_MAX_FW_SIZE;
 	} else if (DISPLAY_VER(dev_priv) == 11) {
 		dmc->fw_path = ICL_DMC_PATH;
-		dmc->required_version = ICL_DMC_VERSION_REQUIRED;
 		dmc->max_fw_size = ICL_DMC_MAX_FW_SIZE;
 	} else if (IS_GEMINILAKE(dev_priv)) {
 		dmc->fw_path = GLK_DMC_PATH;
-		dmc->required_version = GLK_DMC_VERSION_REQUIRED;
 		dmc->max_fw_size = GLK_DMC_MAX_FW_SIZE;
 	} else if (IS_KABYLAKE(dev_priv) ||
 		   IS_COFFEELAKE(dev_priv) ||
 		   IS_COMETLAKE(dev_priv)) {
 		dmc->fw_path = KBL_DMC_PATH;
-		dmc->required_version = KBL_DMC_VERSION_REQUIRED;
 		dmc->max_fw_size = KBL_DMC_MAX_FW_SIZE;
 	} else if (IS_SKYLAKE(dev_priv)) {
 		dmc->fw_path = SKL_DMC_PATH;
-		dmc->required_version = SKL_DMC_VERSION_REQUIRED;
 		dmc->max_fw_size = SKL_DMC_MAX_FW_SIZE;
 	} else if (IS_BROXTON(dev_priv)) {
 		dmc->fw_path = BXT_DMC_PATH;
-		dmc->required_version = BXT_DMC_VERSION_REQUIRED;
 		dmc->max_fw_size = BXT_DMC_MAX_FW_SIZE;
 	}
 
@@ -958,8 +925,6 @@  void intel_dmc_ucode_init(struct drm_i915_private *dev_priv)
 		}
 
 		dmc->fw_path = dev_priv->params.dmc_firmware_path;
-		/* Bypass version check for firmware override. */
-		dmc->required_version = 0;
 	}
 
 	if (!dmc->fw_path) {
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.h b/drivers/gpu/drm/i915/display/intel_dmc.h
index 67e03315ef99..435eab9b016b 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc.h
@@ -25,7 +25,6 @@  enum {
 struct intel_dmc {
 	struct work_struct work;
 	const char *fw_path;
-	u32 required_version;
 	u32 max_fw_size; /* bytes */
 	u32 version;
 	struct dmc_fw_info {