[22/23] drm/i915: Remove unneeded disconnect in TypeC legacy port mode

Submitted by Imre Deak on June 4, 2019, 2:58 p.m.

Details

Message ID 20190604145826.16424-23-imre.deak@intel.com
State New
Headers show
Series "drm/i915: Fix TypeC port mode switching" ( rev: 2 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Imre Deak June 4, 2019, 2:58 p.m.
Disconnecting the TypeC PHY when the port is in legacy mode is not
necessary:
- BSpec doesn't specify a disconnect sequence for legacy mode.
- The use of the PHY is dedicated for the display in legacy mode.
- We keep the PHY always connected during runtime as well in legacy
  mode.

We disconnect the PHY when needed during a disabling modeset for the
port, so we can also remove the disconnect call from the destroy hook.

Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 21 +--------------------
 drivers/gpu/drm/i915/intel_tc.c  |  4 +++-
 drivers/gpu/drm/i915/intel_tc.h  |  2 --
 3 files changed, 4 insertions(+), 23 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 138950941246..9c198f1a3a91 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3941,31 +3941,12 @@  static int intel_ddi_compute_config(struct intel_encoder *encoder,
 	return 0;
 }
 
-static void intel_ddi_encoder_suspend(struct intel_encoder *encoder)
-{
-	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
-
-	intel_dp_encoder_suspend(encoder);
-
-	/*
-	 * TODO: disconnect also from USB DP alternate mode once we have a
-	 * way to handle the modeset restore in that mode during resume
-	 * even if the sink has disappeared while being suspended.
-	 */
-	if (dig_port->tc_legacy_port)
-		icl_tc_phy_disconnect(dig_port);
-}
-
 static void intel_ddi_encoder_destroy(struct drm_encoder *encoder)
 {
 	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
-	struct drm_i915_private *i915 = to_i915(encoder->dev);
 
 	intel_dp_encoder_flush_work(encoder);
 
-	if (intel_port_is_tc(i915, dig_port->base.port))
-		icl_tc_phy_disconnect(dig_port);
-
 	drm_encoder_cleanup(encoder);
 	kfree(dig_port);
 }
@@ -4254,7 +4235,7 @@  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_encoder->update_pipe = intel_ddi_update_pipe;
 	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
 	intel_encoder->get_config = intel_ddi_get_config;
-	intel_encoder->suspend = intel_ddi_encoder_suspend;
+	intel_encoder->suspend = intel_dp_encoder_suspend;
 	intel_encoder->get_power_domains = intel_ddi_get_power_domains;
 	intel_encoder->type = INTEL_OUTPUT_DDI;
 	intel_encoder->power_domain = intel_port_to_power_domain(port);
diff --git a/drivers/gpu/drm/i915/intel_tc.c b/drivers/gpu/drm/i915/intel_tc.c
index d807127ad5f1..29a59ce7f073 100644
--- a/drivers/gpu/drm/i915/intel_tc.c
+++ b/drivers/gpu/drm/i915/intel_tc.c
@@ -245,10 +245,12 @@  static void icl_tc_phy_connect(struct intel_digital_port *dig_port,
  * See the comment at the connect function. This implements the Disconnect
  * Flow.
  */
-void icl_tc_phy_disconnect(struct intel_digital_port *dig_port)
+static void icl_tc_phy_disconnect(struct intel_digital_port *dig_port)
 {
 	switch (dig_port->tc_mode) {
 	case TC_PORT_LEGACY:
+		/* Nothing to do, we never disconnect from legacy mode */
+		break;
 	case TC_PORT_DP_ALT:
 		icl_tc_phy_set_safe_mode(dig_port, true);
 		dig_port->tc_mode = TC_PORT_TBT_ALT;
diff --git a/drivers/gpu/drm/i915/intel_tc.h b/drivers/gpu/drm/i915/intel_tc.h
index 568844e1846f..6d7e813c082b 100644
--- a/drivers/gpu/drm/i915/intel_tc.h
+++ b/drivers/gpu/drm/i915/intel_tc.h
@@ -5,8 +5,6 @@ 
 #include <linux/mutex.h>
 #include "intel_drv.h"
 
-void icl_tc_phy_disconnect(struct intel_digital_port *dig_port);
-
 bool intel_tc_port_connected(struct intel_digital_port *dig_port);
 u32 intel_tc_port_get_lane_info(struct intel_digital_port *dig_port);
 int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port);

Comments

On Tue, 2019-06-04 at 17:58 +0300, Imre Deak wrote:
> Disconnecting the TypeC PHY when the port is in legacy mode is not

> necessary:

> - BSpec doesn't specify a disconnect sequence for legacy mode.

> - The use of the PHY is dedicated for the display in legacy mode.

> - We keep the PHY always connected during runtime as well in legacy

>   mode.

> 

> We disconnect the PHY when needed during a disabling modeset for the

> port, so we can also remove the disconnect call from the destroy

> hook.

> 


Reviewed-by: José Roberto de Souza <jose.souza@intel.com>


> Cc: José Roberto de Souza <jose.souza@intel.com>

> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

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

> ---

>  drivers/gpu/drm/i915/intel_ddi.c | 21 +--------------------

>  drivers/gpu/drm/i915/intel_tc.c  |  4 +++-

>  drivers/gpu/drm/i915/intel_tc.h  |  2 --

>  3 files changed, 4 insertions(+), 23 deletions(-)

> 

> diff --git a/drivers/gpu/drm/i915/intel_ddi.c

> b/drivers/gpu/drm/i915/intel_ddi.c

> index 138950941246..9c198f1a3a91 100644

> --- a/drivers/gpu/drm/i915/intel_ddi.c

> +++ b/drivers/gpu/drm/i915/intel_ddi.c

> @@ -3941,31 +3941,12 @@ static int intel_ddi_compute_config(struct

> intel_encoder *encoder,

>  	return 0;

>  }

>  

> -static void intel_ddi_encoder_suspend(struct intel_encoder *encoder)

> -{

> -	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder-

> >base);

> -

> -	intel_dp_encoder_suspend(encoder);

> -

> -	/*

> -	 * TODO: disconnect also from USB DP alternate mode once we

> have a

> -	 * way to handle the modeset restore in that mode during resume

> -	 * even if the sink has disappeared while being suspended.

> -	 */

> -	if (dig_port->tc_legacy_port)

> -		icl_tc_phy_disconnect(dig_port);

> -}

> -

>  static void intel_ddi_encoder_destroy(struct drm_encoder *encoder)

>  {

>  	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);

> -	struct drm_i915_private *i915 = to_i915(encoder->dev);

>  

>  	intel_dp_encoder_flush_work(encoder);

>  

> -	if (intel_port_is_tc(i915, dig_port->base.port))

> -		icl_tc_phy_disconnect(dig_port);

> -

>  	drm_encoder_cleanup(encoder);

>  	kfree(dig_port);

>  }

> @@ -4254,7 +4235,7 @@ void intel_ddi_init(struct drm_i915_private

> *dev_priv, enum port port)

>  	intel_encoder->update_pipe = intel_ddi_update_pipe;

>  	intel_encoder->get_hw_state = intel_ddi_get_hw_state;

>  	intel_encoder->get_config = intel_ddi_get_config;

> -	intel_encoder->suspend = intel_ddi_encoder_suspend;

> +	intel_encoder->suspend = intel_dp_encoder_suspend;

>  	intel_encoder->get_power_domains = intel_ddi_get_power_domains;

>  	intel_encoder->type = INTEL_OUTPUT_DDI;

>  	intel_encoder->power_domain = intel_port_to_power_domain(port);

> diff --git a/drivers/gpu/drm/i915/intel_tc.c

> b/drivers/gpu/drm/i915/intel_tc.c

> index d807127ad5f1..29a59ce7f073 100644

> --- a/drivers/gpu/drm/i915/intel_tc.c

> +++ b/drivers/gpu/drm/i915/intel_tc.c

> @@ -245,10 +245,12 @@ static void icl_tc_phy_connect(struct

> intel_digital_port *dig_port,

>   * See the comment at the connect function. This implements the

> Disconnect

>   * Flow.

>   */

> -void icl_tc_phy_disconnect(struct intel_digital_port *dig_port)

> +static void icl_tc_phy_disconnect(struct intel_digital_port

> *dig_port)

>  {

>  	switch (dig_port->tc_mode) {

>  	case TC_PORT_LEGACY:

> +		/* Nothing to do, we never disconnect from legacy mode

> */

> +		break;

>  	case TC_PORT_DP_ALT:

>  		icl_tc_phy_set_safe_mode(dig_port, true);

>  		dig_port->tc_mode = TC_PORT_TBT_ALT;

> diff --git a/drivers/gpu/drm/i915/intel_tc.h

> b/drivers/gpu/drm/i915/intel_tc.h

> index 568844e1846f..6d7e813c082b 100644

> --- a/drivers/gpu/drm/i915/intel_tc.h

> +++ b/drivers/gpu/drm/i915/intel_tc.h

> @@ -5,8 +5,6 @@

>  #include <linux/mutex.h>

>  #include "intel_drv.h"

>  

> -void icl_tc_phy_disconnect(struct intel_digital_port *dig_port);

> -

>  bool intel_tc_port_connected(struct intel_digital_port *dig_port);

>  u32 intel_tc_port_get_lane_info(struct intel_digital_port

> *dig_port);

>  int intel_tc_port_fia_max_lane_count(struct intel_digital_port

> *dig_port);