[Intel-gfx] [DMC_BUGFIX_SKL_V2 4/5] drm/i915/skl: Do not disable cdclk PLL if csr firmware is present

[Hindman, Gavin]

Unless I'm misreading that would imply that we are moving away from our previous position that DMC FW is optional, correct?    Would this not render power-sequencing broken if a distro chose not to include DMC FW?

Gavin Hindman
Senior Program Manager
SSG/OTC – Open Source Technology Center


-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf Of Daniel Vetter
Sent: Wednesday, August 26, 2015 6:12 AM
To: Manna, Animesh
Cc: Bhardwaj, Rajneesh; intel-gfx at lists.freedesktop.org; Vetter, Daniel
Subject: Re: [Intel-gfx] [DMC_BUGFIX_SKL_V2 4/5] drm/i915/skl: Do not disable cdclk PLL if csr firmware is present

On Wed, Aug 26, 2015 at 01:36:08AM +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.
> 
> v1: Initial version.
> 
> v2: Based on review comment from Daniel added code commnent.
> 
> 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 | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index f604ce1..b6bef20 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5687,10 +5687,16 @@ 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");
>  
> -	/* 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");
> +	/*
> +	 * DMC assumes ownership of LCPLL and will get confused if we touch it.

This should get a FIXME - once we have dmc loading fixed up we require the firmware and there's no point in this check any more. Flexibilty just because is something we simply don't have the developer and validation resources for.
-Daniel

> +	 */
> +	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);  }
> --
> 2.0.2
> 
> _______________________________________________
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx