On Fri, Oct 16, 2015 at 03:22:45PM +0300, Imre Deak wrote: > On ti, 2015-09-29 at 11:01 +0530, Animesh Manna wrote: > > Mmio register access after dc6/dc5 entry is not allowed when > > DC6 power states are enabled according to bspec (bspec-id 0527), > > so enabling dc6 as the last call in suspend flow. > > > > v1: Initial version. > > > > v2: Based on review comment from Daniel, > > - created a seperate patch for csr uninitialization set call. > > > > v3: Rebased on top of latest code. > > > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx> > > Cc: Damien Lespiau <damien.lespiau@xxxxxxxxx> > > Cc: Imre Deak <imre.deak@xxxxxxxxx> > > Cc: Sunil Kamath <sunil.kamath@xxxxxxxxx> > > Signed-off-by: Animesh Manna <animesh.manna@xxxxxxxxx> > > Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@xxxxxxxxx> > > Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@xxxxxxxxx> > > Acked-by: Imre Deak <imre.deak@xxxxxxxxx> > > Suggestion for the commit message: > > Currently we keep DC6 enabled during modesets and DPAUX transfers, which > is not allowed according to the specification. This can lead at least to > PLL locking failures, DPAUX timeouts and prevent deeper package power > states (PC9/10). Fix this for now by enabling DC6 only when we know the > above events (modeset, DPAUX) can't happen. > > This a temporary solution as some issues are still unsolved as described > in [1] and [2], we'll address those as a follow-up. > > [1] > http://lists.freedesktop.org/archives/intel-gfx/2015-October/077669.html > [2] > http://lists.freedesktop.org/archives/intel-gfx/2015-October/077787.html Queued for -next, thanks for the patch. -Daniel > > > > --- > > drivers/gpu/drm/i915/i915_drv.c | 13 +++++++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > drivers/gpu/drm/i915/intel_runtime_pm.c | 19 +++++++------------ > > 3 files changed, 22 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index 1cb6b82..51075d5 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -1049,10 +1049,20 @@ static int i915_pm_resume(struct device *dev) > > > > static int skl_suspend_complete(struct drm_i915_private *dev_priv) > > { > > + enum csr_state state; > > /* Enabling DC6 is not a hard requirement to enter runtime D3 */ > > > > skl_uninit_cdclk(dev_priv); > > > > + /* TODO: wait for a completion event or > > + * similar here instead of busy > > + * waiting using wait_for function. > > + */ > > + wait_for((state = intel_csr_load_status_get(dev_priv)) != > > + FW_UNINITIALIZED, 1000); > > + if (state == FW_LOADED) > > + skl_enable_dc6(dev_priv); > > + > > return 0; > > } > > > > @@ -1099,6 +1109,9 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv) > > { > > struct drm_device *dev = dev_priv->dev; > > > > + if (intel_csr_load_status_get(dev_priv) == FW_LOADED) > > + skl_disable_dc6(dev_priv); > > + > > skl_init_cdclk(dev_priv); > > intel_csr_load_program(dev); > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index c96289d..990161d 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1143,6 +1143,8 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv); > > void bxt_disable_dc9(struct drm_i915_private *dev_priv); > > void skl_init_cdclk(struct drm_i915_private *dev_priv); > > void skl_uninit_cdclk(struct drm_i915_private *dev_priv); > > +void skl_enable_dc6(struct drm_i915_private *dev_priv); > > +void skl_disable_dc6(struct drm_i915_private *dev_priv); > > void intel_dp_get_m_n(struct intel_crtc *crtc, > > struct intel_crtc_state *pipe_config); > > void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n); > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index d8e9416..d6b4f61 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -551,7 +551,7 @@ static void assert_can_disable_dc6(struct drm_i915_private *dev_priv) > > "DC6 already programmed to be disabled.\n"); > > } > > > > -static void skl_enable_dc6(struct drm_i915_private *dev_priv) > > +void skl_enable_dc6(struct drm_i915_private *dev_priv) > > { > > uint32_t val; > > > > @@ -568,7 +568,7 @@ static void skl_enable_dc6(struct drm_i915_private *dev_priv) > > POSTING_READ(DC_STATE_EN); > > } > > > > -static void skl_disable_dc6(struct drm_i915_private *dev_priv) > > +void skl_disable_dc6(struct drm_i915_private *dev_priv) > > { > > uint32_t val; > > > > @@ -629,10 +629,10 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv, > > !I915_READ(HSW_PWR_WELL_BIOS), > > "Invalid for power well status to be enabled, unless done by the BIOS, \ > > when request is to disable!\n"); > > - if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) && > > - power_well->data == SKL_DISP_PW_2) { > > + if (power_well->data == SKL_DISP_PW_2) { > > + if (GEN9_ENABLE_DC5(dev)) > > + gen9_disable_dc5(dev_priv); > > if (SKL_ENABLE_DC6(dev)) { > > - skl_disable_dc6(dev_priv); > > /* > > * DDI buffer programming unnecessary during driver-load/resume > > * as it's already done during modeset initialization then. > > @@ -640,8 +640,6 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv, > > */ > > if (!dev_priv->power_domains.initializing) > > intel_prepare_ddi(dev); > > - } else { > > - gen9_disable_dc5(dev_priv); > > } > > } > > I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask); > > @@ -667,7 +665,7 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv, > > DRM_DEBUG_KMS("Disabling %s\n", power_well->name); > > } > > > > - if ((GEN9_ENABLE_DC5(dev) || SKL_ENABLE_DC6(dev)) && > > + if (GEN9_ENABLE_DC5(dev) && > > power_well->data == SKL_DISP_PW_2) { > > enum csr_state state; > > /* TODO: wait for a completion event or > > @@ -680,10 +678,7 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv, > > DRM_DEBUG("CSR firmware not ready (%d)\n", > > state); > > else > > - if (SKL_ENABLE_DC6(dev)) > > - skl_enable_dc6(dev_priv); > > - else > > - gen9_enable_dc5(dev_priv); > > + gen9_enable_dc5(dev_priv); > > } > > } > > } > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
