Re: [PATCH 1/2] drm/i915: reuse WRPLL when possible

[Damien Lespiau <damien.lespiau@intel.com>]

On Wed, Oct 30, 2013 at 06:27:43PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> It seems we do have machines with 3 HDMI/DVI outputs, so sharing
> WRPLLs is the only way to get 3 pipes working.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=68485
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 37 ++++++++++++++++++++++---------------
>  1 file changed, 22 insertions(+), 15 deletions(-)
> 
> It is that easy because I already planned to enable PLL sharing when I wrote the
> original code :)
> 
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 31f4fe2..f2144b2 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -636,8 +636,6 @@ bool intel_ddi_pll_mode_set(struct drm_crtc *crtc)
>  	uint32_t reg, val;
>  	int clock = intel_crtc->config.port_clock;
>  
> -	/* TODO: reuse PLLs when possible (compare values) */
> -
>  	intel_ddi_put_crtc_pll(crtc);
>  
>  	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
> @@ -665,31 +663,40 @@ bool intel_ddi_pll_mode_set(struct drm_crtc *crtc)
>  	} else if (type == INTEL_OUTPUT_HDMI) {
>  		unsigned p, n2, r2;
>  
> -		if (plls->wrpll1_refcount == 0) {
> +		intel_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
> +
> +		val = WRPLL_PLL_ENABLE | WRPLL_PLL_SELECT_LCPLL_2700 |
> +		      WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
> +		      WRPLL_DIVIDER_POST(p);
> +
> +		if (val == I915_READ(WRPLL_CTL1)) {
> +			DRM_DEBUG_KMS("Reusing WRPLL 1 on pipe %c\n",
> +				      pipe_name(pipe));
> +			reg = WRPLL_CTL1;
> +		} else if (val == I915_READ(WRPLL_CTL2)) {
> +			DRM_DEBUG_KMS("Reusing WRPLL 2 on pipe %c\n",
> +				      pipe_name(pipe));
> +			reg = WRPLL_CTL2;
> +		} else if (plls->wrpll1_refcount == 0) {
>  			DRM_DEBUG_KMS("Using WRPLL 1 on pipe %c\n",
>  				      pipe_name(pipe));
> -			plls->wrpll1_refcount++;
>  			reg = WRPLL_CTL1;
> -			intel_crtc->ddi_pll_sel = PORT_CLK_SEL_WRPLL1;
>  		} else if (plls->wrpll2_refcount == 0) {
>  			DRM_DEBUG_KMS("Using WRPLL 2 on pipe %c\n",
>  				      pipe_name(pipe));
> -			plls->wrpll2_refcount++;
>  			reg = WRPLL_CTL2;
> -			intel_crtc->ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
>  		} else {
>  			DRM_ERROR("No WRPLLs available!\n");
>  			return false;
>  		}
>  
> -		WARN(I915_READ(reg) & WRPLL_PLL_ENABLE,
> -		     "WRPLL already enabled\n");
> -
> -		intel_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
> -
> -		val = WRPLL_PLL_ENABLE | WRPLL_PLL_SELECT_LCPLL_2700 |
> -		      WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
> -		      WRPLL_DIVIDER_POST(p);
> +		if (reg == WRPLL_CTL1) {
> +			plls->wrpll1_refcount++;
> +			intel_crtc->ddi_pll_sel = PORT_CLK_SEL_WRPLL1;
> +		} else {
> +			plls->wrpll2_refcount++;
> +			intel_crtc->ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
> +		}
>  
>  	} else if (type == INTEL_OUTPUT_ANALOG) {
>  		if (plls->spll_refcount == 0) {
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx