On Mon, 2017-10-30 at 10:48 -0700, Rodrigo Vivi wrote: > 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@xxxxxxxxxxxxxxxxxx> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > > > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > > > > Cc: Jani Nikula <jani.nikula@xxxxxxxxx> > > > > Cc: Imre Deak <imre.deak@xxxxxxxxx> > > > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > > > > Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > > > --- > > > > > > 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. It's convenient for sure, but the unsafe module parameters seems to be finding their way into way too many HOWTOs, and from there to some "productized" use-cases. Chris states that setting .enable_rc6=0 to solving an issue on publicly shipping products has been some years ago, so I don't see a need for carrying this. We shouldn't allow the convenience of not having to change one line and recompile kernel during development to affect the end-users who are Googling how to get the best performance out of their hardware (I could mention some distro here). This seems the like the best option as I don't think introducing kernel parameters that only exists on debug builds would be too convenient either. It'd maybe just add more confusion. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
