| Message ID | 20131103211814.GC4167@phenom.ffwll.local (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Nov 03, 2013 at 10:18:14PM +0100, Daniel Vetter wrote: > > On Sun, Nov 3, 2013 at 8:00 PM, Thomas Richter <thor@math.tu-berlin.de> wrote: > > On 03.11.2013 18:13, Daniel Vetter wrote: > >>> > >>> Have you tried my patch to reorder the dvo sequence a bit? That /should/ > >>> get all these things right: > >>> > >>> http://www.spinics.net/lists/intel-gfx/msg34349.html > >> > >> There's also a follow-up patch for you to test: > >> > >> > >> http://www.spinics.net/lists/intel-gfx/msg34350.html > >> > >> That would prove that the first patch does indeed work. Note that patch 1 > >> in this series is already merged. > > > > Thanks, I tried. Actually, this worked fairly (but not perfectly) well. I no > > longer get a "stuck DVO" as I did before, i.e. the "re-enable" logic is not > > triggered anymore. However, I do get (occasionally) a kernel-oops: > > That's just a warning, not an oops ... > > > > > ------------[ cut here ]------------ > > WARNING: CPU: 0 PID: 2311 at drivers/gpu/drm/i915/intel_display.c:1098 > > assert_cursor.constprop.68+0xba/0xc0 [i915]() > > cursor on pipe A assertion failure (expected off, current on) > > Modules linked in: cpufreq_stats binfmt_misc michael_mic arc4 ecb > > lib80211_crypt_tkip lib80211_crypt_ccmp fuse nfsd exportfs nfs_acl nfs lockd > > fscache sunrpc loop firewire_sbp2 hostap_cs hostap snd_intel8x0 > > snd_ac97_codec ac97_bus lib80211 snd_pcm i915 snd_page_alloc snd_seq > > snd_seq_device snd_timer psmouse snd pcmcia apanel input_polldev soundcore > > yenta_socket pcmcia_rsrc pcmcia_core lpc_ich i2c_i801 mfd_core pcspkr > > i2c_algo_bit rng_core serio_raw evdev drm_kms_helper drm fujitsu_laptop > > led_class intel_agp intel_gtt agpgart i2c_core video ac button hid_generic > > usbhid hid sg sr_mod cdrom firewire_ohci firewire_core crc_itu_t 8139too > > 8139cp mii uhci_hcd usbcore usb_common > > CPU: 0 PID: 2311 Comm: Xorg Tainted: G W 3.12.0-rc7+ #13 > > Hardware name: FUJITSU SIEMENS LIFEBOOK S Series/FJNB159, BIOS Version 1.06 > > 07/02/2002 > > c12f8c44 c1033def f8ad3990 f6adfab0 00000907 f8ad25d8 0000044a f8a81ffa > > f8a81ffa f5ca8000 00000041 00000000 f5ca8000 c1033eb3 00000009 f6adfa98 > > f8ad3990 f6adfab0 f8a81ffa f8ad25d8 0000044a f8ad3990 00000041 f8adb9a0 > > Call Trace: > > [<c12f8c44>] ? dump_stack+0xa/0x13 > > [<c1033def>] ? warn_slowpath_common+0x7f/0xb0 > > [<f8a81ffa>] ? assert_cursor.constprop.68+0xba/0xc0 [i915] > > [<f8a81ffa>] ? assert_cursor.constprop.68+0xba/0xc0 [i915] > > [<c1033eb3>] ? warn_slowpath_fmt+0x33/0x40 > > [<f8a81ffa>] ? assert_cursor.constprop.68+0xba/0xc0 [i915] > > [<f8a83b90>] ? intel_disable_pipe+0x30/0xb0 [i915] > > [<f8a8491a>] ? i9xx_crtc_disable+0xca/0x2f0 [i915] > > [<f8a8b25e>] ? __intel_set_mode+0x7ae/0x13b0 [i915] > > [<f8a8e1f3>] ? intel_set_mode+0x23/0x40 [i915] > > [<f8a8e9bc>] ? intel_crtc_set_config+0x7ac/0x9d0 [i915] > > [<f802fe33>] ? drm_mode_set_config_internal+0x43/0xb0 [drm] > > [<f81386a5>] ? drm_fb_helper_set_par+0x45/0xac [drm_kms_helper] > > [<c11a7976>] ? fb_set_var+0x1a6/0x420 > > [<c10eec6e>] ? __find_get_block+0x9e/0x160 > > [<c11b56e0>] ? fbcon_blank+0x290/0x2d0 > > [<c1203848>] ? do_unblank_screen+0x98/0x1b0 > > [<c1201acd>] ? notify_update+0x1d/0x30 > > [<c11faa12>] ? complete_change_console+0x52/0xe0 > > [<c11fbba4>] ? vt_ioctl+0x1104/0x1220 > > [<f802a1d0>] ? drm_setmaster_ioctl+0xe0/0xe0 [drm] > > [<c11faaa0>] ? complete_change_console+0xe0/0xe0 > > [<c11f17da>] ? tty_ioctl+0x21a/0x9a0 > > [<c10adb12>] ? do_wp_page.isra.92+0x2a2/0x650 > > [<c10af428>] ? handle_mm_fault+0x318/0x610 > > [<c11f15c0>] ? no_tty+0x20/0x20 > > [<c10d66ac>] ? do_vfs_ioctl+0x7c/0x550 > > [<c102d221>] ? __do_page_fault+0x1b1/0x490 > > [<c10c7d17>] ? vfs_write+0x137/0x1b0 > > [<c10d6bc6>] ? SyS_ioctl+0x46/0x80 > > [<c12fc19e>] ? sysenter_do_call+0x12/0x26 > > ---[ end trace 85d231d1072e932a ]--- > > > > If this happens, I get a black screen with only a cursor on it. Switching to > > the console and back restores the screen. > > Hm, that would mean that the cursor is somehow stuck in the enabled state, > despite that we've tried to disabled it very hard. Can you please try out > the below patch? If this doesn't work please take not of the different > WARNINGs you're hitting and whether it's always the same one with the same > calltrace or something different. > > I think for now we should try to get the single monitor case working - I > have a few theories for the dual-screen issues, but there's not much point > working on them if the simple case doesn't work yet. > > Also I think I'll merge the two patches if they don't make things worse > for you, imo it's the right approach and at least conceptually should be > able to avoid all these retry loops. > > Thanks, Daniel > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index f34252d134b6..04d2699f51b4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -7123,7 +7123,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base) > intel_crtc->cursor_visible = visible; > } > /* and commit changes on next vblank */ > + POSTING_READ(CURCNTR(pipe)); > I915_WRITE(CURBASE(pipe), base); > + POSTING_READ(CURBASE(pipe)); > } > > static void ivb_update_cursor(struct drm_crtc *crtc, u32 base) > @@ -7152,7 +7154,9 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base) > intel_crtc->cursor_visible = visible; > } > /* and commit changes on next vblank */ > + POSTING_READ(CURCNTR_IVB(pipe)); > I915_WRITE(CURBASE_IVB(pipe), base); > + POSTING_READ(CURBASE_IVB(pipe)); > } To clarify: Do you need this patch to make the single-pipe mode work reliably? It's a bit unclear in your answer ... Thanks, Daniel
On 06.11.2013 11:34, Daniel Vetter wrote: >> } > > To clarify: Do you need this patch to make the single-pipe mode work > reliably? It's a bit unclear in your answer ... Well, from what I can tell, I haven't seen the above warning since, but it was neither easily reproducable. "Working reliable" is probably a bit too much. (-; No, the vertical position of the boot console is off, panning flickers, and the two-monitor case is close to non-working. But other than that, it works now regardless of whether the bios is set to internal or shared monitors. So I would guess it works as good as it currently can. (-: The harddisk of the R31 (the other laptop with the same dreadful chipset) died today, so I fished a "new old one" from our pile of junk, and re-installed Linux. It currently compiles the latest git, so please have some patience with a 1.2Ghz Celeron. Will be ready tomorrow, hopefully with more results for the "anti-flicker" patch. Anyhow. This patch is currently *only* tested for the linear framebuffer alignment, and most certainly not "right". I believe its some sort of memory alignment issue, and I need to dig a bit deeper what exactly happens there. Greetings, Thomas
On Wed, Nov 06, 2013 at 11:34:05AM +0100, Daniel Vetter wrote: > On Sun, Nov 03, 2013 at 10:18:14PM +0100, Daniel Vetter wrote: > > Hm, that would mean that the cursor is somehow stuck in the enabled state, > > despite that we've tried to disabled it very hard. Can you please try out > > the below patch? If this doesn't work please take not of the different > > WARNINGs you're hitting and whether it's always the same one with the same > > calltrace or something different. > > > > I think for now we should try to get the single monitor case working - I > > have a few theories for the dual-screen issues, but there's not much point > > working on them if the simple case doesn't work yet. > > > > Also I think I'll merge the two patches if they don't make things worse > > for you, imo it's the right approach and at least conceptually should be > > able to avoid all these retry loops. > > > > Thanks, Daniel > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index f34252d134b6..04d2699f51b4 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -7123,7 +7123,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base) > > intel_crtc->cursor_visible = visible; > > } > > /* and commit changes on next vblank */ > > + POSTING_READ(CURCNTR(pipe)); > > I915_WRITE(CURBASE(pipe), base); > > + POSTING_READ(CURBASE(pipe)); > > } > > > > static void ivb_update_cursor(struct drm_crtc *crtc, u32 base) > > @@ -7152,7 +7154,9 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base) > > intel_crtc->cursor_visible = visible; > > } > > /* and commit changes on next vblank */ > > + POSTING_READ(CURCNTR_IVB(pipe)); > > I915_WRITE(CURBASE_IVB(pipe), base); > > + POSTING_READ(CURBASE_IVB(pipe)); > > } > > To clarify: Do you need this patch to make the single-pipe mode work > reliably? It's a bit unclear in your answer ... To clarify my clarification question: Do you need the above quoted patch to make the cursor work better on your system? This is not about a WARN or the flicker or dual pipe (last time I've asked you kinda went on a tangent). I'm asking again since this patch for the cursor code is currently blocked from merging because I couldn't get a clear "this is needed, yes" from you. Thanks, Daniel
On 15.11.2013 18:33, Daniel Vetter wrote: >> To clarify: Do you need this patch to make the single-pipe mode work >> reliably? It's a bit unclear in your answer ... > > To clarify my clarification question: Do you need the above quoted patch > to make the cursor work better on your system? Well, at least it avoided one kernel warning. Or rather, I haven't seen it since. But otherwise, the cursor works fine. I don't have any problems with the cursor whatsoever, it always appears. > This is not about a WARN or > the flicker or dual pipe (last time I've asked you kinda went on a > tangent). I'm asking again since this patch for the cursor code is > currently blocked from merging because I couldn't get a clear "this is > needed, yes" from you. It's hard to say whether it is needed, unfortunately. The kernel warning is not so easy to provoke, and there is no clear way to reproduce it. All I can say is that I haven't seen it since, and the patch has surely not made anything worse, so I would say go for it and include it. Greetings, Thomas
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f34252d134b6..04d2699f51b4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7123,7 +7123,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base) intel_crtc->cursor_visible = visible; } /* and commit changes on next vblank */ + POSTING_READ(CURCNTR(pipe)); I915_WRITE(CURBASE(pipe), base); + POSTING_READ(CURBASE(pipe)); } static void ivb_update_cursor(struct drm_crtc *crtc, u32 base) @@ -7152,7 +7154,9 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base) intel_crtc->cursor_visible = visible; } /* and commit changes on next vblank */ + POSTING_READ(CURCNTR_IVB(pipe)); I915_WRITE(CURBASE_IVB(pipe), base); + POSTING_READ(CURBASE_IVB(pipe)); } /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */