Re: [PATCH v4] drm/i915/bxt: eDP Panel Power sequencing

["Kannan, Vandana" <vandana.kannan@intel.com>]

Please help review this patch.

- Vandana

On 5/13/2015 2:52 PM, Kannan, Vandana wrote:
>
>
> On 5/13/2015 3:13 PM, Vandana Kannan wrote:
>> Changes for BXT - added a IS_BROXTON check to use the macro related to PPS
>> registers for BXT.
>> BXT does not have PP_DIV register. Making changes to handle this.
>> Second set of PPS registers have been defined but will be used when VBT
>> provides a selection between the 2 sets of registers.
>>
>> v2:
>> [Jani] Added 2nd set of PPS registers and the macro
>> Jani's review comments
>> 	- remove reference in i915_suspend.c
>> 	- Use BXT PP macro
>> Squashing all PPS related patches into one.
>>
>> v3: Jani's review comments addressed
>> 	- Use pp_ctl instead of pp
>> 	- ironlake_get_pp_control() is not required for BXT
>> 	- correct the use of && in the print statement
>> 	- drop the shift in the print statement
>>
>> v4: Jani's comments
>> 	- modify ironlake_get_pp_control() - dont set unlock key for bxt
>>
>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
>> ---
>>    drivers/gpu/drm/i915/i915_reg.h | 13 +++++++
>>    drivers/gpu/drm/i915/intel_dp.c | 76 ++++++++++++++++++++++++++++++++---------
>>    2 files changed, 72 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 1b31238..777c51a 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6315,6 +6315,8 @@ enum skl_disp_power_wells {
>>    #define PCH_PP_CONTROL		0xc7204
>>    #define  PANEL_UNLOCK_REGS	(0xabcd << 16)
>>    #define  PANEL_UNLOCK_MASK	(0xffff << 16)
>> +#define  BXT_POWER_CYCLE_DELAY_MASK	(0x1f0)
>> +#define  BXT_POWER_CYCLE_DELAY_SHIFT	4
>>    #define  EDP_FORCE_VDD		(1 << 3)
>>    #define  EDP_BLC_ENABLE		(1 << 2)
>>    #define  PANEL_POWER_RESET	(1 << 1)
>> @@ -6343,6 +6345,17 @@ enum skl_disp_power_wells {
>>    #define  PANEL_POWER_CYCLE_DELAY_MASK	(0x1f)
>>    #define  PANEL_POWER_CYCLE_DELAY_SHIFT	0
>>
>> +/* BXT PPS changes - 2nd set of PPS registers */
>> +#define _BXT_PP_STATUS2		0xc7300
>> +#define _BXT_PP_CONTROL2 	0xc7304
>> +#define _BXT_PP_ON_DELAYS2	0xc7308
>> +#define _BXT_PP_OFF_DELAYS2	0xc730c
>> +
>> +#define BXT_PP_STATUS(n)	((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2)
>> +#define BXT_PP_CONTROL(n)	((!n) ? PCH_PP_CONTROL : _BXT_PP_CONTROL2)
>> +#define BXT_PP_ON_DELAYS(n)	((!n) ? PCH_PP_ON_DELAYS : _BXT_PP_ON_DELAYS2)
>> +#define BXT_PP_OFF_DELAYS(n)	((!n) ? PCH_PP_OFF_DELAYS : _BXT_PP_OFF_DELAYS2)
>> +
>>    #define PCH_DP_B		0xe4100
>>    #define PCH_DPB_AUX_CH_CTL	0xe4110
>>    #define PCH_DPB_AUX_CH_DATA1	0xe4114
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index bacdec5..c7432a2 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -559,7 +559,9 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
>>    {
>>    	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>
>> -	if (HAS_PCH_SPLIT(dev))
>> +	if (IS_BROXTON(dev))
>> +		return BXT_PP_CONTROL(0);
>> +	else if (HAS_PCH_SPLIT(dev))
>>    		return PCH_PP_CONTROL;
>>    	else
>>    		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
>> @@ -569,7 +571,9 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
>>    {
>>    	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>
>> -	if (HAS_PCH_SPLIT(dev))
>> +	if (IS_BROXTON(dev))
>> +		return BXT_PP_STATUS(0);
>> +	else if (HAS_PCH_SPLIT(dev))
>>    		return PCH_PP_STATUS;
>>    	else
>>    		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
>> @@ -1681,8 +1685,10 @@ static  u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
>>    	lockdep_assert_held(&dev_priv->pps_mutex);
>>
>>    	control = I915_READ(_pp_ctrl_reg(intel_dp));
>> -	control &= ~PANEL_UNLOCK_MASK;
>> -	control |= PANEL_UNLOCK_REGS;
>> +	if (!IS_BROXTON(dev)) {
>> +		control &= ~PANEL_UNLOCK_MASK;
>> +		control |= PANEL_UNLOCK_REGS;
>> +	}
>>    	return control;
>>    }
>>
>> @@ -4969,8 +4975,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>>    	struct drm_i915_private *dev_priv = dev->dev_private;
>>    	struct edp_power_seq cur, vbt, spec,
>>    		*final = &intel_dp->pps_delays;
>> -	u32 pp_on, pp_off, pp_div, pp;
>> -	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg;
>> +	u32 pp_on, pp_off, pp_div = 0, pp_ctl = 0;
>> +	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
>>
>>    	lockdep_assert_held(&dev_priv->pps_mutex);
>>
>> @@ -4978,7 +4984,16 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>>    	if (final->t11_t12 != 0)
>>    		return;
>>
>> -	if (HAS_PCH_SPLIT(dev)) {
>> +	if (IS_BROXTON(dev)) {
>> +		/*
>> +		 * TODO: BXT has 2 sets of PPS registers.
>> +		 * Correct Register for Broxton need to be identified
>> +		 * using VBT. hardcoding for now
>> +		 */
>> +		pp_ctrl_reg = BXT_PP_CONTROL(0);
>> +		pp_on_reg = BXT_PP_ON_DELAYS(0);
>> +		pp_off_reg = BXT_PP_OFF_DELAYS(0);
>> +	} else if (HAS_PCH_SPLIT(dev)) {
>>    		pp_ctrl_reg = PCH_PP_CONTROL;
>>    		pp_on_reg = PCH_PP_ON_DELAYS;
>>    		pp_off_reg = PCH_PP_OFF_DELAYS;
>> @@ -4994,12 +5009,13 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>>
>>    	/* Workaround: Need to write PP_CONTROL with the unlock key as
>>    	 * the very first thing. */
>> -	pp = ironlake_get_pp_control(intel_dp);
>> -	I915_WRITE(pp_ctrl_reg, pp);
>> +	pp_ctl = ironlake_get_pp_control(intel_dp);
>> +	I915_WRITE(pp_ctrl_reg, pp_ctl);
>>
> This write may not be required for BXT.
>
> - Vandana
>>    	pp_on = I915_READ(pp_on_reg);
>>    	pp_off = I915_READ(pp_off_reg);
>> -	pp_div = I915_READ(pp_div_reg);
>> +	if (!IS_BROXTON(dev))
>> +		pp_div = I915_READ(pp_div_reg);
>>
>>    	/* Pull timing values out of registers */
>>    	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
>> @@ -5014,7 +5030,11 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>>    	cur.t10 = (pp_off & PANEL_POWER_DOWN_DELAY_MASK) >>
>>    		PANEL_POWER_DOWN_DELAY_SHIFT;
>>
>> -	cur.t11_t12 = ((pp_div & PANEL_POWER_CYCLE_DELAY_MASK) >>
>> +	if (IS_BROXTON(dev))
>> +		cur.t11_t12 = (((pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
>> +			BXT_POWER_CYCLE_DELAY_SHIFT) - 1) * 1000;
>> +	else
>> +		cur.t11_t12 = ((pp_div & PANEL_POWER_CYCLE_DELAY_MASK) >>
>>    		       PANEL_POWER_CYCLE_DELAY_SHIFT) * 1000;
>>
>>    	DRM_DEBUG_KMS("cur t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
>> @@ -5072,13 +5092,23 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>>    	struct drm_i915_private *dev_priv = dev->dev_private;
>>    	u32 pp_on, pp_off, pp_div, port_sel = 0;
>>    	int div = HAS_PCH_SPLIT(dev) ? intel_pch_rawclk(dev) : intel_hrawclk(dev);
>> -	int pp_on_reg, pp_off_reg, pp_div_reg;
>> +	int pp_on_reg, pp_off_reg, pp_div_reg = 0, pp_ctrl_reg;
>>    	enum port port = dp_to_dig_port(intel_dp)->port;
>>    	const struct edp_power_seq *seq = &intel_dp->pps_delays;
>>
>>    	lockdep_assert_held(&dev_priv->pps_mutex);
>>
>> -	if (HAS_PCH_SPLIT(dev)) {
>> +	if (IS_BROXTON(dev)) {
>> +		/*
>> +		 * TODO: BXT has 2 sets of PPS registers.
>> +		 * Correct Register for Broxton need to be identified
>> +		 * using VBT. hardcoding for now
>> +		 */
>> +		pp_ctrl_reg = BXT_PP_CONTROL(0);
>> +		pp_on_reg = BXT_PP_ON_DELAYS(0);
>> +		pp_off_reg = BXT_PP_OFF_DELAYS(0);
>> +
>> +	} else if (HAS_PCH_SPLIT(dev)) {
>>    		pp_on_reg = PCH_PP_ON_DELAYS;
>>    		pp_off_reg = PCH_PP_OFF_DELAYS;
>>    		pp_div_reg = PCH_PP_DIVISOR;
>> @@ -5104,9 +5134,16 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>>    		 (seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT);
>>    	/* Compute the divisor for the pp clock, simply match the Bspec
>>    	 * formula. */
>> -	pp_div = ((100 * div)/2 - 1) << PP_REFERENCE_DIVIDER_SHIFT;
>> -	pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000)
>> -			<< PANEL_POWER_CYCLE_DELAY_SHIFT);
>> +	if (IS_BROXTON(dev)) {
>> +		pp_div = I915_READ(pp_ctrl_reg);
>> +		pp_div &= ~BXT_POWER_CYCLE_DELAY_MASK;
>> +		pp_div |= (DIV_ROUND_UP((seq->t11_t12 + 1), 1000)
>> +				<< BXT_POWER_CYCLE_DELAY_SHIFT);
>> +	} else {
>> +		pp_div = ((100 * div)/2 - 1) << PP_REFERENCE_DIVIDER_SHIFT;
>> +		pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000)
>> +				<< PANEL_POWER_CYCLE_DELAY_SHIFT);
>> +	}
>>
>>    	/* Haswell doesn't have any port selection bits for the panel
>>    	 * power sequencer any more. */
>> @@ -5123,11 +5160,16 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>>
>>    	I915_WRITE(pp_on_reg, pp_on);
>>    	I915_WRITE(pp_off_reg, pp_off);
>> -	I915_WRITE(pp_div_reg, pp_div);
>> +	if (IS_BROXTON(dev))
>> +		I915_WRITE(pp_ctrl_reg, pp_div);
>> +	else
>> +		I915_WRITE(pp_div_reg, pp_div);
>>
>>    	DRM_DEBUG_KMS("panel power sequencer register settings: PP_ON %#x, PP_OFF %#x, PP_DIV %#x\n",
>>    		      I915_READ(pp_on_reg),
>>    		      I915_READ(pp_off_reg),
>> +		      IS_BROXTON(dev) ?
>> +		      (I915_READ(pp_ctrl_reg) & BXT_POWER_CYCLE_DELAY_MASK) :
>>    		      I915_READ(pp_div_reg));
>>    }
>>
>>
> _______________________________________________
> 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