Re: [PATCH v13 04/21] drm/i915/guc: Remove enable_guc_submission dependency for invoking GuC log functions

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

 





On 10/11/2017 9:10 PM, Michal Wajdeczko wrote:
On Wed, 11 Oct 2017 10:53:59 +0200, Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx> wrote:

With guc_log_level parameter sanitized and GuC interrupts control
functions made self sufficient w.r.t interrupts state, we can remove
the enable_guc_submission checks from flush_guc_logs and
i915_guc_log_register/unregister and intel_uc_fini_hw.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
---
 drivers/gpu/drm/i915/intel_guc_log.c | 9 ++-------
 drivers/gpu/drm/i915/intel_uc.c      | 3 +--
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 0f201c0..fb5eb2b9 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -505,8 +505,7 @@ static void guc_flush_logs(struct intel_guc *guc)
 {
     struct drm_i915_private *dev_priv = guc_to_i915(guc);
-    if (!i915_guc_submission_enabled(guc) ||
-        i915_modparams.guc_log_level < 0)
+    if (i915_modparams.guc_log_level < 0)
         return;
    /* First disable the interrupts, will be renabled afterwards */
@@ -646,8 +645,7 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
void i915_guc_log_register(struct drm_i915_private *dev_priv)
 {
-    if (!i915_guc_submission_enabled(&dev_priv->guc) ||
-        i915_modparams.guc_log_level < 0)
+    if (i915_modparams.guc_log_level < 0)

I would expect above two fixes in patch 2/21
Since there are multiple places needing the update and for better division I had created separate patches.
Will keep same for now.

         return;
    mutex_lock(&dev_priv->drm.struct_mutex);
@@ -657,9 +655,6 @@ void i915_guc_log_register(struct drm_i915_private *dev_priv)
void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
 {
-    if (!i915_guc_submission_enabled(&dev_priv->guc))
-        return;

Hmm, as i915_guc_log_unregister() is called unconditionally by
i915_driver_unregister so maybe here we should have at least

    if (i915_modparams.guc_log_level < 0)
        return;
Yes. Will add this.

-
     mutex_lock(&dev_priv->drm.struct_mutex);
     /* GuC logging is currently the only user of Guc2Host interrupts */
     gen9_disable_guc_interrupts(dev_priv);

What about dropping this gen9_disable_guc_interrupts from here
and rely on the intel_uc_fini_hw() that will do the same?

This needs to be handled separately as if we remove it from here we will have to synchronize the logging irq
with relay runtime destruction. Will defer this for now.
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 3cf3cbd..974434e 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -280,8 +280,7 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
    guc_disable_communication(guc);
-    if (i915_guc_submission_enabled(guc))
-        gen9_disable_guc_interrupts(dev_priv);
+    gen9_disable_guc_interrupts(dev_priv);
     i915_guc_submission_fini(dev_priv);
    i915_ggtt_disable_guc(dev_priv);


_______________________________________________
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