[Intel-gfx] [PATCH 07/22] drm/i915: Stop needlessly acquiring wakeref for debugfs/drop_caches_set

[Tvrtko Ursulin]

On 18/03/2019 09:51, Chris Wilson wrote:
> We only need to acquire a wakeref for ourselves for a few operations, as
> most either already acquire their own wakeref or imply a wakeref. In
> particular, it is i915_gem_set_wedged() that needed us to present it
> with a wakeref, which is incongruous with its "use anywhere" ability.
> 
> Suggested-by: Yokoyama, Caz <caz.yokoyama at intel.com>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Yokoyama, Caz <caz.yokoyama at intel.com>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c | 12 ++++--------
>   drivers/gpu/drm/i915/i915_reset.c   |  4 +++-
>   2 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 6a90558de213..08683dca7775 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3888,12 +3888,9 @@ static int
>   i915_drop_caches_set(void *data, u64 val)
>   {
>   	struct drm_i915_private *i915 = data;
> -	intel_wakeref_t wakeref;
> -	int ret = 0;
>   
>   	DRM_DEBUG("Dropping caches: 0x%08llx [0x%08llx]\n",
>   		  val, val & DROP_ALL);
> -	wakeref = intel_runtime_pm_get(i915);
>   
>   	if (val & DROP_RESET_ACTIVE &&
>   	    wait_for(intel_engines_are_idle(i915), I915_IDLE_ENGINES_TIMEOUT))
> @@ -3902,9 +3899,11 @@ i915_drop_caches_set(void *data, u64 val)
>   	/* No need to check and wait for gpu resets, only libdrm auto-restarts
>   	 * on ioctls on -EAGAIN. */
>   	if (val & (DROP_ACTIVE | DROP_RETIRE | DROP_RESET_SEQNO)) {
> +		int ret;
> +
>   		ret = mutex_lock_interruptible(&i915->drm.struct_mutex);
>   		if (ret)
> -			goto out;
> +			return ret;
>   
>   		if (val & DROP_ACTIVE)
>   			ret = i915_gem_wait_for_idle(i915,
> @@ -3943,10 +3942,7 @@ i915_drop_caches_set(void *data, u64 val)
>   	if (val & DROP_FREED)
>   		i915_gem_drain_freed_objects(i915);
>   
> -out:
> -	intel_runtime_pm_put(i915, wakeref);
> -
> -	return ret;
> +	return 0;
>   }
>   
>   DEFINE_SIMPLE_ATTRIBUTE(i915_drop_caches_fops,
> diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
> index b8daec7ddc06..e61bfa0fc4e0 100644
> --- a/drivers/gpu/drm/i915/i915_reset.c
> +++ b/drivers/gpu/drm/i915/i915_reset.c
> @@ -863,9 +863,11 @@ static void __i915_gem_set_wedged(struct drm_i915_private *i915)
>   void i915_gem_set_wedged(struct drm_i915_private *i915)
>   {
>   	struct i915_gpu_error *error = &i915->gpu_error;
> +	intel_wakeref_t wakeref;
>   
>   	mutex_lock(&error->wedge_mutex);
> -	__i915_gem_set_wedged(i915);
> +	with_intel_runtime_pm(i915, wakeref)
> +		__i915_gem_set_wedged(i915);
>   	mutex_unlock(&error->wedge_mutex);
>   }
>   
> 

Statement that all paths will take a wakeref looks true.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko