Re: [PATCH] drm/i915: reset forcewake count after reset

[Ben Widawsky <ben@bwidawsk.net>]

On Fri, Jun 24, 2011 at 12:45:27AM +0100, Chris Wilson wrote:
> On Thu, 23 Jun 2011 16:06:22 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > 
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 0defd42..9292499 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -579,6 +579,7 @@ int i915_reset(struct drm_device *dev, u8 flags)
> >  	} else switch (INTEL_INFO(dev)->gen) {
> >  	case 6:
> >  		ret = gen6_do_reset(dev, flags);
> > +		atomic_set(&dev_priv->forcewake_count, 0);
> >  		break;
> >  	case 5:
> >  		ret = ironlake_do_reset(dev, flags);
> 
> Can forcewake be non-zero here? If it has been bumped by a user wakelock,
> then what happens when that is subsequently released? I don't think this
> is safe...
> 
> What scenario are you trying to fix?
> -Chris

This is not the cleanest fix, but the problem is the following:

1. User bumps refcount
2. GPU hangs
3. Reset occurs
4. User doesn't close the file (or even the race before the user closes
   the file after the reset) the driver is now completely screwed in
   this case, once the user does close the file, things will go back to
   normal.

I was actually just about to respond to my original email to say this
belongs in -fixes (unless I'm confused).

Ben