[Intel-gfx] [PATCH v3 21/21] drm/i915: Unify GPU resets upon shutdown

[Ville Syrjälä]

On Mon, Apr 25, 2016 at 06:45:29PM +0100, Chris Wilson wrote:
> On Mon, Apr 25, 2016 at 08:25:14PM +0300, Ville Syrjälä wrote:
> > On Sun, Apr 24, 2016 at 07:10:19PM +0100, Chris Wilson wrote:
> > > Both execlists and legacy need to reset the context (and mode) of the
> > > GPU before we lose control of the system. By resetting the GPU, we
> > > revert back to default settings. This simplifies the life of any
> > > subsequent driver (in particular for virtualized setups) as it does not
> > > then have to try and recover from an unknown condition. As both paths
> > > need to reset for the same reason, move the reset to a common point.
> > > 
> > > This unifies the resets added in a647828afc (drm/i915: Also perform gpu
> > > reset under execlist mode) and 8e96d9c4d9 (drm/i915: reset the GPU on
> > > context fini).
> > > 
> > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> > > CC: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > > Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> > > Cc: "Niu, Bing" <bing.niu at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_dma.c         | 41 ++++++++++++++++++++++++++-------
> > >  drivers/gpu/drm/i915/i915_gem.c         |  8 -------
> > >  drivers/gpu/drm/i915/i915_gem_context.c |  8 +------
> > >  3 files changed, 34 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > > index 5c7615041b31..68c60f4fd44d 100644
> > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > @@ -425,6 +425,37 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
> > >  	.can_switch = i915_switcheroo_can_switch,
> > >  };
> > >  
> > > +static void i915_gem_fini(struct drm_device *dev)
> > > +{
> > > +	int reset;
> > > +
> > > +	/*
> > > +	 * Neither the BIOS, ourselves or any other kernel
> > > +	 * expects the system to be in execlists mode on startup,
> > > +	 * so we need to reset the GPU back to legacy mode. And the only
> > > +	 * known way to disable logical contexts is through a GPU reset.
> > > +	 *
> > > +	 * So in order to leave the system in a known default configuration,
> > > +	 * always reset the GPU upon unload. Afterwards we then clean up the
> > > +	 * GEM state tracking, flushing off the requests and leaving the
> > > +	 * system in a known idle state.
> > > +	 *
> > > +	 * Note that is of the upmost importance that the GPU is idle and
> > > +	 * all stray writes are flushed *before* we dismantle the backing
> > > +	 * storage for the pinned objects.
> > > +	 */
> > > +	reset = intel_gpu_reset(dev, ALL_ENGINES);
> > 
> > That's going to kill pre-g4x macines I think. At least until Maarten's
> > reset locking fixes go in. Even then I'm not sure we want to do that on
> > those old machines.
> 
> Reset locking in shutdown? intel_gpu_reset() is just the register poke.
> We can limit it to say gen5 to be safe?

Oh, that's the low level thing. Right. Yeah, I'm not sure yanking the
rug from under the display is entirely safe, which is why the reset code
shuts the display down around the reset. If we don't want to do that
here, then skipping the reset on pre-g4x seems safer.

-- 
Ville Syrjälä
Intel OTC