diff mbox

drm/i915: Fix BXT lane latenccy optimal setting with MST

Message ID 20171027134348.31190-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Oct. 27, 2017, 1:43 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Call the DDI .pre_pll_enable() hook from the MST code so that BXT gets
the correct lane latency optimal setting applied. And we obviously need
to compute the correct value, and read it out to keep the state checker
happy.

While at it drop the useless 'encoder' parameter to
bxt_ddi_phy_calc_lane_lat_optim_mask()

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  3 +--
 drivers/gpu/drm/i915/intel_ddi.c      |  3 +--
 drivers/gpu/drm/i915/intel_dp_mst.c   | 23 +++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dpio_phy.c |  3 +--
 4 files changed, 26 insertions(+), 6 deletions(-)

Comments

Ville Syrjälä Oct. 27, 2017, 1:53 p.m. UTC | #1
On Fri, Oct 27, 2017 at 04:43:48PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Call the DDI .pre_pll_enable() hook from the MST code so that BXT gets
> the correct lane latency optimal setting applied. And we obviously need
> to compute the correct value, and read it out to keep the state checker
> happy.
> 
> While at it drop the useless 'encoder' parameter to
> bxt_ddi_phy_calc_lane_lat_optim_mask()
> 
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  3 +--
>  drivers/gpu/drm/i915/intel_ddi.c      |  3 +--
>  drivers/gpu/drm/i915/intel_dp_mst.c   | 23 +++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dpio_phy.c |  3 +--
>  4 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 366ba74b0ad2..de32576b2540 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -4177,8 +4177,7 @@ bool bxt_ddi_phy_is_enabled(struct drm_i915_private *dev_priv,
>  			    enum dpio_phy phy);
>  bool bxt_ddi_phy_verify_state(struct drm_i915_private *dev_priv,
>  			      enum dpio_phy phy);
> -uint8_t bxt_ddi_phy_calc_lane_lat_optim_mask(struct intel_encoder *encoder,
> -					     uint8_t lane_count);
> +uint8_t bxt_ddi_phy_calc_lane_lat_optim_mask(uint8_t lane_count);
>  void bxt_ddi_phy_set_lane_optim_mask(struct intel_encoder *encoder,
>  				     uint8_t lane_lat_optim_mask);
>  uint8_t bxt_ddi_phy_get_lane_lat_optim_mask(struct intel_encoder *encoder);
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 28c25cb9eb2c..d4b336cffe68 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2673,8 +2673,7 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
>  
>  	if (IS_GEN9_LP(dev_priv) && ret)
>  		pipe_config->lane_lat_optim_mask =
> -			bxt_ddi_phy_calc_lane_lat_optim_mask(encoder,
> -							     pipe_config->lane_count);
> +			bxt_ddi_phy_calc_lane_lat_optim_mask(pipe_config->lane_count);
>  
>  	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 3d62c63c0763..c34ffa959e90 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -88,6 +88,10 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  
>  	pipe_config->dp_m_n.tu = slots;
>  
> +	if (IS_GEN9_LP(dev_priv))
> +		pipe_config->lane_lat_optim_mask =
> +			bxt_ddi_phy_calc_lane_lat_optim_mask(pipe_config->lane_count);
> +
>  	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
>  
>  	return true;
> @@ -182,6 +186,20 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
>  	DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
>  }
>  
> +static void intel_mst_pre_pll_enable_dp(struct intel_encoder *encoder,
> +					const struct intel_crtc_state *pipe_config,
> +					const struct drm_connector_state *conn_state)
> +{
> +	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
> +	struct intel_digital_port *intel_dig_port = intel_mst->primary;
> +	struct intel_dp *intel_dp = &intel_dig_port->dp;
> +
> +	if (intel_dp->active_mst_links == 0 &&

BTW this is yet another place that's probably totally broken when
it comes to parallel modesets.

I'm thinking we should do something higher up to prevent parallel
modesets in general, until someone properly reviews all the modeset
code. That's assuming we allow parallel modesets, which I think we
do with nonblocking.

> +	    intel_dig_port->base.pre_pll_enable)
> +		intel_dig_port->base.pre_pll_enable(&intel_dig_port->base,
> +						    pipe_config, NULL);
> +}
> +
>  static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
>  				    const struct intel_crtc_state *pipe_config,
>  				    const struct drm_connector_state *conn_state)
> @@ -311,6 +329,10 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
>  
>  	intel_ddi_clock_get(&intel_dig_port->base, pipe_config);
>  
> +	if (IS_GEN9_LP(dev_priv))
> +		pipe_config->lane_lat_optim_mask =
> +			bxt_ddi_phy_get_lane_lat_optim_mask(encoder);
> +
>  	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
>  }
>  
> @@ -582,6 +604,7 @@ intel_dp_create_fake_mst_encoder(struct intel_digital_port *intel_dig_port, enum
>  	intel_encoder->compute_config = intel_dp_mst_compute_config;
>  	intel_encoder->disable = intel_mst_disable_dp;
>  	intel_encoder->post_disable = intel_mst_post_disable_dp;
> +	intel_encoder->pre_pll_enable = intel_mst_pre_pll_enable_dp;
>  	intel_encoder->pre_enable = intel_mst_pre_enable_dp;
>  	intel_encoder->enable = intel_mst_enable_dp;
>  	intel_encoder->get_hw_state = intel_dp_mst_enc_get_hw_state;
> diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c
> index de38d014ed39..63b76eac018f 100644
> --- a/drivers/gpu/drm/i915/intel_dpio_phy.c
> +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
> @@ -567,8 +567,7 @@ bool bxt_ddi_phy_verify_state(struct drm_i915_private *dev_priv,
>  }
>  
>  uint8_t
> -bxt_ddi_phy_calc_lane_lat_optim_mask(struct intel_encoder *encoder,
> -				     uint8_t lane_count)
> +bxt_ddi_phy_calc_lane_lat_optim_mask(uint8_t lane_count)
>  {
>  	switch (lane_count) {
>  	case 1:
> -- 
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst Oct. 27, 2017, 2 p.m. UTC | #2
Op 27-10-17 om 15:43 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Call the DDI .pre_pll_enable() hook from the MST code so that BXT gets
> the correct lane latency optimal setting applied. And we obviously need
> to compute the correct value, and read it out to keep the state checker
> happy.
>
> While at it drop the useless 'encoder' parameter to
> bxt_ddi_phy_calc_lane_lat_optim_mask()
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  3 +--
>  drivers/gpu/drm/i915/intel_ddi.c      |  3 +--
>  drivers/gpu/drm/i915/intel_dp_mst.c   | 23 +++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dpio_phy.c |  3 +--
>  4 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 366ba74b0ad2..de32576b2540 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -4177,8 +4177,7 @@ bool bxt_ddi_phy_is_enabled(struct drm_i915_private *dev_priv,
>  			    enum dpio_phy phy);
>  bool bxt_ddi_phy_verify_state(struct drm_i915_private *dev_priv,
>  			      enum dpio_phy phy);
> -uint8_t bxt_ddi_phy_calc_lane_lat_optim_mask(struct intel_encoder *encoder,
> -					     uint8_t lane_count);
> +uint8_t bxt_ddi_phy_calc_lane_lat_optim_mask(uint8_t lane_count);
>  void bxt_ddi_phy_set_lane_optim_mask(struct intel_encoder *encoder,
>  				     uint8_t lane_lat_optim_mask);
>  uint8_t bxt_ddi_phy_get_lane_lat_optim_mask(struct intel_encoder *encoder);
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 28c25cb9eb2c..d4b336cffe68 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2673,8 +2673,7 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
>  
>  	if (IS_GEN9_LP(dev_priv) && ret)
>  		pipe_config->lane_lat_optim_mask =
> -			bxt_ddi_phy_calc_lane_lat_optim_mask(encoder,
> -							     pipe_config->lane_count);
> +			bxt_ddi_phy_calc_lane_lat_optim_mask(pipe_config->lane_count);
>  
>  	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 3d62c63c0763..c34ffa959e90 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -88,6 +88,10 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  
>  	pipe_config->dp_m_n.tu = slots;
>  
> +	if (IS_GEN9_LP(dev_priv))
> +		pipe_config->lane_lat_optim_mask =
> +			bxt_ddi_phy_calc_lane_lat_optim_mask(pipe_config->lane_count);
> +
>  	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
>  
>  	return true;
> @@ -182,6 +186,20 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
>  	DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
>  }
>  
> +static void intel_mst_pre_pll_enable_dp(struct intel_encoder *encoder,
> +					const struct intel_crtc_state *pipe_config,
> +					const struct drm_connector_state *conn_state)
> +{
> +	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
> +	struct intel_digital_port *intel_dig_port = intel_mst->primary;
> +	struct intel_dp *intel_dp = &intel_dig_port->dp;
> +
> +	if (intel_dp->active_mst_links == 0 &&
> +	    intel_dig_port->base.pre_pll_enable)
> +		intel_dig_port->base.pre_pll_enable(&intel_dig_port->base,
> +						    pipe_config, NULL);
> +}
We really need to get better at locking in the future, but looks ok.
>  static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
>  				    const struct intel_crtc_state *pipe_config,
>  				    const struct drm_connector_state *conn_state)
> @@ -311,6 +329,10 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
>  
>  	intel_ddi_clock_get(&intel_dig_port->base, pipe_config);
>  
> +	if (IS_GEN9_LP(dev_priv))
> +		pipe_config->lane_lat_optim_mask =
> +			bxt_ddi_phy_get_lane_lat_optim_mask(encoder);
> +
>  	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
>  }
>  
> @@ -582,6 +604,7 @@ intel_dp_create_fake_mst_encoder(struct intel_digital_port *intel_dig_port, enum
>  	intel_encoder->compute_config = intel_dp_mst_compute_config;
>  	intel_encoder->disable = intel_mst_disable_dp;
>  	intel_encoder->post_disable = intel_mst_post_disable_dp;
> +	intel_encoder->pre_pll_enable = intel_mst_pre_pll_enable_dp;
>  	intel_encoder->pre_enable = intel_mst_pre_enable_dp;
>  	intel_encoder->enable = intel_mst_enable_dp;
>  	intel_encoder->get_hw_state = intel_dp_mst_enc_get_hw_state;
> diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c
> index de38d014ed39..63b76eac018f 100644
> --- a/drivers/gpu/drm/i915/intel_dpio_phy.c
> +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
> @@ -567,8 +567,7 @@ bool bxt_ddi_phy_verify_state(struct drm_i915_private *dev_priv,
>  }
>  
>  uint8_t
> -bxt_ddi_phy_calc_lane_lat_optim_mask(struct intel_encoder *encoder,
> -				     uint8_t lane_count)
> +bxt_ddi_phy_calc_lane_lat_optim_mask(uint8_t lane_count)
>  {
>  	switch (lane_count) {
>  	case 1:

I don't have a BXT with DP-MST to test with, so this might need an ack by someone who does :)

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 366ba74b0ad2..de32576b2540 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -4177,8 +4177,7 @@  bool bxt_ddi_phy_is_enabled(struct drm_i915_private *dev_priv,
 			    enum dpio_phy phy);
 bool bxt_ddi_phy_verify_state(struct drm_i915_private *dev_priv,
 			      enum dpio_phy phy);
-uint8_t bxt_ddi_phy_calc_lane_lat_optim_mask(struct intel_encoder *encoder,
-					     uint8_t lane_count);
+uint8_t bxt_ddi_phy_calc_lane_lat_optim_mask(uint8_t lane_count);
 void bxt_ddi_phy_set_lane_optim_mask(struct intel_encoder *encoder,
 				     uint8_t lane_lat_optim_mask);
 uint8_t bxt_ddi_phy_get_lane_lat_optim_mask(struct intel_encoder *encoder);
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 28c25cb9eb2c..d4b336cffe68 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2673,8 +2673,7 @@  static bool intel_ddi_compute_config(struct intel_encoder *encoder,
 
 	if (IS_GEN9_LP(dev_priv) && ret)
 		pipe_config->lane_lat_optim_mask =
-			bxt_ddi_phy_calc_lane_lat_optim_mask(encoder,
-							     pipe_config->lane_count);
+			bxt_ddi_phy_calc_lane_lat_optim_mask(pipe_config->lane_count);
 
 	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
 
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 3d62c63c0763..c34ffa959e90 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -88,6 +88,10 @@  static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 
 	pipe_config->dp_m_n.tu = slots;
 
+	if (IS_GEN9_LP(dev_priv))
+		pipe_config->lane_lat_optim_mask =
+			bxt_ddi_phy_calc_lane_lat_optim_mask(pipe_config->lane_count);
+
 	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
 
 	return true;
@@ -182,6 +186,20 @@  static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
 	DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
 }
 
+static void intel_mst_pre_pll_enable_dp(struct intel_encoder *encoder,
+					const struct intel_crtc_state *pipe_config,
+					const struct drm_connector_state *conn_state)
+{
+	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
+	struct intel_digital_port *intel_dig_port = intel_mst->primary;
+	struct intel_dp *intel_dp = &intel_dig_port->dp;
+
+	if (intel_dp->active_mst_links == 0 &&
+	    intel_dig_port->base.pre_pll_enable)
+		intel_dig_port->base.pre_pll_enable(&intel_dig_port->base,
+						    pipe_config, NULL);
+}
+
 static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
 				    const struct intel_crtc_state *pipe_config,
 				    const struct drm_connector_state *conn_state)
@@ -311,6 +329,10 @@  static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
 
 	intel_ddi_clock_get(&intel_dig_port->base, pipe_config);
 
+	if (IS_GEN9_LP(dev_priv))
+		pipe_config->lane_lat_optim_mask =
+			bxt_ddi_phy_get_lane_lat_optim_mask(encoder);
+
 	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
 }
 
@@ -582,6 +604,7 @@  intel_dp_create_fake_mst_encoder(struct intel_digital_port *intel_dig_port, enum
 	intel_encoder->compute_config = intel_dp_mst_compute_config;
 	intel_encoder->disable = intel_mst_disable_dp;
 	intel_encoder->post_disable = intel_mst_post_disable_dp;
+	intel_encoder->pre_pll_enable = intel_mst_pre_pll_enable_dp;
 	intel_encoder->pre_enable = intel_mst_pre_enable_dp;
 	intel_encoder->enable = intel_mst_enable_dp;
 	intel_encoder->get_hw_state = intel_dp_mst_enc_get_hw_state;
diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c
index de38d014ed39..63b76eac018f 100644
--- a/drivers/gpu/drm/i915/intel_dpio_phy.c
+++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
@@ -567,8 +567,7 @@  bool bxt_ddi_phy_verify_state(struct drm_i915_private *dev_priv,
 }
 
 uint8_t
-bxt_ddi_phy_calc_lane_lat_optim_mask(struct intel_encoder *encoder,
-				     uint8_t lane_count)
+bxt_ddi_phy_calc_lane_lat_optim_mask(uint8_t lane_count)
 {
 	switch (lane_count) {
 	case 1: