[PATCH] drm/i915: Remove ctx->last_ring

[PATCH] drm/i915: Remove ctx->last_ring

[oscar.mateo]

From: Oscar Mateo <oscar.mateo@intel.com>

The original comment that introduced it said:

commit 0009e46cd54324c4af20b0b52b89973b1b914167
Author: Ben Widawsky <ben@bwidawsk.net>
Date:   Fri Dec 6 14:11:02 2013 -0800

    drm/i915: Track which ring a context ran on

    Previously we dropped the association of a context to a ring. It is
    however very important to know which ring a context ran on (we could
    have reused the other member, but I was nitpicky).

    This is very important when we switch address spaces, which unlike
    context objects, do change per ring.

    As an example, if we have:

            RCS   BCS
    ctx            A
    ctx      A
    ctx      B
    ctx            B

    Without tracking the last ring B ran on, we wouldn't know to switch the
    address space on BCS in the last row.

But this is not really true, because we are already checking to != from (with
"from" being = ring->last_context) and that should be enough to make sure we
switch to the right address space.

We would have a problem if we switched the context object for every ring (since
then we would fail to do it in some situations) but we only switch it for the
render ring, so we don't care.

Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         | 1 -
 drivers/gpu/drm/i915/i915_gem_context.c | 3 +--
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3b3fe04..9f402cb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -591,7 +591,6 @@ struct intel_context {
 	bool is_initialized;
 	uint8_t remap_slice;
 	struct drm_i915_file_private *file_priv;
-	struct intel_engine_cs *last_ring;
 	struct drm_i915_gem_object *obj;
 	struct i915_ctx_hang_stats hang_stats;
 	struct i915_address_space *vm;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index bf07d9d..5a62a19 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -598,7 +598,7 @@ static int do_switch(struct intel_engine_cs *ring,
 		BUG_ON(!i915_gem_obj_is_pinned(from->obj));
 	}
 
-	if (from == to && from->last_ring == ring && !to->remap_slice)
+	if (from == to && !to->remap_slice)
 		return 0;
 
 	/* Trying to pin first makes error handling easier. */
@@ -692,7 +692,6 @@ static int do_switch(struct intel_engine_cs *ring,
 done:
 	i915_gem_context_reference(to);
 	ring->last_context = to;
-	to->last_ring = ring;
 
 	if (ring->id == RCS && !to->is_initialized && from == NULL) {
 		ret = i915_gem_render_state_init(ring);
-- 
1.9.0

Re: [PATCH] drm/i915: Remove ctx->last_ring

[Chris Wilson]

On Wed, Jun 18, 2014 at 05:16:03PM +0100, oscar.mateo@intel.com wrote:
> From: Oscar Mateo <oscar.mateo@intel.com>
> 
> The original comment that introduced it said:
> 
> commit 0009e46cd54324c4af20b0b52b89973b1b914167
> Author: Ben Widawsky <ben@bwidawsk.net>
> Date:   Fri Dec 6 14:11:02 2013 -0800
> 
>     drm/i915: Track which ring a context ran on
> 
>     Previously we dropped the association of a context to a ring. It is
>     however very important to know which ring a context ran on (we could
>     have reused the other member, but I was nitpicky).
> 
>     This is very important when we switch address spaces, which unlike
>     context objects, do change per ring.
> 
>     As an example, if we have:
> 
>             RCS   BCS
>     ctx            A
>     ctx      A
>     ctx      B
>     ctx            B
> 
>     Without tracking the last ring B ran on, we wouldn't know to switch the
>     address space on BCS in the last row.
> 
> But this is not really true, because we are already checking to != from (with
> "from" being = ring->last_context) and that should be enough to make sure we
> switch to the right address space.
> 
> We would have a problem if we switched the context object for every ring (since
> then we would fail to do it in some situations) but we only switch it for the
> render ring, so we don't care.
> 
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>

I have sent the very same patch, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Remove ctx->last_ring

[Daniel Vetter]

On Wed, Jun 18, 2014 at 05:34:49PM +0100, Chris Wilson wrote:
> On Wed, Jun 18, 2014 at 05:16:03PM +0100, oscar.mateo@intel.com wrote:
> > From: Oscar Mateo <oscar.mateo@intel.com>
> > 
> > The original comment that introduced it said:
> > 
> > commit 0009e46cd54324c4af20b0b52b89973b1b914167
> > Author: Ben Widawsky <ben@bwidawsk.net>
> > Date:   Fri Dec 6 14:11:02 2013 -0800
> > 
> >     drm/i915: Track which ring a context ran on
> > 
> >     Previously we dropped the association of a context to a ring. It is
> >     however very important to know which ring a context ran on (we could
> >     have reused the other member, but I was nitpicky).
> > 
> >     This is very important when we switch address spaces, which unlike
> >     context objects, do change per ring.
> > 
> >     As an example, if we have:
> > 
> >             RCS   BCS
> >     ctx            A
> >     ctx      A
> >     ctx      B
> >     ctx            B
> > 
> >     Without tracking the last ring B ran on, we wouldn't know to switch the
> >     address space on BCS in the last row.
> > 
> > But this is not really true, because we are already checking to != from (with
> > "from" being = ring->last_context) and that should be enough to make sure we
> > switch to the right address space.
> > 
> > We would have a problem if we switched the context object for every ring (since
> > then we would fail to do it in some situations) but we only switch it for the
> > render ring, so we don't care.
> > 
> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> 
> I have sent the very same patch, so
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch