Re: [PATCH] drm/i915: Restore rps/rc6 on reset

["Mcgee, Jeff" <jeff.mcgee@intel.com>]

On Tue, 2013-11-05 at 20:38 +0000, Mcgee, Jeff wrote:
> On Mon, 2013-11-04 at 09:44 +0100, Daniel Vetter wrote:
> > On Thu, Oct 31, 2013 at 8:52 PM,  <jeff.mcgee@intel.com> wrote:
> > > From: Jeff McGee <jeff.mcgee@intel.com>
> > >
> > > A check of rps/rc6 state after i915_reset determined that the ring
> > > MAX_IDLE registers were returned to their hardware defaults and that
> > > the GEN6_PMIMR register was set to mask all interrupts. This change
> > > restores those values to their pre-reset states by re-initializing
> > > rps/rc6 in i915_reset. A full re-initialization was opted for versus
> > > a targeted set of restore operations for simplicity and maintain-
> > > ability. Note that the re-initialization is not done for Ironlake,
> > > due to a past comment that it causes problems.
> > >
> > > Also updated the rps initialization sequence to preserve existing
> > > min/max values in the case of a re-init. We assume the values were
> > > validated upon being set and do not do further range checking. The
> > > debugfs interface for changing min/max was updated with range
> > > checking to ensure this condition (already present in sysfs
> > > interface).
> > >
> > > Issue: VIZ-3142
> > > Issue: AXIA-4676
> > > OTC-Tracker: VIZ-3143
> > > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> > 
> > Can I have a testcase in i-g-t for this please? I think the following
> > should work:
> > 
> > 1. Throw a dummy load onto the gpu, check that cagf goes up.
> > 
> > 2. Limit min/max to a non-default value (and install an igt atexit
> > handler to undo this).
> > 
> > 3. Throw a dummy load onto the gpu, check that cagf jumps from the
> > idle freq to the selected one directly.
> > 
> > 4. Inject a gpu hang with the stop_rings stuff (see e.g. kms_flip.c or
> > ZZ_hangman).
> > 
> > 5. Reject that the limts still work as in step 3.
> > 
> > Cheers, Daniel
> 
> I'll see what can be done. I understand the emphasis on adding
> formalized tests. There will have to be some resourcing discussions on
> the product side if this is now a requirement for upstream patch
> acceptance.
> 
> Jeff

This discussion is starting in VPG Android, but it may be a while until
test code gets submitted. This is a fairly serious bug and easy to
reproduce manually. So please consider the patch without the test case
for now. Thanks

Jeff