Bug 10620 - Disable on-chip RMX modes for DFP monitors
Summary: Disable on-chip RMX modes for DFP monitors
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/Radeon (show other bugs)
Version: 7.1 (2006.05)
Hardware: All Linux (All)
: medium major
Assignee: xf86-video-ati maintainers
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-04-11 20:01 UTC by LisaWu
Modified: 2008-12-02 23:57 UTC (History)
7 users (show)

See Also:
i915 platform:
i915 features:


Attachments
disable on-chip RMX for DFP monitor (2.22 KB, patch)
2007-04-11 20:20 UTC, LisaWu
no flags Details | Splinter Review
validate ddcmodes for DFP monitor by default (1000 bytes, patch)
2007-04-11 22:38 UTC, LisaWu
no flags Details | Splinter Review
Updated patch from LisaWu (2.18 KB, patch)
2007-04-16 08:10 UTC, Matthias Hopf
no flags Details | Splinter Review
xorg.conf file which will cause the video corruption on IBM server (5.80 KB, application/octet-stream)
2007-04-16 18:45 UTC, LisaWu
no flags Details
log file with corrupted video output (36.72 KB, application/octet-stream)
2007-04-16 18:55 UTC, LisaWu
no flags Details
log file with RMX disabled (35.67 KB, application/octet-stream)
2007-04-16 19:02 UTC, LisaWu
no flags Details
log file from benh on irc... (5.33 KB, text/plain)
2007-04-22 23:37 UTC, Dave Airlie
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description LisaWu 2007-04-11 20:01:43 UTC
on-chip RMX mode is only enabled when the driver thinks it’s driving a
digital panel which will cause poor quality on CRT. This is also an old day’s radeon driver preference that probably shouldn’t be used any more because almost all modern TMDS panels have internal scaler which usually can result in better quality than our internal RMX.
in some KVM systems, radeon driver use RMX modes for remote video by default, however, the stretched RMX modes can't be well support. 
to update our driver, we change digital pannel mode validation routine not to 
use RMX mode by default, but validate modes by matching modes from EDID info 
instead.
Comment 1 Benjamin Herrenschmidt 2007-04-11 20:07:19 UTC
If we're going to do that, we need to be careful and make sure we detect things like Apple flat panels who have no scaler and only support a fixed mode. In those cases, we must enable use of RMX.
Comment 2 LisaWu 2007-04-11 20:20:09 UTC
Created attachment 9571 [details] [review]
disable on-chip RMX for DFP monitor
Comment 3 Benjamin Herrenschmidt 2007-04-11 20:40:59 UTC
There are several issues with the patch:

 - You completely comment yout the ValidateFPMode call, instead of just calling it when more than one mode was found... that will make RMX not work on panels that don't support scaling like most Apple panels.

 - Your patch seems to change all sorts of unrelated bits, some of them possibly reverting fixes, or at least without any explanation of why you did those changes.  If some of these are fixes on their own, please post them as separate patches.
Comment 4 LisaWu 2007-04-11 22:38:28 UTC
Created attachment 9572 [details] [review]
validate ddcmodes for DFP monitor by default

do not use on-chip RMX modes for digital monitors by default.
only if one or no modes found by ddc modes, then on-chip RMX modes will be used instead.
Comment 5 Matthias Hopf 2007-04-16 08:10:14 UTC
Created attachment 9617 [details] [review]
Updated patch from LisaWu

This is an updated patch. It influences this particular IBM server and doesn't change behavior otherwise.

I would prefer to the variable ignoreRMX instead of IsIBMServer, and to make it (additionally) settable by and option. It's always better to track feature sets than to track specific hardware (IsDellServer is actually a very bad example). Also the patch has some whitespace problems.

Would a fixed version WRT the stated issues be ok for upstream?
Comment 6 Matthias Hopf 2007-04-16 09:10:30 UTC
The other two changes in the first patch are an obvious bug fix (linked list termination), and a signal polarity semantic change of which I'm a bit unsure whether it might have some regressions with monitors with broken DDC:

LisaWu:
on IBM Lewis server, if (d_timings->sync==3)is set,the signal polarity
restrictions are not considered by radeon driver,which cause video corruption
on remote video controller. when if(d_timing->sync == 3)is removed, the
required signal polarity is passed to the output video mode, thus the remote
video show correctly.

Benjamin, do you know whether this if-clause was introduced later, or whether it came with the original implementation? I've tracked it down until the initial git version from 2003-11-14, and even back then the whole clause was present as it is today.
Comment 7 Jerome Glisse 2007-04-16 10:54:09 UTC
I believe bugs #8541 is related to RMX, thus it exist monitor
out there that doesn't like the card RMX scaler, would be nice
if we can have some kind of policy to choose btw enabling or
disabling rmx. Is it possible to detect monitor without RMX
(like apple one) and enable RMX only for them ?
Comment 8 Dave Airlie 2007-04-16 14:04:04 UTC
can you post an Xorg.0.log from one of these machines?
Comment 9 LisaWu 2007-04-16 18:45:16 UTC
Created attachment 9620 [details]
xorg.conf file which will cause the video corruption on IBM server
Comment 10 LisaWu 2007-04-16 18:55:19 UTC
Created attachment 9621 [details]
log file with corrupted video output

refer to the log file, please notice that the on-chip RMX is enabled and the video output mode is 
(**) RADEON(0): *Mode "1024x768": 65.0 MHz (scaled from 0.0 MHz), 48.4 kHz, 60.0 Hz
(II) RADEON(0): Modeline "1024x768"   65.00  1024 1048 1184 1344  768 771 777 806
with no signal polarity defined.
this timing is not acceptable to the remote video for the IBM server.
Comment 11 LisaWu 2007-04-16 19:02:13 UTC
Created attachment 9622 [details]
log file with RMX disabled

when disable on-chip RMX in our driver, and remove "if(d_timings->sync == 3)"(mentioned in comment #6)
both remote video and local monitor show clear video out.
please refer to the log file to have a detailed information of the valid timings for the server. and please pay attention to the signal polarity of every valid mode.
Comment 12 Matthias Hopf 2007-04-17 00:58:50 UTC
Dave asked about a logfile of an Xserver with the card connected to a monitor w/o RMX scaler like the Apple one. So one of those (apparently rare) cases, where we still have to use the internal RMX of the gfx card.
Comment 13 LisaWu 2007-04-17 01:02:52 UTC
oops,misunderstood :(
Comment 14 Matthias Hopf 2007-04-17 01:33:32 UTC
(In reply to comment #6)
> LisaWu:
> on IBM Lewis server, if (d_timings->sync==3)is set,the signal polarity
> restrictions are not considered by radeon driver,which cause video corruption
> 
> Benjamin, do you know whether this if-clause was introduced later, or whether
> it came with the original implementation? I've tracked it down until the

I've further tracked down the if-clause. It has been introduced by Hui Yu, on 2002/07/11:

      - Fix Dell OEM VE card support
      - Add better clone mode support
      - Fix large panel (>= 1600x1200) detection and initialization problems
      - Remove "PanelSize" and "CrtScreen" options since they are no longer
        needed with new CloneMode and improved flat panel support
      - Add "DDCMode" option to detect and use DDC modes
      - Add "PanelOff" option to disable panel on laptops
      - Fix corrupted console problem
      - Other misc fixes
      (#A.1043, Hui Yu@ATI)

which is a huge 2279 lines patch, which basically introduces RMX and turns mode selection upside down. Also, the infamous IsDell logic was introduced.

All in all I think this change is safe.
Comment 15 Matthias Hopf 2007-04-19 02:57:23 UTC
Fixed in git.

We probably still need a better logic for when RMX should enabled and when not.
Comment 16 Roland Scheidegger 2007-04-19 05:50:41 UTC
(In reply to comment #15)
> Fixed in git.
> 
> We probably still need a better logic for when RMX should enabled and when not.
Isn't it possible to determine this just by the edid data? I think if there's a frequency range field, the monitor has a scaler and it's safe to disable RMX, and if there's no such field it doesn't (unless it's edid 1.0 but I guess only crts used that). Or are we talking about monitors with buggy (or no) edid data?
Comment 17 Dave Airlie 2007-04-22 23:37:02 UTC
The mode list handling patches seem to miss the fact that list if circular or not

they definitely regress things for benh...

Comment 18 Dave Airlie 2007-04-22 23:37:38 UTC
Created attachment 9690 [details]
log file from benh on irc...
Comment 19 LisaWu 2007-04-23 01:01:38 UTC
from the log, it seems radeon failed to read out the EDID information from the primary monitor which lead to a mode validation failure.
the final patch enables DDCModes to disable RMX mode.if you want to use on-chip RMX mode for LCD monitors, you may disable the DDCModes option and have another try.
Comment 20 Matthias Hopf 2007-04-23 08:50:27 UTC
The patches don't activate ddc_mode by default, so this shouldn't be the source for the regression.

Unfortunately benh isn't online on xorg-devel right now.

Commit 0abce69f0d826a7ca1a41d963cd4730b6e01c145 or 16ef77df4ebaf5ea13baa82972aaf98e71ac32ee is probably the culprit so if benh could test these two commits, it help be of great help.

(In reply to comment #17)
> The mode list handling patches seem to miss the fact that list if circular or
> not

I most certainly do not understand what you want to tell me.
Can the modelist be a circular list? In that case, how did it ever work, the mode list scanning end conditions certainly didn't test for that so far. Or only in a way that is so non-obvious that I still can see how.
Comment 21 Dave Airlie 2007-04-23 13:03:17 UTC
actually I relooked at the modelist handling and they looked correct to me, mode list handling is very arcane in X and whether something is a circular list or not isn't always obvious ...
Comment 22 Matthias Hopf 2007-04-25 03:26:19 UTC
(In reply to comment #21)
> actually I relooked at the modelist handling and they looked correct to me,

I assume you mean the old version, pre-commit 0abce69f?

> mode list handling is very arcane in X and whether something is a circular list
> or not isn't always obvious ...

Sure, but how does "p && p->next" check for circular lists? To me this stops on the previous to last entry of a regular linked list.
The other change (not going in steps of 2 over the list) has been proposed by Lisa, and it is changing code originally submitted by ATI.
Comment 23 Dave Airlie 2007-11-06 21:08:56 UTC
so the patch that got checked in for this bug seems to be in the tree anymore.. anyone got any way to test master on one of these machines to see if we regressed it?

Comment 24 Alex Deucher 2007-11-07 07:47:01 UTC
(In reply to comment #23)
> so the patch that got checked in for this bug seems to be in the tree anymore..
> anyone got any way to test master on one of these machines to see if we
> regressed it?
> 

In ati master we already turn rmx off by default for TMDS. Works fine on my DVI panels with or without RMX.  I suppose 6.6.x may need a fix though.
Comment 25 Alex Deucher 2008-12-02 23:57:41 UTC
closing due to lack of feedback.


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.