More patches (resend)

More patches (resend)

[Chris Wilson]

Sorry for the mailbomb. The first attempt was aborted by the relay
part-way through the series, so I am trying a different mail server.

Please refer to 1282474317-14470-1-git-send-email-chris@chris-wilson.co.uk
for an overview of this series, and note that you can also fetch these
patches from annarchy in my for-anholt branch.
-Chris

[PATCH 01/29] drm/i915: unload: fix intel dp encoder cleanup

[Chris Wilson]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

struct intel_dp contains both struct intel_encoder at the beginning (as
it's base-class) and an i2c adapater. When initializing, the i2c adapter
gets assigned

	intel_encoder->ddc_adaptor = &intel_dp->adapter

and the generic intel_encode_destroy happily calls kfree on this pointer.
Ouch. Fix this by using a dp specific cleanup function.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_dp.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9caccd0..999ac91 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1481,6 +1481,15 @@ intel_dp_destroy (struct drm_connector *connector)
 	kfree(connector);
 }
 
+static void intel_dp_encoder_destroy(struct drm_encoder *encoder)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+
+	i2c_del_adapter(&intel_dp->adapter);
+	drm_encoder_cleanup(encoder);
+	kfree(intel_dp);
+}
+
 static const struct drm_encoder_helper_funcs intel_dp_helper_funcs = {
 	.dpms = intel_dp_dpms,
 	.mode_fixup = intel_dp_mode_fixup,
@@ -1503,7 +1512,7 @@ static const struct drm_connector_helper_funcs intel_dp_connector_helper_funcs =
 };
 
 static const struct drm_encoder_funcs intel_dp_enc_funcs = {
-	.destroy = intel_encoder_destroy,
+	.destroy = intel_dp_encoder_destroy,
 };
 
 void
-- 
1.7.1

[PATCH 02/29] drm/i915: unload: fix error_work races

[Chris Wilson]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

This is the first patch to clean up module unload races due to
outstanding timers/work. Preparatory step: Thou shalt not destroy
the workqueue when new work might still get enqued.

Now error_work gets queued by the hangcheck timer and only (atomically)
reads the chip wedged status. So cancel it right after the hangcheck
timer is killed. But the hangcheck is armed by interrupts, so move
everything after irqs are disabled.

Also change a del_timer to a del_timer_sync in the ums gem code, the
hangcheck timer is self-rearming.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_dma.c |    8 +++++---
 drivers/gpu/drm/i915/i915_gem.c |    2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 44af317..4463fba 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2246,9 +2246,6 @@ int i915_driver_unload(struct drm_device *dev)
 	i915_mch_dev = NULL;
 	spin_unlock(&mchdev_lock);
 
-	destroy_workqueue(dev_priv->wq);
-	del_timer_sync(&dev_priv->hangcheck_timer);
-
 	io_mapping_free(dev_priv->mm.gtt_mapping);
 	if (dev_priv->mm.gtt_mtrr >= 0) {
 		mtrr_del(dev_priv->mm.gtt_mtrr, dev->agp->base,
@@ -2273,6 +2270,9 @@ int i915_driver_unload(struct drm_device *dev)
 		vga_client_register(dev->pdev, NULL, NULL, NULL);
 	}
 
+	del_timer_sync(&dev_priv->hangcheck_timer);
+	cancel_work_sync(&dev_priv->error_work);
+
 	if (dev->pdev->msi_enabled)
 		pci_disable_msi(dev->pdev);
 
@@ -2297,6 +2297,8 @@ int i915_driver_unload(struct drm_device *dev)
 
 	intel_teardown_mchbar(dev);
 
+	destroy_workqueue(dev_priv->wq);
+
 	pci_dev_put(dev_priv->bridge_dev);
 	kfree(dev->dev_private);
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index df5a713..c101944 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4403,7 +4403,7 @@ i915_gem_idle(struct drm_device *dev)
 	 * And not confound mm.suspended!
 	 */
 	dev_priv->mm.suspended = 1;
-	del_timer(&dev_priv->hangcheck_timer);
+	del_timer_sync(&dev_priv->hangcheck_timer);
 
 	i915_kernel_lost_context(dev);
 	i915_gem_cleanup_ringbuffer(dev);
-- 
1.7.1

[PATCH 03/29] drm/i915: unload: fix hotplug_work races

[Chris Wilson]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

hotplug_work is queued by the hotplug interrupt and only either emits
a hotplug uevent or queues a crt poll slow-work. No other locking.  So
it's safe to cancel this work _after_ irq's have been turned off.  But
before the modesetting objects are destroyed because the hotplug
function accesses them (without locking).

The current code (for kms) only switches irqs off after modesetting
teardown, hence move the irq teardown into the modeset cleanup right
before the crtc cleanup.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_dma.c      |    2 +-
 drivers/gpu/drm/i915/intel_display.c |    5 +++++
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4463fba..c073995 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2265,7 +2265,7 @@ int i915_driver_unload(struct drm_device *dev)
 			dev_priv->child_dev = NULL;
 			dev_priv->child_dev_num = 0;
 		}
-		drm_irq_uninstall(dev);
+
 		vga_switcheroo_unregister_client(dev->pdev);
 		vga_client_register(dev->pdev, NULL, NULL, NULL);
 	}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 23157e1..4c663a8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6108,6 +6108,11 @@ void intel_modeset_cleanup(struct drm_device *dev)
 
 	mutex_unlock(&dev->struct_mutex);
 
+	/* Disable the irq before mode object teardown, for the irq might
+	 * enqueue unpin/hotplug work. */
+	drm_irq_uninstall(dev);
+	cancel_work_sync(&dev_priv->hotplug_work);
+
 	drm_mode_config_cleanup(dev);
 }
 
-- 
1.7.1

[PATCH 04/29] drm/i915: unload: don't leak error state

[Chris Wilson]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

With kms, interrupts now get disabled in the modesetting cleanup. So
free the error state afterwards, it currently gets allocated in
the interrupt handler.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_dma.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index c073995..8e3ae34 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2240,8 +2240,6 @@ int i915_driver_unload(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	i915_destroy_error_state(dev);
-
 	spin_lock(&mchdev_lock);
 	i915_mch_dev = NULL;
 	spin_unlock(&mchdev_lock);
@@ -2270,8 +2268,10 @@ int i915_driver_unload(struct drm_device *dev)
 		vga_client_register(dev->pdev, NULL, NULL, NULL);
 	}
 
+	/* Free error state after interrupts are fully disabled. */
 	del_timer_sync(&dev_priv->hangcheck_timer);
 	cancel_work_sync(&dev_priv->error_work);
+	i915_destroy_error_state(dev);
 
 	if (dev->pdev->msi_enabled)
 		pci_disable_msi(dev->pdev);
-- 
1.7.1

[PATCH 05/29] drm/i915: unload: fix idle_timer/idle_work races

[Chris Wilson]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

idle_work wasn't cleaned up at all. It takes &dev->struct_mutex, but
accesss the mode_config crtc list (without any other locking!). Hence
this work needs to be canceled before calling drm_mode_config_cleanup.

As evidenced by the kernel's object debuggin code, the current code
also cleans up the timer to early (it gets rearmed). So move it right
before the final cleanup (it seems to work).

Also unconditionally set up the idle_timer in intel_increase_pllclock.
If we're unlucky the timer might fire right away, rendering the call
in the modesetting teardown pointless.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |   29 ++++++++++++++++-------------
 1 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4c663a8..ef26839 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -43,7 +43,7 @@
 
 bool intel_pipe_has_type (struct drm_crtc *crtc, int type);
 static void intel_update_watermarks(struct drm_device *dev);
-static void intel_increase_pllclock(struct drm_crtc *crtc, bool schedule);
+static void intel_increase_pllclock(struct drm_crtc *crtc);
 static void intel_crtc_update_cursor(struct drm_crtc *crtc);
 
 typedef struct {
@@ -1512,7 +1512,7 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 		intel_update_fbc(crtc, &crtc->mode);
 
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
-	intel_increase_pllclock(crtc, true);
+	intel_increase_pllclock(crtc);
 
 	return 0;
 }
@@ -1636,7 +1636,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 		obj_priv = to_intel_bo(intel_fb->obj);
 		i915_gem_object_unpin(intel_fb->obj);
 	}
-	intel_increase_pllclock(crtc, true);
+	intel_increase_pllclock(crtc);
 
 	mutex_unlock(&dev->struct_mutex);
 
@@ -4719,7 +4719,7 @@ static void intel_crtc_idle_timer(unsigned long arg)
 	queue_work(dev_priv->wq, &dev_priv->idle_work);
 }
 
-static void intel_increase_pllclock(struct drm_crtc *crtc, bool schedule)
+static void intel_increase_pllclock(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	drm_i915_private_t *dev_priv = dev->dev_private;
@@ -4754,9 +4754,8 @@ static void intel_increase_pllclock(struct drm_crtc *crtc, bool schedule)
 	}
 
 	/* Schedule downclock */
-	if (schedule)
-		mod_timer(&intel_crtc->idle_timer, jiffies +
-			  msecs_to_jiffies(CRTC_IDLE_TIMEOUT));
+	mod_timer(&intel_crtc->idle_timer, jiffies +
+		  msecs_to_jiffies(CRTC_IDLE_TIMEOUT));
 }
 
 static void intel_decrease_pllclock(struct drm_crtc *crtc)
@@ -4892,7 +4891,7 @@ void intel_mark_busy(struct drm_device *dev, struct drm_gem_object *obj)
 					I915_WRITE(FW_BLC_SELF, fw_blc_self | FW_BLC_SELF_EN_MASK);
 				}
 				/* Non-busy -> busy, upclock */
-				intel_increase_pllclock(crtc, true);
+				intel_increase_pllclock(crtc);
 				intel_crtc->busy = true;
 			} else {
 				/* Busy -> busy, put off timer */
@@ -6074,12 +6073,9 @@ void intel_modeset_cleanup(struct drm_device *dev)
 			continue;
 
 		intel_crtc = to_intel_crtc(crtc);
-		intel_increase_pllclock(crtc, false);
-		del_timer_sync(&intel_crtc->idle_timer);
+		intel_increase_pllclock(crtc);
 	}
 
-	del_timer_sync(&dev_priv->idle_timer);
-
 	if (dev_priv->display.disable_fbc)
 		dev_priv->display.disable_fbc(dev);
 
@@ -6113,10 +6109,17 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	drm_irq_uninstall(dev);
 	cancel_work_sync(&dev_priv->hotplug_work);
 
+	/* Shut off idle work before the crtcs get freed. */
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		intel_crtc = to_intel_crtc(crtc);
+		del_timer_sync(&intel_crtc->idle_timer);
+	}
+	del_timer_sync(&dev_priv->idle_timer);
+	cancel_work_sync(&dev_priv->idle_work);
+
 	drm_mode_config_cleanup(dev);
 }
 
-
 /*
  * Return which encoder is currently attached for connector.
  */
-- 
1.7.1

[PATCH 06/29] drm/i915: unload: fix unpin_work related races

[Chris Wilson]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Kill any outstanding unpin_work when destroying the corresponding
crtc. Then flush the workqueue before the gem teardown, in case
any unpin work is still outstanding.

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

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 8e3ae34..bb59058 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2282,6 +2282,9 @@ int i915_driver_unload(struct drm_device *dev)
 	intel_opregion_free(dev, 0);
 
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		/* Flush any outstanding unpin_work. */
+		flush_workqueue(dev_priv->wq);
+
 		i915_gem_free_all_phys_object(dev);
 
 		mutex_lock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ef26839..6ccb797 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4902,14 +4902,6 @@ void intel_mark_busy(struct drm_device *dev, struct drm_gem_object *obj)
 	}
 }
 
-static void intel_crtc_destroy(struct drm_crtc *crtc)
-{
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-	drm_crtc_cleanup(crtc);
-	kfree(intel_crtc);
-}
-
 struct intel_unpin_work {
 	struct work_struct work;
 	struct drm_device *dev;
@@ -4919,6 +4911,28 @@ struct intel_unpin_work {
 	int pending;
 };
 
+static void intel_crtc_destroy(struct drm_crtc *crtc)
+{
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
+	struct intel_unpin_work *work;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->event_lock, flags);
+	work = intel_crtc->unpin_work;
+	intel_crtc->unpin_work = NULL;
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+
+	if (work) {
+		cancel_work_sync(&work->work);
+		kfree(work);
+	}
+
+	drm_crtc_cleanup(crtc);
+
+	kfree(intel_crtc);
+}
+
 static void intel_unpin_work_fn(struct work_struct *__work)
 {
 	struct intel_unpin_work *work =
-- 
1.7.1

[PATCH 07/29] drm/i915: unload: ensure that gem is idle

[Chris Wilson]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

When the module unloads, all users should be gone, hence all bo references
held by userspace, too. This should already result in an idle ringbuffer.
Still, be paranoid and idle gem before starting the unload dance.

Also kill the call to i915_gem_lastclose under an if (kms), it's a noop
for kms.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_dma.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index bb59058..3dea76d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2239,11 +2239,18 @@ free_priv:
 int i915_driver_unload(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret;
 
 	spin_lock(&mchdev_lock);
 	i915_mch_dev = NULL;
 	spin_unlock(&mchdev_lock);
 
+	mutex_lock(&dev->struct_mutex);
+	ret = i915_gpu_idle(dev);
+	if (ret)
+		DRM_ERROR("failed to idle hardware: %d\n", ret);
+	mutex_unlock(&dev->struct_mutex);
+
 	io_mapping_free(dev_priv->mm.gtt_mapping);
 	if (dev_priv->mm.gtt_mtrr >= 0) {
 		mtrr_del(dev_priv->mm.gtt_mtrr, dev->agp->base,
@@ -2293,7 +2300,6 @@ int i915_driver_unload(struct drm_device *dev)
 		if (I915_HAS_FBC(dev) && i915_powersave)
 			i915_cleanup_compression(dev);
 		drm_mm_takedown(&dev_priv->vram);
-		i915_gem_lastclose(dev);
 
 		intel_cleanup_overlay(dev);
 	}
-- 
1.7.1

[PATCH 08/29] drm/i915: unload: fix retire_work races

[Chris Wilson]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

ums-gem code correctly cancels the retire work (at lastclose time),
kms does not do so. Fix this by canceling the work right after ideling
the gpu.

While staring at the code I noticed that the work function is not
static. Fix this, too.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_dma.c |    3 +++
 drivers/gpu/drm/i915/i915_drv.h |    1 -
 drivers/gpu/drm/i915/i915_gem.c |    2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 3dea76d..77af26d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2251,6 +2251,9 @@ int i915_driver_unload(struct drm_device *dev)
 		DRM_ERROR("failed to idle hardware: %d\n", ret);
 	mutex_unlock(&dev->struct_mutex);
 
+	/* Cancel the retire work handler, which should be idle now. */
+	cancel_delayed_work_sync(&dev_priv->mm.retire_work);
+
 	io_mapping_free(dev_priv->mm.gtt_mapping);
 	if (dev_priv->mm.gtt_mtrr >= 0) {
 		mtrr_del(dev_priv->mm.gtt_mtrr, dev->agp->base,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 047cd7c..11dff01 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -978,7 +978,6 @@ bool i915_seqno_passed(uint32_t seq1, uint32_t seq2);
 int i915_gem_object_get_fence_reg(struct drm_gem_object *obj);
 int i915_gem_object_put_fence_reg(struct drm_gem_object *obj);
 void i915_gem_retire_requests(struct drm_device *dev);
-void i915_gem_retire_work_handler(struct work_struct *work);
 void i915_gem_clflush_object(struct drm_gem_object *obj);
 int i915_gem_object_set_domain(struct drm_gem_object *obj,
 			       uint32_t read_domains,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c101944..85e7c64 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1793,7 +1793,7 @@ i915_gem_retire_requests(struct drm_device *dev)
 		i915_gem_retire_requests_ring(dev, &dev_priv->bsd_ring);
 }
 
-void
+static void
 i915_gem_retire_work_handler(struct work_struct *work)
 {
 	drm_i915_private_t *dev_priv;
-- 
1.7.1

[PATCH 09/29] drm/i915: Fixup intel_wait_for_vblank*()

[Chris Wilson]

Add a msleep to intel_wait_for_vblank().

Change intel_wait_for_vblank_off() to repeat *until* timeout.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6ccb797..d50830e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -993,7 +993,7 @@ void intel_wait_for_vblank(struct drm_device *dev, int pipe)
 	/* Wait for vblank interrupt bit to set */
 	if (wait_for((I915_READ(pipestat_reg) &
 		      PIPE_VBLANK_INTERRUPT_STATUS) == 0,
-		     50, 0))
+		     50, 1))
 		DRM_DEBUG_KMS("vblank wait timed out\n");
 }
 
@@ -1019,9 +1019,12 @@ void intel_wait_for_vblank_off(struct drm_device *dev, int pipe)
 	/* Wait for the display line to settle */
 	do {
 		last_line = I915_READ(pipedsl_reg) & DSL_LINEMASK;
-		mdelay(5);
+		if (in_dbg_master())
+			mdelay(5);
+		else
+			msleep(5);
 	} while (((I915_READ(pipedsl_reg) & DSL_LINEMASK) != last_line) &&
-		 time_after(timeout, jiffies));
+		 !time_after(timeout, jiffies));
 
 	if (time_after(jiffies, timeout))
 		DRM_DEBUG_KMS("vblank wait timed out\n");
-- 
1.7.1

[PATCH] drm/i915: Drop the msleep parameter to wait_for()

[Chris Wilson]

Jesse's feedback from using the wait_for() macro was that the msleep
argument was that it was superfluous and made the macro more difficult
to use and to read. As the actually amount of time to sleep is not
critical, the crucial part is to sleep and let the processor schedule
something else whilst we wait for the event, replace the argument with a
hardcoded value.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
---

Jesse pointed out that my patch to fix intel_wait_for_vblank_off() was
broken and that perhaps I should concentrate on improving wait_for() and
make it usable.

---
 drivers/gpu/drm/i915/intel_crt.c     |    4 ++--
 drivers/gpu/drm/i915/intel_display.c |   12 ++++++------
 drivers/gpu/drm/i915/intel_dp.c      |    4 ++--
 drivers/gpu/drm/i915/intel_drv.h     |    5 ++++-
 drivers/gpu/drm/i915/intel_lvds.c    |    4 ++--
 5 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 4b77351..c2982e4 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -187,7 +187,7 @@ static bool intel_ironlake_crt_detect_hotplug(struct drm_connector *connector)
 	I915_WRITE(PCH_ADPA, adpa);
 
 	if (wait_for((I915_READ(PCH_ADPA) & ADPA_CRT_HOTPLUG_FORCE_TRIGGER) == 0,
-		     1000, 1))
+		     1000))
 		DRM_ERROR("timed out waiting for FORCE_TRIGGER");
 
 	if (turn_off_dac) {
@@ -244,7 +244,7 @@ static bool intel_crt_detect_hotplug(struct drm_connector *connector)
 		/* wait for FORCE_DETECT to go off */
 		if (wait_for((I915_READ(PORT_HOTPLUG_EN) &
 			      CRT_HOTPLUG_FORCE_DETECT) == 0,
-			     1000, 1))
+			     1000))
 			DRM_ERROR("timed out waiting for FORCE_DETECT to go off");
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6ccb797..ecf2d41 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -993,7 +993,7 @@ void intel_wait_for_vblank(struct drm_device *dev, int pipe)
 	/* Wait for vblank interrupt bit to set */
 	if (wait_for((I915_READ(pipestat_reg) &
 		      PIPE_VBLANK_INTERRUPT_STATUS) == 0,
-		     50, 0))
+		     50))
 		DRM_DEBUG_KMS("vblank wait timed out\n");
 }
 
@@ -1092,7 +1092,7 @@ void i8xx_disable_fbc(struct drm_device *dev)
 	I915_WRITE(FBC_CONTROL, fbc_ctl);
 
 	/* Wait for compressing bit to clear */
-	if (wait_for((I915_READ(FBC_STATUS) & FBC_STAT_COMPRESSING) == 0, 10, 0)) {
+	if (wait_for((I915_READ(FBC_STATUS) & FBC_STAT_COMPRESSING) == 0, 10)) {
 		DRM_DEBUG_KMS("FBC idle timed out\n");
 		return;
 	}
@@ -2115,7 +2115,7 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
 			I915_WRITE(transconf_reg, temp | TRANS_ENABLE);
 			I915_READ(transconf_reg);
 
-			if (wait_for(I915_READ(transconf_reg) & TRANS_STATE_ENABLE, 10, 0))
+			if (wait_for(I915_READ(transconf_reg) & TRANS_STATE_ENABLE, 10))
 				DRM_ERROR("failed to enable transcoder\n");
 		}
 
@@ -2147,7 +2147,7 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
 			I915_WRITE(pipeconf_reg, temp & ~PIPEACONF_ENABLE);
 
 			/* wait for cpu pipe off, pipe state */
-			if (wait_for((I915_READ(pipeconf_reg) & I965_PIPECONF_ACTIVE) == 0, 50, 1))
+			if (wait_for((I915_READ(pipeconf_reg) & I965_PIPECONF_ACTIVE) == 0, 50))
 				DRM_ERROR("failed to turn off cpu pipe\n");
 		} else
 			DRM_DEBUG_KMS("crtc %d is disabled\n", pipe);
@@ -2211,7 +2211,7 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
 			I915_WRITE(transconf_reg, temp & ~TRANS_ENABLE);
 
 			/* wait for PCH transcoder off, transcoder state */
-			if (wait_for((I915_READ(transconf_reg) & TRANS_STATE_ENABLE) == 0, 50, 1))
+			if (wait_for((I915_READ(transconf_reg) & TRANS_STATE_ENABLE) == 0, 50))
 				DRM_ERROR("failed to disable transcoder\n");
 		}
 
@@ -5553,7 +5553,7 @@ void ironlake_enable_drps(struct drm_device *dev)
 	rgvmodectl |= MEMMODE_SWMODE_EN;
 	I915_WRITE(MEMMODECTL, rgvmodectl);
 
-	if (wait_for((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 1, 0))
+	if (wait_for((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 10))
 		DRM_ERROR("stuck trying to change perf mode\n");
 	msleep(1);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 999ac91..0879331 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -774,7 +774,7 @@ static void ironlake_edp_panel_on (struct drm_device *dev)
 	pp |= PANEL_UNLOCK_REGS | POWER_TARGET_ON;
 	I915_WRITE(PCH_PP_CONTROL, pp);
 
-	if (wait_for(I915_READ(PCH_PP_STATUS) & PP_ON, 5000, 10))
+	if (wait_for(I915_READ(PCH_PP_STATUS) & PP_ON, 5000))
 		DRM_ERROR("panel on wait timed out: 0x%08x\n",
 			  I915_READ(PCH_PP_STATUS));
 
@@ -799,7 +799,7 @@ static void ironlake_edp_panel_off (struct drm_device *dev)
 	pp &= ~POWER_TARGET_ON;
 	I915_WRITE(PCH_PP_CONTROL, pp);
 
-	if (wait_for((I915_READ(PCH_PP_STATUS) & PP_ON) == 0, 5000, 10))
+	if (wait_for((I915_READ(PCH_PP_STATUS) & PP_ON) == 0, 5000))
 		DRM_ERROR("panel off wait timed out: 0x%08x\n",
 			  I915_READ(PCH_PP_STATUS));
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0e92aa0..19ab95d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -33,7 +33,7 @@
 
 #include "drm_crtc_helper.h"
 
-#define wait_for(COND, MS, W) ({ \
+#define _wait_for(COND, MS, W) ({ \
 	unsigned long timeout__ = jiffies + msecs_to_jiffies(MS);	\
 	int ret__ = 0;							\
 	while (! (COND)) {						\
@@ -46,6 +46,9 @@
 	ret__;								\
 })
 
+#define wait_for(COND, MS) _wait_for(COND, MS, 1)
+#define wait_for_atomic(COND, MS) _wait_for(COND, MS, 0)
+
 /*
  * Display related stuff
  */
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index b819c10..c9b62fe 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -114,7 +114,7 @@ static void intel_lvds_set_power(struct drm_device *dev, bool on)
 
 		I915_WRITE(ctl_reg, I915_READ(ctl_reg) |
 			   POWER_TARGET_ON);
-		if (wait_for(I915_READ(status_reg) & PP_ON, 1000, 0))
+		if (wait_for(I915_READ(status_reg) & PP_ON, 1000))
 			DRM_ERROR("timed out waiting to enable LVDS pipe");
 
 		intel_lvds_set_backlight(dev, dev_priv->backlight_duty_cycle);
@@ -123,7 +123,7 @@ static void intel_lvds_set_power(struct drm_device *dev, bool on)
 
 		I915_WRITE(ctl_reg, I915_READ(ctl_reg) &
 			   ~POWER_TARGET_ON);
-		if (wait_for((I915_READ(status_reg) & PP_ON) == 0, 1000, 0))
+		if (wait_for((I915_READ(status_reg) & PP_ON) == 0, 1000))
 			DRM_ERROR("timed out waiting for LVDS pipe to turn off");
 
 		I915_WRITE(lvds_reg, I915_READ(lvds_reg) & ~LVDS_PORT_EN);
-- 
1.7.1

Re: [PATCH] drm/i915: Drop the msleep parameter to wait_for()

[Peter Clifton]

On Mon, 2010-08-23 at 17:56 +0100, Chris Wilson wrote:
> Jesse's feedback from using the wait_for() macro was that the msleep
> argument was that it was superfluous and made the macro more difficult
> to use and to read. As the actually amount of time to sleep is not
> critical, the crucial part is to sleep and let the processor schedule
> something else whilst we wait for the event, replace the argument with a
> hardcoded value.


I noticed that the patch changes the semantics of some of the wait_for
calls. Previously, many were called with a zero msleep parameter -
meaning the call would not msleep. With this patch, the cases below will
now msleep(1), rather than not msleep'ing at all.


> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6ccb797..ecf2d41 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -993,7 +993,7 @@ void intel_wait_for_vblank(struct drm_device *dev, int pipe)
>  	/* Wait for vblank interrupt bit to set */
>  	if (wait_for((I915_READ(pipestat_reg) &
              ^___ wait_for_atomic??
>  		      PIPE_VBLANK_INTERRUPT_STATUS) == 0,
> -		     50, 0))
> +		     50))
>  		DRM_DEBUG_KMS("vblank wait timed out\n");
>  }
>  
> @@ -1092,7 +1092,7 @@ void i8xx_disable_fbc(struct drm_device *dev)
>  	I915_WRITE(FBC_CONTROL, fbc_ctl);
>  
>  	/* Wait for compressing bit to clear */
> -	if (wait_for((I915_READ(FBC_STATUS) & FBC_STAT_COMPRESSING) == 0, 10, 0)) {
> +	if (wait_for((I915_READ(FBC_STATUS) & FBC_STAT_COMPRESSING) == 0, 10)) {
                ^___ wait_for_atomic?

>  		DRM_DEBUG_KMS("FBC idle timed out\n");
>  		return;
>  	}
> @@ -2115,7 +2115,7 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
>  			I915_WRITE(transconf_reg, temp | TRANS_ENABLE);
>  			I915_READ(transconf_reg);
>  
> -			if (wait_for(I915_READ(transconf_reg) & TRANS_STATE_ENABLE, 10, 0))
> +			if (wait_for(I915_READ(transconf_reg) & TRANS_STATE_ENABLE, 10))
                                ^___ wait_for_atomic?

> @@ -5553,7 +5553,7 @@ void ironlake_enable_drps(struct drm_device *dev)
>  	rgvmodectl |= MEMMODE_SWMODE_EN;
>  	I915_WRITE(MEMMODECTL, rgvmodectl);
>  
> -	if (wait_for((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 1, 0))
> +	if (wait_for((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 10))
                ^___ wait_for_atomic?

>  		DRM_ERROR("stuck trying to change perf mode\n");
>  	msleep(1);

 
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index b819c10..c9b62fe 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -114,7 +114,7 @@ static void intel_lvds_set_power(struct drm_device *dev, bool on)
>  
>  		I915_WRITE(ctl_reg, I915_READ(ctl_reg) |
>  			   POWER_TARGET_ON);
> -		if (wait_for(I915_READ(status_reg) & PP_ON, 1000, 0))
> +		if (wait_for(I915_READ(status_reg) & PP_ON, 1000))
                          ^___ wait_for_atomic?

>  			DRM_ERROR("timed out waiting to enable LVDS pipe");
>  
>  		intel_lvds_set_backlight(dev, dev_priv->backlight_duty_cycle);
> @@ -123,7 +123,7 @@ static void intel_lvds_set_power(struct drm_device *dev, bool on)
>  
>  		I915_WRITE(ctl_reg, I915_READ(ctl_reg) &
>  			   ~POWER_TARGET_ON);
> -		if (wait_for((I915_READ(status_reg) & PP_ON) == 0, 1000, 0))
> +		if (wait_for((I915_READ(status_reg) & PP_ON) == 0, 1000))
                         ^___ wait_for_atomic?


-- 
Peter Clifton

Electrical Engineering Division,
Engineering Department,
University of Cambridge,
9, JJ Thomson Avenue,
Cambridge
CB3 0FA

Tel: +44 (0)7729 980173 - (No signal in the lab!)
Tel: +44 (0)1223 748328 - (Shared lab phone, ask for me)

Re: [PATCH] drm/i915: Drop the msleep parameter to wait_for()

[Chris Wilson]

On Tue, 24 Aug 2010 00:16:37 +0100, Peter Clifton <pcjc2@cam.ac.uk> wrote:
> I noticed that the patch changes the semantics of some of the wait_for
> calls. Previously, many were called with a zero msleep parameter -
> meaning the call would not msleep. With this patch, the cases below will
> now msleep(1), rather than not msleep'ing at all.

Intentionally. The choices I made when adding the wait_for() were fairly
arbitrary. I'd err on the side of sleeping the extra milliseconds rather
than spend 500 microseconds busy-spinning. (I've have a different opinion
if these ever become the rate-limiting step in modesetting... ;-)

> >  	/* Wait for compressing bit to clear */
> > -	if (wait_for((I915_READ(FBC_STATUS) & FBC_STAT_COMPRESSING) == 0, 10, 0)) {
> > +	if (wait_for((I915_READ(FBC_STATUS) & FBC_STAT_COMPRESSING) == 0, 10)) {
>                 ^___ wait_for_atomic?

This is perhaps the most debatable as I have no feel for what the
compression delay is and the spin may only be on the order of a few
hundred microseconds. However, I think the wait_for() here is entirely
superfluous and have removed it in my drm-testing.

A couple of the later patches increase those short timeouts you
highlighted to fix reported issues, so those are poor candidates for
busy-spinning. So outside of kdb, I don't see a reason where we need to be
continuously polling the register.

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Drop the msleep parameter to wait_for()

[Peter Clifton]

On Tue, 2010-08-24 at 00:33 +0100, Chris Wilson wrote:
> On Tue, 24 Aug 2010 00:16:37 +0100, Peter Clifton <pcjc2@cam.ac.uk> wrote:
> > I noticed that the patch changes the semantics of some of the wait_for
> > calls. Previously, many were called with a zero msleep parameter -
> > meaning the call would not msleep. With this patch, the cases below will
> > now msleep(1), rather than not msleep'ing at all.
> 
> Intentionally. The choices I made when adding the wait_for() were fairly
> arbitrary. I'd err on the side of sleeping the extra milliseconds rather
> than spend 500 microseconds busy-spinning. (I've have a different opinion
> if these ever become the rate-limiting step in modesetting... ;-)

Fair enough.. it just looked like it might have been a mistake since you
had both re-factored the interface, and altered the semantics of some of
its callers within the same commit.

-- 
Peter Clifton

Electrical Engineering Division,
Engineering Department,
University of Cambridge,
9, JJ Thomson Avenue,
Cambridge
CB3 0FA

Tel: +44 (0)7729 980173 - (No signal in the lab!)
Tel: +44 (0)1223 748328 - (Shared lab phone, ask for me)

[PATCH 10/29] drm/i915: Avoid using msleep under kdb and wait_for()

[Chris Wilson]

wait_for() uses msleep() to yield the cpu whilst spinning waiting for a
register to change. kdb asserts that mode changes are atomic and so
prohibits msleep. The alternative would be to use mdelay or to simply
probe the register more often instead of busy waiting.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_drv.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0e92aa0..bce6d07 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -41,7 +41,7 @@
 			ret__ = -ETIMEDOUT;				\
 			break;						\
 		}							\
-		if (W) msleep(W);					\
+		if (W && !in_dbg_master()) msleep(W);			\
 	}								\
 	ret__;								\
 })
-- 
1.7.1

[PATCH 11/29] drm/i915: Include a generation number in the device info

[Chris Wilson]

To simplify the IS_GEN[234] macros and to enable switching.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c |   61 ++++++++++++++++++---------------------
 drivers/gpu/drm/i915/i915_drv.h |   27 ++++-------------
 2 files changed, 34 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 00befce..5363985 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -61,91 +61,86 @@ extern int intel_agp_enabled;
 	.driver_data = (unsigned long) info }
 
 static const struct intel_device_info intel_i830_info = {
-	.is_i8xx = 1, .is_mobile = 1, .cursor_needs_physical = 1,
+	.gen = 2, .is_i8xx = 1, .is_mobile = 1, .cursor_needs_physical = 1,
 };
 
 static const struct intel_device_info intel_845g_info = {
-	.is_i8xx = 1,
+	.gen = 2, .is_i8xx = 1,
 };
 
 static const struct intel_device_info intel_i85x_info = {
-	.is_i8xx = 1, .is_i85x = 1, .is_mobile = 1,
+	.gen = 2, .is_i8xx = 1, .is_i85x = 1, .is_mobile = 1,
 	.cursor_needs_physical = 1,
 };
 
 static const struct intel_device_info intel_i865g_info = {
-	.is_i8xx = 1,
+	.gen = 2, .is_i8xx = 1,
 };
 
 static const struct intel_device_info intel_i915g_info = {
-	.is_i915g = 1, .is_i9xx = 1, .cursor_needs_physical = 1,
+	.gen = 3, .is_i915g = 1, .is_i9xx = 1, .cursor_needs_physical = 1,
 };
 static const struct intel_device_info intel_i915gm_info = {
-	.is_i9xx = 1,  .is_mobile = 1,
+	.gen = 3, .is_i9xx = 1,  .is_mobile = 1,
 	.cursor_needs_physical = 1,
 };
 static const struct intel_device_info intel_i945g_info = {
-	.is_i9xx = 1, .has_hotplug = 1, .cursor_needs_physical = 1,
+	.gen = 3, .is_i9xx = 1, .has_hotplug = 1, .cursor_needs_physical = 1,
 };
 static const struct intel_device_info intel_i945gm_info = {
-	.is_i945gm = 1, .is_i9xx = 1, .is_mobile = 1,
+	.gen = 3, .is_i945gm = 1, .is_i9xx = 1, .is_mobile = 1,
 	.has_hotplug = 1, .cursor_needs_physical = 1,
 };
 
 static const struct intel_device_info intel_i965g_info = {
-	.is_broadwater = 1, .is_i965g = 1, .is_i9xx = 1, .has_hotplug = 1,
+	.gen = 4, .is_broadwater = 1, .is_i965g = 1, .is_i9xx = 1,
+	.has_hotplug = 1,
 };
 
 static const struct intel_device_info intel_i965gm_info = {
-	.is_crestline = 1, .is_i965g = 1, .is_i965gm = 1, .is_i9xx = 1,
-	.is_mobile = 1, .has_fbc = 1, .has_rc6 = 1,
-	.has_hotplug = 1,
+	.gen = 4, .is_crestline = 1, .is_i965g = 1, .is_i965gm = 1, .is_i9xx = 1,
+	.is_mobile = 1, .has_fbc = 1, .has_rc6 = 1, .has_hotplug = 1,
 };
 
 static const struct intel_device_info intel_g33_info = {
-	.is_g33 = 1, .is_i9xx = 1, .need_gfx_hws = 1,
-	.has_hotplug = 1,
+	.gen = 3, .is_g33 = 1, .is_i9xx = 1,
+	.need_gfx_hws = 1, .has_hotplug = 1,
 };
 
 static const struct intel_device_info intel_g45_info = {
-	.is_i965g = 1, .is_g4x = 1, .is_i9xx = 1, .need_gfx_hws = 1,
-	.has_pipe_cxsr = 1,
-	.has_hotplug = 1,
+	.gen = 4, .is_i965g = 1, .is_g4x = 1, .is_i9xx = 1, .need_gfx_hws = 1,
+	.has_pipe_cxsr = 1, .has_hotplug = 1,
 };
 
 static const struct intel_device_info intel_gm45_info = {
-	.is_i965g = 1, .is_g4x = 1, .is_i9xx = 1,
+	.gen = 4, .is_i965g = 1, .is_g4x = 1, .is_i9xx = 1,
 	.is_mobile = 1, .need_gfx_hws = 1, .has_fbc = 1, .has_rc6 = 1,
-	.has_pipe_cxsr = 1,
-	.has_hotplug = 1,
+	.has_pipe_cxsr = 1, .has_hotplug = 1,
 };
 
 static const struct intel_device_info intel_pineview_info = {
-	.is_g33 = 1, .is_pineview = 1, .is_mobile = 1, .is_i9xx = 1,
-	.need_gfx_hws = 1,
-	.has_hotplug = 1,
+	.gen = 3, .is_g33 = 1, .is_pineview = 1, .is_mobile = 1, .is_i9xx = 1,
+	.need_gfx_hws = 1, .has_hotplug = 1,
 };
 
 static const struct intel_device_info intel_ironlake_d_info = {
-	.is_ironlake = 1, .is_i965g = 1, .is_i9xx = 1, .need_gfx_hws = 1,
-	.has_pipe_cxsr = 1,
-	.has_hotplug = 1,
+	.gen = 5, .is_ironlake = 1, .is_i965g = 1, .is_i9xx = 1,
+	.need_gfx_hws = 1, .has_pipe_cxsr = 1, .has_hotplug = 1,
 };
 
 static const struct intel_device_info intel_ironlake_m_info = {
-	.is_ironlake = 1, .is_mobile = 1, .is_i965g = 1, .is_i9xx = 1,
-	.need_gfx_hws = 1, .has_fbc = 1, .has_rc6 = 1,
-	.has_hotplug = 1,
+	.gen = 5, .is_ironlake = 1, .is_mobile = 1, .is_i965g = 1, .is_i9xx = 1,
+	.need_gfx_hws = 1, .has_fbc = 1, .has_rc6 = 1, .has_hotplug = 1,
 };
 
 static const struct intel_device_info intel_sandybridge_d_info = {
-	.is_i965g = 1, .is_i9xx = 1, .need_gfx_hws = 1,
-	.has_hotplug = 1, .is_gen6 = 1,
+	.gen = 6, .is_i965g = 1, .is_i9xx = 1,
+	.need_gfx_hws = 1, .has_hotplug = 1,
 };
 
 static const struct intel_device_info intel_sandybridge_m_info = {
-	.is_i965g = 1, .is_mobile = 1, .is_i9xx = 1, .need_gfx_hws = 1,
-	.has_hotplug = 1, .is_gen6 = 1,
+	.gen = 6, .is_i965g = 1, .is_mobile = 1, .is_i9xx = 1,
+	.need_gfx_hws = 1, .has_hotplug = 1,
 };
 
 static const struct pci_device_id pciidlist[] = {		/* aka */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 11dff01..04aada0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -191,6 +191,7 @@ struct drm_i915_display_funcs {
 };
 
 struct intel_device_info {
+	u8 gen;
 	u8 is_mobile : 1;
 	u8 is_i8xx : 1;
 	u8 is_i85x : 1;
@@ -206,7 +207,6 @@ struct intel_device_info {
 	u8 is_broadwater : 1;
 	u8 is_crestline : 1;
 	u8 is_ironlake : 1;
-	u8 is_gen6 : 1;
 	u8 has_fbc : 1;
 	u8 has_rc6 : 1;
 	u8 has_pipe_cxsr : 1;
@@ -1161,7 +1161,6 @@ extern void intel_overlay_print_error_state(struct seq_file *m, struct intel_ove
 #define IS_845G(dev)		((dev)->pci_device == 0x2562)
 #define IS_I85X(dev)		(INTEL_INFO(dev)->is_i85x)
 #define IS_I865G(dev)		((dev)->pci_device == 0x2572)
-#define IS_GEN2(dev)		(INTEL_INFO(dev)->is_i8xx)
 #define IS_I915G(dev)		(INTEL_INFO(dev)->is_i915g)
 #define IS_I915GM(dev)		((dev)->pci_device == 0x2592)
 #define IS_I945G(dev)		((dev)->pci_device == 0x2772)
@@ -1180,27 +1179,13 @@ extern void intel_overlay_print_error_state(struct seq_file *m, struct intel_ove
 #define IS_IRONLAKE_M(dev)	((dev)->pci_device == 0x0046)
 #define IS_IRONLAKE(dev)	(INTEL_INFO(dev)->is_ironlake)
 #define IS_I9XX(dev)		(INTEL_INFO(dev)->is_i9xx)
-#define IS_GEN6(dev)		(INTEL_INFO(dev)->is_gen6)
 #define IS_MOBILE(dev)		(INTEL_INFO(dev)->is_mobile)
 
-#define IS_GEN3(dev)	(IS_I915G(dev) ||			\
-			 IS_I915GM(dev) ||			\
-			 IS_I945G(dev) ||			\
-			 IS_I945GM(dev) ||			\
-			 IS_G33(dev) || \
-			 IS_PINEVIEW(dev))
-#define IS_GEN4(dev)	((dev)->pci_device == 0x2972 ||		\
-			 (dev)->pci_device == 0x2982 ||		\
-			 (dev)->pci_device == 0x2992 ||		\
-			 (dev)->pci_device == 0x29A2 ||		\
-			 (dev)->pci_device == 0x2A02 ||		\
-			 (dev)->pci_device == 0x2A12 ||		\
-			 (dev)->pci_device == 0x2E02 ||		\
-			 (dev)->pci_device == 0x2E12 ||		\
-			 (dev)->pci_device == 0x2E22 ||		\
-			 (dev)->pci_device == 0x2E32 ||		\
-			 (dev)->pci_device == 0x2A42 ||		\
-			 (dev)->pci_device == 0x2E42)
+#define IS_GEN2(dev)	(INTEL_INFO(dev)->gen == 2)
+#define IS_GEN3(dev)	(INTEL_INFO(dev)->gen == 3)
+#define IS_GEN4(dev)	(INTEL_INFO(dev)->gen == 4)
+#define IS_GEN5(dev)	(INTEL_INFO(dev)->gen == 5)
+#define IS_GEN6(dev)	(INTEL_INFO(dev)->gen == 6)
 
 #define HAS_BSD(dev)            (IS_IRONLAKE(dev) || IS_G4X(dev))
 #define I915_NEED_GFX_HWS(dev)	(INTEL_INFO(dev)->need_gfx_hws)
-- 
1.7.1

[PATCH 12/29] drm/i915: Fix offset page-flips on i965+

[Chris Wilson]

i965 uses the Display Registers to compute the offset from the display
base so the new base does not need adjusting when flipping. The older
chipsets use a fence to access the display and so do perceive the
surface as linear and have a single base register which is reprogrammed
using the flip.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Reported-by: Marty Jack <martyj19@comcast.net>
---
 drivers/gpu/drm/i915/intel_display.c |   67 ++++++++++++++++++++++++----------
 1 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d50830e..e57596a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5042,9 +5042,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_unpin_work *work;
 	unsigned long flags, offset;
-	int pipesrc_reg = (intel_crtc->pipe == 0) ? PIPEASRC : PIPEBSRC;
-	int ret, pipesrc;
-	u32 flip_mask;
+	int pipe = intel_crtc->pipe;
+	u32 pf, pipesrc;
+	int ret;
 
 	work = kzalloc(sizeof *work, GFP_KERNEL);
 	if (work == NULL)
@@ -5093,12 +5093,14 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	atomic_inc(&obj_priv->pending_flip);
 	work->pending_flip_obj = obj;
 
-	if (intel_crtc->plane)
-		flip_mask = MI_WAIT_FOR_PLANE_B_FLIP;
-	else
-		flip_mask = MI_WAIT_FOR_PLANE_A_FLIP;
-
 	if (IS_GEN3(dev) || IS_GEN2(dev)) {
+		u32 flip_mask;
+
+		if (intel_crtc->plane)
+			flip_mask = MI_WAIT_FOR_PLANE_B_FLIP;
+		else
+			flip_mask = MI_WAIT_FOR_PLANE_A_FLIP;
+
 		BEGIN_LP_RING(2);
 		OUT_RING(MI_WAIT_FOR_EVENT | flip_mask);
 		OUT_RING(0);
@@ -5106,29 +5108,56 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	}
 
 	/* Offset into the new buffer for cases of shared fbs between CRTCs */
-	offset = obj_priv->gtt_offset;
-	offset += (crtc->y * fb->pitch) + (crtc->x * (fb->bits_per_pixel) / 8);
+	offset = crtc->y * fb->pitch + crtc->x * fb->bits_per_pixel/8;
 
 	BEGIN_LP_RING(4);
-	if (IS_I965G(dev)) {
+	switch(INTEL_INFO(dev)->gen) {
+	case 2:
 		OUT_RING(MI_DISPLAY_FLIP |
 			 MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 		OUT_RING(fb->pitch);
-		OUT_RING(offset | obj_priv->tiling_mode);
-		pipesrc = I915_READ(pipesrc_reg); 
-		OUT_RING(pipesrc & 0x0fff0fff);
-	} else if (IS_GEN3(dev)) {
+		OUT_RING(obj_priv->gtt_offset + offset);
+		OUT_RING(MI_NOOP);
+		break;
+
+	case 3:
 		OUT_RING(MI_DISPLAY_FLIP_I915 |
 			 MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 		OUT_RING(fb->pitch);
-		OUT_RING(offset);
+		OUT_RING(obj_priv->gtt_offset + offset);
 		OUT_RING(MI_NOOP);
-	} else {
+		break;
+
+	case 4:
+	case 5:
+		/* i965+ uses the linear or tiled offsets from the
+		 * Display Registers (which do not change across a page-flip)
+		 * so we need only reprogram the base address.
+		 */
 		OUT_RING(MI_DISPLAY_FLIP |
 			 MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 		OUT_RING(fb->pitch);
-		OUT_RING(offset);
-		OUT_RING(MI_NOOP);
+		OUT_RING(obj_priv->gtt_offset | obj_priv->tiling_mode);
+
+		/* XXX Enabling the panel-fitter across page-flip is so far
+		 * untested on non-native modes, so ignore it for now.
+		 * pf = I915_READ(pipe == 0 ? PFA_CTL_1 : PFB_CTL_1) & PF_ENABLE;
+		 */
+		pf = 0;
+		pipesrc = I915_READ(pipe == 0 ? PIPEASRC : PIPEBSRC) & 0x0fff0fff;
+		OUT_RING(pf | pipesrc);
+		break;
+
+	case 6:
+		OUT_RING(MI_DISPLAY_FLIP |
+			 MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
+		OUT_RING(fb->pitch | obj_priv->tiling_mode);
+		OUT_RING(obj_priv->gtt_offset);
+
+		pf = I915_READ(pipe == 0 ? PFA_CTL_1 : PFB_CTL_1) & PF_ENABLE;
+		pipesrc = I915_READ(pipe == 0 ? PIPEASRC : PIPEBSRC) & 0x0fff0fff;
+		OUT_RING(pf | pipesrc);
+		break;
 	}
 	ADVANCE_LP_RING();
 
-- 
1.7.1

[PATCH 13/29] drm/i915: Clear scanline waits after disabling the pipe.

[Chris Wilson]

If we disable the pipe and the GPU is currently waiting on a scanline
WAIT_FOR_EVENT, the GPU will hang. Fortunately, there is a magic bit
which we can write on i915+ to break this wait after disabling the
pipe.

References:

  Bug 29252 - [Arrandale] Hung WAIT_FOR_EVENT when running rss-glx-skyrocket
  https://bugs.freedesktop.org/show_bug.cgi?id=29252

  Bug 28964 - [i965gm] GPU infinite MI_WAIT_FOR_EVENT while watching video in Totem
  https://bugs.freedesktop.org/show_bug.cgi?id=28964

and many others.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_reg.h      |    2 ++
 drivers/gpu/drm/i915/intel_display.c |   31 +++++++++++++++++++++++++++++--
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 67e3ec1..9dedf36 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -295,6 +295,8 @@
 #define   RING_VALID_MASK	0x00000001
 #define   RING_VALID		0x00000001
 #define   RING_INVALID		0x00000000
+#define   RING_WAIT_I8XX	(1<<0) /* gen2, PRBx_HEAD */
+#define   RING_WAIT		(1<<11) /* gen3+, PRBx_CTL */
 #define PRB1_TAIL	0x02040 /* 915+ only */
 #define PRB1_HEAD	0x02044 /* 915+ only */
 #define PRB1_START	0x02048 /* 915+ only */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e57596a..b9c6487 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2412,6 +2412,26 @@ static void i9xx_crtc_dpms(struct drm_crtc *crtc, int mode)
 	}
 }
 
+/*
+ * When we disable a pipe, we need to clear any pending scanline wait events
+ * to avoid hanging the ring, which we assume we are waiting on.
+ */
+static void intel_clear_scanline_wait(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 tmp;
+
+	if (IS_GEN2(dev))
+		/* Can't break the hang on i8xx */
+		return;
+
+	tmp = I915_READ(PRB0_CTL);
+	if (tmp & RING_WAIT) {
+		I915_WRITE(PRB0_CTL, tmp);
+		POSTING_READ(PRB0_CTL);
+	}
+}
+
 /**
  * Sets the power management mode of the pipe and plane.
  */
@@ -2431,7 +2451,8 @@ static void intel_crtc_dpms(struct drm_crtc *crtc, int mode)
 	 * with multiple pipes prior to enabling to new pipe.
 	 *
 	 * When switching off the display, make sure the cursor is
-	 * properly hidden prior to disabling the pipe.
+	 * properly hidden and there are no pending waits prior to
+	 * disabling the pipe.
 	 */
 	if (mode == DRM_MODE_DPMS_ON)
 		intel_update_watermarks(dev);
@@ -2442,8 +2463,14 @@ static void intel_crtc_dpms(struct drm_crtc *crtc, int mode)
 
 	if (mode == DRM_MODE_DPMS_ON)
 		intel_crtc_update_cursor(crtc);
-	else
+	else {
+		/* XXX Note that this is not a complete solution, but a hack
+		 * to avoid the most frequently hit hang.
+		 */
+		intel_clear_scanline_wait(dev);
+
 		intel_update_watermarks(dev);
+	}
 
 	if (!dev->primary->master)
 		return;
-- 
1.7.1

[PATCH 14/29] drm/i915: Sanity check user framebuffer parameters on creation

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b9c6487..13b3292 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5460,8 +5460,25 @@ int intel_framebuffer_init(struct drm_device *dev,
 			   struct drm_mode_fb_cmd *mode_cmd,
 			   struct drm_gem_object *obj)
 {
+	struct drm_i915_gem_object *obj_priv = to_intel_bo(obj);
 	int ret;
 
+	if (obj_priv->tiling_mode == I915_TILING_Y)
+		return -EINVAL;
+
+	if (mode_cmd->pitch & 63)
+		return -EINVAL;
+
+	switch (mode_cmd->bpp) {
+	case 8:
+	case 16:
+	case 24:
+	case 32:
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
 	if (ret) {
 		DRM_ERROR("framebuffer init failed %d\n", ret);
-- 
1.7.1

[PATCH 15/29] drm/i915: Re-use set_base_atomic to share setting of the display registers

[Chris Wilson]

Lets try to avoid duplicating bugs.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |   80 ++++------------------------------
 1 files changed, 9 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 13b3292..eb0f8ff 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1489,7 +1489,7 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 			dspcntr &= ~DISPPLANE_TILED;
 	}
 
-	if (IS_IRONLAKE(dev))
+	if (HAS_PCH_SPLIT(dev))
 		/* must disable */
 		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
@@ -1498,20 +1498,19 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	Start = obj_priv->gtt_offset;
 	Offset = y * fb->pitch + x * (fb->bits_per_pixel / 8);
 
-	DRM_DEBUG("Writing base %08lX %08lX %d %d\n", Start, Offset, x, y);
+	DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
+		      Start, Offset, x, y, fb->pitch);
 	I915_WRITE(dspstride, fb->pitch);
 	if (IS_I965G(dev)) {
-		I915_WRITE(dspbase, Offset);
-		I915_READ(dspbase);
 		I915_WRITE(dspsurf, Start);
-		I915_READ(dspsurf);
 		I915_WRITE(dsptileoff, (y << 16) | x);
+		I915_WRITE(dspbase, Offset);
 	} else {
 		I915_WRITE(dspbase, Start + Offset);
-		I915_READ(dspbase);
 	}
+	POSTING_READ(dspbase);
 
-	if ((IS_I965G(dev) || plane == 0))
+	if (IS_I965G(dev) || plane == 0)
 		intel_update_fbc(crtc, &crtc->mode);
 
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
@@ -1525,7 +1524,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 		    struct drm_framebuffer *old_fb)
 {
 	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_master_private *master_priv;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_framebuffer *intel_fb;
@@ -1533,13 +1531,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 	struct drm_gem_object *obj;
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
-	unsigned long Start, Offset;
-	int dspbase = (plane == 0 ? DSPAADDR : DSPBADDR);
-	int dspsurf = (plane == 0 ? DSPASURF : DSPBSURF);
-	int dspstride = (plane == 0) ? DSPASTRIDE : DSPBSTRIDE;
-	int dsptileoff = (plane == 0 ? DSPATILEOFF : DSPBTILEOFF);
-	int dspcntr_reg = (plane == 0) ? DSPACNTR : DSPBCNTR;
-	u32 dspcntr;
 	int ret;
 
 	/* no fb bound */
@@ -1575,71 +1566,18 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 		return ret;
 	}
 
-	dspcntr = I915_READ(dspcntr_reg);
-	/* Mask out pixel format bits in case we change it */
-	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
-	switch (crtc->fb->bits_per_pixel) {
-	case 8:
-		dspcntr |= DISPPLANE_8BPP;
-		break;
-	case 16:
-		if (crtc->fb->depth == 15)
-			dspcntr |= DISPPLANE_15_16BPP;
-		else
-			dspcntr |= DISPPLANE_16BPP;
-		break;
-	case 24:
-	case 32:
-		if (crtc->fb->depth == 30)
-			dspcntr |= DISPPLANE_32BPP_30BIT_NO_ALPHA;
-		else
-			dspcntr |= DISPPLANE_32BPP_NO_ALPHA;
-		break;
-	default:
-		DRM_ERROR("Unknown color depth\n");
+	ret = intel_pipe_set_base_atomic(crtc, crtc->fb, x, y);
+	if (ret) {
 		i915_gem_object_unpin(obj);
 		mutex_unlock(&dev->struct_mutex);
-		return -EINVAL;
-	}
-	if (IS_I965G(dev)) {
-		if (obj_priv->tiling_mode != I915_TILING_NONE)
-			dspcntr |= DISPPLANE_TILED;
-		else
-			dspcntr &= ~DISPPLANE_TILED;
-	}
-
-	if (HAS_PCH_SPLIT(dev))
-		/* must disable */
-		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
-
-	I915_WRITE(dspcntr_reg, dspcntr);
-
-	Start = obj_priv->gtt_offset;
-	Offset = y * crtc->fb->pitch + x * (crtc->fb->bits_per_pixel / 8);
-
-	DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
-		      Start, Offset, x, y, crtc->fb->pitch);
-	I915_WRITE(dspstride, crtc->fb->pitch);
-	if (IS_I965G(dev)) {
-		I915_WRITE(dspsurf, Start);
-		I915_WRITE(dsptileoff, (y << 16) | x);
-		I915_WRITE(dspbase, Offset);
-	} else {
-		I915_WRITE(dspbase, Start + Offset);
+		return ret;
 	}
-	POSTING_READ(dspbase);
-
-	if ((IS_I965G(dev) || plane == 0))
-		intel_update_fbc(crtc, &crtc->mode);
-
-	intel_wait_for_vblank(dev, pipe);
 
 	if (old_fb) {
 		intel_fb = to_intel_framebuffer(old_fb);
 		obj_priv = to_intel_bo(intel_fb->obj);
 		i915_gem_object_unpin(intel_fb->obj);
 	}
-	intel_increase_pllclock(crtc);
 
 	mutex_unlock(&dev->struct_mutex);
 
-- 
1.7.1

[PATCH 16/29] drm/i915/sdvo: Propagate error from switching control buses.

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_sdvo.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 093e914..4f73cb8 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -525,8 +525,8 @@ static int intel_sdvo_get_pixel_multiplier(struct drm_display_mode *mode)
  * another I2C transaction after issuing the DDC bus switch, it will be
  * switched to the internal SDVO register.
  */
-static void intel_sdvo_set_control_bus_switch(struct intel_sdvo *intel_sdvo,
-					      u8 target)
+static int intel_sdvo_set_control_bus_switch(struct intel_sdvo *intel_sdvo,
+					     u8 target)
 {
 	u8 out_buf[2], cmd_buf[2], ret_value[2], ret;
 	struct i2c_msg msgs[] = {
@@ -567,14 +567,15 @@ static void intel_sdvo_set_control_bus_switch(struct intel_sdvo *intel_sdvo,
 	if (ret != 3) {
 		/* failure in I2C transfer */
 		DRM_DEBUG_KMS("I2c transfer returned %d\n", ret);
-		return;
+		return -EIO;
 	}
 	if (ret_value[0] != SDVO_CMD_STATUS_SUCCESS) {
 		DRM_DEBUG_KMS("DDC switch command returns response %d\n",
 					ret_value[0]);
-		return;
+		return -EIO;
 	}
-	return;
+
+	return 0;
 }
 
 static bool intel_sdvo_set_value(struct intel_sdvo *intel_sdvo, u8 cmd, const void *data, int len)
@@ -1984,6 +1985,7 @@ static int intel_sdvo_master_xfer(struct i2c_adapter *i2c_adap,
 	struct intel_sdvo *intel_sdvo;
 	struct i2c_algo_bit_data *algo_data;
 	const struct i2c_algorithm *algo;
+	int ret;
 
 	algo_data = (struct i2c_algo_bit_data *)i2c_adap->algo_data;
 	intel_sdvo =
@@ -1994,7 +1996,11 @@ static int intel_sdvo_master_xfer(struct i2c_adapter *i2c_adap,
 
 	algo = intel_sdvo->base.i2c_bus->algo;
 
-	intel_sdvo_set_control_bus_switch(intel_sdvo, intel_sdvo->ddc_bus);
+	ret = intel_sdvo_set_control_bus_switch(intel_sdvo,
+						intel_sdvo->ddc_bus);
+	if (ret)
+		return ret;
+
 	return algo->master_xfer(i2c_adap, msgs, num);
 }
 
-- 
1.7.1

[PATCH 17/29] drm/i915: Add ringbuffer wait reset to hangcheck

[Chris Wilson]

The GPU records whether it is currently waiting for a completion of a
WAIT_FOR_EVENT in the RB_WAIT bit in the ringbuffer control registers.
On third generation chipsets and later, a write of 1 to this bit breaks
the hang and returns the GPU to arbitration, i.e. the GPU should
continue executing the reminder of the batchbuffer and return to normal
operations.

By adding this to hangcheck we can avoid a full GPU reset under these
conditions.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_irq.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 16861b8..d11983d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1322,6 +1322,21 @@ void i915_hangcheck_elapsed(unsigned long data)
 	    dev_priv->last_instdone1 == instdone1) {
 		if (dev_priv->hangcheck_count++ > 1) {
 			DRM_ERROR("Hangcheck timer elapsed... GPU hung\n");
+
+			if (!IS_GEN2(dev)) {
+				/* Is the chip hanging on a WAIT_FOR_EVENT?
+				 * If so we can simply poke the RB_WAIT bit
+				 * and break the hang. This should work on
+				 * all but the second generation chipsets.
+				 */
+				u32 tmp = I915_READ(PRB0_CTL);
+				if (tmp & RING_WAIT) {
+					I915_WRITE(PRB0_CTL, tmp);
+					POSTING_READ(PRB0_CTL);
+					goto out;
+				}
+			}
+
 			i915_handle_error(dev, true);
 			return;
 		}
@@ -1333,6 +1348,7 @@ void i915_hangcheck_elapsed(unsigned long data)
 		dev_priv->last_instdone1 = instdone1;
 	}
 
+out:
 	/* Reset timer case chip hangs without another request being added */
 	mod_timer(&dev_priv->hangcheck_timer, jiffies + DRM_I915_HANGCHECK_PERIOD);
 }
-- 
1.7.1

[PATCH 18/29] drm/i915/crt: Flush register prior to waiting for vblank.

[Chris Wilson]

If we don't flush the write then we can not be sure that the border
colour will have taken effect by the time we try to read it back.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_crt.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 4b77351..f915793 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -327,6 +327,7 @@ intel_crt_load_detect(struct drm_crtc *crtc, struct intel_encoder *intel_encoder
 	if (IS_I9XX(dev)) {
 		uint32_t pipeconf = I915_READ(pipeconf_reg);
 		I915_WRITE(pipeconf_reg, pipeconf | PIPECONF_FORCE_BORDER);
+		POSTING_READ(pipeconf_reg);
 		/* Wait for next Vblank to substitue
 		 * border color for Color info */
 		intel_wait_for_vblank(dev, pipe);
-- 
1.7.1

[PATCH 19/29] drm/i915/dp: Boost timeout for enabling transcoder to 100ms

[Chris Wilson]

Adam Hill reported that his Arrandale system required a much longer, up
to 200x500us, wait for the panel to initialise or else modesetting would
fail.

References:

  https://bugs.freedesktop.org/show_bug.cgi?id=29141

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reported-by: Adam Hill <sidepipeuk@yahoo.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index eb0f8ff..c0ab92f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2056,7 +2056,7 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
 			I915_WRITE(transconf_reg, temp | TRANS_ENABLE);
 			I915_READ(transconf_reg);
 
-			if (wait_for(I915_READ(transconf_reg) & TRANS_STATE_ENABLE, 10, 0))
+			if (wait_for(I915_READ(transconf_reg) & TRANS_STATE_ENABLE, 100, 1))
 				DRM_ERROR("failed to enable transcoder\n");
 		}
 
-- 
1.7.1

[PATCH 20/29] drm/i915/sdvo: Guess the DDC bus in absence of VBIOS

[Chris Wilson]

If the VBIOS tells us the mapping of the SDVO device onto the DDC bus,
use it. However, if there is no VBIOS available that mapping is
uninitialised and we should fallback to our earlier guess.

Fix regression introduced in b1083333 (which in turn is a fix for the
regression caused by the introduction of this guess, 14571b4).

References:

  Bug 29499 - [945GM] Screen disconnected because of missing VBIOS
  https://bugs.freedesktop.org/show_bug.cgi?id=29499

  Bug 15109 - i945GM fails to detect EDID on DVI port
  https://bugzilla.kernel.org/show_bug.cgi?id=15109

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reported-and-tested-by: Paul Neumann <paul104x@yahoo.de>
Cc: Adam Jackson <ajax@redhat.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sdvo.c |   40 ++++++++++++++++++++++++++++++++++++-
 1 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 4f73cb8..41962f8 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1933,6 +1933,41 @@ static const struct drm_encoder_funcs intel_sdvo_enc_funcs = {
 	.destroy = intel_sdvo_enc_destroy,
 };
 
+static void
+intel_sdvo_guess_ddc_bus(struct intel_sdvo *sdvo)
+{
+	uint16_t mask = 0;
+	unsigned int num_bits;
+
+	/* Make a mask of outputs less than or equal to our own priority in the
+	 * list.
+	 */
+	switch (sdvo->controlled_output) {
+	case SDVO_OUTPUT_LVDS1:
+		mask |= SDVO_OUTPUT_LVDS1;
+	case SDVO_OUTPUT_LVDS0:
+		mask |= SDVO_OUTPUT_LVDS0;
+	case SDVO_OUTPUT_TMDS1:
+		mask |= SDVO_OUTPUT_TMDS1;
+	case SDVO_OUTPUT_TMDS0:
+		mask |= SDVO_OUTPUT_TMDS0;
+	case SDVO_OUTPUT_RGB1:
+		mask |= SDVO_OUTPUT_RGB1;
+	case SDVO_OUTPUT_RGB0:
+		mask |= SDVO_OUTPUT_RGB0;
+		break;
+	}
+
+	/* Count bits to find what number we are in the priority list. */
+	mask &= sdvo->caps.output_flags;
+	num_bits = hweight16(mask);
+	/* If more than 3 outputs, default to DDC bus 3 for now. */
+	if (num_bits > 3)
+		num_bits = 3;
+
+	/* Corresponds to SDVO_CONTROL_BUS_DDCx */
+	sdvo->ddc_bus = 1 << num_bits;
+}
 
 /**
  * Choose the appropriate DDC bus for control bus switch command for this
@@ -1952,7 +1987,10 @@ intel_sdvo_select_ddc_bus(struct drm_i915_private *dev_priv,
 	else
 		mapping = &(dev_priv->sdvo_mappings[1]);
 
-	sdvo->ddc_bus = 1 << ((mapping->ddc_pin & 0xf0) >> 4);
+	if (mapping->initialized)
+		sdvo->ddc_bus = 1 << ((mapping->ddc_pin & 0xf0) >> 4);
+	else
+		intel_sdvo_guess_ddc_bus(sdvo);
 }
 
 static bool
-- 
1.7.1

[PATCH 21/29] drm/i915/tv: Flush register writes before sleeping.

[Chris Wilson]

If we need to wait until the next vblank for the register to be updated
and to take effect, make sure the write is actually flushed to the register
prior to sleeping.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_tv.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index d2029ef..19b9739 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1157,10 +1157,13 @@ intel_tv_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 		I915_WRITE(dspbase_reg, I915_READ(dspbase_reg));
 
 		/* Wait for vblank for the disable to take effect */
-		if (!IS_I9XX(dev))
+		if (!IS_I9XX(dev)) {
+			POSTING_READ(dspbase_reg);
 			intel_wait_for_vblank(dev, intel_crtc->pipe);
+		}
 
 		I915_WRITE(pipeconf_reg, pipeconf & ~PIPEACONF_ENABLE);
+		POSTING_READ(pipeconf_reg);
 		/* Wait for vblank for the disable to take effect. */
 		intel_wait_for_vblank(dev, intel_crtc->pipe);
 
@@ -1268,11 +1271,15 @@ intel_tv_detect_type (struct intel_tv *intel_tv)
 		   DAC_C_0_7_V);
 	I915_WRITE(TV_CTL, tv_ctl);
 	I915_WRITE(TV_DAC, tv_dac);
+	POSTING_READ(TV_DAC);
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
+
 	tv_dac = I915_READ(TV_DAC);
 	I915_WRITE(TV_DAC, save_tv_dac);
 	I915_WRITE(TV_CTL, save_tv_ctl);
+	POSTING_READ(TV_CTL);
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
+
 	/*
 	 *  A B C
 	 *  0 1 1 Composite
-- 
1.7.1

[PATCH 22/29] drm/i915/dp: Really try 5 times before giving up.

[Chris Wilson]

Only stop trying if the aux channel sucessfully reports that the
transmission was completed, otherwise try again. On the 5th failure,
bail and report that something is amiss.

This fixes a sporadic failure in reading the EDID for my external panel
over DP.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_dp.c |   58 +++++++++++++++++++--------------------
 1 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 999ac91..b1fc65b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -239,7 +239,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	uint32_t ch_data = ch_ctl + 4;
 	int i;
 	int recv_bytes;
-	uint32_t ctl;
 	uint32_t status;
 	uint32_t aux_clock_divider;
 	int try, precharge;
@@ -263,41 +262,43 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	else
 		precharge = 5;
 
+	if (I915_READ(ch_ctl) & DP_AUX_CH_CTL_SEND_BUSY) {
+		DRM_ERROR("dp_aux_ch not started status 0x%08x\n",
+			  I915_READ(ch_ctl));
+		return -EBUSY;
+	}
+
 	/* Must try at least 3 times according to DP spec */
 	for (try = 0; try < 5; try++) {
 		/* Load the send data into the aux channel data registers */
-		for (i = 0; i < send_bytes; i += 4) {
-			uint32_t    d = pack_aux(send + i, send_bytes - i);
-	
-			I915_WRITE(ch_data + i, d);
-		}
-	
-		ctl = (DP_AUX_CH_CTL_SEND_BUSY |
-		       DP_AUX_CH_CTL_TIME_OUT_400us |
-		       (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
-		       (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
-		       (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT) |
-		       DP_AUX_CH_CTL_DONE |
-		       DP_AUX_CH_CTL_TIME_OUT_ERROR |
-		       DP_AUX_CH_CTL_RECEIVE_ERROR);
+		for (i = 0; i < send_bytes; i += 4)
+			I915_WRITE(ch_data + i,
+				   pack_aux(send + i, send_bytes - i));
 	
 		/* Send the command and wait for it to complete */
-		I915_WRITE(ch_ctl, ctl);
-		(void) I915_READ(ch_ctl);
+		I915_WRITE(ch_ctl,
+			   DP_AUX_CH_CTL_SEND_BUSY |
+			   DP_AUX_CH_CTL_TIME_OUT_400us |
+			   (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
+			   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
+			   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT) |
+			   DP_AUX_CH_CTL_DONE |
+			   DP_AUX_CH_CTL_TIME_OUT_ERROR |
+			   DP_AUX_CH_CTL_RECEIVE_ERROR);
 		for (;;) {
-			udelay(100);
 			status = I915_READ(ch_ctl);
 			if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0)
 				break;
+			udelay(100);
 		}
 	
 		/* Clear done status and any errors */
-		I915_WRITE(ch_ctl, (status |
-				DP_AUX_CH_CTL_DONE |
-				DP_AUX_CH_CTL_TIME_OUT_ERROR |
-				DP_AUX_CH_CTL_RECEIVE_ERROR));
-		(void) I915_READ(ch_ctl);
-		if ((status & DP_AUX_CH_CTL_TIME_OUT_ERROR) == 0)
+		I915_WRITE(ch_ctl,
+			   status |
+			   DP_AUX_CH_CTL_DONE |
+			   DP_AUX_CH_CTL_TIME_OUT_ERROR |
+			   DP_AUX_CH_CTL_RECEIVE_ERROR);
+		if (status & DP_AUX_CH_CTL_DONE)
 			break;
 	}
 
@@ -324,15 +325,12 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	/* Unload any bytes sent back from the other side */
 	recv_bytes = ((status & DP_AUX_CH_CTL_MESSAGE_SIZE_MASK) >>
 		      DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT);
-
 	if (recv_bytes > recv_size)
 		recv_bytes = recv_size;
 	
-	for (i = 0; i < recv_bytes; i += 4) {
-		uint32_t    d = I915_READ(ch_data + i);
-
-		unpack_aux(d, recv + i, recv_bytes - i);
-	}
+	for (i = 0; i < recv_bytes; i += 4)
+		unpack_aux(I915_READ(ch_data + i),
+			   recv + i, recv_bytes - i);
 
 	return recv_bytes;
 }
-- 
1.7.1

Re: [PATCH 22/29] drm/i915/dp: Really try 5 times before giving up.

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 496 bytes --]

On Sun, 22 Aug 2010 12:05:41 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Only stop trying if the aux channel sucessfully reports that the
> transmission was completed, otherwise try again. On the 5th failure,
> bail and report that something is amiss.
> 
> This fixes a sporadic failure in reading the EDID for my external panel
> over DP.

It's hard to find the actual change in here among all the reformatting.
I did eventually, but I'd rather see 2 patches in that case.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 23/29] drm/i915/debug: Include Ironlake in self-refresh status

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 92d5605..744a2a7 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -626,15 +626,17 @@ static int i915_sr_status(struct seq_file *m, void *unused)
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	bool sr_enabled = false;
 
-	if (IS_I965GM(dev) || IS_I945G(dev) || IS_I945GM(dev))
+	if (IS_IRONLAKE(dev))
+		sr_enabled = I915_READ(WM1_LP_ILK) & WM1_LP_SR_EN;
+	else if (IS_I965GM(dev) || IS_I945G(dev) || IS_I945GM(dev))
 		sr_enabled = I915_READ(FW_BLC_SELF) & FW_BLC_SELF_EN;
 	else if (IS_I915GM(dev))
 		sr_enabled = I915_READ(INSTPM) & INSTPM_SELF_EN;
 	else if (IS_PINEVIEW(dev))
 		sr_enabled = I915_READ(DSPFW3) & PINEVIEW_SELF_REFRESH_EN;
 
-	seq_printf(m, "self-refresh: %s\n", sr_enabled ? "enabled" :
-		   "disabled");
+	seq_printf(m, "self-refresh: %s\n",
+		   sr_enabled ? "enabled" : "disabled");
 
 	return 0;
 }
-- 
1.7.1

[PATCH 24/29] drm/i915: Allocate the PCI resource for the MCHBAR

[Chris Wilson]

We were failing when trying to allocate the resource for MMIO of the
MCHBAR because we forgot to specify what type of resource we wanted.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_dma.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 77af26d..f3433ba 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -885,7 +885,7 @@ intel_alloc_mchbar_resource(struct drm_device *dev)
 	int reg = IS_I965G(dev) ? MCHBAR_I965 : MCHBAR_I915;
 	u32 temp_lo, temp_hi = 0;
 	u64 mchbar_addr;
-	int ret = 0;
+	int ret;
 
 	if (IS_I965G(dev))
 		pci_read_config_dword(dev_priv->bridge_dev, reg + 4, &temp_hi);
@@ -895,22 +895,23 @@ intel_alloc_mchbar_resource(struct drm_device *dev)
 	/* If ACPI doesn't have it, assume we need to allocate it ourselves */
 #ifdef CONFIG_PNP
 	if (mchbar_addr &&
-	    pnp_range_reserved(mchbar_addr, mchbar_addr + MCHBAR_SIZE)) {
-		ret = 0;
-		goto out;
-	}
+	    pnp_range_reserved(mchbar_addr, mchbar_addr + MCHBAR_SIZE))
+		return 0;
 #endif
 
 	/* Get some space for it */
-	ret = pci_bus_alloc_resource(dev_priv->bridge_dev->bus, &dev_priv->mch_res,
+	dev_priv->mch_res.name = "i915 MCHBAR";
+	dev_priv->mch_res.flags = IORESOURCE_MEM;
+	ret = pci_bus_alloc_resource(dev_priv->bridge_dev->bus,
+				     &dev_priv->mch_res,
 				     MCHBAR_SIZE, MCHBAR_SIZE,
 				     PCIBIOS_MIN_MEM,
-				     0,   pcibios_align_resource,
+				     0, pcibios_align_resource,
 				     dev_priv->bridge_dev);
 	if (ret) {
 		DRM_DEBUG_DRIVER("failed bus alloc: %d\n", ret);
 		dev_priv->mch_res.start = 0;
-		goto out;
+		return ret;
 	}
 
 	if (IS_I965G(dev))
@@ -919,8 +920,7 @@ intel_alloc_mchbar_resource(struct drm_device *dev)
 
 	pci_write_config_dword(dev_priv->bridge_dev, reg,
 			       lower_32_bits(dev_priv->mch_res.start));
-out:
-	return ret;
+	return 0;
 }
 
 /* Setup MCHBAR if possible, return true if we should disable it again */
-- 
1.7.1

[PATCH 25/29] drm/i915: Use the VBT from OpRegion when available (v2)

[Chris Wilson]

It is recommended that we use the Video BIOS tables that were copied
into the OpRegion during POST when initialising the driver. This saves
us from having to furtle around inside the ROM ourselves and possibly
allows the vBIOS to adjust the tables prior to initialisation.

On some systems, such as the Samsung N210, there is no accessible VBIOS
and the only means of finding the VBT is through the OpRegion.

v2: Rearrange the code so that ASLE is enabled along with ACPI

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |   21 ++++++++++
 drivers/gpu/drm/i915/i915_dma.c      |   11 ++++-
 drivers/gpu/drm/i915/i915_drv.c      |    7 ++-
 drivers/gpu/drm/i915/i915_drv.h      |    7 ++-
 drivers/gpu/drm/i915/i915_opregion.c |   74 +++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_bios.c    |   69 ++++++++++++++++++--------------
 6 files changed, 115 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 744a2a7..8352679 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -671,6 +671,26 @@ static int i915_gfxec(struct seq_file *m, void *unused)
 	return 0;
 }
 
+static int i915_opregion(struct seq_file *m, void *unused)
+{
+	struct drm_info_node *node = (struct drm_info_node *) m->private;
+	struct drm_device *dev = node->minor->dev;
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct intel_opregion *opregion = &dev_priv->opregion;
+	struct opregion_header {
+		u8 signature[16];
+		u32 size;
+	} __attribute__((packed)) *hdr;
+
+	if (!opregion->header)
+		return 0;
+
+	hdr = (struct opregion_header *)opregion->header;
+	seq_write(m, hdr, hdr->size * 1024);
+
+	return 0;
+}
+
 static int
 i915_wedged_open(struct inode *inode,
 		 struct file *filp)
@@ -797,6 +817,7 @@ static struct drm_info_list i915_debugfs_list[] = {
 	{"i915_gfxec", i915_gfxec, 0},
 	{"i915_fbc_status", i915_fbc_status, 0},
 	{"i915_sr_status", i915_sr_status, 0},
+	{"i915_opregion", i915_opregion, 0},
 };
 #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
 
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index f3433ba..85c86fc 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -40,6 +40,7 @@
 #include <linux/pnp.h>
 #include <linux/vga_switcheroo.h>
 #include <linux/slab.h>
+#include <acpi/video.h>
 
 extern int intel_max_stolen; /* from AGP driver */
 
@@ -2156,6 +2157,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	/* Try to make sure MCHBAR is enabled before poking at it */
 	intel_setup_mchbar(dev);
+	intel_opregion_setup(dev);
 
 	i915_gem_load(dev);
 
@@ -2211,7 +2213,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	}
 
 	/* Must be done after probing outputs */
-	intel_opregion_init(dev, 0);
+	intel_opregion_init(dev);
+	if(dev_priv->opregion.acpi)
+		acpi_video_register();
 
 	setup_timer(&dev_priv->hangcheck_timer, i915_hangcheck_elapsed,
 		    (unsigned long) dev);
@@ -2261,6 +2265,9 @@ int i915_driver_unload(struct drm_device *dev)
 		dev_priv->mm.gtt_mtrr = -1;
 	}
 
+	if (dev_priv->opregion.acpi)
+		acpi_video_unregister();
+
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		intel_modeset_cleanup(dev);
 
@@ -2289,7 +2296,7 @@ int i915_driver_unload(struct drm_device *dev)
 	if (dev_priv->regs != NULL)
 		iounmap(dev_priv->regs);
 
-	intel_opregion_free(dev, 0);
+	intel_opregion_fini(dev);
 
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		/* Flush any outstanding unpin_work. */
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5363985..0236b07 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -232,7 +232,7 @@ static int i915_drm_freeze(struct drm_device *dev)
 
 	i915_save_state(dev);
 
-	intel_opregion_free(dev, 1);
+	intel_opregion_fini(dev);
 
 	/* Modeset on resume, not lid events */
 	dev_priv->modeset_on_lid = 0;
@@ -272,8 +272,7 @@ static int i915_drm_thaw(struct drm_device *dev)
 	int error = 0;
 
 	i915_restore_state(dev);
-
-	intel_opregion_init(dev, 1);
+	intel_opregion_setup(dev);
 
 	/* KMS EnterVT equivalent */
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
@@ -289,6 +288,8 @@ static int i915_drm_thaw(struct drm_device *dev)
 		drm_helper_resume_force_mode(dev);
 	}
 
+	intel_opregion_init(dev);
+
 	dev_priv->modeset_on_lid = 0;
 
 	return error;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 04aada0..3b0f770 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -110,7 +110,7 @@ struct intel_opregion {
 	struct opregion_acpi *acpi;
 	struct opregion_swsci *swsci;
 	struct opregion_asle *asle;
-	int enabled;
+	void *vbt;
 };
 
 struct intel_overlay;
@@ -1055,8 +1055,9 @@ extern int i915_restore_state(struct drm_device *dev);
 
 #ifdef CONFIG_ACPI
 /* i915_opregion.c */
-extern int intel_opregion_init(struct drm_device *dev, int resume);
-extern void intel_opregion_free(struct drm_device *dev, int suspend);
+extern int intel_opregion_setup(struct drm_device *dev);
+extern void intel_opregion_init(struct drm_device *dev);
+extern void intel_opregion_fini(struct drm_device *dev);
 extern void opregion_asle_intr(struct drm_device *dev);
 extern void ironlake_opregion_gse_intr(struct drm_device *dev);
 extern void opregion_enable_asle(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_opregion.c b/drivers/gpu/drm/i915/i915_opregion.c
index ea5d3fe..b4c851f 100644
--- a/drivers/gpu/drm/i915/i915_opregion.c
+++ b/drivers/gpu/drm/i915/i915_opregion.c
@@ -41,7 +41,7 @@
 #define OPREGION_ACPI_OFFSET   0x100
 #define OPREGION_SWSCI_OFFSET  0x200
 #define OPREGION_ASLE_OFFSET   0x300
-#define OPREGION_VBT_OFFSET    0x1000
+#define OPREGION_VBT_OFFSET    0x400
 
 #define OPREGION_SIGNATURE "IntelGraphicsMem"
 #define MBOX_ACPI      (1<<0)
@@ -464,7 +464,7 @@ blind_set:
 	goto end;
 }
 
-int intel_opregion_init(struct drm_device *dev, int resume)
+int intel_opregion_setup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_opregion *opregion = &dev_priv->opregion;
@@ -483,25 +483,19 @@ int intel_opregion_init(struct drm_device *dev, int resume)
 	if (!base)
 		return -ENOMEM;
 
-	opregion->header = base;
-	if (memcmp(opregion->header->signature, OPREGION_SIGNATURE, 16)) {
+	if (memcmp(base, OPREGION_SIGNATURE, 16)) {
 		DRM_DEBUG_DRIVER("opregion signature mismatch\n");
 		err = -EINVAL;
 		goto err_out;
 	}
+	opregion->header = base;
+	opregion->vbt = base + OPREGION_VBT_OFFSET;
 
 	mboxes = opregion->header->mboxes;
 	if (mboxes & MBOX_ACPI) {
 		DRM_DEBUG_DRIVER("Public ACPI methods supported\n");
 		opregion->acpi = base + OPREGION_ACPI_OFFSET;
-		if (drm_core_check_feature(dev, DRIVER_MODESET))
-			intel_didl_outputs(dev);
-	} else {
-		DRM_DEBUG_DRIVER("Public ACPI methods not supported\n");
-		err = -ENOTSUPP;
-		goto err_out;
 	}
-	opregion->enabled = 1;
 
 	if (mboxes & MBOX_SWSCI) {
 		DRM_DEBUG_DRIVER("SWSCI supported\n");
@@ -510,46 +504,55 @@ int intel_opregion_init(struct drm_device *dev, int resume)
 	if (mboxes & MBOX_ASLE) {
 		DRM_DEBUG_DRIVER("ASLE supported\n");
 		opregion->asle = base + OPREGION_ASLE_OFFSET;
-		opregion_enable_asle(dev);
 	}
 
-	if (!resume)
-		acpi_video_register();
-
-
-	/* Notify BIOS we are ready to handle ACPI video ext notifs.
-	 * Right now, all the events are handled by the ACPI video module.
-	 * We don't actually need to do anything with them. */
-	opregion->acpi->csts = 0;
-	opregion->acpi->drdy = 1;
-
-	system_opregion = opregion;
-	register_acpi_notifier(&intel_opregion_notifier);
-
 	return 0;
 
 err_out:
 	iounmap(opregion->header);
-	opregion->header = NULL;
-	acpi_video_register();
 	return err;
 }
 
-void intel_opregion_free(struct drm_device *dev, int suspend)
+void intel_opregion_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_opregion *opregion = &dev_priv->opregion;
 
-	if (!opregion->enabled)
+	if (!opregion->header)
 		return;
 
-	if (!suspend)
-		acpi_video_unregister();
+	if (opregion->acpi) {
+		if (drm_core_check_feature(dev, DRIVER_MODESET))
+			intel_didl_outputs(dev);
 
-	opregion->acpi->drdy = 0;
+		/* Notify BIOS we are ready to handle ACPI video ext notifs.
+		 * Right now, all the events are handled by the ACPI video module.
+		 * We don't actually need to do anything with them. */
+		opregion->acpi->csts = 0;
+		opregion->acpi->drdy = 1;
 
-	system_opregion = NULL;
-	unregister_acpi_notifier(&intel_opregion_notifier);
+		system_opregion = opregion;
+		register_acpi_notifier(&intel_opregion_notifier);
+	}
+
+	if (opregion->asle)
+		opregion_enable_asle(dev);
+}
+
+void intel_opregion_fini(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_opregion *opregion = &dev_priv->opregion;
+
+	if (!opregion->header)
+		return;
+
+	if (opregion->acpi) {
+		opregion->acpi->drdy = 0;
+
+		system_opregion = NULL;
+		unregister_acpi_notifier(&intel_opregion_notifier);
+	}
 
 	/* just clear all opregion memory pointers now */
 	iounmap(opregion->header);
@@ -557,6 +560,5 @@ void intel_opregion_free(struct drm_device *dev, int suspend)
 	opregion->acpi = NULL;
 	opregion->swsci = NULL;
 	opregion->asle = NULL;
-
-	opregion->enabled = 0;
+	opregion->vbt = NULL;
 }
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 96f75d7..867136b 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -317,7 +317,7 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
 
 static void
 parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
-		       struct bdb_header *bdb)
+			  struct bdb_header *bdb)
 {
 	struct sdvo_device_mapping *p_mapping;
 	struct bdb_general_definitions *p_defs;
@@ -327,7 +327,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
 
 	p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
 	if (!p_defs) {
-		DRM_DEBUG_KMS("No general definition block is found\n");
+		DRM_DEBUG_KMS("No general definition block is found, unable to construct sdvo mapping.\n");
 		return;
 	}
 	/* judge whether the size of child device meets the requirements.
@@ -460,7 +460,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
 
 	p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
 	if (!p_defs) {
-		DRM_DEBUG_KMS("No general definition block is found\n");
+		DRM_DEBUG_KMS("No general definition block is found, no devices defined.\n");
 		return;
 	}
 	/* judge whether the size of child device meets the requirements.
@@ -513,6 +513,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
 	}
 	return;
 }
+
 /**
  * intel_init_bios - initialize VBIOS settings & find VBT
  * @dev: DRM device
@@ -520,11 +521,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
  * Loads the Video BIOS and checks that the VBT exists.  Sets scratch registers
  * to appropriate values.
  *
- * VBT existence is a sanity check that is relied on by other i830_bios.c code.
- * Note that it would be better to use a BIOS call to get the VBT, as BIOSes may
- * feed an updated VBT back through that, compared to what we'll fetch using
- * this method of groping around in the BIOS data.
- *
  * Returns 0 on success, nonzero on failure.
  */
 bool
@@ -532,31 +528,43 @@ intel_init_bios(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct pci_dev *pdev = dev->pdev;
-	struct vbt_header *vbt = NULL;
-	struct bdb_header *bdb;
-	u8 __iomem *bios;
-	size_t size;
-	int i;
-
-	bios = pci_map_rom(pdev, &size);
-	if (!bios)
-		return -1;
-
-	/* Scour memory looking for the VBT signature */
-	for (i = 0; i + 4 < size; i++) {
-		if (!memcmp(bios + i, "$VBT", 4)) {
-			vbt = (struct vbt_header *)(bios + i);
-			break;
+	struct bdb_header *bdb = NULL;
+	u8 __iomem *bios = NULL;
+
+	if (dev_priv->opregion.vbt) {
+		struct vbt_header *vbt = dev_priv->opregion.vbt;
+		if (memcmp(vbt->signature, "$VBT", 4) == 0) {
+			DRM_DEBUG_DRIVER("Using VBT from OpRegion: %20s\n",
+					 vbt->signature);
+			bdb = (struct bdb_header *)((char *)vbt + vbt->bdb_offset);
 		}
 	}
 
-	if (!vbt) {
-		DRM_ERROR("VBT signature missing\n");
-		pci_unmap_rom(pdev, bios);
-		return -1;
-	}
+	if (bdb == NULL) {
+		struct vbt_header *vbt = NULL;
+		size_t size;
+		int i;
+
+		bios = pci_map_rom(pdev, &size);
+		if (!bios)
+			return -1;
 
-	bdb = (struct bdb_header *)(bios + i + vbt->bdb_offset);
+		/* Scour memory looking for the VBT signature */
+		for (i = 0; i + 4 < size; i++) {
+			if (!memcmp(bios + i, "$VBT", 4)) {
+				vbt = (struct vbt_header *)(bios + i);
+				break;
+			}
+		}
+
+		if (!vbt) {
+			DRM_ERROR("VBT signature missing\n");
+			pci_unmap_rom(pdev, bios);
+			return -1;
+		}
+
+		bdb = (struct bdb_header *)(bios + i + vbt->bdb_offset);
+	}
 
 	/* Grab useful general definitions */
 	parse_general_features(dev_priv, bdb);
@@ -568,7 +576,8 @@ intel_init_bios(struct drm_device *dev)
 	parse_driver_features(dev_priv, bdb);
 	parse_edp(dev_priv, bdb);
 
-	pci_unmap_rom(pdev, bios);
+	if (bios)
+		pci_unmap_rom(pdev, bios);
 
 	return 0;
 }
-- 
1.7.1

[PATCH 26/29] drm/i915: Invert watermarks used for i8xx, i9xx.

[Chris Wilson]

The calculation seem backwards as they compute the number of entries
that would be drained during a memory fetch, and but return the opposite
value.

This fixes a common flicker [or even completely scrambled display] on
high resolution outputs (e.g. 1920x1080 with 915GM). However, as we do not
adjust DSPARB, we are still limited by the FIFO setup by the BIOS which may
continue to cause underruns with high resolutions on some machines.

References:

  Bug 22996 - [945GM] external lcd does not work on high resolution
  https://bugs.freedesktop.org/show_bug.cgi?id=22996

  Bug 25284 - [915GM] random flickering when using metacity-compositor
  https://bugs.freedesktop.org/show_bug.cgi?id=25284

  Bug 25857 - [945]: Entire screen "jerks" randomly after removing external monitor
  https://bugs.freedesktop.org/show_bug.cgi?id=25857

  Bug 27738 - [915] 1920x1200 resolution fails, probable FIFO issue.
  https://bugs.freedesktop.org/show_bug.cgi?id=27738

  Bug 28149 - [945GM] KMS regression.
  https://bugs.freedesktop.org/show_bug.cgi?id=28149

Also it may address some instances where the monitor fails to sync
due to underruns.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |   26 +++++++++-----------------
 1 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c0ab92f..e574e6a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2762,7 +2762,7 @@ static unsigned long intel_calculate_wm(unsigned long clock_in_khz,
 					int pixel_size,
 					unsigned long latency_ns)
 {
-	long entries_required, wm_size;
+	long entries;
 
 	/*
 	 * Note: we need to make sure we don't overflow for various clock &
@@ -2770,28 +2770,20 @@ static unsigned long intel_calculate_wm(unsigned long clock_in_khz,
 	 * clocks go from a few thousand to several hundred thousand.
 	 * latency is usually a few thousand
 	 */
-	entries_required = ((clock_in_khz / 1000) * pixel_size * latency_ns) /
-		1000;
-	entries_required = DIV_ROUND_UP(entries_required, wm->cacheline_size);
-
-	DRM_DEBUG_KMS("FIFO entries required for mode: %d\n", entries_required);
-
-	wm_size = wm->fifo_size - (entries_required + wm->guard_size);
+	entries = ((clock_in_khz / 1000) * pixel_size * latency_ns) / 1000;
+	entries = DIV_ROUND_UP(entries, wm->cacheline_size);
+	entries += wm->guard_size;
 
-	DRM_DEBUG_KMS("FIFO watermark level: %d\n", wm_size);
+	DRM_DEBUG_KMS("FIFO entries required for mode: %d\n", entries);
 
-	/* Don't promote wm_size to unsigned... */
-	if (wm_size > (long)wm->max_wm)
-		wm_size = wm->max_wm;
-	if (wm_size <= 0) {
-		wm_size = wm->default_wm;
+	if (entries >= (long)wm->fifo_size) {
 		DRM_ERROR("Insufficient FIFO for plane, expect flickering:"
 			  " entries required = %ld, available = %lu.\n",
-			  entries_required + wm->guard_size,
-			  wm->fifo_size);
+			  entries, wm->fifo_size);
+		entries = wm->fifo_size - 1;
 	}
 
-	return wm_size;
+	return entries;
 }
 
 struct cxsr_latency {
-- 
1.7.1

Re: [PATCH 26/29] drm/i915: Invert watermarks used for i8xx, i9xx.

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1473 bytes --]

On Sun, 22 Aug 2010 12:05:45 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> The calculation seem backwards as they compute the number of entries
> that would be drained during a memory fetch, and but return the opposite
> value.
> 
> This fixes a common flicker [or even completely scrambled display] on
> high resolution outputs (e.g. 1920x1080 with 915GM). However, as we do not
> adjust DSPARB, we are still limited by the FIFO setup by the BIOS which may
> continue to cause underruns with high resolutions on some machines.
> 
> References:
> 
>   Bug 22996 - [945GM] external lcd does not work on high resolution
>   https://bugs.freedesktop.org/show_bug.cgi?id=22996
> 
>   Bug 25284 - [915GM] random flickering when using metacity-compositor
>   https://bugs.freedesktop.org/show_bug.cgi?id=25284
> 
>   Bug 25857 - [945]: Entire screen "jerks" randomly after removing external monitor
>   https://bugs.freedesktop.org/show_bug.cgi?id=25857
> 
>   Bug 27738 - [915] 1920x1200 resolution fails, probable FIFO issue.
>   https://bugs.freedesktop.org/show_bug.cgi?id=27738
> 
>   Bug 28149 - [945GM] KMS regression.
>   https://bugs.freedesktop.org/show_bug.cgi?id=28149
> 
> Also it may address some instances where the monitor fails to sync
> due to underruns.

What's up with all the bug references here?  I hit the first 3 and
didn't see any mention of this patch.  I'd probably prefer to see either
Tested-by or nothing.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 27/29] drm: Scan EDID for an audio-capable HDMI output

[Chris Wilson]

Parse the EDID looking for a CEA-extension block that details whether
the connected monitor has audio support over HDMI.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_edid.c |   27 +++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |    1 +
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 96e9631..922b325 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1326,6 +1326,33 @@ end:
 EXPORT_SYMBOL(drm_detect_hdmi_monitor);
 
 /**
+ * drm_edid_supports_audio - detect whether monitor supports audio.
+ * @edid: monitor EDID information
+ *
+ * Parse the CEA extension according to CEA-861-B.
+ * Return true if a monitor is connected that supports basic audio,
+ * false if not or unknown.
+ */
+bool drm_edid_supports_audio(struct edid *edid)
+{
+	int i;
+
+	/* No EDID or EDID extensions */
+	if (edid == NULL || edid->extensions == 0)
+		return false;
+
+	/* Find CEA extension */
+	for (i = 0; i < edid->extensions; i++) {
+		const char *edid_ext = (char *)edid + EDID_LENGTH * (i + 1);
+		if (edid_ext[0] == 0x02)
+			return edid_ext[3] & (1 << 6);
+	}
+
+	return false;
+}
+EXPORT_SYMBOL(drm_edid_supports_audio);
+
+/**
  * drm_add_edid_modes - add modes from EDID data, if available
  * @connector: connector we're probing
  * @edid: edid data
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index c9f3cc5..c817796 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -754,6 +754,7 @@ extern int drm_mode_gamma_get_ioctl(struct drm_device *dev,
 extern int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 				    void *data, struct drm_file *file_priv);
 extern bool drm_detect_hdmi_monitor(struct edid *edid);
+extern bool drm_edid_supports_audio(struct edid *edid);
 extern int drm_mode_page_flip_ioctl(struct drm_device *dev,
 				    void *data, struct drm_file *file_priv);
 extern struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
-- 
1.7.1

Re: [PATCH 27/29] drm: Scan EDID for an audio-capable HDMI output

[Adam Jackson]

[-- Attachment #1.1: Type: text/plain, Size: 2315 bytes --]

On Sun, 2010-08-22 at 12:05 +0100, Chris Wilson wrote:
> Parse the EDID looking for a CEA-extension block that details whether
> the connected monitor has audio support over HDMI.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/drm_edid.c |   27 +++++++++++++++++++++++++++
>  include/drm/drm_crtc.h     |    1 +
>  2 files changed, 28 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 96e9631..922b325 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1326,6 +1326,33 @@ end:
>  EXPORT_SYMBOL(drm_detect_hdmi_monitor);
>  
>  /**
> + * drm_edid_supports_audio - detect whether monitor supports audio.
> + * @edid: monitor EDID information
> + *
> + * Parse the CEA extension according to CEA-861-B.
> + * Return true if a monitor is connected that supports basic audio,
> + * false if not or unknown.
> + */
> +bool drm_edid_supports_audio(struct edid *edid)
> +{
> +	int i;
> +
> +	/* No EDID or EDID extensions */
> +	if (edid == NULL || edid->extensions == 0)
> +		return false;
> +
> +	/* Find CEA extension */
> +	for (i = 0; i < edid->extensions; i++) {
> +		const char *edid_ext = (char *)edid + EDID_LENGTH * (i + 1);
> +		if (edid_ext[0] == 0x02)
> +			return edid_ext[3] & (1 << 6);
> +	}
> +
> +	return false;
> +}

Nack, CEA is worse than that.  What you have here is "does the zero'th
CEA subblock's type field have this one bit set", which isn't a
particularly meaningful thing to check.  In particular this will say
_any_ HDMI sink (with the HDMI vendor-specific block first in the CEA
block collection) is an audio sink.

You have to:
- check that the CEA version field is high enough
- scan for a vendor-specific data block
- check that its OUI matches the HDMI association OUI
- check that its length is 6 or greater so that you know the byte
containing the audio feature field is present
- check whether the top bit of that byte is set (indicating it can
consume ACP, ISRC1, or ISRC2 packets)

Refer to:

http://cgit.freedesktop.org/xorg/app/edid-decode/tree/edid-decode.c#n564

in particular the calls down to cea_block() and then cea_hdmi_block().

- ajax

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 28/29] drm/i915/hdmi: Only enable audio if supported by the monitor

[Chris Wilson]

References:

  Bug 26864 - Green Screen in LG Projector using HDMI output
  https://bugs.freedesktop.org/show_bug.cgi?id=26864

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_hdmi.c |   12 ++++++++----
 drivers/gpu/drm/i915/intel_sdvo.c |   11 +++++++----
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index ccd4c97..88faa0b 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -40,6 +40,7 @@
 struct intel_hdmi {
 	struct intel_encoder base;
 	u32 sdvox_reg;
+	bool has_audio_sink;
 	bool has_hdmi_sink;
 };
 
@@ -65,11 +66,10 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
 		sdvox |= SDVO_HSYNC_ACTIVE_HIGH;
 
-	if (intel_hdmi->has_hdmi_sink) {
+	if (intel_hdmi->has_audio_sink)
 		sdvox |= SDVO_AUDIO_ENABLE;
-		if (HAS_PCH_CPT(dev))
-			sdvox |= HDMI_MODE_SELECT;
-	}
+	if (intel_hdmi->has_hdmi_sink && HAS_PCH_CPT(dev))
+		sdvox |= HDMI_MODE_SELECT;
 
 	if (intel_crtc->pipe == 1) {
 		if (HAS_PCH_CPT(dev))
@@ -147,12 +147,16 @@ intel_hdmi_detect(struct drm_connector *connector)
 	enum drm_connector_status status = connector_status_disconnected;
 
 	intel_hdmi->has_hdmi_sink = false;
+	intel_hdmi->has_audio_sink = false;
+
 	edid = drm_get_edid(connector, intel_hdmi->base.ddc_bus);
 
 	if (edid) {
 		if (edid->input & DRM_EDID_INPUT_DIGITAL) {
 			status = connector_status_connected;
 			intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
+			if (intel_hdmi->has_hdmi_sink)
+				intel_hdmi->has_audio_sink = drm_edid_supports_audio(edid);
 		}
 		connector->display_info.raw_edid = NULL;
 		kfree(edid);
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 41962f8..dfaa547 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -104,6 +104,7 @@ struct intel_sdvo {
 	 * This is set if we treat the device as HDMI, instead of DVI.
 	 */
 	bool is_hdmi;
+	bool has_hdmi_audio;
 
 	/**
 	 * This is set if we detect output of sdvo device as LVDS.
@@ -1114,12 +1115,12 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
 				  &in_out, sizeof(in_out)))
 		return;
 
-	if (intel_sdvo->is_hdmi) {
+	if (intel_sdvo->is_hdmi)
 		if (!intel_sdvo_set_avi_infoframe(intel_sdvo, mode))
 			return;
 
+	if (intel_sdvo->has_hdmi_audio)
 		sdvox |= SDVO_AUDIO_ENABLE;
-	}
 
 	/* We have tried to get input timing in mode_fixup, and filled into
 	   adjusted_mode */
@@ -1476,9 +1477,11 @@ intel_sdvo_hdmi_sink_detect(struct drm_connector *connector)
 		bool need_digital = !!(intel_sdvo_connector->output_flag & SDVO_TMDS_MASK);
 
 		/* DDC bus is shared, match EDID to connector type */
-		if (is_digital && need_digital)
+		if (is_digital && need_digital) {
 			intel_sdvo->is_hdmi = drm_detect_hdmi_monitor(edid);
-		else if (is_digital != need_digital)
+			if (intel_sdvo->is_hdmi)
+				intel_sdvo->has_hdmi_audio = drm_edid_supports_audio(edid);
+		} else if (is_digital != need_digital)
 			status = connector_status_disconnected;
 
 		connector->display_info.raw_edid = NULL;
-- 
1.7.1

[PATCH 29/29] drm/i915: Tightly scope intel_encoder to prevent invalid use

[Chris Wilson]

We reset intel_encoder for every matching encoder whilst iterating over
the encoders attached to this crtc when changing mode. As such in a
cloned configuration intel_encoder may not correspond to the correct
is_edp encoder.

By scoping intel_encoder to the loop, not only is the compiler able to
spot this mistake, we also improve readiability for ourselves.
[It might not be a mistake, within this function it is unclear as to
whether it is permissable for eDP to be cloned...]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |   25 ++++++++++++-------------
 1 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e574e6a..dd7fb7f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3514,10 +3514,9 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
 	u32 dpll = 0, fp = 0, fp2 = 0, dspcntr, pipeconf;
 	bool ok, has_reduced_clock = false, is_sdvo = false, is_dvo = false;
 	bool is_crt = false, is_lvds = false, is_tv = false, is_dp = false;
-	bool is_edp = false;
+	struct intel_encoder *has_edp_encoder = NULL;
 	struct drm_mode_config *mode_config = &dev->mode_config;
 	struct drm_encoder *encoder;
-	struct intel_encoder *intel_encoder = NULL;
 	const intel_limit_t *limit;
 	int ret;
 	struct fdi_m_n m_n = {0};
@@ -3538,12 +3537,12 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
 	drm_vblank_pre_modeset(dev, pipe);
 
 	list_for_each_entry(encoder, &mode_config->encoder_list, head) {
+		struct intel_encoder *intel_encoder = NULL;
 
-		if (!encoder || encoder->crtc != crtc)
+		if (encoder->crtc != crtc)
 			continue;
 
 		intel_encoder = enc_to_intel_encoder(encoder);
-
 		switch (intel_encoder->type) {
 		case INTEL_OUTPUT_LVDS:
 			is_lvds = true;
@@ -3567,7 +3566,7 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
 			is_dp = true;
 			break;
 		case INTEL_OUTPUT_EDP:
-			is_edp = true;
+			has_edp_encoder = intel_encoder;
 			break;
 		}
 
@@ -3645,10 +3644,10 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
 		int lane = 0, link_bw, bpp;
 		/* eDP doesn't require FDI link, so just set DP M/N
 		   according to current link config */
-		if (is_edp) {
+		if (has_edp_encoder) {
 			target_clock = mode->clock;
-			intel_edp_link_config(intel_encoder,
-					&lane, &link_bw);
+			intel_edp_link_config(has_edp_encoder,
+					      &lane, &link_bw);
 		} else {
 			/* DP over FDI requires target mode clock
 			   instead of link clock */
@@ -3669,7 +3668,7 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
 				temp |= PIPE_8BPC;
 			else
 				temp |= PIPE_6BPC;
-		} else if (is_edp || (is_dp && intel_pch_has_edp(crtc))) {
+		} else if (has_edp_encoder || (is_dp && intel_pch_has_edp(crtc))) {
 			switch (dev_priv->edp_bpp/3) {
 			case 8:
 				temp |= PIPE_8BPC;
@@ -3742,7 +3741,7 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
 
 		udelay(200);
 
-		if (is_edp) {
+		if (has_edp_encoder) {
 			if (dev_priv->lvds_use_ssc) {
 				temp |= DREF_SSC1_ENABLE;
 				I915_WRITE(PCH_DREF_CONTROL, temp);
@@ -3891,7 +3890,7 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
 		dpll_reg = pch_dpll_reg;
 	}
 
-	if (!is_edp) {
+	if (!has_edp_encoder) {
 		I915_WRITE(fp_reg, fp);
 		I915_WRITE(dpll_reg, dpll & ~DPLL_VCO_ENABLE);
 		I915_READ(dpll_reg);
@@ -3986,7 +3985,7 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
 		}
 	}
 
-	if (!is_edp) {
+	if (!has_edp_encoder) {
 		I915_WRITE(fp_reg, fp);
 		I915_WRITE(dpll_reg, dpll);
 		I915_READ(dpll_reg);
@@ -4065,7 +4064,7 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
 		I915_WRITE(link_m1_reg, m_n.link_m);
 		I915_WRITE(link_n1_reg, m_n.link_n);
 
-		if (is_edp) {
+		if (has_edp_encoder) {
 			ironlake_set_pll_edp(crtc, adjusted_mode->clock);
 		} else {
 			/* enable FDI RX PLL too */
-- 
1.7.1