All of lore.kernel.org
 help / color / mirror / Atom feed
From: Deepak <deepak.s@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 13/15] drm/i915: Clean up CHV lane soft reset programming
Date: Thu, 27 Aug 2015 09:55:29 +0530	[thread overview]
Message-ID: <55DE9139.2020008@linux.intel.com> (raw)
In-Reply-To: <1436388361-11130-14-git-send-email-ville.syrjala@linux.intel.com>



On 07/09/2015 02:15 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently we relesar the lane soft reset before lane stagger settings
> have been programmed. I believe that means we don't actually do lane
> staggering. So move the soft reset deassert to happen after lane
> staggering has been programmed.
>
> The one confusing thing in this is that when we remove the power down
> override from the lanes, they power up with defaul register values,
> which do not have the soft reset overrides enabled. And according to
> some docs by default the data lane resets are tied to cmnreset. So that
> would mean that lanes would come out of reset without staggering as
> soon as the power down overrides are removed. But sine we can't access
> either the lane stagger register nor the soft reset override registers
> until the lanes are powered on, we can really do anything about it.
> So let's just set the soft reset overrides as soon as the lane is
> powered on and hope for the best.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c   | 91 +++++++++++++++++++++------------------
>   drivers/gpu/drm/i915/intel_hdmi.c | 91 ++++++++++++++++++++++-----------------
>   2 files changed, 100 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 817df87..297bd14 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2389,42 +2389,62 @@ static void vlv_post_disable_dp(struct intel_encoder *encoder)
>   	intel_dp_link_down(intel_dp);
>   }
>   
> -static void chv_post_disable_dp(struct intel_encoder *encoder)
> +static void chv_data_lane_soft_reset(struct intel_encoder *encoder,
> +				     bool reset)
>   {
> -	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> -	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
> -	struct drm_device *dev = encoder->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc =
> -		to_intel_crtc(encoder->base.crtc);
> -	enum dpio_channel ch = vlv_dport_to_channel(dport);
> -	enum pipe pipe = intel_crtc->pipe;
> -	u32 val;
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	enum dpio_channel ch = vlv_dport_to_channel(enc_to_dig_port(&encoder->base));
> +	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> +	enum pipe pipe = crtc->pipe;
> +	uint32_t val;
>   
> -	intel_dp_link_down(intel_dp);
> +	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW0(ch));
> +	if (reset)
> +		val &= ~(DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> +	else
> +		val |= DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET;
> +	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW0(ch), val);
>   
> -	mutex_lock(&dev_priv->sb_lock);
> +	if (crtc->config->lane_count > 2) {
> +		val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW0(ch));
> +		if (reset)
> +			val &= ~(DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> +		else
> +			val |= DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET;
> +		vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW0(ch), val);
> +	}
>   
> -	/* Propagate soft reset to data lane reset */
>   	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW1(ch));
>   	val |= CHV_PCS_REQ_SOFTRESET_EN;
> +	if (reset)
> +		val &= ~DPIO_PCS_CLK_SOFT_RESET;
> +	else
> +		val |= DPIO_PCS_CLK_SOFT_RESET;
>   	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW1(ch), val);
>   
> -	if (intel_crtc->config->lane_count > 2) {
> +	if (crtc->config->lane_count > 2) {
>   		val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW1(ch));
>   		val |= CHV_PCS_REQ_SOFTRESET_EN;
> +		if (reset)
> +			val &= ~DPIO_PCS_CLK_SOFT_RESET;
> +		else
> +			val |= DPIO_PCS_CLK_SOFT_RESET;
>   		vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW1(ch), val);
>   	}
> +}
>   
> -	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW0(ch));
> -	val &= ~(DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> -	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW0(ch), val);
> +static void chv_post_disable_dp(struct intel_encoder *encoder)
> +{
> +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +	struct drm_device *dev = encoder->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>   
> -	if (intel_crtc->config->lane_count > 2) {
> -		val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW0(ch));
> -		val &= ~(DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> -		vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW0(ch), val);
> -	}
> +	intel_dp_link_down(intel_dp);
> +
> +	mutex_lock(&dev_priv->sb_lock);
> +
> +	/* Assert data lane reset */
> +	chv_data_lane_soft_reset(encoder, true);
>   
>   	mutex_unlock(&dev_priv->sb_lock);
>   }
> @@ -2802,27 +2822,6 @@ static void chv_pre_enable_dp(struct intel_encoder *encoder)
>   		vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW11(ch), val);
>   	}
>   
> -	/* Deassert soft data lane reset*/
> -	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW1(ch));
> -	val |= CHV_PCS_REQ_SOFTRESET_EN;
> -	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW1(ch), val);
> -
> -	if (intel_crtc->config->lane_count > 2) {
> -		val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW1(ch));
> -		val |= CHV_PCS_REQ_SOFTRESET_EN;
> -		vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW1(ch), val);
> -	}
> -
> -	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW0(ch));
> -	val |= (DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> -	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW0(ch), val);
> -
> -	if (intel_crtc->config->lane_count > 2) {
> -		val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW0(ch));
> -		val |= (DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> -		vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW0(ch), val);
> -	}
> -
>   	/* Program Tx lane latency optimal setting*/
>   	for (i = 0; i < intel_crtc->config->lane_count; i++) {
>   		/* Set the upar bit */
> @@ -2872,6 +2871,9 @@ static void chv_pre_enable_dp(struct intel_encoder *encoder)
>   			       DPIO_TX2_STAGGER_MULT(5));
>   	}
>   
> +	/* Deassert data lane reset */
> +	chv_data_lane_soft_reset(encoder, false);
> +
>   	mutex_unlock(&dev_priv->sb_lock);
>   
>   	intel_enable_dp(encoder);
> @@ -2910,6 +2912,9 @@ static void chv_dp_pre_pll_enable(struct intel_encoder *encoder)
>   
>   	mutex_lock(&dev_priv->sb_lock);
>   
> +	/* Assert data lane reset */
> +	chv_data_lane_soft_reset(encoder, true);
> +
>   	/* program left/right clock distribution */
>   	if (pipe != PIPE_B) {
>   		val = vlv_dpio_read(dev_priv, pipe, _CHV_CMN_DW5_CH0);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 4b604ee..176d681 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1612,6 +1612,50 @@ static void vlv_hdmi_pre_pll_enable(struct intel_encoder *encoder)
>   	mutex_unlock(&dev_priv->sb_lock);
>   }
>   
> +static void chv_data_lane_soft_reset(struct intel_encoder *encoder,
> +				     bool reset)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	enum dpio_channel ch = vlv_dport_to_channel(enc_to_dig_port(&encoder->base));
> +	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> +	enum pipe pipe = crtc->pipe;
> +	uint32_t val;
> +
> +	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW0(ch));
> +	if (reset)
> +		val &= ~(DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> +	else
> +		val |= DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET;
> +	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW0(ch), val);
> +
> +	if (crtc->config->lane_count > 2) {
> +		val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW0(ch));
> +		if (reset)
> +			val &= ~(DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> +		else
> +			val |= DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET;
> +		vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW0(ch), val);
> +	}
> +
> +	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW1(ch));
> +	val |= CHV_PCS_REQ_SOFTRESET_EN;
> +	if (reset)
> +		val &= ~DPIO_PCS_CLK_SOFT_RESET;
> +	else
> +		val |= DPIO_PCS_CLK_SOFT_RESET;
> +	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW1(ch), val);
> +
> +	if (crtc->config->lane_count > 2) {
> +		val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW1(ch));
> +		val |= CHV_PCS_REQ_SOFTRESET_EN;
> +		if (reset)
> +			val &= ~DPIO_PCS_CLK_SOFT_RESET;
> +		else
> +			val |= DPIO_PCS_CLK_SOFT_RESET;
> +		vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW1(ch), val);
> +	}
> +}
> +
>   static void chv_hdmi_pre_pll_enable(struct intel_encoder *encoder)
>   {
>   	struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
> @@ -1637,6 +1681,9 @@ static void chv_hdmi_pre_pll_enable(struct intel_encoder *encoder)
>   
>   	mutex_lock(&dev_priv->sb_lock);
>   
> +	/* Assert data lane reset */
> +	chv_data_lane_soft_reset(encoder, true);
> +
>   	/* program left/right clock distribution */
>   	if (pipe != PIPE_B) {
>   		val = vlv_dpio_read(dev_priv, pipe, _CHV_CMN_DW5_CH0);
> @@ -1739,33 +1786,13 @@ static void vlv_hdmi_post_disable(struct intel_encoder *encoder)
>   
>   static void chv_hdmi_post_disable(struct intel_encoder *encoder)
>   {
> -	struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
>   	struct drm_device *dev = encoder->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc =
> -		to_intel_crtc(encoder->base.crtc);
> -	enum dpio_channel ch = vlv_dport_to_channel(dport);
> -	enum pipe pipe = intel_crtc->pipe;
> -	u32 val;
>   
>   	mutex_lock(&dev_priv->sb_lock);
>   
> -	/* Propagate soft reset to data lane reset */
> -	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW1(ch));
> -	val |= CHV_PCS_REQ_SOFTRESET_EN;
> -	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW1(ch), val);
> -
> -	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW1(ch));
> -	val |= CHV_PCS_REQ_SOFTRESET_EN;
> -	vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW1(ch), val);
> -
> -	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW0(ch));
> -	val &= ~(DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> -	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW0(ch), val);
> -
> -	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW0(ch));
> -	val &= ~(DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> -	vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW0(ch), val);
> +	/* Assert data lane reset */
> +	chv_data_lane_soft_reset(encoder, true);
>   
>   	mutex_unlock(&dev_priv->sb_lock);
>   }
> @@ -1796,23 +1823,6 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
>   	val &= ~DPIO_LANEDESKEW_STRAP_OVRD;
>   	vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW11(ch), val);
>   
> -	/* Deassert soft data lane reset*/
> -	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW1(ch));
> -	val |= CHV_PCS_REQ_SOFTRESET_EN;
> -	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW1(ch), val);
> -
> -	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW1(ch));
> -	val |= CHV_PCS_REQ_SOFTRESET_EN;
> -	vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW1(ch), val);
> -
> -	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW0(ch));
> -	val |= (DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> -	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW0(ch), val);
> -
> -	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW0(ch));
> -	val |= (DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> -	vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW0(ch), val);
> -
>   	/* Program Tx latency optimal setting */
>   	for (i = 0; i < 4; i++) {
>   		/* Set the upar bit */
> @@ -1855,6 +1865,9 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
>   		       DPIO_TX1_STAGGER_MULT(7) |
>   		       DPIO_TX2_STAGGER_MULT(5));
>   
> +	/* Deassert data lane reset */
> +	chv_data_lane_soft_reset(encoder, false);
> +
>   	/* Clear calc init */
>   	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW10(ch));
>   	val &= ~(DPIO_PCS_SWING_CALC_TX0_TX2 | DPIO_PCS_SWING_CALC_TX1_TX3);
Changes looks fine
Reviewed-by: Deepak S <deepak.s@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2015-08-27  4:27 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-08 20:45 [PATCH 00/15] drm/i915: CHV DPIO power gating, take two ville.syrjala
2015-07-08 20:45 ` [PATCH 01/15] drm/i915: Always program m2 fractional value on CHV ville.syrjala
2015-08-17  2:19   ` Deepak
2015-08-17 11:45     ` Ville Syrjälä
2015-08-26  8:11       ` Deepak
2015-07-08 20:45 ` [PATCH 02/15] drm/i915: Always program unique transition scale for CHV ville.syrjala
2015-08-17  2:31   ` Deepak
2015-07-08 20:45 ` [PATCH 03/15] drm/i915: Add encoder->post_pll_disable() hooks and move CHV clock buffer disables there ville.syrjala
2015-08-17  4:16   ` Deepak
2015-08-17 11:53     ` Ville Syrjälä
2015-08-26  8:14       ` Deepak
2015-07-08 20:45 ` [PATCH 04/15] drm/i915: Move DPIO port init earlier ville.syrjala
2015-08-17  4:18   ` Deepak
2015-07-08 20:45 ` [PATCH 05/15] drm/i915: Add locking around chv_phy_control_init() ville.syrjala
2015-08-17  4:23   ` Deepak
2015-07-08 20:45 ` [PATCH 06/15] drm/i915: Move VLV/CHV prepare_pll later ville.syrjala
2015-08-17  4:36   ` Deepak
2015-08-17  4:39   ` Deepak
2015-07-08 20:45 ` [PATCH 07/15] drm/i915: Add vlv_dport_to_phy() ville.syrjala
2015-08-17  4:56   ` Deepak
2015-08-26  8:24   ` Daniel Vetter
2015-07-08 20:45 ` [PATCH 08/15] drm/i915: Implement PHY lane power gating for CHV ville.syrjala
2015-08-19  1:48   ` Deepak
2015-08-26  8:27   ` Daniel Vetter
2015-07-08 20:45 ` [PATCH 09/15] drm/i915: Trick CL2 into life on CHV when using pipe B with port B ville.syrjala
2015-08-19  2:17   ` Deepak
2015-08-19 11:29     ` Ville Syrjälä
2015-08-26 12:36       ` Daniel Vetter
2015-07-08 20:45 ` [PATCH 10/15] drm/i915: Force common lane on for the PPS kick on CHV ville.syrjala
2015-07-10  7:56   ` [PATCH v2 " ville.syrjala
2015-08-19  2:21   ` [PATCH " Deepak
2015-08-19 11:32     ` Ville Syrjälä
2015-07-08 20:45 ` [PATCH 11/15] drm/i915: Enable DPIO SUS clock gating " ville.syrjala
2015-08-19 13:09   ` Deepak
2015-07-08 20:45 ` [PATCH 12/15] drm/i915: Force CL2 off in CHV x1 PHY ville.syrjala
2015-08-19 13:22   ` Deepak
2015-08-19 13:39     ` Ville Syrjälä
2015-08-26 12:38       ` Daniel Vetter
2015-07-08 20:45 ` [PATCH 13/15] drm/i915: Clean up CHV lane soft reset programming ville.syrjala
2015-07-09 16:27   ` Daniel Vetter
2015-07-09 17:14   ` [PATCH v2 " ville.syrjala
2015-08-27  4:25   ` Deepak [this message]
2015-07-08 20:46 ` [PATCH 14/15] drm/i915: Add some CHV DPIO lane power state asserts ville.syrjala
2015-08-27  4:36   ` Deepak
2015-08-27 11:02     ` Ville Syrjälä
2015-08-31 10:47       ` Deepak
2015-07-08 20:46 ` [PATCH 15/15] drm/i915: Add CHV PHY LDO power sanity checks ville.syrjala
2015-08-27  4:39   ` Deepak
2015-09-01  9:45     ` Daniel Vetter
2015-07-09 13:24 ` [PATCH 00/15] drm/i915: CHV DPIO power gating, take two Deepak

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=55DE9139.2020008@linux.intel.com \
    --to=deepak.s@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /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.