[Intel-gfx] [PATCH 06/19] drm/i915: Enable i915_gem_wait_for_idle() without holding struct_mutex

[Joonas Lahtinen]

On to, 2016-08-04 at 20:52 +0100, Chris Wilson wrote:
> -i915_gem_suspend(struct drm_device *dev)
> +int i915_gem_suspend(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	int ret = 0;
> +	int ret;
>  
>  	intel_suspend_gt_powersave(dev_priv);
>  
> @@ -4102,7 +4099,7 @@ i915_gem_suspend(struct drm_device *dev)
>  	if (ret)
>  		goto err;
>  
> -	ret = i915_gem_wait_for_idle(dev_priv);
> +	ret = i915_gem_wait_for_idle(dev_priv, true);
>  	if (ret)
>  		goto err;
>  
> @@ -4123,7 +4120,7 @@ i915_gem_suspend(struct drm_device *dev)
>  	return 0;
>  
>  err:
> -	mutex_unlock(&dev->struct_mutex);
> +	mutex_unlock(&dev_priv->drm.struct_mutex);

Did you intend to convert the parameter to dev_priv?

> @@ -486,7 +486,8 @@ void __i915_add_request(struct drm_i915_gem_request *request,
>  	 */
>  	request->emitted_jiffies = jiffies;
>  	request->previous_seqno = engine->last_submitted_seqno;
> -	smp_store_mb(engine->last_submitted_seqno, request->fence.seqno);
> +	engine->last_submitted_seqno = request->fence.seqno;

What's up with this change?

> +	i915_gem_active_set(&engine->last_request, request);
>  	list_add_tail(&request->link, &engine->request_list);
>  	list_add_tail(&request->ring_link, &ring->request_list);
>  
> <SNIP>
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6336,7 +6336,7 @@ bool i915_gpu_busy(void)
>  	dev_priv = i915_mch_dev;
>  
>  	for_each_engine(engine, dev_priv)
> -		ret |= !list_empty(&engine->request_list);
> +		ret |= intel_engine_is_active(engine);

|= always makes me think of bitfields because, well -- it is bitwise
operation :P

		if (intel_engine_is_active(engine))
			ret = true;

But I can live with what it is.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation