[PATCH 00/10] gmbus/dp aux irqfication

[PATCH 00/10] gmbus/dp aux irqfication

[Daniel Vetter]

Hi all,

Irq-drive gmbus/dp aux transfer, simply because we can (and at least in the case
of gmbus, it's quite a bit faster than the msleep(1) loop - we now reliably
transfer at full wire speed insteaf of sometimes 2-3x slower).

Compared to the older version I've floated on irc way back and which Chris has
carried around in his wip branchs (and inflicted upon tons of unsuspecting bug
reporters) there are a few differences:
- handle the hpd vs. setup race - even the current code enables hpd processing
  before the fbdev is set up, which is too early.
- fix the gen4 gmbus support, was totally busted.
- tested on hsw (although all the relevant bits are a 100% match with cpt/ivb).
- disable dp aux irq on vlv - too complicated to get at the docs (moved again,
  old access revoked) and I don't have the hw.

While reviewing this patch series I've also noticed two small things in the irq
handling code in general, patches for that at the beginning of the series.

Comment&review highgly welcome.

Cheers, Daniel

Daniel Vetter (10):
  drm/i915: haswell has the same irq handlers as ivb
  drm/i915: don't handle PIPE_LEGACY_BLC_EVENT_STATUS on vlv
  drm/i915: reorder setup sequence to have irqs for output setup
  drm/i915: extract gmbus_wait_hw_status
  drm/i915: wire up gmbus irq handler
  drm/i915: use the gmbus irq for waits
  drm/i915: use gmbus irq to wait for gmbus idle
  drm/i915: wire up do aux channel done interrupt
  drm/i915: irq-drive the dp aux communication
  drm/i915: use _NOTRACE for gmbus/dp aux wait loops

 drivers/gpu/drm/i915/i915_dma.c  |  10 +++-
 drivers/gpu/drm/i915/i915_drv.h  |   8 ++++
 drivers/gpu/drm/i915/i915_irq.c  |  74 ++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_dp.c  |  59 ++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_i2c.c | 101 +++++++++++++++++++++++++++++----------
 5 files changed, 193 insertions(+), 59 deletions(-)

-- 
1.7.11.7

[PATCH 01/10] drm/i915: haswell has the same irq handlers as ivb

[Daniel Vetter]

No need to have the exaxt same code twice.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 26753ee..ff78818 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2706,7 +2706,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;
-	} else if (IS_IVYBRIDGE(dev)) {
+	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
 		/* Share pre & uninstall handlers with ILK/SNB */
 		dev->driver->irq_handler = ivybridge_irq_handler;
 		dev->driver->irq_preinstall = ironlake_irq_preinstall;
@@ -2714,14 +2714,6 @@ void intel_irq_init(struct drm_device *dev)
 		dev->driver->irq_uninstall = ironlake_irq_uninstall;
 		dev->driver->enable_vblank = ivybridge_enable_vblank;
 		dev->driver->disable_vblank = ivybridge_disable_vblank;
-	} else if (IS_HASWELL(dev)) {
-		/* Share interrupts handling with IVB */
-		dev->driver->irq_handler = ivybridge_irq_handler;
-		dev->driver->irq_preinstall = ironlake_irq_preinstall;
-		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
-		dev->driver->irq_uninstall = ironlake_irq_uninstall;
-		dev->driver->enable_vblank = ivybridge_enable_vblank;
-		dev->driver->disable_vblank = ivybridge_disable_vblank;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev->driver->irq_handler = ironlake_irq_handler;
 		dev->driver->irq_preinstall = ironlake_irq_preinstall;
-- 
1.7.11.7

[PATCH 02/10] drm/i915: don't handle PIPE_LEGACY_BLC_EVENT_STATUS on vlv

[Daniel Vetter]

This is for legacy legacy stuff, and checking with the leftover
pipe from the previous loop is propably not what we want. Since
pipe == 2 after the loop ...

Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ff78818..2028137 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -533,7 +533,6 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 	unsigned long irqflags;
 	int pipe;
 	u32 pipe_stats[I915_MAX_PIPES];
-	bool blc_event;
 
 	atomic_inc(&dev_priv->irq_received);
 
@@ -590,9 +589,6 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 			I915_READ(PORT_HOTPLUG_STAT);
 		}
 
-		if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
-			blc_event = true;
-
 		if (pm_iir & GEN6_PM_DEFERRED_EVENTS)
 			gen6_queue_rps_work(dev_priv, pm_iir);
 
-- 
1.7.11.7

Re: [PATCH 02/10] drm/i915: don't handle PIPE_LEGACY_BLC_EVENT_STATUS on vlv

[Jesse Barnes]

On Sat,  1 Dec 2012 13:53:41 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> This is for legacy legacy stuff, and checking with the leftover
> pipe from the previous loop is propably not what we want. Since
> pipe == 2 after the loop ...
> 
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index ff78818..2028137 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -533,7 +533,6 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>  	unsigned long irqflags;
>  	int pipe;
>  	u32 pipe_stats[I915_MAX_PIPES];
> -	bool blc_event;
>  
>  	atomic_inc(&dev_priv->irq_received);
>  
> @@ -590,9 +589,6 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>  			I915_READ(PORT_HOTPLUG_STAT);
>  		}
>  
> -		if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
> -			blc_event = true;
> -
>  		if (pm_iir & GEN6_PM_DEFERRED_EVENTS)
>  			gen6_queue_rps_work(dev_priv, pm_iir);
>  

I think this is ok; I don't think hotkeys will be handled this way on
VLV platforms.  And if it is, the check will need to be in the per pipe
loop anyway!

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 02/10] drm/i915: don't handle PIPE_LEGACY_BLC_EVENT_STATUS on vlv

[Imre Deak]

On Sat, 2012-12-01 at 13:53 +0100, Daniel Vetter wrote:
> This is for legacy legacy stuff, and checking with the leftover
> pipe from the previous loop is propably not what we want. Since
> pipe == 2 after the loop ...

This doesn't seem to match the change, blc_event is simply never used
here.

> 
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index ff78818..2028137 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -533,7 +533,6 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>  	unsigned long irqflags;
>  	int pipe;
>  	u32 pipe_stats[I915_MAX_PIPES];
> -	bool blc_event;
>  
>  	atomic_inc(&dev_priv->irq_received);
>  
> @@ -590,9 +589,6 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>  			I915_READ(PORT_HOTPLUG_STAT);
>  		}
>  
> -		if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
> -			blc_event = true;
> -
>  		if (pm_iir & GEN6_PM_DEFERRED_EVENTS)
>  			gen6_queue_rps_work(dev_priv, pm_iir);
>  

Re: [PATCH 02/10] drm/i915: don't handle PIPE_LEGACY_BLC_EVENT_STATUS on vlv

[Daniel Vetter]

On Tue, Dec 4, 2012 at 3:37 PM, Imre Deak <imre.deak@intel.com> wrote:
> On Sat, 2012-12-01 at 13:53 +0100, Daniel Vetter wrote:
>> This is for legacy legacy stuff, and checking with the leftover
>> pipe from the previous loop is propably not what we want. Since
>> pipe == 2 after the loop ...
>
> This doesn't seem to match the change, blc_event is simply never used
> here.

