[Intel-gfx] [PATCH 5/5] drm/i915/uc: Simplify fw_path

Arkadiusz Hiler arkadiusz.hiler at intel.com
Wed Feb 15 14:12:56 UTC 2017


On Tue, Feb 14, 2017 at 09:23:33PM +0100, Michal Wajdeczko wrote:
> On Tue, Feb 14, 2017 at 05:15:41PM +0100, Arkadiusz Hiler wrote:
> > Currently fw_path values can represent one of three possible states:
> > 
> >  1) NULL - device without the uC
> >  2) '\0' - device with the uC but have no firmware
> >  3) else - device with the uC and we have firmware
> > 
> > Second case is used only to WARN at a later stage.
> > 
> > We can WARN right away and merge cases 1 and 2.

[ snip ]

> >  /**
> > diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> > index 94d369b..70606e6 100644
> > --- a/drivers/gpu/drm/i915/intel_huc.c
> > +++ b/drivers/gpu/drm/i915/intel_huc.c
> > @@ -154,18 +154,13 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv)
> >   */
> >  void intel_huc_init(struct drm_i915_private *dev_priv)
> >  {
> > -	struct intel_huc *huc = &dev_priv->huc;
> > -	struct intel_uc_fw *huc_fw = &huc->fw;
> > +	struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
> >  	const char *fw_path = NULL;
> >  
> > -	huc_fw->path = NULL;
> >  	huc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE;
> >  	huc_fw->load_status = INTEL_UC_FIRMWARE_NONE;
> >  	huc_fw->fw = INTEL_UC_FW_TYPE_HUC;
> >  
> > -	if (!HAS_HUC_UCODE(dev_priv))
> > -		return;
> > -
> >  	if (IS_SKYLAKE(dev_priv)) {
> >  		fw_path = I915_SKL_HUC_UCODE;
> >  		huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR;
> > @@ -178,15 +173,18 @@ void intel_huc_init(struct drm_i915_private *dev_priv)
> >  		fw_path = I915_KBL_HUC_UCODE;
> >  		huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR;
> >  		huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR;
> > +	} else if (HAS_HUC_UCODE(dev_priv)) {
> > +		WARN(1, "No HuC firmware known for platform with HuC!\n");
> 
> Can we simplify this into:
> 
> 	} else {
> 		WARN(HAS_HUC_UCODE(dev_priv), "No HuC firmware known for platform with HuC!\n");
> 		return;

We can even simplify it further dropping the conditional on `HAS_?_UCODE`.

The function is called only when we have the UCODE - and the macro will
always be true here.

Reset of the feedback incorporated. Thanks!


-- 
Cheers,
Arek


More information about the Intel-gfx mailing list