[Intel-gfx] [P v4 02/11] drm/i915/guc: Move GuC boot param initialization out of xfer
Daniele Ceraolo Spurio
daniele.ceraolospurio at intel.com
Wed Oct 11 22:55:29 UTC 2017
On 10/10/17 07:51, Michal Wajdeczko wrote:
> We want to keep ucode xfer functions separate from other
> initialization. Once separated, add explicit forcewake.
>
> Suggested-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> ---
> drivers/gpu/drm/i915/intel_guc.c | 88 +++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_guc.h | 1 +
> drivers/gpu/drm/i915/intel_guc_loader.c | 85 -------------------------------
> drivers/gpu/drm/i915/intel_uc.c | 1 +
> 4 files changed, 90 insertions(+), 85 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index 90c3dd8..d75515c 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -67,6 +67,94 @@ void intel_guc_init_early(struct intel_guc *guc)
> guc->notify = gen8_guc_raise_irq;
> }
>
> +static u32 get_gttype(struct drm_i915_private *dev_priv)
> +{
> + /* XXX: GT type based on PCI device ID? field seems unused by fw */
> + return 0;
> +}
> +
> +static u32 get_core_family(struct drm_i915_private *dev_priv)
> +{
> + u32 gen = INTEL_GEN(dev_priv);
> +
> + switch (gen) {
> + case 9:
> + return GUC_CORE_FAMILY_GEN9;
> +
> + default:
> + MISSING_CASE(gen);
> + return GUC_CORE_FAMILY_UNKNOWN;
> + }
> +}
> +
> +/*
> + * Initialise the GuC parameter block before starting the firmware
> + * transfer. These parameters are read by the firmware on startup
> + * and cannot be changed thereafter.
> + */
> +void intel_guc_init_params(struct intel_guc *guc)
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + u32 params[GUC_CTL_MAX_DWORDS];
> + int i;
> +
> + memset(¶ms, 0, sizeof(params));
> +
> + params[GUC_CTL_DEVICE_INFO] |=
> + (get_gttype(dev_priv) << GUC_CTL_GTTYPE_SHIFT) |
> + (get_core_family(dev_priv) << GUC_CTL_COREFAMILY_SHIFT);
> +
> + /*
> + * GuC ARAT increment is 10 ns. GuC default scheduler quantum is one
> + * second. This ARAR is calculated by:
> + * Scheduler-Quantum-in-ns / ARAT-increment-in-ns = 1000000000 / 10
> + */
> + params[GUC_CTL_ARAT_HIGH] = 0;
> + params[GUC_CTL_ARAT_LOW] = 100000000;
> +
> + params[GUC_CTL_WA] |= GUC_CTL_WA_UK_BY_DRIVER;
> +
> + params[GUC_CTL_FEATURE] |= GUC_CTL_DISABLE_SCHEDULER |
> + GUC_CTL_VCS2_ENABLED;
> +
> + params[GUC_CTL_LOG_PARAMS] = guc->log.flags;
> +
> + if (i915_modparams.guc_log_level >= 0) {
> + params[GUC_CTL_DEBUG] =
> + i915_modparams.guc_log_level << GUC_LOG_VERBOSITY_SHIFT;
> + } else
> + params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED;
> +
> + /* If GuC submission is enabled, set up additional parameters here */
> + if (i915_modparams.enable_guc_submission) {
> + u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
> + u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
> + u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
> +
> + params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
> + params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED;
> +
> + pgs >>= PAGE_SHIFT;
> + params[GUC_CTL_CTXINFO] = (pgs << GUC_CTL_BASE_ADDR_SHIFT) |
> + (ctx_in_16 << GUC_CTL_CTXNUM_IN16_SHIFT);
> +
> + params[GUC_CTL_FEATURE] |= GUC_CTL_KERNEL_SUBMISSIONS;
> +
> + /* Unmask this bit to enable the GuC's internal scheduler */
> + params[GUC_CTL_FEATURE] &= ~GUC_CTL_DISABLE_SCHEDULER;
> + }
> +
> + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
I don't think we need this explicit forcewake if we use I915_WRITE(),
because that should already wake the required wells. If you want to
explicitly take forcewake then we can use I915_WRITE_FW(). Also,
FORCEWAKE_BLITTER should be enough.
I'd also add a comment to say that the register are power context saved
so it's ok to release forcewake here and take it again at xfer time.
> +
> + I915_WRITE(SOFT_SCRATCH(0), 0);
> +
> + for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
> + I915_WRITE(SOFT_SCRATCH(1 + i), params[i]);
> +
> + intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> +
> +}
> +
> int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
> {
> WARN(1, "Unexpected send: action=%#x\n", *action);
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index aa9a7b5..8b44165 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -95,6 +95,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
>
> void intel_guc_init_early(struct intel_guc *guc);
> void intel_guc_init_send_regs(struct intel_guc *guc);
> +void intel_guc_init_params(struct intel_guc *guc);
> int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
> int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
> int intel_guc_sample_forcewake(struct intel_guc *guc);
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 30b70f5..d9089bc 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -79,89 +79,6 @@ MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
> #define I915_GLK_GUC_UCODE GUC_FW_PATH(glk, GLK_FW_MAJOR, GLK_FW_MINOR)
>
>
> -static u32 get_gttype(struct drm_i915_private *dev_priv)
> -{
> - /* XXX: GT type based on PCI device ID? field seems unused by fw */
> - return 0;
> -}
> -
> -static u32 get_core_family(struct drm_i915_private *dev_priv)
> -{
> - u32 gen = INTEL_GEN(dev_priv);
> -
> - switch (gen) {
> - case 9:
> - return GUC_CORE_FAMILY_GEN9;
> -
> - default:
> - MISSING_CASE(gen);
> - return GUC_CORE_FAMILY_UNKNOWN;
> - }
> -}
> -
> -/*
> - * Initialise the GuC parameter block before starting the firmware
> - * transfer. These parameters are read by the firmware on startup
> - * and cannot be changed thereafter.
> - */
> -static void guc_params_init(struct drm_i915_private *dev_priv)
> -{
> - struct intel_guc *guc = &dev_priv->guc;
> - u32 params[GUC_CTL_MAX_DWORDS];
> - int i;
> -
> - memset(¶ms, 0, sizeof(params));
> -
> - params[GUC_CTL_DEVICE_INFO] |=
> - (get_gttype(dev_priv) << GUC_CTL_GTTYPE_SHIFT) |
> - (get_core_family(dev_priv) << GUC_CTL_COREFAMILY_SHIFT);
> -
> - /*
> - * GuC ARAT increment is 10 ns. GuC default scheduler quantum is one
> - * second. This ARAR is calculated by:
> - * Scheduler-Quantum-in-ns / ARAT-increment-in-ns = 1000000000 / 10
> - */
> - params[GUC_CTL_ARAT_HIGH] = 0;
> - params[GUC_CTL_ARAT_LOW] = 100000000;
> -
> - params[GUC_CTL_WA] |= GUC_CTL_WA_UK_BY_DRIVER;
> -
> - params[GUC_CTL_FEATURE] |= GUC_CTL_DISABLE_SCHEDULER |
> - GUC_CTL_VCS2_ENABLED;
> -
> - params[GUC_CTL_LOG_PARAMS] = guc->log.flags;
> -
> - if (i915_modparams.guc_log_level >= 0) {
> - params[GUC_CTL_DEBUG] =
> - i915_modparams.guc_log_level << GUC_LOG_VERBOSITY_SHIFT;
> - } else
> - params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED;
> -
> - /* If GuC submission is enabled, set up additional parameters here */
> - if (i915_modparams.enable_guc_submission) {
> - u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
> - u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
> - u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
> -
> - params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
> - params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED;
> -
> - pgs >>= PAGE_SHIFT;
> - params[GUC_CTL_CTXINFO] = (pgs << GUC_CTL_BASE_ADDR_SHIFT) |
> - (ctx_in_16 << GUC_CTL_CTXNUM_IN16_SHIFT);
> -
> - params[GUC_CTL_FEATURE] |= GUC_CTL_KERNEL_SUBMISSIONS;
> -
> - /* Unmask this bit to enable the GuC's internal scheduler */
> - params[GUC_CTL_FEATURE] &= ~GUC_CTL_DISABLE_SCHEDULER;
> - }
> -
> - I915_WRITE(SOFT_SCRATCH(0), 0);
> -
> - for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
> - I915_WRITE(SOFT_SCRATCH(1 + i), params[i]);
> -}
> -
> /*
> * Read the GuC status register (GUC_STATUS) and store it in the
> * specified location; then return a boolean indicating whether
> @@ -301,8 +218,6 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
> I915_WRITE(GUC_ARAT_C6DIS, 0x1FF);
> }
>
> - guc_params_init(dev_priv);
> -
> ret = guc_ucode_xfer_dma(dev_priv, vma);
>
> intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 7b938e8..53fdd9a 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -195,6 +195,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> goto err_submission;
>
> intel_huc_init_hw(&dev_priv->huc);
> + intel_guc_init_params(guc);
Is the value of the registers lost/corrupted on guc reset? if not we
could just move this outside the retry loop to avoid doing it more than
once.
Daniele
> ret = intel_guc_init_hw(&dev_priv->guc);
> if (ret == 0 || ret != -EAGAIN)
> break;
>
More information about the Intel-gfx
mailing list