[PATCH] drm/i915: Increase the amount of defense before computing vblank timestamps

[PATCH] drm/i915: Increase the amount of defense before computing vblank timestamps

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---

Chris, does this help?

If you can convert the address to a line number that would pinpoint the
NULL pointer exactly. Though judging by the address and position of the
dereference a NULL crtc looks most likely.
-Chris

---
 drivers/gpu/drm/i915/i915_irq.c |   24 +++++++++++++++++-------
 1 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f0c87bd..d999d00 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -274,24 +274,34 @@ int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
 	return ret;
 }
 
-int i915_get_vblank_timestamp(struct drm_device *dev, int crtc,
+int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
 			      int *max_error,
 			      struct timeval *vblank_time,
 			      unsigned flags)
 {
-	struct drm_crtc *drmcrtc;
+	struct drm_crtc *crtc;
 
-	if (crtc < 0 || crtc >= dev->num_crtcs) {
-		DRM_ERROR("Invalid crtc %d\n", crtc);
+	if (pipe < 0 || pipe >= dev->num_crtcs) {
+		DRM_ERROR("Invalid crtc %d\n", pipe);
 		return -EINVAL;
 	}
 
 	/* Get drm_crtc to timestamp: */
-	drmcrtc = intel_get_crtc_for_pipe(dev, crtc);
+	crtc = intel_get_crtc_for_pipe(dev, pipe);
+	if (crtc == NULL) {
+		DRM_ERROR("Invalid crtc %d\n", pipe);
+		return -EINVAL;
+	}
+
+	if (!crtc->enabled) {
+		DRM_DEBUG_KMS("crtc %d is disabled\n", pipe);
+		return -EBUSY;
+	}
 
 	/* Helper routine in DRM core does all the work: */
-	return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc, max_error,
-						     vblank_time, flags, drmcrtc);
+	return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
+						     vblank_time, flags,
+						     crtc);
 }
 
 /*
-- 
1.7.2.3

Re: [PATCH] drm/i915: Increase the amount of defense before computing vblank timestamps

[Chris Clayton]

On Saturday 22 January 2011, Chris Wilson wrote:
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>
> Chris, does this help?
>

Sorry Chris, X still doesn't start with this patch applied. The symptoms are 
slightly different in that the monitor doesn't go into power saving mode, but 
the screen is still black and I have to recycle the power to get the machine 
back.
The messages in the log file has changed though.We now have:

Jan 22 11:03:47 upstairs kernel: r8169 0000:02:00.0: eth0: link up
Jan 22 11:03:57 upstairs kernel: [drm] Initialized drm 1.1.0 20060810
Jan 22 11:03:58 upstairs kernel: pci 0000:00:02.0: PCI INT A -> GSI 16 (level, 
low) -> IRQ 16
Jan 22 11:03:58 upstairs kernel: pci 0000:00:02.0: setting latency timer to 64
Jan 22 11:03:58 upstairs kernel: ioremap error for 0xbdce0000-0xbdce3000, 
requested 0x10, got 0x0
Jan 22 11:03:58 upstairs kernel: [drm] Supports vblank timestamp caching Rev 1 
(10.10.2010).
Jan 22 11:03:58 upstairs kernel: [drm] Driver supports precise vblank timestamp 
query.
Jan 22 11:03:58 upstairs kernel: [drm] Initialized i915 1.6.0 20080730 for 
0000:00:02.0 on minor 0
Jan 22 11:04:18 upstairs kernel: [drm:i915_get_vblank_timestamp] *ERROR* Invalid 
crtc 0
Jan 22 11:04:18 upstairs last message repeated 8 times
Jan 22 11:04:18 upstairs kernel: [drm:i915_do_wait_request] *ERROR* something 
(likely vbetool) disabled interrupts, re-enabling
Jan 22 11:04:21 upstairs kernel: [drm:i915_get_vblank_timestamp] *ERROR* Invalid 
crtc 0
Jan 22 11:04:29 upstairs last message repeated 35 times
Jan 22 11:04:29 upstairs kernel: [drm:i915_do_wait_request] *ERROR* something 
(likely vbetool) disabled interrupts, re-enabling
Jan 22 11:04:32 upstairs kernel: [drm:i915_get_vblank_timestamp] *ERROR* Invalid 
crtc 0
Jan 22 11:05:00 upstairs last message repeated 98 times
Jan 22 11:05:00 upstairs kernel: [drm:i915_do_wait_request] *ERROR* something 
(likely vbetool) disabled interrupts, re-enabling
Jan 22 11:05:03 upstairs kernel: [drm:i915_get_vblank_timestamp] *ERROR* Invalid 
crtc 0
Jan 22 11:05:06 upstairs last message repeated 17 times
Jan 22 11:05:06 upstairs kernel: [drm:i915_do_wait_request] *ERROR* something 
(likely vbetool) disabled interrupts, re-enabling

> If you can convert the address to a line number that would pinpoint the

Not done that before and nothing jumped out of my search on google, so how do I 
go about getting the line number for you, please?

> NULL pointer exactly. Though judging by the address and position of the
> dereference a NULL crtc looks most likely.

<snip>


-- 
The more I see, the more I know. The more I know, the less I understand. 
Changing Man - Paul Weller

Re: [PATCH] drm/i915: Increase the amount of defense before computing vblank timestamps

[Chris Wilson]

On Sat, 22 Jan 2011 11:18:53 +0000, Chris Clayton <chris2553@googlemail.com> wrote:
> On Saturday 22 January 2011, Chris Wilson wrote:
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >
> > Chris, does this help?
> >
> 
> Sorry Chris, X still doesn't start with this patch applied. The symptoms are 
> slightly different in that the monitor doesn't go into power saving mode, but 
> the screen is still black and I have to recycle the power to get the machine 
> back.
> The messages in the log file has changed though.We now have:

Despite appearances to the contrary, it is a step forward. We have the
crash under control, and now need to tackle the underlying problem.

However, I'm worried by the fact that no CRTCs are attached. Is this not
using KMS?
 
> > If you can convert the address to a line number that would pinpoint the
> 
> Not done that before and nothing jumped out of my search on google, so how do I 
> go about getting the line number for you, please?

Something like:

$ gdb drivers/gpu/drm/drm.ko
(gdb) list *drm_calc_vbltimestamp_from_scanoutpos+0x3e

should do the trick (if compiled as a module, vmlinux otherwise)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Increase the amount of defense before computing vblank timestamps

[Chris Clayton]

On Saturday 22 January 2011, Chris Wilson wrote:
> On Sat, 22 Jan 2011 11:18:53 +0000, Chris Clayton <chris2553@googlemail.com> 
wrote:
> > On Saturday 22 January 2011, Chris Wilson wrote:
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > >
> > > Chris, does this help?
> >
> > Sorry Chris, X still doesn't start with this patch applied. The symptoms
> > are slightly different in that the monitor doesn't go into power saving
> > mode, but the screen is still black and I have to recycle the power to
> > get the machine back.
> > The messages in the log file has changed though.We now have:
>
> Despite appearances to the contrary, it is a step forward. We have the
> crash under control, and now need to tackle the underlying problem.
>
> However, I'm worried by the fact that no CRTCs are attached. Is this not
> using KMS?
>

No KMS is not configured in. I found that under an earlier version of the kernel 
it caused my kernel to lock up, so I configured it out. I meant to investigate 
further and report it, but then forgot. I'll turn KMS on and report the 
outcome.

> > > If you can convert the address to a line number that would pinpoint the
> >
> > Not done that before and nothing jumped out of my search on google, so
> > how do I go about getting the line number for you, please?
>
> Something like:
>
> $ gdb drivers/gpu/drm/drm.ko
> (gdb) list *drm_calc_vbltimestamp_from_scanoutpos+0x3e
>
> should do the trick (if compiled as a module, vmlinux otherwise)

Thanks for the tutoring. I get

(gdb) list *drm_calc_vbltimestamp_from_scanoutpos+0x3e
0x6afe is in drm_calc_vbltimestamp_from_scanoutpos 
(drivers/gpu/drm/drm_irq.c:590).
585                     DRM_ERROR("Called from driver w/o 
get_scanout_position()!?\n");
586                     return -EIO;
587             }
588
589             mode = &refcrtc->hwmode;
590             vtotal = mode->crtc_vtotal;
591             vdisplay = mode->crtc_vdisplay;
592
593             /* Durations of frames, lines, pixels in nanoseconds. */
594             framedur_ns = refcrtc->framedur_ns;


