* [PATCH] drm/i915: Fixup hpd irq register setup ordering
@ 2012-12-09 19:14 Daniel Vetter
2012-12-09 19:17 ` Daniel Vetter
2012-12-09 19:42 ` Daniel Vetter
0 siblings, 2 replies; 10+ messages in thread
From: Daniel Vetter @ 2012-12-09 19:14 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
For GMCH platforms we set up the hpd irq registers in the irq
postinstall hook. But since we only enable the irq sources we actually
need in PORT_HOTPLUG_EN/STATUS, taking dev_priv->hotplug_supported_mask
into account, no hpd interrupt sources is enabled since
commit 52d7ecedac3f96fb562cb482c139015372728638
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Sat Dec 1 21:03:22 2012 +0100
drm/i915: reorder setup sequence to have irqs for output setup
Wrongly set-up interrupts also lead broken hw-based load-detection on
at least GM45, resulting in ghost VGA/TV-out outputs.
To fix this, delay the hotplug register setup until after all outputs
are set up, by moving it into a new dev_priv->display.hpd_irq_callback.
We might also move the PCH_SPLIT platforms to such a setup eventually.
Another funny part is that we need to delay the fbdev initial config
probing until after the hpd regs are setup, for otherwise it'll detect
ghost outputs. But we can only enable the hpd interrupt handling
itself (and the output polling) _after_ that initial scan, due to
massive locking brain-damage in the fbdev setup code. Add a big
comment to explain this cute little dragon lair.
Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_dma.c | 4 +-
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_irq.c | 79 ++++++++++++++++++++++++++++++++-------
drivers/gpu/drm/i915/intel_fb.c | 2 +-
4 files changed, 70 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 9363066..6c6620c 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1319,9 +1319,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
goto cleanup_gem;
/* Only enable hotplug handling once the fbdev is fully set up. */
- dev_priv->enable_hotplug_processing = true;
-
- drm_kms_helper_poll_init(dev);
+ intel_hpd_init(dev);
/* We're off and running w/KMS */
dev_priv->mm.suspended = 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e9ac360..36a3bde 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -295,6 +295,7 @@ struct drm_i915_display_funcs {
struct drm_i915_gem_object *obj);
int (*update_plane)(struct drm_crtc *crtc, struct drm_framebuffer *fb,
int x, int y);
+ void (*hpd_irq_setup)(struct drm_device *dev);
/* clock updates for mode set */
/* cursor updates */
/* render clock increase/decrease */
@@ -1341,6 +1342,7 @@ void i915_hangcheck_elapsed(unsigned long data);
void i915_handle_error(struct drm_device *dev, bool wedged);
extern void intel_irq_init(struct drm_device *dev);
+extern void intel_hpd_init(struct drm_device *dev);
extern void intel_gt_init(struct drm_device *dev);
extern void intel_gt_reset(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2e1d80d..5bdf043 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1995,7 +1995,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
u32 enable_mask;
- u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN);
u32 pipestat_enable = PLANE_FLIP_DONE_INT_EN_VLV;
u32 render_irqs;
u16 msid;
@@ -2024,6 +2023,9 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
msid |= (1<<14);
pci_write_config_word(dev_priv->dev->pdev, 0x98, msid);
+ I915_WRITE(PORT_HOTPLUG_EN, 0);
+ POSTING_READ(PORT_HOTPLUG_EN);
+
I915_WRITE(VLV_IMR, dev_priv->irq_mask);
I915_WRITE(VLV_IER, enable_mask);
I915_WRITE(VLV_IIR, 0xffffffff);
@@ -2053,6 +2055,15 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
#endif
I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
+
+ return 0;
+}
+
+static void valleyview_hpd_irq_setup(struct drm_device *dev)
+{
+ drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+ u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN);
+
/* Note HDMI and DP share bits */
if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS)
hotplug_en |= HDMIB_HOTPLUG_INT_EN;
@@ -2070,8 +2081,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
}
I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
-
- return 0;
}
static void valleyview_irq_uninstall(struct drm_device *dev)
@@ -2301,6 +2310,9 @@ static int i915_irq_postinstall(struct drm_device *dev)
I915_USER_INTERRUPT;
if (I915_HAS_HOTPLUG(dev)) {
+ I915_WRITE(PORT_HOTPLUG_EN, 0);
+ POSTING_READ(PORT_HOTPLUG_EN);
+
/* Enable in IER... */
enable_mask |= I915_DISPLAY_PORT_INTERRUPT;
/* and unmask in IMR */
@@ -2311,8 +2323,18 @@ static int i915_irq_postinstall(struct drm_device *dev)
I915_WRITE(IER, enable_mask);
POSTING_READ(IER);
+ intel_opregion_enable_asle(dev);
+
+ return 0;
+}
+
+static void i915_hpd_irq_setup(struct drm_device *dev)
+{
+ drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+ u32 hotplug_en;
+
if (I915_HAS_HOTPLUG(dev)) {
- u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN);
+ hotplug_en = I915_READ(PORT_HOTPLUG_EN);
if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS)
hotplug_en |= HDMIB_HOTPLUG_INT_EN;
@@ -2333,10 +2355,6 @@ static int i915_irq_postinstall(struct drm_device *dev)
I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
}
-
- intel_opregion_enable_asle(dev);
-
- return 0;
}
static irqreturn_t i915_irq_handler(int irq, void *arg)
@@ -2496,7 +2514,6 @@ static void i965_irq_preinstall(struct drm_device * dev)
static int i965_irq_postinstall(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
- u32 hotplug_en;
u32 enable_mask;
u32 error_mask;
@@ -2538,6 +2555,19 @@ static int i965_irq_postinstall(struct drm_device *dev)
I915_WRITE(IER, enable_mask);
POSTING_READ(IER);
+ I915_WRITE(PORT_HOTPLUG_EN, 0);
+ POSTING_READ(PORT_HOTPLUG_EN);
+
+ intel_opregion_enable_asle(dev);
+
+ return 0;
+}
+
+static void i965_hpd_irq_setup(struct drm_device *dev)
+{
+ drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+ u32 hotplug_en;
+
/* Note HDMI and DP share hotplug bits */
hotplug_en = 0;
if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS)
@@ -2572,10 +2602,6 @@ static int i965_irq_postinstall(struct drm_device *dev)
/* Ignore TV since it's buggy */
I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
-
- intel_opregion_enable_asle(dev);
-
- return 0;
}
static irqreturn_t i965_irq_handler(int irq, void *arg)
@@ -2754,6 +2780,7 @@ void intel_irq_init(struct drm_device *dev)
dev->driver->irq_uninstall = valleyview_irq_uninstall;
dev->driver->enable_vblank = valleyview_enable_vblank;
dev->driver->disable_vblank = valleyview_disable_vblank;
+ dev_priv->display.hpd_irq_setup = valleyview_hpd_irq_setup;
} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
/* Share pre & uninstall handlers with ILK/SNB */
dev->driver->irq_handler = ivybridge_irq_handler;
@@ -2780,13 +2807,39 @@ void intel_irq_init(struct drm_device *dev)
dev->driver->irq_postinstall = i915_irq_postinstall;
dev->driver->irq_uninstall = i915_irq_uninstall;
dev->driver->irq_handler = i915_irq_handler;
+ dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
} else {
dev->driver->irq_preinstall = i965_irq_preinstall;
dev->driver->irq_postinstall = i965_irq_postinstall;
dev->driver->irq_uninstall = i965_irq_uninstall;
dev->driver->irq_handler = i965_irq_handler;
+ dev_priv->display.hpd_irq_setup = i965_hpd_irq_setup;
}
dev->driver->enable_vblank = i915_enable_vblank;
dev->driver->disable_vblank = i915_disable_vblank;
}
}
+
+void intel_hpd_init(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ if (dev_priv->display.hpd_irq_setup)
+ dev_priv->display.hpd_irq_setup(dev);
+
+ /*
+ * Some ports require correctly set-up hpd registers for detection to
+ * work properly, e.g. VGA on gm45. Hence we can only set up the initial
+ * fbdev config after hpd irqs are fully enabled. Now we should scan for
+ * the initial config only once hotplug handling is enabled, but due to
+ * screwed-up locking around kms/fbdev init we can't protect the fdbev
+ * initial config scanning against hotplug events. Hence do this first
+ * and ignore the tiny window where we will loose hotplug notifactions.
+ */
+ drm_fb_helper_initial_config(&dev_priv->fbdev->helper, 32);
+
+ /* Only enable hotplug handling once the fbdev is fully set up. */
+ dev_priv->enable_hotplug_processing = true;
+
+ drm_kms_helper_poll_init(dev);
+}
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index b7773e5..ddfc329 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -243,7 +243,7 @@ int intel_fbdev_init(struct drm_device *dev)
}
drm_fb_helper_single_add_all_connectors(&ifbdev->helper);
- drm_fb_helper_initial_config(&ifbdev->helper, 32);
+
return 0;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] drm/i915: Fixup hpd irq register setup ordering 2012-12-09 19:14 [PATCH] drm/i915: Fixup hpd irq register setup ordering Daniel Vetter @ 2012-12-09 19:17 ` Daniel Vetter 2012-12-09 19:42 ` Daniel Vetter 1 sibling, 0 replies; 10+ messages in thread From: Daniel Vetter @ 2012-12-09 19:17 UTC (permalink / raw) To: Intel Graphics Development; +Cc: Daniel Vetter On Sun, Dec 09, 2012 at 08:14:58PM +0100, Daniel Vetter wrote: > For GMCH platforms we set up the hpd irq registers in the irq > postinstall hook. But since we only enable the irq sources we actually > need in PORT_HOTPLUG_EN/STATUS, taking dev_priv->hotplug_supported_mask > into account, no hpd interrupt sources is enabled since > > commit 52d7ecedac3f96fb562cb482c139015372728638 > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > Date: Sat Dec 1 21:03:22 2012 +0100 > > drm/i915: reorder setup sequence to have irqs for output setup > > Wrongly set-up interrupts also lead broken hw-based load-detection on > at least GM45, resulting in ghost VGA/TV-out outputs. > > To fix this, delay the hotplug register setup until after all outputs > are set up, by moving it into a new dev_priv->display.hpd_irq_callback. > We might also move the PCH_SPLIT platforms to such a setup eventually. > > Another funny part is that we need to delay the fbdev initial config > probing until after the hpd regs are setup, for otherwise it'll detect > ghost outputs. But we can only enable the hpd interrupt handling > itself (and the output polling) _after_ that initial scan, due to > massive locking brain-damage in the fbdev setup code. Add a big > comment to explain this cute little dragon lair. > > Reported-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54943 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] drm/i915: Fixup hpd irq register setup ordering 2012-12-09 19:14 [PATCH] drm/i915: Fixup hpd irq register setup ordering Daniel Vetter 2012-12-09 19:17 ` Daniel Vetter @ 2012-12-09 19:42 ` Daniel Vetter 2012-12-10 13:40 ` Chris Wilson 2012-12-10 21:01 ` Jesse Barnes 1 sibling, 2 replies; 10+ messages in thread From: Daniel Vetter @ 2012-12-09 19:42 UTC (permalink / raw) To: Intel Graphics Development; +Cc: Daniel Vetter For GMCH platforms we set up the hpd irq registers in the irq postinstall hook. But since we only enable the irq sources we actually need in PORT_HOTPLUG_EN/STATUS, taking dev_priv->hotplug_supported_mask into account, no hpd interrupt sources is enabled since commit 52d7ecedac3f96fb562cb482c139015372728638 Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Sat Dec 1 21:03:22 2012 +0100 drm/i915: reorder setup sequence to have irqs for output setup Wrongly set-up interrupts also lead broken hw-based load-detection on at least GM45, resulting in ghost VGA/TV-out outputs. To fix this, delay the hotplug register setup until after all outputs are set up, by moving it into a new dev_priv->display.hpd_irq_callback. We might also move the PCH_SPLIT platforms to such a setup eventually. Another funny part is that we need to delay the fbdev initial config probing until after the hpd regs are setup, for otherwise it'll detect ghost outputs. But we can only enable the hpd interrupt handling itself (and the output polling) _after_ that initial scan, due to massive locking brain-damage in the fbdev setup code. Add a big comment to explain this cute little dragon lair. v2: Encapsulate all the fbdev handling by wrapping the move call into intel_fbdev_initial_config in intel_fb.c. Requested by Chris Wilson. Bugzilla: Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54943 Reported-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_dma.c | 4 +- drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/i915_irq.c | 79 +++++++++++++++++++++++++++++++------- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_fb.c | 10 ++++- 5 files changed, 79 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 9363066..6c6620c 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1319,9 +1319,7 @@ static int i915_load_modeset_init(struct drm_device *dev) goto cleanup_gem; /* Only enable hotplug handling once the fbdev is fully set up. */ - dev_priv->enable_hotplug_processing = true; - - drm_kms_helper_poll_init(dev); + intel_hpd_init(dev); /* We're off and running w/KMS */ dev_priv->mm.suspended = 0; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e9ac360..36a3bde 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -295,6 +295,7 @@ struct drm_i915_display_funcs { struct drm_i915_gem_object *obj); int (*update_plane)(struct drm_crtc *crtc, struct drm_framebuffer *fb, int x, int y); + void (*hpd_irq_setup)(struct drm_device *dev); /* clock updates for mode set */ /* cursor updates */ /* render clock increase/decrease */ @@ -1341,6 +1342,7 @@ void i915_hangcheck_elapsed(unsigned long data); void i915_handle_error(struct drm_device *dev, bool wedged); extern void intel_irq_init(struct drm_device *dev); +extern void intel_hpd_init(struct drm_device *dev); extern void intel_gt_init(struct drm_device *dev); extern void intel_gt_reset(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 2e1d80d..c8a3f61 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1995,7 +1995,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; u32 enable_mask; - u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN); u32 pipestat_enable = PLANE_FLIP_DONE_INT_EN_VLV; u32 render_irqs; u16 msid; @@ -2024,6 +2023,9 @@ static int valleyview_irq_postinstall(struct drm_device *dev) msid |= (1<<14); pci_write_config_word(dev_priv->dev->pdev, 0x98, msid); + I915_WRITE(PORT_HOTPLUG_EN, 0); + POSTING_READ(PORT_HOTPLUG_EN); + I915_WRITE(VLV_IMR, dev_priv->irq_mask); I915_WRITE(VLV_IER, enable_mask); I915_WRITE(VLV_IIR, 0xffffffff); @@ -2053,6 +2055,15 @@ static int valleyview_irq_postinstall(struct drm_device *dev) #endif I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE); + + return 0; +} + +static void valleyview_hpd_irq_setup(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; + u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN); + /* Note HDMI and DP share bits */ if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS) hotplug_en |= HDMIB_HOTPLUG_INT_EN; @@ -2070,8 +2081,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev) } I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); - - return 0; } static void valleyview_irq_uninstall(struct drm_device *dev) @@ -2301,6 +2310,9 @@ static int i915_irq_postinstall(struct drm_device *dev) I915_USER_INTERRUPT; if (I915_HAS_HOTPLUG(dev)) { + I915_WRITE(PORT_HOTPLUG_EN, 0); + POSTING_READ(PORT_HOTPLUG_EN); + /* Enable in IER... */ enable_mask |= I915_DISPLAY_PORT_INTERRUPT; /* and unmask in IMR */ @@ -2311,8 +2323,18 @@ static int i915_irq_postinstall(struct drm_device *dev) I915_WRITE(IER, enable_mask); POSTING_READ(IER); + intel_opregion_enable_asle(dev); + + return 0; +} + +static void i915_hpd_irq_setup(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; + u32 hotplug_en; + if (I915_HAS_HOTPLUG(dev)) { - u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN); + hotplug_en = I915_READ(PORT_HOTPLUG_EN); if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS) hotplug_en |= HDMIB_HOTPLUG_INT_EN; @@ -2333,10 +2355,6 @@ static int i915_irq_postinstall(struct drm_device *dev) I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); } - - intel_opregion_enable_asle(dev); - - return 0; } static irqreturn_t i915_irq_handler(int irq, void *arg) @@ -2496,7 +2514,6 @@ static void i965_irq_preinstall(struct drm_device * dev) static int i965_irq_postinstall(struct drm_device *dev) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; - u32 hotplug_en; u32 enable_mask; u32 error_mask; @@ -2538,6 +2555,19 @@ static int i965_irq_postinstall(struct drm_device *dev) I915_WRITE(IER, enable_mask); POSTING_READ(IER); + I915_WRITE(PORT_HOTPLUG_EN, 0); + POSTING_READ(PORT_HOTPLUG_EN); + + intel_opregion_enable_asle(dev); + + return 0; +} + +static void i965_hpd_irq_setup(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; + u32 hotplug_en; + /* Note HDMI and DP share hotplug bits */ hotplug_en = 0; if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS) @@ -2572,10 +2602,6 @@ static int i965_irq_postinstall(struct drm_device *dev) /* Ignore TV since it's buggy */ I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); - - intel_opregion_enable_asle(dev); - - return 0; } static irqreturn_t i965_irq_handler(int irq, void *arg) @@ -2754,6 +2780,7 @@ void intel_irq_init(struct drm_device *dev) dev->driver->irq_uninstall = valleyview_irq_uninstall; dev->driver->enable_vblank = valleyview_enable_vblank; dev->driver->disable_vblank = valleyview_disable_vblank; + dev_priv->display.hpd_irq_setup = valleyview_hpd_irq_setup; } else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) { /* Share pre & uninstall handlers with ILK/SNB */ dev->driver->irq_handler = ivybridge_irq_handler; @@ -2780,13 +2807,39 @@ void intel_irq_init(struct drm_device *dev) dev->driver->irq_postinstall = i915_irq_postinstall; dev->driver->irq_uninstall = i915_irq_uninstall; dev->driver->irq_handler = i915_irq_handler; + dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; } else { dev->driver->irq_preinstall = i965_irq_preinstall; dev->driver->irq_postinstall = i965_irq_postinstall; dev->driver->irq_uninstall = i965_irq_uninstall; dev->driver->irq_handler = i965_irq_handler; + dev_priv->display.hpd_irq_setup = i965_hpd_irq_setup; } dev->driver->enable_vblank = i915_enable_vblank; dev->driver->disable_vblank = i915_disable_vblank; } } + +void intel_hpd_init(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + + if (dev_priv->display.hpd_irq_setup) + dev_priv->display.hpd_irq_setup(dev); + + /* + * Some ports require correctly set-up hpd registers for detection to + * work properly, e.g. VGA on gm45. Hence we can only set up the initial + * fbdev config after hpd irqs are fully enabled. Now we should scan for + * the initial config only once hotplug handling is enabled, but due to + * screwed-up locking around kms/fbdev init we can't protect the fdbev + * initial config scanning against hotplug events. Hence do this first + * and ignore the tiny window where we will loose hotplug notifactions. + */ + intel_fbdev_initial_config(dev); + + /* Only enable hotplug handling once the fbdev is fully set up. */ + dev_priv->enable_hotplug_processing = true; + + drm_kms_helper_poll_init(dev); +} diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 7ca7772..22728f2 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -587,6 +587,7 @@ extern int intel_framebuffer_init(struct drm_device *dev, struct drm_mode_fb_cmd2 *mode_cmd, struct drm_i915_gem_object *obj); extern int intel_fbdev_init(struct drm_device *dev); +extern void intel_fbdev_initial_config(struct drm_device *dev); extern void intel_fbdev_fini(struct drm_device *dev); extern void intel_fbdev_set_suspend(struct drm_device *dev, int state); extern void intel_prepare_page_flip(struct drm_device *dev, int plane); diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c index b7773e5..82289678 100644 --- a/drivers/gpu/drm/i915/intel_fb.c +++ b/drivers/gpu/drm/i915/intel_fb.c @@ -243,10 +243,18 @@ int intel_fbdev_init(struct drm_device *dev) } drm_fb_helper_single_add_all_connectors(&ifbdev->helper); - drm_fb_helper_initial_config(&ifbdev->helper, 32); + return 0; } +void intel_fbdev_initial_config(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + + /* Due to peculiar init order wrt to hpd handling this is separate. */ + drm_fb_helper_initial_config(&dev_priv->fbdev->helper, 32); +} + void intel_fbdev_fini(struct drm_device *dev) { drm_i915_private_t *dev_priv = dev->dev_private; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Fixup hpd irq register setup ordering 2012-12-09 19:42 ` Daniel Vetter @ 2012-12-10 13:40 ` Chris Wilson 2012-12-10 21:01 ` Jesse Barnes 1 sibling, 0 replies; 10+ messages in thread From: Chris Wilson @ 2012-12-10 13:40 UTC (permalink / raw) To: Intel Graphics Development; +Cc: Daniel Vetter On Sun, 9 Dec 2012 20:42:01 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > For GMCH platforms we set up the hpd irq registers in the irq > postinstall hook. But since we only enable the irq sources we actually > need in PORT_HOTPLUG_EN/STATUS, taking dev_priv->hotplug_supported_mask > into account, no hpd interrupt sources is enabled since > > commit 52d7ecedac3f96fb562cb482c139015372728638 > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > Date: Sat Dec 1 21:03:22 2012 +0100 > > drm/i915: reorder setup sequence to have irqs for output setup > > Wrongly set-up interrupts also lead broken hw-based load-detection on > at least GM45, resulting in ghost VGA/TV-out outputs. > > To fix this, delay the hotplug register setup until after all outputs > are set up, by moving it into a new dev_priv->display.hpd_irq_callback. > We might also move the PCH_SPLIT platforms to such a setup eventually. > > Another funny part is that we need to delay the fbdev initial config > probing until after the hpd regs are setup, for otherwise it'll detect > ghost outputs. But we can only enable the hpd interrupt handling > itself (and the output polling) _after_ that initial scan, due to > massive locking brain-damage in the fbdev setup code. Add a big > comment to explain this cute little dragon lair. > > v2: Encapsulate all the fbdev handling by wrapping the move call into > intel_fbdev_initial_config in intel_fb.c. Requested by Chris Wilson. > > Bugzilla: Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54943 > Reported-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Works for me, thanks. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Fixup hpd irq register setup ordering 2012-12-09 19:42 ` Daniel Vetter 2012-12-10 13:40 ` Chris Wilson @ 2012-12-10 21:01 ` Jesse Barnes 2012-12-10 21:55 ` Daniel Vetter 2012-12-11 12:06 ` Daniel Vetter 1 sibling, 2 replies; 10+ messages in thread From: Jesse Barnes @ 2012-12-10 21:01 UTC (permalink / raw) To: Daniel Vetter; +Cc: Intel Graphics Development On Sun, 9 Dec 2012 20:42:01 +0100 Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > For GMCH platforms we set up the hpd irq registers in the irq > postinstall hook. But since we only enable the irq sources we actually > need in PORT_HOTPLUG_EN/STATUS, taking dev_priv->hotplug_supported_mask > into account, no hpd interrupt sources is enabled since > > commit 52d7ecedac3f96fb562cb482c139015372728638 > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > Date: Sat Dec 1 21:03:22 2012 +0100 > > drm/i915: reorder setup sequence to have irqs for output setup > > Wrongly set-up interrupts also lead broken hw-based load-detection on > at least GM45, resulting in ghost VGA/TV-out outputs. ..."can lead to broken"... or "also leads to" ? > + > +void intel_hpd_init(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + if (dev_priv->display.hpd_irq_setup) > + dev_priv->display.hpd_irq_setup(dev); > + > + /* > + * Some ports require correctly set-up hpd registers for detection to > + * work properly, e.g. VGA on gm45. Hence we can only set up the initial > + * fbdev config after hpd irqs are fully enabled. Now we should scan for > + * the initial config only once hotplug handling is enabled, but due to > + * screwed-up locking around kms/fbdev init we can't protect the fdbev > + * initial config scanning against hotplug events. Hence do this first > + * and ignore the tiny window where we will loose hotplug notifactions. > + */ > + intel_fbdev_initial_config(dev); > + > + /* Only enable hotplug handling once the fbdev is fully set up. */ > + dev_priv->enable_hotplug_processing = true; > + > + drm_kms_helper_poll_init(dev); > +} I'd rather see these calls in load_modeset_init(). Having fbdev_initial_config here at least is confusing, and since poll_init depends on it... With those two nits: Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] drm/i915: Fixup hpd irq register setup ordering 2012-12-10 21:01 ` Jesse Barnes @ 2012-12-10 21:55 ` Daniel Vetter 2012-12-11 13:05 ` Daniel Vetter 2012-12-11 12:06 ` Daniel Vetter 1 sibling, 1 reply; 10+ messages in thread From: Daniel Vetter @ 2012-12-10 21:55 UTC (permalink / raw) To: Intel Graphics Development; +Cc: Daniel Vetter For GMCH platforms we set up the hpd irq registers in the irq postinstall hook. But since we only enable the irq sources we actually need in PORT_HOTPLUG_EN/STATUS, taking dev_priv->hotplug_supported_mask into account, no hpd interrupt sources is enabled since commit 52d7ecedac3f96fb562cb482c139015372728638 Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Sat Dec 1 21:03:22 2012 +0100 drm/i915: reorder setup sequence to have irqs for output setup Wrongly set-up interrupts also lead to broken hw-based load-detection on at least GM45, resulting in ghost VGA/TV-out outputs. To fix this, delay the hotplug register setup until after all outputs are set up, by moving it into a new dev_priv->display.hpd_irq_callback. We might also move the PCH_SPLIT platforms to such a setup eventually. Another funny part is that we need to delay the fbdev initial config probing until after the hpd regs are setup, for otherwise it'll detect ghost outputs. But we can only enable the hpd interrupt handling itself (and the output polling) _after_ that initial scan, due to massive locking brain-damage in the fbdev setup code. Add a big comment to explain this cute little dragon lair. v2: Encapsulate all the fbdev handling by wrapping the move call into intel_fbdev_initial_config in intel_fb.c. Requested by Chris Wilson. v3: Applied bikeshed from Jesse Barnes. Bugzilla: Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54943 Reported-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_dma.c | 14 +++++++++ drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_irq.c | 63 ++++++++++++++++++++++++++++++-------- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_fb.c | 10 +++++- 5 files changed, 76 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 9363066..5e03979 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1319,6 +1319,20 @@ static int i915_load_modeset_init(struct drm_device *dev) goto cleanup_gem; /* Only enable hotplug handling once the fbdev is fully set up. */ + intel_hpd_init(dev); + + /* + * Some ports require correctly set-up hpd registers for detection to + * work properly, e.g. VGA on gm45. Hence we can only set up the initial + * fbdev config after hpd irqs are fully enabled. Now we should scan for + * the initial config only once hotplug handling is enabled, but due to + * screwed-up locking around kms/fbdev init we can't protect the fdbev + * initial config scanning against hotplug events. Hence do this first + * and ignore the tiny window where we will loose hotplug notifactions. + */ + intel_fbdev_initial_config(dev); + + /* Only enable hotplug handling once the fbdev is fully set up. */ dev_priv->enable_hotplug_processing = true; drm_kms_helper_poll_init(dev); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e9ac360..36a3bde 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -295,6 +295,7 @@ struct drm_i915_display_funcs { struct drm_i915_gem_object *obj); int (*update_plane)(struct drm_crtc *crtc, struct drm_framebuffer *fb, int x, int y); + void (*hpd_irq_setup)(struct drm_device *dev); /* clock updates for mode set */ /* cursor updates */ /* render clock increase/decrease */ @@ -1341,6 +1342,7 @@ void i915_hangcheck_elapsed(unsigned long data); void i915_handle_error(struct drm_device *dev, bool wedged); extern void intel_irq_init(struct drm_device *dev); +extern void intel_hpd_init(struct drm_device *dev); extern void intel_gt_init(struct drm_device *dev); extern void intel_gt_reset(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 2e1d80d..de9947a 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1995,7 +1995,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; u32 enable_mask; - u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN); u32 pipestat_enable = PLANE_FLIP_DONE_INT_EN_VLV; u32 render_irqs; u16 msid; @@ -2024,6 +2023,9 @@ static int valleyview_irq_postinstall(struct drm_device *dev) msid |= (1<<14); pci_write_config_word(dev_priv->dev->pdev, 0x98, msid); + I915_WRITE(PORT_HOTPLUG_EN, 0); + POSTING_READ(PORT_HOTPLUG_EN); + I915_WRITE(VLV_IMR, dev_priv->irq_mask); I915_WRITE(VLV_IER, enable_mask); I915_WRITE(VLV_IIR, 0xffffffff); @@ -2053,6 +2055,15 @@ static int valleyview_irq_postinstall(struct drm_device *dev) #endif I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE); + + return 0; +} + +static void valleyview_hpd_irq_setup(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; + u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN); + /* Note HDMI and DP share bits */ if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS) hotplug_en |= HDMIB_HOTPLUG_INT_EN; @@ -2070,8 +2081,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev) } I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); - - return 0; } static void valleyview_irq_uninstall(struct drm_device *dev) @@ -2301,6 +2310,9 @@ static int i915_irq_postinstall(struct drm_device *dev) I915_USER_INTERRUPT; if (I915_HAS_HOTPLUG(dev)) { + I915_WRITE(PORT_HOTPLUG_EN, 0); + POSTING_READ(PORT_HOTPLUG_EN); + /* Enable in IER... */ enable_mask |= I915_DISPLAY_PORT_INTERRUPT; /* and unmask in IMR */ @@ -2311,8 +2323,18 @@ static int i915_irq_postinstall(struct drm_device *dev) I915_WRITE(IER, enable_mask); POSTING_READ(IER); + intel_opregion_enable_asle(dev); + + return 0; +} + +static void i915_hpd_irq_setup(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; + u32 hotplug_en; + if (I915_HAS_HOTPLUG(dev)) { - u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN); + hotplug_en = I915_READ(PORT_HOTPLUG_EN); if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS) hotplug_en |= HDMIB_HOTPLUG_INT_EN; @@ -2333,10 +2355,6 @@ static int i915_irq_postinstall(struct drm_device *dev) I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); } - - intel_opregion_enable_asle(dev); - - return 0; } static irqreturn_t i915_irq_handler(int irq, void *arg) @@ -2496,7 +2514,6 @@ static void i965_irq_preinstall(struct drm_device * dev) static int i965_irq_postinstall(struct drm_device *dev) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; - u32 hotplug_en; u32 enable_mask; u32 error_mask; @@ -2538,6 +2555,19 @@ static int i965_irq_postinstall(struct drm_device *dev) I915_WRITE(IER, enable_mask); POSTING_READ(IER); + I915_WRITE(PORT_HOTPLUG_EN, 0); + POSTING_READ(PORT_HOTPLUG_EN); + + intel_opregion_enable_asle(dev); + + return 0; +} + +static void i965_hpd_irq_setup(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; + u32 hotplug_en; + /* Note HDMI and DP share hotplug bits */ hotplug_en = 0; if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS) @@ -2572,10 +2602,6 @@ static int i965_irq_postinstall(struct drm_device *dev) /* Ignore TV since it's buggy */ I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); - - intel_opregion_enable_asle(dev); - - return 0; } static irqreturn_t i965_irq_handler(int irq, void *arg) @@ -2754,6 +2780,7 @@ void intel_irq_init(struct drm_device *dev) dev->driver->irq_uninstall = valleyview_irq_uninstall; dev->driver->enable_vblank = valleyview_enable_vblank; dev->driver->disable_vblank = valleyview_disable_vblank; + dev_priv->display.hpd_irq_setup = valleyview_hpd_irq_setup; } else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) { /* Share pre & uninstall handlers with ILK/SNB */ dev->driver->irq_handler = ivybridge_irq_handler; @@ -2780,13 +2807,23 @@ void intel_irq_init(struct drm_device *dev) dev->driver->irq_postinstall = i915_irq_postinstall; dev->driver->irq_uninstall = i915_irq_uninstall; dev->driver->irq_handler = i915_irq_handler; + dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; } else { dev->driver->irq_preinstall = i965_irq_preinstall; dev->driver->irq_postinstall = i965_irq_postinstall; dev->driver->irq_uninstall = i965_irq_uninstall; dev->driver->irq_handler = i965_irq_handler; + dev_priv->display.hpd_irq_setup = i965_hpd_irq_setup; } dev->driver->enable_vblank = i915_enable_vblank; dev->driver->disable_vblank = i915_disable_vblank; } } + +void intel_hpd_init(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + + if (dev_priv->display.hpd_irq_setup) + dev_priv->display.hpd_irq_setup(dev); +} diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 7ca7772..22728f2 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -587,6 +587,7 @@ extern int intel_framebuffer_init(struct drm_device *dev, struct drm_mode_fb_cmd2 *mode_cmd, struct drm_i915_gem_object *obj); extern int intel_fbdev_init(struct drm_device *dev); +extern void intel_fbdev_initial_config(struct drm_device *dev); extern void intel_fbdev_fini(struct drm_device *dev); extern void intel_fbdev_set_suspend(struct drm_device *dev, int state); extern void intel_prepare_page_flip(struct drm_device *dev, int plane); diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c index b7773e5..82289678 100644 --- a/drivers/gpu/drm/i915/intel_fb.c +++ b/drivers/gpu/drm/i915/intel_fb.c @@ -243,10 +243,18 @@ int intel_fbdev_init(struct drm_device *dev) } drm_fb_helper_single_add_all_connectors(&ifbdev->helper); - drm_fb_helper_initial_config(&ifbdev->helper, 32); + return 0; } +void intel_fbdev_initial_config(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + + /* Due to peculiar init order wrt to hpd handling this is separate. */ + drm_fb_helper_initial_config(&dev_priv->fbdev->helper, 32); +} + void intel_fbdev_fini(struct drm_device *dev) { drm_i915_private_t *dev_priv = dev->dev_private; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] drm/i915: Fixup hpd irq register setup ordering 2012-12-10 21:55 ` Daniel Vetter @ 2012-12-11 13:05 ` Daniel Vetter 2012-12-11 15:58 ` Imre Deak 0 siblings, 1 reply; 10+ messages in thread From: Daniel Vetter @ 2012-12-11 13:05 UTC (permalink / raw) To: Intel Graphics Development; +Cc: Daniel Vetter For GMCH platforms we set up the hpd irq registers in the irq postinstall hook. But since we only enable the irq sources we actually need in PORT_HOTPLUG_EN/STATUS, taking dev_priv->hotplug_supported_mask into account, no hpd interrupt sources is enabled since commit 52d7ecedac3f96fb562cb482c139015372728638 Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Sat Dec 1 21:03:22 2012 +0100 drm/i915: reorder setup sequence to have irqs for output setup Wrongly set-up interrupts also lead to broken hw-based load-detection on at least GM45, resulting in ghost VGA/TV-out outputs. To fix this, delay the hotplug register setup until after all outputs are set up, by moving it into a new dev_priv->display.hpd_irq_callback. We might also move the PCH_SPLIT platforms to such a setup eventually. Another funny part is that we need to delay the fbdev initial config probing until after the hpd regs are setup, for otherwise it'll detect ghost outputs. But we can only enable the hpd interrupt handling itself (and the output polling) _after_ that initial scan, due to massive locking brain-damage in the fbdev setup code. Add a big comment to explain this cute little dragon lair. v2: Encapsulate all the fbdev handling by wrapping the move call into intel_fbdev_initial_config in intel_fb.c. Requested by Chris Wilson. v3: Applied bikeshed from Jesse Barnes. v4: Imre Deak noticed that we also need to call intel_hpd_init after the drm_irqinstall calls in the gpu reset and resume paths - otherwise hotplug will be broken. Also improve the comment a bit about why hpd_init needs to be called before we set up the initial fbdev config. Bugzilla: Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54943 Reported-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_dma.c | 15 +++++++++ drivers/gpu/drm/i915/i915_drv.c | 2 ++ drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_irq.c | 63 ++++++++++++++++++++++++++++++-------- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_fb.c | 10 +++++- 6 files changed, 79 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 9363066..0c2ab40 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1319,6 +1319,21 @@ static int i915_load_modeset_init(struct drm_device *dev) goto cleanup_gem; /* Only enable hotplug handling once the fbdev is fully set up. */ + intel_hpd_init(dev); + + /* + * Some ports require correctly set-up hpd registers for detection to + * work properly (leading to ghost connected connector status), e.g. VGA + * on gm45. Hence we can only set up the initial fbdev config after hpd + * irqs are fully enabled. Now we should scan for the initial config + * only once hotplug handling is enabled, but due to screwed-up locking + * around kms/fbdev init we can't protect the fdbev initial config + * scanning against hotplug events. Hence do this first and ignore the + * tiny window where we will loose hotplug notifactions. + */ + intel_fbdev_initial_config(dev); + + /* Only enable hotplug handling once the fbdev is fully set up. */ dev_priv->enable_hotplug_processing = true; drm_kms_helper_poll_init(dev); diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index a129218..fbd0b28 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -566,6 +566,7 @@ static int __i915_drm_thaw(struct drm_device *dev) intel_modeset_init_hw(dev); intel_modeset_setup_hw_state(dev, false); drm_irq_install(dev); + intel_hpd_init(dev); } intel_opregion_init(dev); @@ -871,6 +872,7 @@ int i915_reset(struct drm_device *dev) drm_irq_uninstall(dev); drm_irq_install(dev); + intel_hpd_init(dev); } else { mutex_unlock(&dev->struct_mutex); } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e9ac360..36a3bde 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -295,6 +295,7 @@ struct drm_i915_display_funcs { struct drm_i915_gem_object *obj); int (*update_plane)(struct drm_crtc *crtc, struct drm_framebuffer *fb, int x, int y); + void (*hpd_irq_setup)(struct drm_device *dev); /* clock updates for mode set */ /* cursor updates */ /* render clock increase/decrease */ @@ -1341,6 +1342,7 @@ void i915_hangcheck_elapsed(unsigned long data); void i915_handle_error(struct drm_device *dev, bool wedged); extern void intel_irq_init(struct drm_device *dev); +extern void intel_hpd_init(struct drm_device *dev); extern void intel_gt_init(struct drm_device *dev); extern void intel_gt_reset(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 2e1d80d..de9947a 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1995,7 +1995,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; u32 enable_mask; - u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN); u32 pipestat_enable = PLANE_FLIP_DONE_INT_EN_VLV; u32 render_irqs; u16 msid; @@ -2024,6 +2023,9 @@ static int valleyview_irq_postinstall(struct drm_device *dev) msid |= (1<<14); pci_write_config_word(dev_priv->dev->pdev, 0x98, msid); + I915_WRITE(PORT_HOTPLUG_EN, 0); + POSTING_READ(PORT_HOTPLUG_EN); + I915_WRITE(VLV_IMR, dev_priv->irq_mask); I915_WRITE(VLV_IER, enable_mask); I915_WRITE(VLV_IIR, 0xffffffff); @@ -2053,6 +2055,15 @@ static int valleyview_irq_postinstall(struct drm_device *dev) #endif I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE); + + return 0; +} + +static void valleyview_hpd_irq_setup(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; + u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN); + /* Note HDMI and DP share bits */ if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS) hotplug_en |= HDMIB_HOTPLUG_INT_EN; @@ -2070,8 +2081,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev) } I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); - - return 0; } static void valleyview_irq_uninstall(struct drm_device *dev) @@ -2301,6 +2310,9 @@ static int i915_irq_postinstall(struct drm_device *dev) I915_USER_INTERRUPT; if (I915_HAS_HOTPLUG(dev)) { + I915_WRITE(PORT_HOTPLUG_EN, 0); + POSTING_READ(PORT_HOTPLUG_EN); + /* Enable in IER... */ enable_mask |= I915_DISPLAY_PORT_INTERRUPT; /* and unmask in IMR */ @@ -2311,8 +2323,18 @@ static int i915_irq_postinstall(struct drm_device *dev) I915_WRITE(IER, enable_mask); POSTING_READ(IER); + intel_opregion_enable_asle(dev); + + return 0; +} + +static void i915_hpd_irq_setup(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; + u32 hotplug_en; + if (I915_HAS_HOTPLUG(dev)) { - u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN); + hotplug_en = I915_READ(PORT_HOTPLUG_EN); if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS) hotplug_en |= HDMIB_HOTPLUG_INT_EN; @@ -2333,10 +2355,6 @@ static int i915_irq_postinstall(struct drm_device *dev) I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); } - - intel_opregion_enable_asle(dev); - - return 0; } static irqreturn_t i915_irq_handler(int irq, void *arg) @@ -2496,7 +2514,6 @@ static void i965_irq_preinstall(struct drm_device * dev) static int i965_irq_postinstall(struct drm_device *dev) { drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; - u32 hotplug_en; u32 enable_mask; u32 error_mask; @@ -2538,6 +2555,19 @@ static int i965_irq_postinstall(struct drm_device *dev) I915_WRITE(IER, enable_mask); POSTING_READ(IER); + I915_WRITE(PORT_HOTPLUG_EN, 0); + POSTING_READ(PORT_HOTPLUG_EN); + + intel_opregion_enable_asle(dev); + + return 0; +} + +static void i965_hpd_irq_setup(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; + u32 hotplug_en; + /* Note HDMI and DP share hotplug bits */ hotplug_en = 0; if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS) @@ -2572,10 +2602,6 @@ static int i965_irq_postinstall(struct drm_device *dev) /* Ignore TV since it's buggy */ I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); - - intel_opregion_enable_asle(dev); - - return 0; } static irqreturn_t i965_irq_handler(int irq, void *arg) @@ -2754,6 +2780,7 @@ void intel_irq_init(struct drm_device *dev) dev->driver->irq_uninstall = valleyview_irq_uninstall; dev->driver->enable_vblank = valleyview_enable_vblank; dev->driver->disable_vblank = valleyview_disable_vblank; + dev_priv->display.hpd_irq_setup = valleyview_hpd_irq_setup; } else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) { /* Share pre & uninstall handlers with ILK/SNB */ dev->driver->irq_handler = ivybridge_irq_handler; @@ -2780,13 +2807,23 @@ void intel_irq_init(struct drm_device *dev) dev->driver->irq_postinstall = i915_irq_postinstall; dev->driver->irq_uninstall = i915_irq_uninstall; dev->driver->irq_handler = i915_irq_handler; + dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; } else { dev->driver->irq_preinstall = i965_irq_preinstall; dev->driver->irq_postinstall = i965_irq_postinstall; dev->driver->irq_uninstall = i965_irq_uninstall; dev->driver->irq_handler = i965_irq_handler; + dev_priv->display.hpd_irq_setup = i965_hpd_irq_setup; } dev->driver->enable_vblank = i915_enable_vblank; dev->driver->disable_vblank = i915_disable_vblank; } } + +void intel_hpd_init(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + + if (dev_priv->display.hpd_irq_setup) + dev_priv->display.hpd_irq_setup(dev); +} diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 7ca7772..22728f2 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -587,6 +587,7 @@ extern int intel_framebuffer_init(struct drm_device *dev, struct drm_mode_fb_cmd2 *mode_cmd, struct drm_i915_gem_object *obj); extern int intel_fbdev_init(struct drm_device *dev); +extern void intel_fbdev_initial_config(struct drm_device *dev); extern void intel_fbdev_fini(struct drm_device *dev); extern void intel_fbdev_set_suspend(struct drm_device *dev, int state); extern void intel_prepare_page_flip(struct drm_device *dev, int plane); diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c index b7773e5..82289678 100644 --- a/drivers/gpu/drm/i915/intel_fb.c +++ b/drivers/gpu/drm/i915/intel_fb.c @@ -243,10 +243,18 @@ int intel_fbdev_init(struct drm_device *dev) } drm_fb_helper_single_add_all_connectors(&ifbdev->helper); - drm_fb_helper_initial_config(&ifbdev->helper, 32); + return 0; } +void intel_fbdev_initial_config(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + + /* Due to peculiar init order wrt to hpd handling this is separate. */ + drm_fb_helper_initial_config(&dev_priv->fbdev->helper, 32); +} + void intel_fbdev_fini(struct drm_device *dev) { drm_i915_private_t *dev_priv = dev->dev_private; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Fixup hpd irq register setup ordering 2012-12-11 13:05 ` Daniel Vetter @ 2012-12-11 15:58 ` Imre Deak 2012-12-11 16:24 ` Daniel Vetter 0 siblings, 1 reply; 10+ messages in thread From: Imre Deak @ 2012-12-11 15:58 UTC (permalink / raw) To: Daniel Vetter; +Cc: Intel Graphics Development On Tue, 2012-12-11 at 14:05 +0100, Daniel Vetter wrote: > For GMCH platforms we set up the hpd irq registers in the irq > postinstall hook. But since we only enable the irq sources we actually > need in PORT_HOTPLUG_EN/STATUS, taking dev_priv->hotplug_supported_mask > into account, no hpd interrupt sources is enabled since > > commit 52d7ecedac3f96fb562cb482c139015372728638 > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > Date: Sat Dec 1 21:03:22 2012 +0100 > > drm/i915: reorder setup sequence to have irqs for output setup > > Wrongly set-up interrupts also lead to broken hw-based load-detection > on at least GM45, resulting in ghost VGA/TV-out outputs. > > To fix this, delay the hotplug register setup until after all outputs > are set up, by moving it into a new dev_priv->display.hpd_irq_callback. > We might also move the PCH_SPLIT platforms to such a setup eventually. > > Another funny part is that we need to delay the fbdev initial config > probing until after the hpd regs are setup, for otherwise it'll detect > ghost outputs. But we can only enable the hpd interrupt handling > itself (and the output polling) _after_ that initial scan, due to > massive locking brain-damage in the fbdev setup code. Add a big > comment to explain this cute little dragon lair. > > v2: Encapsulate all the fbdev handling by wrapping the move call into > intel_fbdev_initial_config in intel_fb.c. Requested by Chris Wilson. > > v3: Applied bikeshed from Jesse Barnes. > > v4: Imre Deak noticed that we also need to call intel_hpd_init after > the drm_irqinstall calls in the gpu reset and resume paths - otherwise > hotplug will be broken. Also improve the comment a bit about why > hpd_init needs to be called before we set up the initial fbdev config. > > Bugzilla: Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54943 > Reported-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Looks ok: Reviewed-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/i915_dma.c | 15 +++++++++ > drivers/gpu/drm/i915/i915_drv.c | 2 ++ > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/i915_irq.c | 63 ++++++++++++++++++++++++++++++-------- > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_fb.c | 10 +++++- > 6 files changed, 79 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 9363066..0c2ab40 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1319,6 +1319,21 @@ static int i915_load_modeset_init(struct drm_device *dev) > goto cleanup_gem; > > /* Only enable hotplug handling once the fbdev is fully set up. */ > + intel_hpd_init(dev); > + > + /* > + * Some ports require correctly set-up hpd registers for detection to > + * work properly (leading to ghost connected connector status), e.g. VGA > + * on gm45. Hence we can only set up the initial fbdev config after hpd > + * irqs are fully enabled. Now we should scan for the initial config > + * only once hotplug handling is enabled, but due to screwed-up locking > + * around kms/fbdev init we can't protect the fdbev initial config > + * scanning against hotplug events. Hence do this first and ignore the > + * tiny window where we will loose hotplug notifactions. > + */ > + intel_fbdev_initial_config(dev); > + > + /* Only enable hotplug handling once the fbdev is fully set up. */ > dev_priv->enable_hotplug_processing = true; > > drm_kms_helper_poll_init(dev); > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index a129218..fbd0b28 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -566,6 +566,7 @@ static int __i915_drm_thaw(struct drm_device *dev) > intel_modeset_init_hw(dev); > intel_modeset_setup_hw_state(dev, false); > drm_irq_install(dev); > + intel_hpd_init(dev); > } > > intel_opregion_init(dev); > @@ -871,6 +872,7 @@ int i915_reset(struct drm_device *dev) > > drm_irq_uninstall(dev); > drm_irq_install(dev); > + intel_hpd_init(dev); > } else { > mutex_unlock(&dev->struct_mutex); > } > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index e9ac360..36a3bde 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -295,6 +295,7 @@ struct drm_i915_display_funcs { > struct drm_i915_gem_object *obj); > int (*update_plane)(struct drm_crtc *crtc, struct drm_framebuffer *fb, > int x, int y); > + void (*hpd_irq_setup)(struct drm_device *dev); > /* clock updates for mode set */ > /* cursor updates */ > /* render clock increase/decrease */ > @@ -1341,6 +1342,7 @@ void i915_hangcheck_elapsed(unsigned long data); > void i915_handle_error(struct drm_device *dev, bool wedged); > > extern void intel_irq_init(struct drm_device *dev); > +extern void intel_hpd_init(struct drm_device *dev); > extern void intel_gt_init(struct drm_device *dev); > extern void intel_gt_reset(struct drm_device *dev); > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 2e1d80d..de9947a 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1995,7 +1995,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev) > { > drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; > u32 enable_mask; > - u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN); > u32 pipestat_enable = PLANE_FLIP_DONE_INT_EN_VLV; > u32 render_irqs; > u16 msid; > @@ -2024,6 +2023,9 @@ static int valleyview_irq_postinstall(struct drm_device *dev) > msid |= (1<<14); > pci_write_config_word(dev_priv->dev->pdev, 0x98, msid); > > + I915_WRITE(PORT_HOTPLUG_EN, 0); > + POSTING_READ(PORT_HOTPLUG_EN); > + > I915_WRITE(VLV_IMR, dev_priv->irq_mask); > I915_WRITE(VLV_IER, enable_mask); > I915_WRITE(VLV_IIR, 0xffffffff); > @@ -2053,6 +2055,15 @@ static int valleyview_irq_postinstall(struct drm_device *dev) > #endif > > I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE); > + > + return 0; > +} > + > +static void valleyview_hpd_irq_setup(struct drm_device *dev) > +{ > + drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; > + u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN); > + > /* Note HDMI and DP share bits */ > if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS) > hotplug_en |= HDMIB_HOTPLUG_INT_EN; > @@ -2070,8 +2081,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev) > } > > I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); > - > - return 0; > } > > static void valleyview_irq_uninstall(struct drm_device *dev) > @@ -2301,6 +2310,9 @@ static int i915_irq_postinstall(struct drm_device *dev) > I915_USER_INTERRUPT; > > if (I915_HAS_HOTPLUG(dev)) { > + I915_WRITE(PORT_HOTPLUG_EN, 0); > + POSTING_READ(PORT_HOTPLUG_EN); > + > /* Enable in IER... */ > enable_mask |= I915_DISPLAY_PORT_INTERRUPT; > /* and unmask in IMR */ > @@ -2311,8 +2323,18 @@ static int i915_irq_postinstall(struct drm_device *dev) > I915_WRITE(IER, enable_mask); > POSTING_READ(IER); > > + intel_opregion_enable_asle(dev); > + > + return 0; > +} > + > +static void i915_hpd_irq_setup(struct drm_device *dev) > +{ > + drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; > + u32 hotplug_en; > + > if (I915_HAS_HOTPLUG(dev)) { > - u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN); > + hotplug_en = I915_READ(PORT_HOTPLUG_EN); > > if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS) > hotplug_en |= HDMIB_HOTPLUG_INT_EN; > @@ -2333,10 +2355,6 @@ static int i915_irq_postinstall(struct drm_device *dev) > > I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); > } > - > - intel_opregion_enable_asle(dev); > - > - return 0; > } > > static irqreturn_t i915_irq_handler(int irq, void *arg) > @@ -2496,7 +2514,6 @@ static void i965_irq_preinstall(struct drm_device * dev) > static int i965_irq_postinstall(struct drm_device *dev) > { > drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; > - u32 hotplug_en; > u32 enable_mask; > u32 error_mask; > > @@ -2538,6 +2555,19 @@ static int i965_irq_postinstall(struct drm_device *dev) > I915_WRITE(IER, enable_mask); > POSTING_READ(IER); > > + I915_WRITE(PORT_HOTPLUG_EN, 0); > + POSTING_READ(PORT_HOTPLUG_EN); > + > + intel_opregion_enable_asle(dev); > + > + return 0; > +} > + > +static void i965_hpd_irq_setup(struct drm_device *dev) > +{ > + drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; > + u32 hotplug_en; > + > /* Note HDMI and DP share hotplug bits */ > hotplug_en = 0; > if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS) > @@ -2572,10 +2602,6 @@ static int i965_irq_postinstall(struct drm_device *dev) > /* Ignore TV since it's buggy */ > > I915_WRITE(PORT_HOTPLUG_EN, hotplug_en); > - > - intel_opregion_enable_asle(dev); > - > - return 0; > } > > static irqreturn_t i965_irq_handler(int irq, void *arg) > @@ -2754,6 +2780,7 @@ void intel_irq_init(struct drm_device *dev) > dev->driver->irq_uninstall = valleyview_irq_uninstall; > dev->driver->enable_vblank = valleyview_enable_vblank; > dev->driver->disable_vblank = valleyview_disable_vblank; > + dev_priv->display.hpd_irq_setup = valleyview_hpd_irq_setup; > } else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) { > /* Share pre & uninstall handlers with ILK/SNB */ > dev->driver->irq_handler = ivybridge_irq_handler; > @@ -2780,13 +2807,23 @@ void intel_irq_init(struct drm_device *dev) > dev->driver->irq_postinstall = i915_irq_postinstall; > dev->driver->irq_uninstall = i915_irq_uninstall; > dev->driver->irq_handler = i915_irq_handler; > + dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; > } else { > dev->driver->irq_preinstall = i965_irq_preinstall; > dev->driver->irq_postinstall = i965_irq_postinstall; > dev->driver->irq_uninstall = i965_irq_uninstall; > dev->driver->irq_handler = i965_irq_handler; > + dev_priv->display.hpd_irq_setup = i965_hpd_irq_setup; > } > dev->driver->enable_vblank = i915_enable_vblank; > dev->driver->disable_vblank = i915_disable_vblank; > } > } > + > +void intel_hpd_init(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + if (dev_priv->display.hpd_irq_setup) > + dev_priv->display.hpd_irq_setup(dev); > +} > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 7ca7772..22728f2 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -587,6 +587,7 @@ extern int intel_framebuffer_init(struct drm_device *dev, > struct drm_mode_fb_cmd2 *mode_cmd, > struct drm_i915_gem_object *obj); > extern int intel_fbdev_init(struct drm_device *dev); > +extern void intel_fbdev_initial_config(struct drm_device *dev); > extern void intel_fbdev_fini(struct drm_device *dev); > extern void intel_fbdev_set_suspend(struct drm_device *dev, int state); > extern void intel_prepare_page_flip(struct drm_device *dev, int plane); > diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c > index b7773e5..82289678 100644 > --- a/drivers/gpu/drm/i915/intel_fb.c > +++ b/drivers/gpu/drm/i915/intel_fb.c > @@ -243,10 +243,18 @@ int intel_fbdev_init(struct drm_device *dev) > } > > drm_fb_helper_single_add_all_connectors(&ifbdev->helper); > - drm_fb_helper_initial_config(&ifbdev->helper, 32); > + > return 0; > } > > +void intel_fbdev_initial_config(struct drm_device *dev) > +{ > + drm_i915_private_t *dev_priv = dev->dev_private; > + > + /* Due to peculiar init order wrt to hpd handling this is separate. */ > + drm_fb_helper_initial_config(&dev_priv->fbdev->helper, 32); > +} > + > void intel_fbdev_fini(struct drm_device *dev) > { > drm_i915_private_t *dev_priv = dev->dev_private; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Fixup hpd irq register setup ordering 2012-12-11 15:58 ` Imre Deak @ 2012-12-11 16:24 ` Daniel Vetter 0 siblings, 0 replies; 10+ messages in thread From: Daniel Vetter @ 2012-12-11 16:24 UTC (permalink / raw) To: Imre Deak; +Cc: Daniel Vetter, Intel Graphics Development On Tue, Dec 11, 2012 at 05:58:48PM +0200, Imre Deak wrote: > On Tue, 2012-12-11 at 14:05 +0100, Daniel Vetter wrote: > > For GMCH platforms we set up the hpd irq registers in the irq > > postinstall hook. But since we only enable the irq sources we actually > > need in PORT_HOTPLUG_EN/STATUS, taking dev_priv->hotplug_supported_mask > > into account, no hpd interrupt sources is enabled since > > > > commit 52d7ecedac3f96fb562cb482c139015372728638 > > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > > Date: Sat Dec 1 21:03:22 2012 +0100 > > > > drm/i915: reorder setup sequence to have irqs for output setup > > > > Wrongly set-up interrupts also lead to broken hw-based load-detection > > on at least GM45, resulting in ghost VGA/TV-out outputs. > > > > To fix this, delay the hotplug register setup until after all outputs > > are set up, by moving it into a new dev_priv->display.hpd_irq_callback. > > We might also move the PCH_SPLIT platforms to such a setup eventually. > > > > Another funny part is that we need to delay the fbdev initial config > > probing until after the hpd regs are setup, for otherwise it'll detect > > ghost outputs. But we can only enable the hpd interrupt handling > > itself (and the output polling) _after_ that initial scan, due to > > massive locking brain-damage in the fbdev setup code. Add a big > > comment to explain this cute little dragon lair. > > > > v2: Encapsulate all the fbdev handling by wrapping the move call into > > intel_fbdev_initial_config in intel_fb.c. Requested by Chris Wilson. > > > > v3: Applied bikeshed from Jesse Barnes. > > > > v4: Imre Deak noticed that we also need to call intel_hpd_init after > > the drm_irqinstall calls in the gpu reset and resume paths - otherwise > > hotplug will be broken. Also improve the comment a bit about why > > hpd_init needs to be called before we set up the initial fbdev config. > > > > Bugzilla: Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54943 > > Reported-by: Chris Wilson <chris@chris-wilson.co.uk> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Looks ok: > Reviewed-by: Imre Deak <imre.deak@intel.com> Ok, patch merged again, thanks a lot for catching that resume/reset bug. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: Fixup hpd irq register setup ordering 2012-12-10 21:01 ` Jesse Barnes 2012-12-10 21:55 ` Daniel Vetter @ 2012-12-11 12:06 ` Daniel Vetter 1 sibling, 0 replies; 10+ messages in thread From: Daniel Vetter @ 2012-12-11 12:06 UTC (permalink / raw) To: Jesse Barnes; +Cc: Daniel Vetter, Intel Graphics Development On Mon, Dec 10, 2012 at 01:01:43PM -0800, Jesse Barnes wrote: > On Sun, 9 Dec 2012 20:42:01 +0100 > Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > > For GMCH platforms we set up the hpd irq registers in the irq > > postinstall hook. But since we only enable the irq sources we actually > > need in PORT_HOTPLUG_EN/STATUS, taking dev_priv->hotplug_supported_mask > > into account, no hpd interrupt sources is enabled since > > > > commit 52d7ecedac3f96fb562cb482c139015372728638 > > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > > Date: Sat Dec 1 21:03:22 2012 +0100 > > > > drm/i915: reorder setup sequence to have irqs for output setup > > > > Wrongly set-up interrupts also lead broken hw-based load-detection on > > at least GM45, resulting in ghost VGA/TV-out outputs. > > ..."can lead to broken"... or "also leads to" ? > > > + > > +void intel_hpd_init(struct drm_device *dev) > > +{ > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + > > + if (dev_priv->display.hpd_irq_setup) > > + dev_priv->display.hpd_irq_setup(dev); > > + > > + /* > > + * Some ports require correctly set-up hpd registers for detection to > > + * work properly, e.g. VGA on gm45. Hence we can only set up the initial > > + * fbdev config after hpd irqs are fully enabled. Now we should scan for > > + * the initial config only once hotplug handling is enabled, but due to > > + * screwed-up locking around kms/fbdev init we can't protect the fdbev > > + * initial config scanning against hotplug events. Hence do this first > > + * and ignore the tiny window where we will loose hotplug notifactions. > > + */ > > + intel_fbdev_initial_config(dev); > > + > > + /* Only enable hotplug handling once the fbdev is fully set up. */ > > + dev_priv->enable_hotplug_processing = true; > > + > > + drm_kms_helper_poll_init(dev); > > +} > > I'd rather see these calls in load_modeset_init(). Having > fbdev_initial_config here at least is confusing, and since poll_init > depends on it... > > With those two nits: > > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> Bikeshedded patch queued for -next, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-12-11 16:22 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-12-09 19:14 [PATCH] drm/i915: Fixup hpd irq register setup ordering Daniel Vetter 2012-12-09 19:17 ` Daniel Vetter 2012-12-09 19:42 ` Daniel Vetter 2012-12-10 13:40 ` Chris Wilson 2012-12-10 21:01 ` Jesse Barnes 2012-12-10 21:55 ` Daniel Vetter 2012-12-11 13:05 ` Daniel Vetter 2012-12-11 15:58 ` Imre Deak 2012-12-11 16:24 ` Daniel Vetter 2012-12-11 12:06 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).