Pending i915 fixes

Pending i915 fixes

[Chris Wilson]

Whilst this may appear to be a big batch, it is actually only a few bug
fixes from the last couple of weeks broken down into small steps.

1,2: Seem to be well tested now and fix the condition where the pipes
are still active and reading from memory as we rewrite the GTT. This
impacts both boot and resume.
3: A simple memleak on an error path
4: Defend against left-over state in the DP registers causing an OOPS on
boot.
5: A fix for Cantiga TV detection reverted for unknown reasons.
6: A debugging patch that has caught some real bugs, just on the limited
sample of the machines I have...
7: One of the aforementioned bugs: Enable planes after attaching the fb.
   This conflicts with the modesetting refactoring branch, so I'd prefer
   to get this into -fixes asap and then rebase -next on top.
   However, needs extensive testing just in case it was intentional!
8-16: The other bug. Using an invalid block of memory for load detection
17: Dave's multi-gpu fix for X server (or other drm client) shutdown.

These are available on the -staging branch for testing.
Kindly review, thanks.
-Chris

[PATCH 01/17] drm/i915: Move the irq wait queue initialisation into the ring init

[Chris Wilson]

Required so that we don't obliterate the queue if initialising the
rings after the global IRQ handler is installed.

[Jesse, you recently looked at refactoring the IRQ installation
routines, does moving the initialisation of ring buffer data structures away
from that routine make sense in your grand scheme?]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_irq.c         |    6 ------
 drivers/gpu/drm/i915/intel_ringbuffer.c |    1 +
 2 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 188b497..46ccfc8 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1688,12 +1688,6 @@ int i915_driver_irq_postinstall(struct drm_device *dev)
 	u32 enable_mask = I915_INTERRUPT_ENABLE_FIX | I915_INTERRUPT_ENABLE_VAR;
 	u32 error_mask;
 
-	DRM_INIT_WAITQUEUE(&dev_priv->ring[RCS].irq_queue);
-	if (HAS_BSD(dev))
-		DRM_INIT_WAITQUEUE(&dev_priv->ring[VCS].irq_queue);
-	if (HAS_BLT(dev))
-		DRM_INIT_WAITQUEUE(&dev_priv->ring[BCS].irq_queue);
-
 	dev_priv->vblank_pipe = DRM_I915_VBLANK_PIPE_A | DRM_I915_VBLANK_PIPE_B;
 
 	if (HAS_PCH_SPLIT(dev))
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index e9e6f71..884556d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -800,6 +800,7 @@ int intel_init_ring_buffer(struct drm_device *dev,
 	INIT_LIST_HEAD(&ring->request_list);
 	INIT_LIST_HEAD(&ring->gpu_write_list);
 
+	init_waitqueue_head(&ring->irq_queue);
 	spin_lock_init(&ring->irq_lock);
 	ring->irq_mask = ~0;
 
-- 
1.7.4.1

Re: [PATCH 01/17] drm/i915: Move the irq wait queue initialisation into the ring init

[Jesse Barnes]

On Thu, 21 Apr 2011 22:18:16 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> Required so that we don't obliterate the queue if initialising the
> rings after the global IRQ handler is installed.
> 
> [Jesse, you recently looked at refactoring the IRQ installation
> routines, does moving the initialisation of ring buffer data structures away
> from that routine make sense in your grand scheme?]

Yeah, this looks fine.  It'll cause a trivial conflict with my split
patch, but that's ok.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 02/17] drm/i915: Disable all outputs early, before KMS takeover

[Chris Wilson]

If the outputs are active and continuing to access the GATT when we
teardown the PTEs, then there is a potential for us to hang the GPU.
The hang tends to be a PGTBL_ER with either an invalid host access or
an invalid display plane fetch.

v2: Reorder IRQ initialisation to defer until after GEM is setup.

Reported-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Daniel Vetter <daniel.vetter@ffwll.ch> (855GM)
Tested-by: Pekka Enberg <penberg@kernel.org>
           # note that this doesn't fix the underlying problem of the
             PGTBL_ER and pipe underruns being reported immediately upon
             init on his 965GM MacBook
Reported-and-tested-by: Rick Bramley <richard.bramley@hp.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=35635
Reported-and-tested-by: Zdenek Kabelac <zdenek.kabelac@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36048
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_dma.c      |   31 ++++++++++++++++++++++---------
 drivers/gpu/drm/i915/i915_drv.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |   17 +++++++++++------
 3 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 7273037..b28e023 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1176,11 +1176,11 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
 	return can_switch;
 }
 
-static int i915_load_modeset_init(struct drm_device *dev)
+static int i915_load_gem_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	unsigned long prealloc_size, gtt_size, mappable_size;
-	int ret = 0;
+	int ret;
 
 	prealloc_size = dev_priv->mm.gtt->stolen_size;
 	gtt_size = dev_priv->mm.gtt->gtt_total_entries << PAGE_SHIFT;
@@ -1204,7 +1204,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	ret = i915_gem_init_ringbuffer(dev);
 	mutex_unlock(&dev->struct_mutex);
 	if (ret)
-		goto out;
+		return ret;
 
 	/* Try to set up FBC with a reasonable compressed buffer size */
 	if (I915_HAS_FBC(dev) && i915_powersave) {
@@ -1222,6 +1222,13 @@ static int i915_load_modeset_init(struct drm_device *dev)
 
 	/* Allow hardware batchbuffers unless told otherwise. */
 	dev_priv->allow_batchbuffer = 1;
+	return 0;
+}
+
+static int i915_load_modeset_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret;
 
 	ret = intel_parse_bios(dev);
 	if (ret)
@@ -1236,7 +1243,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	 */
 	ret = vga_client_register(dev->pdev, dev, NULL, i915_vga_set_decode);
 	if (ret && ret != -ENODEV)
-		goto cleanup_ringbuffer;
+		goto out;
 
 	intel_register_dsm_handler();
 
@@ -1253,10 +1260,16 @@ static int i915_load_modeset_init(struct drm_device *dev)
 
 	intel_modeset_init(dev);
 
-	ret = drm_irq_install(dev);
+	ret = i915_load_gem_init(dev);
 	if (ret)
 		goto cleanup_vga_switcheroo;
 
+	intel_modeset_gem_init(dev);
+
+	ret = drm_irq_install(dev);
+	if (ret)
+		goto cleanup_gem;
+
 	/* Always safe in the mode setting case. */
 	/* FIXME: do pre/post-mode set stuff in core KMS code */
 	dev->vblank_disable_allowed = 1;
@@ -1274,14 +1287,14 @@ static int i915_load_modeset_init(struct drm_device *dev)
 
 cleanup_irq:
 	drm_irq_uninstall(dev);
+cleanup_gem:
+	mutex_lock(&dev->struct_mutex);
+	i915_gem_cleanup_ringbuffer(dev);
+	mutex_unlock(&dev->struct_mutex);
 cleanup_vga_switcheroo:
 	vga_switcheroo_unregister_client(dev->pdev);
 cleanup_vga_client:
 	vga_client_register(dev->pdev, NULL, NULL, NULL);
-cleanup_ringbuffer:
-	mutex_lock(&dev->struct_mutex);
-	i915_gem_cleanup_ringbuffer(dev);
-	mutex_unlock(&dev->struct_mutex);
 out:
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1c1b27c..2f92a3c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1265,6 +1265,7 @@ static inline void intel_unregister_dsm_handler(void) { return; }
 
 /* modesetting */
 extern void intel_modeset_init(struct drm_device *dev);
+extern void intel_modeset_gem_init(struct drm_device *dev);
 extern void intel_modeset_cleanup(struct drm_device *dev);
 extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
 extern void i8xx_disable_fbc(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e522c70..2183c4d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6504,6 +6504,9 @@ static void intel_setup_outputs(struct drm_device *dev)
 	}
 
 	intel_panel_setup_backlight(dev);
+
+	/* disable all the possible outputs/crtcs before entering KMS mode */
+	drm_helper_disable_unused_functions(dev);
 }
 
 static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
@@ -7439,13 +7442,12 @@ void intel_modeset_init(struct drm_device *dev)
 		intel_crtc_init(dev, i);
 	}
 
+	/* Just disable it once at startup */
+	i915_disable_vga(dev);
 	intel_setup_outputs(dev);
 
 	intel_enable_clock_gating(dev);
 
-	/* Just disable it once at startup */
-	i915_disable_vga(dev);
-
 	if (IS_IRONLAKE_M(dev)) {
 		ironlake_enable_drps(dev);
 		intel_init_emon(dev);
@@ -7454,12 +7456,15 @@ void intel_modeset_init(struct drm_device *dev)
 	if (IS_GEN6(dev))
 		gen6_enable_rps(dev_priv);
 
-	if (IS_IRONLAKE_M(dev))
-		ironlake_enable_rc6(dev);
-
 	INIT_WORK(&dev_priv->idle_work, intel_idle_update);
 	setup_timer(&dev_priv->idle_timer, intel_gpu_idle_timer,
 		    (unsigned long)dev);
+}
+
+void intel_modeset_gem_init(struct drm_device *dev)
+{
+	if (IS_IRONLAKE_M(dev))
+		ironlake_enable_rc6(dev);
 
 	intel_setup_overlay(dev);
 }
-- 
1.7.4.1

[PATCH 03/17] drm/i915: Release object along create user fb error path

[Chris Wilson]

Reported-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@kernel.org
---
 drivers/gpu/drm/i915/intel_display.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2183c4d..16f38e4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6582,8 +6582,10 @@ intel_user_framebuffer_create(struct drm_device *dev,
 		return ERR_PTR(-ENOENT);
 
 	intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
-	if (!intel_fb)
+	if (!intel_fb) {
+		drm_gem_object_unreference_unlocked(&obj->base);
 		return ERR_PTR(-ENOMEM);
+	}
 
 	ret = intel_framebuffer_init(dev, intel_fb, mode_cmd, obj);
 	if (ret) {
-- 
1.7.4.1

_______________________________________________
stable mailing list
stable@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/stable

Re: [PATCH 03/17] drm/i915: Release object along create user fb error path

[Ben Widawsky]

On Thu, Apr 21, 2011 at 10:18:18PM +0100, Chris Wilson wrote:
> Reported-by: Alan Cox <alan@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@kernel.org
> ---
>  drivers/gpu/drm/i915/intel_display.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2183c4d..16f38e4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6582,8 +6582,10 @@ intel_user_framebuffer_create(struct drm_device *dev,
>  		return ERR_PTR(-ENOENT);
>  
>  	intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
> -	if (!intel_fb)
> +	if (!intel_fb) {
> +		drm_gem_object_unreference_unlocked(&obj->base);
>  		return ERR_PTR(-ENOMEM);
> +	}
>  
>  	ret = intel_framebuffer_init(dev, intel_fb, mode_cmd, obj);
>  	if (ret) {


Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

[PATCH 04/17] drm/i915/dp: Be paranoid in case we disable a DP before it is attached

[Chris Wilson]

Given that the hardware may be left in a random condition by the BIOS,
it is conceivable that we then attempt to clear the DP_PIPEB_SELECT bit
without us ever enabling/attaching the DP encoder to a pipe. Thus
causing a NULL deference when we attempt to wait for a vblank on that
crtc.

Reported-and-tested-by: Bryan Christ <bryan.christ@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36314
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@kernel.org
---
 drivers/gpu/drm/i915/intel_dp.c |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index cb8578b..a4d8031 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1470,7 +1470,8 @@ intel_dp_link_down(struct intel_dp *intel_dp)
 
 	if (!HAS_PCH_CPT(dev) &&
 	    I915_READ(intel_dp->output_reg) & DP_PIPEB_SELECT) {
-		struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp->base.base.crtc);
+		struct drm_crtc *crtc = intel_dp->base.base.crtc;
+
 		/* Hardware workaround: leaving our transcoder select
 		 * set to transcoder B while it's off will prevent the
 		 * corresponding HDMI output on transcoder A.
@@ -1485,7 +1486,19 @@ intel_dp_link_down(struct intel_dp *intel_dp)
 		/* Changes to enable or select take place the vblank
 		 * after being written.
 		 */
-		intel_wait_for_vblank(dev, intel_crtc->pipe);
+		if (crtc == NULL) {
+			/* We can arrive here never having been attached
+			 * to a CRTC, for instance, due to inheriting
+			 * random state from the BIOS.
+			 *
+			 * If the pipe is not running, play safe and
+			 * wait for the clocks to stabilise before
+			 * continuing.
+			 */
+			POSTING_READ(intel_dp->output_reg);
+			msleep(50);
+		} else
+			intel_wait_for_vblank(dev, to_intel_crtc(crtc)->pipe);
 	}
 
 	I915_WRITE(intel_dp->output_reg, DP & ~DP_PORT_EN);
-- 
1.7.4.1

_______________________________________________
stable mailing list
stable@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/stable

[PATCH 05/17] drm/i915/tv: Clear state sense detection for Cantiga

[Chris Wilson]

From: Zhao Yakui <yakui.zhao@intel.com>

... otherwise the TV type will be misdetected and cause spurious
connections.

This was originally applied as fb8b5a39b6310379d7b54c0c7113703a8eaf4a57
(drm/i915: Configure the TV sense state correctly on GM45 to make TV
detection reliable)

Eric: Shortly after applying this patch you requested it to be reverted,
d4b74bf07873da2e94219a7b67a334fc1c3ce649 (Revert "drm/i915: Configure
the TV sense state correctly on GM45 to make TV), but we have no clear
information just what is broken by this patch and how to resolve it.

Reported-and-tested-by: darkbasic4@gmail.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=27168
Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
Tested-by: Santi <santi@agolina.net>
Cc: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/i915/intel_tv.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 6b22c1d..3047a66 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1269,6 +1269,15 @@ intel_tv_detect_type (struct intel_tv *intel_tv,
 		   DAC_B_0_7_V |
 		   DAC_C_0_7_V);
 
+
+	/*
+	 * The TV sense state should be cleared to zero on cantiga platform. Otherwise
+	 * the TV is misdetected. This is hardware requirement.
+	 */
+	if (IS_GM45(dev))
+		tv_dac &= ~(TVDAC_STATE_CHG_EN | TVDAC_A_SENSE_CTL |
+			    TVDAC_B_SENSE_CTL | TVDAC_C_SENSE_CTL);
+
 	I915_WRITE(TV_CTL, tv_ctl);
 	I915_WRITE(TV_DAC, tv_dac);
 	POSTING_READ(TV_DAC);
-- 
1.7.4.1

Re: [PATCH 05/17] drm/i915/tv: Clear state sense detection for Cantiga

[Eric Anholt]

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

On Thu, 21 Apr 2011 22:18:20 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> From: Zhao Yakui <yakui.zhao@intel.com>
> 
> ... otherwise the TV type will be misdetected and cause spurious
> connections.
> 
> This was originally applied as fb8b5a39b6310379d7b54c0c7113703a8eaf4a57
> (drm/i915: Configure the TV sense state correctly on GM45 to make TV
> detection reliable)
> 
> Eric: Shortly after applying this patch you requested it to be reverted,
> d4b74bf07873da2e94219a7b67a334fc1c3ce649 (Revert "drm/i915: Configure
> the TV sense state correctly on GM45 to make TV), but we have no clear
> information just what is broken by this patch and how to resolve it.

The patch, as explained at the time, basically said "TV detection is
unreliable", and the contents of the patch looked like it was turning
off the detection by disabling TVDAC_STATE_CHG_EN.  The tester said that
it successfully made his TV appear disconnected.  Thus it looked to me
like a patch that was just disabling TV detection on this platform in a
roundabout way, and because of that I hadn't meant to send it upstream.

Maybe it actually makes things work (both for not-detecting no TV, and
detecting a real TV).  But I also didn't like the "because HW
requirement", instead of some specific explanation (some reason why we
need low sense level on the channels instead of high, and some reason to
disable tvdac_state_chg_en at the same time) or a pointer at some docs.

[-- 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

Re: [PATCH 05/17] drm/i915/tv: Clear state sense detection for Cantiga

[Chris Wilson]

On Thu, 21 Apr 2011 16:36:08 -0700, Eric Anholt <eric@anholt.net> wrote:
> Maybe it actually makes things work (both for not-detecting no TV, and
> detecting a real TV).  But I also didn't like the "because HW
> requirement", instead of some specific explanation (some reason why we
> need low sense level on the channels instead of high, and some reason to
> disable tvdac_state_chg_en at the same time) or a pointer at some docs.

Thanks Eric, that clears up that a lot. So the question is: does TV
detection work at all after the patch? Let's see which is quicker to get
an answer, searching fruitlessly through the docs or asking for testers...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 05/17] drm/i915/tv: Clear state sense detection for Cantiga

[Peter Clifton]

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

On Thu, 2011-04-21 at 16:36 -0700, Eric Anholt wrote:
> On Thu, 21 Apr 2011 22:18:20 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > From: Zhao Yakui <yakui.zhao@intel.com>
> > 
> > ... otherwise the TV type will be misdetected and cause spurious
> > connections.
> > 
> > This was originally applied as fb8b5a39b6310379d7b54c0c7113703a8eaf4a57
> > (drm/i915: Configure the TV sense state correctly on GM45 to make TV
> > detection reliable)
> > 
> > Eric: Shortly after applying this patch you requested it to be reverted,
> > d4b74bf07873da2e94219a7b67a334fc1c3ce649 (Revert "drm/i915: Configure
> > the TV sense state correctly on GM45 to make TV), but we have no clear
> > information just what is broken by this patch and how to resolve it.

This is an absolute must for my GM45, and I've been patching it back in
ever since it was dropped.

I had meant to follow up after having contacted the original patch
author - who claimed the patch was derived from an internally documented
errata or some such - and the requirements were contrary to the GM45
publicly released documentation.

The one vital testing step I still wasn't able to take is actually
hooking up the S-Video output of this laptop and making sure the patch
did not stop it detecting a connected TV. I don't actually own a TV, but
could get access to one.. if I had an S-Video cable I could do the test!

-- 
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)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 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

