[Intel-gfx] [PATCH 1/1] drm/i915: allow to select rc6 modes via kernel parameter

[Eugeni Dodonov]

On Feb 12, 2012 6:53 PM, "Ben Widawsky" <ben at bwidawsk.net> 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 <eugeni.dodonov at intel.com>
> > ---
> >  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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20120212/3f8c6fd4/attachment.html>