[Intel-gfx] [PATCH v3 02/14] drm/i915/guc: Move GuC boot param initialization out of xfer
Sagar Arun Kamble
sagar.a.kamble at intel.com
Fri Oct 13 05:15:45 UTC 2017
Ignore checkpatch related review. Saw the fixes in 03/14
On 10/13/2017 10:43 AM, Sagar Arun Kamble wrote:
>
>
> On 10/13/2017 4:24 AM, Michal Wajdeczko wrote:
>> We want to keep ucode xfer functions separate from other
>> initialization. Once separated, add explicit forcewake.
>>
>> v2: use BLITTER domain only and add comment (Daniele)
>>
>> 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>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk> #1
>> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_guc.c | 92
>> +++++++++++++++++++++++++++++++++
>> 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, 94 insertions(+), 85 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc.c
>> b/drivers/gpu/drm/i915/intel_guc.c
>> index be1c9a7..719144a 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.c
>> +++ b/drivers/gpu/drm/i915/intel_guc.c
>> @@ -67,6 +67,98 @@ 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)
> I think renaming this as get_gt_type as suggested would be good.
>> +{
>> + /* 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;
> Unbalanced {} here. Doing this checkpatch fix now would be good.
>> +
>> + /* 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;
>> + }
>> +
>> + /*
>> + * All SOFT_SCRATCH registers are in FORCEWAKE_BLITTER domain and
>> + * they are power context saved so it's ok to release forcewake
>> + * when we are done here and take it again at xfer time.
>> + */
>> + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_BLITTER);
>> +
>> + I915_WRITE(SOFT_SCRATCH(0), 0);
>> +
>> + for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
>> + I915_WRITE(SOFT_SCRATCH(1 + i), params[i]);
> May be for later (since retry is only for Gen9) but will it be good
> idea to keep these params in intel_guc struct and just doing register
> writes during guc_init_hw.
> That way possibly we can move the params initialization to
> guc_submission_init (although that is more than submission currently)
>> +
>> + intel_uncore_forcewake_put(dev_priv, FORCEWAKE_BLITTER);
>> +}
>> +
>> 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);
>> ret = intel_guc_init_hw(&dev_priv->guc);
>> if (ret == 0 || ret != -EAGAIN)
>> break;
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list