Chris
> -Chris

-- 
The more I see, the more I know. The more I know, the less I understand. 
Changing Man - Paul Weller

Re: [PATCH] drm/i915: Increase the amount of defense before computing vblank timestamps

[Chris Clayton]

On Sunday 23 January 2011, Chris Clayton wrote:
> On Saturday 22 January 2011, Chris Wilson wrote:
> > On Sat, 22 Jan 2011 11:18:53 +0000, Chris Clayton
> > <chris2553@googlemail.com>
>
> wrote:
> > > On Saturday 22 January 2011, Chris Wilson wrote:
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > ---
> > > >
> > > > Chris, does this help?
> > >
> > > Sorry Chris, X still doesn't start with this patch applied. The
> > > symptoms are slightly different in that the monitor doesn't go into
> > > power saving mode, but the screen is still black and I have to recycle
> > > the power to get the machine back.
> > > The messages in the log file has changed though.We now have:
> >
> > Despite appearances to the contrary, it is a step forward. We have the
> > crash under control, and now need to tackle the underlying problem.
> >
> > However, I'm worried by the fact that no CRTCs are attached. Is this not
> > using KMS?
>
> No KMS is not configured in. I found that under an earlier version of the
> kernel it caused my kernel to lock up, so I configured it out. I meant to
> investigate further and report it, but then forgot. I'll turn KMS on and
> report the outcome.
>

Mmmm, I get a lock up when turning KMS on in 2.3.38-rc2+ and it looks like KMS 
is inducing a crash in X. The Xorg log contains:

Backtrace:
0: /usr/X11/bin/X(xorg_backtrace+0x3b) [0x81352db]
1: /usr/X11/bin/X(xf86SigHandler+0x51) [0x80ba0b1]
2: [0xb78b5400]
3: /usr/X11R6/lib/libdrm_intel.so.1(+0x624b) [0xb72d024b]
4: /usr/X11R6/lib/libdrm_intel.so.1(drm_intel_bo_unreference+0x16) [0xb72cb446]
5: /usr/X11/lib/xorg/modules/drivers//intel_drv.so(intel_batch_flush+0xb6) 
[0xb72ec876]
6: /usr/X11/lib/xorg/modules/drivers//intel_drv.so(+0x4b5d0) [0xb73215d0]
7: /usr/X11/lib/xorg/modules/drivers//intel_drv.so(uxa_fill_region_tiled+0x264) 
[0xb7338fc4]
8: /usr/X11/lib/xorg/modules/drivers//intel_drv.so(+0x6343b) [0xb733943b]
9: /usr/X11/bin/X() [0x817b8c6]
10: /usr/X11/bin/X(miPaintWindow+0x1cb) [0x811484b]
11: /usr/X11/bin/X(miWindowExposures+0xbf) [0x8114bbf]
12: /usr/X11/bin/X() [0x80b364f]
13: /usr/X11/bin/X(MapWindow+0x3a8) [0x8072cc8]
14: /usr/X11/bin/X(InitRootWindow+0x10c) [0x8072dfc]
15: /usr/X11/bin/X(main+0x40a) [0x806be0a]
16: /lib/libc.so.6(__libc_start_main+0xe6) [0xb7412b86]
17: /usr/X11/bin/X() [0x806b2c1]

Fatal server error:
Caught signal 11.  Server aborting

and the kernel log contains:

Jan 23 09:07:38 upstairs kernel: X[1273]: segfault at 3fe8 ip b72ebc95 sp 
bfdbf4b0 error 6 in intel_drv.so[b72d6000+76000]

So it looks like there are two problems. I'm happy to help fix both, but would 
prefer to get the non-KMS setup working first, so that I can run the 
development kernel to see if I find any other problems. Once the graphics mode 
has been switched, the text on the screen as the kernel boots is so small, that 
my old eyes struggle to see it, so normally I will probably run without KMS, as 
long as that is supported.

<snip>

Chris
-- 
The more I see, the more I know. The more I know, the less I understand. 
Changing Man - Paul Weller

[PATCH] drm/i915: Disable high-precision vblank timestamping for UMS

[Chris Wilson]

We only have sufficient information for accurate (sub-frame) timestamping
when the modesetting is under our control.

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

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 59eb19b..66796bb 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -752,6 +752,9 @@ static int __init i915_init(void)
 		driver.driver_features &= ~DRIVER_MODESET;
 #endif
 
+	if (!(driver.driver_features & DRIVER_MODESET))
+		driver.get_vblank_timestamp = NULL;
+
 	return drm_init(&driver);
 }
 
-- 
1.7.2.3

Re: [PATCH] drm/i915: Increase the amount of defense before computing vblank timestamps

[Chris Wilson]

On Sun, 23 Jan 2011 09:32:49 +0000, Chris Clayton <chris2553@googlemail.com> wrote:
> On Sunday 23 January 2011, Chris Clayton wrote:
> > On Saturday 22 January 2011, Chris Wilson wrote:
> > > On Sat, 22 Jan 2011 11:18:53 +0000, Chris Clayton
> > > <chris2553@googlemail.com>
> >
> > wrote:
> > > > On Saturday 22 January 2011, Chris Wilson wrote:
> > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > ---
> > > > >
> > > > > Chris, does this help?
> > > >
> > > > Sorry Chris, X still doesn't start with this patch applied. The
> > > > symptoms are slightly different in that the monitor doesn't go into
> > > > power saving mode, but the screen is still black and I have to recycle
> > > > the power to get the machine back.
> > > > The messages in the log file has changed though.We now have:
> > >
> > > Despite appearances to the contrary, it is a step forward. We have the
> > > crash under control, and now need to tackle the underlying problem.
> > >
> > > However, I'm worried by the fact that no CRTCs are attached. Is this not
> > > using KMS?
> >
> > No KMS is not configured in. I found that under an earlier version of the
> > kernel it caused my kernel to lock up, so I configured it out. I meant to
> > investigate further and report it, but then forgot. I'll turn KMS on and
> > report the outcome.
> >
> 
> Mmmm, I get a lock up when turning KMS on in 2.3.38-rc2+ and it looks like KMS 
> is inducing a crash in X. The Xorg log contains:
> 
> Backtrace:
> 0: /usr/X11/bin/X(xorg_backtrace+0x3b) [0x81352db]
> 1: /usr/X11/bin/X(xf86SigHandler+0x51) [0x80ba0b1]
> 2: [0xb78b5400]
> 3: /usr/X11R6/lib/libdrm_intel.so.1(+0x624b) [0xb72d024b]
> 4: /usr/X11R6/lib/libdrm_intel.so.1(drm_intel_bo_unreference+0x16) [0xb72cb446]
> 5: /usr/X11/lib/xorg/modules/drivers//intel_drv.so(intel_batch_flush+0xb6) 
> [0xb72ec876]
> 6: /usr/X11/lib/xorg/modules/drivers//intel_drv.so(+0x4b5d0) [0xb73215d0]
> 7: /usr/X11/lib/xorg/modules/drivers//intel_drv.so(uxa_fill_region_tiled+0x264) 
> [0xb7338fc4]
> 8: /usr/X11/lib/xorg/modules/drivers//intel_drv.so(+0x6343b) [0xb733943b]
> 9: /usr/X11/bin/X() [0x817b8c6]
> 10: /usr/X11/bin/X(miPaintWindow+0x1cb) [0x811484b]
> 11: /usr/X11/bin/X(miWindowExposures+0xbf) [0x8114bbf]
> 12: /usr/X11/bin/X() [0x80b364f]
> 13: /usr/X11/bin/X(MapWindow+0x3a8) [0x8072cc8]
> 14: /usr/X11/bin/X(InitRootWindow+0x10c) [0x8072dfc]
> 15: /usr/X11/bin/X(main+0x40a) [0x806be0a]
> 16: /lib/libc.so.6(__libc_start_main+0xe6) [0xb7412b86]
> 17: /usr/X11/bin/X() [0x806b2c1]

This looks more like typical memory corruption, and the usual requests to
make sure you have the latest stable drivers apply: Xorg-1.9,
xf86-video-intel-2.14, libdrm-2.4.23, [mesa-7.10].
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Disable high-precision vblank timestamping for UMS

[Chris Clayton]

Hi Chris,

On Sunday 23 January 2011, Chris Wilson wrote:
> We only have sufficient information for accurate (sub-frame) timestamping
> when the modesetting is under our control.
>
> Reported-by: Chris Clayton <chris2553@googlemail.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c index 59eb19b..66796bb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -752,6 +752,9 @@ static int __init i915_init(void)
>  		driver.driver_features &= ~DRIVER_MODESET;
>  #endif
>
> +	if (!(driver.driver_features & DRIVER_MODESET))
> +		driver.get_vblank_timestamp = NULL;
> +
>  	return drm_init(&driver);
>  }

With this patch (and the earlier one) applied, X still doesn't start. At the end 
of xorg's log file I see:

Fatal server error:
Failure to wait for IRQ: No such device

and in the kernel log I see:

Jan 23 11:15:25 upstairs kernel: [drm] Initialized drm 1.1.0 20060810
Jan 23 11:15:26 upstairs kernel: pci 0000:00:02.0: PCI INT A -> GSI 16 (level, 
low) -> IRQ 16
Jan 23 11:15:26 upstairs kernel: pci 0000:00:02.0: setting latency timer to 64
Jan 23 11:15:26 upstairs kernel: ioremap error for 0xbdce0000-0xbdce3000, 
requested 0x10, got 0x0
Jan 23 11:15:26 upstairs kernel: [drm] Supports vblank timestamp caching Rev 1 
(10.10.2010).
Jan 23 11:15:26 upstairs kernel: [drm] No driver support for vblank timestamp 
query.
Jan 23 11:15:26 upstairs kernel: [drm] Initialized i915 1.6.0 20080730 for 
0000:00:02.0 on minor 0

Chris

-- 
The more I see, the more I know. The more I know, the less I understand. 
Changing Man - Paul Weller

Re: [PATCH] drm/i915: Disable high-precision vblank timestamping for UMS

[Chris Wilson]

On Sun, 23 Jan 2011 11:28:22 +0000, Chris Clayton <chris2553@googlemail.com> wrote:
> Hi Chris,
> 
> On Sunday 23 January 2011, Chris Wilson wrote:
> > We only have sufficient information for accurate (sub-frame) timestamping
> > when the modesetting is under our control.
> >
> With this patch (and the earlier one) applied, X still doesn't start. At the end 
> of xorg's log file I see:
> 
> Fatal server error:
> Failure to wait for IRQ: No such device

Now we are getting closer! The X server has been caught red-handed trying
to use an interrupt when the driver never enabled them. Ho hum.

The interrupts for UMS are only enabled between EnterVT/LeaveVT, but that
code looks defunct. What version of xf86-video-intel are you using?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Disable high-precision vblank timestamping for UMS

[Chris Clayton]

On Sunday 23 January 2011, Chris Wilson wrote:
> On Sun, 23 Jan 2011 11:28:22 +0000, Chris Clayton <chris2553@googlemail.com> 
wrote:
> > Hi Chris,
> >
> > On Sunday 23 January 2011, Chris Wilson wrote:
> > > We only have sufficient information for accurate (sub-frame)
> > > timestamping when the modesetting is under our control.
> >
> > With this patch (and the earlier one) applied, X still doesn't start. At
> > the end of xorg's log file I see:
> >
> > Fatal server error:
> > Failure to wait for IRQ: No such device
>
> Now we are getting closer! The X server has been caught red-handed trying
> to use an interrupt when the driver never enabled them. Ho hum.
>
> The interrupts for UMS are only enabled between EnterVT/LeaveVT, but that
> code looks defunct. What version of xf86-video-intel are you using?

It's 2.7.1, which, IIRC, is the latest version that will build against all the 
other xorg libraries that are on my system. I'll do a quick check after lunch 
to see if any later versions will build without me going through dependency 
hell.

Thanks for your help so far,

Chris

> -Chris

-- 
The more I see, the more I know. The more I know, the less I understand. 
Changing Man - Paul Weller

[PATCH] drm/i915: Handle the no-interrupts case for UMS by polling

[Chris Wilson]

If the driver calls into the kernel to wait for a breadcrumb to pass,
but hasn't enabled interrupts, fallback to polling the breadcrumb value.

Reported-by: Chris Clayton <chris2553@googlemail.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_irq.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 98106b7..4b5a35c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1293,12 +1293,12 @@ static int i915_wait_irq(struct drm_device * dev, int irq_nr)
 	if (master_priv->sarea_priv)
 		master_priv->sarea_priv->perf_boxes |= I915_BOX_WAIT;
 
-	ret = -ENODEV;
 	if (ring->irq_get(ring)) {
 		DRM_WAIT_ON(ret, ring->irq_queue, 3 * DRM_HZ,
 			    READ_BREADCRUMB(dev_priv) >= irq_nr);
 		ring->irq_put(ring);
-	}
+	} else if (wait_for(READ_BREADCRUMB(dev_priv) >= irq_nr, 3000))
+		ret = -EBUSY;
 
 	if (ret == -EBUSY) {
 		DRM_ERROR("EBUSY -- rec: %d emitted: %d\n",
-- 
1.7.2.3

[PATCH] drm/i915: Handle the no-interrupts case for UMS by polling

[Chris Wilson]

If the driver calls into the kernel to wait for a breadcrumb to pass,
but hasn't enabled interrupts, fallback to polling the breadcrumb value.

Reported-by: Chris Clayton <chris2553@googlemail.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_irq.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 98106b7..4b5a35c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1293,12 +1293,12 @@ static int i915_wait_irq(struct drm_device * dev, int irq_nr)
 	if (master_priv->sarea_priv)
 		master_priv->sarea_priv->perf_boxes |= I915_BOX_WAIT;
 
-	ret = -ENODEV;
 	if (ring->irq_get(ring)) {
 		DRM_WAIT_ON(ret, ring->irq_queue, 3 * DRM_HZ,
 			    READ_BREADCRUMB(dev_priv) >= irq_nr);
 		ring->irq_put(ring);
-	}
+	} else if (wait_for(READ_BREADCRUMB(dev_priv) >= irq_nr, 3000))
+		ret = -EBUSY;
 
 	if (ret == -EBUSY) {
 		DRM_ERROR("EBUSY -- rec: %d emitted: %d\n",
-- 
1.7.2.3

Re: [PATCH] drm/i915: Handle the no-interrupts case for UMS by polling

[Chris Clayton]

On Sunday 23 January 2011, Chris Wilson wrote:
> If the driver calls into the kernel to wait for a breadcrumb to pass,
> but hasn't enabled interrupts, fallback to polling the breadcrumb value.
>
> Reported-by: Chris Clayton <chris2553@googlemail.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_irq.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c index 98106b7..4b5a35c 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1293,12 +1293,12 @@ static int i915_wait_irq(struct drm_device * dev,
> int irq_nr) if (master_priv->sarea_priv)
>  		master_priv->sarea_priv->perf_boxes |= I915_BOX_WAIT;
>
> -	ret = -ENODEV;
>  	if (ring->irq_get(ring)) {
>  		DRM_WAIT_ON(ret, ring->irq_queue, 3 * DRM_HZ,
>  			    READ_BREADCRUMB(dev_priv) >= irq_nr);
>  		ring->irq_put(ring);
> -	}
> +	} else if (wait_for(READ_BREADCRUMB(dev_priv) >= irq_nr, 3000))
> +		ret = -EBUSY;
>
>  	if (ret == -EBUSY) {
>  		DRM_ERROR("EBUSY -- rec: %d emitted: %d\n",

That's done it. With all three patches applied, X and KDE are starting now. 
Thanks for your work on this.

A couple of points though.

1. The latest patch applied but the following warning was spat out:

patching file drivers/gpu/drm/i915/i915_irq.c
Hunk #1 succeeded at 1288 (offset -5 lines).

I guess we may be working with slightly different versions of the source.

2. The kernel log now contains the following:

r8169 0000:02:00.0: eth0: link up
[drm] Initialized drm 1.1.0 20060810
irq 16: nobody cared (try booting with the "irqpoll" option)
Pid: 0, comm: kworker/0:1 Not tainted 2.6.38-rc2+ #502
Call Trace:
 [<c1070a54>] ? __report_bad_irq.clone.2+0x24/0x90
 [<c1070c17>] ? note_interrupt+0x157/0x190
 [<c101b1cd>] ? ack_apic_level+0x5d/0x1e0
 [<c107175b>] ? handle_fasteoi_irq+0xab/0xd0
 [<c10716b0>] ? handle_fasteoi_irq+0x0/0xd0
 <IRQ>  [<c1004ddd>] ? do_IRQ+0x3d/0xc0
 [<c1037f27>] ? irq_exit+0x57/0x70
 [<c1019173>] ? smp_apic_timer_interrupt+0x53/0x90
 [<c1003229>] ? common_interrupt+0x29/0x30
 [<c119fbe9>] ? acpi_safe_halt+0x19/0x26
 [<c119fc50>] ? acpi_idle_enter_c1+0x5a/0xa9
 [<c1244d98>] ? cpuidle_idle_call+0x68/0xf0
 [<c1001726>] ? cpu_idle+0x46/0x80
handlers:
[<c129dd60>] (azx_interrupt+0x0/0x180)
[<c12210d0>] (usb_hcd_irq+0x0/0x60)
Disabling IRQ #16
pci 0000:00:02.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
pci 0000:00:02.0: setting latency timer to 64
ioremap error for 0xbdce0000-0xbdce3000, requested 0x10, got 0x0
[drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
[drm] No driver support for vblank timestamp query.
[drm] Initialized i915 1.6.0 20080730 for 0000:00:02.0 on minor 0

Is this likely to be related the the irq issues your patch addressed or is it a 
separate thing that I should report separately?

Thanks again.

Chris

-- 
The more I see, the more I know. The more I know, the less I understand. 
Changing Man - Paul Weller

Re: [PATCH] drm/i915: Handle the no-interrupts case for UMS by polling

[Chris Wilson]

On Sun, 23 Jan 2011 15:38:41 +0000, Chris Clayton <chris2553@googlemail.com> wrote:
> That's done it. With all three patches applied, X and KDE are starting now. 
> Thanks for your work on this.

That's good to hear!

> A couple of points though.
> 
> 1. The latest patch applied but the following warning was spat out:
> 
> patching file drivers/gpu/drm/i915/i915_irq.c
> Hunk #1 succeeded at 1288 (offset -5 lines).

Minor drift as I now have a number of outstanding patches for 2.6.38.
 
> I guess we may be working with slightly different versions of the source.
> 
> 2. The kernel log now contains the following:
> 
> r8169 0000:02:00.0: eth0: link up
> [drm] Initialized drm 1.1.0 20060810
> irq 16: nobody cared (try booting with the "irqpoll" option)
> Pid: 0, comm: kworker/0:1 Not tainted 2.6.38-rc2+ #502
> Call Trace:
>  [<c1070a54>] ? __report_bad_irq.clone.2+0x24/0x90
>  [<c1070c17>] ? note_interrupt+0x157/0x190
>  [<c101b1cd>] ? ack_apic_level+0x5d/0x1e0
>  [<c107175b>] ? handle_fasteoi_irq+0xab/0xd0
>  [<c10716b0>] ? handle_fasteoi_irq+0x0/0xd0
>  <IRQ>  [<c1004ddd>] ? do_IRQ+0x3d/0xc0
>  [<c1037f27>] ? irq_exit+0x57/0x70
>  [<c1019173>] ? smp_apic_timer_interrupt+0x53/0x90
>  [<c1003229>] ? common_interrupt+0x29/0x30
>  [<c119fbe9>] ? acpi_safe_halt+0x19/0x26
>  [<c119fc50>] ? acpi_idle_enter_c1+0x5a/0xa9
>  [<c1244d98>] ? cpuidle_idle_call+0x68/0xf0
>  [<c1001726>] ? cpu_idle+0x46/0x80
> handlers:
> [<c129dd60>] (azx_interrupt+0x0/0x180)
> [<c12210d0>] (usb_hcd_irq+0x0/0x60)
> Disabling IRQ #16
> pci 0000:00:02.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
> pci 0000:00:02.0: setting latency timer to 64
> ioremap error for 0xbdce0000-0xbdce3000, requested 0x10, got 0x0
> [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
> [drm] No driver support for vblank timestamp query.
> [drm] Initialized i915 1.6.0 20080730 for 0000:00:02.0 on minor 0
> 
> Is this likely to be related the the irq issues your patch addressed or is it a 
> separate thing that I should report separately?

It's the i915 IRQ alright, but I'm not sure what the cause is. An
unexpected interrupt arrived, although you don't use any paths in the
driver that use interrupts, nor should one be generated when all we are
doing is initialising the device structs. Should just be a minor nuisance
fortunately.

If you can open a bug report to track it and attach
/sys/kernel/debug/dri/0/i915_gem_interrupts maybe we can work out from
the interrupt that arrived, why it was triggered.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Handle the no-interrupts case for UMS by polling

[Chris Clayton]

Hi Chris,

On Sunday 23 January 2011, Chris Wilson wrote:
> On Sun, 23 Jan 2011 15:38:41 +0000, Chris Clayton <chris2553@googlemail.com> 
wrote:

<snip>

> > I guess we may be working with slightly different versions of the source.
> >
> > 2. The kernel log now contains the following:
> >
> > r8169 0000:02:00.0: eth0: link up
> > [drm] Initialized drm 1.1.0 20060810
> > irq 16: nobody cared (try booting with the "irqpoll" option)
> > Pid: 0, comm: kworker/0:1 Not tainted 2.6.38-rc2+ #502
> > Call Trace:
> >  [<c1070a54>] ? __report_bad_irq.clone.2+0x24/0x90
> >  [<c1070c17>] ? note_interrupt+0x157/0x190
> >  [<c101b1cd>] ? ack_apic_level+0x5d/0x1e0
> >  [<c107175b>] ? handle_fasteoi_irq+0xab/0xd0
> >  [<c10716b0>] ? handle_fasteoi_irq+0x0/0xd0
> >  <IRQ>  [<c1004ddd>] ? do_IRQ+0x3d/0xc0
> >  [<c1037f27>] ? irq_exit+0x57/0x70
> >  [<c1019173>] ? smp_apic_timer_interrupt+0x53/0x90
> >  [<c1003229>] ? common_interrupt+0x29/0x30
> >  [<c119fbe9>] ? acpi_safe_halt+0x19/0x26
> >  [<c119fc50>] ? acpi_idle_enter_c1+0x5a/0xa9
> >  [<c1244d98>] ? cpuidle_idle_call+0x68/0xf0
> >  [<c1001726>] ? cpu_idle+0x46/0x80
> > handlers:
> > [<c129dd60>] (azx_interrupt+0x0/0x180)
> > [<c12210d0>] (usb_hcd_irq+0x0/0x60)
> > Disabling IRQ #16
> > pci 0000:00:02.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
> > pci 0000:00:02.0: setting latency timer to 64
> > ioremap error for 0xbdce0000-0xbdce3000, requested 0x10, got 0x0
> > [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
> > [drm] No driver support for vblank timestamp query.
> > [drm] Initialized i915 1.6.0 20080730 for 0000:00:02.0 on minor 0
> >
> > Is this likely to be related the the irq issues your patch addressed or
> > is it a separate thing that I should report separately?
>
> It's the i915 IRQ alright, but I'm not sure what the cause is. An
> unexpected interrupt arrived, although you don't use any paths in the
> driver that use interrupts, nor should one be generated when all we are
> doing is initialising the device structs. Should just be a minor nuisance
> fortunately.
>

OK, I haven't raised the bug report yet, but I now know how to create it - 
reboot via kexec which is what I must have done when I built and installed the 
kernel to test your patches. I guess not having booted via the bios is leaving 
the card in an odd state. Do you care any more, please? If you do, I'll open 
the bug report. 

> If you can open a bug report to track it and attach
> /sys/kernel/debug/dri/0/i915_gem_interrupts maybe we can work out from
> the interrupt that arrived, why it was triggered.
> -Chris

Chris

-- 
The more I see, the more I know. The more I know, the less I understand. 
Changing Man - Paul Weller

Re: [PATCH] drm/i915: Handle the no-interrupts case for UMS by polling

[Chris Wilson]

On Thu, 10 Feb 2011 19:58:12 +0000, Chris Clayton <chris2553@googlemail.com> wrote:
> OK, I haven't raised the bug report yet, but I now know how to create it - 
> reboot via kexec which is what I must have done when I built and installed the 
> kernel to test your patches. I guess not having booted via the bios is leaving 
> the card in an odd state. Do you care any more, please? If you do, I'll open 
> the bug report. 

Please, I think we left my very small field of expertise so a bug may help
attract the attention of those in the know...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre