* [PATCH v4 2/6] drm/i915/guc: Enable interrupts before resuming GuC during runtime resume
2018-01-24 15:46 [PATCH v4 1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts Sagar Arun Kamble
@ 2018-01-24 15:46 ` Sagar Arun Kamble
2018-01-24 15:46 ` [PATCH v4 3/6] drm/i915/guc: Fix lockdep due to log relay channel handling under struct_mutex Sagar Arun Kamble
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Sagar Arun Kamble @ 2018-01-24 15:46 UTC (permalink / raw)
To: intel-gfx
GuC log streaming needs interrupts enabled prior to GuC resume but
runtime pm interrupt setup was happening post GuC resume. Fix it.
While at it, fix the unwinding of steps in the runtime suspend path.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104695
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Marta Lofstedt <marta.lofstedt@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_drv.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1fbe378..95e1c16 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2600,6 +2600,11 @@ static int intel_runtime_suspend(struct device *kdev)
intel_runtime_pm_enable_interrupts(dev_priv);
+ intel_guc_resume(dev_priv);
+
+ i915_gem_init_swizzling(dev_priv);
+ i915_gem_restore_fences(dev_priv);
+
enable_rpm_wakeref_asserts(dev_priv);
return ret;
@@ -2665,8 +2670,6 @@ static int intel_runtime_resume(struct device *kdev)
if (intel_uncore_unclaimed_mmio(dev_priv))
DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
- intel_guc_resume(dev_priv);
-
if (IS_GEN9_LP(dev_priv)) {
bxt_disable_dc9(dev_priv);
bxt_display_core_init(dev_priv, true);
@@ -2681,6 +2684,10 @@ static int intel_runtime_resume(struct device *kdev)
intel_uncore_runtime_resume(dev_priv);
+ intel_runtime_pm_enable_interrupts(dev_priv);
+
+ intel_guc_resume(dev_priv);
+
/*
* No point of rolling back things in case of an error, as the best
* we can do is to hope that things will still work (and disable RPM).
@@ -2688,8 +2695,6 @@ static int intel_runtime_resume(struct device *kdev)
i915_gem_init_swizzling(dev_priv);
i915_gem_restore_fences(dev_priv);
- intel_runtime_pm_enable_interrupts(dev_priv);
-
/*
* On VLV/CHV display interrupts are part of the display
* power well, so hpd is reinitialized from there. For
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v4 3/6] drm/i915/guc: Fix lockdep due to log relay channel handling under struct_mutex
2018-01-24 15:46 [PATCH v4 1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts Sagar Arun Kamble
2018-01-24 15:46 ` [PATCH v4 2/6] drm/i915/guc: Enable interrupts before resuming GuC during runtime resume Sagar Arun Kamble
@ 2018-01-24 15:46 ` Sagar Arun Kamble
2018-01-24 19:40 ` Chris Wilson
2018-01-24 15:46 ` [PATCH v4 4/6] drm/i915/guc: Update name and prototype of i915_guc_log_control Sagar Arun Kamble
` (4 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Sagar Arun Kamble @ 2018-01-24 15:46 UTC (permalink / raw)
To: intel-gfx
This patch fixes lockdep issue due to circular locking dependency of
struct_mutex, i_mutex_key, mmap_sem, relay_channels_mutex.
For GuC log relay channel we create debugfs file that requires i_mutex_key
lock and we are doing that under struct_mutex. So we introduced newer
dependency as:
&dev->struct_mutex --> &sb->s_type->i_mutex_key#3 --> &mm->mmap_sem
However, there is dependency from mmap_sem to struct_mutex. Hence we
separate the relay create/destroy operation from under struct_mutex.
Also added runtime check of relay buffer status.
======================================================
WARNING: possible circular locking dependency detected
4.15.0-rc6-CI-Patchwork_7614+ #1 Not tainted
------------------------------------------------------
debugfs_test/1388 is trying to acquire lock:
(&dev->struct_mutex){+.+.}, at: [<00000000d5e1d915>] i915_mutex_lock_interruptible+0x47/0x130 [i915]
but task is already holding lock:
(&mm->mmap_sem){++++}, at: [<0000000029a9c131>] __do_page_fault+0x106/0x560
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #3 (&mm->mmap_sem){++++}:
_copy_to_user+0x1e/0x70
filldir+0x8c/0xf0
dcache_readdir+0xeb/0x160
iterate_dir+0xdc/0x140
SyS_getdents+0xa0/0x130
entry_SYSCALL_64_fastpath+0x1c/0x89
-> #2 (&sb->s_type->i_mutex_key#3){++++}:
start_creating+0x59/0x110
__debugfs_create_file+0x2e/0xe0
relay_create_buf_file+0x62/0x80
relay_late_setup_files+0x84/0x250
guc_log_late_setup+0x4f/0x110 [i915]
i915_guc_log_register+0x32/0x40 [i915]
i915_driver_load+0x7b6/0x1720 [i915]
i915_pci_probe+0x2e/0x90 [i915]
pci_device_probe+0x9c/0x120
driver_probe_device+0x2a3/0x480
__driver_attach+0xd9/0xe0
bus_for_each_dev+0x57/0x90
bus_add_driver+0x168/0x260
driver_register+0x52/0xc0
do_one_initcall+0x39/0x150
do_init_module+0x56/0x1ef
load_module+0x231c/0x2d70
SyS_finit_module+0xa5/0xe0
entry_SYSCALL_64_fastpath+0x1c/0x89
-> #1 (relay_channels_mutex){+.+.}:
relay_open+0x12c/0x2b0
intel_guc_log_runtime_create+0xab/0x230 [i915]
intel_guc_init+0x81/0x120 [i915]
intel_uc_init+0x29/0xa0 [i915]
i915_gem_init+0x182/0x530 [i915]
i915_driver_load+0xaa9/0x1720 [i915]
i915_pci_probe+0x2e/0x90 [i915]
pci_device_probe+0x9c/0x120
driver_probe_device+0x2a3/0x480
__driver_attach+0xd9/0xe0
bus_for_each_dev+0x57/0x90
bus_add_driver+0x168/0x260
driver_register+0x52/0xc0
do_one_initcall+0x39/0x150
do_init_module+0x56/0x1ef
load_module+0x231c/0x2d70
SyS_finit_module+0xa5/0xe0
entry_SYSCALL_64_fastpath+0x1c/0x89
-> #0 (&dev->struct_mutex){+.+.}:
__mutex_lock+0x81/0x9b0
i915_mutex_lock_interruptible+0x47/0x130 [i915]
i915_gem_fault+0x201/0x790 [i915]
__do_fault+0x15/0x70
__handle_mm_fault+0x677/0xdc0
handle_mm_fault+0x14f/0x2f0
__do_page_fault+0x2d1/0x560
page_fault+0x4c/0x60
other info that might help us debug this:
Chain exists of:
&dev->struct_mutex --> &sb->s_type->i_mutex_key#3 --> &mm->mmap_sem
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&mm->mmap_sem);
lock(&sb->s_type->i_mutex_key#3);
lock(&mm->mmap_sem);
lock(&dev->struct_mutex);
*** DEADLOCK ***
1 lock held by debugfs_test/1388:
#0: (&mm->mmap_sem){++++}, at: [<0000000029a9c131>] __do_page_fault+0x106/0x560
stack backtrace:
CPU: 2 PID: 1388 Comm: debugfs_test Not tainted 4.15.0-rc6-CI-Patchwork_7614+ #1
Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./J4205-ITX, BIOS P1.10 09/29/2016
Call Trace:
dump_stack+0x5f/0x86
print_circular_bug.isra.18+0x1d0/0x2c0
__lock_acquire+0x14ae/0x1b60
? lock_acquire+0xaf/0x200
lock_acquire+0xaf/0x200
? i915_mutex_lock_interruptible+0x47/0x130 [i915]
__mutex_lock+0x81/0x9b0
? i915_mutex_lock_interruptible+0x47/0x130 [i915]
? i915_mutex_lock_interruptible+0x47/0x130 [i915]
? i915_mutex_lock_interruptible+0x47/0x130 [i915]
i915_mutex_lock_interruptible+0x47/0x130 [i915]
? __pm_runtime_resume+0x4f/0x80
i915_gem_fault+0x201/0x790 [i915]
__do_fault+0x15/0x70
? _raw_spin_unlock+0x29/0x40
__handle_mm_fault+0x677/0xdc0
handle_mm_fault+0x14f/0x2f0
__do_page_fault+0x2d1/0x560
? page_fault+0x36/0x60
page_fault+0x4c/0x60
v2: Added lock protection to guc->log.runtime.relay_chan (Chris)
Fixed locking inside guc_flush_logs uncovered by new lockdep.
v3: Locking guc_read_update_log_buffer entirely with relay_lock. (Chris)
Prepared intel_guc_init_early. Moved relay_lock inside relay_create
relay_destroy, relay_file_create, guc_read_update_log_buffer. (Michal)
Removed struct_mutex lock around guc_log_flush and removed usage
of guc_log_has_relay() from runtime_create path as it needs
struct_mutex lock.
v4: Handle NULL relay sub buffer pointer earlier in read_update_log_buffer
(Chris). Fixed comment suffix **/. (Michal)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104693
Testcase: igt/debugfs_test/read_all_entries # with enable_guc=1 and guc_log_level=1
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Marta Lofstedt <marta.lofstedt@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 12 +--
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 4 +-
drivers/gpu/drm/i915/intel_guc.c | 7 +-
drivers/gpu/drm/i915/intel_guc_log.c | 171 ++++++++++++++++++++++++++---------
drivers/gpu/drm/i915/intel_guc_log.h | 12 +++
drivers/gpu/drm/i915/intel_uc.c | 26 +++++-
drivers/gpu/drm/i915/intel_uc.h | 4 +-
8 files changed, 172 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 80dc679..b45be0d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2467,7 +2467,6 @@ static int i915_guc_log_control_get(void *data, u64 *val)
static int i915_guc_log_control_set(void *data, u64 val)
{
struct drm_i915_private *dev_priv = data;
- int ret;
if (!HAS_GUC(dev_priv))
return -ENODEV;
@@ -2475,16 +2474,7 @@ static int i915_guc_log_control_set(void *data, u64 val)
if (!dev_priv->guc.log.vma)
return -EINVAL;
- ret = mutex_lock_interruptible(&dev_priv->drm.struct_mutex);
- if (ret)
- return ret;
-
- intel_runtime_pm_get(dev_priv);
- ret = i915_guc_log_control(dev_priv, val);
- intel_runtime_pm_put(dev_priv);
-
- mutex_unlock(&dev_priv->drm.struct_mutex);
- return ret;
+ return i915_guc_log_control(dev_priv, val);
}
DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_control_fops,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 95e1c16..1ec12ad 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -626,7 +626,7 @@ static void i915_gem_fini(struct drm_i915_private *dev_priv)
i915_gem_contexts_fini(dev_priv);
mutex_unlock(&dev_priv->drm.struct_mutex);
- intel_uc_fini_wq(dev_priv);
+ intel_uc_fini_misc(dev_priv);
i915_gem_cleanup_userptr(dev_priv);
i915_gem_drain_freed_objects(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7f0684c..34d0d51 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5193,7 +5193,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
if (ret)
return ret;
- ret = intel_uc_init_wq(dev_priv);
+ ret = intel_uc_init_misc(dev_priv);
if (ret)
return ret;
@@ -5289,7 +5289,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
mutex_unlock(&dev_priv->drm.struct_mutex);
- intel_uc_fini_wq(dev_priv);
+ intel_uc_fini_misc(dev_priv);
if (ret != -EIO)
i915_gem_cleanup_userptr(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index ea30e7c..21140cc 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -64,6 +64,7 @@ void intel_guc_init_early(struct intel_guc *guc)
{
intel_guc_fw_init_early(guc);
intel_guc_ct_init_early(&guc->ct);
+ intel_guc_log_init_early(guc);
mutex_init(&guc->send_mutex);
guc->send = intel_guc_send_nop;
@@ -87,8 +88,10 @@ int intel_guc_init_wq(struct intel_guc *guc)
*/
guc->log.runtime.flush_wq = alloc_ordered_workqueue("i915-guc_log",
WQ_HIGHPRI | WQ_FREEZABLE);
- if (!guc->log.runtime.flush_wq)
+ if (!guc->log.runtime.flush_wq) {
+ DRM_ERROR("Couldn't allocate workqueue for GuC log\n");
return -ENOMEM;
+ }
/*
* Even though both sending GuC action, and adding a new workitem to
@@ -109,6 +112,8 @@ int intel_guc_init_wq(struct intel_guc *guc)
WQ_HIGHPRI);
if (!guc->preempt_wq) {
destroy_workqueue(guc->log.runtime.flush_wq);
+ DRM_ERROR("Couldn't allocate workqueue for GuC "
+ "preemption\n");
return -ENOMEM;
}
}
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 8f2da30..d10ec53 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -153,6 +153,8 @@ static int guc_log_relay_file_create(struct intel_guc *guc)
if (!i915_modparams.guc_log_level)
return 0;
+ mutex_lock(&guc->log.runtime.relay_lock);
+
/* For now create the log file in /sys/kernel/debug/dri/0 dir */
log_dir = dev_priv->drm.primary->debugfs_root;
@@ -169,16 +171,26 @@ static int guc_log_relay_file_create(struct intel_guc *guc)
*/
if (!log_dir) {
DRM_ERROR("Debugfs dir not available yet for GuC log file\n");
- return -ENODEV;
+ ret = -ENODEV;
+ goto out_unlock;
}
ret = relay_late_setup_files(guc->log.runtime.relay_chan, "guc_log", log_dir);
if (ret < 0 && ret != -EEXIST) {
DRM_ERROR("Couldn't associate relay chan with file %d\n", ret);
- return ret;
+ goto out_unlock;
}
- return 0;
+out_unlock:
+ mutex_unlock(&guc->log.runtime.relay_lock);
+ return ret;
+}
+
+static bool guc_log_has_relay(struct intel_guc *guc)
+{
+ lockdep_assert_held(&guc->log.runtime.relay_lock);
+
+ return guc->log.runtime.relay_chan != NULL;
}
static void guc_move_to_next_buf(struct intel_guc *guc)
@@ -188,6 +200,9 @@ static void guc_move_to_next_buf(struct intel_guc *guc)
*/
smp_wmb();
+ if (!guc_log_has_relay(guc))
+ return;
+
/* All data has been written, so now move the offset of sub buffer. */
relay_reserve(guc->log.runtime.relay_chan, guc->log.vma->obj->base.size);
@@ -197,7 +212,7 @@ static void guc_move_to_next_buf(struct intel_guc *guc)
static void *guc_get_write_buffer(struct intel_guc *guc)
{
- if (!guc->log.runtime.relay_chan)
+ if (!guc_log_has_relay(guc))
return NULL;
/* Just get the base address of a new sub buffer and copy data into it
@@ -265,9 +280,22 @@ static void guc_read_update_log_buffer(struct intel_guc *guc)
/* Get the pointer to shared GuC log buffer */
log_buf_state = src_data = guc->log.runtime.buf_addr;
+ mutex_lock(&guc->log.runtime.relay_lock);
+
/* Get the pointer to local buffer to store the logs */
log_buf_snapshot_state = dst_data = guc_get_write_buffer(guc);
+ if (unlikely(!log_buf_snapshot_state)) {
+ /* Used rate limited to avoid deluge of messages, logs might be
+ * getting consumed by User at a slow rate.
+ */
+ DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n");
+ guc->log.capture_miss_count++;
+ mutex_unlock(&guc->log.runtime.relay_lock);
+
+ return;
+ }
+
/* Actual logs are present from the 2nd page */
src_data += PAGE_SIZE;
dst_data += PAGE_SIZE;
@@ -293,9 +321,6 @@ static void guc_read_update_log_buffer(struct intel_guc *guc)
log_buf_state->flush_to_file = 0;
log_buf_state++;
- if (unlikely(!log_buf_snapshot_state))
- continue;
-
/* First copy the state structure in snapshot buffer */
memcpy(log_buf_snapshot_state, &log_buf_state_local,
sizeof(struct guc_log_buffer_state));
@@ -335,15 +360,9 @@ static void guc_read_update_log_buffer(struct intel_guc *guc)
dst_data += buffer_size;
}
- if (log_buf_snapshot_state)
- guc_move_to_next_buf(guc);
- else {
- /* Used rate limited to avoid deluge of messages, logs might be
- * getting consumed by User at a slow rate.
- */
- DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n");
- guc->log.capture_miss_count++;
- }
+ guc_move_to_next_buf(guc);
+
+ mutex_unlock(&guc->log.runtime.relay_lock);
}
static void capture_logs_work(struct work_struct *work)
@@ -363,8 +382,6 @@ static int guc_log_runtime_create(struct intel_guc *guc)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
void *vaddr;
- struct rchan *guc_log_relay_chan;
- size_t n_subbufs, subbuf_size;
int ret;
lockdep_assert_held(&dev_priv->drm.struct_mutex);
@@ -387,8 +404,44 @@ static int guc_log_runtime_create(struct intel_guc *guc)
guc->log.runtime.buf_addr = vaddr;
+ return 0;
+}
+
+static void guc_log_runtime_destroy(struct intel_guc *guc)
+{
+ /*
+ * It's possible that the runtime stuff was never allocated because
+ * GuC log was disabled at the boot time.
+ */
+ if (!guc_log_has_runtime(guc))
+ return;
+
+ i915_gem_object_unpin_map(guc->log.vma->obj);
+ guc->log.runtime.buf_addr = NULL;
+}
+
+void intel_guc_log_init_early(struct intel_guc *guc)
+{
+ mutex_init(&guc->log.runtime.relay_lock);
+ INIT_WORK(&guc->log.runtime.flush_work, capture_logs_work);
+}
+
+int intel_guc_log_relay_create(struct intel_guc *guc)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ struct rchan *guc_log_relay_chan;
+ size_t n_subbufs, subbuf_size;
+ int ret;
+
+ if (!i915_modparams.guc_log_level)
+ return 0;
+
+ mutex_lock(&guc->log.runtime.relay_lock);
+
+ GEM_BUG_ON(guc_log_has_relay(guc));
+
/* Keep the size of sub buffers same as shared log buffer */
- subbuf_size = guc->log.vma->obj->base.size;
+ subbuf_size = GUC_LOG_SIZE;
/* Store up to 8 snapshots, which is large enough to buffer sufficient
* boot time logs and provides enough leeway to User, in terms of
@@ -407,33 +460,39 @@ static int guc_log_runtime_create(struct intel_guc *guc)
DRM_ERROR("Couldn't create relay chan for GuC logging\n");
ret = -ENOMEM;
- goto err_vaddr;
+ goto err;
}
GEM_BUG_ON(guc_log_relay_chan->subbuf_size < subbuf_size);
guc->log.runtime.relay_chan = guc_log_relay_chan;
- INIT_WORK(&guc->log.runtime.flush_work, capture_logs_work);
+ mutex_unlock(&guc->log.runtime.relay_lock);
+
return 0;
-err_vaddr:
- i915_gem_object_unpin_map(guc->log.vma->obj);
- guc->log.runtime.buf_addr = NULL;
+err:
+ mutex_unlock(&guc->log.runtime.relay_lock);
+ /* logging will be off */
+ i915_modparams.guc_log_level = 0;
return ret;
}
-static void guc_log_runtime_destroy(struct intel_guc *guc)
+void intel_guc_log_relay_destroy(struct intel_guc *guc)
{
+ mutex_lock(&guc->log.runtime.relay_lock);
+
/*
- * It's possible that the runtime stuff was never allocated because
+ * It's possible that the relay was never allocated because
* GuC log was disabled at the boot time.
*/
- if (!guc_log_has_runtime(guc))
- return;
+ if (!guc_log_has_relay(guc))
+ goto out_unlock;
relay_close(guc->log.runtime.relay_chan);
- i915_gem_object_unpin_map(guc->log.vma->obj);
- guc->log.runtime.buf_addr = NULL;
+ guc->log.runtime.relay_chan = NULL;
+
+out_unlock:
+ mutex_unlock(&guc->log.runtime.relay_lock);
}
static int guc_log_late_setup(struct intel_guc *guc)
@@ -441,17 +500,24 @@ static int guc_log_late_setup(struct intel_guc *guc)
struct drm_i915_private *dev_priv = guc_to_i915(guc);
int ret;
- lockdep_assert_held(&dev_priv->drm.struct_mutex);
-
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 = guc_log_runtime_create(guc);
+ 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);
@@ -461,7 +527,11 @@ static int guc_log_late_setup(struct intel_guc *guc)
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;
@@ -490,7 +560,11 @@ static void guc_flush_logs(struct intel_guc *guc)
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.
@@ -498,7 +572,9 @@ static void guc_flush_logs(struct intel_guc *guc)
flush_work(&guc->log.runtime.flush_work);
/* Ask GuC to update the log buffer state */
+ intel_runtime_pm_get(dev_priv);
guc_log_flush(guc);
+ intel_runtime_pm_put(dev_priv);
/* GuC would have updated log buffer by now, so capture it */
guc_log_capture_logs(guc);
@@ -509,17 +585,10 @@ int intel_guc_log_create(struct intel_guc *guc)
struct i915_vma *vma;
unsigned long offset;
u32 flags;
- u32 size;
int ret;
GEM_BUG_ON(guc->log.vma);
- /* The first page is to save log buffer state. Allocate one
- * extra page for others in case for overlap */
- size = (1 + GUC_LOG_DPC_PAGES + 1 +
- GUC_LOG_ISR_PAGES + 1 +
- GUC_LOG_CRASH_PAGES + 1) << PAGE_SHIFT;
-
/* We require SSE 4.1 for fast reads from the GuC log buffer and
* it should be present on the chipsets supporting GuC based
* submisssions.
@@ -529,7 +598,7 @@ int intel_guc_log_create(struct intel_guc *guc)
goto err;
}
- vma = intel_guc_allocate_vma(guc, size);
+ vma = intel_guc_allocate_vma(guc, GUC_LOG_SIZE);
if (IS_ERR(vma)) {
ret = PTR_ERR(vma);
goto err;
@@ -584,7 +653,15 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
return 0;
verbosity = enable_logging ? control_val - 1 : 0;
+
+ ret = mutex_lock_interruptible(&dev_priv->drm.struct_mutex);
+ if (ret)
+ return ret;
+ intel_runtime_pm_get(dev_priv);
ret = guc_log_control(guc, enable_logging, verbosity);
+ intel_runtime_pm_put(dev_priv);
+ mutex_unlock(&dev_priv->drm.struct_mutex);
+
if (ret < 0) {
DRM_DEBUG_DRIVER("guc_logging_control action failed %d\n", ret);
return ret;
@@ -605,7 +682,11 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
}
/* 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 {
/*
* Once logging is disabled, GuC won't generate logs & send an
@@ -627,13 +708,13 @@ void i915_guc_log_register(struct drm_i915_private *dev_priv)
if (!USES_GUC_SUBMISSION(dev_priv) || !i915_modparams.guc_log_level)
return;
- mutex_lock(&dev_priv->drm.struct_mutex);
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)
{
+ struct intel_guc *guc = &dev_priv->guc;
+
if (!USES_GUC_SUBMISSION(dev_priv))
return;
@@ -643,6 +724,8 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
gen9_disable_guc_interrupts(dev_priv);
intel_runtime_pm_put(dev_priv);
- guc_log_runtime_destroy(&dev_priv->guc);
+ guc_log_runtime_destroy(guc);
mutex_unlock(&dev_priv->drm.struct_mutex);
+
+ intel_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 f512cf7..c638b9d 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.h
+++ b/drivers/gpu/drm/i915/intel_guc_log.h
@@ -32,6 +32,13 @@
struct drm_i915_private;
struct intel_guc;
+/*
+ * The first page is to save log buffer state. Allocate one
+ * extra page for others in case for overlap
+ */
+#define GUC_LOG_SIZE ((1 + GUC_LOG_DPC_PAGES + 1 + GUC_LOG_ISR_PAGES + \
+ 1 + GUC_LOG_CRASH_PAGES + 1) << PAGE_SHIFT)
+
struct intel_guc_log {
u32 flags;
struct i915_vma *vma;
@@ -41,6 +48,8 @@ struct intel_guc_log {
struct workqueue_struct *flush_wq;
struct work_struct flush_work;
struct rchan *relay_chan;
+ /* To serialize the access to relay_chan */
+ struct mutex relay_lock;
} runtime;
/* logging related stats */
u32 capture_miss_count;
@@ -52,6 +61,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 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);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index f78a17b..e3f3509 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -236,28 +236,44 @@ static void guc_disable_communication(struct intel_guc *guc)
guc->send = intel_guc_send_nop;
}
-int intel_uc_init_wq(struct drm_i915_private *dev_priv)
+int intel_uc_init_misc(struct drm_i915_private *dev_priv)
{
+ struct intel_guc *guc = &dev_priv->guc;
int ret;
if (!USES_GUC(dev_priv))
return 0;
- ret = intel_guc_init_wq(&dev_priv->guc);
+ ret = intel_guc_init_wq(guc);
if (ret) {
DRM_ERROR("Couldn't allocate workqueues for GuC\n");
- return ret;
+ goto err;
+ }
+
+ ret = intel_guc_log_relay_create(guc);
+ if (ret) {
+ DRM_ERROR("Couldn't allocate relay for GuC log\n");
+ goto err_relay;
}
return 0;
+
+err_relay:
+ intel_guc_fini_wq(guc);
+err:
+ return ret;
}
-void intel_uc_fini_wq(struct drm_i915_private *dev_priv)
+void intel_uc_fini_misc(struct drm_i915_private *dev_priv)
{
+ struct intel_guc *guc = &dev_priv->guc;
+
if (!USES_GUC(dev_priv))
return;
- intel_guc_fini_wq(&dev_priv->guc);
+ 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 8a72497..f2984e0 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -33,8 +33,8 @@
void intel_uc_init_mmio(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_wq(struct drm_i915_private *dev_priv);
-void intel_uc_fini_wq(struct drm_i915_private *dev_priv);
+int intel_uc_init_misc(struct drm_i915_private *dev_priv);
+void intel_uc_fini_misc(struct drm_i915_private *dev_priv);
int intel_uc_init_hw(struct drm_i915_private *dev_priv);
void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
int intel_uc_init(struct drm_i915_private *dev_priv);
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v4 3/6] drm/i915/guc: Fix lockdep due to log relay channel handling under struct_mutex
2018-01-24 15:46 ` [PATCH v4 3/6] drm/i915/guc: Fix lockdep due to log relay channel handling under struct_mutex Sagar Arun Kamble
@ 2018-01-24 19:40 ` Chris Wilson
0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2018-01-24 19:40 UTC (permalink / raw)
To: Sagar Arun Kamble, intel-gfx
Quoting Sagar Arun Kamble (2018-01-24 15:46:58)
> This patch fixes lockdep issue due to circular locking dependency of
> struct_mutex, i_mutex_key, mmap_sem, relay_channels_mutex.
> For GuC log relay channel we create debugfs file that requires i_mutex_key
> lock and we are doing that under struct_mutex. So we introduced newer
> dependency as:
> &dev->struct_mutex --> &sb->s_type->i_mutex_key#3 --> &mm->mmap_sem
> However, there is dependency from mmap_sem to struct_mutex. Hence we
> separate the relay create/destroy operation from under struct_mutex.
> Also added runtime check of relay buffer status.
>
> ======================================================
> WARNING: possible circular locking dependency detected
> 4.15.0-rc6-CI-Patchwork_7614+ #1 Not tainted
> ------------------------------------------------------
> debugfs_test/1388 is trying to acquire lock:
> (&dev->struct_mutex){+.+.}, at: [<00000000d5e1d915>] i915_mutex_lock_interruptible+0x47/0x130 [i915]
>
> but task is already holding lock:
> (&mm->mmap_sem){++++}, at: [<0000000029a9c131>] __do_page_fault+0x106/0x560
>
> which lock already depends on the new lock.
>
> the existing dependency chain (in reverse order) is:
>
> -> #3 (&mm->mmap_sem){++++}:
> _copy_to_user+0x1e/0x70
> filldir+0x8c/0xf0
> dcache_readdir+0xeb/0x160
> iterate_dir+0xdc/0x140
> SyS_getdents+0xa0/0x130
> entry_SYSCALL_64_fastpath+0x1c/0x89
>
> -> #2 (&sb->s_type->i_mutex_key#3){++++}:
> start_creating+0x59/0x110
> __debugfs_create_file+0x2e/0xe0
> relay_create_buf_file+0x62/0x80
> relay_late_setup_files+0x84/0x250
> guc_log_late_setup+0x4f/0x110 [i915]
> i915_guc_log_register+0x32/0x40 [i915]
> i915_driver_load+0x7b6/0x1720 [i915]
> i915_pci_probe+0x2e/0x90 [i915]
> pci_device_probe+0x9c/0x120
> driver_probe_device+0x2a3/0x480
> __driver_attach+0xd9/0xe0
> bus_for_each_dev+0x57/0x90
> bus_add_driver+0x168/0x260
> driver_register+0x52/0xc0
> do_one_initcall+0x39/0x150
> do_init_module+0x56/0x1ef
> load_module+0x231c/0x2d70
> SyS_finit_module+0xa5/0xe0
> entry_SYSCALL_64_fastpath+0x1c/0x89
>
> -> #1 (relay_channels_mutex){+.+.}:
> relay_open+0x12c/0x2b0
> intel_guc_log_runtime_create+0xab/0x230 [i915]
> intel_guc_init+0x81/0x120 [i915]
> intel_uc_init+0x29/0xa0 [i915]
> i915_gem_init+0x182/0x530 [i915]
> i915_driver_load+0xaa9/0x1720 [i915]
> i915_pci_probe+0x2e/0x90 [i915]
> pci_device_probe+0x9c/0x120
> driver_probe_device+0x2a3/0x480
> __driver_attach+0xd9/0xe0
> bus_for_each_dev+0x57/0x90
> bus_add_driver+0x168/0x260
> driver_register+0x52/0xc0
> do_one_initcall+0x39/0x150
> do_init_module+0x56/0x1ef
> load_module+0x231c/0x2d70
> SyS_finit_module+0xa5/0xe0
> entry_SYSCALL_64_fastpath+0x1c/0x89
>
> -> #0 (&dev->struct_mutex){+.+.}:
> __mutex_lock+0x81/0x9b0
> i915_mutex_lock_interruptible+0x47/0x130 [i915]
> i915_gem_fault+0x201/0x790 [i915]
> __do_fault+0x15/0x70
> __handle_mm_fault+0x677/0xdc0
> handle_mm_fault+0x14f/0x2f0
> __do_page_fault+0x2d1/0x560
> page_fault+0x4c/0x60
>
> other info that might help us debug this:
>
> Chain exists of:
> &dev->struct_mutex --> &sb->s_type->i_mutex_key#3 --> &mm->mmap_sem
>
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(&mm->mmap_sem);
> lock(&sb->s_type->i_mutex_key#3);
> lock(&mm->mmap_sem);
> lock(&dev->struct_mutex);
>
> *** DEADLOCK ***
>
> 1 lock held by debugfs_test/1388:
> #0: (&mm->mmap_sem){++++}, at: [<0000000029a9c131>] __do_page_fault+0x106/0x560
>
> stack backtrace:
> CPU: 2 PID: 1388 Comm: debugfs_test Not tainted 4.15.0-rc6-CI-Patchwork_7614+ #1
> Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./J4205-ITX, BIOS P1.10 09/29/2016
> Call Trace:
> dump_stack+0x5f/0x86
> print_circular_bug.isra.18+0x1d0/0x2c0
> __lock_acquire+0x14ae/0x1b60
> ? lock_acquire+0xaf/0x200
> lock_acquire+0xaf/0x200
> ? i915_mutex_lock_interruptible+0x47/0x130 [i915]
> __mutex_lock+0x81/0x9b0
> ? i915_mutex_lock_interruptible+0x47/0x130 [i915]
> ? i915_mutex_lock_interruptible+0x47/0x130 [i915]
> ? i915_mutex_lock_interruptible+0x47/0x130 [i915]
> i915_mutex_lock_interruptible+0x47/0x130 [i915]
> ? __pm_runtime_resume+0x4f/0x80
> i915_gem_fault+0x201/0x790 [i915]
> __do_fault+0x15/0x70
> ? _raw_spin_unlock+0x29/0x40
> __handle_mm_fault+0x677/0xdc0
> handle_mm_fault+0x14f/0x2f0
> __do_page_fault+0x2d1/0x560
> ? page_fault+0x36/0x60
> page_fault+0x4c/0x60
>
> v2: Added lock protection to guc->log.runtime.relay_chan (Chris)
> Fixed locking inside guc_flush_logs uncovered by new lockdep.
>
> v3: Locking guc_read_update_log_buffer entirely with relay_lock. (Chris)
> Prepared intel_guc_init_early. Moved relay_lock inside relay_create
> relay_destroy, relay_file_create, guc_read_update_log_buffer. (Michal)
> Removed struct_mutex lock around guc_log_flush and removed usage
> of guc_log_has_relay() from runtime_create path as it needs
> struct_mutex lock.
>
> v4: Handle NULL relay sub buffer pointer earlier in read_update_log_buffer
> (Chris). Fixed comment suffix **/. (Michal)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104693
> Testcase: igt/debugfs_test/read_all_entries # with enable_guc=1 and guc_log_level=1
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Marta Lofstedt <marta.lofstedt@intel.com>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 4/6] drm/i915/guc: Update name and prototype of i915_guc_log_control
2018-01-24 15:46 [PATCH v4 1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts Sagar Arun Kamble
2018-01-24 15:46 ` [PATCH v4 2/6] drm/i915/guc: Enable interrupts before resuming GuC during runtime resume Sagar Arun Kamble
2018-01-24 15:46 ` [PATCH v4 3/6] drm/i915/guc: Fix lockdep due to log relay channel handling under struct_mutex Sagar Arun Kamble
@ 2018-01-24 15:46 ` Sagar Arun Kamble
2018-01-24 15:47 ` [PATCH v4 5/6] drm/i915/guc: Fix comments style in intel_guc_log.c Sagar Arun Kamble
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Sagar Arun Kamble @ 2018-01-24 15:46 UTC (permalink / raw)
To: intel-gfx
i915_guc_log_control is GuC interface and GuC APIs that are not user
facing should be named with "intel_guc" prefix hence we change name to
intel_guc_log_control. Also changed the parameter to intel_guc struct.
v2: Move log vma check to intel_guc_log_control (Michal)
Return -ENODEV when log isn't initialized. (Chris)
Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 5 +----
drivers/gpu/drm/i915/intel_guc_log.c | 7 +++++--
drivers/gpu/drm/i915/intel_guc_log.h | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b45be0d..b511b92 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2471,10 +2471,7 @@ static int i915_guc_log_control_set(void *data, u64 val)
if (!HAS_GUC(dev_priv))
return -ENODEV;
- if (!dev_priv->guc.log.vma)
- return -EINVAL;
-
- return i915_guc_log_control(dev_priv, val);
+ return intel_guc_log_control(&dev_priv->guc, val);
}
DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_control_fops,
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index d10ec53..cb10f7a 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -637,13 +637,16 @@ void intel_guc_log_destroy(struct intel_guc *guc)
i915_vma_unpin_and_release(&guc->log.vma);
}
-int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
+int intel_guc_log_control(struct intel_guc *guc, u64 control_val)
{
- struct intel_guc *guc = &dev_priv->guc;
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
bool enable_logging = control_val > 0;
u32 verbosity;
int ret;
+ if (!guc->log.vma)
+ return -ENODEV;
+
BUILD_BUG_ON(GUC_LOG_VERBOSITY_MIN);
if (control_val > 1 + GUC_LOG_VERBOSITY_MAX)
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h
index c638b9d..dab0e94 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.h
+++ b/drivers/gpu/drm/i915/intel_guc_log.h
@@ -64,7 +64,7 @@ struct intel_guc_log {
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 i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
+int intel_guc_log_control(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);
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v4 5/6] drm/i915/guc: Fix comments style in intel_guc_log.c
2018-01-24 15:46 [PATCH v4 1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts Sagar Arun Kamble
` (2 preceding siblings ...)
2018-01-24 15:46 ` [PATCH v4 4/6] drm/i915/guc: Update name and prototype of i915_guc_log_control Sagar Arun Kamble
@ 2018-01-24 15:47 ` Sagar Arun Kamble
2018-01-24 19:41 ` Chris Wilson
2018-01-24 15:47 ` [PATCH v4 6/6] [HAX] Revert "drm/i915/guc: Keep GuC log disabled by default" Sagar Arun Kamble
` (2 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Sagar Arun Kamble @ 2018-01-24 15:47 UTC (permalink / raw)
To: intel-gfx
Use consistent multi-line comment style as per guideline.
v2: Reverted comments prefix update to kernel-doc comment. (Chris)
Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_guc_log.c | 45 ++++++++++++++++++++++++------------
1 file changed, 30 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index cb10f7a..86a3321 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -81,7 +81,8 @@ static int subbuf_start_callback(struct rchan_buf *buf,
void *prev_subbuf,
size_t prev_padding)
{
- /* Use no-overwrite mode by default, where relay will stop accepting
+ /*
+ * Use no-overwrite mode by default, where relay will stop accepting
* new data if there are no empty sub buffers left.
* There is no strict synchronization enforced by relay between Consumer
* and Producer. In overwrite mode, there is a possibility of getting
@@ -107,7 +108,8 @@ static struct dentry *create_buf_file_callback(const char *filename,
{
struct dentry *buf_file;
- /* This to enable the use of a single buffer for the relay channel and
+ /*
+ * This to enable the use of a single buffer for the relay channel and
* correspondingly have a single file exposed to User, through which
* it can collect the logs in order without any post-processing.
* Need to set 'is_global' even if parent is NULL for early logging.
@@ -117,7 +119,8 @@ static struct dentry *create_buf_file_callback(const char *filename,
if (!parent)
return NULL;
- /* Not using the channel filename passed as an argument, since for each
+ /*
+ * Not using the channel filename passed as an argument, since for each
* channel relay appends the corresponding CPU number to the filename
* passed in relay_open(). This should be fine as relay just needs a
* dentry of the file associated with the channel buffer and that file's
@@ -158,7 +161,8 @@ static int guc_log_relay_file_create(struct intel_guc *guc)
/* For now create the log file in /sys/kernel/debug/dri/0 dir */
log_dir = dev_priv->drm.primary->debugfs_root;
- /* If /sys/kernel/debug/dri/0 location do not exist, then debugfs is
+ /*
+ * If /sys/kernel/debug/dri/0 location do not exist, then debugfs is
* not mounted and so can't create the relay file.
* The relay API seems to fit well with debugfs only, for availing relay
* there are 3 requirements which can be met for debugfs file only in a
@@ -195,7 +199,8 @@ static bool guc_log_has_relay(struct intel_guc *guc)
static void guc_move_to_next_buf(struct intel_guc *guc)
{
- /* Make sure the updates made in the sub buffer are visible when
+ /*
+ * Make sure the updates made in the sub buffer are visible when
* Consumer sees the following update to offset inside the sub buffer.
*/
smp_wmb();
@@ -215,7 +220,8 @@ static void *guc_get_write_buffer(struct intel_guc *guc)
if (!guc_log_has_relay(guc))
return NULL;
- /* Just get the base address of a new sub buffer and copy data into it
+ /*
+ * Just get the base address of a new sub buffer and copy data into it
* ourselves. NULL will be returned in no-overwrite mode, if all sub
* buffers are full. Could have used the relay_write() to indirectly
* copy the data, but that would have been bit convoluted, as we need to
@@ -286,7 +292,8 @@ static void guc_read_update_log_buffer(struct intel_guc *guc)
log_buf_snapshot_state = dst_data = guc_get_write_buffer(guc);
if (unlikely(!log_buf_snapshot_state)) {
- /* Used rate limited to avoid deluge of messages, logs might be
+ /*
+ * Used rate limited to avoid deluge of messages, logs might be
* getting consumed by User at a slow rate.
*/
DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n");
@@ -301,7 +308,8 @@ static void guc_read_update_log_buffer(struct intel_guc *guc)
dst_data += PAGE_SIZE;
for (type = GUC_ISR_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER; type++) {
- /* Make a copy of the state structure, inside GuC log buffer
+ /*
+ * Make a copy of the state structure, inside GuC log buffer
* (which is uncached mapped), on the stack to avoid reading
* from it multiple times.
*/
@@ -325,7 +333,8 @@ static void guc_read_update_log_buffer(struct intel_guc *guc)
memcpy(log_buf_snapshot_state, &log_buf_state_local,
sizeof(struct guc_log_buffer_state));
- /* The write pointer could have been updated by GuC firmware,
+ /*
+ * The write pointer could have been updated by GuC firmware,
* after sending the flush interrupt to Host, for consistency
* set write pointer value to same value of sampled_write_ptr
* in the snapshot buffer.
@@ -392,7 +401,8 @@ static int guc_log_runtime_create(struct intel_guc *guc)
if (ret)
return ret;
- /* Create a WC (Uncached for read) vmalloc mapping of log
+ /*
+ * Create a WC (Uncached for read) vmalloc mapping of log
* buffer pages, so that we can directly get the data
* (up-to-date) from memory.
*/
@@ -443,14 +453,16 @@ int intel_guc_log_relay_create(struct intel_guc *guc)
/* Keep the size of sub buffers same as shared log buffer */
subbuf_size = GUC_LOG_SIZE;
- /* Store up to 8 snapshots, which is large enough to buffer sufficient
+ /*
+ * Store up to 8 snapshots, which is large enough to buffer sufficient
* boot time logs and provides enough leeway to User, in terms of
* latency, for consuming the logs from relay. Also doesn't take
* up too much memory.
*/
n_subbufs = 8;
- /* Create a relay channel, so that we have buffers for storing
+ /*
+ * Create a relay channel, so that we have buffers for storing
* the GuC firmware logs, the channel will be linked with a file
* later on when debugfs is registered.
*/
@@ -544,7 +556,8 @@ static void guc_log_capture_logs(struct intel_guc *guc)
guc_read_update_log_buffer(guc);
- /* Generally device is expected to be active only at this
+ /*
+ * Generally device is expected to be active only at this
* time, so get/put should be really quick.
*/
intel_runtime_pm_get(dev_priv);
@@ -566,7 +579,8 @@ static void guc_flush_logs(struct intel_guc *guc)
intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev_priv->drm.struct_mutex);
- /* Before initiating the forceful flush, wait for any pending/ongoing
+ /*
+ * Before initiating the forceful flush, wait for any pending/ongoing
* flush to complete otherwise forceful flush may not actually happen.
*/
flush_work(&guc->log.runtime.flush_work);
@@ -589,7 +603,8 @@ int intel_guc_log_create(struct intel_guc *guc)
GEM_BUG_ON(guc->log.vma);
- /* We require SSE 4.1 for fast reads from the GuC log buffer and
+ /*
+ * We require SSE 4.1 for fast reads from the GuC log buffer and
* it should be present on the chipsets supporting GuC based
* submisssions.
*/
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v4 6/6] [HAX] Revert "drm/i915/guc: Keep GuC log disabled by default"
2018-01-24 15:46 [PATCH v4 1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts Sagar Arun Kamble
` (3 preceding siblings ...)
2018-01-24 15:47 ` [PATCH v4 5/6] drm/i915/guc: Fix comments style in intel_guc_log.c Sagar Arun Kamble
@ 2018-01-24 15:47 ` Sagar Arun Kamble
2018-01-24 16:14 ` ✓ Fi.CI.BAT: success for series starting with [v4,1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts Patchwork
2018-01-24 19:39 ` ✗ Fi.CI.IGT: failure " Patchwork
6 siblings, 0 replies; 11+ messages in thread
From: Sagar Arun Kamble @ 2018-01-24 15:47 UTC (permalink / raw)
To: intel-gfx
Comment DRM_ERROR_RATELIMIT outputs as log capturer won't be
running.
This reverts commit bd724318b682587ad2f989ab8e0f7b3d4486ced5.
---
drivers/gpu/drm/i915/i915_params.h | 2 +-
drivers/gpu/drm/i915/intel_guc_log.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 430f5f9..c963603 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -48,7 +48,7 @@
param(int, enable_ips, 1) \
param(int, invert_brightness, 0) \
param(int, enable_guc, 0) \
- param(int, guc_log_level, 0) \
+ param(int, guc_log_level, -1) \
param(char *, guc_firmware_path, NULL) \
param(char *, huc_firmware_path, NULL) \
param(int, mmio_debug, 0) \
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 86a3321..1ceb81a 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -249,7 +249,7 @@ static bool guc_check_log_buf_overflow(struct intel_guc *guc,
/* buffer_full_cnt is a 4 bit counter */
guc->log.total_overflow_count[type] += 16;
}
- DRM_ERROR_RATELIMITED("GuC log buffer overflow\n");
+ //DRM_ERROR_RATELIMITED("GuC log buffer overflow\n");
}
return overflow;
@@ -296,7 +296,7 @@ static void guc_read_update_log_buffer(struct intel_guc *guc)
* Used rate limited to avoid deluge of messages, logs might be
* getting consumed by User at a slow rate.
*/
- DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n");
+ //DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n");
guc->log.capture_miss_count++;
mutex_unlock(&guc->log.runtime.relay_lock);
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread* ✓ Fi.CI.BAT: success for series starting with [v4,1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
2018-01-24 15:46 [PATCH v4 1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts Sagar Arun Kamble
` (4 preceding siblings ...)
2018-01-24 15:47 ` [PATCH v4 6/6] [HAX] Revert "drm/i915/guc: Keep GuC log disabled by default" Sagar Arun Kamble
@ 2018-01-24 16:14 ` Patchwork
2018-01-24 19:39 ` ✗ Fi.CI.IGT: failure " Patchwork
6 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-01-24 16:14 UTC (permalink / raw)
To: Sagar Arun Kamble; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v4,1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
URL : https://patchwork.freedesktop.org/series/37044/
State : success
== Summary ==
Series 37044v1 series starting with [v4,1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
https://patchwork.freedesktop.org/api/1.0/series/37044/revisions/1/mbox/
Test debugfs_test:
Subgroup read_all_entries:
dmesg-warn -> FAIL (fi-elk-e7500) fdo#103989 +1
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass -> INCOMPLETE (fi-snb-2520m) fdo#103713
fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:422s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:424s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:371s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:489s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:280s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:482s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:484s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:466s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:461s
fi-elk-e7500 total:224 pass:167 dwarn:10 dfail:0 fail:1 skip:45
fi-gdg-551 total:288 pass:180 dwarn:0 dfail:0 fail:0 skip:108 time:278s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:512s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:390s
fi-hsw-4770r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:400s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:412s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:460s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:417s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:457s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:491s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:456s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:509s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:580s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:431s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:511s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:528s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:492s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:483s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:415s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:435s
fi-snb-2520m total:245 pass:211 dwarn:0 dfail:0 fail:0 skip:33
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:394s
Blacklisted hosts:
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:563s
fi-glk-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:469s
92c5caeeb0cae3952f5e01d0305be09559ce49b4 drm-tip: 2018y-01m-24d-15h-29m-34s UTC integration manifest
475cd4c8c2b8 Revert "drm/i915/guc: Keep GuC log disabled by default"
4ea239fa225c drm/i915/guc: Fix comments style in intel_guc_log.c
4800c9d3713e drm/i915/guc: Update name and prototype of i915_guc_log_control
0e0263e727c1 drm/i915/guc: Fix lockdep due to log relay channel handling under struct_mutex
03ab713e21d0 drm/i915/guc: Enable interrupts before resuming GuC during runtime resume
14022b30f2f8 drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7768/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread* ✗ Fi.CI.IGT: failure for series starting with [v4,1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
2018-01-24 15:46 [PATCH v4 1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts Sagar Arun Kamble
` (5 preceding siblings ...)
2018-01-24 16:14 ` ✓ Fi.CI.BAT: success for series starting with [v4,1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts Patchwork
@ 2018-01-24 19:39 ` Patchwork
2018-01-24 19:46 ` Chris Wilson
6 siblings, 1 reply; 11+ messages in thread
From: Patchwork @ 2018-01-24 19:39 UTC (permalink / raw)
To: Sagar Arun Kamble; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v4,1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
URL : https://patchwork.freedesktop.org/series/37044/
State : failure
== Summary ==
Test kms_cursor_crc:
Subgroup cursor-256x256-suspend:
skip -> PASS (shard-hsw) fdo#103375 +1
Test kms_frontbuffer_tracking:
Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
pass -> FAIL (shard-snb) fdo#101623 +1
Test kms_flip:
Subgroup 2x-plain-flip-ts-check-interruptible:
fail -> PASS (shard-hsw)
Test drv_selftest:
Subgroup live_hangcheck:
pass -> INCOMPLETE (shard-apl)
Test kms_draw_crc:
Subgroup draw-method-xrgb8888-mmap-wc-untiled:
pass -> SKIP (shard-snb)
Test kms_vblank:
Subgroup crtc-id:
pass -> SKIP (shard-snb)
Test perf:
Subgroup oa-exponents:
fail -> PASS (shard-apl) fdo#102254
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
shard-apl total:2820 pass:1734 dwarn:1 dfail:0 fail:22 skip:1062 time:12164s
shard-hsw total:2838 pass:1735 dwarn:1 dfail:0 fail:11 skip:1090 time:11886s
shard-snb total:2838 pass:1326 dwarn:1 dfail:0 fail:11 skip:1500 time:6632s
Blacklisted hosts:
shard-kbl total:2838 pass:1871 dwarn:5 dfail:1 fail:23 skip:938 time:9485s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7768/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: ✗ Fi.CI.IGT: failure for series starting with [v4,1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
2018-01-24 19:39 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-01-24 19:46 ` Chris Wilson
0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2018-01-24 19:46 UTC (permalink / raw)
To: Patchwork, Sagar Arun Kamble; +Cc: intel-gfx
Quoting Patchwork (2018-01-24 19:39:56)
> == Series Details ==
>
> Series: series starting with [v4,1/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
> URL : https://patchwork.freedesktop.org/series/37044/
> State : failure
>
> == Summary ==
>
> Test kms_cursor_crc:
> Subgroup cursor-256x256-suspend:
> skip -> PASS (shard-hsw) fdo#103375 +1
> Test kms_frontbuffer_tracking:
> Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
> pass -> FAIL (shard-snb) fdo#101623 +1
> Test kms_flip:
> Subgroup 2x-plain-flip-ts-check-interruptible:
> fail -> PASS (shard-hsw)
> Test drv_selftest:
> Subgroup live_hangcheck:
> pass -> INCOMPLETE (shard-apl)
> Test kms_draw_crc:
> Subgroup draw-method-xrgb8888-mmap-wc-untiled:
> pass -> SKIP (shard-snb)
> Test kms_vblank:
> Subgroup crtc-id:
> pass -> SKIP (shard-snb)
> Test perf:
> Subgroup oa-exponents:
> fail -> PASS (shard-apl) fdo#102254
And pushed (except for the last patch ofc). Thanks for the patches and
review,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread