[PATCH 1/2] drm/i915/gt: Drop false assertion on user_forcewake

[PATCH 1/2] drm/i915/gt: Drop false assertion on user_forcewake

[Chris Wilson]

The counter is removed from the pm wakeref count, but it remains intact
so that we can restore it upon resume. Ergo inside suspend, it may have
a value.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_gt_pm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index 6374744bb65e..060a27d9af34 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -251,7 +251,6 @@ static void wait_for_suspend(struct intel_gt *gt)
 		intel_gt_set_wedged(gt);
 	}
 
-	GEM_BUG_ON(atomic_read(&gt->user_wakeref));
 	intel_gt_pm_wait_for_idle(gt);
 }
 
-- 
2.24.0.rc2

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

[Intel-gfx] [PATCH 1/2] drm/i915/gt: Drop false assertion on user_forcewake

[Chris Wilson]

The counter is removed from the pm wakeref count, but it remains intact
so that we can restore it upon resume. Ergo inside suspend, it may have
a value.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_gt_pm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index 6374744bb65e..060a27d9af34 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -251,7 +251,6 @@ static void wait_for_suspend(struct intel_gt *gt)
 		intel_gt_set_wedged(gt);
 	}
 
-	GEM_BUG_ON(atomic_read(&gt->user_wakeref));
 	intel_gt_pm_wait_for_idle(gt);
 }
 
-- 
2.24.0.rc2

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

[PATCH 2/2] drm/i915: Protect request peeking with RCU

[Chris Wilson]

Since the execlists_active() is no longer protected by the
engine->active.lock, we need to protect the request pointer with RCU to
prevent it being freed as we evaluate whether or not we need to preempt.

Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_scheduler.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index d2edb527dcb8..010d67f48ad9 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -202,21 +202,26 @@ static void kick_submission(struct intel_engine_cs *engine,
 	if (prio <= engine->execlists.queue_priority_hint)
 		return;
 
+	rcu_read_lock();
+
 	/* Nothing currently active? We're overdue for a submission! */
 	inflight = execlists_active(&engine->execlists);
 	if (!inflight)
-		return;
+		goto unlock;
 
 	/*
 	 * If we are already the currently executing context, don't
 	 * bother evaluating if we should preempt ourselves.
 	 */
 	if (inflight->hw_context == rq->hw_context)
-		return;
+		goto unlock;
 
 	engine->execlists.queue_priority_hint = prio;
 	if (need_preempt(prio, rq_prio(inflight)))
 		tasklet_hi_schedule(&engine->execlists.tasklet);
+
+unlock:
+	rcu_read_unlock();
 }
 
 static void __i915_schedule(struct i915_sched_node *node,
-- 
2.24.0.rc2

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

[Intel-gfx] [PATCH 2/2] drm/i915: Protect request peeking with RCU

[Chris Wilson]

Since the execlists_active() is no longer protected by the
engine->active.lock, we need to protect the request pointer with RCU to
prevent it being freed as we evaluate whether or not we need to preempt.

Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_scheduler.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index d2edb527dcb8..010d67f48ad9 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -202,21 +202,26 @@ static void kick_submission(struct intel_engine_cs *engine,
 	if (prio <= engine->execlists.queue_priority_hint)
 		return;
 
+	rcu_read_lock();
+
 	/* Nothing currently active? We're overdue for a submission! */
 	inflight = execlists_active(&engine->execlists);
 	if (!inflight)
-		return;
+		goto unlock;
 
 	/*
 	 * If we are already the currently executing context, don't
 	 * bother evaluating if we should preempt ourselves.
 	 */
 	if (inflight->hw_context == rq->hw_context)
-		return;
+		goto unlock;
 
 	engine->execlists.queue_priority_hint = prio;
 	if (need_preempt(prio, rq_prio(inflight)))
 		tasklet_hi_schedule(&engine->execlists.tasklet);
+
+unlock:
+	rcu_read_unlock();
 }
 
 static void __i915_schedule(struct i915_sched_node *node,
-- 
2.24.0.rc2

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

Re: [PATCH 1/2] drm/i915/gt: Drop false assertion on user_forcewake

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> The counter is removed from the pm wakeref count, but it remains intact
> so that we can restore it upon resume. Ergo inside suspend, it may have
> a value.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

oh, user_forcewake needs a better name with a verb...

> ---
>  drivers/gpu/drm/i915/gt/intel_gt_pm.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> index 6374744bb65e..060a27d9af34 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -251,7 +251,6 @@ static void wait_for_suspend(struct intel_gt *gt)
>  		intel_gt_set_wedged(gt);
>  	}
>  
> -	GEM_BUG_ON(atomic_read(&gt->user_wakeref));
>  	intel_gt_pm_wait_for_idle(gt);
>  }
>  
> -- 
> 2.24.0.rc2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/gt: Drop false assertion on user_forcewake

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> The counter is removed from the pm wakeref count, but it remains intact
> so that we can restore it upon resume. Ergo inside suspend, it may have
> a value.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

