[Intel-gfx] [DMC_BUGFIX_V3] drm/i915/skl: Making DC6 entry is the last call in suspend flow.
Daniel Vetter
daniel at ffwll.ch
Mon Oct 19 02:26:19 PDT 2015
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 at intel.com>
> > Cc: Damien Lespiau <damien.lespiau at intel.com>
> > Cc: Imre Deak <imre.deak at intel.com>
> > Cc: Sunil Kamath <sunil.kamath at intel.com>
> > Signed-off-by: Animesh Manna <animesh.manna at intel.com>
> > Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju at intel.com>
> > Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj at intel.com>
>
> Acked-by: Imre Deak <imre.deak at intel.com>
>
> 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list