Message ID | 20171027171555.54188-1-michal.wajdeczko@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 10/27/2017 10:45 PM, Michal Wajdeczko wrote: > 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.wajdeczko@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Sagar Arun Kamble <sagar.a.kamble@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. */ Clock gating comment is linked with below WaDisableMinuteIa*. Can you update in this patch. Better to drop with Wa name telling the intent. > + 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); > + Will it be valuable to create dma_response function for this wait? Might be able to use in HuC too. > + 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); >
On Mon, 30 Oct 2017 15:00:52 +0100, Sagar Arun Kamble <sagar.a.kamble@intel.com> wrote: > > > On 10/27/2017 10:45 PM, Michal Wajdeczko wrote: >> 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.wajdeczko@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> Cc: Sagar Arun Kamble <sagar.a.kamble@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. */ > Clock gating comment is linked with below WaDisableMinuteIa*. Can you > update in this patch. Better to drop with Wa name telling the intent. >> + 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); >> + > Will it be valuable to create dma_response function for this wait? Might > be able to use in HuC too. Not yet. And I would rather export intel_guc_dma_xfer_ucode() and do this in separate patch that will make this function also usable for HuC transfer. >> + 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 Note that intel_uc_fw_upload() takes "xfer" func pointer as parameter.. > s/guc_ucode_xfer_rsa/guc_ucode_upload_rsa guc_mmio_xfer_rsa ? > s/guc_ucode_xfer_dma/dma_upload_guc_ucode (DMA can xfer GuC/HuC) guc_dma_xfer_ucode ? >> - /* 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); >>
On 11/1/2017 7:54 PM, Michal Wajdeczko wrote: > On Mon, 30 Oct 2017 15:00:52 +0100, Sagar Arun Kamble > <sagar.a.kamble@intel.com> wrote: > >> >> >> On 10/27/2017 10:45 PM, Michal Wajdeczko wrote: >>> 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.wajdeczko@intel.com> >>> Cc: Chris Wilson <chris@chris-wilson.co.uk> >>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >>> Cc: Sagar Arun Kamble <sagar.a.kamble@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. */ >> Clock gating comment is linked with below WaDisableMinuteIa*. Can you >> update in this patch. Better to drop with Wa name telling the intent. >>> + 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); >>> + >> Will it be valuable to create dma_response function for this wait? >> Might be able to use in HuC too. > > Not yet. And I would rather export intel_guc_dma_xfer_ucode() and do this > in separate patch that will make this function also usable for HuC > transfer. Sure. > >>> + 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 > > Note that intel_uc_fw_upload() takes "xfer" func pointer as parameter.. Wanted either upload (except for xfer in "dma xfer") or xfer everywhere. Can be done in separate patch I guess. > >> s/guc_ucode_xfer_rsa/guc_ucode_upload_rsa > > guc_mmio_xfer_rsa ? This looks good. > >> s/guc_ucode_xfer_dma/dma_upload_guc_ucode (DMA can xfer GuC/HuC) > > guc_dma_xfer_ucode ? This too looks good. > >>> - /* 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); >>>
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. */ + 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); - /* 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);
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.wajdeczko@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> --- drivers/gpu/drm/i915/intel_guc_fw.c | 173 ++++++++++++++++++++++-------------- 1 file changed, 106 insertions(+), 67 deletions(-)