Re: [PATCH] drm/i915: reset GPU after clock gating init

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Thu, 15 Nov 2012 12:10:28 -0800
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Thu, 15 Nov 2012 21:05:04 +0100
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Thu, Nov 15, 2012 at 7:24 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > > This is needed for SNB at least after disabling CSunit clock gating, and
> > > shouldn't hurt on other platforms either.
> > >
> > > This fixes an issue on James's machine where RC6 wouldn't always get
> > > enabled.
> > >
> > > Tested-by: James Kukunas <james.t.kukunas@intel.com>
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c |    1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 6d8a5ed..2fccd8f 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -8718,6 +8718,7 @@ void intel_modeset_init_hw(struct drm_device *dev)
> > >         intel_prepare_ddi(dev);
> > >
> > >         intel_init_clock_gating(dev);
> > > +       intel_gpu_reset(dev);
> > 
> > Unconditionally resetting machines tends to upset users, especially
> > since it isn't implemented or doesn't work on gen2/3, leaving a wedged
> > machine behind. And it spews ERRORs into dmesg. Also, a tiny comment
> > explaining what's going on would be preferable.
> > 
> > I think the better approach is to either call the low-level reset
> > function from the relevant clock-gating callbacks (maybe shovel the
> > reset functions into dev_priv->gt.reset while at it). Or alternatively
> > figure out what's wrong with our init sequence and reorder things
> > (maybe that specific w/a needs to happen before we enable rings, it
> > would not be the first one).
> 
> No this workaround is specifically: disable CSunit, reset render.
> 
> Yeah it can be stuffed in the gen6 clock gating init, or even pushed
> off into the GT rps init work handler, since it may take awhile.  I'll
> clean it up.
> 

Apparently this causes performance regressions on the Dell machines it
"fixes".  Can anyone else confirm that?  Maybe we're not restoring some
state after reset which causes trouble...

-- 
Jesse Barnes, Intel Open Source Technology Center