Bug 70132

(In reply to comment #0)
> A regression between 2.20.19 and 2.99.903.

It's a too wide range, let's narrow it: between 2.99.902-5-gf25235a and 2.99.903.

Hmm, cannot be changed?

Let's start with Xorg.0.log, xrandr --verbose before and after.

Created attachment 87117 [details]
xorg.log -- good

(In reply to comment #2)
> Hmm, cannot be changed?

When I move them in kscreen configurator, LVDS is still a clone of the left top of DP2. If I move LVDS right of DP2, it corrupts whole screen. Windows are half-size, the other half of DP2 is black. LVDS is black too.

Created attachment 87118 [details]
xorg.log -- bad

Created attachment 87119 [details]
xrandr -- good

Created attachment 87121 [details]
xranrd -- bad

xrandr --output DP2 --above LVDS1
X Error of failed request:  BadMatch (invalid parameter attributes)
  Major opcode of failed request:  140 (RANDR)
  Minor opcode of failed request:  21 (RRSetCrtcConfig)
  Serial number of failed request:  48
  Current serial number in output stream:  48

Mind attaching the Xorg.0.log from the last-known-good version?

There are a couple of commits that touch CRTC, but not that worrying. A git bisect would be very welcome, as would an "xtrace xrandr --output DP2 --above LVDS1".

The oddity in the xrandr is that the current mode of the LVDS is not listed. I presume it is the duplicate mode that fell off the bottom.

(In reply to comment #10)
> The oddity in the xrandr is that the current mode of the LVDS is not listed.
> I presume it is the duplicate mode that fell off the bottom.

Odds are that I downgraded to a previously working ddx and it still does not work. Only a very long version works.

Where to find xtrace?

I updated a kernel too. From 3.11.2 to 3.11.3. Any ideas:

Daniel Vetter (3):
      drm/i915: fix hpd work vs. flush_work in the pageflip code deadlock
      drm/i915: fix gpu hang vs. flip stall deadlocks
      drm/i915: fix wait_for_pending_flips vs gpu hang deadlock

Jani Nikula (2):
      drm/i915: try not to lose backlight CBLV precision
      drm/i915: do not update cursor in crtc mode set

Ville Syrjälä (1):
      drm/i915: Don't enable the cursor on a disable pipe

(In reply to comment #11)
> Where to find xtrace?

I found one...

> I updated a kernel too. From 3.11.2 to 3.11.3. Any ideas:

Actually from 3.11.1. So one more i915 patch:
Imre Deak (1):
      drm/i915: make user mode sync polarity setting explicit

And sure, downgrading the kernel back to 3.11.1 makes it work...

Huh. I would have said that it was a ddx problem... If you have the time to bisect both the ddx and the kernel, that would be much appreciated.

(In reply to comment #14)
> Huh. I would have said that it was a ddx problem... If you have the time to
> bisect both the ddx and the kernel, that would be much appreciated.

Hmm, it disappeared with 3.11.4.

(In reply to comment #15)
> (In reply to comment #14)
> > Huh. I would have said that it was a ddx problem... If you have the time to
> > bisect both the ddx and the kernel, that would be much appreciated.
> 
> Hmm, it disappeared with 3.11.4.

Not really, it is kind of random and not dependent on the kernel. I think it happens after I update ddx and restart X server. But not every time...

Let me watch it for some time.

The modes come from two places. The first is read back from the crtc, the second is read back from the output. They should be identical in the kernel. They look identical in xrandr, so I'm trying to figure out the magic that is used by RandR to identify the current mode on the output.

The downside is that we might end up with two nearly identical modes listed in xrandr, but it will stop xrandr from bugging out with an error:

commit 6fda305e2f2f991b39d09e67d0b17c8c3d50f9a4
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed Oct 9 15:59:42 2013 +0100

    sna: Append the current mode to the output list if not found
    
    If for some reason the current mode on the CRTC (inherited most likely
    from fastboot) doesn't match any of the modes reported by the output, we
    end up with a stray mode that the client cannot control.
    
    Reported-by: Jiri Slaby <jirislaby@gmail.com>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70132
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

As to why we end up with two almost identical modes - we can leave that for the masochists...

Created attachment 87358 [details]
xranrd -- still bad

Should it work with 2.99.904? It does not. xrandr from that session attached.

Bisection not so successful (it pretty narrows down the range, but there are still some commits), as X crashes with a series of commits (skipped).

# bad: [db086d02affe5109202dc9ee06c9e728a3ff0f3f] sna: Do a quick pass on dirty damage before reduction
# good: [a88a9b9a59fa2d5fd427fa6e1f74fb9844379264] 2.20.19 release
git bisect start 'HEAD' '2.20.19'
# skip: [2f2f439c9cfdd394ad2b9a125db51e2dba7d3ff7] sna: Fake the output physical width/height from the CRTC size
git bisect skip 2f2f439c9cfdd394ad2b9a125db51e2dba7d3ff7
# good: [f57a65c35268f215c17f1a02b3af50a6678ce3c1] sna: Correct assertions to allow discarding of cpu hint for inplace ops
git bisect good f57a65c35268f215c17f1a02b3af50a6678ce3c1
# bad: [0cd154039ab02799dc972d93c415e762226df1aa] 2.21.14 release
git bisect bad 0cd154039ab02799dc972d93c415e762226df1aa
# bad: [626b5e541663f838475eaef2c1bc3ae4d4848165] sna: Add debug control for disabling accelerated GetImage
git bisect bad 626b5e541663f838475eaef2c1bc3ae4d4848165
# good: [a86b217934a9a16843ac17b38cd5b61393f65aa0] sna: Mark the base bo as unreusable for temporary mappings
git bisect good a86b217934a9a16843ac17b38cd5b61393f65aa0
# good: [98feba417c87dce020df6a6ed157e3546591d8e7] sna: Drop master when closing the screen
git bisect good 98feba417c87dce020df6a6ed157e3546591d8e7
# skip: [18ee5c20d5f8f1752b89f2377e9b9c3f1c140d53] sna: We can read from cloned pixmaps inplace - so long as we don't write
git bisect skip 18ee5c20d5f8f1752b89f2377e9b9c3f1c140d53
# bad: [7ce487617445c81f0178823de8896a2b73bbaaf1] sna: Trim the large object threshold
git bisect bad 7ce487617445c81f0178823de8896a2b73bbaaf1
# skip: [491f4fab21df97af1dae92eaf30e76ff62920fb2] sna: Correct typo s/\j/\n/
git bisect skip 491f4fab21df97af1dae92eaf30e76ff62920fb2
# good: [e033a28bcb37867f3c947475714e1ef45c868825] sna: Optimize clears to white
git bisect good e033a28bcb37867f3c947475714e1ef45c868825
# skip: [8f6dcc15c8d6ea726a10ef5df18536f8c769291e] sna: Detect and handle cloned pixels for manual tiled uploads
git bisect skip 8f6dcc15c8d6ea726a10ef5df18536f8c769291e
# good: [3f33abee370bb1ce60bca91f29affc62d06b0bad] sna: Do not force creation of CPU maps on !llc
git bisect good 3f33abee370bb1ce60bca91f29affc62d06b0bad
# skip: [591981a36c333cdf08a91f28fb9670a62b95d31a] sna: Refactor freeing gpu_bo in manual tiled upload
git bisect skip 591981a36c333cdf08a91f28fb9670a62b95d31a
# good: [362b0dc6a3b1692b752d8f250075ccc81debfca3] sna/gen4+: Fix determination of intermediate extents
git bisect good 362b0dc6a3b1692b752d8f250075ccc81debfca3
# skip: [36993618b4c5d4fa1fde7800eaaa711b074bd623] sna: Tweak ordering of userptr temporary mappings for uploads
git bisect skip 36993618b4c5d4fa1fde7800eaaa711b074bd623
# skip: [01ce3ef9f12d3b6541e166f22e1ae67033237159] sna: Set RR_Rotate_0 instead of 0 as desired initial rotation
git bisect skip 01ce3ef9f12d3b6541e166f22e1ae67033237159
# skip: [c6ad9861f00a3166fd89a7c43d602a4c2a5bde24] sna: Fix DBG printing of can_upload_tiled_x()
git bisect skip c6ad9861f00a3166fd89a7c43d602a4c2a5bde24
# bad: [6493c8c65f93ad2554c2512a07ba640e966fd026] sna: Implement memcpy_from_tiled functions (for X-tiling only atm)
git bisect bad 6493c8c65f93ad2554c2512a07ba640e966fd026
# skip: [d7be3df2fe632bbc8e4f09709cf3cf7a5ef61015] sna: Se the default gamma if left uninitialized by the driver
git bisect skip d7be3df2fe632bbc8e4f09709cf3cf7a5ef61015
# skip: [c5651254c3c152daf1ed79073779c1bed6ed0a9b] sna: Fallback to xf86InitialConfiguration if nothing is connected
git bisect skip c5651254c3c152daf1ed79073779c1bed6ed0a9b
# skip: [5124f35168c0cc162cf5e8bcaafdf756945f6ca9] sna: Support operations inplace on CPU mappings of a region
git bisect skip 5124f35168c0cc162cf5e8bcaafdf756945f6ca9
# skip: [263e87d5e1915e6c40fa8bc1b325a36f21f92b30] sna: Set the current mode when initialising CRTCs
git bisect skip 263e87d5e1915e6c40fa8bc1b325a36f21f92b30
# skip: [a40dd0ed6db35c891710199cc9d18345ab53b397] sna: Explicitly initialise the probed transform for a CRTC
git bisect skip a40dd0ed6db35c891710199cc9d18345ab53b397
# bad: [60d716b53993b08a2a00c22f523c575e62e0a18d] sna: Add the probed CRTC mode to the list of output modes
git bisect bad 60d716b53993b08a2a00c22f523c575e62e0a18d
# skip: [8a6a21bff86100144ba7960fc32a299ac54ada83] sna: Use the existing configuration for initial modes
git bisect skip 8a6a21bff86100144ba7960fc32a299ac54ada83
# only skipped commits left to test
# possible first bad commit: [60d716b53993b08a2a00c22f523c575e62e0a18d] sna: Add the probed CRTC mode to the list of output modes
# possible first bad commit: [263e87d5e1915e6c40fa8bc1b325a36f21f92b30] sna: Set the current mode when initialising CRTCs
# possible first bad commit: [2f2f439c9cfdd394ad2b9a125db51e2dba7d3ff7] sna: Fake the output physical width/height from the CRTC size
# possible first bad commit: [5124f35168c0cc162cf5e8bcaafdf756945f6ca9] sna: Support operations inplace on CPU mappings of a region
# possible first bad commit: [18ee5c20d5f8f1752b89f2377e9b9c3f1c140d53] sna: We can read from cloned pixmaps inplace - so long as we don't write
# possible first bad commit: [36993618b4c5d4fa1fde7800eaaa711b074bd623] sna: Tweak ordering of userptr temporary mappings for uploads
# possible first bad commit: [c6ad9861f00a3166fd89a7c43d602a4c2a5bde24] sna: Fix DBG printing of can_upload_tiled_x()
# possible first bad commit: [591981a36c333cdf08a91f28fb9670a62b95d31a] sna: Refactor freeing gpu_bo in manual tiled upload
# possible first bad commit: [8f6dcc15c8d6ea726a10ef5df18536f8c769291e] sna: Detect and handle cloned pixels for manual tiled uploads
# possible first bad commit: [d7be3df2fe632bbc8e4f09709cf3cf7a5ef61015] sna: Se the default gamma if left uninitialized by the driver
# possible first bad commit: [491f4fab21df97af1dae92eaf30e76ff62920fb2] sna: Correct typo s/\j/\n/
# possible first bad commit: [a40dd0ed6db35c891710199cc9d18345ab53b397] sna: Explicitly initialise the probed transform for a CRTC
# possible first bad commit: [01ce3ef9f12d3b6541e166f22e1ae67033237159] sna: Set RR_Rotate_0 instead of 0 as desired initial rotation
# possible first bad commit: [c5651254c3c152daf1ed79073779c1bed6ed0a9b] sna: Fallback to xf86InitialConfiguration if nothing is connected
# possible first bad commit: [8a6a21bff86100144ba7960fc32a299ac54ada83] sna: Use the existing configuration for initial modes

Ah, I see the difference. The mode reported by the EDID has hsync/vsync flags, the currently active mode reported by the kernel does not.

I think this should fix up my mistake in adding the current CRTC mode to the output list:

commit 33be42bf509b6d57face6b8d99d72dd5f6c90900
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Sun Oct 20 09:03:47 2013 +0100

    sna: Fix the addition of the current output Mode to the probed lists
    
    We need to add the current mode to the Modes list and not directly to
    the output->probed_modes as that gets overwritten.
    
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

But I fear the root cause is much deeper in the kernel. Can you please attach a drm.debug=6 for the boot sequence so we can try and spot where the flags disappear?

Created attachment 87865 [details]
drm.debug=6 dmesg

(In reply to comment #21)
> I think this should fix up my mistake in adding the current CRTC mode to the
> output list:

Seems to be fixed too.

> But I fear the root cause is much deeper in the kernel. Can you please
> attach a drm.debug=6 for the boot sequence so we can try and spot where the
> flags disappear?

Attached.

Reviewing the code, I cannot spot a location where the flags would be dropped, and certainly all the modelines in the dmesg have the right flags. One extra sanity check would be to dump everything the kernel reports so that I can see if the DDX see those flags returned correctly for the current CRTC mode.

My belief that this is just a quirk in how X presents the mismatching mode. Inspecting what is returned by the kernel for both the CRTC / EDID modes, it looks like we do correctly reconstruct the appropriate randr modes.

Nope, it still does not work.

Is xrandr still showing garbage?

(In reply to comment #26)
> Is xrandr still showing garbage?

Dunno, you have to fix bug 70461 first ;). (It crashes during startup.)

Created attachment 88449 [details]
xrandr still bad

Hmm, I have a cheeky idea. I'm not going to like it much.

Still broken with latest SNA?

It's not an SNA bug...

(Oops was thinking of the other multiple monitor regressions in the kernel).

The issue here is that state we read back from the CRTC and create a mode from does not always correspond with the mode we create from the EDID mode list - notably the CRTC flags go astray.

I've reviewed the mode handling code in sna and the kernel, and found no reason at all for the mode->flags to disappear. If you can still trigger it, then please do grab a full debug log so we have what we receive from the kernel. Then if that mismatches what is printed out by xrandr, we can move on to the next link in the chain.

This should fix that stray mode:

commit 1c5ccf5d9d8beb7e8343eb2d07bbf97f53c1a224
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu Feb 13 01:06:41 2014 +0000

    sna: Always assign a name to the modes
    
    In some cases, such as querying the mode from the CRTC, we may not have
    a name associated with the mode. However, RandR always expects a valid
    name. To satisfy this requirement, always generate the canonical mode
    name if no other is specified.
    
    References: https://bugs.freedesktop.org/show_bug.cgi?id=70132
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

There's still one issue that the mode recovered from the CRTC doesn't match any known modes associated with the output (and so we have to add it for RandR).

What's the status of the automatic multihead setup on your machine?

Time to poke the bear.