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 <[email protected]> > Cc: Joonas Lahtinen <[email protected]> > Cc: Chris Wilson <[email protected]> > Cc: Sagar Arun Kamble <[email protected]> > Reviewed-by: Sagar Arun Kamble <[email protected]> <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 <[email protected]> Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
