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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx
> > 
> > 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@xxxxxxxxx>
> > > ---
> > >  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
> > > 



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux