2016-03-29 20:46:36

by Lyude Paul

[permalink] [raw]
Subject: [PATCH] drm/i915/vlv: Enable/disable VGA hotplugging properly

On Valleyview, VGA hotplugging is controlled through a seperate register
than everything else, VLV_ADPA, which must be explicitly set.

While VGA hotplugging worked(ish) before, it looks like that was mainly
because we'd unintentionally enable it in
valleyview_crt_detect_hotplug() when we did a force trigger. This
doesn't work reliably enough because whenever the display powerwell on
vlv gets disabled, the values set in VLV_ADPA get cleared and
consequently VGA hotplugging gets disabled. This causes bugs such as one
we found on an Intel NUC, where doing the following sequence of
hotplugs:

- Disconnect all monitors
- Connect VGA
- Disconnect VGA
- Connect HDMI

Would result in hotplugging getting disabled, due to the display
powerwells getting toggled in the process of connecting HDMI.

CC: [email protected]
Signed-off-by: Lyude <[email protected]>
---
drivers/gpu/drm/i915/i915_irq.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5aa4239..60592a4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3611,6 +3611,7 @@ static void valleyview_display_irqs_install(struct drm_i915_private *dev_priv)
{
u32 pipestat_mask;
u32 iir_mask;
+ u32 adpa_reg;
enum pipe pipe;

pipestat_mask = PIPESTAT_INT_STATUS_MASK |
@@ -3627,6 +3628,12 @@ static void valleyview_display_irqs_install(struct drm_i915_private *dev_priv)
for_each_pipe(dev_priv, pipe)
i915_enable_pipestat(dev_priv, pipe, pipestat_mask);

+ if (IS_VALLEYVIEW(dev_priv)) {
+ adpa_reg = I915_READ(VLV_ADPA);
+ adpa_reg |= ADPA_CRT_HOTPLUG_ENABLE;
+ I915_WRITE(VLV_ADPA, adpa_reg);
+ }
+
iir_mask = I915_DISPLAY_PORT_INTERRUPT |
I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
@@ -3645,8 +3652,15 @@ static void valleyview_display_irqs_uninstall(struct drm_i915_private *dev_priv)
{
u32 pipestat_mask;
u32 iir_mask;
+ u32 adpa_reg;
enum pipe pipe;

+ if (IS_VALLEYVIEW(dev_priv)) {
+ adpa_reg = I915_READ(VLV_ADPA);
+ adpa_reg &= ~ADPA_CRT_HOTPLUG_ENABLE;
+ I915_WRITE(VLV_ADPA, adpa_reg);
+ }
+
iir_mask = I915_DISPLAY_PORT_INTERRUPT |
I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
--
2.5.5


2016-04-15 19:40:31

by Lyude Paul

[permalink] [raw]
Subject: [PATCH v2 1/2] drm/i915/vlv: Make intel_crt_reset() per-encoder

This lets call intel_crt_reset() in contexts where IRQs are disabled and
as such, can't hold the locks required to work with the connectors.

CC: [email protected]
Signed-off-by: Lyude <[email protected]>
---
drivers/gpu/drm/i915/intel_crt.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index a2a31fd..220ca91 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -707,11 +707,11 @@ static int intel_crt_set_property(struct drm_connector *connector,
return 0;
}

-static void intel_crt_reset(struct drm_connector *connector)
+static void intel_crt_reset(struct drm_encoder *encoder)
{
- struct drm_device *dev = connector->dev;
+ struct drm_device *dev = encoder->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_crt *crt = intel_attached_crt(connector);
+ struct intel_crt *crt = intel_encoder_to_crt(to_intel_encoder(encoder));

if (INTEL_INFO(dev)->gen >= 5) {
u32 adpa;
@@ -733,7 +733,6 @@ static void intel_crt_reset(struct drm_connector *connector)
*/

static const struct drm_connector_funcs intel_crt_connector_funcs = {
- .reset = intel_crt_reset,
.dpms = drm_atomic_helper_connector_dpms,
.detect = intel_crt_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
@@ -751,6 +750,7 @@ static const struct drm_connector_helper_funcs intel_crt_connector_helper_funcs
};

static const struct drm_encoder_funcs intel_crt_enc_funcs = {
+ .reset = intel_crt_reset,
.destroy = intel_encoder_destroy,
};

@@ -896,5 +896,5 @@ void intel_crt_init(struct drm_device *dev)
dev_priv->fdi_rx_config = I915_READ(FDI_RX_CTL(PIPE_A)) & fdi_config;
}

- intel_crt_reset(connector);
+ intel_crt_reset(&crt->base.base);
}
--
2.5.5

2016-04-15 19:40:53

by Lyude Paul

[permalink] [raw]
Subject: [PATCH v2 2/2] drm/i915/vlv: Reset the ADPA in vlv_display_power_well_init()

While VGA hotplugging worked(ish) before, it looks like that was mainly
because we'd unintentionally enable it in
valleyview_crt_detect_hotplug() when we did a force trigger. This
doesn't work reliably enough because whenever the display powerwell on
vlv gets disabled, the values set in VLV_ADPA get cleared and
consequently VGA hotplugging gets disabled. This causes bugs such as one
we found on an Intel NUC, where doing the following sequence of
hotplugs:

- Disconnect all monitors
- Connect VGA
- Disconnect VGA
- Connect HDMI

Would result in VGA hotplugging becoming disabled, due to the powerwells
getting toggled in the process of connecting HDMI.

Changes since v1:
- Instead of handling the register writes ourself, we just reuse
intel_crt_detect()
- Instead of resetting the ADPA during display IRQ installation, we now
reset them in vlv_display_power_well_init()

CC: [email protected]
Signed-off-by: Lyude <[email protected]>
---
drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 80e8bd4..c7d195f 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -902,6 +902,7 @@ static bool vlv_power_well_enabled(struct drm_i915_private *dev_priv,

static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
{
+ struct drm_encoder *encoder, *vga_encoder = NULL;
enum pipe pipe;

/*
@@ -935,6 +936,17 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)

intel_hpd_init(dev_priv);

+ /* Re-enable the ADPA, if we have one */
+ drm_for_each_encoder(encoder, dev_priv->dev) {
+ if (encoder->encoder_type == DRM_MODE_ENCODER_DAC) {
+ vga_encoder = encoder;
+ break;
+ }
+ }
+
+ if (vga_encoder && vga_encoder->funcs->reset)
+ vga_encoder->funcs->reset(vga_encoder);
+
i915_redisable_vga_power_on(dev_priv->dev);
}

--
2.5.5