Re: [PATCH 16/21] drm/i915: Move idle barrier cleanup into engine-pm

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 02/09/2019 05:02, Chris Wilson wrote:
> Now that we now longer need to guarantee that the active callback is
> under the struct_mutex, we can lift it out of the i915_gem_park() and
> into the engine parking itself.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_pm.c    | 19 -------------------
>   drivers/gpu/drm/i915/gt/intel_engine_pm.c | 15 +++++++++++++++
>   drivers/gpu/drm/i915/i915_active.c        |  1 +
>   3 files changed, 16 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> index 92558fa47108..6e4cc177cc7a 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> @@ -11,29 +11,10 @@
>   #include "i915_drv.h"
>   #include "i915_globals.h"
>   
> -static void call_idle_barriers(struct intel_engine_cs *engine)
> -{
> -	struct llist_node *node, *next;
> -
> -	llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks)) {
> -		struct dma_fence_cb *cb =
> -			container_of((struct list_head *)node,
> -				     typeof(*cb), node);
> -
> -		cb->func(NULL, cb);
> -	}
> -}
> -
>   static void i915_gem_park(struct drm_i915_private *i915)
>   {
> -	struct intel_engine_cs *engine;
> -	enum intel_engine_id id;
> -
>   	lockdep_assert_held(&i915->drm.struct_mutex);
>   
> -	for_each_engine(engine, i915, id)
> -		call_idle_barriers(engine); /* cleanup after wedging */
> -
>   	i915_vma_parked(i915);
>   
>   	i915_globals_park();
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index 65b5ca74b394..472b2259f629 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -123,6 +123,19 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
>   	return result;
>   }
>   
> +static void call_idle_barriers(struct intel_engine_cs *engine)
> +{
> +	struct llist_node *node, *next;
> +
> +	llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks)) {
> +		struct dma_fence_cb *cb =
> +			container_of((struct list_head *)node,
> +				     typeof(*cb), node);
> +
> +		cb->func(NULL, cb);
> +	}
> +}
> +
>   static int __engine_park(struct intel_wakeref *wf)
>   {
>   	struct intel_engine_cs *engine =
> @@ -142,6 +155,8 @@ static int __engine_park(struct intel_wakeref *wf)
>   
>   	GEM_TRACE("%s\n", engine->name);
>   
> +	call_idle_barriers(engine); /* cleanup after wedging */
> +
>   	intel_engine_disarm_breadcrumbs(engine);
>   	intel_engine_pool_park(&engine->pool);
>   
> diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
> index 2e2ab8176620..dcf5bc1d87e6 100644
> --- a/drivers/gpu/drm/i915/i915_active.c
> +++ b/drivers/gpu/drm/i915/i915_active.c
> @@ -679,6 +679,7 @@ void i915_active_acquire_barrier(struct i915_active *ref)
>   		rb_link_node(&node->node, parent, p);
>   		rb_insert_color(&node->node, &ref->tree);
>   
> +		GEM_BUG_ON(!intel_engine_pm_is_awake(engine));
>   		llist_add(barrier_to_ll(node), &engine->barrier_tasks);
>   		intel_engine_pm_put(engine);
>   	}
> 

If the rest plays out this is simple. :)

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

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx