| Message ID | 1461253488-18043-1-git-send-email-cpaul@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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);
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(-)