All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Drop useless HAS_PSR() check
@ 2017-09-18 17:31 Ville Syrjala
  2017-09-18 17:31 ` [PATCH 2/2] drm/i915: Reorganize .disable hooks for pre-DDI DP Ville Syrjala
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Ville Syrjala @ 2017-09-18 17:31 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

It is safe to call intel_psr_disable() on a platform without PSR. We
don't have such a check when calling intel_psr_enable() either.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 887953c0f495..d0ea9c4f87c8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2697,8 +2697,7 @@ static void intel_disable_dp(struct intel_encoder *encoder,
 	if (old_crtc_state->has_audio)
 		intel_audio_codec_disable(encoder);
 
-	if (HAS_PSR(dev_priv) && !HAS_DDI(dev_priv))
-		intel_psr_disable(intel_dp, old_crtc_state);
+	intel_psr_disable(intel_dp, old_crtc_state);
 
 	/* Make sure the panel is off before trying to change the mode. But also
 	 * ensure that we have vdd while we switch off the panel. */
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] drm/i915: Reorganize .disable hooks for pre-DDI DP
  2017-09-18 17:31 [PATCH 1/2] drm/i915: Drop useless HAS_PSR() check Ville Syrjala
@ 2017-09-18 17:31 ` Ville Syrjala
  2017-09-18 20:45   ` Rodrigo Vivi
  2017-09-20 15:12   ` [PATCH v2 " Ville Syrjala
  2017-09-18 17:55 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Drop useless HAS_PSR() check Patchwork
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 10+ messages in thread
From: Ville Syrjala @ 2017-09-18 17:31 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Most of our DP encoder hooks are split into per-platform variants.
.disable() an exception, and thus it's a bit messy. Let's split it
up as well. We'll leave the common parts in a helper called by
each platform specific hook.

There is a subtle change on VLV/CHV where we now disable PSR before
audio, whereas before we disabled PSR after audio. That should be
totally fine, and PSR is disabled by default anyway.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 44 +++++++++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d0ea9c4f87c8..dcdefd986569 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2692,23 +2692,46 @@ static void intel_disable_dp(struct intel_encoder *encoder,
 			     const struct drm_connector_state *old_conn_state)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 
 	if (old_crtc_state->has_audio)
 		intel_audio_codec_disable(encoder);
 
-	intel_psr_disable(intel_dp, old_crtc_state);
-
 	/* Make sure the panel is off before trying to change the mode. But also
 	 * ensure that we have vdd while we switch off the panel. */
 	intel_edp_panel_vdd_on(intel_dp);
 	intel_edp_backlight_off(old_conn_state);
 	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 	intel_edp_panel_off(intel_dp);
+}
+
+static void g4x_disable_dp(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *old_crtc_state,
+			   const struct drm_connector_state *old_conn_state)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+	intel_disable_dp(encoder, old_crtc_state, old_conn_state);
 
 	/* disable the port before the pipe on g4x */
-	if (INTEL_GEN(dev_priv) < 5)
-		intel_dp_link_down(intel_dp);
+	intel_dp_link_down(intel_dp);
+}
+
+static void ilk_disable_dp(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *old_crtc_state,
+			   const struct drm_connector_state *old_conn_state)
+{
+	intel_disable_dp(encoder, old_crtc_state, old_conn_state);
+}
+
+static void vlv_disable_dp(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *old_crtc_state,
+			   const struct drm_connector_state *old_conn_state)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+	intel_psr_disable(intel_dp, old_crtc_state);
+
+	intel_disable_dp(encoder, old_crtc_state, old_conn_state);
 }
 
 static void ilk_post_disable_dp(struct intel_encoder *encoder,
@@ -6144,7 +6167,6 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
 		goto err_encoder_init;
 
 	intel_encoder->compute_config = intel_dp_compute_config;
-	intel_encoder->disable = intel_disable_dp;
 	intel_encoder->get_hw_state = intel_dp_get_hw_state;
 	intel_encoder->get_config = intel_dp_get_config;
 	intel_encoder->suspend = intel_dp_encoder_suspend;
@@ -6152,18 +6174,24 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
 		intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
 		intel_encoder->pre_enable = chv_pre_enable_dp;
 		intel_encoder->enable = vlv_enable_dp;
+		intel_encoder->disable = vlv_disable_dp;
 		intel_encoder->post_disable = chv_post_disable_dp;
 		intel_encoder->post_pll_disable = chv_dp_post_pll_disable;
 	} else if (IS_VALLEYVIEW(dev_priv)) {
 		intel_encoder->pre_pll_enable = vlv_dp_pre_pll_enable;
 		intel_encoder->pre_enable = vlv_pre_enable_dp;
 		intel_encoder->enable = vlv_enable_dp;
+		intel_encoder->disable = vlv_disable_dp;
 		intel_encoder->post_disable = vlv_post_disable_dp;
+	} else if (INTEL_GEN(dev_priv) >= 5) {
+		intel_encoder->pre_enable = g4x_pre_enable_dp;
+		intel_encoder->enable = g4x_enable_dp;
+		intel_encoder->disable = ilk_disable_dp;
+		intel_encoder->post_disable = ilk_post_disable_dp;
 	} else {
 		intel_encoder->pre_enable = g4x_pre_enable_dp;
 		intel_encoder->enable = g4x_enable_dp;
-		if (INTEL_GEN(dev_priv) >= 5)
-			intel_encoder->post_disable = ilk_post_disable_dp;
+		intel_encoder->disable = g4x_disable_dp;
 	}
 
 	intel_dig_port->port = port;
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Drop useless HAS_PSR() check
  2017-09-18 17:31 [PATCH 1/2] drm/i915: Drop useless HAS_PSR() check Ville Syrjala
  2017-09-18 17:31 ` [PATCH 2/2] drm/i915: Reorganize .disable hooks for pre-DDI DP Ville Syrjala
@ 2017-09-18 17:55 ` Patchwork
  2017-09-18 20:35 ` [PATCH 1/2] " Rodrigo Vivi
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2017-09-18 17:55 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Drop useless HAS_PSR() check
URL   : https://patchwork.freedesktop.org/series/30543/
State : warning

== Summary ==

Series 30543v1 series starting with [1/2] drm/i915: Drop useless HAS_PSR() check
https://patchwork.freedesktop.org/api/1.0/series/30543/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102514
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6770hq)

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:445s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:475s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:420s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:516s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:280s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:506s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:497s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:498s
fi-cfl-s         total:289  pass:223  dwarn:34  dfail:0   fail:0   skip:32  time:547s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:417s
fi-glk-2a        total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:593s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:429s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:407s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:432s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:498s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:462s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:467s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:584s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:586s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:544s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:456s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:764s
fi-skl-6770hq    total:289  pass:268  dwarn:1   dfail:0   fail:0   skip:20  time:494s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:475s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:563s
fi-snb-2600      total:289  pass:248  dwarn:0   dfail:0   fail:2   skip:39  time:414s

dee3ba3a8c8bc9b11aca9cddc4e47a17b15db7eb drm-tip: 2017y-09m-18d-15h-59m-43s UTC integration manifest
b21b43f20de6 drm/i915: Reorganize .disable hooks for pre-DDI DP
27589ea90963 drm/i915: Drop useless HAS_PSR() check

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5738/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/i915: Drop useless HAS_PSR() check
  2017-09-18 17:31 [PATCH 1/2] drm/i915: Drop useless HAS_PSR() check Ville Syrjala
  2017-09-18 17:31 ` [PATCH 2/2] drm/i915: Reorganize .disable hooks for pre-DDI DP Ville Syrjala
  2017-09-18 17:55 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Drop useless HAS_PSR() check Patchwork