Re: [PATCH 05/17] drm/i915/tv: Clear state sense detection for Cantiga

[Niccolò Belli]

Il 22/04/2011 23:44, Peter Clifton ha scritto:
> This is an absolute must for my GM45, and I've been patching it back in
> ever since it was dropped.

I absolutely agree.

> The one vital testing step I still wasn't able to take is actually
> hooking up the S-Video output of this laptop and making sure the patch
> did not stop it detecting a connected TV. I don't actually own a TV, but
> could get access to one.. if I had an S-Video cable I could do the test!

Did you test it? Unfortunately my laptop (Samsung X360) doesn't have a
tv-out connector :(

Thank you,
Darkbasic

Re: [PATCH 05/17] drm/i915/tv: Clear state sense detection for Cantiga

[Keith Packard]

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

On Fri, 20 May 2011 11:16:43 +0200, Niccolò Belli <darkbasic4@gmail.com> wrote:

> Did you test it? Unfortunately my laptop (Samsung X360) doesn't have a
> tv-out connector :(

I've posted a couple of patches that make my machines reliably detect TV
connections now. They're the last two patches on the drm-intel-tv-detect
branch in my kernel repository:

git://git.kernel.org/pub/scm/linux/kernel/git/keithp/linux-2.6.git

I'd like to hear whether these resolve the issue for you as they are
known to not break TV detect on at least two machines, one 965GM and one
GM45.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 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

Re: [PATCH 05/17] drm/i915/tv: Clear state sense detection for Cantiga

[Niccolò Belli]

Il 20/05/2011 12:00, Keith Packard ha scritto:
> I've posted a couple of patches that make my machines reliably detect TV
> connections now. They're the last two patches on the drm-intel-tv-detect
> branch in my kernel repository:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/keithp/linux-2.6.git
> 
> I'd like to hear whether these resolve the issue for you as they are
> known to not break TV detect on at least two machines, one 965GM and one
> GM45.

Unfortunately it doesn't work....

Any chance that someone will test tv-out with this
(https://bugs.freedesktop.org/attachment.cgi?id=42814) patch so that it
will eventually be pushed before the 2.6.40 merge window closes?
It's a long outstanding regression, since 2.6.33-rc1 :/

Thank you,
Darkbasic

[PATCH 06/17] drm/i915: Check that the plane points to the pipe's framebuffer before enabling

[Chris Wilson]

Knut Petersen reported a GPU hang when he left x11perf running
overnight. The error state quite clearly indicates that plane A was
enabled without being fully setup:

PGTBL_ER: 0x00000010
    Display A: Invalid GTT PTE
Plane [0]:
  CNTR: c1000000
  STRIDE: 00000c80
  SIZE: 03ff04ff
  POS: 00000000
  ADDR: 00000000

[That GTT offset on his system being pinned for the ringbuffer.]

This is a simple debugging patch to assert that this cannot be so!

References: https://bugs.freedesktop.org/show_bug.cgi?id=36246
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 16f38e4..647e492 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1560,6 +1560,34 @@ static void intel_disable_pipe(struct drm_i915_private *dev_priv,
 	intel_wait_for_pipe_off(dev_priv->dev, pipe);
 }
 
+/* Check that the display plane points to the pipe's framebuffer. */
+static void assert_fb_bound_to_plane(struct drm_i915_private *dev_priv,
+				     enum pipe pipe, enum plane plane)
+{
+	struct drm_crtc *crtc;
+	struct intel_framebuffer *intel_fb;
+	u32 val, base, size;
+
+	crtc = intel_get_crtc_for_pipe(dev_priv->dev, pipe);
+	if (WARN(crtc->fb == NULL,
+		 "no framebuffer attached to pipe %c\n",
+		 pipe_name(pipe)))
+		return;
+
+	intel_fb = to_intel_framebuffer(crtc->fb);
+	base = intel_fb->obj->gtt_offset;
+	size = intel_fb->obj->base.size;
+
+	if (dev_priv->info->gen >= 4)
+		val = I915_READ(DSPSURF(plane));
+	else
+		val = I915_READ(DSPADDR(plane));
+	WARN(val < base || val >= base + size,
+	     "mismatching framebuffer for plane %c attached to pipe %c, expected %x-%x found %x\n",
+	     plane_name(plane), pipe_name(pipe),
+	     base, base + size, val);
+}
+
 /**
  * intel_enable_plane - enable a display plane on a given pipe
  * @dev_priv: i915 private structure
@@ -1576,6 +1604,7 @@ static void intel_enable_plane(struct drm_i915_private *dev_priv,
 
 	/* If the pipe isn't enabled, we can't pump pixels and may hang */
 	assert_pipe_enabled(dev_priv, pipe);
+	assert_fb_bound_to_plane(dev_priv, pipe, plane);
 
 	reg = DSPCNTR(plane);
 	val = I915_READ(reg);
-- 
1.7.4.1

[PATCH 07/17] drm/i915: Only enable the plane after setting the fb base (pre-ILK)

[Chris Wilson]

When enabling the plane, it is helpful to have already pointed that
plane to valid memory or else we may incur the wrath of a PGTBL_ER.
This code preserved the behaviour from the bad old days for unknown
reasons...

Found by assert_fb_bound_for_plane().

References: https://bugs.freedesktop.org/show_bug.cgi?id=36246
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 647e492..8af6adc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5183,8 +5183,6 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
 
 	I915_WRITE(DSPCNTR(plane), dspcntr);
 	POSTING_READ(DSPCNTR(plane));
-	if (!HAS_PCH_SPLIT(dev))
-		intel_enable_plane(dev_priv, plane, pipe);
 
 	ret = intel_pipe_set_base(crtc, x, y, old_fb);
 
-- 
1.7.4.1

Re: [PATCH 07/17] drm/i915: Only enable the plane after setting the fb base (pre-ILK)

[Jesse Barnes]

On Thu, 21 Apr 2011 22:18:22 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> When enabling the plane, it is helpful to have already pointed that
> plane to valid memory or else we may incur the wrath of a PGTBL_ER.
> This code preserved the behaviour from the bad old days for unknown
> reasons...
> 
> Found by assert_fb_bound_for_plane().
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=36246
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---

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

(needs lots of testing on 8xx, 9xx and 965 I think)

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 08/17] drm/i915: Move the tracking of dpms_mode down into crtc enable/disable

[Chris Wilson]

As we failed to update the dpms_mode after modeset, where it is presumed
to have been changed to DRM_MODE_DPMS_ON by the upper layers the dpms state
on the crtc became inconsistent with its actual active state. This
notably confused code and left the pipe active when it was meant to be
disabled, leading to PGTBL_ER if the fb was subsequently moved.

As we use the dpms_mode state for restoring the crtc after load-detection,
we can not simply remove it in favour of simply using the active state.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8af6adc..4b7aa6a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2904,6 +2904,8 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	intel_crtc_load_lut(crtc);
 	intel_update_fbc(dev);
 	intel_crtc_update_cursor(crtc, true);
+
+	intel_crtc->dpms_mode = DRM_MODE_DPMS_ON;
 }
 
 static void ironlake_crtc_disable(struct drm_crtc *crtc)
@@ -3000,6 +3002,8 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 	intel_update_watermarks(dev);
 	intel_update_fbc(dev);
 	intel_clear_scanline_wait(dev);
+
+	intel_crtc->dpms_mode = DRM_MODE_DPMS_OFF;
 }
 
 static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
@@ -3052,6 +3056,9 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
 
+	DRM_DEBUG_KMS("[CRTC:%d] active: %d\n",
+		      crtc->base.id, intel_crtc->active);
+
 	if (intel_crtc->active)
 		return;
 
@@ -3068,6 +3075,8 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 	/* Give the overlay scaler a chance to enable if it's on this pipe */
 	intel_crtc_dpms_overlay(intel_crtc, true);
 	intel_crtc_update_cursor(crtc, true);
+
+	intel_crtc->dpms_mode = DRM_MODE_DPMS_ON;
 }
 
 static void i9xx_crtc_disable(struct drm_crtc *crtc)
@@ -3078,6 +3087,9 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
 
+	DRM_DEBUG_KMS("[CRTC:%d] active: %d\n",
+		      crtc->base.id, intel_crtc->active);
+
 	if (!intel_crtc->active)
 		return;
 
@@ -3099,6 +3111,8 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	intel_update_fbc(dev);
 	intel_update_watermarks(dev);
 	intel_clear_scanline_wait(dev);
+
+	intel_crtc->dpms_mode = DRM_MODE_DPMS_OFF;
 }
 
 static void i9xx_crtc_dpms(struct drm_crtc *crtc, int mode)
@@ -3130,11 +3144,13 @@ static void intel_crtc_dpms(struct drm_crtc *crtc, int mode)
 	int pipe = intel_crtc->pipe;
 	bool enabled;
 
+	DRM_DEBUG_KMS("[CRTC:%d] current dpms %d, new %d\n",
+		      crtc->base.id,
+		      intel_crtc->dpms_mode, mode);
+
 	if (intel_crtc->dpms_mode == mode)
 		return;
 
-	intel_crtc->dpms_mode = mode;
-
 	dev_priv->display.dpms(crtc, mode);
 
 	if (!dev->primary->master)
-- 
1.7.4.1

Re: [PATCH 08/17] drm/i915: Move the tracking of dpms_mode down into crtc enable/disable

[Keith Packard]

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

On Thu, 21 Apr 2011 22:18:23 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:

> As we failed to update the dpms_mode after modeset, where it is presumed
> to have been changed to DRM_MODE_DPMS_ON by the upper layers the dpms state
> on the crtc became inconsistent with its actual active state. This
> notably confused code and left the pipe active when it was meant to be
> disabled, leading to PGTBL_ER if the fb was subsequently moved.
> 
> As we use the dpms_mode state for restoring the crtc after load-detection,
> we can not simply remove it in favour of simply using the active
> state.

I don't understand this comment -- this patch changes the code so that
dpms_mode exactly tracks active, instead of tracking the desired DPMS state.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 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

Re: [PATCH 08/17] drm/i915: Move the tracking of dpms_mode down into crtc enable/disable

[Chris Wilson]

On Wed, 04 May 2011 12:10:06 -0700, Keith Packard <keithp@keithp.com> wrote:
> On Thu, 21 Apr 2011 22:18:23 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > As we failed to update the dpms_mode after modeset, where it is presumed
> > to have been changed to DRM_MODE_DPMS_ON by the upper layers the dpms state
> > on the crtc became inconsistent with its actual active state. This
> > notably confused code and left the pipe active when it was meant to be
> > disabled, leading to PGTBL_ER if the fb was subsequently moved.
> > 
> > As we use the dpms_mode state for restoring the crtc after load-detection,
> > we can not simply remove it in favour of simply using the active
> > state.
> 
> I don't understand this comment -- this patch changes the code so that
> dpms_mode exactly tracks active, instead of tracking the desired DPMS state.

Indeed for our purposes of tracking the register states, we do not want to
track the desired mode and only the actual state of the crtc. Yes, it is
active by another value. I was tempted to do a more complicated patch to
remove one of the two tracking variables.

We can remove dpms_mode in favour of solely using active.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 08/17] drm/i915: Move the tracking of dpms_mode down into crtc enable/disable

[Keith Packard]

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

On Wed, 04 May 2011 20:40:37 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:

> We can remove dpms_mode in favour of solely using active.

That sounds like a good plan.

And what about the intel_dp->dpms_mode value? Should it be switched to a
simple 'active' boolean as well? Or is the existing crtc->active value
sufficient?

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 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

Re: [PATCH 08/17] drm/i915: Move the tracking of dpms_mode down into crtc enable/disable

[Chris Wilson]

On Wed, 04 May 2011 14:20:27 -0700, Keith Packard <keithp@keithp.com> wrote:
> On Wed, 04 May 2011 20:40:37 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > We can remove dpms_mode in favour of solely using active.
> 
> That sounds like a good plan.
> 
> And what about the intel_dp->dpms_mode value? Should it be switched to a
> simple 'active' boolean as well? Or is the existing crtc->active value
> sufficient?

The sticky point is the usage within release-load-detect-pipe where we
restore the "dpms-mode". As it stands today, we treat dpms-mode as if it
were just a boolean value (ignoring the unknown value). But we should keep
the code generic, if it doesn't incur any additional burden, so I'm
favouring a multi-valued active or actual-dpms-mode.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 09/17] drm/i915: Simplify return value from intel_get_load_detect_pipe

[Chris Wilson]

... and so remove the confusion as to whether to use the returned crtc
or intel_encoder->base.crtc with the subsequent load-detection. Even
though they were the same, the two instances of load-detection code
disagreed over which was the more correct.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_crt.c     |   17 +++++++----------
 drivers/gpu/drm/i915/intel_display.c |   16 +++++++++-------
 drivers/gpu/drm/i915/intel_drv.h     |    8 ++++----
 drivers/gpu/drm/i915/intel_tv.c      |    6 ++----
 4 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index d03fc05..2eb60cd 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -305,13 +305,11 @@ static bool intel_crt_detect_ddc(struct drm_connector *connector)
 }
 
 static enum drm_connector_status
