drm/i915/guc: Create common entry points for log register/unregister

We have many functions responsible for allocating different parts of
GuC log runtime called from multiple places. Let's stick with keeping
everything in guc_log_register instead.

v2: Use more generic intel_uc_register name, keep using "misc" suffix (Michał)
    s/dev_priv/i915 (Sagar)
    Make guc_log_relay_* static (sparse)

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20180308154707.21716-2-michal.winiarski@intel.com

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d7c4de45644dff066fc47dc1bc05476f668694ce..987c6770d1a69251abeed0ab45541e5c692fb175 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1238,9 +1238,11 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
        /* Reveal our presence to userspace */
        if (drm_dev_register(dev, 0) == 0) {
                i915_debugfs_register(dev_priv);
-               i915_guc_log_register(dev_priv);
                i915_setup_sysfs(dev_priv);
 
+               /* Depends on debugfs having been initialized */
+               intel_uc_register(dev_priv);
+
                /* Depends on sysfs having been initialized */
                i915_perf_register(dev_priv);
        } else
@@ -1298,7 +1300,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
        i915_pmu_unregister(dev_priv);
 
        i915_teardown_sysfs(dev_priv);
-       i915_guc_log_unregister(dev_priv);
+       intel_uc_unregister(dev_priv);
        drm_dev_unregister(&dev_priv->drm);
 
        i915_gem_shrinker_unregister(dev_priv);

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 7e59fb07b06bc1aedc72bce70848b797f41ff78e..90b395f34808147b0ff06acafdbc56febb0d8ae6 100644 (file)
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -443,7 +443,7 @@ void intel_guc_log_init_early(struct intel_guc *guc)
        INIT_WORK(&guc->log.runtime.flush_work, capture_logs_work);
 }
 
-int intel_guc_log_relay_create(struct intel_guc *guc)
+static int guc_log_relay_create(struct intel_guc *guc)
 {
        struct drm_i915_private *dev_priv = guc_to_i915(guc);
        struct rchan *guc_log_relay_chan;
@@ -496,7 +496,7 @@ err:
        return ret;
 }
 
-void intel_guc_log_relay_destroy(struct intel_guc *guc)
+static void guc_log_relay_destroy(struct intel_guc *guc)
 {
        mutex_lock(&guc->log.runtime.relay_lock);
 
@@ -514,49 +514,6 @@ out_unlock:
        mutex_unlock(&guc->log.runtime.relay_lock);
 }
 
