On Thu, Mar 26, 2015 at 12:41:13PM -0700, yu....@intel.com wrote:
> From: Dave Gordon <david.s.gor...@intel.com>
> 
> 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.gor...@intel.com>
> Signed-off-by: Alex Dai <yu....@intel.com>
> ---
>  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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to