* [PATCH v2 2/3] drm/i915: Release DDI power well references in MST ports
2018-11-05 20:25 [PATCH v2 1/3] drm/i915: Reuse the aux_domain cached José Roberto de Souza
@ 2018-11-05 20:25 ` José Roberto de Souza
2018-11-05 20:25 ` [PATCH v2 3/3] drm/i915/mst: Drop pre_pll_enable null check José Roberto de Souza
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: José Roberto de Souza @ 2018-11-05 20:25 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/3] drm/i915/mst: Drop pre_pll_enable null check
2018-11-05 20:25 [PATCH v2 1/3] drm/i915: Reuse the aux_domain cached José Roberto de Souza
2018-11-05 20:25 ` [PATCH v2 2/3] drm/i915: Release DDI power well references in MST ports José Roberto de Souza
@ 2018-11-05 20:25 ` José Roberto de Souza
2018-11-05 20:36 ` Rodrigo Vivi
2018-11-06 8:34 ` ✗ Fi.CI.BAT: failure for series starting with [v2,1/3] drm/i915: Reuse the aux_domain cached Patchwork
2018-11-06 11:32 ` [PATCH v2 1/3] " Imre Deak
3 siblings, 1 reply; 6+ messages in thread
From: José Roberto de Souza @ 2018-11-05 20:25 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] drm/i915/mst: Drop pre_pll_enable null check
2018-11-05 20:25 ` [PATCH v2 3/3] drm/i915/mst: Drop pre_pll_enable null check José Roberto de Souza
@ 2018-11-05 20:36 ` Rodrigo Vivi
0 siblings, 0 replies; 6+ messages in thread
From: Rodrigo Vivi @ 2018-11-05 20:36 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [v2,1/3] drm/i915: Reuse the aux_domain cached
2018-11-05 20:25 [PATCH v2 1/3] drm/i915: Reuse the aux_domain cached José Roberto de Souza
2018-11-05 20:25 ` [PATCH v2 2/3] drm/i915: Release DDI power well references in MST ports José Roberto de Souza
2018-11-05 20:25 ` [PATCH v2 3/3] drm/i915/mst: Drop pre_pll_enable null check José Roberto de Souza
@ 2018-11-06 8:34 ` Patchwork
2018-11-06 11:32 ` [PATCH v2 1/3] " Imre Deak
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-11-06 8:34 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/3] drm/i915: Reuse the aux_domain cached
2018-11-05 20:25 [PATCH v2 1/3] drm/i915: Reuse the aux_domain cached José Roberto de Souza
` (2 preceding siblings ...)
2018-11-06 8:34 ` ✗ Fi.CI.BAT: failure for series starting with [v2,1/3] drm/i915: Reuse the aux_domain cached Patchwork
@ 2018-11-06 11:32 ` Imre Deak
3 siblings, 0 replies; 6+ messages in thread
From: Imre Deak @ 2018-11-06 11:32 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 6+ messages in thread