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 <[email protected]>
> > Cc: Damien Lespiau <[email protected]>
> > Cc: Imre Deak <[email protected]>
> > Cc: Sunil Kamath <[email protected]>
> > Signed-off-by: Animesh Manna <[email protected]>
> > Signed-off-bt: Vathsala Nagaraju <[email protected]>
> > Signed-off-by: Rajneesh Bhardwaj <[email protected]>
> > ---
> >  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.

> >  }
> 
> 
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to