Oops, indeed. Somehow I've thought there's the function call right
there. Will adjust the commit message a bit when applying.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 03/10] drm/i915: reorder setup sequence to have irqs for output setup

[Daniel Vetter]

Otherwise the new&shiny irq-driven gmbus and dp aux code won't work that
well. Noticed since the dp aux code doesn't have an automatic fallback
with a timeout (since the hw provides for that already).

v2: Simple move drm_irq_install before intel_modeset_gem_init, as
suggested by Ben Widawsky.

v3: Now that interrupts are enabled before all connectors are fully
set up, we might fall over serving a HPD interrupt while things are
still being set up. Instead of jumping through massive hoops and
complicating the code with a separate hpd irq enable step, simply
block out the hotplug work item from doing anything until things are
in place.

v4: Actually, we can enable hotplug processing only after the fbdev is
fully set up, since we call down into the fbdev from the hotplug work
functions. So stick the hpd enabling right next to the poll helper
initialization.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_dma.c | 9 +++++++--
 drivers/gpu/drm/i915/i915_drv.h | 1 +
 drivers/gpu/drm/i915/i915_irq.c | 4 ++++
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 80ed751..1b0b6c5 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1300,14 +1300,16 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		goto cleanup_gem_stolen;
 
-	intel_modeset_gem_init(dev);
-
 	INIT_WORK(&dev_priv->console_resume_work, intel_console_resume);
 
 	ret = drm_irq_install(dev);
 	if (ret)
 		goto cleanup_gem;
 
+	/* Important: The output setup functions called by modeset_gem_init need
+	 * working irqs for e.g. gmbus transfers. */
+	intel_modeset_gem_init(dev);
+
 	/* Always safe in the mode setting case. */
 	/* FIXME: do pre/post-mode set stuff in core KMS code */
 	dev->vblank_disable_allowed = 1;
@@ -1316,6 +1318,9 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		goto cleanup_irq;
 
+	/* Only enable hotplug handling once the fbdev is fully set up. */
+	dev_priv->enable_hotplug_processing = true;
+
 	drm_kms_helper_poll_init(dev);
 
 	/* We're off and running w/KMS */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 31ab43b..9e05d98 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -664,6 +664,7 @@ typedef struct drm_i915_private {
 
 	u32 hotplug_supported_mask;
 	struct work_struct hotplug_work;
+	bool enable_hotplug_processing;
 
 	int num_pipe;
 	int num_pch_pll;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2028137..6ba94db 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -287,6 +287,10 @@ static void i915_hotplug_work_func(struct work_struct *work)
 	struct drm_mode_config *mode_config = &dev->mode_config;
 	struct intel_encoder *encoder;
 
+	/* HPD irq before everything is fully set up. */
+	if (!dev_priv->enable_hotplug_processing)
+		return;
+
 	mutex_lock(&mode_config->mutex);
 	DRM_DEBUG_KMS("running encoder hotplug functions\n");
 
-- 
1.7.11.7

[PATCH 1/2] drm/i915: setup the hangcheck timer early

[Daniel Vetter]

... together with all the other irq related resources in
intel_irq_init. I've managed to oops in the notify_ring function on my
ilk, presumably because of the powerctx setup call to i915_gpu_idle.

Note that this is only a problem with the reorder irq setup sequence
for irq-driver gmbus/dp aux.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_dma.c | 3 ---
 drivers/gpu/drm/i915/i915_irq.c | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 80ed751..2e8b8cf 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1611,9 +1611,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	intel_opregion_init(dev);
 	acpi_video_register();
 
-	setup_timer(&dev_priv->hangcheck_timer, i915_hangcheck_elapsed,
-		    (unsigned long) dev);
-
 	if (IS_GEN5(dev))
 		intel_gpu_ips_init(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2028137..30a2fb5 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2682,6 +2682,9 @@ void intel_irq_init(struct drm_device *dev)
 	INIT_WORK(&dev_priv->rps.work, gen6_pm_rps_work);
 	INIT_WORK(&dev_priv->l3_parity.error_work, ivybridge_parity_work);
 
+	setup_timer(&dev_priv->hangcheck_timer, i915_hangcheck_elapsed,
+		    (unsigned long) dev);
+
 	dev->driver->get_vblank_counter = i915_get_vblank_counter;
 	dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
 	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
-- 
1.7.11.7

[PATCH 2/2] drm/i915: reorder setup sequence to have irqs for output setup

[Daniel Vetter]

Otherwise the new&shiny irq-driven gmbus and dp aux code won't work that
well. Noticed since the dp aux code doesn't have an automatic fallback
with a timeout (since the hw provides for that already).

v2: Simple move drm_irq_install before intel_modeset_gem_init, as
suggested by Ben Widawsky.

v3: Now that interrupts are enabled before all connectors are fully
set up, we might fall over serving a HPD interrupt while things are
still being set up. Instead of jumping through massive hoops and
complicating the code with a separate hpd irq enable step, simply
block out the hotplug work item from doing anything until things are
in place.

v4: Actually, we can enable hotplug processing only after the fbdev is
fully set up, since we call down into the fbdev from the hotplug work
functions. So stick the hpd enabling right next to the poll helper
initialization.

v5: We need to enable irqs before intel_modeset_init, since that
function sets up the outputs.

v6: Fixup cleanup sequence, too.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_dma.c | 23 ++++++++++++++---------
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_irq.c |  4 ++++
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2e8b8cf..644481c 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1294,19 +1294,21 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		goto cleanup_vga_switcheroo;
 
+	ret = drm_irq_install(dev);
+	if (ret)
+		goto cleanup_gem_stolen;
+
+	/* Important: The output setup functions called by modeset_init need
+	 * working irqs for e.g. gmbus and dp aux transfers. */
 	intel_modeset_init(dev);
 
 	ret = i915_gem_init(dev);
 	if (ret)
-		goto cleanup_gem_stolen;
-
-	intel_modeset_gem_init(dev);
+		goto cleanup_irq;
 
 	INIT_WORK(&dev_priv->console_resume_work, intel_console_resume);
 
-	ret = drm_irq_install(dev);
-	if (ret)
-		goto cleanup_gem;
+	intel_modeset_gem_init(dev);
 
 	/* Always safe in the mode setting case. */
 	/* FIXME: do pre/post-mode set stuff in core KMS code */
@@ -1314,7 +1316,10 @@ static int i915_load_modeset_init(struct drm_device *dev)
 
 	ret = intel_fbdev_init(dev);
 	if (ret)
-		goto cleanup_irq;
+		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);
 
@@ -1323,13 +1328,13 @@ static int i915_load_modeset_init(struct drm_device *dev)
 
 	return 0;
 
-cleanup_irq:
-	drm_irq_uninstall(dev);
 cleanup_gem:
 	mutex_lock(&dev->struct_mutex);
 	i915_gem_cleanup_ringbuffer(dev);
 	mutex_unlock(&dev->struct_mutex);
 	i915_gem_cleanup_aliasing_ppgtt(dev);
+cleanup_irq:
+	drm_irq_uninstall(dev);
 cleanup_gem_stolen:
 	i915_gem_cleanup_stolen(dev);
 cleanup_vga_switcheroo:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 31ab43b..9e05d98 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -664,6 +664,7 @@ typedef struct drm_i915_private {
 
 	u32 hotplug_supported_mask;
 	struct work_struct hotplug_work;
+	bool enable_hotplug_processing;
 
 	int num_pipe;
 	int num_pch_pll;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 30a2fb5..712b088 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -287,6 +287,10 @@ static void i915_hotplug_work_func(struct work_struct *work)
 	struct drm_mode_config *mode_config = &dev->mode_config;
 	struct intel_encoder *encoder;
 
+	/* HPD irq before everything is fully set up. */
+	if (!dev_priv->enable_hotplug_processing)
+		return;
+
 	mutex_lock(&mode_config->mutex);
 	DRM_DEBUG_KMS("running encoder hotplug functions\n");
 
-- 
1.7.11.7

[PATCH 04/10] drm/i915: extract gmbus_wait_hw_status

[Daniel Vetter]

The gmbus interrupt generation is rather fiddly: We can only ever
enable one interrupt source (but we always want to check for NAK
in addition to the real bit). And the bits in the gmbus status
register don't map at all to the bis in the irq register.

To prepare for this mess, start by extracting the hw status wait
loop into it's own function, consolidate the NAK error handling a
bit. To keep things flexible, pass in the status bit we care about
(in addition to any NAK signalling).

v2: I've failed to notice that the sens of GMBUS_ACTIVE is inverted,
Chris Wilson gladly pointed that out for me. To keep things simple,
ignore that case for  now (we only need to idle the gmbus controller
at the end of an entire i2c transaction, not after every message).

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_i2c.c | 46 ++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 3ef5af1..a16eecd 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -203,6 +203,24 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
 }
 
 static int
+gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
+		     u32 gmbus2_status)
+{
+	int ret;
+	int reg_offset = dev_priv->gpio_mmio_base;
+	u32 gmbus2;
+
+	ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) &
+		       (GMBUS_SATOER | gmbus2_status),
+		       50);
+
+	if (gmbus2 & GMBUS_SATOER)
+		return -ENXIO;
+
+	return ret;
+}
+
+static int
 gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
 		u32 gmbus1_index)
 {
@@ -219,15 +237,10 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
 	while (len) {
 		int ret;
 		u32 val, loop = 0;
-		u32 gmbus2;
 
-		ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) &
-			       (GMBUS_SATOER | GMBUS_HW_RDY),
-			       50);
+		ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY);
 		if (ret)
-			return -ETIMEDOUT;
-		if (gmbus2 & GMBUS_SATOER)
-			return -ENXIO;
+			return ret;
 
 		val = I915_READ(GMBUS3 + reg_offset);
 		do {
@@ -261,7 +274,6 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg)
 		   GMBUS_SLAVE_WRITE | GMBUS_SW_RDY);
 	while (len) {
 		int ret;
-		u32 gmbus2;
 
 		val = loop = 0;
 		do {
@@ -270,13 +282,9 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg)
 
 		I915_WRITE(GMBUS3 + reg_offset, val);
 
-		ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) &
-			       (GMBUS_SATOER | GMBUS_HW_RDY),
-			       50);
+		ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY);
 		if (ret)
-			return -ETIMEDOUT;
-		if (gmbus2 & GMBUS_SATOER)
-			return -ENXIO;
+			return ret;
 	}
 	return 0;
 }
@@ -345,8 +353,6 @@ gmbus_xfer(struct i2c_adapter *adapter,
 	I915_WRITE(GMBUS0 + reg_offset, bus->reg0);
 
 	for (i = 0; i < num; i++) {
-		u32 gmbus2;
-
 		if (gmbus_is_index_read(msgs, i, num)) {
 			ret = gmbus_xfer_index_read(dev_priv, &msgs[i]);
 			i += 1;  /* set i to the index of the read xfer */
@@ -361,13 +367,11 @@ gmbus_xfer(struct i2c_adapter *adapter,
 		if (ret == -ENXIO)
 			goto clear_err;
 
-		ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) &
-			       (GMBUS_SATOER | GMBUS_HW_WAIT_PHASE),
-			       50);
+		ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_WAIT_PHASE);
+		if (ret == -ENXIO)
+			goto clear_err;
 		if (ret)
 			goto timeout;
-		if (gmbus2 & GMBUS_SATOER)
-			goto clear_err;
 	}
 
 	/* Generate a STOP condition on the bus. Note that gmbus can't generata
-- 
1.7.11.7

[PATCH 05/10] drm/i915: wire up gmbus irq handler

[Daniel Vetter]

Only enables the interrupt and puts a irq handler into place, doesn't
do anything yet.

Unfortunately there's no gmbus interrupt support for gen2/3 (safe for
pnv, but there the irq is marked as "Test mode").

v2: Wire up the irq handler for vlv and gen4 properly.

v3: i915_enable_pipestat expects the mask bit, not the status bits ... and
for added hilarity those are rather inconsistently named.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6ba94db..5062501 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -528,6 +528,11 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv,
 	queue_work(dev_priv->wq, &dev_priv->rps.work);
 }
 
+static void gmbus_irq_handler(struct drm_device *dev)
+{
+	DRM_DEBUG_DRIVER("GMBUS interrupt\n");
+}
+
 static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *) arg;
@@ -593,6 +598,9 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 			I915_READ(PORT_HOTPLUG_STAT);
 		}
 
+		if (pipe_stats[0] & PIPE_GMBUS_INTERRUPT_STATUS)
+			gmbus_irq_handler(dev);
+
 		if (pm_iir & GEN6_PM_DEFERRED_EVENTS)
 			gen6_queue_rps_work(dev_priv, pm_iir);
 
@@ -619,7 +627,7 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
 				 SDE_AUDIO_POWER_SHIFT);
 
 	if (pch_iir & SDE_GMBUS)
-		DRM_DEBUG_DRIVER("PCH GMBUS interrupt\n");
+		gmbus_irq_handler(dev);
 
 	if (pch_iir & SDE_AUDIO_HDCP_MASK)
 		DRM_DEBUG_DRIVER("PCH HDCP audio interrupt\n");
@@ -665,7 +673,7 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
 		DRM_DEBUG_DRIVER("AUX channel interrupt\n");
 
 	if (pch_iir & SDE_GMBUS_CPT)
-		DRM_DEBUG_DRIVER("PCH GMBUS interrupt\n");
+		gmbus_irq_handler(dev);
 
 	if (pch_iir & SDE_AUDIO_CP_REQ_CPT)
 		DRM_DEBUG_DRIVER("Audio CP request interrupt\n");
@@ -1875,12 +1883,14 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 		hotplug_mask = (SDE_CRT_HOTPLUG_CPT |
 				SDE_PORTB_HOTPLUG_CPT |
 				SDE_PORTC_HOTPLUG_CPT |
-				SDE_PORTD_HOTPLUG_CPT);
+				SDE_PORTD_HOTPLUG_CPT |
+				SDE_GMBUS_CPT);
 	} else {
 		hotplug_mask = (SDE_CRT_HOTPLUG |
 				SDE_PORTB_HOTPLUG |
 				SDE_PORTC_HOTPLUG |
 				SDE_PORTD_HOTPLUG |
+				SDE_GMBUS |
 				SDE_AUX_MASK);
 	}
 
@@ -1940,7 +1950,8 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
 	hotplug_mask = (SDE_CRT_HOTPLUG_CPT |
 			SDE_PORTB_HOTPLUG_CPT |
 			SDE_PORTC_HOTPLUG_CPT |
-			SDE_PORTD_HOTPLUG_CPT);
+			SDE_PORTD_HOTPLUG_CPT |
+			SDE_GMBUS_CPT);
 	dev_priv->pch_irq_mask = ~hotplug_mask;
 
 	I915_WRITE(SDEIIR, I915_READ(SDEIIR));
@@ -1994,6 +2005,7 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
 	POSTING_READ(VLV_IER);
 
 	i915_enable_pipestat(dev_priv, 0, pipestat_enable);
+	i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_EVENT_ENABLE);
 	i915_enable_pipestat(dev_priv, 1, pipestat_enable);
 
 	I915_WRITE(VLV_IIR, 0xffffffff);
@@ -2478,6 +2490,7 @@ static int i965_irq_postinstall(struct drm_device *dev)
 
 	dev_priv->pipestat[0] = 0;
 	dev_priv->pipestat[1] = 0;
+	i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_EVENT_ENABLE);
 
 	/*
 	 * Enable some error detection, note the instruction error mask
@@ -2631,6 +2644,9 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 		if (blc_event || (iir & I915_ASLE_INTERRUPT))
 			intel_opregion_asle_intr(dev);
 
+		if (pipe_stats[0] & PIPE_GMBUS_INTERRUPT_STATUS)
+			gmbus_irq_handler(dev);
+
 		/* With MSI, interrupts are only generated when iir
 		 * transitions from zero to nonzero.  If another bit got
 		 * set while we were handling the existing iir bits, then
-- 
1.7.11.7

[PATCH 06/10] drm/i915: use the gmbus irq for waits

[Daniel Vetter]

We need two special things to properly wire this up:
- Add another argument to gmbus_wait_hw_status to pass in the
  correct interrupt bit in gmbus4.
- Since we can only get an irq for one of the two events we want,
  hand-roll the wait_event_timeout code so that we wake up every
  jiffie and can check for NAKs. This way we also subsume gmbus
  support for platforms without interrupts (or where those are not
  yet enabled).

The important bit really is to only enable one gmbus interrupt source
at the same time - with that piece of lore figured out, this seems to
work flawlessly.

Ben Widawsky rightfully complained the lack of measurements for the
claimed benefits (especially since the first version was actually
broken and fell back to bit-banging). Previously reading the 256 byte
hdmi EDID takes about 72 ms here. With this patch it's down to 33 ms.
Given that transfering the 256 bytes over i2c at wire speed takes
20.5ms alone, the reduction in additional overhead is rather nice.

v2: Chris Wilson wondered whether GMBUS4 might contain some set bits
when booting up an hence result in some spurious interrupts. Since we
clear GMBUS4 after every wait and we do gmbus transfer really early in
the setup sequence to detect displays the window is small, but still
be paranoid and clear it properly.

v3: Clarify the comment that gmbus irq generation can only support one
kind of event, why it bothers us and how we work around that limit.

Cc: Daniel Kurtz <djkurtz@chromium.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h  |  3 +++
 drivers/gpu/drm/i915/i915_irq.c  |  4 ++++
 drivers/gpu/drm/i915/intel_i2c.c | 45 ++++++++++++++++++++++++++++++----------
 3 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9e05d98..8e9f292 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -632,6 +632,7 @@ typedef struct drm_i915_private {
 
 	struct intel_gmbus gmbus[GMBUS_NUM_PORTS];
 
+
 	/** gmbus_mutex protects against concurrent usage of the single hw gmbus
 	 * controller on different i2c buses. */
 	struct mutex gmbus_mutex;
@@ -641,6 +642,8 @@ typedef struct drm_i915_private {
 	 */
 	uint32_t gpio_mmio_base;
 
+	wait_queue_head_t gmbus_wait_queue;
+
 	struct pci_dev *bridge_dev;
 	struct intel_ring_buffer ring[I915_NUM_RINGS];
 	uint32_t next_seqno;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5062501..6fe62d0 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -530,7 +530,11 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv,
 
 static void gmbus_irq_handler(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = (drm_i915_private_t *) dev->dev_private;
+
 	DRM_DEBUG_DRIVER("GMBUS interrupt\n");
+
+	wake_up_all(&dev_priv->gmbus_wait_queue);
 }
 
 static irqreturn_t valleyview_irq_handler(int irq, void *arg)
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index a16eecd..8b71892 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -63,6 +63,7 @@ intel_i2c_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	I915_WRITE(dev_priv->gpio_mmio_base + GMBUS0, 0);
+	I915_WRITE(dev_priv->gpio_mmio_base + GMBUS4, 0);
 }
 
 static void intel_i2c_quirk_set(struct drm_i915_private *dev_priv, bool enable)
@@ -204,20 +205,38 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
 
 static int
 gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
-		     u32 gmbus2_status)
+		     u32 gmbus2_status,
+		     u32 gmbus4_irq_en)
 {
-	int ret;
+	int i;
 	int reg_offset = dev_priv->gpio_mmio_base;
-	u32 gmbus2;
+	u32 gmbus2 = 0;
+	DEFINE_WAIT(wait);
+
+	/* Important: The hw handles only the first bit, so set only one! Since
+	 * we also need to check for NAKs besides the hw ready/idle signal, we
+	 * need to wake up periodically and check that ourselves. */
+	I915_WRITE(GMBUS4 + reg_offset, gmbus4_irq_en);
 
-	ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) &
-		       (GMBUS_SATOER | gmbus2_status),
-		       50);
+	for (i = 0; i < msecs_to_jiffies(50) + 1; i++) {
+		prepare_to_wait(&dev_priv->gmbus_wait_queue, &wait,
+				TASK_UNINTERRUPTIBLE);
+
+		gmbus2 = I915_READ(GMBUS2 + reg_offset);
+		if (gmbus2 & (GMBUS_SATOER | gmbus2_status))
+			break;
+
+		schedule_timeout(1);
+	}
+	finish_wait(&dev_priv->gmbus_wait_queue, &wait);
+
+	I915_WRITE(GMBUS4 + reg_offset, 0);
 
 	if (gmbus2 & GMBUS_SATOER)
 		return -ENXIO;
-
-	return ret;
+	if (gmbus2 & gmbus2_status)
+		return 0;
+	return -ETIMEDOUT;
 }
 
 static int
@@ -238,7 +257,8 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
 		int ret;
 		u32 val, loop = 0;
 
-		ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY);
+		ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY,
+					   GMBUS_HW_RDY_EN);
 		if (ret)
 			return ret;
 
@@ -282,7 +302,8 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg)
 
 		I915_WRITE(GMBUS3 + reg_offset, val);
 
-		ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY);
+		ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_RDY,
+					   GMBUS_HW_RDY_EN);
 		if (ret)
 			return ret;
 	}
@@ -367,7 +388,8 @@ gmbus_xfer(struct i2c_adapter *adapter,
 		if (ret == -ENXIO)
 			goto clear_err;
 
-		ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_WAIT_PHASE);
+		ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_WAIT_PHASE,
+					   GMBUS_HW_WAIT_EN);
 		if (ret == -ENXIO)
 			goto clear_err;
 		if (ret)
@@ -473,6 +495,7 @@ int intel_setup_gmbus(struct drm_device *dev)
 		dev_priv->gpio_mmio_base = 0;
 
 	mutex_init(&dev_priv->gmbus_mutex);
+	init_waitqueue_head(&dev_priv->gmbus_wait_queue);
 
 	for (i = 0; i < GMBUS_NUM_PORTS; i++) {
 		struct intel_gmbus *bus = &dev_priv->gmbus[i];
-- 
1.7.11.7

[PATCH 07/10] drm/i915: use gmbus irq to wait for gmbus idle

[Daniel Vetter]

GMBUS_ACTIVE has inverted sense and so doesn't fit into the
wait_hw_status helper, hence create a new gmbus_wait_idle functions.
Also, we only care about the idle irq event and nothing else, which
allows us to use the wait_event_timeout helper directly without
jumping through hoops to catch NAKs.

Since gen2/3 don't have gmbus interrupts, handle them separately with
the old wait_for macro.

This shaves another few ms off reading EDID from a hdmi screen on my
testbox here. EDID reading with interrupt driven gmbus is now as fast
as with busy-looping gmbus at 28 ms here (with negligible cpu
overhead).

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_i2c.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 8b71892..1fc3119 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -203,6 +203,7 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin)
 	algo->data = bus;
 }
 
+#define HAS_GMBUS_IRQ(dev) (INTEL_INFO(dev)->gen >= 4)
 static int
 gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
 		     u32 gmbus2_status,
@@ -240,6 +241,31 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
 }
 
 static int
+gmbus_wait_idle(struct drm_i915_private *dev_priv)
+{
+	int ret;
+	int reg_offset = dev_priv->gpio_mmio_base;
+
+#define C ((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0)
+
+	if (!HAS_GMBUS_IRQ(dev_priv->dev))
+		return wait_for(C, 10);
+
+	/* Important: The hw handles only the first bit, so set only one! */
+	I915_WRITE(GMBUS4 + reg_offset, GMBUS_IDLE_EN);
+
+	ret = wait_event_timeout(dev_priv->gmbus_wait_queue, C, 10);
+
+	I915_WRITE(GMBUS4 + reg_offset, 0);
+
+	if (ret)
+		return 0;
+	else
+		return -ETIMEDOUT;
+#undef C
+}
+
+static int
 gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
 		u32 gmbus1_index)
 {
@@ -406,8 +432,7 @@ gmbus_xfer(struct i2c_adapter *adapter,
 	 * We will re-enable it at the start of the next xfer,
 	 * till then let it sleep.
 	 */
-	if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0,
-		     10)) {
+	if (gmbus_wait_idle(dev_priv)) {
 		DRM_DEBUG_KMS("GMBUS [%s] timed out waiting for idle\n",
 			 adapter->name);
 		ret = -ETIMEDOUT;
@@ -431,8 +456,7 @@ clear_err:
 	 * it's slow responding and only answers on the 2nd retry.
 	 */
 	ret = -ENXIO;
-	if (wait_for((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0,
-		     10)) {
+	if (gmbus_wait_idle(dev_priv)) {
 		DRM_DEBUG_KMS("GMBUS [%s] timed out after NAK\n",
 			      adapter->name);
 		ret = -ETIMEDOUT;
-- 
1.7.11.7

[PATCH 08/10] drm/i915: wire up do aux channel done interrupt

[Daniel Vetter]

Doesn't do anything yet than call dp_aux_irq_handler.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6fe62d0..ec44640 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -537,6 +537,11 @@ static void gmbus_irq_handler(struct drm_device *dev)
 	wake_up_all(&dev_priv->gmbus_wait_queue);
 }
 
+static void dp_aux_irq_handler(struct drm_device *dev)
+{
+	DRM_DEBUG_DRIVER("AUX channel interrupt\n");
+}
+
 static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *) arg;
@@ -630,6 +635,9 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
 				 (pch_iir & SDE_AUDIO_POWER_MASK) >>
 				 SDE_AUDIO_POWER_SHIFT);
 
+	if (pch_iir & SDE_AUX_MASK)
+		dp_aux_irq_handler(dev);
+
 	if (pch_iir & SDE_GMBUS)
 		gmbus_irq_handler(dev);
 
@@ -674,7 +682,7 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
 				 SDE_AUDIO_POWER_SHIFT_CPT);
 
 	if (pch_iir & SDE_AUX_MASK_CPT)
-		DRM_DEBUG_DRIVER("AUX channel interrupt\n");
+		dp_aux_irq_handler(dev);
 
 	if (pch_iir & SDE_GMBUS_CPT)
 		gmbus_irq_handler(dev);
@@ -715,6 +723,9 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 
 	de_iir = I915_READ(DEIIR);
 	if (de_iir) {
+		if (de_iir & DE_AUX_CHANNEL_A_IVB)
+			dp_aux_irq_handler(dev);
+
 		if (de_iir & DE_GSE_IVB)
 			intel_opregion_gse_intr(dev);
 
@@ -793,6 +804,9 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	else
 		snb_gt_irq_handler(dev, dev_priv, gt_iir);
 
+	if (de_iir & DE_AUX_CHANNEL_A)
+		dp_aux_irq_handler(dev);
+
 	if (de_iir & DE_GSE)
 		intel_opregion_gse_intr(dev);
 
@@ -1853,7 +1867,8 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	/* enable kind of interrupts always enabled */
 	u32 display_mask = DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
-			   DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE;
+			   DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
+			   DE_AUX_CHANNEL_A;
 	u32 render_irqs;
 	u32 hotplug_mask;
 
@@ -1888,7 +1903,8 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 				SDE_PORTB_HOTPLUG_CPT |
 				SDE_PORTC_HOTPLUG_CPT |
 				SDE_PORTD_HOTPLUG_CPT |
-				SDE_GMBUS_CPT);
+				SDE_GMBUS_CPT |
+				SDE_AUX_MASK_CPT);
 	} else {
 		hotplug_mask = (SDE_CRT_HOTPLUG |
 				SDE_PORTB_HOTPLUG |
@@ -1925,7 +1941,8 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
 		DE_MASTER_IRQ_CONTROL | DE_GSE_IVB | DE_PCH_EVENT_IVB |
 		DE_PLANEC_FLIP_DONE_IVB |
 		DE_PLANEB_FLIP_DONE_IVB |
-		DE_PLANEA_FLIP_DONE_IVB;
+		DE_PLANEA_FLIP_DONE_IVB |
+		DE_AUX_CHANNEL_A_IVB;
 	u32 render_irqs;
 	u32 hotplug_mask;
 
@@ -1955,7 +1972,8 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
 			SDE_PORTB_HOTPLUG_CPT |
 			SDE_PORTC_HOTPLUG_CPT |
 			SDE_PORTD_HOTPLUG_CPT |
-			SDE_GMBUS_CPT);
+			SDE_GMBUS_CPT |
+			SDE_AUX_MASK_CPT);
 	dev_priv->pch_irq_mask = ~hotplug_mask;
 
 	I915_WRITE(SDEIIR, I915_READ(SDEIIR));
-- 
1.7.11.7

[PATCH 09/10] drm/i915: irq-drive the dp aux communication

[Daniel Vetter]

At least on the platforms that have a dp aux irq and also have it
enabled - vlv/hsw should have one, too. But I don't have a machine to
test this on, and the current code doesn't support dp yet anyway on
those platforms. Judging from docs there's no dp aux interrupt for gm45.

Also, I only have an ivb cpu edp machine, so the dp aux A code for
snb/ilk is untested.

For dpcd probing when nothing is connected it slashes about 5ms of cpu
time (cpu time is now negligible), which agrees with 3 * 5 400 usec
timeouts.

A previous version of this patch increases the time required to go
through the dp_detect cycle (which includes reading the edid) from
around 33 ms to around 40 ms. Experiments indicated that this is
purely due to the irq latency - the hw doesn't allow us to queue up
dp aux transactions and hence irq latency directly affects throughput.
gmbus is much better, there we have a 8 byte buffer, and we get the
irq once another 4 bytes can be queued up.

But by using the pm_qos interface to request the lowest possible cpu
wake-up latency this slowdown completely disappeared.

Since all our output detection logic is single-threaded with the
mode_config mutex right now anyway, I've decide not ot play fancy and
to just reuse the gmbus wait queue. But this would definitely prep the
way to run dp detection on different ports in parallel

v2: Add a timeout for dp aux transfers when using interrupts - the hw
_does_  prevent this with the hw-based 400 usec timeout, but if the
irq somehow doesn't arrive we're screwed. Lesson learned while
developing this ;-)

v3: While at it also convert the busy-loop to wait_for_atomic, so that
we don't run the risk of an infinite loop any more.

v4: Ensure we have the smallest possible irq latency by using the
pm_qos interface.

v5: Add a comment to the code to explain why we frob pm_qos. Suggested
by Chris Wilson.

v6: Disable dp irq for vlv, that's easier than trying to get at docs
and hw.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_dma.c |  1 +
 drivers/gpu/drm/i915/i915_drv.h |  4 +++
 drivers/gpu/drm/i915/i915_irq.c |  6 +++++
 drivers/gpu/drm/i915/intel_dp.c | 59 ++++++++++++++++++++++++++++++++---------
 4 files changed, 57 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 1b0b6c5..61aeb13 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1740,6 +1740,7 @@ int i915_driver_unload(struct drm_device *dev)
 	intel_teardown_mchbar(dev);
 
 	destroy_workqueue(dev_priv->wq);
+	pm_qos_remove_request(&dev_priv->pm_qos);
 
 	pci_dev_put(dev_priv->bridge_dev);
 	kfree(dev->dev_private);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8e9f292..3afbad3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -40,6 +40,7 @@
 #include <linux/backlight.h>
 #include <linux/intel-iommu.h>
 #include <linux/kref.h>
+#include <linux/pm_qos.h>
 
 /* General customization:
  */
@@ -656,6 +657,9 @@ typedef struct drm_i915_private {
 	/* protects the irq masks */
 	spinlock_t irq_lock;
 
+	/* To control wakeup latency, e.g. for irq-driven dp aux transfers. */
+	struct pm_qos_request pm_qos;
+
 	/* DPIO indirect register protection */
 	spinlock_t dpio_lock;
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ec44640..311694c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -539,7 +539,11 @@ static void gmbus_irq_handler(struct drm_device *dev)
 
 static void dp_aux_irq_handler(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = (drm_i915_private_t *) dev->dev_private;
+
 	DRM_DEBUG_DRIVER("AUX channel interrupt\n");
+
+	wake_up_all(&dev_priv->gmbus_wait_queue);
 }
 
 static irqreturn_t valleyview_irq_handler(int irq, void *arg)
@@ -2724,6 +2728,8 @@ void intel_irq_init(struct drm_device *dev)
 	INIT_WORK(&dev_priv->rps.work, gen6_pm_rps_work);
 	INIT_WORK(&dev_priv->l3_parity.error_work, ivybridge_parity_work);
 
+	pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, 0);
+
 	dev->driver->get_vblank_counter = i915_get_vblank_counter;
 	dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
 	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b51043e..33dd233 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -322,6 +322,28 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
 	}
 }
 
+static uint32_t
+intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t ch_ctl = intel_dp->output_reg + 0x10;
+	uint32_t status;
+	bool done;
+
+#define C (((status = I915_READ(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
+	if (has_aux_irq)
+		done = wait_event_timeout(dev_priv->gmbus_wait_queue, C, 10);
+	else
+		done = wait_for_atomic(C, 10) == 0;
+	if (!done)
+		DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
+			  has_aux_irq);
+#undef C
+
+	return status;
+}
+
 static int
 intel_dp_aux_ch(struct intel_dp *intel_dp,
 		uint8_t *send, int send_bytes,
@@ -333,11 +355,17 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t ch_ctl = output_reg + 0x10;
 	uint32_t ch_data = ch_ctl + 4;
-	int i;
-	int recv_bytes;
+	int i, ret, recv_bytes;
 	uint32_t status;
 	uint32_t aux_clock_divider;
 	int try, precharge;
+	bool has_aux_irq = INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev);
+
+	/* dp aux is extremely sensitive to irq latency, hence request the
+	 * lowest possible wakeup latency and so prevent the cpu from going into
+	 * deep sleep states.
+	 */
+	pm_qos_update_request(&dev_priv->pm_qos, 0);
 
 	if (IS_HASWELL(dev)) {
 		switch (intel_dig_port->port) {
@@ -400,7 +428,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	if (try == 3) {
 		WARN(1, "dp_aux_ch not started status 0x%08x\n",
 		     I915_READ(ch_ctl));
-		return -EBUSY;
+		ret = -EBUSY;
+		goto out;
 	}
 
 	/* Must try at least 3 times according to DP spec */
@@ -413,6 +442,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 		/* Send the command and wait for it to complete */
 		I915_WRITE(ch_ctl,
 			   DP_AUX_CH_CTL_SEND_BUSY |
+			   (has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
 			   DP_AUX_CH_CTL_TIME_OUT_400us |
 			   (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
 			   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
@@ -420,12 +450,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 			   DP_AUX_CH_CTL_DONE |
 			   DP_AUX_CH_CTL_TIME_OUT_ERROR |
 			   DP_AUX_CH_CTL_RECEIVE_ERROR);
-		for (;;) {
-			status = I915_READ(ch_ctl);
-			if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0)
-				break;
-			udelay(100);
-		}
+
+		status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
 
 		/* Clear done status and any errors */
 		I915_WRITE(ch_ctl,
@@ -443,7 +469,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 
 	if ((status & DP_AUX_CH_CTL_DONE) == 0) {
 		DRM_ERROR("dp_aux_ch not done status 0x%08x\n", status);
-		return -EBUSY;
+		ret = -EBUSY;
+		goto out;
 	}
 
 	/* Check for timeout or receive error.
@@ -451,14 +478,16 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	 */
 	if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
 		DRM_ERROR("dp_aux_ch receive error status 0x%08x\n", status);
-		return -EIO;
+		ret = -EIO;
+		goto out;
 	}
 
 	/* Timeouts occur when the device isn't connected, so they're
 	 * "normal" -- don't fill the kernel log with these */
 	if (status & DP_AUX_CH_CTL_TIME_OUT_ERROR) {
 		DRM_DEBUG_KMS("dp_aux_ch timeout status 0x%08x\n", status);
-		return -ETIMEDOUT;
+		ret = -ETIMEDOUT;
+		goto out;
 	}
 
 	/* Unload any bytes sent back from the other side */
@@ -471,7 +500,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 		unpack_aux(I915_READ(ch_data + i),
 			   recv + i, recv_bytes - i);
 
-	return recv_bytes;
+	ret = recv_bytes;
+out:
+	pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE);
+
+	return ret;
 }
 
 /* Write data to the aux channel in native mode */
-- 
1.7.11.7

[PATCH 10/10] drm/i915: use _NOTRACE for gmbus/dp aux wait loops

[Daniel Vetter]

Less clutter in the traces. And in both cases we yell rather loud
into the logs if we time out. Patch suggested by Chris Wilson.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c  | 2 +-
 drivers/gpu/drm/i915/intel_i2c.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 33dd233..1b13b66 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -331,7 +331,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 	uint32_t status;
 	bool done;
 
-#define C (((status = I915_READ(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
+#define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
 	if (has_aux_irq)
 		done = wait_event_timeout(dev_priv->gmbus_wait_queue, C, 10);
 	else
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 1fc3119..7f09041 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -223,7 +223,7 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
 		prepare_to_wait(&dev_priv->gmbus_wait_queue, &wait,
 				TASK_UNINTERRUPTIBLE);
 
-		gmbus2 = I915_READ(GMBUS2 + reg_offset);
+		gmbus2 = I915_READ_NOTRACE(GMBUS2 + reg_offset);
 		if (gmbus2 & (GMBUS_SATOER | gmbus2_status))
 			break;
 
@@ -246,7 +246,7 @@ gmbus_wait_idle(struct drm_i915_private *dev_priv)
 	int ret;
 	int reg_offset = dev_priv->gpio_mmio_base;
 
-#define C ((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0)
+#define C ((I915_READ_NOTRACE(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0)
 
 	if (!HAS_GMBUS_IRQ(dev_priv->dev))
 		return wait_for(C, 10);
-- 
1.7.11.7

Re: [PATCH 10/10] drm/i915: use _NOTRACE for gmbus/dp aux wait loops

[Chris Wilson]

On Sat,  1 Dec 2012 13:53:49 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Less clutter in the traces. And in both cases we yell rather loud
> into the logs if we time out. Patch suggested by Chris Wilson.

Similar request for dp_aux. :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] drm/i915: use _NOTRACE for gmbus/dp aux wait loops

[Daniel Vetter]

Less clutter in the traces. And in both cases we yell rather loud
into the logs if we time out. Patch suggested by Chris Wilson.

v2: Annotate another I915_READ in dp_aux to be consistent - we filter
out all register io in wait_for and similar loops. Chris also
suggested to mark all dp_aux register access as _NOTRACE, but I think
we should keep all functionally relevant access around, and filter
unneeded bits in userspace after the trace is captured.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c  | 4 ++--
 drivers/gpu/drm/i915/intel_i2c.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 33dd233..3ed5f60 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -331,7 +331,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 	uint32_t status;
 	bool done;
 
-#define C (((status = I915_READ(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
+#define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
 	if (has_aux_irq)
 		done = wait_event_timeout(dev_priv->gmbus_wait_queue, C, 10);
 	else
@@ -419,7 +419,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 
 	/* Try to wait for any previous AUX channel activity */
 	for (try = 0; try < 3; try++) {
-		status = I915_READ(ch_ctl);
+		status = I915_READ_NOTRACE(ch_ctl);
 		if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0)
 			break;
 		msleep(1);
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 1fc3119..7f09041 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -223,7 +223,7 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv,
 		prepare_to_wait(&dev_priv->gmbus_wait_queue, &wait,
 				TASK_UNINTERRUPTIBLE);
 
-		gmbus2 = I915_READ(GMBUS2 + reg_offset);
+		gmbus2 = I915_READ_NOTRACE(GMBUS2 + reg_offset);
 		if (gmbus2 & (GMBUS_SATOER | gmbus2_status))
 			break;
 
@@ -246,7 +246,7 @@ gmbus_wait_idle(struct drm_i915_private *dev_priv)
 	int ret;
 	int reg_offset = dev_priv->gpio_mmio_base;
 
-#define C ((I915_READ(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0)
+#define C ((I915_READ_NOTRACE(GMBUS2 + reg_offset) & GMBUS_ACTIVE) == 0)
 
 	if (!HAS_GMBUS_IRQ(dev_priv->dev))
 		return wait_for(C, 10);
-- 
1.7.11.7

Re: [PATCH 00/10] gmbus/dp aux irqfication

[Chris Wilson]

On Sat,  1 Dec 2012 13:53:39 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Hi all,
> 
> Irq-drive gmbus/dp aux transfer, simply because we can (and at least in the case
> of gmbus, it's quite a bit faster than the msleep(1) loop - we now reliably
> transfer at full wire speed insteaf of sometimes 2-3x slower).
> 
> Compared to the older version I've floated on irc way back and which Chris has
> carried around in his wip branchs (and inflicted upon tons of unsuspecting bug
> reporters) there are a few differences:
> - handle the hpd vs. setup race - even the current code enables hpd processing
>   before the fbdev is set up, which is too early.
> - fix the gen4 gmbus support, was totally busted.
> - tested on hsw (although all the relevant bits are a 100% match with cpt/ivb).
> - disable dp aux irq on vlv - too complicated to get at the docs (moved again,
>   old access revoked) and I don't have the hw.
> 
> While reviewing this patch series I've also noticed two small things in the irq
> handling code in general, patches for that at the beginning of the series.
> 
> Comment&review highgly welcome.

gm45 appears to be functioning big time, good start. The init sequence
looks less convoluted and better understood now, so I am reasonably
happy with this series. Will report back later after more testing.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 00/10] gmbus/dp aux irqfication

[Chris Wilson]

On Sat, 01 Dec 2012 16:47:46 +0000, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Sat,  1 Dec 2012 13:53:39 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Hi all,
> > 
> > Irq-drive gmbus/dp aux transfer, simply because we can (and at least in the case
> > of gmbus, it's quite a bit faster than the msleep(1) loop - we now reliably
> > transfer at full wire speed insteaf of sometimes 2-3x slower).
> > 
> > Compared to the older version I've floated on irc way back and which Chris has
> > carried around in his wip branchs (and inflicted upon tons of unsuspecting bug
> > reporters) there are a few differences:
> > - handle the hpd vs. setup race - even the current code enables hpd processing
> >   before the fbdev is set up, which is too early.
> > - fix the gen4 gmbus support, was totally busted.
> > - tested on hsw (although all the relevant bits are a 100% match with cpt/ivb).
> > - disable dp aux irq on vlv - too complicated to get at the docs (moved again,
> >   old access revoked) and I don't have the hw.
> > 
> > While reviewing this patch series I've also noticed two small things in the irq
> > handling code in general, patches for that at the beginning of the series.
> > 
> > Comment&review highgly welcome.
> 
> gm45 appears to be functioning big time, good start. The init sequence
> looks less convoluted and better understood now, so I am reasonably
> happy with this series. Will report back later after more testing.

ilk: [    0.698644] [drm:intel_dp_aux_wait_done] *ERROR* dp aux hw did
not signal timeout (has irq: 1)! and slow
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 00/10] gmbus/dp aux irqfication

[Daniel Vetter]

On Sat, Dec 01, 2012 at 06:01:37PM +0000, Chris Wilson wrote:
> On Sat, 01 Dec 2012 16:47:46 +0000, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Sat,  1 Dec 2012 13:53:39 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > Hi all,
> > > 
> > > Irq-drive gmbus/dp aux transfer, simply because we can (and at least in the case
> > > of gmbus, it's quite a bit faster than the msleep(1) loop - we now reliably
> > > transfer at full wire speed insteaf of sometimes 2-3x slower).
> > > 
> > > Compared to the older version I've floated on irc way back and which Chris has
> > > carried around in his wip branchs (and inflicted upon tons of unsuspecting bug
> > > reporters) there are a few differences:
> > > - handle the hpd vs. setup race - even the current code enables hpd processing
> > >   before the fbdev is set up, which is too early.
> > > - fix the gen4 gmbus support, was totally busted.
> > > - tested on hsw (although all the relevant bits are a 100% match with cpt/ivb).
> > > - disable dp aux irq on vlv - too complicated to get at the docs (moved again,
> > >   old access revoked) and I don't have the hw.
> > > 
> > > While reviewing this patch series I've also noticed two small things in the irq
> > > handling code in general, patches for that at the beginning of the series.
> > > 
> > > Comment&review highgly welcome.
> > 
> > gm45 appears to be functioning big time, good start. The init sequence
> > looks less convoluted and better understood now, so I am reasonably
> > happy with this series. Will report back later after more testing.
> 
> ilk: [    0.698644] [drm:intel_dp_aux_wait_done] *ERROR* dp aux hw did
> not signal timeout (has irq: 1)! and slow

Should be fixed now.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 00/10] gmbus/dp aux irqfication

[Chris Wilson]

On Sat, 1 Dec 2012 21:15:14 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Sat, Dec 01, 2012 at 06:01:37PM +0000, Chris Wilson wrote:
> > ilk: [    0.698644] [drm:intel_dp_aux_wait_done] *ERROR* dp aux hw did
> > not signal timeout (has irq: 1)! and slow
> 
> Should be fixed now.

Looks that way, thanks.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 00/10] gmbus/dp aux irqfication

[Imre Deak]

On Sat, 2012-12-01 at 13:53 +0100, Daniel Vetter wrote:
> Hi all,
> 
> Irq-drive gmbus/dp aux transfer, simply because we can (and at least in the case
> of gmbus, it's quite a bit faster than the msleep(1) loop - we now reliably
> transfer at full wire speed insteaf of sometimes 2-3x slower).
> 
> Compared to the older version I've floated on irc way back and which Chris has
> carried around in his wip branchs (and inflicted upon tons of unsuspecting bug
> reporters) there are a few differences:
> - handle the hpd vs. setup race - even the current code enables hpd processing
>   before the fbdev is set up, which is too early.
> - fix the gen4 gmbus support, was totally busted.
> - tested on hsw (although all the relevant bits are a 100% match with cpt/ivb).
> - disable dp aux irq on vlv - too complicated to get at the docs (moved again,
>   old access revoked) and I don't have the hw.
> 
> While reviewing this patch series I've also noticed two small things in the irq
> handling code in general, patches for that at the beginning of the series.
> 
> Comment&review highgly welcome.

Looks good to me. On the series:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> 
> Cheers, Daniel
> 
> Daniel Vetter (10):
>   drm/i915: haswell has the same irq handlers as ivb
>   drm/i915: don't handle PIPE_LEGACY_BLC_EVENT_STATUS on vlv
>   drm/i915: reorder setup sequence to have irqs for output setup
>   drm/i915: extract gmbus_wait_hw_status
>   drm/i915: wire up gmbus irq handler
>   drm/i915: use the gmbus irq for waits
>   drm/i915: use gmbus irq to wait for gmbus idle
>   drm/i915: wire up do aux channel done interrupt
>   drm/i915: irq-drive the dp aux communication
>   drm/i915: use _NOTRACE for gmbus/dp aux wait loops
> 
>  drivers/gpu/drm/i915/i915_dma.c  |  10 +++-
>  drivers/gpu/drm/i915/i915_drv.h  |   8 ++++
>  drivers/gpu/drm/i915/i915_irq.c  |  74 ++++++++++++++++++++--------
>  drivers/gpu/drm/i915/intel_dp.c  |  59 ++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_i2c.c | 101 +++++++++++++++++++++++++++++----------
>  5 files changed, 193 insertions(+), 59 deletions(-)
> 

Re: [PATCH 00/10] gmbus/dp aux irqfication

[Daniel Vetter]

On Tue, Dec 04, 2012 at 06:04:48PM +0200, Imre Deak wrote:
> On Sat, 2012-12-01 at 13:53 +0100, Daniel Vetter wrote:
> > Hi all,
> > 
> > Irq-drive gmbus/dp aux transfer, simply because we can (and at least in the case
> > of gmbus, it's quite a bit faster than the msleep(1) loop - we now reliably
> > transfer at full wire speed insteaf of sometimes 2-3x slower).
> > 
> > Compared to the older version I've floated on irc way back and which Chris has
> > carried around in his wip branchs (and inflicted upon tons of unsuspecting bug
> > reporters) there are a few differences:
> > - handle the hpd vs. setup race - even the current code enables hpd processing
> >   before the fbdev is set up, which is too early.
> > - fix the gen4 gmbus support, was totally busted.
> > - tested on hsw (although all the relevant bits are a 100% match with cpt/ivb).
> > - disable dp aux irq on vlv - too complicated to get at the docs (moved again,
> >   old access revoked) and I don't have the hw.
> > 
> > While reviewing this patch series I've also noticed two small things in the irq
> > handling code in general, patches for that at the beginning of the series.
> > 
> > Comment&review highgly welcome.
> 
> Looks good to me. On the series:
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Thanks for the review, series merged.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch