Bug 65212

Summary: call xf86SetDDCproperties() after compat_output change
Product: xorg Reporter: vdb128 <vdb128>
Component: Server/GeneralAssignee: Keith Packard <keithp>
Status: RESOLVED MOVED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: medium Keywords: patch
Version: git   
Hardware: All   
OS: All   
Whiteboard:
i915 platform: i915 features:
Bug Depends on: 65210    
Bug Blocks: 67069    
Attachments:
Description Flags
call xf86SetDDCproperties() from SetCompatOutput()
none
Xorg log file of EDID monitor dis- and reconnect. none

Description vdb128 2013-05-31 13:03:29 UTC
Solves the xserver/hw/xfree86/modes scrn->monitor and scrn->modes
inconsistency.  

The commit 37d956e3ac9513b74078882dff489f9b0a7a5a28 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: 
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@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

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.
Comment 1 vdb128 2013-05-31 13:09:28 UTC
Created attachment 80095 [details] [review]
call xf86SetDDCproperties() from SetCompatOutput()
Comment 2 vdb128 2013-05-31 13:12:10 UTC
Created attachment 80096 [details]
Xorg log file of EDID monitor dis- and reconnect.

Search for 'EDID' and 'Printing DDC '.
Comment 3 GitLab Migration User 2018-12-13 22:28:34 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xorg/xserver/issues/441.

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.