[PATCH 2/2] drm/i915/mst: Use MST sideband message transaction for dpms

[Ville Syrjälä]

On Tue, Sep 05, 2017 at 06:26:34PM -0700, Dhinakaran Pandiyan wrote:
> Use the POWER_DOWN_PHY and POWER_UP_PHY sideband message trasactions to
> set power states for downstream sinks. Apart from giving us the ability
> to set power state for individual sinks, this fixes the below test for
> me
> 
> $ xrandr --display :0 --output DP-2-2-8 --off
> $ xrandr --display :0 --output DP-2-2-1 --off
> $ xrandr --display :0 --output DP-2-2-8 --auto #Black screen
> $ xrandr --display :0 --output DP-2-2-1 --auto
> 
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Lyude <lyude at redhat.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c    | 6 ++++--
>  drivers/gpu/drm/i915/intel_dp_mst.c | 8 ++++----
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 1da3bb2cc4b4..8aebacc0aa31 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2161,7 +2161,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  		intel_prepare_dp_ddi_buffers(encoder);
>  
>  	intel_ddi_init_dp_buf_reg(encoder);
> -	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> +	if (!link_mst)
> +		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
>  	intel_dp_start_link_train(intel_dp);
>  	if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
>  		intel_dp_stop_link_train(intel_dp);
> @@ -2240,7 +2241,8 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder,
>  	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) {
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  
> -		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> +		if (old_crtc_state && old_conn_state)
> +			intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);

I would make that
!intel_crtc_has_type(DP_MST)

>  	}
>  
>  	val = I915_READ(DDI_BUF_CTL(port));
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 8e3aad0ea60b..81e63724e24b 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -167,12 +167,11 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
>  	intel_dp->active_mst_links--;
>  
>  	intel_mst->connector = NULL;
> -	if (intel_dp->active_mst_links == 0) {
> +	if (intel_dp->active_mst_links == 0)
>  		intel_dig_port->base.post_disable(&intel_dig_port->base,
>  						  NULL, NULL);
> -
> -		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> -	}
> +	drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port,
> +				     false);

One thing that bothers me here is whether we'll start getting bogus link
training requests during shutdown since we'll do the D3 only after the
stream has been stopped. If you'll look at commit 7618138d8b40
("drm/i915/ddi: Avoid long delays during system suspend / eDP
disabling") Imre explicitly moved the D3 earlier for SST to avoid this.

Hmm. Actually by the time we call mst_post_disable() we should have
deallocated the VC etc. so the current stream should be already
terminated at that point. So I think we should be able to do the
PHY_POWER_DOWN before we shut down the link, just like we do for SST.

>  }
>  
>  static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
> @@ -197,6 +196,7 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
>  
>  	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
>  
> +	drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port, true);
>  	if (intel_dp->active_mst_links == 0)
>  		intel_dig_port->base.pre_enable(&intel_dig_port->base,
>  						pipe_config, NULL);
> -- 
> 2.11.0

-- 
Ville Syrjälä
Intel OTC