[Intel-gfx] [PATCH v2 2/2] drm/i915/guc: Reset GuC and retry on firmware load failure
Arun Siluvery
arun.siluvery at linux.intel.com
Fri Mar 11 11:03:58 UTC 2016
On 10/03/2016 17:57, Yu Dai wrote:
> LGTM. Reviewed-by: Alex Dai <yu.dai at intel.com>
>
thanks for the review.
This patch is dependent on patch1 which is being reworked. I will rebase
send this to the list again once all dependencies are resolved.
regards
Arun
> On 03/08/2016 03:38 AM, Arun Siluvery wrote:
>> Due to timing issues in the HW some of the status bits required for GuC
>> authentication doesn't get set occassionally, when that happens, GuC
>> cannot
>> be initialized and we will be left with a wedged GPU. The WA suggested is
>> to perform a soft reset of GuC and attempt to reload the fw again for few
>> times before giving up.
>>
>> As the failure is dependent on timing, tests performed by triggering
>> manual
>> full gpu reset (i915_wedged) showed that we could sometimes hit this
>> after
>> several thousand iterations but sometimes tests ran even longer
>> without any
>> issues. Reset and reload mechanism proved helpful when we indeed hit fw
>> load failure so it is better to include this to improve driver stability.
>>
>> This change implements the following WA,
>>
>> WaEnableuKernelHeaderValidFix:skl,bxt
>> WaEnableGuCBootHashCheckNotSet:skl,bxt
>>
>> Cc: Dave Gordon <david.s.gordon at intel.com>
>> Cc: Alex Dai <yu.dai at intel.com>
>> Signed-off-by: Arun Siluvery <arun.siluvery at linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 1 +
>> drivers/gpu/drm/i915/i915_guc_reg.h | 1 +
>> drivers/gpu/drm/i915/i915_reg.h | 1 +
>> drivers/gpu/drm/i915/intel_guc_loader.c | 49
>> +++++++++++++++++++++++++++++++--
>> drivers/gpu/drm/i915/intel_uncore.c | 19 +++++++++++++
>> 5 files changed, 69 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index f37ac12..0df7c82 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2757,6 +2757,7 @@ extern long i915_compat_ioctl(struct file *filp,
>> unsigned int cmd,
>> extern int intel_gpu_reset(struct drm_device *dev);
>> extern bool intel_has_gpu_reset(struct drm_device *dev);
>> extern int i915_reset(struct drm_device *dev);
>> +extern int intel_guc_reset(struct drm_i915_private *dev_priv);
>> extern unsigned long i915_chipset_val(struct drm_i915_private
>> *dev_priv);
>> extern unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
>> extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv);
>> diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h
>> b/drivers/gpu/drm/i915/i915_guc_reg.h
>> index e4ba582..94ceee5 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_guc_reg.h
>> @@ -27,6 +27,7 @@
>> /* Definitions of GuC H/W registers, bits, etc */
>> #define GUC_STATUS _MMIO(0xc000)
>> +#define GS_MIA_IN_RESET (1 << 0)
>> #define GS_BOOTROM_SHIFT 1
>> #define GS_BOOTROM_MASK (0x7F << GS_BOOTROM_SHIFT)
>> #define GS_BOOTROM_RSA_FAILED (0x50 << GS_BOOTROM_SHIFT)
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 7dfc400..48a23de 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -164,6 +164,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t
>> reg)
>> #define GEN6_GRDOM_RENDER (1 << 1)
>> #define GEN6_GRDOM_MEDIA (1 << 2)
>> #define GEN6_GRDOM_BLT (1 << 3)
>> +#define GEN9_GRDOM_GUC (1 << 5)
>> #define RING_PP_DIR_BASE(ring) _MMIO((ring)->mmio_base+0x228)
>> #define RING_PP_DIR_BASE_READ(ring) _MMIO((ring)->mmio_base+0x518)
>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
>> b/drivers/gpu/drm/i915/intel_guc_loader.c
>> index 82a3c03..f9cb814 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> @@ -353,6 +353,24 @@ static int guc_ucode_xfer(struct drm_i915_private
>> *dev_priv)
>> return ret;
>> }
>> +static int i915_reset_guc(struct drm_i915_private *dev_priv)
>> +{
>> + int ret;
>> + u32 guc_status;
>> +
>> + ret = intel_guc_reset(dev_priv);
>> + if (ret) {
>> + DRM_ERROR("GuC reset failed, ret = %d\n", ret);
>> + return ret;
>> + }
>> +
>> + guc_status = I915_READ(GUC_STATUS);
>> + WARN(!(guc_status & GS_MIA_IN_RESET),
>> + "GuC status: 0x%x, MIA core expected to be in reset\n",
>> guc_status);
>> +
>> + return ret;
>> +}
>> +
>> /**
>> * intel_guc_ucode_load() - load GuC uCode into the device
>> * @dev: drm device
>> @@ -417,9 +435,36 @@ int intel_guc_ucode_load(struct drm_device *dev)
>> if (err)
>> goto fail;
>> + /*
>> + * WaEnableuKernelHeaderValidFix:skl,bxt
>> + * For BXT, this is only upto B0 but below WA is required for later
>> + * steppings also so this is extended as well.
>> + */
>> + /* WaEnableGuCBootHashCheckNotSet:skl,bxt */
>> err = guc_ucode_xfer(dev_priv);
>> - if (err)
>> - goto fail;
>> + if (err) {
>> + int retries = 3;
>> +
>> + DRM_ERROR("GuC fw load failed, err=%d, attempting reset and
>> retry\n", err);
>> +
>> + while (retries--) {
>> + err = i915_reset_guc(dev_priv);
>> + if (err)
>> + break;
>> +
>> + err = guc_ucode_xfer(dev_priv);
>> + if (!err) {
>> + DRM_DEBUG_DRIVER("GuC fw reload succeeded after
>> reset\n");
>> + break;
>> + }
>> + DRM_DEBUG_DRIVER("GuC fw reload retries left: %d\n",
>> retries);
>> + }
>> +
>> + if (err) {
>> + DRM_ERROR("GuC fw reload attempt failed, ret=%d\n", err);
>> + goto fail;
>> + }
>> + }
>> guc_fw->guc_fw_load_status = GUC_FIRMWARE_SUCCESS;
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
>> b/drivers/gpu/drm/i915/intel_uncore.c
>> index 80e38d5..92be76a 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -1647,6 +1647,25 @@ bool intel_has_gpu_reset(struct drm_device *dev)
>> return intel_get_gpu_reset(dev) != NULL;
>> }
>> +int intel_guc_reset(struct drm_i915_private *dev_priv)
>> +{
>> + int ret;
>> + unsigned long irqflags;
>> +
>> + if (!i915.enable_guc_submission)
>> + return -EINVAL;
>> +
>> + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>> + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>> +
>> + ret = gen6_domain_reset(dev_priv, GEN9_GRDOM_GUC);
>> +
>> + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>> + intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>> +
>> + return ret;
>> +}
>> +
>> bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv)
>> {
>> return check_for_unclaimed_mmio(dev_priv);
>
>
More information about the Intel-gfx
mailing list