I have an xorg.conf setup on a laptop with settings such that the CRT output is always on, using default modes: the "Device" section contains Option "MonitorLayout" "LVDS, CRT" Option "MergedFB" "on" Option "CRT2Hsync" "50-92" Option "CRT2VRefresh" "50-85" Option "MetaModes" "1400x1050 1152x864 1024x768 800x600" Now Xrandr reports the resolutions in MetaModes, but gives bizarre negative refresh rates: SZ: Pixels Physical Refresh *0 1400 x 1050 ( 474mm x 356mm ) *-8255 1 1152 x 864 ( 474mm x 356mm ) -8269 2 1024 x 768 ( 474mm x 356mm ) -8265 3 800 x 600 ( 474mm x 356mm ) -8277 This is on a ATI Technologies, Inc. Radeon R250 Lf [Radeon Mobility 9000 M9]
Created attachment 5677 [details] Xorg.log
Created attachment 5678 [details] xorg.conf
I found the same problem when using GNOME/JDS, where, if MergedFB is used, you will find negative refresh rates in desktop resolution selection screen, resulting in not being able to select resolutions. The cause of the problem seems to be inappropriate setting of clock for the merged mode, as seen in this portion of code mode->Clock = (((i->Clock >> 3) + i->HTotal) << 16) | ((j->Clock >> 2) + j->HTotal); mode->Clock ^= ((i->VTotal << 19) | (j->VTotal << 3)); in RADEONCopyModeNLink() of radeon_mergedfb.c, where i and j represent two modes to be merged, and the clock generated is a bogus negative number. I don't know what this calculation means, it starts showing up in version 1.5. The clock is set to 0 prior to that. For a testing (in clone mode), using mode->Clock = i->Clock seems to fix the problem.
Alex, please remind us of the reasons behind this. IIRC it's related to preventing metamodes from getting discarded under some circumstances.
(In reply to comment #4) > Alex, please remind us of the reasons behind this. IIRC it's related to > preventing metamodes from getting discarded under some circumstances. Henry and I were actually just discussing this. Unfortunately, I don't recall the exact circumstance where it was needed, however, IIRC, xrandr or xvidmode would discard modes that looked the same but were not. so for example, if you had the following metamodes: crtc1: 1024x768@60 crtc2: 1024x768@60 crtc1: 1024x768@75 crtc2: 1024x768@60 crtc1: 1024x768@60 crtc2: 1024x768@75 crtc1: 1024x768@60 crtc2: 1024x768@85 potentially they could all end up looking like the same mode (1024x768@60 or 2048x768@60, etc.) so whatever app you are using to switch modes may discard one or more of them thinking they are the same. there's probably a nicer way to do it, but I had never run into any problems so I never did anything about it.
So what is the recommended way out? Is the mode->Clock = i->Clock solution going to trigger weird behaviour in other areas or is this the best short-term patch? Or is a better solution in the works?
mode->Clock = i->Clock is not a solution, but a workaround to not disply negative refresh rates so that resolution selection will function in GNOME. In terms of unique representation of metamodes, we are more concerned about situation where merged modes sharing same mode name, but different contents. An example of this is CRT1 CRT2 merged mode name 1280x1024 1280x10241 1280x1024 1280x1024 1024x768 1280x1024 However, the curent "Clock setting" mechanism does not solve this problem, at least on GNOME resolution screen, only one "1280x1024" is listed.
Created attachment 5972 [details] [review] Fix of negative refresh rates from merged modes
In randr code of xf86RandR.c, refresh rate is calculated from clock only when mode's refresh rate is not provided in mode info, as seen in code below: static int xf86RandRModeRefresh (DisplayModePtr mode) { if (mode->VRefresh) return (int) (mode->VRefresh + 0.5); else return (int) (mode->Clock * 1000.0 / mode->HTotal / mode->VTotal + 0.5); } That means, in radeon_mergedfb.c if mode's refresh is provided before the "sophisticated fake DotClock" is set we can get correct refresh rate (corresponding to the CRT1). This fix keeps the current clock setting for merged modes, although it is not yet proved this clock setting can ensure unique merged mode representation, at least required by xrandr. A patch is attached.
Created attachment 6296 [details] [review] Hash fake dotclocks to <2^15 and avoid clipping The patch in attachment 5972 [details] [review] means metamodes with the same size and CRTC1 refresh rate cannot be differentiated -- a major drawback. This attachment provides an alternative solution: hash the fake dotclock to make it less than 2^16, so it will fit into xrandr's positive shorts. Two random metamodes will collide with probability ~1/2^16, which is the best we can expect via this approach. The attachment also solves another issue with the original code: when xf86RandR.c/xf86RandRModeRefresh() converts dotclock to refresh rate, the division loses the least significant bits (e.g., CRTC1's dotlock). I've seen several reasonable metamodes that collide for that reason. The formula used here avoids that problem. Tested with git head, works perfectly.
This is a major issue for many users, since it breaks RandR. Isn't the above two-line patch a satisfactory workaround, until the new XrandR matures?
(In reply to comment #11) > This is a major issue for many users, since it breaks RandR. > > Isn't the above two-line patch a satisfactory workaround, until the new XrandR > matures? The patch looks fine. Henry had mentioned committing the fix (along with serveral other of his patches) so I was going to leave it to him, but I can commit it if he's not going to get to it soon.
Didn't make it to 6.6.2... Can the patch in attachment 6296 [details] [review] be comitted?
I see you've applied the first patch (git commit e742aeb28c7d9d6e75932c408bcc7c44af52e303). In comment #10 I've explained why that patch is insufficient and proposed an alternative one. Please reconsider.
I hate to nag, but is anyone looking a this? It seems to me like the wrong patch has been applied and shipped (comment #10, comment #14).
(In reply to comment #14) > I see you've applied the first patch (git commit > e742aeb28c7d9d6e75932c408bcc7c44af52e303). > > In comment #10 I've explained why that patch is insufficient and proposed an > alternative one. Please reconsider. I tried the patch in attachment 6296 [details] [review], but got large positive refresh rates (in the range of above thousands), is this expected ? Can someone else try it ?
Yes, patch 6296 is intended to create random-looking refresh rates between 0 and 32748. These are hashes of the metamode parameters. True, these fake refresh rates aren't very beautiful; but this is the best we can do to minimizes the chances of metamode collisions (same size and refresh rate) which render RANDR useless. To stress, this isn't academic: with patch 5972 I've seen perfectly sensible metamodes colliding on my laptop's X setup.
(In reply to comment #14) > I see you've applied the first patch (git commit > e742aeb28c7d9d6e75932c408bcc7c44af52e303). > > In comment #10 I've explained why that patch is insufficient and proposed an > alternative one. Please reconsider. I tried the patch in attachment 6296 [details] [review], but got large positive refresh rates (in the range of above thousands), is this expected ? Can someone else try it ?(In reply to comment #17) > Yes, patch 6296 is intended to create random-looking refresh rates between 0 and > 32748. These are hashes of the metamode parameters. > > True, these fake refresh rates aren't very beautiful; but this is the best we > can do to minimizes the chances of metamode collisions (same size and refresh > rate) which render RANDR useless. > > To stress, this isn't academic: with patch 5972 I've seen perfectly sensible > metamodes colliding on my laptop's X setup. I am concerned that large positive refresh rates up to 32748 may cause another bug to be generated that people don't know how to select a refresh rate because the refresh numbers don't make sense. There could be another way to increase the chance of differentiating meta modes having same size and CRTC1 refresh, yet to generate reasonable number of refresh rates, is to use the average refresh rate of the two modes: if (!(mode->VRefresh)) mode->VRefresh = (i->Clock * 1000.0 / i->HTotal / i->VTotal / 2) + (j->Clock * 1000.0 / j->HTotal / j->VTotal / 2); Plus, people are working on RandR version 1.2, I know they will address metamode issues like this, but don't know the details.
(In reply to comment #18) Since we're deep into kludge-land, how about a fake refresh rate of X*100+Y, where X is the refresh rate of CRTC1 and is the refresh rate of CRTC2? On a dual-head laptop, a typical value will be 6085 for 60Hz on LCD and 85 on the VGA port. And even if someone has a refresh rate a bit above 100Hz, the result will be human-decipherable (but take it modulo 2^15-1, just in case). This seems both more informative and less likely to collide than taking the average. It still ignores HTotal and VTotal (unlike patch 6296), but maybe that's a reasonable price to pay for human-readable values. Yes, keithp's RANDR 1.2 should clear away this mess, but it's some way off and we'll still have legacy applications to deal with.
Created attachment 7345 [details] [review] Patch based on comment #18 and comment #19 Per discussion with multinymous@gmail.com, and based on comment #19, this patch will generate a merged refresh rate representing a combination of refresh rates of CRTC1 and CRTC2, to reduce chance of collision.
This should be resolved in ati git now.
We should probably apply Henry or Shem's latest patch before we close this.
(In reply to comment #22) > We should probably apply Henry or Shem's latest patch before we close this. I will commit patch 7345 if no one opposes.
Patch 7345 works perfectly here, and (as expected) reveals the metamodes that were hidden by the first patch.
Henry go ahead and commit it.
I went ahead and committed this. Let me know if there are any issues.
Created attachment 7454 [details] [review] revision to previous patch 7435 This patch generates better results than previous patch 7345.
(In reply to comment #27) > Created an attachment (id=7454) [edit] > revision to previous patch 7435 > > This patch generates better results than previous patch 7345. committed.
The problem with all of this is we seem to fall over with the DIX randr code in the new server as it seems to calculate another refresh rate... which doesn't equal the refresh rate we give it ..
mergedfb has been replace by randr.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.