| Message ID | 20180308154707.21716-2-michal.winiarski@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 3/8/2018 9:16 PM, Michał Winiarski wrote: > 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> > --- > drivers/gpu/drm/i915/i915_drv.c | 6 +- > drivers/gpu/drm/i915/intel_guc_log.c | 156 ++++++++++++++--------------------- > drivers/gpu/drm/i915/intel_guc_log.h | 6 +- > drivers/gpu/drm/i915/intel_uc.c | 41 +++++---- > drivers/gpu/drm/i915/intel_uc.h | 2 + > 5 files changed, 95 insertions(+), 116 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index d7c4de45644d..987c6770d1a6 100644 > --- 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 7e59fb07b06b..90b395f34808 100644 > --- 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 @@ int intel_guc_log_relay_create(struct intel_guc *guc) > 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 @@ void intel_guc_log_relay_destroy(struct intel_guc *guc) > 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 @@ int intel_guc_log_control_set(struct intel_guc *guc, u64 val) > 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 141ce9ca22ce..09dd2ef1933d 100644 > --- 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 e5bf0d37bf43..1c1a00df010b 100644 > --- 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 f76d51d1ce70..d6af984cd789 100644 > --- 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);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index d7c4de45644d..987c6770d1a6 100644 --- 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 7e59fb07b06b..90b395f34808 100644 --- 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 @@ int intel_guc_log_relay_create(struct intel_guc *guc) 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 @@ void intel_guc_log_relay_destroy(struct intel_guc *guc) 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 @@ int intel_guc_log_control_set(struct intel_guc *guc, u64 val) 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 141ce9ca22ce..09dd2ef1933d 100644 --- 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 e5bf0d37bf43..1c1a00df010b 100644 --- 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 f76d51d1ce70..d6af984cd789 100644 --- 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);
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> --- drivers/gpu/drm/i915/i915_drv.c | 6 +- drivers/gpu/drm/i915/intel_guc_log.c | 156 ++++++++++++++--------------------- drivers/gpu/drm/i915/intel_guc_log.h | 6 +- drivers/gpu/drm/i915/intel_uc.c | 41 +++++---- drivers/gpu/drm/i915/intel_uc.h | 2 + 5 files changed, 95 insertions(+), 116 deletions(-)