[PATCH v2 1/3] drm/i915: Reuse the aux_domain cached

[PATCH v2 1/3] drm/i915: Reuse the aux_domain cached

[José Roberto de Souza]

intel_dp_detect() caches the aux_domain in the beginning of the
function as it is used twice, so lets also use it as the aux_domain
don't change in runtime by jumping to the end of function when
retrain the link fails.

v2: jumping to the end of the function instead of just reuse
aux_domain

Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5258c9d654f4..c7814d53f70f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5165,13 +5165,9 @@ intel_dp_detect(struct drm_connector *connector,
 	 * with an IRQ_HPD, so force a link status check.
 	 */
 	if (!intel_dp_is_edp(intel_dp)) {
-		int ret;
-
-		ret = intel_dp_retrain_link(encoder, ctx);
-		if (ret) {
-			intel_display_power_put(dev_priv,
-						intel_aux_power_domain(dig_port));
-			return ret;
+		if (intel_dp_retrain_link(encoder, ctx)) {
+			status = connector_status_disconnected;
+			goto out;
 		}
 	}
 
-- 
2.19.1

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

[PATCH v2 2/3] drm/i915: Release DDI power well references in MST ports

[José Roberto de Souza]

MST ports did not had the post_pll_disable() hook causing the
references get in pre_pll_enable() never being released causing
DDI and AUX CH being enabled all the times.

v2: renamed intel_mst_post_pll_disable_dp() parameters

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 8b71d64ebd9d..d57dc7900eb1 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -215,6 +215,20 @@ static void intel_mst_pre_pll_enable_dp(struct intel_encoder *encoder,
 						    pipe_config, NULL);
 }
 
+static void intel_mst_post_pll_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_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
+	struct intel_digital_port *intel_dig_port = intel_mst->primary;
+	struct intel_dp *intel_dp = &intel_dig_port->dp;
+
+	if (intel_dp->active_mst_links == 0)
+		intel_dig_port->base.post_pll_disable(&intel_dig_port->base,
+						      old_crtc_state,
+						      old_conn_state);
+}
+
 static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
 				    const struct intel_crtc_state *pipe_config,
 				    const struct drm_connector_state *conn_state)
@@ -549,6 +563,7 @@ intel_dp_create_fake_mst_encoder(struct intel_digital_port *intel_dig_port, enum
 	intel_encoder->disable = intel_mst_disable_dp;
 	intel_encoder->post_disable = intel_mst_post_disable_dp;
 	intel_encoder->pre_pll_enable = intel_mst_pre_pll_enable_dp;
+	intel_encoder->post_pll_disable = intel_mst_post_pll_disable_dp;
 	intel_encoder->pre_enable = intel_mst_pre_enable_dp;
 	intel_encoder->enable = intel_mst_enable_dp;
 	intel_encoder->get_hw_state = intel_dp_mst_enc_get_hw_state;
-- 
2.19.1

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

[PATCH v2 3/3] drm/i915/mst: Drop pre_pll_enable null check

[José Roberto de Souza]

MST is only supported in DDI ports that have this hook, so the null
check can be dropped.

Suggested-by: Imre Deak <imre.deak@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index d57dc7900eb1..34ac1a7c2202 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -209,8 +209,7 @@ static void intel_mst_pre_pll_enable_dp(struct intel_encoder *encoder,
 	struct intel_digital_port *intel_dig_port = intel_mst->primary;
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
 
-	if (intel_dp->active_mst_links == 0 &&
-	    intel_dig_port->base.pre_pll_enable)
+	if (intel_dp->active_mst_links == 0)
 		intel_dig_port->base.pre_pll_enable(&intel_dig_port->base,
 						    pipe_config, NULL);
 }
-- 
2.19.1

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

Re: [PATCH v2 3/3] drm/i915/mst: Drop pre_pll_enable null check

[Rodrigo Vivi]

On Mon, Nov 05, 2018 at 12:25:52PM -0800, José Roberto de Souza wrote:
> MST is only supported in DDI ports that have this hook, so the null
> check can be dropped.
> 
> Suggested-by: Imre Deak <imre.deak@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

I feel that the extra check doesn't hurt since it is an
"optional" hook, but I don't believe it would be possible
to have any mst without this pre-enalble hook anyway so
we can remove...

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp_mst.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index d57dc7900eb1..34ac1a7c2202 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -209,8 +209,7 @@ static void intel_mst_pre_pll_enable_dp(struct intel_encoder *encoder,
>  	struct intel_digital_port *intel_dig_port = intel_mst->primary;
>  	struct intel_dp *intel_dp = &intel_dig_port->dp;
>  
> -	if (intel_dp->active_mst_links == 0 &&
> -	    intel_dig_port->base.pre_pll_enable)
> +	if (intel_dp->active_mst_links == 0)
>  		intel_dig_port->base.pre_pll_enable(&intel_dig_port->base,
>  						    pipe_config, NULL);
>  }
> -- 
> 2.19.1
> 
> _______________________________________________
> 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

✗ Fi.CI.BAT: failure for series starting with [v2,1/3] drm/i915: Reuse the aux_domain cached

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Reuse the aux_domain cached
URL   : https://patchwork.freedesktop.org/series/52048/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_5090 -> Patchwork_10729 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_10729 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10729, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/52048/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10729:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_requests:
      fi-icl-u:           NOTRUN -> INCOMPLETE

    igt@gem_exec_suspend@basic-s4-devices:
      fi-skl-6700k2:      PASS -> DMESG-WARN +6
      fi-skl-guc:         PASS -> DMESG-WARN +1

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c-frame-sequence:
      fi-skl-6770hq:      PASS -> DMESG-WARN +3

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      fi-cfl-8109u:       PASS -> DMESG-WARN +8
      fi-kbl-7567u:       PASS -> DMESG-WARN +1
      fi-hsw-4770r:       PASS -> DMESG-WARN

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-ilk-650:         PASS -> DMESG-WARN
      fi-apl-guc:         PASS -> DMESG-WARN +1

    igt@pm_rpm@module-reload:
      fi-kbl-7500u:       PASS -> DMESG-FAIL
      fi-bxt-j4205:       PASS -> DMESG-WARN +2
      fi-kbl-7567u:       PASS -> DMESG-FAIL

    igt@prime_vgem@basic-fence-flip:
      fi-kbl-7500u:       PASS -> DMESG-WARN +4

    
    ==== Warnings ====

    igt@kms_flip@basic-flip-vs-modeset:
      fi-kbl-7567u:       PASS -> SKIP

    igt@kms_flip@basic-plain-flip:
      fi-skl-6770hq:      PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_10729 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-snb-2520m:       NOTRUN -> DMESG-FAIL (fdo#103713)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       NOTRUN -> INCOMPLETE (fdo#103713)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      fi-snb-2520m:       DMESG-FAIL (fdo#103713) -> PASS

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

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


== Participating hosts (50 -> 47) ==

  Additional (2): fi-icl-u fi-pnv-d510 
  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_5090 -> Patchwork_10729

  CI_DRM_5090: 756a0fd616c3ea0486f5c239f7801f71303ff389 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4709: 15dff9353621d0746b80fae534c20621e03a9f01 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10729: 91747bb375d854e6287f585a7d963e3a8c28630c @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

91747bb375d8 drm/i915/mst: Drop pre_pll_enable null check
f4db0c240f04 drm/i915: Release DDI power well references in MST ports
b34a30d59954 drm/i915: Reuse the aux_domain cached

== Logs ==

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

Re: [PATCH v2 1/3] drm/i915: Reuse the aux_domain cached

[Imre Deak]

On Mon, Nov 05, 2018 at 12:25:50PM -0800, José Roberto de Souza wrote:
> intel_dp_detect() caches the aux_domain in the beginning of the
> function as it is used twice, so lets also use it as the aux_domain
> don't change in runtime by jumping to the end of function when
> retrain the link fails.
> 
> v2: jumping to the end of the function instead of just reuse
> aux_domain
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 5258c9d654f4..c7814d53f70f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5165,13 +5165,9 @@ intel_dp_detect(struct drm_connector *connector,
>  	 * with an IRQ_HPD, so force a link status check.
>  	 */
>  	if (!intel_dp_is_edp(intel_dp)) {
> -		int ret;
> -
> -		ret = intel_dp_retrain_link(encoder, ctx);
> -		if (ret) {
> -			intel_display_power_put(dev_priv,
> -						intel_aux_power_domain(dig_port));
> -			return ret;
> +		if (intel_dp_retrain_link(encoder, ctx)) {
> +			status = connector_status_disconnected;
> +			goto out;

The only way intel_dp_retrain_link() can fail is -EDEADLK, which we
should return then. Not doing that (and calling intel_dp_unset_edid() as
a result) would be anyway an independent change not belonging to this
patch.

Luckily caught by CONFIG_DEBUG_WW_MUTEX_SLOWPATH=y btw.

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