-static int guc_log_late_setup(struct intel_guc *guc)
-{
-       struct drm_i915_private *dev_priv = guc_to_i915(guc);
-       int ret;
-
-       if (!guc_log_has_runtime(guc)) {
-               /*
-                * If log was disabled at boot time, then setup needed to handle
-                * log buffer flush interrupts would not have been done yet, so
-                * do that now.
-                */
-               ret = intel_guc_log_relay_create(guc);
-               if (ret)
-                       goto err;
-
-               mutex_lock(&dev_priv->drm.struct_mutex);
-               intel_runtime_pm_get(dev_priv);
-               ret = guc_log_runtime_create(guc);
-               intel_runtime_pm_put(dev_priv);
-               mutex_unlock(&dev_priv->drm.struct_mutex);
-
-               if (ret)
-                       goto err_relay;
-       }
-
-       ret = guc_log_relay_file_create(guc);
-       if (ret)
-               goto err_runtime;
-
-       return 0;
-
-err_runtime:
-       mutex_lock(&dev_priv->drm.struct_mutex);
-       guc_log_runtime_destroy(guc);
-       mutex_unlock(&dev_priv->drm.struct_mutex);
-err_relay:
-       intel_guc_log_relay_destroy(guc);
-err:
-       /* logging will remain off */
-       i915_modparams.guc_log_level = 0;
-       return ret;
-}
-
 static void guc_log_capture_logs(struct intel_guc *guc)
 {
        struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -576,16 +533,6 @@ static void guc_flush_logs(struct intel_guc *guc)
 {
        struct drm_i915_private *dev_priv = guc_to_i915(guc);
 
-       if (!USES_GUC_SUBMISSION(dev_priv) || !i915_modparams.guc_log_level)
-               return;
-
-       /* First disable the interrupts, will be renabled afterwards */
-       mutex_lock(&dev_priv->drm.struct_mutex);
-       intel_runtime_pm_get(dev_priv);
-       gen9_disable_guc_interrupts(dev_priv);
-       intel_runtime_pm_put(dev_priv);
-       mutex_unlock(&dev_priv->drm.struct_mutex);
-
        /*
         * Before initiating the forceful flush, wait for any pending/ongoing
         * flush to complete otherwise forceful flush may not actually happen.
@@ -628,12 +575,6 @@ int intel_guc_log_create(struct intel_guc *guc)
 
        guc->log.vma = vma;
 
-       if (i915_modparams.guc_log_level) {
-               ret = guc_log_runtime_create(guc);
-               if (ret < 0)
-                       goto err_vma;
-       }
-
        /* each allocated unit is a page */
        flags = GUC_LOG_VALID | GUC_LOG_NOTIFY_ON_HALF_FULL |
                (GUC_LOG_DPC_PAGES << GUC_LOG_DPC_SHIFT) |
@@ -645,8 +586,6 @@ int intel_guc_log_create(struct intel_guc *guc)
 
        return 0;
 
-err_vma:
-       i915_vma_unpin_and_release(&guc->log.vma);
 err:
        /* logging will be off */
        i915_modparams.guc_log_level = 0;
@@ -712,26 +651,14 @@ int intel_guc_log_control_set(struct intel_guc *guc, u64 val)
        mutex_unlock(&dev_priv->drm.struct_mutex);
 
        if (enabled && !guc_log_has_runtime(guc)) {
-               ret = guc_log_late_setup(guc);
+               ret = intel_guc_log_register(guc);
                if (ret) {
-                       DRM_DEBUG_DRIVER("GuC log late setup failed %d\n", ret);
+                       /* logging will remain off */
+                       i915_modparams.guc_log_level = 0;
                        goto out;
                }
-
-               /* GuC logging is currently the only user of Guc2Host interrupts */
-               mutex_lock(&dev_priv->drm.struct_mutex);
-               intel_runtime_pm_get(dev_priv);
-               gen9_enable_guc_interrupts(dev_priv);
-               intel_runtime_pm_put(dev_priv);
-               mutex_unlock(&dev_priv->drm.struct_mutex);
        } else if (!enabled && guc_log_has_runtime(guc)) {
-               /*
-                * Once logging is disabled, GuC won't generate logs & send an
-                * interrupt. But there could be some data in the log buffer
-                * which is yet to be captured. So request GuC to update the log
-                * buffer state and then collect the left over logs.
-                */
-               guc_flush_logs(guc);
+               intel_guc_log_unregister(guc);
        }
 
        return 0;
@@ -742,29 +669,72 @@ out:
        return ret;
 }
 
-void i915_guc_log_register(struct drm_i915_private *dev_priv)
+int intel_guc_log_register(struct intel_guc *guc)
 {
-       if (!USES_GUC_SUBMISSION(dev_priv) || !i915_modparams.guc_log_level)
-               return;
+       struct drm_i915_private *i915 = guc_to_i915(guc);
+       int ret;
+
+       GEM_BUG_ON(guc_log_has_runtime(guc));
+
+       /*
+        * If log was disabled at boot time, then setup needed to handle
+        * log buffer flush interrupts would not have been done yet, so
+        * do that now.
+        */
+       ret = guc_log_relay_create(guc);
+       if (ret)
+               goto err;
+
+       mutex_lock(&i915->drm.struct_mutex);
+       ret = guc_log_runtime_create(guc);
+       mutex_unlock(&i915->drm.struct_mutex);
+
+       if (ret)
+               goto err_relay;
+
+       ret = guc_log_relay_file_create(guc);
+       if (ret)
+               goto err_runtime;
+
+       /* GuC logging is currently the only user of Guc2Host interrupts */
+       mutex_lock(&i915->drm.struct_mutex);
+       intel_runtime_pm_get(i915);
+       gen9_enable_guc_interrupts(i915);
+       intel_runtime_pm_put(i915);
+       mutex_unlock(&i915->drm.struct_mutex);
+
+       return 0;
 
-       guc_log_late_setup(&dev_priv->guc);
+err_runtime:
+       mutex_lock(&i915->drm.struct_mutex);
+       guc_log_runtime_destroy(guc);
+       mutex_unlock(&i915->drm.struct_mutex);
+err_relay:
+       guc_log_relay_destroy(guc);
+err:
+       return ret;
 }
 
-void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
+void intel_guc_log_unregister(struct intel_guc *guc)
 {
-       struct intel_guc *guc = &dev_priv->guc;
+       struct drm_i915_private *i915 = guc_to_i915(guc);
 
-       if (!USES_GUC_SUBMISSION(dev_priv))
-               return;
+       /*
+        * Once logging is disabled, GuC won't generate logs & send an
+        * interrupt. But there could be some data in the log buffer
+        * which is yet to be captured. So request GuC to update the log
+        * buffer state and then collect the left over logs.
+        */
+       guc_flush_logs(guc);
 
-       mutex_lock(&dev_priv->drm.struct_mutex);
+       mutex_lock(&i915->drm.struct_mutex);
        /* GuC logging is currently the only user of Guc2Host interrupts */
-       intel_runtime_pm_get(dev_priv);
-       gen9_disable_guc_interrupts(dev_priv);
-       intel_runtime_pm_put(dev_priv);
+       intel_runtime_pm_get(i915);
+       gen9_disable_guc_interrupts(i915);
+       intel_runtime_pm_put(i915);
 
        guc_log_runtime_destroy(guc);
-       mutex_unlock(&dev_priv->drm.struct_mutex);
+       mutex_unlock(&i915->drm.struct_mutex);
 
-       intel_guc_log_relay_destroy(guc);
+       guc_log_relay_destroy(guc);
 }

diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h
index 141ce9ca22ce80691900d70a656b6fe2ea795cf5..09dd2ef1933d23887054d060ec40559d532c9cd3 100644 (file)
--- a/drivers/gpu/drm/i915/intel_guc_log.h
+++ b/drivers/gpu/drm/i915/intel_guc_log.h
@@ -62,11 +62,9 @@ struct intel_guc_log {
 int intel_guc_log_create(struct intel_guc *guc);
 void intel_guc_log_destroy(struct intel_guc *guc);
 void intel_guc_log_init_early(struct intel_guc *guc);
-int intel_guc_log_relay_create(struct intel_guc *guc);
-void intel_guc_log_relay_destroy(struct intel_guc *guc);
 int intel_guc_log_control_get(struct intel_guc *guc);
 int intel_guc_log_control_set(struct intel_guc *guc, u64 control_val);
-void i915_guc_log_register(struct drm_i915_private *dev_priv);
-void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
+int intel_guc_log_register(struct intel_guc *guc);
+void intel_guc_log_unregister(struct intel_guc *guc);
 
 #endif

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index e5bf0d37bf438931ccdb15d6ae218f715722e668..1c1a00df010bc37804f804e39b6d715190faa5c6 100644 (file)
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -219,6 +219,28 @@ static void guc_free_load_err_log(struct intel_guc *guc)
                i915_gem_object_put(guc->load_err_log);
 }
 
+int intel_uc_register(struct drm_i915_private *i915)
+{
+       int ret = 0;
+
+       if (!USES_GUC(i915))
+               return 0;
+
+       if (i915_modparams.guc_log_level)
+               ret = intel_guc_log_register(&i915->guc);
+
+       return ret;
+}
+
+void intel_uc_unregister(struct drm_i915_private *i915)
+{
+       if (!USES_GUC(i915))
+               return;
+
+       if (i915_modparams.guc_log_level)
+               intel_guc_log_unregister(&i915->guc);
+}
+
 static int guc_enable_communication(struct intel_guc *guc)
 {
        struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -249,23 +271,10 @@ int intel_uc_init_misc(struct drm_i915_private *dev_priv)
                return 0;
 
        ret = intel_guc_init_wq(guc);
-       if (ret) {
-               DRM_ERROR("Couldn't allocate workqueues for GuC\n");
-               goto err;
-       }
-
-       ret = intel_guc_log_relay_create(guc);
-       if (ret) {
-               DRM_ERROR("Couldn't allocate relay for GuC log\n");
-               goto err_relay;
-       }
+       if (ret)
+               return ret;
 
        return 0;
-
-err_relay:
-       intel_guc_fini_wq(guc);
-err:
-       return ret;
 }
 
 void intel_uc_fini_misc(struct drm_i915_private *dev_priv)
@@ -276,8 +285,6 @@ void intel_uc_fini_misc(struct drm_i915_private *dev_priv)
                return;
 
        intel_guc_fini_wq(guc);
-
-       intel_guc_log_relay_destroy(guc);
 }
 
 int intel_uc_init(struct drm_i915_private *dev_priv)

diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index f76d51d1ce70958ca12e23cc64e552e0560f508f..d6af984cd78972bf6170e64d6b84a9855e12dd74 100644 (file)
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -31,6 +31,8 @@
 void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
 void intel_uc_init_early(struct drm_i915_private *dev_priv);
 void intel_uc_init_mmio(struct drm_i915_private *dev_priv);
+int intel_uc_register(struct drm_i915_private *dev_priv);
+void intel_uc_unregister(struct drm_i915_private *dev_priv);
 void intel_uc_init_fw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_misc(struct drm_i915_private *dev_priv);