Bug 77351

Created attachment 97244 [details]
KDE's default black cursors (again with correct type...)

Created attachment 97245 [details]
KDE's oxygen cursors

with a pure startx and LXDE similar things happen (with current git version): Cursor is a a styled (non-black) pointer after LXDE start BUT it does not change to a textcursor when I hover ofter a text field / console (it does with 2.99.910).

I dug out an 855gm to check, and I have not noticed any odd behaviour (or lag) with both 2-color and argb cursors.

Please do install the latest from xf86-video-intel.git and confirm that Xorg.0.log reports the right version.

If it still occurs, apply

diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index 3f2b912..067f086 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -87,8 +87,8 @@ union compat_mode_get_connector{
 #define DEFAULT_DPI 96
 #endif
 
-#if 0
-#define __DBG DBG
+#if 1
+#define __DBG(x) ErrorF x
 #else
 #define __DBG(x)
 #endif

and attach the Xorg.0.log

Thank you! Which parts of the Xorg.0.log are you interested in? I do not want to post it completely due to privacy reasons.

The tail of the log should be full of cursor messages, which do not contain any sensitive information.

the normal behaviour is: watch cursor when KDM login screen loads, then black pointer. Changes to white, styled pointer when my user profile has loaded.

Verified that the problem still occurs with:

intel(0): SNA compiled from 2.99.911-73-g9ae8213

Adding the debug patch:

patch sna_display.c ~⁄thepatch 

told something about ffuzz 1 but the patch seems to have worked.

Created attachment 97352 [details]
Xorg.0.log after logging in

With KDM into KDE4 and browsing a bit in Firefox.  After editing a text field I had a text cursor for the whole time.

Created attachment 97353 [details]
Xorg.0.log after logoff and relogin

in KDM login screen I had the basic watch cursor which stayed even after login completed. At last it changed to a Oxygen text cursor.

The log behaves as I expect. The change of cursors is being sent to the kernel, but you observe that the updates do not occur.

Can you please try this kernel patch:

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 764467e..efcfd53 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5055,6 +5055,7 @@ i915_gem_phys_pwrite(struct drm_device *dev,
                        return -EFAULT;
        }
 
+       drm_clflush_virt_range(vaddr, args->size);
        i915_gem_chipset_flush(dev);
        return 0;
 }

The other thing may be the two CRTCs using the same physical address. It's not prohibited by the spec, but it may just be upsetting the hw?

(In reply to comment #10)
> The log behaves as I expect. The change of cursors is being sent to the
> kernel, but you observe that the updates do not occur.

Okay, but it was working with the older driver versions?! Just mentioning in case you did not see it. 

> Can you please try this kernel patch:

Uuh, a kernel patch... Never did this (also never compiled the kernel myself). Sorry, can you tell me how in a few words? Do I need to get the kernel, apply the patch and then compile the kernel myself? ... If it is worth the effort - apparently I am running into a seldom bug here. Thank you anyway!

> The other thing may be the two CRTCs using the same physical address. It's
> not prohibited by the spec, but it may just be upsetting the hw?

Do you mean my both screens (VGA1 LVDS1)? One is the laptop's built-in screen and another one is attached via VGA port. I could not find a physical address in my Xorg.0.log. Do I (or my distro's defaults) use a not-so-good configuration? If yes, sorry about this.

(In reply to comment #11)
> (In reply to comment #10)
> > The log behaves as I expect. The change of cursors is being sent to the
> > kernel, but you observe that the updates do not occur.
> 
> Okay, but it was working with the older driver versions?! Just mentioning in
> case you did not see it.

Right, and the only difference between then and now would appear to be use of a single cursor for both CRTCs. (Previously each CRTC had an independent cursor allocated for it.) It just may be that our cursor programming sequence is wrong in the kernel, we keep finding bugs there so it would not be too surprising.

> > Can you please try this kernel patch:
> 
> Uuh, a kernel patch... Never did this (also never compiled the kernel
> myself). Sorry, can you tell me how in a few words? Do I need to get the
> kernel, apply the patch and then compile the kernel myself? ... If it is
> worth the effort - apparently I am running into a seldom bug here. Thank you
> anyway!

Getting to the bottom of this is most certainly worth the effort!

First of to compile the kernel, run

$ git clone --depth 1 --branch drm-intel-fixes git://people.freedesktop.org/~danvet/drm-intel
$ cd drm-intel
$ cp /boot/config-`uname -r` .config
$ make oldconfig
$ make
$ sudo make modules_install install

That should make the new kernel available for booting. Check that it does boot! Applying the patch should then be easier (you can probably do it faster using a text editor than pasting it into patch -p1) and then you just need to repeat make && sudo make modules_install install.
 
> > The other thing may be the two CRTCs using the same physical address. It's
> > not prohibited by the spec, but it may just be upsetting the hw?
> 
> Do you mean my both screens (VGA1 LVDS1)? One is the laptop's built-in
> screen and another one is attached via VGA port. I could not find a physical
> address in my Xorg.0.log. Do I (or my distro's defaults) use a not-so-good
> configuration? If yes, sorry about this.

No, it's a quirk of this particular hardware that it reads the cursor using a physical address to a contiguous chunk of memory.

Okay, thanks for your great comments so far!

What is this ~danvet repo? By the way: It says "IMPORTANT: drm-intel moved to git://anongit.freedesktop.org/drm-intel".  It seems to be deprecated.

I was thinking of downloading the kernel from kernel.org and not some private source (of which I do not know its trustworthiness). Also okay?

I will try to look into it more closely tomorrow.

(In reply to comment #13)
> Okay, thanks for your great comments so far!
> 
> What is this ~danvet repo? By the way: It says "IMPORTANT: drm-intel moved
> to git://anongit.freedesktop.org/drm-intel".  It seems to be deprecated.

Oops, sorry. Keep forgetting Daniel moved it, use the new address instead.
danvet == Daniel Vetter, our kernel driver maintainer
 
> I was thinking of downloading the kernel from kernel.org and not some
> private source (of which I do not know its trustworthiness). Also okay?

Using Linus should be ok in this case. drm-intel contains everything we are sending upstream.

I had a look at https://wiki.archlinux.org/index.php/Kernels/Compilation/Traditional and https://wiki.archlinux.org/index.php/Kernels/Compilation/Arch_Build_System. I never used the Arch build system (ABS) before. I think this is the more easy and safe method, but I think I should try to understand the ABS before I try to use it for a kernel compilation/installation. The traditional method seems not to be easy too. The machine which shows the problems is my main machine and I do not like to mess it up. Also I am running out of backup HDDs.

I tried to follow your command list in brain dry-run: Problems already start  with the issue that I do not have any /boot/config-* file which could be cp'ed. ls: grub  initramfs-linux-fallback.img  initramfs-linux.img  vmlinuz-linux

So, sorry, I think this all would take too much time from me (although it is interesting). Isn't there another option? Or is the only other option to wait for someone with the same problems who can do a kernel patch?  I plan to move to a new main computer in the next months which would make my old computer free for messy testing, but I guess this would be too late.

The other option is to wait and see if I can reproduce your issue. So far I have only looked at an 855gm with its builtin LVDS, and it appears to work just fine. Next I need to see what happens when I plug in an external monitor.

good idea! I have tested with several configs (listed wich devices connected to the notebook):

* LAN: OK
* LAN, USB keyboard, USB mouse: OK
* LAN, USB keyboard, USB mouse, VGA: PROBLEM

The cursor problem only seems to happen with the external VGA monitor connected.

Note that my VGA has a bigger resolution as the LVDS (did not test if this makes a difference): 

[    41.920] (II) intel(0): switch to mode 1280x1024@60.0 on VGA1 using pipe 0, position (0, 0), rotation normal, reflection none
[    41.936] (II) intel(0): switch to mode 1024x768@60.0 on LVDS1 using pipe 1, position (0, 0), rotation normal, reflection none
[    41.951] (II) intel(0): Setting screen physical size to 338 x 270

Spotted the bug:

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3d017f5b656a..261fc7196e57 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7776,13 +7776,14 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
 
                addr = i915_gem_obj_ggtt_offset(obj);
        } else {
-               int align = IS_I830(dev) ? 16 * 1024 : 256;
-               ret = i915_gem_attach_phys_object(dev, obj,
-                                                 (intel_crtc->pipe == 0) ? I915_GEM_PHYS_CURSOR_0 : I915_GEM_PHYS_CURSOR_1,
-                                                 align);
-               if (ret) {
-                       DRM_DEBUG_KMS("failed to attach phys object\n");
-                       goto fail_locked;
+               if (obj->phys_obj == NULL) {
+                       ret = i915_gem_attach_phys_object(dev, obj,
+                                                         (intel_crtc->pipe == 0) ? I915_GEM_PHYS_CURSOR_0 : I915_GEM_PHYS_CURSOR_1,
+                                                         IS_I830(dev) ? 16 * 1024 : 256);
+                       if (ret) {
+                               DRM_DEBUG_KMS("failed to attach phys object\n");
+                               goto fail_locked;
+                       }
                }
                addr = obj->phys_obj->handle->busaddr;
        }

Created attachment 97503 [details] [review]
Fix dynamic allocation of physical handles

Super, Chris! The bug was in the Kernel, correct? So I cannot easily test, if it fixes the issue on my computer too.

So all driver versions after snapshot 2.99.911 (exactly after 2014-03-27 25ca8f1 sna: Support variable sized cursors) will only work with a patched / new kernel version (something later than 3.14.1), if I understand correctly.

The bug is limited to just a few models of machines, and we should be pushing the kernel patch back through the stable trees since we end up doing nasty things to hardware. But I am working on a follow up patch to add a kernel feature that will both make the cursors more efficient and allow userspace to detect when it needs to workaround a broken kernel.

Created attachment 97557 [details] [review]
Don't reuse cursors on broken kernels

This should do the trick for your machine and kernel...

This should w/a the issue in userspace

commit 154f7e9668bffdf565b6914a3a3e5bdfe17aa1b9
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Apr 18 09:28:02 2014 +0100

    sna: Do not reuse physical cursors for the kernel is broken
    
    Big bug in the kernel that prevents the sharing of cursors across pipes
    when they are backed by a phys_obj. To prevent hitting that bug, don't
    do that!
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77351
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

(In reply to comment #23)
> This should w/a the issue in userspace

Hmmmm, I have tested with 2d9ae02 (sna/video: Provide a fallback path for pwrite failure) and 3.14.1 kernel but the problem still occurs. I will add your debug patch from comment 4 and post the error log tomorrow if you did not comment otherwise until then.

patching does not seem to work anymore - quite understandable:

patching file sna_display.c
patch unexpectedly ends in middle of line
Hunk #1 succeeded at 87 with fuzz 1.


I also saw that you committed a new driver version. Will try 58a757b "sna: Do not reuse physical cursors for the kernel is broken, harder".

Tried to update bugzilla earlier, but it was playing up. Hopefully this is enough to prevent keep a cursor per pipe...

(In reply to comment #26)
> Tried to update bugzilla earlier, but it was playing up. 

Don't worry.

It works, I see no cursor problems with 3.14.1-1-ARCH and (II) intel(0): SNA compiled from 2.99.911-88-g58a757b !

Great! I will continue to use this driver version to see if I spot any problems.

As announced in my Comment 27 I continued to use the git driver: With driver version 58a757b I experienced a *freeze* of keyboard (even numlock LED did not respond to numlock key), mouse cursor and any screen change (old image is displayed) at not good reproducible conditions. I did not recognize a pattern when exactly it happens  (it occured only three times, I think it was never in idle but when starting a big program or doing some work inside a program). 

Access to the machine via ssh and shutdown or kill of the X server is possible.

I have changed to the old version 2.99.910 again since two days and I did not experience a freeze anymore (as I never did up to last week). Yes, the freezes may still have been caused by something else and randomness may have caused no freezes since two days.

I had a look in the logs (Xorg.0.log and my system's journalctl) but did not find any suspicious entry just before/at the freeze.

I know this problem report is far from helpful, but I hope it is not useless...

If it happens again, grab a stacktrace of X (gdb --pid=`pidof X` ; bt) and cat /proc/`pidof X`/stack and please file a new bug. I suspect that is a fun and separate issue.

okay, thanks Chris. I guess this one (cursors) is resolved, fixed then. Sure, I will post a new bug with a stack trace (swiching to the last git version now, again).

It's not entirely resolved yet. I've still got to get the patches to fix the kernel from giving stale pointers to the hardware upstream.

okay, sorry about the closing. 

The freeze issue is now at bug 77975 ("X freezes for no apparent reason and reproducability").

Please test http://mid.gmane.org/1400071997-30498-1-git-send-email-chris@chris-wilson.co.uk

(In reply to comment #33)
> Please test
> http://mid.gmane.org/1400071997-30498-1-git-send-email-chris@chris-wilson.co.
> uk

Poke.

(In reply to comment #34)
> (In reply to comment #33)
> > Please test
> > http://mid.gmane.org/1400071997-30498-1-git-send-email-chris@chris-wilson.co.
> > uk
> 
> Poke.

Sorry, did not notice due to very short new text here (I do not use the default email notification).  

Did you mean me to test? If so - what and how?

On Tue, May 20, 2014 at 12:44 AM,  <bugzilla-daemon@freedesktop.org> wrote:
> --- Comment #35 from CarlEitsger <4607vrfcr84spd21f08@weg-werf-email.de> ---
> (In reply to comment #34)
>> (In reply to comment #33)
>> > Please test
>> > http://mid.gmane.org/1400071997-30498-1-git-send-email-chris@chris-wilson.co.
>> > uk
>>
>> Poke.
>
> Sorry, did not notice due to very short new text here (I do not use the default
> email notification).
>
> Did you mean me to test? If so - what and how?

Apply patch to kernel, compile and test it.
-Daniel

(In reply to comment #36)
> Apply patch to kernel, compile and test it.
> -Daniel

Sorry, kernel compiling/patching is outside my capabilities - currently. I only can compile/patch the graphics driver xf86-video-intel.git. Maybe Chris can test it (on his spare 855gm hardware).

commit 00731155a73020c8e3c215723be193f96b4fcb1f
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed May 21 12:42:56 2014 +0100

    drm/i915: Fix dynamic allocation of physical handles
    
    A single object may be referenced by multiple registers fundamentally
    breaking the static allotment of ids in the current design. When the
    object is used the second time, the physical address of the first
    assignment is relinquished and a second one granted. However, the
    hardware is still reading (and possibly writing) to the old physical
    address now returned to the system. Eventually hilarity will ensue, but
    in the short term, it just means that cursors are broken when using more
    than one pipe.
    
    v2: Fix up leak of pci handle when handling an error during attachment,
    and avoid a double kmap/kunmap. (Ville)
    Rebase against -fixes.
    
    v3: And fix the error handling added in v2 (Ville)
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77351
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
    Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
    Cc: Jani Nikula <jani.nikula@linux.intel.com>
    Cc: stable@vger.kernel.org
    Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>