[PATCH] xserver/hw/modes compat_output xf86SetDDCproperties()

[vdb at picaros.org]

The commit 37d956e3ac9513b74078882dff489f9b0a7a5a28 into
xserver/hw/modes/xf86Crtc.c xf86CrtcConfigInit() modified the
initialization of config->compat_output from default 0 to -1.

This change exposes a bug.  During initial configuration a monitor
detection is attempted and if an EDID block is available the driver
calls xf86OutputSetEDID().  There the output in process is checked for
equality to the compat_output, and if equal then EDID is additionally
copied into the screen monitor properties, through
xf86SetDDCproperties() and xf86EdidMonitorSet().  Tracing:

xf86InitialConfiguration(ScrnInfoPtr scrn, Bool canGrow)
-> xf86ProbeOutputModes(scrn, width, height);
  --> output_modes = (*output->funcs->get_modes) (output);
    ---> xf86OutputSetEDID(xf86OutputPtr output, xf86MonPtr edid_mon)
      ----> if (output == xf86CompatOutput(scrn))    XX now bad dereference
        -----> return config->output[config->compat_output];
      ----> xf86SetDDCproperties(scrn, edid_mon);    XX for screen monitor
-> xf86SetScrnInfoModes(scrn);
  --> output = SetCompatOutput(config);              XX sets compat_output
    ---> config->compat_output = compat;

Hence during initial boot xf86SetDDCproperties() used to be called for
output 0 but is now never called since there is no output -1.

To reproduce:
1. in the Section "Device" add the Option "ModeDebug" "on",
2. connect a single EDID monitor,
3. ~# X :1 -retro
4. un- and replug the monitor.

Xorg.1.log:

[347954.434] (II) RADEON(0): Output VGA-0 has no monitor section
[347954.434] (II) RADEON(0): xf86InitialConfiguration config->compat_output=-1
[347954.434] (**) RADEON(0): Option "ModeDebug" "on"
[347954.466] (II) RADEON(0): EDID for output DVI-0
...
[347954.467] (II) RADEON(0): 	00363230303036313959420a2020003f
[347954.467] (II) RADEON(0): Not using mode "720x576 at 25i" (hsync out of range)
...

XX disconnect
[347977.171] (II) RADEON(0): EDID for output DVI-0
[347977.173] (II) RADEON(0): EDID for output HDMI-0
...

XX reconnect
[347992.820] (II) RADEON(0): EDID for output DVI-0
...
[347992.821] (II) RADEON(0): 	00363230303036313959420a2020003f
XX xf86SetDDCproperties() -> xf86EdidMonitorSet() -> xf86DDCGetModes()
[347992.821] (II) RADEON(0): EDID vendor "NEC", prod id 26288
[347992.821] (II) RADEON(0): Using hsync ranges from config file
[347992.821] (II) RADEON(0): Using vrefresh ranges from config file
[347992.821] (II) RADEON(0): Printing DDC gathered Modelines:
...
[347992.821] (II) RADEON(0): Not using mode "720x576 at 25i" (hsync out of range)
...

So consistency between scrn->monitor and scrn->modes requires a call
sequence xf86SetDDCproperties() followed by xf86SetScrnInfoModes().
Two places seem fit: at the begin of xf86SetScrnInfoModes() or at the
end of SetCompatOutput().

This patch takes the latter route.  It combines the if-then-else tree
for readability, but execution is unchanged since

1. output == NULL  ==>  compat = -1
2. num_output > 0  ==>  config->output[0 .. (num_output-1)] != NULL

Also, the compat_output preset in xf86TargetFallback() is removed
since its selection differs from the SetCompatOutput() logic.

Signed-off-by: Servaas Vandenberghe
---
 hw/xfree86/modes/xf86Crtc.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
index f9ae465..1d15a46 100644
--- a/hw/xfree86/modes/xf86Crtc.c
+++ b/hw/xfree86/modes/xf86Crtc.c
@@ -1832,17 +1832,21 @@ SetCompatOutput(xf86CrtcConfigPtr config)
     }
 
     if (compat >= 0) {
-        config->compat_output = compat;
+        if (compat != config->compat_output) {
+            config->compat_output = compat;
+            /* Set screen scrn->monitor info. */
+            xf86SetDDCproperties(output->scrn, output->MonInfo);
+        }
     }
     else if (config->compat_output >= 0 && config->compat_output < config->num_output) {
         /* Don't change the compat output when no valid outputs found */
         output = config->output[config->compat_output];
     }
-
-    /* All outputs are disconnected, select one to fake */
-    if (!output && config->num_output) {
+    else if (config->num_output > 0) {
+        /* All outputs are disconnected, select one to fake */
         config->compat_output = 0;
         output = config->output[config->compat_output];
+        xf86SetDDCproperties(output->scrn, output->MonInfo);
     }
 
     return output;
@@ -2174,7 +2178,7 @@ xf86TargetFallback(ScrnInfoPtr scrn, xf86CrtcConfigPtr config,
     DisplayModePtr target_mode = NULL;
     Rotation target_rotation = RR_Rotate_0;
     DisplayModePtr default_mode;
-    int default_preferred, target_preferred = 0, o;
+    int default_preferred, target_preferred = 0, target_output, o;
 
     /* User preferred > preferred > other modes */
     for (o = -1; nextEnabledOutput(config, enabled, &o);) {
@@ -2189,12 +2193,12 @@ xf86TargetFallback(ScrnInfoPtr scrn, xf86CrtcConfigPtr config,
             target_mode = default_mode;
             target_preferred = default_preferred;
             target_rotation = config->output[o]->initial_rotation;
-            config->compat_output = o;
+            target_output = o;
         }
     }
 
     if (target_mode)
-        modes[config->compat_output] = target_mode;
+        modes[target_output] = target_mode;
 
     /* Fill in other output modes */
     for (o = -1; nextEnabledOutput(config, enabled, &o);) {
-- 
1.7.4.5