@ 2017-09-18 20:35 ` Rodrigo Vivi
  2017-09-20 15:12 ` [PATCH v2 " Ville Syrjala
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Rodrigo Vivi @ 2017-09-18 20:35 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Mon, Sep 18, 2017 at 05:31:27PM +0000, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> It is safe to call intel_psr_disable() on a platform without PSR. We
> don't have such a check when calling intel_psr_enable() either.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 887953c0f495..d0ea9c4f87c8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2697,8 +2697,7 @@ static void intel_disable_dp(struct intel_encoder *encoder,
>  	if (old_crtc_state->has_audio)
>  		intel_audio_codec_disable(encoder);
>  
> -	if (HAS_PSR(dev_priv) && !HAS_DDI(dev_priv))

By removing the !HAS_DDI you are now duplicating the psr_disable on all platforms but VLV/CHV.

Maybe we should integrate that once for all and only call enable and disable in one place for
all platforms before removing this check.

But I agree HAS_PSR could be removed already.

> -		intel_psr_disable(intel_dp, old_crtc_state);
> +	intel_psr_disable(intel_dp, old_crtc_state);
>  
>  	/* Make sure the panel is off before trying to change the mode. But also
>  	 * ensure that we have vdd while we switch off the panel. */
> -- 
> 2.13.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/i915: Reorganize .disable hooks for pre-DDI DP
  2017-09-18 17:31 ` [PATCH 2/2] drm/i915: Reorganize .disable hooks for pre-DDI DP Ville Syrjala
@ 2017-09-18 20:45   ` Rodrigo Vivi
  2017-09-20 17:00     ` Ville Syrjälä
  2017-09-20 15:12   ` [PATCH v2 " Ville Syrjala
  1 sibling, 1 reply; 10+ messages in thread
From: Rodrigo Vivi @ 2017-09-18 20:45 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Mon, Sep 18, 2017 at 05:31:28PM +0000, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Most of our DP encoder hooks are split into per-platform variants.
> .disable() an exception, and thus it's a bit messy. Let's split it
> up as well. We'll leave the common parts in a helper called by
> each platform specific hook.
> 
> There is a subtle change on VLV/CHV where we now disable PSR before
> audio, whereas before we disabled PSR after audio. That should be
> totally fine, and PSR is disabled by default anyway.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 44 +++++++++++++++++++++++++++++++++--------
>  1 file changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d0ea9c4f87c8..dcdefd986569 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2692,23 +2692,46 @@ static void intel_disable_dp(struct intel_encoder *encoder,
>  			     const struct drm_connector_state *old_conn_state)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  
>  	if (old_crtc_state->has_audio)
>  		intel_audio_codec_disable(encoder);
>  
> -	intel_psr_disable(intel_dp, old_crtc_state);
> -
>  	/* Make sure the panel is off before trying to change the mode. But also
>  	 * ensure that we have vdd while we switch off the panel. */
>  	intel_edp_panel_vdd_on(intel_dp);
>  	intel_edp_backlight_off(old_conn_state);
>  	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>  	intel_edp_panel_off(intel_dp);
> +}
> +
> +static void g4x_disable_dp(struct intel_encoder *encoder,
> +			   const struct intel_crtc_state *old_crtc_state,
> +			   const struct drm_connector_state *old_conn_state)
> +{
> +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +
> +	intel_disable_dp(encoder, old_crtc_state, old_conn_state);
>  
>  	/* disable the port before the pipe on g4x */
> -	if (INTEL_GEN(dev_priv) < 5)
> -		intel_dp_link_down(intel_dp);
> +	intel_dp_link_down(intel_dp);
> +}
> +
> +static void ilk_disable_dp(struct intel_encoder *encoder,
> +			   const struct intel_crtc_state *old_crtc_state,
> +			   const struct drm_connector_state *old_conn_state)
> +{
> +	intel_disable_dp(encoder, old_crtc_state, old_conn_state);
> +}
> +
> +static void vlv_disable_dp(struct intel_encoder *encoder,
> +			   const struct intel_crtc_state *old_crtc_state,
> +			   const struct drm_connector_state *old_conn_state)
> +{
> +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +
> +	intel_psr_disable(intel_dp, old_crtc_state);

oh! not it makes sense...
please move the that removal of !HAS_DDI from the first patch to this patch
and feel free to ad my rv-b on both patches.

> +
> +	intel_disable_dp(encoder, old_crtc_state, old_conn_state);
>  }
>  
>  static void ilk_post_disable_dp(struct intel_encoder *encoder,
> @@ -6144,7 +6167,6 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
>  		goto err_encoder_init;
>  
>  	intel_encoder->compute_config = intel_dp_compute_config;
> -	intel_encoder->disable = intel_disable_dp;
>  	intel_encoder->get_hw_state = intel_dp_get_hw_state;
>  	intel_encoder->get_config = intel_dp_get_config;
>  	intel_encoder->suspend = intel_dp_encoder_suspend;
> @@ -6152,18 +6174,24 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
>  		intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
>  		intel_encoder->pre_enable = chv_pre_enable_dp;
>  		intel_encoder->enable = vlv_enable_dp;
> +		intel_encoder->disable = vlv_disable_dp;
>  		intel_encoder->post_disable = chv_post_disable_dp;
>  		intel_encoder->post_pll_disable = chv_dp_post_pll_disable;
>  	} else if (IS_VALLEYVIEW(dev_priv)) {
>  		intel_encoder->pre_pll_enable = vlv_dp_pre_pll_enable;
>  		intel_encoder->pre_enable = vlv_pre_enable_dp;
>  		intel_encoder->enable = vlv_enable_dp;
> +		intel_encoder->disable = vlv_disable_dp;
>  		intel_encoder->post_disable = vlv_post_disable_dp;
> +	} else if (INTEL_GEN(dev_priv) >= 5) {
> +		intel_encoder->pre_enable = g4x_pre_enable_dp;
> +		intel_encoder->enable = g4x_enable_dp;
> +		intel_encoder->disable = ilk_disable_dp;
> +		intel_encoder->post_disable = ilk_post_disable_dp;
>  	} else {
>  		intel_encoder->pre_enable = g4x_pre_enable_dp;
>  		intel_encoder->enable = g4x_enable_dp;
> -		if (INTEL_GEN(dev_priv) >= 5)
> -			intel_encoder->post_disable = ilk_post_disable_dp;
> +		intel_encoder->disable = g4x_disable_dp;
>  	}
>  
>  	intel_dig_port->port = port;
> -- 
> 2.13.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] drm/i915: Drop useless HAS_PSR() check
  2017-09-18 17:31 [PATCH 1/2] drm/i915: Drop useless HAS_PSR() check Ville Syrjala
                   ` (2 preceding siblings ...)
  2017-09-18 20:35 ` [PATCH 1/2] " Rodrigo Vivi
@ 2017-09-20 15:12 ` Ville Syrjala
  2017-09-20 16:04 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915: Drop useless HAS_PSR() check (rev3) Patchwork
  2017-09-20 17:19 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjala @ 2017-09-20 15:12 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

It is safe to call intel_psr_disable() on a platform without PSR. We
don't have such a check when calling intel_psr_enable() either.

v2: Don't drop the HAS_DDI check quite yet (Rodrigo)

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8db6b11f103f..e536f5942d7f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2697,7 +2697,7 @@ static void intel_disable_dp(struct intel_encoder *encoder,
 	if (old_crtc_state->has_audio)
 		intel_audio_codec_disable(encoder);
 
-	if (HAS_PSR(dev_priv) && !HAS_DDI(dev_priv))
+	if (!HAS_DDI(dev_priv))
 		intel_psr_disable(intel_dp, old_crtc_state);
 
 	/* Make sure the panel is off before trying to change the mode. But also
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] drm/i915: Reorganize .disable hooks for pre-DDI DP
  2017-09-18 17:31 ` [PATCH 2/2] drm/i915: Reorganize .disable hooks for pre-DDI DP Ville Syrjala
  2017-09-18 20:45   ` Rodrigo Vivi
@ 2017-09-20 15:12   ` Ville Syrjala
  1 sibling, 0 replies; 10+ messages in thread
From: Ville Syrjala @ 2017-09-20 15:12 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Most of our DP encoder hooks are split into per-platform variants.
.disable() an exception, and thus it's a bit messy. Let's split it
up as well. We'll leave the common parts in a helper called by
each platform specific hook. Now each platform has mostly its own
hooks. Some hooks are still shared between vlv and chv, and between
g4x and ilk. None of the remaining shared hooks have any platform
checks in them however so duplicating them doesn't seem particularly
useful.

There is a subtle change on VLV/CHV where we now disable PSR before
audio, whereas before we disabled PSR after audio. That should be
totally fine, and PSR is disabled by default anyway. Jani also pointed
out to me that PSR + audio doesn't seem like a particularly realistic
combination.

v2: Drop the PSR HAS_DDI check here (Rodrigo)
    Pimp up the commit message a bit based on a chat with Jani

Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 45 ++++++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e536f5942d7f..1e0bfbe6b4f3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2692,24 +2692,46 @@ static void intel_disable_dp(struct intel_encoder *encoder,
 			     const struct drm_connector_state *old_conn_state)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 
 	if (old_crtc_state->has_audio)
 		intel_audio_codec_disable(encoder);
 
-	if (!HAS_DDI(dev_priv))
-		intel_psr_disable(intel_dp, old_crtc_state);
-
 	/* Make sure the panel is off before trying to change the mode. But also
 	 * ensure that we have vdd while we switch off the panel. */
 	intel_edp_panel_vdd_on(intel_dp);
 	intel_edp_backlight_off(old_conn_state);
 	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 	intel_edp_panel_off(intel_dp);
+}
+
+static void g4x_disable_dp(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *old_crtc_state,
+			   const struct drm_connector_state *old_conn_state)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+	intel_disable_dp(encoder, old_crtc_state, old_conn_state);
 
 	/* disable the port before the pipe on g4x */
-	if (INTEL_GEN(dev_priv) < 5)
-		intel_dp_link_down(intel_dp);
+	intel_dp_link_down(intel_dp);
+}
+
+static void ilk_disable_dp(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *old_crtc_state,
+			   const struct drm_connector_state *old_conn_state)
+{
+	intel_disable_dp(encoder, old_crtc_state, old_conn_state);
+}
+
+static void vlv_disable_dp(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *old_crtc_state,
+			   const struct drm_connector_state *old_conn_state)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+	intel_psr_disable(intel_dp, old_crtc_state);
+
+	intel_disable_dp(encoder, old_crtc_state, old_conn_state);
 }
 
 static void ilk_post_disable_dp(struct intel_encoder *encoder,
@@ -6145,7 +6167,6 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
 		goto err_encoder_init;
 
 	intel_encoder->compute_config = intel_dp_compute_config;
-	intel_encoder->disable = intel_disable_dp;
 	intel_encoder->get_hw_state = intel_dp_get_hw_state;
 	intel_encoder->get_config = intel_dp_get_config;
 	intel_encoder->suspend = intel_dp_encoder_suspend;
@@ -6153,18 +6174,24 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
 		intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
 		intel_encoder->pre_enable = chv_pre_enable_dp;
 		intel_encoder->enable = vlv_enable_dp;
+		intel_encoder->disable = vlv_disable_dp;
 		intel_encoder->post_disable = chv_post_disable_dp;
 		intel_encoder->post_pll_disable = chv_dp_post_pll_disable;
 	} else if (IS_VALLEYVIEW(dev_priv)) {
 		intel_encoder->pre_pll_enable = vlv_dp_pre_pll_enable;
 		intel_encoder->pre_enable = vlv_pre_enable_dp;
 		intel_encoder->enable = vlv_enable_dp;
+		intel_encoder->disable = vlv_disable_dp;
 		intel_encoder->post_disable = vlv_post_disable_dp;
+	} else if (INTEL_GEN(dev_priv) >= 5) {
+		intel_encoder->pre_enable = g4x_pre_enable_dp;
+		intel_encoder->enable = g4x_enable_dp;
+		intel_encoder->disable = ilk_disable_dp;
+		intel_encoder->post_disable = ilk_post_disable_dp;
 	} else {
 		intel_encoder->pre_enable = g4x_pre_enable_dp;
 		intel_encoder->enable = g4x_enable_dp;
-		if (INTEL_GEN(dev_priv) >= 5)
-			intel_encoder->post_disable = ilk_post_disable_dp;
+		intel_encoder->disable = g4x_disable_dp;
 	}
 
 	intel_dig_port->port = port;
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915: Drop useless HAS_PSR() check (rev3)
  2017-09-18 17:31 [PATCH 1/2] drm/i915: Drop useless HAS_PSR() check Ville Syrjala
                   ` (3 preceding siblings ...)
  2017-09-20 15:12 ` [PATCH v2 " Ville Syrjala
@ 2017-09-20 16:04 ` Patchwork
  2017-09-20 17:19 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2017-09-20 16:04 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] drm/i915: Drop useless HAS_PSR() check (rev3)
URL   : https://patchwork.freedesktop.org/series/30543/
State : success

== Summary ==

Series 30543v3 series starting with [v2,1/2] drm/i915: Drop useless HAS_PSR() check
https://patchwork.freedesktop.org/api/1.0/series/30543/revisions/3/mbox/

Test gem_exec_suspend:
        Subgroup basic-s3:
                incomplete -> PASS       (fi-kbl-7500u) fdo#102850 +1
Test kms_force_connector_basic:
        Subgroup force-connector-state:
                skip       -> PASS       (fi-ivb-3520m)
        Subgroup force-edid:
                skip       -> PASS       (fi-ivb-3520m)
        Subgroup force-load-detect:
                skip       -> PASS       (fi-ivb-3520m)
        Subgroup prune-stale-modes:
                skip       -> PASS       (fi-ivb-3520m)

fdo#102850 https://bugs.freedesktop.org/show_bug.cgi?id=102850

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:444s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:467s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:419s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:528s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:284s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:509s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:489s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:494s
fi-cfl-s         total:289  pass:222  dwarn:35  dfail:0   fail:0   skip:32  time:543s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:418s
fi-glk-1         total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:563s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:425s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:405s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:434s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:490s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:464s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:482s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:572s
fi-kbl-r         total:118  pass:97   dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:457s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:749s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:489s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:473s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:567s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:1   skip:39  time:422s
fi-pnv-d510 failed to connect after reboot

ed7a99bf23cea2276e77ba26d219e58e069d1e32 drm-tip: 2017y-09m-20d-11h-03m-37s UTC integration manifest
b830aa113ed4 drm/i915: Reorganize .disable hooks for pre-DDI DP
9fb36cda67cb drm/i915: Drop useless HAS_PSR() check

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5768/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/i915: Reorganize .disable hooks for pre-DDI DP
  2017-09-18 20:45   ` Rodrigo Vivi
@ 2017-09-20 17:00     ` Ville Syrjälä
  0 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjälä @ 2017-09-20 17:00 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Mon, Sep 18, 2017 at 01:45:50PM -0700, Rodrigo Vivi wrote:
> On Mon, Sep 18, 2017 at 05:31:28PM +0000, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Most of our DP encoder hooks are split into per-platform variants.
> > .disable() an exception, and thus it's a bit messy. Let's split it
> > up as well. We'll leave the common parts in a helper called by
> > each platform specific hook.
> > 
> > There is a subtle change on VLV/CHV where we now disable PSR before
> > audio, whereas before we disabled PSR after audio. That should be
> > totally fine, and PSR is disabled by default anyway.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 44 +++++++++++++++++++++++++++++++++--------
> >  1 file changed, 36 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index d0ea9c4f87c8..dcdefd986569 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2692,23 +2692,46 @@ static void intel_disable_dp(struct intel_encoder *encoder,
> >  			     const struct drm_connector_state *old_conn_state)
> >  {
> >  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  
> >  	if (old_crtc_state->has_audio)
> >  		intel_audio_codec_disable(encoder);
> >  
> > -	intel_psr_disable(intel_dp, old_crtc_state);
> > -
> >  	/* Make sure the panel is off before trying to change the mode. But also
> >  	 * ensure that we have vdd while we switch off the panel. */
> >  	intel_edp_panel_vdd_on(intel_dp);
> >  	intel_edp_backlight_off(old_conn_state);
> >  	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> >  	intel_edp_panel_off(intel_dp);
> > +}
> > +
> > +static void g4x_disable_dp(struct intel_encoder *encoder,
> > +			   const struct intel_crtc_state *old_crtc_state,
> > +			   const struct drm_connector_state *old_conn_state)
> > +{
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > +
> > +	intel_disable_dp(encoder, old_crtc_state, old_conn_state);
> >  
> >  	/* disable the port before the pipe on g4x */
> > -	if (INTEL_GEN(dev_priv) < 5)
> > -		intel_dp_link_down(intel_dp);
> > +	intel_dp_link_down(intel_dp);
> > +}
> > +
> > +static void ilk_disable_dp(struct intel_encoder *encoder,
> > +			   const struct intel_crtc_state *old_crtc_state,
> > +			   const struct drm_connector_state *old_conn_state)
> > +{
> > +	intel_disable_dp(encoder, old_crtc_state, old_conn_state);
> > +}
> > +
> > +static void vlv_disable_dp(struct intel_encoder *encoder,
> > +			   const struct intel_crtc_state *old_crtc_state,
> > +			   const struct drm_connector_state *old_conn_state)
> > +{
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > +
> > +	intel_psr_disable(intel_dp, old_crtc_state);
> 
> oh! not it makes sense...
> please move the that removal of !HAS_DDI from the first patch to this patch
> and feel free to ad my rv-b on both patches.

Updated patches pushed pushed to dinq. Thanks for the review.

> 
> > +
> > +	intel_disable_dp(encoder, old_crtc_state, old_conn_state);
> >  }
> >  
> >  static void ilk_post_disable_dp(struct intel_encoder *encoder,
> > @@ -6144,7 +6167,6 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
> >  		goto err_encoder_init;
> >  
> >  	intel_encoder->compute_config = intel_dp_compute_config;
> > -	intel_encoder->disable = intel_disable_dp;
> >  	intel_encoder->get_hw_state = intel_dp_get_hw_state;
> >  	intel_encoder->get_config = intel_dp_get_config;
> >  	intel_encoder->suspend = intel_dp_encoder_suspend;
> > @@ -6152,18 +6174,24 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
> >  		intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
> >  		intel_encoder->pre_enable = chv_pre_enable_dp;
> >  		intel_encoder->enable = vlv_enable_dp;
> > +		intel_encoder->disable = vlv_disable_dp;
> >  		intel_encoder->post_disable = chv_post_disable_dp;
> >  		intel_encoder->post_pll_disable = chv_dp_post_pll_disable;
> >  	} else if (IS_VALLEYVIEW(dev_priv)) {
> >  		intel_encoder->pre_pll_enable = vlv_dp_pre_pll_enable;
> >  		intel_encoder->pre_enable = vlv_pre_enable_dp;
> >  		intel_encoder->enable = vlv_enable_dp;
> > +		intel_encoder->disable = vlv_disable_dp;
> >  		intel_encoder->post_disable = vlv_post_disable_dp;
> > +	} else if (INTEL_GEN(dev_priv) >= 5) {
> > +		intel_encoder->pre_enable = g4x_pre_enable_dp;
> > +		intel_encoder->enable = g4x_enable_dp;
> > +		intel_encoder->disable = ilk_disable_dp;
> > +		intel_encoder->post_disable = ilk_post_disable_dp;
> >  	} else {
> >  		intel_encoder->pre_enable = g4x_pre_enable_dp;
> >  		intel_encoder->enable = g4x_enable_dp;
> > -		if (INTEL_GEN(dev_priv) >= 5)
> > -			intel_encoder->post_disable = ilk_post_disable_dp;
> > +		intel_encoder->disable = g4x_disable_dp;
> >  	}
> >  
> >  	intel_dig_port->port = port;
> > -- 
> > 2.13.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [v2,1/2] drm/i915: Drop useless HAS_PSR() check (rev3)
  2017-09-18 17:31 [PATCH 1/2] drm/i915: Drop useless HAS_PSR() check Ville Syrjala
                   ` (4 preceding siblings ...)
  2017-09-20 16:04 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915: Drop useless HAS_PSR() check (rev3) Patchwork
@ 2017-09-20 17:19 ` Patchwork
  5 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2017-09-20 17:19 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] drm/i915: Drop useless HAS_PSR() check (rev3)
URL   : https://patchwork.freedesktop.org/series/30543/
State : success

== Summary ==

Test perf:
        Subgroup polling:
                fail       -> PASS       (shard-hsw) fdo#102252

fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-hsw        total:2317 pass:1246 dwarn:2   dfail:0   fail:12  skip:1057 time:9551s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5768/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-09-20 17:19 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-18 17:31 [PATCH 1/2] drm/i915: Drop useless HAS_PSR() check Ville Syrjala
2017-09-18 17:31 ` [PATCH 2/2] drm/i915: Reorganize .disable hooks for pre-DDI DP Ville Syrjala
2017-09-18 20:45   ` Rodrigo Vivi
2017-09-20 17:00     ` Ville Syrjälä
2017-09-20 15:12   ` [PATCH v2 " Ville Syrjala
2017-09-18 17:55 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Drop useless HAS_PSR() check Patchwork
2017-09-18 20:35 ` [PATCH 1/2] " Rodrigo Vivi
2017-09-20 15:12 ` [PATCH v2 " Ville Syrjala
2017-09-20 16:04 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915: Drop useless HAS_PSR() check (rev3) Patchwork
2017-09-20 17:19 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.