Re: [PATCH v3 02/14] drm/i915/guc: Move GuC boot param initialization out of xfer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxx>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> #1
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
---
  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(&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;
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(&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;
-
-    /* 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux