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

["Souza, Jose" <jose.souza@intel.com>]

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);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx