Re: [PATCH] drm/i915/bxt: PORT_PLL_REF_SEL bit should be set for all BXT variations.

[Imre Deak <imre.deak@intel.com>]

On to, 2016-04-14 at 15:37 -0700, Dongwon Kim wrote:
> This patch is to correct one thing in this commit:
> 
> commit 25a56705332add0363e47b3a0eca001d6fbd5bec
> Author: Dongwon Kim <dongwon.kim@intel.com>
> Date:   Wed Mar 16 18:06:13 2016 -0700
> 
>     drm/i915/bxt: Reversed polarity of PORT_PLL_REF_SEL bit
> 
>     For BXT, description of polarities of PORT_PLL_REF_SEL
>     has been reversed for newer Gen9LP steppings according to the
>     recent update in Bspec. This bit now should be set for
>     "Non-SSC" mode for all Gen9LP starting from B0 stepping.
> 
>     v2: Only B0 and newer stepping should be affected by this
>     change.
> 
> This reversed bit polarity is actually common
> for all BXT and APL SoCs. Therefore, revision checking
> in the original commit should be removed to make
> the bit set regardless of revision ID of GFX block.
> 
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

Thanks for the patch I pushed it to -dinq.

Nitpick: no need to quote whole commit messages, it's enough to include
 the part up to the subject line. I fixed this up now while merging.

Also would've been nice to get this right in the first version by
waiting for the clarification about the stepping detail. I applied that
early since it fixed problems for people with B stepping. I tried the
patch now on A stepping too and it seems to be the right thing to do
there too: the HDMI output blanks out if we clear the select bit and
recovers by setting it again. I should've done this earlier.

--Imre

> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 0bde6a4..7fc39ab 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -1295,17 +1295,9 @@ static void bxt_ddi_pll_enable(struct
> drm_i915_private *dev_priv,
>  	uint32_t temp;
>  	enum port port = (enum port)pll->id;	/* 1:1 port->PLL 
> mapping */
>  
> -	temp = I915_READ(BXT_PORT_PLL_ENABLE(port));
> -	/*
> -	 * Definition of each bit polarity has been changed
> -	 * after A1 stepping
> -	 */
> -	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1))
> -		temp &= ~PORT_PLL_REF_SEL;
> -	else
> -		temp |= PORT_PLL_REF_SEL;
> -
>  	/* Non-SSC reference */
> +	temp = I915_READ(BXT_PORT_PLL_ENABLE(port));
> +	temp |= PORT_PLL_REF_SEL;
>  	I915_WRITE(BXT_PORT_PLL_ENABLE(port), temp);
>  
>  	/* Disable 10 bit clock */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx