[Intel-gfx] [PATCH 1/2] drm/i915: Wait for the struct_mutex on idling

[Tvrtko Ursulin]

On 30/04/2019 10:44, Chris Wilson wrote:
> When the system is idling, contention for struct_mutex should be low and
> so we will be more efficient to wait for a contended mutex than
> reschedule.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_gem_pm.c | 8 +-------
>   1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
> index 3554d55dae35..3b6e8d5be8e1 100644
> --- a/drivers/gpu/drm/i915/i915_gem_pm.c
> +++ b/drivers/gpu/drm/i915/i915_gem_pm.c
> @@ -47,13 +47,7 @@ static void idle_work_handler(struct work_struct *work)
>   	struct drm_i915_private *i915 =
>   		container_of(work, typeof(*i915), gem.idle_work.work);
>   
> -	if (!mutex_trylock(&i915->drm.struct_mutex)) {
> -		/* Currently busy, come back later */
> -		mod_delayed_work(i915->wq,
> -				 &i915->gem.idle_work,
> -				 msecs_to_jiffies(50));
> -		return;
> -	}
> +	mutex_lock(&i915->drm.struct_mutex);
>   
>   	intel_wakeref_lock(&i915->gt.wakeref);
>   	if (!intel_wakeref_active(&i915->gt.wakeref))
> 

I don't see any real downsides to this indeed.

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

Possible tweak could be to leave this as is, maybe just not go for the 
reduced idle timer on re-schedule, but add a cancel_delayed_work on the 
unparking side of things. That way any mutex activity without actual 
device unparking would only slightly delay going idle, while idle_work 
would retain it's minimal disturbance of the mutex.

Regards,

Tvrtko