[PATCH 1/5] drm/i915: Skip vswing programming for TBT

[PATCH 1/5] drm/i915: Skip vswing programming for TBT

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

In thunderbolt mode the PHY is owned by the thunderbolt controller.
We are not supposed to touch it. So skip the vswing programming
as well (we already skipped the other steps not applicable to TBT).

Touching this stuff could supposedly interfere with the PHY
programming done by the thunderbolt controller.

Cc: stable@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 9506b8048530..c94650488dc1 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2827,6 +2827,9 @@ static void icl_mg_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
 	int n_entries, ln;
 	u32 val;
 
+	if (enc_to_dig_port(encoder)->tc_mode == TC_PORT_TBT_ALT)
+		return;
+
 	ddi_translations = icl_get_mg_buf_trans(encoder, crtc_state, &n_entries);
 
 	if (drm_WARN_ON_ONCE(&dev_priv->drm, !ddi_translations))
@@ -2962,6 +2965,9 @@ tgl_dkl_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
 	u32 val, dpcnt_mask, dpcnt_val;
 	int n_entries, ln;
 
+	if (enc_to_dig_port(encoder)->tc_mode == TC_PORT_TBT_ALT)
+		return;
+
 	ddi_translations = tgl_get_dkl_buf_trans(encoder, crtc_state, &n_entries);
 
 	if (drm_WARN_ON_ONCE(&dev_priv->drm, !ddi_translations))
-- 
2.26.2

[PATCH 2/5] drm/i915: Extract intel_ddi_power_up_lanes()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reduce the copypasta by pulling the combo PHY lane
power up stuff into a helper. We'll have a third user soon.

Cc: stable@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 35 +++++++++++++-----------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index c94650488dc1..88cc6e2fbe91 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3641,6 +3641,23 @@ static void intel_ddi_disable_fec_state(struct intel_encoder *encoder,
 	intel_de_posting_read(dev_priv, dp_tp_ctl_reg(encoder, crtc_state));
 }
 
+static void intel_ddi_power_up_lanes(struct intel_encoder *encoder,
+				     const struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
+	enum phy phy = intel_port_to_phy(i915, encoder->port);
+
+	if (intel_phy_is_combo(i915, phy)) {
+		bool lane_reversal =
+			dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
+
+		intel_combo_phy_power_up_lanes(i915, phy, false,
+					       crtc_state->lane_count,
+					       lane_reversal);
+	}
+}
+
 static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
 				  struct intel_encoder *encoder,
 				  const struct intel_crtc_state *crtc_state,
@@ -3732,14 +3749,7 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
 	 * 7.f Combo PHY: Configure PORT_CL_DW10 Static Power Down to power up
 	 * the used lanes of the DDI.
 	 */
-	if (intel_phy_is_combo(dev_priv, phy)) {
-		bool lane_reversal =
-			dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
-
-		intel_combo_phy_power_up_lanes(dev_priv, phy, false,
-					       crtc_state->lane_count,
-					       lane_reversal);
-	}
+	intel_ddi_power_up_lanes(encoder, crtc_state);
 
 	/*
 	 * 7.g Configure and enable DDI_BUF_CTL
@@ -3830,14 +3840,7 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
 	else
 		intel_prepare_dp_ddi_buffers(encoder, crtc_state);
 
-	if (intel_phy_is_combo(dev_priv, phy)) {
-		bool lane_reversal =
-			dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
-
-		intel_combo_phy_power_up_lanes(dev_priv, phy, false,
-					       crtc_state->lane_count,
-					       lane_reversal);
-	}
+	intel_ddi_power_up_lanes(encoder, crtc_state);
 
 	intel_ddi_init_dp_buf_reg(encoder, crtc_state);
 	if (!is_mst)
-- 
2.26.2

[PATCH 3/5] drm/i915: Power up combo PHY lanes for for HDMI as well

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we only explicitly power up the combo PHY lanes
for DP. The spec says we should do it for HDMI as well.

Cc: stable@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 88cc6e2fbe91..8fbeb8c24efb 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4337,6 +4337,8 @@ static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
 		intel_de_write(dev_priv, reg, val);
 	}
 
+	intel_ddi_power_up_lanes(encoder, crtc_state);
+
 	/* In HDMI/DVI mode, the port width, and swing/emphasis values
 	 * are ignored so nothing special needs to be done besides
 	 * enabling the port.
-- 
2.26.2

Re: [Intel-gfx] [PATCH 1/5] drm/i915: Skip vswing programming for TBT

[Imre Deak]

On Thu, Jan 28, 2021 at 05:59:44PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> In thunderbolt mode the PHY is owned by the thunderbolt controller.
> We are not supposed to touch it. So skip the vswing programming
> as well (we already skipped the other steps not applicable to TBT).
> 
> Touching this stuff could supposedly interfere with the PHY
> programming done by the thunderbolt controller.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Matches the spec:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 9506b8048530..c94650488dc1 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2827,6 +2827,9 @@ static void icl_mg_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	int n_entries, ln;
>  	u32 val;
>  
> +	if (enc_to_dig_port(encoder)->tc_mode == TC_PORT_TBT_ALT)
> +		return;
> +
>  	ddi_translations = icl_get_mg_buf_trans(encoder, crtc_state, &n_entries);
>  
>  	if (drm_WARN_ON_ONCE(&dev_priv->drm, !ddi_translations))
> @@ -2962,6 +2965,9 @@ tgl_dkl_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	u32 val, dpcnt_mask, dpcnt_val;
>  	int n_entries, ln;
>  
> +	if (enc_to_dig_port(encoder)->tc_mode == TC_PORT_TBT_ALT)
> +		return;
> +
>  	ddi_translations = tgl_get_dkl_buf_trans(encoder, crtc_state, &n_entries);
>  
>  	if (drm_WARN_ON_ONCE(&dev_priv->drm, !ddi_translations))
> -- 
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/5] drm/i915: Extract intel_ddi_power_up_lanes()

[Imre Deak]

On Thu, Jan 28, 2021 at 05:59:45PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reduce the copypasta by pulling the combo PHY lane
> power up stuff into a helper. We'll have a third user soon.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 35 +++++++++++++-----------
>  1 file changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index c94650488dc1..88cc6e2fbe91 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3641,6 +3641,23 @@ static void intel_ddi_disable_fec_state(struct intel_encoder *encoder,
>  	intel_de_posting_read(dev_priv, dp_tp_ctl_reg(encoder, crtc_state));
>  }
>  
> +static void intel_ddi_power_up_lanes(struct intel_encoder *encoder,
> +				     const struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +	enum phy phy = intel_port_to_phy(i915, encoder->port);
> +
> +	if (intel_phy_is_combo(i915, phy)) {
> +		bool lane_reversal =
> +			dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
> +
> +		intel_combo_phy_power_up_lanes(i915, phy, false,
> +					       crtc_state->lane_count,
> +					       lane_reversal);
> +	}
> +}
> +
>  static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  				  struct intel_encoder *encoder,
>  				  const struct intel_crtc_state *crtc_state,
> @@ -3732,14 +3749,7 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  	 * 7.f Combo PHY: Configure PORT_CL_DW10 Static Power Down to power up
>  	 * the used lanes of the DDI.
>  	 */
> -	if (intel_phy_is_combo(dev_priv, phy)) {
> -		bool lane_reversal =
> -			dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
> -
> -		intel_combo_phy_power_up_lanes(dev_priv, phy, false,
> -					       crtc_state->lane_count,
> -					       lane_reversal);
> -	}
> +	intel_ddi_power_up_lanes(encoder, crtc_state);
>  
>  	/*
>  	 * 7.g Configure and enable DDI_BUF_CTL
> @@ -3830,14 +3840,7 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  	else
>  		intel_prepare_dp_ddi_buffers(encoder, crtc_state);
>  
> -	if (intel_phy_is_combo(dev_priv, phy)) {
> -		bool lane_reversal =
> -			dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
> -
> -		intel_combo_phy_power_up_lanes(dev_priv, phy, false,
> -					       crtc_state->lane_count,
> -					       lane_reversal);
> -	}
> +	intel_ddi_power_up_lanes(encoder, crtc_state);
>  
>  	intel_ddi_init_dp_buf_reg(encoder, crtc_state);
>  	if (!is_mst)
> -- 
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/5] drm/i915: Power up combo PHY lanes for for HDMI as well

[Imre Deak]

On Thu, Jan 28, 2021 at 05:59:46PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we only explicitly power up the combo PHY lanes
> for DP. The spec says we should do it for HDMI as well.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 88cc6e2fbe91..8fbeb8c24efb 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4337,6 +4337,8 @@ static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
>  		intel_de_write(dev_priv, reg, val);
>  	}
>  
> +	intel_ddi_power_up_lanes(encoder, crtc_state);
> +

Not sure if it matters, but the spec says to apply WA #1143 just before
enabling DDI_BUF_CTL.


>  	/* In HDMI/DVI mode, the port width, and swing/emphasis values
>  	 * are ignored so nothing special needs to be done besides
>  	 * enabling the port.
> -- 
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/5] drm/i915: Power up combo PHY lanes for for HDMI as well

[Ville Syrjälä]

On Fri, Jan 29, 2021 at 07:06:33PM +0200, Imre Deak wrote:
> On Thu, Jan 28, 2021 at 05:59:46PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Currently we only explicitly power up the combo PHY lanes
> > for DP. The spec says we should do it for HDMI as well.
> > 
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 88cc6e2fbe91..8fbeb8c24efb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -4337,6 +4337,8 @@ static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
> >  		intel_de_write(dev_priv, reg, val);
> >  	}
> >  
> > +	intel_ddi_power_up_lanes(encoder, crtc_state);
> > +
> 
> Not sure if it matters, but the spec says to apply WA #1143 just before
> enabling DDI_BUF_CTL.

intel_ddi_power_up_lanes() is a nop for pre-icl, so we still do that.
Also not sure what the final fate of that w/a will be since apparently
it's not working as intended.

That said I was debating with myself what order to put these in, but
in the end I chose this order because the w/a is related to the
vswing programming, and so wanted to keep it next to the BUF_TRANS
programming.

> 
> 
> >  	/* In HDMI/DVI mode, the port width, and swing/emphasis values
> >  	 * are ignored so nothing special needs to be done besides
> >  	 * enabling the port.
> > -- 
> > 2.26.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 3/5] drm/i915: Power up combo PHY lanes for for HDMI as well

[Imre Deak]

On Fri, Jan 29, 2021 at 07:18:03PM +0200, Ville Syrjälä wrote:
> On Fri, Jan 29, 2021 at 07:06:33PM +0200, Imre Deak wrote:
> > On Thu, Jan 28, 2021 at 05:59:46PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Currently we only explicitly power up the combo PHY lanes
> > > for DP. The spec says we should do it for HDMI as well.
> > > 
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_ddi.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index 88cc6e2fbe91..8fbeb8c24efb 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -4337,6 +4337,8 @@ static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
> > >  		intel_de_write(dev_priv, reg, val);
> > >  	}
> > >  
> > > +	intel_ddi_power_up_lanes(encoder, crtc_state);
> > > +
> > 
> > Not sure if it matters, but the spec says to apply WA #1143 just before
> > enabling DDI_BUF_CTL.
> 
> intel_ddi_power_up_lanes() is a nop for pre-icl, so we still do that.

Ok, missed this detail, it looks ok then:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> Also not sure what the final fate of that w/a will be since apparently
> it's not working as intended.
> 
> That said I was debating with myself what order to put these in, but
> in the end I chose this order because the w/a is related to the
> vswing programming, and so wanted to keep it next to the BUF_TRANS
> programming.
>
> > >  	/* In HDMI/DVI mode, the port width, and swing/emphasis values
> > >  	 * are ignored so nothing special needs to be done besides
> > >  	 * enabling the port.
> > > -- 
> > > 2.26.2
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel