On Mon, 2017-10-02 at 14:01 +0000, Michal Wajdeczko wrote:
> This is a prerequisite to unblock next steps.
This is not a very good commit message when somebody is looking at this
patch individually. Something along lines "To improve code separation,
move uC firmware helper code into dedicated files."
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
<SNIP>
> void intel_uc_init_fw(struct drm_i915_private *dev_priv)
> {
> - fetch_uc_fw(dev_priv, &dev_priv->huc.fw);
> - fetch_uc_fw(dev_priv, &dev_priv->guc.fw);
> + i915_fetch_uc_fw(dev_priv, &dev_priv->huc.fw);
> + i915_fetch_uc_fw(dev_priv, &dev_priv->guc.fw);
> }
The microcontroller firmware most definitely is not a driver only
construct, so just call the function intel_uc_fw_init for less
confusion. We also validate and place in GTT the FW in addition to
fetching it.
> void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
> {
> - __intel_uc_fw_fini(&dev_priv->guc.fw);
> - __intel_uc_fw_fini(&dev_priv->huc.fw);
> + intel_uc_fw_fini(&dev_priv->guc.fw);
> + intel_uc_fw_fini(&dev_priv->huc.fw);
> }
And then this looks more symmetric, too.
When we have 'intel_uc_fw.h' for 'intel_uc_fw' object, I pretty much
assume to only see intel_uc_fw_* functions in there. Also, please
reoder the .h and .c so that init is first, and fini is after.
With those changes, this is;
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
