On to, 2015-11-12 at 13:51 +0100, Patrik Jakobsson wrote:
> On Wed, Nov 11, 2015 at 09:04:09PM +0200, Imre Deak wrote:
> > On ma, 2015-11-09 at 16:48 +0100, Patrik Jakobsson wrote:
> > > v2: Use _unsafe (Jani)
> > >
> > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@xxxxxxxxxxxxxxx
> > > >
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > > drivers/gpu/drm/i915/i915_params.c | 6 ++++++
> > > drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++--
> > > 3 files changed, 9 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index c0252ef..5628c5a 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2639,6 +2639,7 @@ struct i915_params {
> > > int panel_use_ssc;
> > > int vbt_sdvo_panel_type;
> > > int enable_rc6;
> > > + int enable_dc6;
> > > int enable_fbc;
> > > int enable_ppgtt;
> > > int enable_execlists;
> > > diff --git a/drivers/gpu/drm/i915/i915_params.c
> > > b/drivers/gpu/drm/i915/i915_params.c
> > > index 368df67..6457f3a 100644
> > > --- a/drivers/gpu/drm/i915/i915_params.c
> > > +++ b/drivers/gpu/drm/i915/i915_params.c
> > > @@ -32,6 +32,7 @@ struct i915_params i915 __read_mostly = {
> > > .panel_use_ssc = -1,
> > > .vbt_sdvo_panel_type = -1,
> > > .enable_rc6 = -1,
> > > + .enable_dc6 = 1,
> > > .enable_fbc = -1,
> > > .enable_execlists = -1,
> > > .enable_hangcheck = true,
> > > @@ -79,6 +80,11 @@ MODULE_PARM_DESC(enable_rc6,
> > > "For example, 3 would enable rc6 and deep rc6, and 7
> > > would
> > > enable everything. "
> > > "default: -1 (use per-chip default)");
> > >
> > > +module_param_named_unsafe(enable_dc6, i915.enable_dc6, int,
> > > 0400);
> > > +MODULE_PARM_DESC(enable_dc6,
> > > + "Enable power-saving display C-state 6. "
> > > + "(0 = disable; 1 = enable [default])");
> > > +
> >
> > It would be more generic to have something like enable_dc, -1=per
> > -chip
> > default, 0=disable, 1=up to dc5, 2=up to dc6.
>
> I'm not sure if this parameter is going to stay for long but if it
> does I
> suppose we should have DC9 as well.
Yea, we could extend this in case we make DC9 a power well.
> But do we really need this level of control?
> My intention was to work around the DC6 corner case. Do you think
> we could make good use of a more generic interface?
This is an experimental functionality and have device wide effects (at
least display wide), so I wouldn't be surprised if we can make a good
use of a more fine grained control here.
> Perhaps useful for testing? If so, I
> definitely think we should go with your more generic solution.
> Otherwise I'd
> rather keep it simple. Feel free to override my decision here.
>
> Also, what would 0=disable be? Not starting the DMC at all or DC3/4?
Just not enabling DC5/6 ever in gen9_dc_off_power_well_disable(). The
idea would be to test if some failure is caused by DC5 for example.
> > module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600);
> > > MODULE_PARM_DESC(enable_fbc,
> > > "Enable frame buffer compression for power savings "
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index 95c3fcc..62c1273 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -708,7 +708,7 @@ static void
> > > gen9_dc_off_power_well_enable(struct
> > > drm_i915_private *dev_priv,
> > > static void gen9_dc_off_power_well_disable(struct
> > > drm_i915_private
> > > *dev_priv,
> > > struct i915_power_well
> > > *power_well)
> > > {
> > > - if (IS_SKYLAKE(dev_priv))
> > > + if (IS_SKYLAKE(dev_priv) && i915.enable_dc6)
> > > skl_enable_dc6(dev_priv);
> > > else
> > > gen9_enable_dc5(dev_priv);
> > > @@ -720,7 +720,7 @@ static void
> > > gen9_dc_off_power_well_sync_hw(struct
> > > drm_i915_private *dev_priv,
> > > if (power_well->count > 0) {
> > > gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > > } else {
> > > - if (IS_SKYLAKE(dev_priv))
> > > + if (IS_SKYLAKE(dev_priv) && i915.enable_dc6)
> > > gen9_set_dc_state(dev_priv,
> > > DC_STATE_EN_UPTO_DC6);
> > > else
> > > gen9_set_dc_state(dev_priv,
> > > DC_STATE_EN_UPTO_DC5);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
