On 10/27/2017 10:45 PM, Michal Wajdeczko wrote:
Clock gating comment is linked with below WaDisableMinuteIa*. Can you update in this patch. Better to drop with Wa name telling the intent.Transfer of GuC firmware requires few steps that currently are implemented in two large functions. Split this code into smaller functions to make these steps small and clear. Signed-off-by: Michal Wajdeczko <michal.wajdec...@intel.com> Cc: Chris Wilson <ch...@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com> Cc: Sagar Arun Kamble <sagar.a.kam...@intel.com> --- drivers/gpu/drm/i915/intel_guc_fw.c | 173 ++++++++++++++++++++++-------------- 1 file changed, 106 insertions(+), 67 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c index ef67a36..2a10bcf 100644 --- a/drivers/gpu/drm/i915/intel_guc_fw.c +++ b/drivers/gpu/drm/i915/intel_guc_fw.c @@ -97,23 +97,53 @@ int intel_guc_fw_select(struct intel_guc *guc) return 0; }-/*- * Read the GuC status register (GUC_STATUS) and store it in the - * specified location; then return a boolean indicating whether - * the value matches either of two values representing completion - * of the GuC boot process. - * - * This is used for polling the GuC status in a wait_for() - * loop below. - */ -static inline bool guc_ucode_response(struct drm_i915_private *dev_priv, - u32 *status) +static void guc_ucode_xfer_prepare(struct drm_i915_private *dev_priv) { - u32 val = I915_READ(GUC_STATUS); - u32 uk_val = val & GS_UKERNEL_MASK; - *status = val; - return (uk_val == GS_UKERNEL_READY || - ((val & GS_MIA_CORE_STATE) && uk_val == GS_UKERNEL_LAPIC_DONE)); + /* Enable MIA caching. GuC clock gating is disabled. */
Will it be valuable to create dma_response function for this wait? Might be able to use in HuC too.+ I915_WRITE(GUC_SHIM_CONTROL, GUC_SHIM_CONTROL_VALUE); + + /* WaDisableMinuteIaClockGating:bxt */ + if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1)) { + I915_WRITE(GUC_SHIM_CONTROL, (I915_READ(GUC_SHIM_CONTROL) & + ~GUC_ENABLE_MIA_CLOCK_GATING)); + } + + /* WaC6DisallowByGfxPause:bxt */ + if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_B0)) + I915_WRITE(GEN6_GFXPAUSE, 0x30FFF); + + if (IS_GEN9_LP(dev_priv)) + I915_WRITE(GEN9LP_GT_PM_CONFIG, GT_DOORBELL_ENABLE); + else + I915_WRITE(GEN9_GT_PM_CONFIG, GT_DOORBELL_ENABLE); + + if (IS_GEN9(dev_priv)) { + /* DOP Clock Gating Enable for GuC clocks */ + I915_WRITE(GEN7_MISCCPCTL, (GEN8_DOP_CLOCK_GATE_GUC_ENABLE | + I915_READ(GEN7_MISCCPCTL))); + + /* allows for 5us (in 10ns units) before GT can go to RC6 */ + I915_WRITE(GUC_ARAT_C6DIS, 0x1FF); + } +} + +/* Copy RSA signature from the fw image to HW for verification */ +static int guc_ucode_xfer_rsa(struct intel_uc_fw *guc_fw, struct i915_vma *vma) +{ + struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw); + struct drm_i915_private *dev_priv = guc_to_i915(guc); + struct sg_table *sg = vma->pages; + u32 rsa[UOS_RSA_SCRATCH_MAX_COUNT]; + int i; + + if (sg_pcopy_to_buffer(sg->sgl, sg->nents, rsa, sizeof(rsa), + guc_fw->rsa_offset) != sizeof(rsa)) + return -EINVAL; + + for (i = 0; i < UOS_RSA_SCRATCH_MAX_COUNT; i++) + I915_WRITE(UOS_RSA_SCRATCH(i), rsa[i]); + + return 0; }/*@@ -122,29 +152,19 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv, * Architecturally, the DMA engine is bidirectional, and can potentially even * transfer between GTT locations. This functionality is left out of the API * for now as there is no need for it. - * - * Note that GuC needs the CSS header plus uKernel code to be copied by the - * DMA engine in one operation, whereas the RSA signature is loaded via MMIO. */ -static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv, - struct i915_vma *vma) +static int guc_ucode_xfer_dma(struct intel_uc_fw *guc_fw, struct i915_vma *vma) { - struct intel_uc_fw *guc_fw = &dev_priv->guc.fw; + struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw); + struct drm_i915_private *dev_priv = guc_to_i915(guc); unsigned long offset; - struct sg_table *sg = vma->pages; - u32 status, rsa[UOS_RSA_SCRATCH_MAX_COUNT]; - int i, ret = 0; - - /* where RSA signature starts */ - offset = guc_fw->rsa_offset; - - /* Copy RSA signature from the fw image to HW for verification */ - sg_pcopy_to_buffer(sg->sgl, sg->nents, rsa, sizeof(rsa), offset); - for (i = 0; i < UOS_RSA_SCRATCH_MAX_COUNT; i++) - I915_WRITE(UOS_RSA_SCRATCH(i), rsa[i]); + u32 status; + int ret;- /* The header plus uCode will be copied to WOPCM via DMA, excluding any- * other components */ + /* + * The header plus uCode will be copied to WOPCM via DMA, excluding any + * other components + */ I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size);/* Set the source address for the new blob */@@ -162,26 +182,55 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv, /* Finally start the DMA */ I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));+ /* Wait for DMA to finish */+ ret = __intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA, 0, + 2, 100, &status); + DRM_DEBUG_DRIVER("GuC DMA status %#x\n", status); +
+ return ret; +} + +/* + * Read the GuC status register (GUC_STATUS) and store it in the + * specified location; then return a boolean indicating whether + * the value matches either of two values representing completion + * of the GuC boot process. + * + * This is used for polling the GuC status in a wait_for() + * loop below. + */ +static inline bool guc_ucode_response(struct intel_guc *guc, u32 *status) +{ + struct drm_i915_private *dev_priv = guc_to_i915(guc); + u32 val = I915_READ(GUC_STATUS); + u32 uk_val = val & GS_UKERNEL_MASK; + + *status = val; + return (uk_val == GS_UKERNEL_READY) || + ((val & GS_MIA_CORE_STATE) && (uk_val == GS_UKERNEL_LAPIC_DONE)); +} + +static int guc_ucode_wait(struct intel_guc *guc) +{ + u32 status; + int ret; + /* - * Wait for the DMA to complete & the GuC to start up. + * Wait for the GuC to start up. * NB: Docs recommend not using the interrupt for completion. * Measurements indicate this should take no more than 20ms, so a * timeout here indicates that the GuC has failed and is unusable. * (Higher levels of the driver will attempt to fall back to * execlist mode if this happens.) */ - ret = wait_for(guc_ucode_response(dev_priv, &status), 100); - - DRM_DEBUG_DRIVER("DMA status 0x%x, GuC status 0x%x\n", - I915_READ(DMA_CTRL), status); + ret = wait_for(guc_ucode_response(guc, &status), 100); + DRM_DEBUG_DRIVER("GuC status %#x\n", status);if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {DRM_ERROR("GuC firmware signature verification failed\n"); ret = -ENOEXEC; }- DRM_DEBUG_DRIVER("returning %d\n", ret);- return ret; }@@ -198,34 +247,24 @@ static int guc_ucode_xfer(struct intel_uc_fw *guc_fw, struct i915_vma *vma) intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); - /* Enable MIA caching. GuC clock gating is disabled. */- I915_WRITE(GUC_SHIM_CONTROL, GUC_SHIM_CONTROL_VALUE); - - /* WaDisableMinuteIaClockGating:bxt */ - if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1)) { - I915_WRITE(GUC_SHIM_CONTROL, (I915_READ(GUC_SHIM_CONTROL) & - ~GUC_ENABLE_MIA_CLOCK_GATING)); - } - - /* WaC6DisallowByGfxPause:bxt */ - if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_B0)) - I915_WRITE(GEN6_GFXPAUSE, 0x30FFF); - - if (IS_GEN9_LP(dev_priv)) - I915_WRITE(GEN9LP_GT_PM_CONFIG, GT_DOORBELL_ENABLE); - else - I915_WRITE(GEN9_GT_PM_CONFIG, GT_DOORBELL_ENABLE); + guc_ucode_xfer_prepare(dev_priv);- if (IS_GEN9(dev_priv)) {- /* DOP Clock Gating Enable for GuC clocks */ - I915_WRITE(GEN7_MISCCPCTL, (GEN8_DOP_CLOCK_GATE_GUC_ENABLE | - I915_READ(GEN7_MISCCPCTL))); + /* + * Note that GuC needs the CSS header plus uKernel code to be copied + * by the DMA engine in one operation, whereas the RSA signature is + * loaded via MMIO. + */ + ret = guc_ucode_xfer_rsa(guc_fw, vma); + if (ret) + DRM_WARN("GuC firmware signature upload error %d\n", ret);
Unless there is a need that I am unaware of, can we rename as: s/guc_ucode_xfer/guc_ucode_upload s/guc_ucode_xfer_rsa/guc_ucode_upload_rsa s/guc_ucode_xfer_dma/dma_upload_guc_ucode (DMA can xfer GuC/HuC)
- /* allows for 5us (in 10ns units) before GT can go to RC6 */- I915_WRITE(GUC_ARAT_C6DIS, 0x1FF); - } + ret = guc_ucode_xfer_dma(guc_fw, vma); + if (ret) + DRM_WARN("GuC firmware upload error %d\n", ret);- ret = guc_ucode_xfer_dma(dev_priv, vma);+ ret = guc_ucode_wait(guc); + if (ret) + DRM_ERROR("GuC firmware error %d\n", ret);intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
_______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
