drm/i915: get a runtime PM ref in i915_wedged_set

Submitted by Michel Thierry on March 14, 2017, 12:26 a.m.

Details

Message ID 20170314002606.8838-1-michel.thierry@intel.com
State New
Headers show
Series "drm/i915: get a runtime PM ref in i915_wedged_set" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Michel Thierry March 14, 2017, 12:26 a.m.
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(+)

Patch hide | download patch | download mbox

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;
 }
 

Comments

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