On Feb 12, 2012 6:53 PM, "Ben Widawsky" <<a href="mailto:ben@bwidawsk.net">ben@bwidawsk.net</a>> wrote: > > On 02/11/2012 08:23 PM, Eugeni Dodonov wrote: > > This allows to select which rc6 modes are to be used via kernel parameter, > > via a bitmask parameter. E.g.: > > > > - to enable rc6, i915_enable_rc6=1 > > - to enable rc6 and deep rc6, i915_enable_rc6=3 > > - to enable rc6 and deepest rc6, use i915_enable_rc6=5 > > - to enable rc6, deep and deepest rc6, use i915_enable_rc6=7 > > > > Please keep in mind that the deepest RC6 state really should NOT be used > > by default, as it could potentially worsen the issues with deep RC6. So do > > enable it only when you know what you are doing. However, having it around > > could help solving possible future rc6-related issues and their debugging > > on user machines. > > > > Note that this changes behavior - previously, value of 1 would enable both > > RC6 and deep RC6. Now it should only enable RC6 and deep/deepest RC6 > > stages must be enabled manually. > > > > v2: address Chris Wilson comments and clean up the code. > > > > Signed-off-by: Eugeni Dodonov <<a href="mailto:eugeni.dodonov@intel.com">eugeni.dodonov@intel.com</a>> > > --- > > drivers/gpu/drm/i915/i915_drv.c | 6 +++++- > > drivers/gpu/drm/i915/i915_drv.h | 21 +++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++---- > > 3 files changed, 42 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index a1103fc..b7a91db 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -66,7 +66,11 @@ MODULE_PARM_DESC(semaphores, > > int i915_enable_rc6 __read_mostly = -1; > > module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0600); > > MODULE_PARM_DESC(i915_enable_rc6, > > - "Enable power-saving render C-state 6 (default: -1 (use per-chip default)"); > > + "Enable power-saving render C-state 6. " > > + "Different stages can be selected via bitmask values " > > + "(0 = disable; 1 = enable rc6; 2 = enable deep rc6; 4 = enable deepest rc6). " > > + "For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. " > > + "default: -1 (use per-chip default)"); > > > > int i915_enable_fbc __read_mostly = -1; > > module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 554bef7..c17bccf 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -997,6 +997,27 @@ struct drm_i915_file_private { > > > > #include "i915_trace.h" > > > > +/** > > + * RC6 is a special power stage which allows the GPU to enter an very > > + * low-voltage mode when idle, using down to 0V while at this stage. This > > + * stage is entered automatically when the GPU is idle when RC6 support is > > + * enabled, and as soon as new workload arises GPU wakes up automatically as well. > > + * > > + * There are different RC6 modes available in Intel GPU, which differentiate > > + * among each other with the latency required to enter and leave RC6 and > > + * voltage consumed by the GPU in different states. > > + * > > + * The combination of the following flags define which states GPU is allowed > > + * to enter, while RC6 is the normal RC6 state, RC6p is the deep RC6, and > > + * RC6pp is deepest RC6. Their support by hardware varies according to the > > + * GPU, BIOS, chipset and platform. RC6 is usually the safest one and the one > > + * which brings the most power savings; deeper states save more power, but > > + * require higher latency to switch to and wake up. > > + */ > > +#define INTEL_RC6_ENABLE (1<<0) > > +#define INTEL_RC6p_ENABLE (1<<1) > > +#define INTEL_RC6pp_ENABLE (1<<2) > > + > > extern struct drm_ioctl_desc i915_ioctls[]; > > extern int i915_max_ioctl; > > extern unsigned int i915_fbpercrtc __always_unused; > > Does it ever make sense to enable a deeper state without enabling the > one before it? Ie. would we ever enable RC6p and not RC6? Maybe we > don't want to have discrete bitfields and instead do: > > +#define INTEL_RC6_ENABLE (1<<0) > > +#define INTEL_RC6p_ENABLE (1<<1) | INTEL_RC6_ENABLE > > +#define INTEL_RC6pp_ENABLE (1<<2) | INTEL_RC6p_ENABLE > > Just a thought for the sake of hearing myself speak. We checked with Jesse, and these states are independent.. So it should be possible to have RC6p and not plain RC6.