-intel_crt_load_detect(struct drm_crtc *crtc, struct intel_crt *crt)
+intel_crt_load_detect(struct intel_crt *crt)
 {
-	struct drm_encoder *encoder = &crt->base.base;
-	struct drm_device *dev = encoder->dev;
+	struct drm_device *dev = crt->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	uint32_t pipe = intel_crtc->pipe;
+	uint32_t pipe = to_intel_crtc(crt->base.base.crtc)->pipe;
 	uint32_t save_bclrpat;
 	uint32_t save_vtotal;
 	uint32_t vtotal, vactive;
@@ -454,15 +452,14 @@ intel_crt_detect(struct drm_connector *connector, bool force)
 	/* for pre-945g platforms use load detect */
 	crtc = crt->base.base.crtc;
 	if (crtc && crtc->enabled) {
-		status = intel_crt_load_detect(crtc, crt);
+		status = intel_crt_load_detect(crt);
 	} else {
-		crtc = intel_get_load_detect_pipe(&crt->base, connector,
-						  NULL, &dpms_mode);
-		if (crtc) {
+		if (intel_get_load_detect_pipe(&crt->base, connector,
+					       NULL, &dpms_mode)) {
 			if (intel_crt_detect_ddc(connector))
 				status = connector_status_connected;
 			else
-				status = intel_crt_load_detect(crtc, crt);
+				status = intel_crt_load_detect(crt);
 			intel_release_load_detect_pipe(&crt->base,
 						       connector, dpms_mode);
 		} else
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4b7aa6a..793245e2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5526,10 +5526,10 @@ static struct drm_display_mode load_detect_mode = {
 		 704, 832, 0, 480, 489, 491, 520, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
 };
 
-struct drm_crtc *intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
-					    struct drm_connector *connector,
-					    struct drm_display_mode *mode,
-					    int *dpms_mode)
+bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
+				struct drm_connector *connector,
+				struct drm_display_mode *mode,
+				int *dpms_mode)
 {
 	struct intel_crtc *intel_crtc;
 	struct drm_crtc *possible_crtc;
@@ -5562,7 +5562,7 @@ struct drm_crtc *intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 			crtc_funcs->dpms(crtc, DRM_MODE_DPMS_ON);
 			encoder_funcs->dpms(encoder, DRM_MODE_DPMS_ON);
 		}
-		return crtc;
+		return true;
 	}
 
 	/* Find an unused one (if possible) */
@@ -5582,7 +5582,8 @@ struct drm_crtc *intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 	 * If we didn't find an unused CRTC, don't use any.
 	 */
 	if (!crtc) {
-		return NULL;
+		DRM_DEBUG_KMS("no pipe available for load-detect\n");
+		return false;
 	}
 
 	encoder->crtc = crtc;
@@ -5606,10 +5607,11 @@ struct drm_crtc *intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 		encoder_funcs->mode_set(encoder, &crtc->mode, &crtc->mode);
 		encoder_funcs->commit(encoder);
 	}
+
 	/* let the connector get through one full cycle before testing */
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
 
-	return crtc;
+	return true;
 }
 
 void intel_release_load_detect_pipe(struct intel_encoder *intel_encoder,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f5b0d83..d405d8e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -291,10 +291,10 @@ int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
 				struct drm_file *file_priv);
 extern void intel_wait_for_vblank(struct drm_device *dev, int pipe);
 extern void intel_wait_for_pipe_off(struct drm_device *dev, int pipe);
-extern struct drm_crtc *intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
-						   struct drm_connector *connector,
-						   struct drm_display_mode *mode,
-						   int *dpms_mode);
+extern bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
+				       struct drm_connector *connector,
+				       struct drm_display_mode *mode,
+				       int *dpms_mode);
 extern void intel_release_load_detect_pipe(struct intel_encoder *intel_encoder,
 					   struct drm_connector *connector,
 					   int dpms_mode);
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 3047a66..1174f77 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1370,12 +1370,10 @@ intel_tv_detect(struct drm_connector *connector, bool force)
 	if (intel_tv->base.base.crtc && intel_tv->base.base.crtc->enabled) {
 		type = intel_tv_detect_type(intel_tv, connector);
 	} else if (force) {
-		struct drm_crtc *crtc;
 		int dpms_mode;
 
-		crtc = intel_get_load_detect_pipe(&intel_tv->base, connector,
-						  &mode, &dpms_mode);
-		if (crtc) {
+		if (intel_get_load_detect_pipe(&intel_tv->base, connector,
+					       &mode, &dpms_mode)) {
 			type = intel_tv_detect_type(intel_tv, connector);
 			intel_release_load_detect_pipe(&intel_tv->base, connector,
 						       dpms_mode);
-- 
1.7.4.1

[PATCH 10/17] drm/i915: Propagate failure to set mode for load-detect pipe

[Chris Wilson]

Check the return value from drm_crtc_set_mode(), report the failure
via a debug message and propagate the error back to the caller. This
prevents us from blissfully continuing to do the load detection on a
disabled pipe. Fortunately actual failure for modesetting is very rare,
and reported failures even rarer.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 793245e2..b254fac 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5596,7 +5596,11 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 	if (!crtc->enabled) {
 		if (!mode)
 			mode = &load_detect_mode;
-		drm_crtc_helper_set_mode(crtc, mode, 0, 0, crtc->fb);
+
+		if (!drm_crtc_helper_set_mode(crtc, mode, 0, 0, crtc->fb)) {
+			DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
+			return false;
+		}
 	} else {
 		if (intel_crtc->dpms_mode != DRM_MODE_DPMS_ON) {
 			crtc_funcs = crtc->helper_private;
-- 
1.7.4.1

[PATCH 11/17] drm/i915: Don't store temporary load-detect variables in the generic encoder

[Chris Wilson]

Keep all the state required for undoing and restoring the previous pipe
configuration together in a single struct passed from
intel_get_load_detect_pipe() to intel_release_load_detect_pipe() rather
than stuffing them inside the common encoder structure.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_crt.c     |   11 ++++++-----
 drivers/gpu/drm/i915/intel_display.c |   26 +++++++++++++++-----------
 drivers/gpu/drm/i915/intel_drv.h     |   10 +++++++---
 drivers/gpu/drm/i915/intel_tv.c      |    9 +++++----
 4 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 2eb60cd..e93f93c 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -430,7 +430,6 @@ intel_crt_detect(struct drm_connector *connector, bool force)
 	struct drm_device *dev = connector->dev;
 	struct intel_crt *crt = intel_attached_crt(connector);
 	struct drm_crtc *crtc;
-	int dpms_mode;
 	enum drm_connector_status status;
 
 	if (I915_HAS_HOTPLUG(dev)) {
@@ -454,14 +453,16 @@ intel_crt_detect(struct drm_connector *connector, bool force)
 	if (crtc && crtc->enabled) {
 		status = intel_crt_load_detect(crt);
 	} else {
-		if (intel_get_load_detect_pipe(&crt->base, connector,
-					       NULL, &dpms_mode)) {
+		struct intel_load_detect_pipe tmp;
+
+		if (intel_get_load_detect_pipe(&crt->base, connector, NULL,
+					       &tmp)) {
 			if (intel_crt_detect_ddc(connector))
 				status = connector_status_connected;
 			else
 				status = intel_crt_load_detect(crt);
-			intel_release_load_detect_pipe(&crt->base,
-						       connector, dpms_mode);
+			intel_release_load_detect_pipe(&crt->base, connector,
+						       &tmp);
 		} else
 			status = connector_status_unknown;
 	}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b254fac..55d8163 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5529,7 +5529,7 @@ static struct drm_display_mode load_detect_mode = {
 bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 				struct drm_connector *connector,
 				struct drm_display_mode *mode,
-				int *dpms_mode)
+				struct intel_load_detect_pipe *old)
 {
 	struct intel_crtc *intel_crtc;
 	struct drm_crtc *possible_crtc;
@@ -5554,14 +5554,18 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 	/* See if we already have a CRTC for this connector */
 	if (encoder->crtc) {
 		crtc = encoder->crtc;
-		/* Make sure the crtc and connector are running */
+
 		intel_crtc = to_intel_crtc(crtc);
-		*dpms_mode = intel_crtc->dpms_mode;
+		old->dpms_mode = intel_crtc->dpms_mode;
+		old->load_detect_temp = false;
+
+		/* Make sure the crtc and connector are running */
 		if (intel_crtc->dpms_mode != DRM_MODE_DPMS_ON) {
 			crtc_funcs = crtc->helper_private;
 			crtc_funcs->dpms(crtc, DRM_MODE_DPMS_ON);
 			encoder_funcs->dpms(encoder, DRM_MODE_DPMS_ON);
 		}
+
 		return true;
 	}
 
@@ -5588,10 +5592,10 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 
 	encoder->crtc = crtc;
 	connector->encoder = encoder;
-	intel_encoder->load_detect_temp = true;
 
 	intel_crtc = to_intel_crtc(crtc);
-	*dpms_mode = intel_crtc->dpms_mode;
+	old->dpms_mode = intel_crtc->dpms_mode;
+	old->load_detect_temp = true;
 
 	if (!crtc->enabled) {
 		if (!mode)
@@ -5619,7 +5623,8 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 }
 
 void intel_release_load_detect_pipe(struct intel_encoder *intel_encoder,
-				    struct drm_connector *connector, int dpms_mode)
+				    struct drm_connector *connector,
+				    struct intel_load_detect_pipe *old)
 {
 	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_device *dev = encoder->dev;
@@ -5627,19 +5632,18 @@ void intel_release_load_detect_pipe(struct intel_encoder *intel_encoder,
 	struct drm_encoder_helper_funcs *encoder_funcs = encoder->helper_private;
 	struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
 
-	if (intel_encoder->load_detect_temp) {
+	if (old->load_detect_temp) {
 		encoder->crtc = NULL;
 		connector->encoder = NULL;
-		intel_encoder->load_detect_temp = false;
 		crtc->enabled = drm_helper_crtc_in_use(crtc);
 		drm_helper_disable_unused_functions(dev);
 	}
 
 	/* Switch crtc and encoder back off if necessary */
-	if (crtc->enabled && dpms_mode != DRM_MODE_DPMS_ON) {
+	if (crtc->enabled && old->dpms_mode != DRM_MODE_DPMS_ON) {
 		if (encoder->crtc == crtc)
-			encoder_funcs->dpms(encoder, dpms_mode);
-		crtc_funcs->dpms(crtc, dpms_mode);
+			encoder_funcs->dpms(encoder, old->dpms_mode);
+		crtc_funcs->dpms(crtc, old->dpms_mode);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d405d8e..e389b46 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -140,7 +140,6 @@ struct intel_fbdev {
 struct intel_encoder {
 	struct drm_encoder base;
 	int type;
-	bool load_detect_temp;
 	bool needs_tv_clock;
 	void (*hot_plug)(struct intel_encoder *);
 	int crtc_mask;
@@ -291,13 +290,18 @@ int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
 				struct drm_file *file_priv);
 extern void intel_wait_for_vblank(struct drm_device *dev, int pipe);
 extern void intel_wait_for_pipe_off(struct drm_device *dev, int pipe);
+
+struct intel_load_detect_pipe {
+	bool load_detect_temp;
+	int dpms_mode;
+};
 extern bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 				       struct drm_connector *connector,
 				       struct drm_display_mode *mode,
-				       int *dpms_mode);
+				       struct intel_load_detect_pipe *old);
 extern void intel_release_load_detect_pipe(struct intel_encoder *intel_encoder,
 					   struct drm_connector *connector,
-					   int dpms_mode);
+					   struct intel_load_detect_pipe *old);
 
 extern struct drm_connector* intel_sdvo_find(struct drm_device *dev, int sdvoB);
 extern int intel_sdvo_supports_hotplug(struct drm_connector *connector);
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 1174f77..529f232 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1370,13 +1370,14 @@ intel_tv_detect(struct drm_connector *connector, bool force)
 	if (intel_tv->base.base.crtc && intel_tv->base.base.crtc->enabled) {
 		type = intel_tv_detect_type(intel_tv, connector);
 	} else if (force) {
-		int dpms_mode;
+		struct intel_load_detect_pipe tmp;
 
 		if (intel_get_load_detect_pipe(&intel_tv->base, connector,
-					       &mode, &dpms_mode)) {
+					       &mode, &tmp)) {
 			type = intel_tv_detect_type(intel_tv, connector);
-			intel_release_load_detect_pipe(&intel_tv->base, connector,
-						       dpms_mode);
+			intel_release_load_detect_pipe(&intel_tv->base,
+						       connector,
+						       &tmp);
 		} else
 			return connector_status_unknown;
 	} else
-- 
1.7.4.1

[PATCH 12/17] drm/i915: Remove unused supported_crtc from intel_load_detect_pipe

[Chris Wilson]

... and the no longer relevant comment. The code ceased stealing a pipe
for load detection a long time ago.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_display.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 55d8163..cfd58a2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5533,7 +5533,6 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 {
 	struct intel_crtc *intel_crtc;
 	struct drm_crtc *possible_crtc;
-	struct drm_crtc *supported_crtc =NULL;
 	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_crtc *crtc = NULL;
 	struct drm_device *dev = encoder->dev;
@@ -5543,12 +5542,12 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 
 	/*
 	 * Algorithm gets a little messy:
+	 *
 	 *   - if the connector already has an assigned crtc, use it (but make
 	 *     sure it's on first)
+	 *
 	 *   - try to find the first unused crtc that can drive this connector,
 	 *     and use that if we find one
-	 *   - if there are no unused crtcs available, try to use the first
-	 *     one we found that supports the connector
 	 */
 
 	/* See if we already have a CRTC for this connector */
@@ -5578,8 +5577,6 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 			crtc = possible_crtc;
 			break;
 		}
-		if (!supported_crtc)
-			supported_crtc = possible_crtc;
 	}
 
 	/*
-- 
1.7.4.1

[PATCH 13/17] drm/i915: Pass the saved adjusted_mode when adding to the load-detect crtc

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Keith Packard <keithp@keithp.com>
---
 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 cfd58a2..132f83d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5609,7 +5609,7 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 		}
 
 		/* Add this connector to the crtc */
-		encoder_funcs->mode_set(encoder, &crtc->mode, &crtc->mode);
+		encoder_funcs->mode_set(encoder, &crtc->mode, &crtc->hwmode);
 		encoder_funcs->commit(encoder);
 	}
 
-- 
1.7.4.1

[PATCH 14/17] drm/i915: Remove dead code from intel_get_load_detect_pipe()

[Chris Wilson]

As we only allow the use of a disabled CRTC, we don't need to handle the
case where we are reusing an already enabled pipe.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_display.c |   28 ++++++++++------------------
 1 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 132f83d..456c41c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5536,8 +5536,6 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_crtc *crtc = NULL;
 	struct drm_device *dev = encoder->dev;
-	struct drm_encoder_helper_funcs *encoder_funcs = encoder->helper_private;
-	struct drm_crtc_helper_funcs *crtc_funcs;
 	int i = -1;
 
 	/*
@@ -5560,8 +5558,13 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 
 		/* Make sure the crtc and connector are running */
 		if (intel_crtc->dpms_mode != DRM_MODE_DPMS_ON) {
+			struct drm_encoder_helper_funcs *encoder_funcs;
+			struct drm_crtc_helper_funcs *crtc_funcs;
+
 			crtc_funcs = crtc->helper_private;
 			crtc_funcs->dpms(crtc, DRM_MODE_DPMS_ON);
+
+			encoder_funcs = encoder->helper_private;
 			encoder_funcs->dpms(encoder, DRM_MODE_DPMS_ON);
 		}
 
@@ -5594,23 +5597,12 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 	old->dpms_mode = intel_crtc->dpms_mode;
 	old->load_detect_temp = true;
 
-	if (!crtc->enabled) {
-		if (!mode)
-			mode = &load_detect_mode;
-
-		if (!drm_crtc_helper_set_mode(crtc, mode, 0, 0, crtc->fb)) {
-			DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
-			return false;
-		}
-	} else {
-		if (intel_crtc->dpms_mode != DRM_MODE_DPMS_ON) {
-			crtc_funcs = crtc->helper_private;
-			crtc_funcs->dpms(crtc, DRM_MODE_DPMS_ON);
-		}
+	if (!mode)
+		mode = &load_detect_mode;
 
-		/* Add this connector to the crtc */
-		encoder_funcs->mode_set(encoder, &crtc->mode, &crtc->hwmode);
-		encoder_funcs->commit(encoder);
+	if (!drm_crtc_helper_set_mode(crtc, mode, 0, 0, crtc->fb)) {
+		DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
+		return false;
 	}
 
 	/* let the connector get through one full cycle before testing */
-- 
1.7.4.1

[PATCH 15/17] drm/i915: Remove dead code from intel_release_load_detect_pipe()

[Chris Wilson]

As we now never attempt to steal a crtc for load detection, we either
set a mode on a new pipe, or change the dpms mode on an existing pipe.
Never both, so we can simplify the code slightly.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 456c41c..27fa835 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5622,16 +5622,14 @@ void intel_release_load_detect_pipe(struct intel_encoder *intel_encoder,
 	struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
 
 	if (old->load_detect_temp) {
-		encoder->crtc = NULL;
 		connector->encoder = NULL;
-		crtc->enabled = drm_helper_crtc_in_use(crtc);
 		drm_helper_disable_unused_functions(dev);
+		return;
 	}
 
 	/* Switch crtc and encoder back off if necessary */
-	if (crtc->enabled && old->dpms_mode != DRM_MODE_DPMS_ON) {
-		if (encoder->crtc == crtc)
-			encoder_funcs->dpms(encoder, old->dpms_mode);
+	if (old->dpms_mode != DRM_MODE_DPMS_ON) {
+		encoder_funcs->dpms(encoder, old->dpms_mode);
 		crtc_funcs->dpms(crtc, old->dpms_mode);
 	}
 }
-- 
1.7.4.1

[PATCH 16/17] drm/i915: Attach a fb to the load-detect pipe

[Chris Wilson]

We need to ensure that we feed valid memory into the display plane
attached to the pipe when switching the pipe on. Otherwise, the display
engine may read through an invalid PTE and so throw an PGTBL_ER
exception.

As we need to perform load detection before even the first object is
allocated for the fbdev, there is no pre-existing object large enough
for us to borrow to use as the framebuffer. So we need to create one
and cleanup afterwards. At other times, the current fbcon may be large
enough for us to borrow it for duration of load detection.

Found by assert_fb_bound_for_plane().

Reported-by: Knut Petersen <Knut_Petersen@t-online.de>
References: https://bugs.freedesktop.org/show_bug.cgi?id=36246
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |  144 ++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_drv.h     |    1 +
 2 files changed, 128 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 27fa835..d6d634e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5526,6 +5526,92 @@ static struct drm_display_mode load_detect_mode = {
 		 704, 832, 0, 480, 489, 491, 520, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
 };
 
+static struct drm_framebuffer *
+intel_framebuffer_create(struct drm_device *dev,
+			 struct drm_mode_fb_cmd *mode_cmd,
+			 struct drm_i915_gem_object *obj)
+{
+	struct intel_framebuffer *intel_fb;
+	int ret;
+
+	intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
+	if (!intel_fb) {
+		drm_gem_object_unreference_unlocked(&obj->base);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	ret = intel_framebuffer_init(dev, intel_fb, mode_cmd, obj);
+	if (ret) {
+		drm_gem_object_unreference_unlocked(&obj->base);
+		kfree(intel_fb);
+		return ERR_PTR(ret);
+	}
+
+	return &intel_fb->base;
+}
+
+static u32
+intel_framebuffer_pitch_for_width(int width, int bpp)
+{
+	u32 pitch = DIV_ROUND_UP(width * bpp, 8);
+	return ALIGN(pitch, 64);
+}
+
+static u32
+intel_framebuffer_size_for_mode(struct drm_display_mode *mode, int bpp)
+{
+	u32 pitch = intel_framebuffer_pitch_for_width(mode->hdisplay, bpp);
+	return ALIGN(pitch * mode->vdisplay, PAGE_SIZE);
+}
+
+static struct drm_framebuffer *
+intel_framebuffer_create_for_mode(struct drm_device *dev,
+				  struct drm_display_mode *mode,
+				  int depth, int bpp)
+{
+	struct drm_i915_gem_object *obj;
+	struct drm_mode_fb_cmd mode_cmd;
+
+	obj = i915_gem_alloc_object(dev,
+				    intel_framebuffer_size_for_mode(mode, bpp));
+	if (obj == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	mode_cmd.width = mode->hdisplay;
+	mode_cmd.height = mode->vdisplay;
+	mode_cmd.depth = depth;
+	mode_cmd.bpp = bpp;
+	mode_cmd.pitch = intel_framebuffer_pitch_for_width(mode_cmd.width, bpp);
+
+	return intel_framebuffer_create(dev, &mode_cmd, obj);
+}
+
+static struct drm_framebuffer *
+mode_fits_in_fbdev(struct drm_device *dev,
+		   struct drm_display_mode *mode)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *obj;
+	struct drm_framebuffer *fb;
+
+	if (dev_priv->fbdev == NULL)
+		return NULL;
+
+	obj = dev_priv->fbdev->ifb.obj;
+	if (obj == NULL)
+		return NULL;
+
+	fb = &dev_priv->fbdev->ifb.base;
+	if (fb->pitch < intel_framebuffer_pitch_for_width(mode->hdisplay,
+							  fb->bits_per_pixel))
+		return NULL;
+
+	if (obj->base.size < mode->vdisplay * fb->pitch)
+		return NULL;
+
+	return fb;
+}
+
 bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 				struct drm_connector *connector,
 				struct drm_display_mode *mode,
@@ -5536,8 +5622,13 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_crtc *crtc = NULL;
 	struct drm_device *dev = encoder->dev;
+	struct drm_framebuffer *old_fb;
 	int i = -1;
 
+	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
+		      connector->base.id, drm_get_connector_name(connector),
+		      encoder->base.id, drm_get_encoder_name(encoder));
+
 	/*
 	 * Algorithm gets a little messy:
 	 *
@@ -5596,12 +5687,38 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
 	intel_crtc = to_intel_crtc(crtc);
 	old->dpms_mode = intel_crtc->dpms_mode;
 	old->load_detect_temp = true;
+	old->release_fb = NULL;
 
 	if (!mode)
 		mode = &load_detect_mode;
 
-	if (!drm_crtc_helper_set_mode(crtc, mode, 0, 0, crtc->fb)) {
+	old_fb = crtc->fb;
+
+	/* We need a framebuffer large enough to accommodate all accesses
+	 * that the plane may generate whilst we perform load detection.
+	 * We can not rely on the fbcon either being present (we get called
+	 * during its initialisation to detect all boot displays, or it may
+	 * not even exist) or that it is large enough to satisfy the
+	 * requested mode.
+	 */
+	crtc->fb = mode_fits_in_fbdev(dev, mode);
+	if (crtc->fb == NULL) {
+		DRM_DEBUG_KMS("creating tmp fb for load-detection\n");
+		crtc->fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32);
+		old->release_fb = crtc->fb;
+	} else
+		DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n");
+	if (IS_ERR(crtc->fb)) {
+		DRM_DEBUG_KMS("failed to allocate framebuffer for load-detection\n");
+		crtc->fb = old_fb;
+		return false;
+	}
+
+	if (!drm_crtc_helper_set_mode(crtc, mode, 0, 0, old_fb)) {
 		DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
+		if (old->release_fb)
+			old->release_fb->funcs->destroy(old->release_fb);
+		crtc->fb = old_fb;
 		return false;
 	}
 
@@ -5621,9 +5738,17 @@ void intel_release_load_detect_pipe(struct intel_encoder *intel_encoder,
 	struct drm_encoder_helper_funcs *encoder_funcs = encoder->helper_private;
 	struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
 
+	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
+		      connector->base.id, drm_get_connector_name(connector),
+		      encoder->base.id, drm_get_encoder_name(encoder));
+
 	if (old->load_detect_temp) {
 		connector->encoder = NULL;
 		drm_helper_disable_unused_functions(dev);
+
+		if (old->release_fb)
+			old->release_fb->funcs->destroy(old->release_fb);
+
 		return;
 	}
 
@@ -6614,27 +6739,12 @@ intel_user_framebuffer_create(struct drm_device *dev,
 			      struct drm_mode_fb_cmd *mode_cmd)
 {
 	struct drm_i915_gem_object *obj;
-	struct intel_framebuffer *intel_fb;
-	int ret;
 
 	obj = to_intel_bo(drm_gem_object_lookup(dev, filp, mode_cmd->handle));
 	if (&obj->base == NULL)
 		return ERR_PTR(-ENOENT);
 
-	intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
-	if (!intel_fb) {
-		drm_gem_object_unreference_unlocked(&obj->base);
-		return ERR_PTR(-ENOMEM);
-	}
-
-	ret = intel_framebuffer_init(dev, intel_fb, mode_cmd, obj);
-	if (ret) {
-		drm_gem_object_unreference_unlocked(&obj->base);
-		kfree(intel_fb);
-		return ERR_PTR(ret);
-	}
-
-	return &intel_fb->base;
+	return intel_framebuffer_create(dev, mode_cmd, obj);
 }
 
 static const struct drm_mode_config_funcs intel_mode_funcs = {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e389b46..6b848d9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -292,6 +292,7 @@ extern void intel_wait_for_vblank(struct drm_device *dev, int pipe);
 extern void intel_wait_for_pipe_off(struct drm_device *dev, int pipe);
 
 struct intel_load_detect_pipe {
+	struct drm_framebuffer *release_fb;
 	bool load_detect_temp;
 	int dpms_mode;
 };
-- 
1.7.4.1

[PATCH 17/17] drm/i915: restore only the mode of this driver on lastclose (v2)

[Chris Wilson]

From: Dave Airlie <airlied@redhat.com>

i915 calls the panic handler function on last close to reset the modes,
however this is a really bad idea for multi-gpu machines, esp shareable
gpus machines. So add a new entry point for the driver to just restore
its own fbcon mode.

v2: move code into fb helper, fix panic code to block mode change on
powered off GPUs.

Signed-off-by: Dave Airlie <airlied@redhat.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_fb_helper.c  |   27 ++++++++++++++++++++-------
 drivers/gpu/drm/i915/i915_dma.c  |    2 +-
 drivers/gpu/drm/i915/intel_drv.h |    1 +
 drivers/gpu/drm/i915/intel_fb.c  |   10 ++++++++++
 include/drm/drm_fb_helper.h      |    1 +
 5 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 9507204..11d7a72 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -342,9 +342,22 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
 }
 EXPORT_SYMBOL(drm_fb_helper_debug_leave);
 
+bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper)
+{
+	bool error = false;
+	int i, ret;
+	for (i = 0; i < fb_helper->crtc_count; i++) {
+		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
+		ret = drm_crtc_helper_set_config(mode_set);
+		if (ret)
+			error = true;
+	}
+	return error;
+}
+EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode);
+
 bool drm_fb_helper_force_kernel_mode(void)
 {
-	int i = 0;
 	bool ret, error = false;
 	struct drm_fb_helper *helper;
 
@@ -352,12 +365,12 @@ bool drm_fb_helper_force_kernel_mode(void)
 		return false;
 
 	list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
-		for (i = 0; i < helper->crtc_count; i++) {
-			struct drm_mode_set *mode_set = &helper->crtc_info[i].mode_set;
-			ret = drm_crtc_helper_set_config(mode_set);
-			if (ret)
-				error = true;
-		}
+		if (helper->dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+			continue;
+
+		ret = drm_fb_helper_restore_fbdev_mode(helper);
+		if (ret)
+			error = true;
 	}
 	return error;
 }
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b28e023..14a234e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2220,7 +2220,7 @@ void i915_driver_lastclose(struct drm_device * dev)
 	drm_i915_private_t *dev_priv = dev->dev_private;
 
 	if (!dev_priv || drm_core_check_feature(dev, DRIVER_MODESET)) {
-		drm_fb_helper_restore();
+		intel_fb_restore_mode(dev);
 		vga_switcheroo_process_delayed_switch();
 		return;
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6b848d9..2e49b62 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -343,4 +343,5 @@ extern int intel_overlay_attrs(struct drm_device *dev, void *data,
 			       struct drm_file *file_priv);
 
 extern void intel_fb_output_poll_changed(struct drm_device *dev);
+extern void intel_fb_restore_mode(struct drm_device *dev);
 #endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 5127827..ec49bae 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -264,3 +264,13 @@ void intel_fb_output_poll_changed(struct drm_device *dev)
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
 }
+
+void intel_fb_restore_mode(struct drm_device *dev)
+{
+	int ret;
+	drm_i915_private_t *dev_priv = dev->dev_private;
+
+	ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper);
+	if (ret)
+		DRM_DEBUG("failed to restore crtc mode\n");
+}
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index f22e7fe..ade09d7 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -118,6 +118,7 @@ int drm_fb_helper_setcolreg(unsigned regno,
 			    unsigned transp,
 			    struct fb_info *info);
 
+bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper);
 void drm_fb_helper_restore(void);
 void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helper,
 			    uint32_t fb_width, uint32_t fb_height);
-- 
1.7.4.1

Re: Pending i915 fixes

[Keith Packard]

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

On Thu, 21 Apr 2011 22:18:15 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Whilst this may appear to be a big batch, it is actually only a few bug
> fixes from the last couple of weeks broken down into small steps.

Do you think any of these are needed in .39?

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 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

Re: Pending i915 fixes

[Dave Airlie]

On Fri, Apr 22, 2011 at 7:56 AM, Keith Packard <keithp@keithp.com> wrote:
> On Thu, 21 Apr 2011 22:18:15 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> Whilst this may appear to be a big batch, it is actually only a few bug
>> fixes from the last couple of weeks broken down into small steps.
>
> Do you think any of these are needed in .39?
>

My one is definitely, I've got a few people who can't use machines
without it, I was going to push it myself, but I'm being nice ;-)

Dave.

Re: Pending i915 fixes

[Keith Packard]

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

On Fri, 22 Apr 2011 08:01:46 +1000, Dave Airlie <airlied@gmail.com> wrote:

> My one is definitely, I've got a few people who can't use machines
> without it, I was going to push it myself, but I'm being nice ;-)

I'll review all of them for .39 then; it wasn't clear from the original
mail whether that was the desired target.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 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

Re: Pending i915 fixes

[Chris Wilson]

On Thu, 21 Apr 2011 17:06:30 -0700, Keith Packard <keithp@keithp.com> wrote:
> On Fri, 22 Apr 2011 08:01:46 +1000, Dave Airlie <airlied@gmail.com> wrote:
> 
> > My one is definitely, I've got a few people who can't use machines
> > without it, I was going to push it myself, but I'm being nice ;-)
> 
> I'll review all of them for .39 then; it wasn't clear from the original
> mail whether that was the desired target.

Sorry Keith, bit of private jargon. I use fixes to denote a patch queue
for bugs in released kernels and next for feature work.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: Pending i915 fixes

[Chris Wilson]

On Thu, 21 Apr 2011 14:56:21 -0700, Keith Packard <keithp@keithp.com> wrote:
> On Thu, 21 Apr 2011 22:18:15 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Whilst this may appear to be a big batch, it is actually only a few bug
> > fixes from the last couple of weeks broken down into small steps.
> 
> Do you think any of these are needed in .39?

These are all patches for bugs reported against current kernels, though
the assertion patch is just diagnostic rather than a fix.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre