Re: [PATCH v4 7/9] drm/i915/guc: Remove i915_guc_log_unregister

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

 



On Thu, 21 Sep 2017 19:40:04 +0200, Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx> wrote:



On 9/21/2017 11:01 PM, Sagar Arun Kamble wrote:


On 9/21/2017 2:28 AM, Michal Wajdeczko wrote:
On Wed, 20 Sep 2017 19:38:22 +0200, Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx> wrote:

Functionality needed to disable GuC interrupts and cleanup the
runtime/relay data structures is already covered in the unload path
via intel_guc_fini_hw and intel_guc_cleanup hence remove
i915_guc_log_unregister

v2: Removed the function i915_guc_log_unregister.

v3: Rebase as intel_guc.h is removed.

v4: Rebase as intel_guc.h is created again. :)

v5: Rebase as intel_guc.h is removed.

Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>

I'm afraid I've to revoke my r-b as with removal of the log_unregister() we will loose symmetry with log_register() where relay_late_setup_files()
was hidden, and we should still clean it up in i915_driver_unregister()

Michal
Agree with you. We will need to maintain symmetry. Will revisit with separate patches. Better to integrate in the CT series I guess as there are multiple logging related
interrupt enable/disable occurrences.
Without this patch we are getting RPM WARN which I had floated patch initally for.
https://patchwork.freedesktop.org/patch/176538/
Kindly let me know if we should get that patch in till we fix the logging/CT interrupt interaction.


Can we only call relay_close() here (without any irq/runtime cleanup,
as that will be covered in unload path) - that way we will keep
symmetry and avoid RPM issue ...

Michal

[173707.659868] RPM wakelock ref not held during HW access
[173707.659897] ------------[ cut here ]------------
[173707.659969] WARNING: CPU: 1 PID: 20769 at drivers/gpu/drm/i915/intel_drv.h:1800 fwtable_write32+0x1ad/0x1d0 [i915]
[173707.660228]  gen6_disable_pm_irq+0x47/0x50 [i915]
[173707.660269]  gen9_disable_guc_interrupts+0x33/0x60 [i915]
[173707.660332]  i915_guc_log_unregister+0x2e/0x50 [i915]
[173707.660373]  i915_driver_unload+0x4d/0x180 [i915]
[173707.660415]  i915_pci_remove+0x19/0x30 [i915]
[173707.660423]  pci_device_remove+0x39/0xc0




Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
---
 drivers/gpu/drm/i915/i915_drv.c      |  1 -
 drivers/gpu/drm/i915/intel_guc_log.c | 12 ------------
 drivers/gpu/drm/i915/intel_uc.h      |  1 -
 3 files changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6f36ced..c69a30a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1252,7 +1252,6 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
     i915_perf_unregister(dev_priv);
    i915_teardown_sysfs(dev_priv);
-    i915_guc_log_unregister(dev_priv);
     drm_dev_unregister(&dev_priv->drm);
    i915_gem_shrinker_cleanup(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 16d3b87..3c45681 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -648,15 +648,3 @@ void i915_guc_log_register(struct drm_i915_private *dev_priv)
     guc_log_late_setup(&dev_priv->guc);
     mutex_unlock(&dev_priv->drm.struct_mutex);
 }
-
-void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
-{
-    if (!i915.enable_guc_submission)
-        return;
-
-    mutex_lock(&dev_priv->drm.struct_mutex);
- /* GuC logging is currently the only user of Guc2Host interrupts */
-    gen9_disable_guc_interrupts(dev_priv);
-    guc_log_runtime_destroy(&dev_priv->guc);
-    mutex_unlock(&dev_priv->drm.struct_mutex);
-}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 8557e33..c2c104a 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -244,7 +244,6 @@ static inline void intel_guc_notify(struct intel_guc *guc)
 void intel_guc_log_destroy(struct intel_guc *guc);
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);
-void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 {

_______________________________________________
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