Re: [PATCH v3 3/3] drm/i915: Enable Resource Streamer state save/restore in HSW

Chris Wilson
2015-05-18T15:41:58Z

On Mon, May 18, 2015 at 06:36:18PM +0300, Ville Syrjälä wrote:
> On Mon, May 18, 2015 at 11:31:56AM +0300, Abdiel Janulgue wrote:
> > Also clarify comments on context size that the extra state for
> > Resource Streamer is included.
> >
> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
> > drivers/gpu/drm/i915/i915_reg.h | 3 ++-
> > 2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> > index f3e84c4..1db107a 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -509,7 +509,7 @@ mi_set_context(struct intel_engine_cs *ring,
> > }
> >
> > /* These flags are for resource streamer on HSW+ */
> > - if (!IS_HASWELL(ring->dev) && INTEL_INFO(ring->dev)->gen < 8)
> > + if (IS_HASWELL(ring->dev))
> > flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);
>
> I don't get it. Previously we told the hardware to save the extended
> context on !hsw, and now we don't. That doesn't seem correct to me.

We don't use the extended state elsewhere. I'd always been dubious of
the origins/intentions of this line of code since it claims only to be
for enabling RS on HSW...

i.e. commit e80f14b6d36e3e07111cf2ab084ef8dd5d015ce2
Author: Ben Widawsky <benjamin.widawsky@intel.com>
Date: Mon Aug 18 10:35:28 2014 -0700

drm/i915: Don't save/restore RS when not used

was backwards.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre