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

[Patrik Jakobsson]

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 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.

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

> > >  }
> > 
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx