[Intel-gfx] [PATCH v3] drm/i915/mst: Use MST sideband message transactions for dpms control

[Jani Nikula]

On Wed, 04 Oct 2017, Lyude Paul <lyude at redhat.com> wrote:
> On Wed, 2017-10-04 at 11:47 +0300, Jani Nikula wrote:
>> On Tue, 03 Oct 2017, Jani Nikula <jani.nikula at intel.com> wrote:
>> > From: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
>> > 
>> > Use the POWER_DOWN_PHY and POWER_UP_PHY sideband message transactions 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
>> > 
>> > v2: Modify and document the dpms and port disable order (Ville)
>> >     Add comment explaining is_mst = !crtc_state equivalence(Ville,
>> > Maarten)
>> > 
>> > v3 by Jani: rebase
>> > 
>> > References: https://bugs.freedesktop.org/show_bug.cgi?id=90963
>> > References: https://bugs.freedesktop.org/show_bug.cgi?id=88124
>> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> > Cc: Lyude <lyude at redhat.com>
>> > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> > Reviewed-by: Lyude Paul <lyude at redhat.com>
>> > Acked-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> 
>> Lyude, Maarten, can you confirm your rb/ack on my rebase please?
> Looks good to me

Thanks Maarten and Lyude, pushed to dinq.

BR,
Jani.

>> 
>> BR,
>> Jani.
>> 
>> 
>> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
>> > Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_ddi.c    | 18 ++++++++++++++----
>> >  drivers/gpu/drm/i915/intel_dp_mst.c | 10 ++++++++--
>> >  2 files changed, 22 insertions(+), 6 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>> > b/drivers/gpu/drm/i915/intel_ddi.c
>> > index 65f4b6786791..511aa60e0176 100644
>> > --- a/drivers/gpu/drm/i915/intel_ddi.c
>> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> > @@ -2162,7 +2162,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);
>> > @@ -2236,12 +2237,21 @@ static void intel_ddi_post_disable(struct
>> > intel_encoder *intel_encoder,
>> >  	uint32_t val;
>> >  	bool wait = false;
>> >  
>> > -	/* old_crtc_state and old_conn_state are NULL when called from
>> > DP_MST */
>> > -
>> >  	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) {
>> > +		/*
>> > +		 * old_crtc_state and old_conn_state are NULL when called
>> > from
>> > +		 * DP_MST. The main connector associated with this port
>> > is never
>> > +		 * bound to a crtc for MST.
>> > +		 */
>> > +		bool is_mst = !old_crtc_state;
>> >  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>> >  
>> > -		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>> > +		/*
>> > +		 * Power down sink before disabling the port, otherwise
>> > we end
>> > +		 * up getting interrupts from the sink on detecting link
>> > loss.
>> > +		 */
>> > +		if (!is_mst)
>> > +			intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>> >  	}
>> >  
>> >  	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 9a396f483f8b..3c131e2544cf 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
>> > @@ -162,14 +162,19 @@ static void intel_mst_post_disable_dp(struct
>> > intel_encoder *encoder,
>> >  
>> >  	drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, connector->port);
>> >  
>> > +	/*
>> > +	 * Power down mst path before disabling the port, otherwise we
>> > end
>> > +	 * up getting interrupts from the sink upon detecting link loss.
>> > +	 */
>> > +	drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port,
>> > +				     false);
>> > +
>> >  	intel_dp->active_mst_links--;
>> >  
>> >  	intel_mst->connector = NULL;
>> >  	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_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
>> >  }
>> > @@ -196,6 +201,7 @@ static void intel_mst_pre_enable_dp(struct
>> > intel_encoder *encoder,
>> >  
>> >  	DRM_DEBUG_KMS("active links %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);
>> 
>> 

-- 
Jani Nikula, Intel Open Source Technology Center