diff mbox series

drm/i915: Keep rings pinned while the context is active

Message ID 20190619134435.11141-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series drm/i915: Keep rings pinned while the context is active | expand

Commit Message

Chris Wilson June 19, 2019, 1:44 p.m. UTC 
Remember to keep the rings pinned as well as the context image until the
GPU is no longer active.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110946
Fixes: ce476c80b8bf ("drm/i915: Keep contexts pinned until after the next kernel context switch")
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/gt/intel_context.c | 17 ++++++++++++-----
 drivers/gpu/drm/i915/gt/intel_lrc.c     |  9 +--------
 2 files changed, 13 insertions(+), 13 deletions(-)

Comments

Tvrtko Ursulin June 19, 2019, 2:35 p.m. UTC | #1 
On 19/06/2019 14:44, Chris Wilson wrote:
> Remember to keep the rings pinned as well as the context image until the
> GPU is no longer active.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110946
> Fixes: ce476c80b8bf ("drm/i915: Keep contexts pinned until after the next kernel context switch")
> 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/gt/intel_context.c | 17 ++++++++++++-----
>   drivers/gpu/drm/i915/gt/intel_lrc.c     |  9 +--------
>   2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
> index 2c454f227c2e..b84f11a52d88 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.c
> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> @@ -126,6 +126,7 @@ static void intel_context_retire(struct i915_active *active)
>   	if (ce->state)
>   		__context_unpin_state(ce->state);
>   
> +	intel_ring_unpin(ce->ring);
>   	intel_context_put(ce);
>   }
>   
> @@ -160,15 +161,16 @@ int intel_context_active_acquire(struct intel_context *ce, unsigned long flags)
>   
>   	intel_context_get(ce);
>   
> +	err = intel_ring_pin(ce->ring);
> +	if (err)
> +		goto err_put;
> +
>   	if (!ce->state)
>   		return 0;
>   
>   	err = __context_pin_state(ce->state, flags);
> -	if (err) {
> -		i915_active_cancel(&ce->active);
> -		intel_context_put(ce);
> -		return err;
> -	}
> +	if (err)
> +		goto err_put;

intel_ring_unpin?

Regards,

Tvrtko

>   
>   	/* Preallocate tracking nodes */
>   	if (!i915_gem_context_is_kernel(ce->gem_context)) {
> @@ -181,6 +183,11 @@ int intel_context_active_acquire(struct intel_context *ce, unsigned long flags)
>   	}
>   
>   	return 0;
> +
> +err_put:
> +	intel_context_put(ce);
> +	i915_active_cancel(&ce->active);
> +	return err;
>   }
>   
>   void intel_context_active_release(struct intel_context *ce)
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index b42b5f158295..4bfb819386ea 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1426,7 +1426,6 @@ static void execlists_context_unpin(struct intel_context *ce)
>   {
>   	i915_gem_context_unpin_hw_id(ce->gem_context);
>   	i915_gem_object_unpin_map(ce->state->obj);
> -	intel_ring_unpin(ce->ring);
>   }
>   
>   static void
> @@ -1478,13 +1477,9 @@ __execlists_context_pin(struct intel_context *ce,
>   		goto unpin_active;
>   	}
>   
> -	ret = intel_ring_pin(ce->ring);
> -	if (ret)
> -		goto unpin_map;
> -
>   	ret = i915_gem_context_pin_hw_id(ce->gem_context);
>   	if (ret)
> -		goto unpin_ring;
> +		goto unpin_map;
>   
>   	ce->lrc_desc = lrc_descriptor(ce, engine);
>   	ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
> @@ -1492,8 +1487,6 @@ __execlists_context_pin(struct intel_context *ce,
>   
>   	return 0;
>   
> -unpin_ring:
> -	intel_ring_unpin(ce->ring);
>   unpin_map:
>   	i915_gem_object_unpin_map(ce->state->obj);
>   unpin_active:
>
Chris Wilson June 19, 2019, 2:38 p.m. UTC | #2 
Quoting Tvrtko Ursulin (2019-06-19 15:35:41)
> 
> On 19/06/2019 14:44, Chris Wilson wrote:
> > Remember to keep the rings pinned as well as the context image until the
> > GPU is no longer active.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110946
> > Fixes: ce476c80b8bf ("drm/i915: Keep contexts pinned until after the next kernel context switch")
> > 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/gt/intel_context.c | 17 ++++++++++++-----
> >   drivers/gpu/drm/i915/gt/intel_lrc.c     |  9 +--------
> >   2 files changed, 13 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
> > index 2c454f227c2e..b84f11a52d88 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_context.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> > @@ -126,6 +126,7 @@ static void intel_context_retire(struct i915_active *active)
> >       if (ce->state)
> >               __context_unpin_state(ce->state);
> >   
> > +     intel_ring_unpin(ce->ring);
> >       intel_context_put(ce);
> >   }
> >   
> > @@ -160,15 +161,16 @@ int intel_context_active_acquire(struct intel_context *ce, unsigned long flags)
> >   
> >       intel_context_get(ce);
> >   
> > +     err = intel_ring_pin(ce->ring);
> > +     if (err)
> > +             goto err_put;
> > +
> >       if (!ce->state)
> >               return 0;
> >   
> >       err = __context_pin_state(ce->state, flags);
> > -     if (err) {
> > -             i915_active_cancel(&ce->active);
> > -             intel_context_put(ce);
> > -             return err;
> > -     }
> > +     if (err)
> > +             goto err_put;
> 
> intel_ring_unpin?

