On ke, 2017-02-22 at 13:41 +0100, Arkadiusz Hiler wrote:
> intel_uc_fw_fetch() is confusingly named in the light of recent changes.
>
> It's also in the wrong place - 'guc_loader.h' - it's used for both guc
> and huc, which was reflected in name, but not it's location, so let's
> move it to 'intel_uc.c'.
>
> We can make a intel_uc_fw callback out of it, to avoid leaking it
> outside `intel_uc.c`
>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Michal Winiarski <michal.winiarski@xxxxxxxxx>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx>
<SNIP>
> @@ -32,7 +36,10 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
>
> void intel_uc_fetch_fw(struct drm_i915_private *dev_priv)
> {
> + dev_priv->huc.fw.fetch = uc_fetch_fw;
> intel_huc_fetch_fw(&dev_priv->huc);
> +
> + dev_priv->guc.fw.fetch = uc_fetch_fw;
I'm bit confused, I was under impression these functions were going to
be different for each uC? If they're not, then it most definitely
should not be a function pointer.
int ret;
ret = intel_huc_select_fw(dev_priv->huc);
if (ret)
goto err;
ret = intel_uc_fw_prepare(&dev_priv->huc->fw);
if (ret)
goto err;
ret = intel_guc_select_fw(dev_priv->guc);
if (ret)
goto err_huc;
ret = intel_uc_fw_prepare(&dev_priv->guc->fw);
if (ret)
goto err_huc;
return 0;
err_huc:
intel_uc_fw_unprepare(&dev_priv->huc->fw);
err:
return ret;
By always involving proper onion teardown, no dangling objects are left
around.
> intel_guc_fetch_fw(&dev_priv->guc);
> }
>
> @@ -120,3 +127,137 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
> > return intel_guc_send(guc, action, ARRAY_SIZE(action));
> }
>
> +static void uc_fetch_fw(struct drm_i915_private *dev_priv,
> + struct intel_uc_fw *uc_fw)
> +{
> + struct pci_dev *pdev = dev_priv->drm.pdev;
> + struct drm_i915_gem_object *obj;
> + const struct firmware *fw = NULL;
> + struct uc_css_header *css;
> + size_t size;
> + int err;
This function should be named differently, because it doesn't simply
fetch it, but also validates and makes an object of it (which may be
left dangling).
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
