| Message ID | 20171010145135.3488-3-michal.wajdeczko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@linux.intel.com> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Sagar Arun Kamble <sagar.a.kamble@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; >
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@linux.intel.com> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Sagar Arun Kamble <sagar.a.kamble@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(¶ms, 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@chris-wilson.co.uk> -Chris
On Thu, 12 Oct 2017 10:51:58 +0200, Chris Wilson <chris@chris-wilson.co.uk> wrote: > 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@linux.intel.com> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> Cc: Sagar Arun Kamble <sagar.a.kamble@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 <snip> >> + >> + for (i = 0; i < GUC_CTL_MAX_DWORDS; i++) > > for (i = 1; i <= MAX; i++) ? > >> + I915_WRITE(SOFT_SCRATCH(1 + i), params[i]); Not so sure, as this loop is over CTL params[] not SCRATCH() regs. Michal
On Thu, 12 Oct 2017 00:55:29 +0200, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote: > > > 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. ... >> + 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 would stay with both explicit forcewake and I915_WRITE() first to be aligned with similar code in send_mmio and second to follow usage rules listed in description of I915_WRITE_FW(). > 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. > ... >> +++ 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. > In theory they should survive as they are defined as software registers. However, I'm not sure we can trust Guc (that just failed to load) that it didn't corrupt them. Also as we retry only for Gen9 cost of this extra initialization should be minimal. Thanks for the review, Michal
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); + + 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); ret = intel_guc_init_hw(&dev_priv->guc); if (ret == 0 || ret != -EAGAIN) break;
We want to keep ucode xfer functions separate from other initialization. Once separated, add explicit forcewake. Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Sagar Arun Kamble <sagar.a.kamble@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(-)