On Thu, Mar 26, 2015 at 12:41:13PM -0700, yu.dai@xxxxxxxxx wrote:
> From: Dave Gordon <david.s.gordon@xxxxxxxxx>
>
> In order to fully initialise the default contexts, we have to execute
> batchbuffer commands on the GPU engines. But we can't do that until any
> required firmware has been loaded, which may not be possible during
> driver load, because the filesystem(s) containing the firmware may not
> be mounted until later.
>
> Therefore, we now allow the first call to the firmware-loading code to
> return -EAGAIN to indicate that it's not yet ready, and that it should
> be retried when the device is first opened from user code, by which
> time we expect that all required filesystems will have been mounted.
> The late-retry code will then re-attempt to load the firmware if the
> early attempt failed.
We've tried a similar approach a while back and it doesn't work well in
conjunction with rps - the hw tends to fall over if the context state
isn't properly initialized when going into rc6.
Why exactly can't we load that firmware right at boot-up, or at least
stall correctly until it's there?
-Daniel
>
> Issue: VIZ-4884
> Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
> Signed-off-by: Alex Dai <yu.dai@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_gem.c | 9 ++++++++-
> drivers/gpu/drm/i915/i915_gem_context.c | 35 ++++++++++++++++++++++++++++-----
> drivers/gpu/drm/i915/intel_guc.h | 2 +-
> drivers/gpu/drm/i915/intel_guc_loader.c | 22 ++++++++++++++++++---
> drivers/gpu/drm/i915/intel_uc_loader.c | 12 +++++++++--
> 6 files changed, 69 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1a60f86..45bdf30 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1755,6 +1755,7 @@ struct drm_i915_private {
> /* hda/i915 audio component */
> bool audio_component_registered;
>
> + bool contexts_ready;
> uint32_t hw_context_size;
> struct list_head context_list;
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 38e49d9..0037ccf 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4828,8 +4828,15 @@ i915_gem_init_hw(struct drm_device *dev)
> i915_gem_cleanup_ringbuffer(dev);
> }
>
> + /* We can't enable contexts until all firmware is loaded */
> + ret = intel_guc_load_ucode(dev, false);
> + if (ret == -EAGAIN)
> + return 0; /* too early */
> +
> ret = i915_gem_context_enable(dev_priv);
> - if (ret && ret != -EIO) {
> + if (ret == 0) {
> + dev_priv->contexts_ready = true;
> + } else if (ret && ret != -EIO) {
> DRM_ERROR("Context enable failed %d\n", ret);
> i915_gem_cleanup_ringbuffer(dev);
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index f3e84c4..2353d8f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -445,23 +445,48 @@ static int context_idr_cleanup(int id, void *p, void *data)
> return 0;
> }
>
> +/* Complete any late initialisation here */
> +static int i915_gem_context_first_open(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + int ret;
> +
> + /* We can't enable contexts until all firmware is loaded */
> + ret = intel_guc_load_ucode(dev, true);
> + WARN_ON(ret == -EAGAIN);
> +
> + ret = i915_gem_context_enable(dev_priv);
> + if (ret == 0)
> + dev_priv->contexts_ready = true;
> + return ret;
> +}
> +
> int i915_gem_context_open(struct drm_device *dev, struct drm_file *file)
> {
> + struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_i915_file_private *file_priv = file->driver_priv;
> struct intel_context *ctx;
> + int ret = 0;
>
> idr_init(&file_priv->context_idr);
>
> mutex_lock(&dev->struct_mutex);
> - ctx = i915_gem_create_context(dev, file_priv);
> +
> + if (!dev_priv->contexts_ready)
> + ret = i915_gem_context_first_open(dev);
> +
> + if (ret == 0) {
> + ctx = i915_gem_create_context(dev, file_priv);
> + if (IS_ERR(ctx))
> + ret = PTR_ERR(ctx);
> + }
> +
> mutex_unlock(&dev->struct_mutex);
>
> - if (IS_ERR(ctx)) {
> + if (ret)
> idr_destroy(&file_priv->context_idr);
> - return PTR_ERR(ctx);
> - }
>
> - return 0;
> + return ret;
> }
>
> void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index a06a7b3..d8262cf 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -89,7 +89,7 @@ struct intel_guc {
> GUC_ENABLE_READ_CACHE_FOR_WOPCM_DATA)
>
> /* intel_guc_loader.c */
> -extern int intel_guc_load_ucode(struct drm_device *dev);
> +extern int intel_guc_load_ucode(struct drm_device *dev, bool wait);
> extern void intel_guc_ucode_fini(struct drm_device *dev);
> extern void intel_guc_ucode_init(struct drm_device *dev);
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index b9923b7..315e5d9 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -217,25 +217,41 @@ out:
> }
>
> /*
> - * Called from gem_init_hw() during driver loading and also after a GPU reset.
> * Check that the firmware fetching process has succeeded, and if so transfer
> * the loaded image to the hardware.
> + *
> + * However, there are a few checks to do first. The very first call should have
> + * (wait == FALSE), but the fetch_state will still be PENDING as the firmware may
> + * not be available that early. Therefore, on this first call, we just return.
> + *
> + * The second call should come from the first open of the device (wait == TRUE).
> + * This is a good time to load the firmware into the device, as by this point it
> + * must be available.
> + *
> + * Any subsequent calls are expected to have wait == FALSE, and indicate that the
> + * hardware has been reset and so the firmware should be reloaded.
> */
> -int intel_guc_load_ucode(struct drm_device *dev)
> +int intel_guc_load_ucode(struct drm_device *dev, bool wait)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_uc_fw *guc_fw = &dev_priv->guc.guc_fw;
> int err;
>
> + DRM_DEBUG_DRIVER("GuC: wait %d, fetch status %d, load status %d\n",
> + wait, guc_fw->uc_fw_fetch_status, guc_fw->uc_fw_load_status);
> +
> + if (guc_fw->uc_fw_fetch_status == INTEL_UC_FIRMWARE_PENDING && !wait)
> + return -EAGAIN;
> +
> guc_fw->uc_fw_load_status = INTEL_UC_FIRMWARE_NONE;
> if (guc_fw->uc_fw_fetch_status == INTEL_UC_FIRMWARE_NONE)
> return 0;
>
> - guc_fw->uc_fw_load_status = INTEL_UC_FIRMWARE_PENDING;
> err = intel_uc_fw_check(dev, guc_fw);
> if (err)
> goto fail;
>
> + guc_fw->uc_fw_load_status = INTEL_UC_FIRMWARE_PENDING;
> err = guc_load_ucode(dev);
> if (err)
> goto fail;
> diff --git a/drivers/gpu/drm/i915/intel_uc_loader.c b/drivers/gpu/drm/i915/intel_uc_loader.c
> index 986dcf3..bc4b1e5 100644
> --- a/drivers/gpu/drm/i915/intel_uc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_uc_loader.c
> @@ -49,8 +49,16 @@ static void uc_fw_finish(struct drm_device *dev, struct intel_uc_fw *uc_fw)
> uc_fw->uc_name, uc_fw->uc_fw_fetch_status, uc_fw->uc_fw_blob);
>
> fw = uc_fw->uc_fw_blob;
> - if (!fw)
> - goto fail;
> + if (!fw) {
> + /* no firmware found; try again in case FS was not mounted */
> + DRM_DEBUG_DRIVER("retry fetching %s fw from <%s>\n",
> + uc_fw->uc_name, uc_fw->uc_fw_path);
> + if (request_firmware(&fw, uc_fw->uc_fw_path, &dev->pdev->dev))
> + goto fail;
> + DRM_DEBUG_DRIVER("fetch %s fw from <%s> succeeded, fw %p\n",
> + uc_fw->uc_name, uc_fw->uc_fw_path, fw);
> + uc_fw->uc_fw_blob = fw;
> + }
>
> if (uc_fw->uc_fw_check && !uc_fw->uc_fw_check(uc_fw))
> goto fail;
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
