Re: [PATCH 2/3] drm/i915: Initialize CHV digital lock detect threshold

From: "Purushothaman, Vijay A" <vijay.a.purushothaman@linux.intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Intel Graphics <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/3] drm/i915: Initialize CHV digital lock detect threshold
Date: Thu, 05 Mar 2015 19:20:06 +0530	[thread overview]
Message-ID: <54F85F0E.8010303@linux.intel.com> (raw)
In-Reply-To: <20150303153829.GF11371@intel.com>

On 3/3/2015 9:08 PM, Ville Syrjälä wrote:
> On Tue, Mar 03, 2015 at 08:43:12PM +0530, Vijay Purushothaman wrote:
>> Initialize lock detect threshold and select coarse threshold for the
>> case where M2 fraction division is disabled.
>>
>> v2: Split the changes into multiple smaller patches based on review by
>> Ville
>>
>> v3: Addressed rest of the review comments. Clear out the old bits before
>> we modify those bits as part of RMW
>>
>> Signed-off-by: Vijay Purushothaman <vijay.a.purushothaman@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h      |    1 +
>>   drivers/gpu/drm/i915/intel_display.c |   13 +++++++++++++
>>   2 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 8200e98..1a0f94e 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -1046,6 +1046,7 @@ enum skl_disp_power_wells {
>>   #define _CHV_PLL_DW9_CH0		0x8024
>>   #define _CHV_PLL_DW9_CH1		0x81A4
>>   #define  DPIO_CHV_INT_LOCK_THRESHOLD_SHIFT		1 /* 3 bits */
>> +#define  DPIO_CHV_INT_LOCK_THRESHOLD_MASK		(7 << 1)
>>   #define  DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE	1 /* 1: coarse & 0 : fine  */
>>   #define CHV_PLL_DW9(ch) _PIPE(ch, _CHV_PLL_DW9_CH0, _CHV_PLL_DW9_CH1)
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 15904a8..a6b5786 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -6176,11 +6176,24 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>>   		dpio_val |= (2 << DPIO_CHV_FEEDFWD_GAIN_SHIFT);
>>   		vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW3(port), dpio_val);
>>   
>> +		/* Program digital lock detect threshold */
>> +		dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW9(port));
>> +		dpio_val &= ~DPIO_CHV_INT_LOCK_THRESHOLD_MASK;
>> +		dpio_val |= (0x5 << DPIO_CHV_INT_LOCK_THRESHOLD_SHIFT);
> Missing
>   dpio_val &= ~DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE;
> here.
>
> So I think to avoid such mistakes it's best to rewrite this too as:
>
> val = read(PLL_DW9)
> val &= ~(DPIO_CHV_INT_LOCK_THRESHOLD_MASK | DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE);
> val |= 0x5 << DPIO_CHV_INT_LOCK_THRESHOLD_SHIFT)
> if (!bestm2_frac)
> 	val |= DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE;
> write(PLL_DW9, val);

I will send v4 with this change and add your signed-off-by tag.

Thanks,
Vijay
>
>> +		vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW9(port), dpio_val);
>> +
>>   	} else {
>>   		/* M2 fraction division disable */
>>   		dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW3(port));
>>   		dpio_val &= ~DPIO_CHV_FRAC_DIV_EN;
>>   		vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW3(port), dpio_val);
>> +
>> +		/* Program digital lock detect threshold */
>> +		dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW9(port));
>> +		dpio_val &= ~DPIO_CHV_INT_LOCK_THRESHOLD_MASK;
>> +		dpio_val |= (0x5 << DPIO_CHV_INT_LOCK_THRESHOLD_SHIFT);
>> +		dpio_val |= DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE;
>> +		vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW9(port), dpio_val);
>>   	}
>>   
>>   	/* Loop filter */
>> -- 
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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