[Intel-gfx] [PATCH] drm/i915: Empty the ring before disabling

[Chris Wilson]

Quoting Mika Kuoppala (2017-10-27 07:44:31)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > An interesting snippet from Sandybridge's prm:
> >
> > "Although a Ring Buffer can be enabled in the non-empty state, it must
> > not be disabled unless it is empty. Attempting to disable a Ring Buffer
> > in the non-empty state is UNDEFINED."
> >
> > Let's avoid the undefined behaviour as we disable the rings prior to
> > reset and resume.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +++-
> >  drivers/gpu/drm/i915/intel_uncore.c     | 4 +++-
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 05e01446b00b..3f2073a9d37a 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -480,10 +480,12 @@ static bool stop_ring(struct intel_engine_cs *engine)
> >               }
> >       }
> >  
> > -     I915_WRITE_CTL(engine, 0);
> >       I915_WRITE_HEAD(engine, 0);
> >       I915_WRITE_TAIL(engine, 0);
> >  
> > +     /* The ring must be empty before it is disabled */
> > +     I915_WRITE_CTL(engine, 0);
> > +
> 
> I am thinking that does it need even more finesse.

I thought so too, but we already stop the ring, and so gave up thinking.

> Like
> 
> I915_WRITE_HEAD(engine, I915_READ_TAIL(engine));

Nevertheless, I'll give that a spin since that's neat ;)
-Chris