[Intel-gfx] [SKL-DMC-BUGFIX 3/5] drm/i915/skl: Do not disable cdclk PLL if csr firmware is present.

[Imre Deak]

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 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-bt: Vathsala Nagaraju <vathsala.nagaraju at intel.com>
> > Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj at intel.com>
> > ---
> >  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
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx