[Intel-gfx] [PATCH v4] drm/i915/bxt: eDP Panel Power sequencing
Kannan, Vandana
vandana.kannan at intel.com
Wed Jun 17 02:22:00 PDT 2015
On 6/17/2015 2:45 PM, Jindal, Sonika wrote:
>
>
> On 6/12/2015 3:57 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 at intel.com>
>> Signed-off-by: A.Sunil Kamath <sunil.kamath at intel.com>
>> Cc: Jani Nikula <jani.nikula at linux.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 7213224..cc03b5b 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6377,6 +6377,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)
>> @@ -6405,6 +6407,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
> Not aligned?
>
>> +#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 15e8892..ad9cb0e9 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -567,7 +567,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));
>> @@ -577,7 +579,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));
>> @@ -1701,8 +1705,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;
>> }
>>
>> @@ -5091,8 +5097,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);
>>
>> @@ -5100,7 +5106,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;
>> @@ -5116,12 +5131,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);
> Since you are not doing anything in ironlake_get_pp_control, you can
> move this write to !IS_BROXTON below
>
Agreed.
>>
>> 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) >>
>> @@ -5136,7 +5152,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;
> Why a -1 here for broxton and not for other platforms?
According to the BSpec, "The value should be programmed to (desired
delay / 100 milliseconds) + 1". This -1 is to make up for that. This is
mentioned for other platforms as well. So maybe we should make the
change there as well ??
> Also, since 0 is also a valid value and cur.t11_t12 is u16, this -1 will
> result in undefined value.
Agreed. Making the change.
- Vandana
>
> Regards,
> Sonika
>> + 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",
>> @@ -5194,13 +5214,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;
>> @@ -5226,9 +5256,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. */
>> @@ -5245,11 +5282,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));
>> }
>>
>>
More information about the Intel-gfx
mailing list