On ma, 2015-10-12 at 16:46 +0200, Patrik Jakobsson wrote:
> On Mon, Oct 12, 2015 at 05:07:13PM +0300, Imre Deak wrote:
> > On ma, 2015-10-12 at 16:37 +0300, Imre Deak wrote:
> > > On ma, 2015-08-03 at 21:55 +0530, Animesh Manna wrote:
> > > > While display engine entering into low power state no need to disable
> > > > cdclk pll as CSR firmware of dmc will take care. If pll is already
> > > > enabled firmware execution sequence will be blocked. This is one
> > > > of the criteria for dmc to work properly.
> > > >
> > > > 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-bt: Vathsala Nagaraju <vathsala.nagaraju@xxxxxxxxx>
> > > > Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@xxxxxxxxx>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_display.c | 11 +++++++----
> > > > 1 file changed, 7 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index af0bcfe..ef2ef4d 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -5675,10 +5675,13 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
> > > > if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE)
> > > > DRM_ERROR("DBuf power disable timeout\n");
> > >
> > > My understanding is that DBUF_CTL is also handled by the firmware and so
> > > we shouldn't need to disable it either manually. I guess that could be
> > > addressed as a follow-up.
> > >
> > > >
> > > > - /* disable DPLL0 */
> > > > - I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) & ~LCPLL_PLL_ENABLE);
> > > > - if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
> > > > - DRM_ERROR("Couldn't disable DPLL0\n");
> > > > + if (dev_priv->csr.dmc_payload) {
> > > > + /* disable DPLL0 */
> > > > + I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) &
> > > > + ~LCPLL_PLL_ENABLE);
> > > > + if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1))
> > > > + DRM_ERROR("Couldn't disable DPLL0\n");
> > > > + }
> > > >
> > > > intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
> >
> > Not introduced in this patch, but the above put looks incorrect. We get
> > here on the runtime suspend path, where all RPM and hence display power
> > domain references should be dropped already. So not sure how this can
> > even work atm. This is for someone to look into as a follow-up.
>
> Hmm, I thought that was fixed already. This seems to be the last comment on
> Paulos attempt at fixing it:
>
> http://lists.freedesktop.org/archives/intel-gfx/2015-August/073122.html
Yes, that would solve this issue. One note about it is that we only want
to manually toggle PW1 and Misc IO if DC6 is disabled (via a module
option for example). And if so, toggling of PW1 and Misc IO would be
part of the bigger
"Sequence for Software to Allow/Disallow Package C9-C10".
> > > > }
> > >
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
