[Intel-gfx] [PATCH v3] drm/i915: Remove unsafe i915.enable_rc6

[Rodrigo Vivi]

On Mon, Oct 30, 2017 at 01:00:51PM +0000, David Weinehall wrote:
> On Fri, Oct 27, 2017 at 01:57:09PM -0700, Daniele Ceraolo Spurio wrote:
> >
> >
> > On 26/10/17 03:32, Chris Wilson wrote:
> > > It has been many years since the last confirmed sighting (and fix) of an
> > > RC6 related bug (usually a system hang). Remove the parameter to stop
> > > users from setting dangerous values, as they often set it during triage
> > > and end up disabling the entire runtime pm instead (the option is not a
> > > fine scalpel!).
> > >
> > > Furthermore, it allows users to set known dangerous values which were
> > > intended for testing and not for production use. For testing, we can
> > > always patch in the required setting without having to expose ourselves
> > > to random abuse.
> > >
> > > v2: Fixup NEEDS_WaRsDisableCoarsePowerGating fumble, and document the
> > > lack of ilk support better.
> > > v3: Clear intel_info->rc6p if we don't support rc6 itself.
> > >
> > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > > Cc: Jani Nikula <jani.nikula at intel.com>
> > > Cc: Imre Deak <imre.deak at intel.com>
> > > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > ---
> >
> > I think that for execution/debug on early silicon we might still want the
> > ability to turn features like RC6 off. Maybe we can add a debug kconfig to
> > force info->has_rc6 = 0? Not a blocker to this patch but worth considering
> > IMO.
>
> Most of the BIOSes I've seen on our RVPs have had an option to disable
> RC6.

BIOS option don't block our code to run and set some MMIOs.
Not sure how the GPU will behave on such cases.

I like the idea of removing some and keeping the parameters clean.
But there are few ones like RC6 and disable_power_wells that are very
useful on platform enabling and also when assisting others to debug issues.

For instance right now that we fixed RC6 on CNL someone told that
he believe seeing more hangs, so I immediately asked to boot with
i915.enable_rc6=0 to double check. It is easier and straighforward
to direct them to the unsafe param than to ask them to compile the code
with different options or to use some BIOS options that we are not sure.

Also on bug triage some options like this are helpful.

Also BIOS and compile are saved flags. So if you need to do a quick test
you have to save it, and then unsave later. Parameters are very convinient
for 1 boot only check.

Maybe we could clean and remove the different options and levels and
let it be just a boolean.

Thanks,
Rodrigo.

>
>
> Kind regards, David