On 02/09/15 23:52, yu.dai@xxxxxxxxx wrote:
From: Alex Dai <yu.dai@xxxxxxxxx> Bit 16 of GuC status indicates resuming from RC6. The LAPIC_DONE status is a reliable readiness flag only when resuming from RC6. This fix a racing issue that allocation of doorbell fails whilst GuC init is not finished. Signed-off-by: Alex Dai <yu.dai@xxxxxxxxx> --- drivers/gpu/drm/i915/i915_guc_reg.h | 1 + drivers/gpu/drm/i915/intel_guc_loader.c | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h b/drivers/gpu/drm/i915/i915_guc_reg.h index 8c8e574..dd0e1e8 100644 --- a/drivers/gpu/drm/i915/i915_guc_reg.h +++ b/drivers/gpu/drm/i915/i915_guc_reg.h @@ -37,6 +37,7 @@ #define GS_UKERNEL_READY (0xF0 << GS_UKERNEL_SHIFT) #define GS_MIA_SHIFT 16 #define GS_MIA_MASK (0x07 << GS_MIA_SHIFT) +#define GS_MIA_CORE_STATE (1 << GS_MIA_SHIFT) #define SOFT_SCRATCH(n) (0xc180 + ((n) * 4)) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 5eafd31..5823615 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -209,9 +209,10 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv, u32 *status) { u32 val = I915_READ(GUC_STATUS); + u32 uk_val = val & GS_UKERNEL_MASK; *status = val; - return ((val & GS_UKERNEL_MASK) == GS_UKERNEL_READY || - (val & GS_UKERNEL_MASK) == GS_UKERNEL_LAPIC_DONE); + return (uk_val == GS_UKERNEL_READY || + ((val & GS_MIA_CORE_STATE) && uk_val == GS_UKERNEL_LAPIC_DONE)); } /*
This does indeed fix the problem of not being able to *re*initialise the GuC correctly after a hang/reset/reload cycle :) However I think the last line looks quite confusing, especially because of the mix of bitwise and logical operators (we have &, &&, and || all in one expression).
So I think it might be clearer if spelled out in two stages: { u32 val = I915_READ(GUC_STATUS); + u32 uk_val = val & GS_UKERNEL_MASK; *status = val; - return ((val & GS_UKERNEL_MASK) == GS_UKERNEL_READY || - (val & GS_UKERNEL_MASK) == GS_UKERNEL_LAPIC_DONE); + + /* Iff resuming from RC6, look for LAPIC_DONE */ + if (val & GS_MIA_CORE_STATE) + if (uk_val == GS_UKERNEL_LAPIC_DONE) + return true; + + /* Otherwise just check whether the uKernel is READY */ + return uk_val == GS_UKERNEL_READY; } Other than that I'm quite happy with it, so either version gets a Reviewed-by: Dave Gordon <david.s.gordon@xxxxxxxxx> _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx