* [PATCH] drm/i915/vlv: Enable polling when we shut off all power domains
@ 2016-04-21 15:44 ` Lyude
0 siblings, 0 replies; 6+ messages in thread
From: Lyude @ 2016-04-21 15:44 UTC (permalink / raw)
To: intel-gfx
Cc: Lyude, stable, Daniel Vetter, Jani Nikula, David Airlie,
open list:INTEL DRM DRIVERS (excluding Poulsbo, Moorestow...),
linux-kernel@vger.kernel.org (open list)
Unfortunately HPD isn't functional once we shut off all of the power
domains. Unfortunately we can end up shutting off all of the power
domains in any situation where we don't have any monitors connected,
essentially breaking hpd for the user unless they reboot with one of
their monitors connected.
In addition, enabling polling has to be done in it's own seperate
worker. The reason for this is that vlv_display_power_well_init/deinit()
will get called during the DRM polling process and try to grab the locks
required for turning on polling and cause a deadlock.
This breaks runtime PM due to the constant wakeups, so this is more of a
temporary workaround then a solution.
Signed-off-by: Lyude <cpaul@redhat.com>
Cc: stable@vger.kernel.org
---
drivers/gpu/drm/i915/intel_display.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 551541b303..f644814 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14531,7 +14531,22 @@ static void intel_setup_outputs(struct drm_device *dev)
intel_dp_is_edp(dev, PORT_C))
intel_dp_init(dev, VLV_DP_C, PORT_C);
- if (IS_CHERRYVIEW(dev)) {
+ if (IS_VALLEYVIEW(dev)) {
+ struct intel_connector *connector;
+
+ /*
+ * On vlv, turning off all of the power domains results
+ * in a loss of hpd, enable polling on all of the
+ * connectors so that drm polls them when this happens
+ */
+ drm_modeset_lock(&dev->mode_config.connection_mutex,
+ NULL);
+ for_each_intel_connector(dev, connector) {
+ connector->polled = DRM_CONNECTOR_POLL_CONNECT |
+ DRM_CONNECTOR_POLL_DISCONNECT;
+ }
+ drm_modeset_unlock(&dev->mode_config.connection_mutex);
+ } else if (IS_CHERRYVIEW(dev)) {
/* eDP not supported on port D, so don't check VBT */
if (I915_READ(CHV_HDMID) & SDVO_DETECTED)
intel_hdmi_init(dev, CHV_HDMID, PORT_D);
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] drm/i915/vlv: Enable polling when we shut off all power domains
@ 2016-04-21 15:44 ` Lyude
0 siblings, 0 replies; 6+ messages in thread
From: Lyude @ 2016-04-21 15:44 UTC (permalink / raw)
To: intel-gfx
Cc: stable, open list:INTEL DRM DRIVERS excluding Poulsbo,
Moorestow...,
linux-kernel@vger.kernel.org open list, Daniel Vetter, Lyude
Unfortunately HPD isn't functional once we shut off all of the power
domains. Unfortunately we can end up shutting off all of the power
domains in any situation where we don't have any monitors connected,
essentially breaking hpd for the user unless they reboot with one of
their monitors connected.
In addition, enabling polling has to be done in it's own seperate
worker. The reason for this is that vlv_display_power_well_init/deinit()
will get called during the DRM polling process and try to grab the locks
required for turning on polling and cause a deadlock.
This breaks runtime PM due to the constant wakeups, so this is more of a
temporary workaround then a solution.
Signed-off-by: Lyude <cpaul@redhat.com>
Cc: stable@vger.kernel.org
---
drivers/gpu/drm/i915/intel_display.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 551541b303..f644814 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14531,7 +14531,22 @@ static void intel_setup_outputs(struct drm_device *dev)
intel_dp_is_edp(dev, PORT_C))
intel_dp_init(dev, VLV_DP_C, PORT_C);
- if (IS_CHERRYVIEW(dev)) {
+ if (IS_VALLEYVIEW(dev)) {
+ struct intel_connector *connector;
+
+ /*
+ * On vlv, turning off all of the power domains results
+ * in a loss of hpd, enable polling on all of the
+ * connectors so that drm polls them when this happens
+ */
+ drm_modeset_lock(&dev->mode_config.connection_mutex,
+ NULL);
+ for_each_intel_connector(dev, connector) {
+ connector->polled = DRM_CONNECTOR_POLL_CONNECT |
+ DRM_CONNECTOR_POLL_DISCONNECT;
+ }
+ drm_modeset_unlock(&dev->mode_config.connection_mutex);
+ } else if (IS_CHERRYVIEW(dev)) {
/* eDP not supported on port D, so don't check VBT */
if (I915_READ(CHV_HDMID) & SDVO_DETECTED)
intel_hdmi_init(dev, CHV_HDMID, PORT_D);
--
2.5.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915/vlv: Enable polling when we shut off all power domains
2016-04-21 15:44 ` Lyude
(?)
@ 2016-04-22 6:57 ` Patchwork
-1 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-04-22 6:57 UTC (permalink / raw)
To: cpaul; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/vlv: Enable polling when we shut off all power domains
URL : https://patchwork.freedesktop.org/series/6095/
State : warning
== Summary ==
Series 6095v1 drm/i915/vlv: Enable polling when we shut off all power domains
http://patchwork.freedesktop.org/api/1.0/series/6095/revisions/1/mbox/
Test gem_busy:
Subgroup basic-blt:
skip -> PASS (bsw-nuc-2)
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
fail -> PASS (ilk-hp8440p) UNSTABLE
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass -> SKIP (ivb-t430s)
bdw-nuci7 total:193 pass:181 dwarn:0 dfail:0 fail:0 skip:12
bdw-ultra total:193 pass:170 dwarn:0 dfail:0 fail:0 skip:23
bsw-nuc-2 total:192 pass:153 dwarn:0 dfail:0 fail:0 skip:39
byt-nuc total:192 pass:154 dwarn:0 dfail:0 fail:0 skip:38
hsw-brixbox total:193 pass:169 dwarn:0 dfail:0 fail:0 skip:24
ilk-hp8440p total:193 pass:136 dwarn:0 dfail:0 fail:0 skip:57
ivb-t430s total:193 pass:164 dwarn:0 dfail:0 fail:0 skip:29
skl-i7k-2 total:193 pass:168 dwarn:0 dfail:0 fail:0 skip:25
skl-nuci5 total:193 pass:182 dwarn:0 dfail:0 fail:0 skip:11
snb-dellxps total:193 pass:155 dwarn:0 dfail:0 fail:0 skip:38
snb-x220t total:193 pass:155 dwarn:0 dfail:0 fail:1 skip:37
Results at /archive/results/CI_IGT_test/Patchwork_1982/
d5b5101bd09a7b7e48b1cd78fe8f8a40b21d4deb drm-intel-nightly: 2016y-04m-21d-16h-37m-06s UTC integration manifest
2642d5a drm/i915/vlv: Enable polling when we shut off all power domains
_______________________________________________
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: success for drm/i915/vlv: Enable polling when we shut off all power domains
2016-04-21 15:44 ` Lyude
(?)
(?)
@ 2016-04-23 17:36 ` Patchwork
-1 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-04-23 17:36 UTC (permalink / raw)
To: cpaul; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/vlv: Enable polling when we shut off all power domains
URL : https://patchwork.freedesktop.org/series/6095/
State : success
== Summary ==
Series 6095v1 drm/i915/vlv: Enable polling when we shut off all power domains
http://patchwork.freedesktop.org/api/1.0/series/6095/revisions/1/mbox/
bdw-ultra total:193 pass:170 dwarn:0 dfail:0 fail:0 skip:23
bsw-nuc-2 total:192 pass:153 dwarn:0 dfail:0 fail:0 skip:39
byt-nuc total:192 pass:154 dwarn:0 dfail:0 fail:0 skip:38
ilk-hp8440p total:193 pass:136 dwarn:0 dfail:0 fail:0 skip:57
ivb-t430s total:193 pass:165 dwarn:0 dfail:0 fail:0 skip:28
skl-i7k-2 total:193 pass:168 dwarn:0 dfail:0 fail:0 skip:25
skl-nuci5 total:193 pass:182 dwarn:0 dfail:0 fail:0 skip:11
snb-dellxps total:193 pass:155 dwarn:0 dfail:0 fail:0 skip:38
bdw-nuci7 failed to collect. IGT log at Patchwork_2024/bdw-nuci7/igt.log
Results at /archive/results/CI_IGT_test/Patchwork_2024/
340c485ad98d0ec0369a3b18d4a09938f3f5537d drm-intel-nightly: 2016y-04m-22d-17h-32m-25s UTC integration manifest
d217255 drm/i915/vlv: Enable polling when we shut off all power domains
_______________________________________________
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: [Intel-gfx] [PATCH] drm/i915/vlv: Enable polling when we shut off all power domains
2016-04-21 15:44 ` Lyude
@ 2016-06-16 22:05 ` Daniel Vetter
-1 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2016-06-16 22:05 UTC (permalink / raw)
To: Lyude
Cc: intel-gfx, David Airlie, stable,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
linux-kernel@vger.kernel.org open list, Daniel Vetter
On Thu, Apr 21, 2016 at 11:44:48AM -0400, Lyude wrote:
> Unfortunately HPD isn't functional once we shut off all of the power
> domains. Unfortunately we can end up shutting off all of the power
> domains in any situation where we don't have any monitors connected,
> essentially breaking hpd for the user unless they reboot with one of
> their monitors connected.
>
> In addition, enabling polling has to be done in it's own seperate
> worker. The reason for this is that vlv_display_power_well_init/deinit()
> will get called during the DRM polling process and try to grab the locks
> required for turning on polling and cause a deadlock.
>
> This breaks runtime PM due to the constant wakeups, so this is more of a
> temporary workaround then a solution.
>
> Signed-off-by: Lyude <cpaul@redhat.com>
> Cc: stable@vger.kernel.org
> ---
> drivers/gpu/drm/i915/intel_display.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 551541b303..f644814 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14531,7 +14531,22 @@ static void intel_setup_outputs(struct drm_device *dev)
> intel_dp_is_edp(dev, PORT_C))
> intel_dp_init(dev, VLV_DP_C, PORT_C);
>
> - if (IS_CHERRYVIEW(dev)) {
> + if (IS_VALLEYVIEW(dev)) {
> + struct intel_connector *connector;
> +
> + /*
> + * On vlv, turning off all of the power domains results
> + * in a loss of hpd, enable polling on all of the
> + * connectors so that drm polls them when this happens
> + */
> + drm_modeset_lock(&dev->mode_config.connection_mutex,
> + NULL);
> + for_each_intel_connector(dev, connector) {
> + connector->polled = DRM_CONNECTOR_POLL_CONNECT |
> + DRM_CONNECTOR_POLL_DISCONNECT;
> + }
> + drm_modeset_unlock(&dev->mode_config.connection_mutex);
> + } else if (IS_CHERRYVIEW(dev)) {
Ok, so this is just review on this patch. Imo what we need to avoid
polling all the time is a intel_hpd_polling() function or similar, which
sets drm_connector->polled to enable polling. We need to insert this in
the right places, i.e. in intel_runtime_suspend for !VLV && !CHV and in
vlv_display_power_well_disable for VLV && CHV. In short exactly symmetric
to how we call intel_hpd_init() on the enable/resume side.
intel_hpd_init() already disables polling again on its own (as long as we
only touch drm_connector->polled and not intel_connector->polled), which
means polling will only be enabled for exactly as much time as we need.
The other bit we need is to re-enabled the poll worker from that new
intel_hpd_polling() functions. We can't just call
drm_kms_helper_poll_enable because that requires a mutex. Instead the
simplest trick is to just schedule the poll work directly:
schedule_delayed_work(&dev->mode_config.output_poll_work, DRM_OUTPUT_POLL_PERIOD);
The simplest solution tbh would be to simplify drm_kms_helper_poll_enable
to drop the locking and just do this directly. The worker will
self-disable if it's not needed.
This would at least fix the "always polls" problem your patch has. We'd
still have the problem with irq storms happening due to the power well
switching all the time. But sounds like Ville has at least a partial
solution for that.
-Daniel
> /* eDP not supported on port D, so don't check VBT */
> if (I915_READ(CHV_HDMID) & SDVO_DETECTED)
> intel_hdmi_init(dev, CHV_HDMID, PORT_D);
> --
> 2.5.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/vlv: Enable polling when we shut off all power domains
@ 2016-06-16 22:05 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2016-06-16 22:05 UTC (permalink / raw)
To: Lyude
Cc: Daniel Vetter, intel-gfx,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
linux-kernel@vger.kernel.org open list, stable
On Thu, Apr 21, 2016 at 11:44:48AM -0400, Lyude wrote:
> Unfortunately HPD isn't functional once we shut off all of the power
> domains. Unfortunately we can end up shutting off all of the power
> domains in any situation where we don't have any monitors connected,
> essentially breaking hpd for the user unless they reboot with one of
> their monitors connected.
>
> In addition, enabling polling has to be done in it's own seperate
> worker. The reason for this is that vlv_display_power_well_init/deinit()
> will get called during the DRM polling process and try to grab the locks
> required for turning on polling and cause a deadlock.
>
> This breaks runtime PM due to the constant wakeups, so this is more of a
> temporary workaround then a solution.
>
> Signed-off-by: Lyude <cpaul@redhat.com>
> Cc: stable@vger.kernel.org
> ---
> drivers/gpu/drm/i915/intel_display.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 551541b303..f644814 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14531,7 +14531,22 @@ static void intel_setup_outputs(struct drm_device *dev)
> intel_dp_is_edp(dev, PORT_C))
> intel_dp_init(dev, VLV_DP_C, PORT_C);
>
> - if (IS_CHERRYVIEW(dev)) {
> + if (IS_VALLEYVIEW(dev)) {
> + struct intel_connector *connector;
> +
> + /*
> + * On vlv, turning off all of the power domains results
> + * in a loss of hpd, enable polling on all of the
> + * connectors so that drm polls them when this happens
> + */
> + drm_modeset_lock(&dev->mode_config.connection_mutex,
> + NULL);
> + for_each_intel_connector(dev, connector) {
> + connector->polled = DRM_CONNECTOR_POLL_CONNECT |
> + DRM_CONNECTOR_POLL_DISCONNECT;
> + }
> + drm_modeset_unlock(&dev->mode_config.connection_mutex);
> + } else if (IS_CHERRYVIEW(dev)) {
Ok, so this is just review on this patch. Imo what we need to avoid
polling all the time is a intel_hpd_polling() function or similar, which
sets drm_connector->polled to enable polling. We need to insert this in
the right places, i.e. in intel_runtime_suspend for !VLV && !CHV and in
vlv_display_power_well_disable for VLV && CHV. In short exactly symmetric
to how we call intel_hpd_init() on the enable/resume side.
intel_hpd_init() already disables polling again on its own (as long as we
only touch drm_connector->polled and not intel_connector->polled), which
means polling will only be enabled for exactly as much time as we need.
The other bit we need is to re-enabled the poll worker from that new
intel_hpd_polling() functions. We can't just call
drm_kms_helper_poll_enable because that requires a mutex. Instead the
simplest trick is to just schedule the poll work directly:
schedule_delayed_work(&dev->mode_config.output_poll_work, DRM_OUTPUT_POLL_PERIOD);
The simplest solution tbh would be to simplify drm_kms_helper_poll_enable
to drop the locking and just do this directly. The worker will
self-disable if it's not needed.
This would at least fix the "always polls" problem your patch has. We'd
still have the problem with irq storms happening due to the power well
switching all the time. But sounds like Ville has at least a partial
solution for that.
-Daniel
> /* eDP not supported on port D, so don't check VBT */
> if (I915_READ(CHV_HDMID) & SDVO_DETECTED)
> intel_hdmi_init(dev, CHV_HDMID, PORT_D);
> --
> 2.5.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-16 22:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-21 15:44 [PATCH] drm/i915/vlv: Enable polling when we shut off all power domains Lyude
2016-04-21 15:44 ` Lyude
2016-04-22 6:57 ` ✗ Fi.CI.BAT: warning for " Patchwork
2016-04-23 17:36 ` ✓ Fi.CI.BAT: success " Patchwork
2016-06-16 22:05 ` [Intel-gfx] [PATCH] " Daniel Vetter
2016-06-16 22:05 ` Daniel Vetter
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.