oh, user_forcewake needs a better name with a verb...

> ---
>  drivers/gpu/drm/i915/gt/intel_gt_pm.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> index 6374744bb65e..060a27d9af34 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -251,7 +251,6 @@ static void wait_for_suspend(struct intel_gt *gt)
>  		intel_gt_set_wedged(gt);
>  	}
>  
> -	GEM_BUG_ON(atomic_read(&gt->user_wakeref));
>  	intel_gt_pm_wait_for_idle(gt);
>  }
>  
> -- 
> 2.24.0.rc2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Protect request peeking with RCU

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Since the execlists_active() is no longer protected by the
> engine->active.lock, we need to protect the request pointer with RCU to
> prevent it being freed as we evaluate whether or not we need to preempt.
>
> Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_scheduler.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
> index d2edb527dcb8..010d67f48ad9 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler.c
> +++ b/drivers/gpu/drm/i915/i915_scheduler.c
> @@ -202,21 +202,26 @@ static void kick_submission(struct intel_engine_cs *engine,
>  	if (prio <= engine->execlists.queue_priority_hint)
>  		return;
>  
> +	rcu_read_lock();
> +
>  	/* Nothing currently active? We're overdue for a submission! */
>  	inflight = execlists_active(&engine->execlists);
>  	if (!inflight)
> -		return;
> +		goto unlock;
>  
>  	/*
>  	 * If we are already the currently executing context, don't
>  	 * bother evaluating if we should preempt ourselves.
>  	 */
>  	if (inflight->hw_context == rq->hw_context)
> -		return;
> +		goto unlock;
>  
>  	engine->execlists.queue_priority_hint = prio;
>  	if (need_preempt(prio, rq_prio(inflight)))
>  		tasklet_hi_schedule(&engine->execlists.tasklet);
> +
> +unlock:
> +	rcu_read_unlock();
>  }
>  
>  static void __i915_schedule(struct i915_sched_node *node,
> -- 
> 2.24.0.rc2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Protect request peeking with RCU

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Since the execlists_active() is no longer protected by the
> engine->active.lock, we need to protect the request pointer with RCU to
> prevent it being freed as we evaluate whether or not we need to preempt.
>
> Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_scheduler.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
> index d2edb527dcb8..010d67f48ad9 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler.c
> +++ b/drivers/gpu/drm/i915/i915_scheduler.c
> @@ -202,21 +202,26 @@ static void kick_submission(struct intel_engine_cs *engine,
>  	if (prio <= engine->execlists.queue_priority_hint)
>  		return;
>  
> +	rcu_read_lock();
> +
>  	/* Nothing currently active? We're overdue for a submission! */
>  	inflight = execlists_active(&engine->execlists);
>  	if (!inflight)
> -		return;
> +		goto unlock;
>  
>  	/*
>  	 * If we are already the currently executing context, don't
>  	 * bother evaluating if we should preempt ourselves.
>  	 */
>  	if (inflight->hw_context == rq->hw_context)
> -		return;
> +		goto unlock;
>  
>  	engine->execlists.queue_priority_hint = prio;
>  	if (need_preempt(prio, rq_prio(inflight)))
>  		tasklet_hi_schedule(&engine->execlists.tasklet);
> +
> +unlock:
> +	rcu_read_unlock();
>  }
>  
>  static void __i915_schedule(struct i915_sched_node *node,
> -- 
> 2.24.0.rc2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx