| Message ID | 20170314002606.8838-1-michel.thierry@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 13, 2017 at 05:26:06PM -0700, Michel Thierry wrote: > At least in bxt (with decoupled mmio), it prevents a warning while > capturing the reg state: > > [ ] WARNING: assert_rpm_wakelock_held.part.4+0x1e/0x20 [i915] > [ ] RPM wakelock ref not held during HW access > [ ] Call Trace: > [ ] dump_stack+0x63/0x87 > [ ] __warn+0xd1/0xf0 > [ ] ? fwtable_write32+0x1a0/0x1a0 [i915] > [ ] warn_slowpath_fmt+0x4f/0x60 > [ ] ? vprintk_default+0x29/0x50 > [ ] assert_rpm_wakelock_held.part.4+0x1e/0x20 [i915] > [ ] gen9_decoupled_read32+0x18a/0x1a0 [i915] > [ ] ? fwtable_write32+0x1a0/0x1a0 [i915] > [ ] i915_handle_error+0x1d0/0x260 [i915] > [ ] ? __check_object_size+0x170/0x1aa > [ ] i915_wedged_set+0x25/0x30 [i915] > > These get/put were removed by commit 9c870d03674f ("drm/i915: Use RPM as the > barrier for controlling user mmap access"), before decoupled mmio was > added in commit 85ee17ebeedd ("drm/i915/bxt: Broxton decoupled MMIO). > > Reported-by: Antonio Argenziano <antonio.argenziano@intel.com> > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 9fcc4f1a86fc..89f211391ef9 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -4155,9 +4155,13 @@ i915_wedged_set(void *data, u64 val) > if (i915_reset_in_progress(&dev_priv->gpu_error)) > return -EAGAIN; > > + intel_runtime_pm_get(dev_priv); They don't belong here. i915_handle_error is already taking rpm, it just forgot a path. -Chris
On 13/03/17 17:26, Michel Thierry wrote: > At least in bxt (with decoupled mmio), it prevents a warning while > capturing the reg state: > > [ ] WARNING: assert_rpm_wakelock_held.part.4+0x1e/0x20 [i915] > [ ] RPM wakelock ref not held during HW access > [ ] Call Trace: > [ ] dump_stack+0x63/0x87 > [ ] __warn+0xd1/0xf0 > [ ] ? fwtable_write32+0x1a0/0x1a0 [i915] > [ ] warn_slowpath_fmt+0x4f/0x60 > [ ] ? vprintk_default+0x29/0x50 > [ ] assert_rpm_wakelock_held.part.4+0x1e/0x20 [i915] > [ ] gen9_decoupled_read32+0x18a/0x1a0 [i915] > [ ] ? fwtable_write32+0x1a0/0x1a0 [i915] > [ ] i915_handle_error+0x1d0/0x260 [i915] > [ ] ? __check_object_size+0x170/0x1aa > [ ] i915_wedged_set+0x25/0x30 [i915] > > These get/put were removed by commit 9c870d03674f ("drm/i915: Use RPM as the > barrier for controlling user mmap access"), before decoupled mmio was > added in commit 85ee17ebeedd ("drm/i915/bxt: Broxton decoupled MMIO). > > Reported-by: Antonio Argenziano <antonio.argenziano@intel.com> > Signed-off-by: Michel Thierry <michel.thierry@intel.com> Tested-By: Antonio Argenziano <antonio.argenziano@intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 9fcc4f1a86fc..89f211391ef9 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -4155,9 +4155,13 @@ i915_wedged_set(void *data, u64 val) > if (i915_reset_in_progress(&dev_priv->gpu_error)) > return -EAGAIN; > > + intel_runtime_pm_get(dev_priv); > + > i915_handle_error(dev_priv, val, > "Manually setting wedged to %llu", val); > > + intel_runtime_pm_put(dev_priv); > + > return 0; > } > >
On 3/14/2017 2:09 AM, Chris Wilson wrote: > On Mon, Mar 13, 2017 at 05:26:06PM -0700, Michel Thierry wrote: >> At least in bxt (with decoupled mmio), it prevents a warning while >> capturing the reg state: >> >> [ ] WARNING: assert_rpm_wakelock_held.part.4+0x1e/0x20 [i915] >> [ ] RPM wakelock ref not held during HW access >> [ ] Call Trace: >> [ ] dump_stack+0x63/0x87 >> [ ] __warn+0xd1/0xf0 >> [ ] ? fwtable_write32+0x1a0/0x1a0 [i915] >> [ ] warn_slowpath_fmt+0x4f/0x60 >> [ ] ? vprintk_default+0x29/0x50 >> [ ] assert_rpm_wakelock_held.part.4+0x1e/0x20 [i915] >> [ ] gen9_decoupled_read32+0x18a/0x1a0 [i915] >> [ ] ? fwtable_write32+0x1a0/0x1a0 [i915] >> [ ] i915_handle_error+0x1d0/0x260 [i915] >> [ ] ? __check_object_size+0x170/0x1aa >> [ ] i915_wedged_set+0x25/0x30 [i915] >> >> These get/put were removed by commit 9c870d03674f ("drm/i915: Use RPM as the >> barrier for controlling user mmap access"), before decoupled mmio was >> added in commit 85ee17ebeedd ("drm/i915/bxt: Broxton decoupled MMIO). >> >> Reported-by: Antonio Argenziano <antonio.argenziano@intel.com> >> Signed-off-by: Michel Thierry <michel.thierry@intel.com> >> --- >> drivers/gpu/drm/i915/i915_debugfs.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c >> index 9fcc4f1a86fc..89f211391ef9 100644 >> --- a/drivers/gpu/drm/i915/i915_debugfs.c >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >> @@ -4155,9 +4155,13 @@ i915_wedged_set(void *data, u64 val) >> if (i915_reset_in_progress(&dev_priv->gpu_error)) >> return -EAGAIN; >> >> + intel_runtime_pm_get(dev_priv); > > They don't belong here. i915_handle_error is already taking rpm, it just > forgot a path. Oh, I should have looked at your tree first ;) (https://cgit.freedesktop.org/~ickle/linux-2.6/commit/drivers/gpu/drm/i915?h=prescheduler&id=e190db9f6898b60dc8905d6c5bf2dcd7ec759d7f) But the rpm still needs to cover capture_error_state.
On Tue, Mar 14, 2017 at 10:05:08AM -0700, Michel Thierry wrote: > On 3/14/2017 2:09 AM, Chris Wilson wrote: > >On Mon, Mar 13, 2017 at 05:26:06PM -0700, Michel Thierry wrote: > >>At least in bxt (with decoupled mmio), it prevents a warning while > >>capturing the reg state: > >> > >> [ ] WARNING: assert_rpm_wakelock_held.part.4+0x1e/0x20 [i915] > >> [ ] RPM wakelock ref not held during HW access > >> [ ] Call Trace: > >> [ ] dump_stack+0x63/0x87 > >> [ ] __warn+0xd1/0xf0 > >> [ ] ? fwtable_write32+0x1a0/0x1a0 [i915] > >> [ ] warn_slowpath_fmt+0x4f/0x60 > >> [ ] ? vprintk_default+0x29/0x50 > >> [ ] assert_rpm_wakelock_held.part.4+0x1e/0x20 [i915] > >> [ ] gen9_decoupled_read32+0x18a/0x1a0 [i915] > >> [ ] ? fwtable_write32+0x1a0/0x1a0 [i915] > >> [ ] i915_handle_error+0x1d0/0x260 [i915] > >> [ ] ? __check_object_size+0x170/0x1aa > >> [ ] i915_wedged_set+0x25/0x30 [i915] > >> > >>These get/put were removed by commit 9c870d03674f ("drm/i915: Use RPM as the > >>barrier for controlling user mmap access"), before decoupled mmio was > >>added in commit 85ee17ebeedd ("drm/i915/bxt: Broxton decoupled MMIO). > >> > >>Reported-by: Antonio Argenziano <antonio.argenziano@intel.com> > >>Signed-off-by: Michel Thierry <michel.thierry@intel.com> > >>--- > >> drivers/gpu/drm/i915/i915_debugfs.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >>diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > >>index 9fcc4f1a86fc..89f211391ef9 100644 > >>--- a/drivers/gpu/drm/i915/i915_debugfs.c > >>+++ b/drivers/gpu/drm/i915/i915_debugfs.c > >>@@ -4155,9 +4155,13 @@ i915_wedged_set(void *data, u64 val) > >> if (i915_reset_in_progress(&dev_priv->gpu_error)) > >> return -EAGAIN; > >> > >>+ intel_runtime_pm_get(dev_priv); > > > >They don't belong here. i915_handle_error is already taking rpm, it just > >forgot a path. > > Oh, I should have looked at your tree first ;) > (https://cgit.freedesktop.org/~ickle/linux-2.6/commit/drivers/gpu/drm/i915?h=prescheduler&id=e190db9f6898b60dc8905d6c5bf2dcd7ec759d7f) Ho hum. > But the rpm still needs to cover capture_error_state. And the error register reset, yeah. -Chris
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 9fcc4f1a86fc..89f211391ef9 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -4155,9 +4155,13 @@ i915_wedged_set(void *data, u64 val) if (i915_reset_in_progress(&dev_priv->gpu_error)) return -EAGAIN; + intel_runtime_pm_get(dev_priv); + i915_handle_error(dev_priv, val, "Manually setting wedged to %llu", val); + intel_runtime_pm_put(dev_priv); + return 0; }
At least in bxt (with decoupled mmio), it prevents a warning while capturing the reg state: [ ] WARNING: assert_rpm_wakelock_held.part.4+0x1e/0x20 [i915] [ ] RPM wakelock ref not held during HW access [ ] Call Trace: [ ] dump_stack+0x63/0x87 [ ] __warn+0xd1/0xf0 [ ] ? fwtable_write32+0x1a0/0x1a0 [i915] [ ] warn_slowpath_fmt+0x4f/0x60 [ ] ? vprintk_default+0x29/0x50 [ ] assert_rpm_wakelock_held.part.4+0x1e/0x20 [i915] [ ] gen9_decoupled_read32+0x18a/0x1a0 [i915] [ ] ? fwtable_write32+0x1a0/0x1a0 [i915] [ ] i915_handle_error+0x1d0/0x260 [i915] [ ] ? __check_object_size+0x170/0x1aa [ ] i915_wedged_set+0x25/0x30 [i915] These get/put were removed by commit 9c870d03674f ("drm/i915: Use RPM as the barrier for controlling user mmap access"), before decoupled mmio was added in commit 85ee17ebeedd ("drm/i915/bxt: Broxton decoupled MMIO). Reported-by: Antonio Argenziano <antonio.argenziano@intel.com> Signed-off-by: Michel Thierry <michel.thierry@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 4 ++++ 1 file changed, 4 insertions(+)