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

2017-09-19 Thread Pandiyan, Dhinakaran 
On Wed, 2017-09-13 at 13:06 -0700, Dhinakaran Pandiyan wrote:
> 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)
> 

The patch probably does not fix all problems of these bugs, but some
problems do go away.

References: https://bugs.freedesktop.org/show_bug.cgi?id=90963
References: https://bugs.freedesktop.org/show_bug.cgi?id=88124

Thanks for the reviews.
-DK


> Cc: Ville Syrjälä 
> Cc: Lyude 
> Cc: Maarten Lankhorst 
> Signed-off-by: Dhinakaran Pandiyan 
> ---
>  drivers/gpu/drm/i915/intel_ddi.c| 18 ++
>  drivers/gpu/drm/i915/intel_dp_mst.c | 13 +
>  2 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 1da3bb2cc4b4..0053d66393f8 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);
> @@ -2235,12 +2236,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 8e3aad0ea60b..187f3f05a828 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -164,15 +164,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) {
> + 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);
> - }
>  }
>  
>  static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
> @@ -197,6 +201,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);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2017-09-18 Thread Maarten Lankhorst 
Op 13-09-17 om 22:06 schreef Dhinakaran Pandiyan:
> 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)
>
> Cc: Ville Syrjälä 
> Cc: Lyude 
> Cc: Maarten Lankhorst 
> Signed-off-by: Dhinakaran Pandiyan 
Acked-by: Maarten Lankhorst 
> ---
>  drivers/gpu/drm/i915/intel_ddi.c| 18 ++
>  drivers/gpu/drm/i915/intel_dp_mst.c | 13 +
>  2 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 1da3bb2cc4b4..0053d66393f8 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);
> @@ -2235,12 +2236,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 8e3aad0ea60b..187f3f05a828 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -164,15 +164,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) {
> + 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);
> - }
>  }
>  
>  static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
> @@ -197,6 +201,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);


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2017-09-14 Thread Lyude Paul 
Reviewed-by: Lyude Paul 

On Wed, 2017-09-13 at 13:06 -0700, Dhinakaran Pandiyan wrote:
> 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)
> 
> Cc: Ville Syrjälä 
> Cc: Lyude 
> Cc: Maarten Lankhorst 
> Signed-off-by: Dhinakaran Pandiyan 
> ---
>  drivers/gpu/drm/i915/intel_ddi.c| 18 ++
>  drivers/gpu/drm/i915/intel_dp_mst.c | 13 +
>  2 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 1da3bb2cc4b4..0053d66393f8 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);
> @@ -2235,12 +2236,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 8e3aad0ea60b..187f3f05a828 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -164,15 +164,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) {
> + 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);
> - }
>  }
>  
>  static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
> @@ -197,6 +201,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);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2017-09-13 Thread Dhinakaran Pandiyan 
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)

Cc: Ville Syrjälä 
Cc: Lyude 
Cc: Maarten Lankhorst 
Signed-off-by: Dhinakaran Pandiyan 
---
 drivers/gpu/drm/i915/intel_ddi.c| 18 ++
 drivers/gpu/drm/i915/intel_dp_mst.c | 13 +
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 1da3bb2cc4b4..0053d66393f8 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);
@@ -2235,12 +2236,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 8e3aad0ea60b..187f3f05a828 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -164,15 +164,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) {
+   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);
-   }
 }
 
 static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
@@ -197,6 +201,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

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx