[Intel-gfx] [P v4 02/11] drm/i915/guc: Move GuC boot param initialization out of xfer

[Chris Wilson]

Quoting Michal Wajdeczko (2017-10-10 15:51:26)
> 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)
> +{

get_gt_type()

I read it as gtt_type and wondered about the mispelling.

> +       /* 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;
> +       }

Ok.

> +}
> +
> +/*
> + * 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(&params, 0, sizeof(params));

memset(params, 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;

Where there is one {, there is all. } else { ... }

> +
> +       /* 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);
> +
> +       I915_WRITE(SOFT_SCRATCH(0), 0);
> +
> +       for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)

for (i = 1; i <= MAX; i++) ?

> +               I915_WRITE(SOFT_SCRATCH(1 + i), params[i]);
> +
> +       intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> +
> +}

Ok, this is just motion, so disregard the changes above (except make it
checkpatch clean?) in this patch.
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris