call xf86SetDDCproperties() after compat_output change
Submitted by vdb128
Assigned to Keith Packard @keithp
Description
Solves the xserver/hw/xfree86/modes scrn->monitor and scrn->modes inconsistency.
The commit 37d956e3 into 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 so then the output EDID is copied into the screen monitor properties, through xf86SetDDCproperties() and xf86EdidMonitorSet(). This saved screen monitor EDID can be queried via
xprop -root XFree86_DDC_EDID1_RAWDATA
Tracing the call tree:
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)) ----> 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:
- in the Section "Device" add the Option "ModeDebug" "on",
- connect a single EDID monitor,
- ~# X :1 -retro
- 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@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@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
- output == NULL ==> compat == -1
- 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.
Version: git