Gah I did... A mistake when rebasing :(
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 2c454f227c2e..b84f11a52d88 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -126,6 +126,7 @@  static void intel_context_retire(struct i915_active *active)
 	if (ce->state)
 		__context_unpin_state(ce->state);
 
+	intel_ring_unpin(ce->ring);
 	intel_context_put(ce);
 }
 
@@ -160,15 +161,16 @@  int intel_context_active_acquire(struct intel_context *ce, unsigned long flags)
 
 	intel_context_get(ce);
 
+	err = intel_ring_pin(ce->ring);
+	if (err)
+		goto err_put;
+
 	if (!ce->state)
 		return 0;
 
 	err = __context_pin_state(ce->state, flags);
-	if (err) {
-		i915_active_cancel(&ce->active);
-		intel_context_put(ce);
-		return err;
-	}
+	if (err)
+		goto err_put;
 
 	/* Preallocate tracking nodes */
 	if (!i915_gem_context_is_kernel(ce->gem_context)) {
@@ -181,6 +183,11 @@  int intel_context_active_acquire(struct intel_context *ce, unsigned long flags)
 	}
 
 	return 0;
+
+err_put:
+	intel_context_put(ce);
+	i915_active_cancel(&ce->active);
+	return err;
 }
 
 void intel_context_active_release(struct intel_context *ce)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index b42b5f158295..4bfb819386ea 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1426,7 +1426,6 @@  static void execlists_context_unpin(struct intel_context *ce)
 {
 	i915_gem_context_unpin_hw_id(ce->gem_context);
 	i915_gem_object_unpin_map(ce->state->obj);
-	intel_ring_unpin(ce->ring);
 }
 
 static void
@@ -1478,13 +1477,9 @@  __execlists_context_pin(struct intel_context *ce,
 		goto unpin_active;
 	}
 
-	ret = intel_ring_pin(ce->ring);
-	if (ret)
-		goto unpin_map;
-
 	ret = i915_gem_context_pin_hw_id(ce->gem_context);
 	if (ret)
-		goto unpin_ring;
+		goto unpin_map;
 
 	ce->lrc_desc = lrc_descriptor(ce, engine);
 	ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
@@ -1492,8 +1487,6 @@  __execlists_context_pin(struct intel_context *ce,
 
 	return 0;
 
-unpin_ring:
-	intel_ring_unpin(ce->ring);
 unpin_map:
 	i915_gem_object_unpin_map(ce->state->obj);
 unpin_active: