Re: [PATCH] drm/i915: add missing rpm ref to i915_gem_pwrite_ioctl

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Nov 12, 2014 at 04:40:35PM +0200, Imre Deak wrote:
> Without this RPM ref we can hit the device suspended WARN via:
> i915_gem_object_pin()->ggtt_bind_vma->gen6_ggtt_insert_entries(). I
> noticed this on my BYT while keeping the i915 device in runtime
> suspended state for a while. I chose this place to take the ref to
> avoid the possible deadlock via the mutex_lock taken both later in this
> function and in the runtime suspend handler. This can happen if an RPM
> suspend event is queued and need to be flushed before taking the RPM
> ref.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

We should be able to hit this easily with an igt by doing a pwrite to an
object which is attached to a cursor. I.e.
1. Set up mode with cursor visible.
2. dpms off
-> ensure rpm is entered
3. pwrite to cursor -> boom.

Can you please add this as a new subtest to pm_rpm.c? I think it should
fit fairly easily there.

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 86cf428..ec2ee28 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1075,6 +1075,7 @@ int
>  i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
>  		      struct drm_file *file)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_i915_gem_pwrite *args = data;
>  	struct drm_i915_gem_object *obj;
>  	int ret;
> @@ -1094,9 +1095,11 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
>  			return -EFAULT;
>  	}
>  
> +	intel_runtime_pm_get(dev_priv);
> +
>  	ret = i915_mutex_lock_interruptible(dev);
>  	if (ret)
> -		return ret;
> +		goto put_rpm;
>  
>  	obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle));
>  	if (&obj->base == NULL) {
> @@ -1148,6 +1151,9 @@ out:
>  	drm_gem_object_unreference(&obj->base);
>  unlock:
>  	mutex_unlock(&dev->struct_mutex);
> +put_rpm:
> +	intel_runtime_pm_put(dev_priv);
> +
>  	return ret;
>  }
>  
> -- 
> 1.8.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx