All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Vandana Kannan <vandana.kannan@intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/3] drm/i915: eDP Panel Power sequencing PP_DIV	register changes
Date: Thu, 30 Apr 2015 11:57:09 +0300	[thread overview]
Message-ID: <87oam69eoq.fsf@intel.com> (raw)
In-Reply-To: <1430379455-21244-2-git-send-email-vandana.kannan@intel.com>

On Thu, 30 Apr 2015, Vandana Kannan <vandana.kannan@intel.com> wrote:
> BXT does not have PP_DIV register. Making changes to handle this.
>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h     |  2 ++
>  drivers/gpu/drm/i915/i915_suspend.c |  8 ++++--
>  drivers/gpu/drm/i915/intel_dp.c     | 56 +++++++++++++++++++++++++++++--------
>  3 files changed, 52 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 36805b6..580f5cb 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)
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index e91d637..24e0b06 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -48,7 +48,9 @@ static void i915_save_display(struct drm_device *dev)
>  		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
>  		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
>  		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
> -		dev_priv->regfile.savePP_DIVISOR = I915_READ(PCH_PP_DIVISOR);
> +		if (!IS_BROXTON(dev))
> +			dev_priv->regfile.savePP_DIVISOR =
> +				I915_READ(PCH_PP_DIVISOR);
>  	} else if (!IS_VALLEYVIEW(dev)) {
>  		dev_priv->regfile.savePP_CONTROL = I915_READ(PP_CONTROL);
>  		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PP_ON_DELAYS);
> @@ -82,7 +84,9 @@ static void i915_restore_display(struct drm_device *dev)
>  	if (!HAS_GMCH_DISPLAY(dev)) {
>  		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
>  		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
> -		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
> +		if (!IS_BROXTON(dev))
> +			I915_WRITE(PCH_PP_DIVISOR,
> +					dev_priv->regfile.savePP_DIVISOR);

Again, no need to do this in i915_suspend.c.

>  		I915_WRITE(PCH_PP_CONTROL, dev_priv->regfile.savePP_CONTROL);
>  	} else if (!IS_VALLEYVIEW(dev)) {
>  		I915_WRITE(PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 68e10c1..37514d3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4954,8 +4954,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, pp;
> +	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
> @@ -4964,10 +4964,16 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>  		return;
>  
>  	if (!HAS_GMCH_DISPLAY(dev)) {
> +		/*
> +		 * TODO: BXT has 2 sets of PPS registers.
> +		 * Correct Register for Broxton need to be identified
> +		 * using VBT. hardcoding for now
> +		 */

This comment really belongs in the previous patch, which would add the
if (IS_BROXTON(dev)) branch as I suggested.

>  		pp_ctrl_reg = PCH_PP_CONTROL;
>  		pp_on_reg = PCH_PP_ON_DELAYS;
>  		pp_off_reg = PCH_PP_OFF_DELAYS;
> -		pp_div_reg = PCH_PP_DIVISOR;
> +		if (!IS_BROXTON(dev))
> +			pp_div_reg = PCH_PP_DIVISOR;

And thus this is naturally included in the previous patch as well.

The rest will probably need some updates - maybe it needs to be fully
incorporated into the previous patch.

>  	} else {
>  		enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
>  
> @@ -4984,7 +4990,10 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>  
>  	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);
> +	else
> +		pp_ctl = I915_READ(pp_ctrl_reg);
>  
>  	/* Pull timing values out of registers */
>  	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
> @@ -4999,8 +5008,12 @@ 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) >>
> -		       PANEL_POWER_CYCLE_DELAY_SHIFT) * 1000;
> +	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",
>  		      cur.t1_t3, cur.t8, cur.t9, cur.t10, cur.t11_t12);
> @@ -5057,16 +5070,22 @@ 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_GMCH_DISPLAY(dev)) {
> +		/*
> +		 * TODO: Correct Register for Broxton need to be identified
> +		 * using VBT. hardcoding for now
> +		 */
>  		pp_on_reg = PCH_PP_ON_DELAYS;
>  		pp_off_reg = PCH_PP_OFF_DELAYS;
> -		pp_div_reg = PCH_PP_DIVISOR;
> +		pp_ctrl_reg = PCH_PP_CONTROL;
> +		if (!IS_BROXTON(dev))
> +			pp_div_reg = PCH_PP_DIVISOR;
>  	} else {
>  		enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
>  
> @@ -5089,9 +5108,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. */
> @@ -5108,11 +5134,17 @@ 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)
> +		       >> BXT_POWER_CYCLE_DELAY_SHIFT) :
>  		      I915_READ(pp_div_reg));
>  }
>  
> -- 
> 2.0.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2015-04-30  8:55 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-30  7:37 [PATCH 1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT Vandana Kannan
2015-04-30  7:37 ` [PATCH 2/3] drm/i915: eDP Panel Power sequencing PP_DIV register changes Vandana Kannan
2015-04-30  8:57   ` Jani Nikula [this message]
2015-04-30  9:27   ` David Weinehall
2015-04-30  7:37 ` [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set Vandana Kannan
2015-04-30  8:43   ` Jani Nikula
2015-04-30 11:15   ` Imre Deak
2015-04-30 11:23     ` Jani Nikula
2015-05-04  6:24       ` Kannan, Vandana
2015-05-04  7:06         ` [PATCH] drm/i915: eDP Panel Power sequencing Vandana Kannan
2015-05-04 11:12           ` shuang.he
2015-05-06 15:05           ` Jani Nikula
2015-05-07  4:13             ` Kannan, Vandana
2015-05-07  7:31             ` [PATCH v3] drm/i915/bxt: " Vandana Kannan
2015-05-07  7:37               ` Jani Nikula
2015-05-11 14:34                 ` Kannan, Vandana
2015-05-13  9:43                   ` [PATCH v4] " Vandana Kannan
2015-05-13  9:22                     ` Kannan, Vandana
2015-06-03 12:01                       ` Kannan, Vandana
2015-05-14  9:13                     ` shuang.he
2015-05-08  5:47               ` [PATCH v3] " shuang.he
2015-05-01 13:30   ` [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set shuang.he
2015-04-30  8:50 ` [PATCH 1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT Jani Nikula
2015-05-06 10:08 ` Daniel Vetter
2015-05-06 10:12   ` Jani Nikula

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87oam69eoq.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=vandana.kannan@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.