Getting the big picture from config file for driver configuration & validation

[Michel Hubert]

Following our discussions, we are submitting this patch to you.

1) It allows the driver to use the xf86configptr so that it can
effectively and cleanly manage the shared (and limited) ressources for
its device.

2) It fixes a bug in order to be able to access the Vendor section.

Finally, it would be useful to have generic helper functions such as
xf86findVendorSubSection(const char* ident, XF86ConfVendSubPtr) for the
vendor section...

Please, let me what you think of all this...

Regards,

Michel


On Tue, 2005-03-15 at 16:55, Alex Deucher wrote:
> On 15 Mar 2005 16:08:06 -0500, Michel Hubert <mhubert at matrox.com> wrote:
> > Our driver architecture includes several layers but among them we have a
> > cross-platform layer and an xorg layer.
> > 
> > Suppose we have a triple head independant configuration with Xinerama.
> > In this case, we have the following steps:
> > 
> > PreInit() for Screen 0:
> > 1) Process & accumulate data configuration in Xorg layer (pScrn,
> > EntityPrivate data structure, ...).
> > 2) initialize and build resource usage and configuration in
> > cross-platform layer.
> > 3) validate request
> > 4) Fill in structures in the xorg layer and return
> > 
> > For brievety sake, this PreInit() process could be recapitulated like
> > this:
> > 
> > 1) Accumulate Screen0-Xorg
> > 2) Build Screen0-Cross0  *See note 1
> > 3) Validate
> > 4) Complete Screen0-Xorg
> > 
> > Note 1:  Screen0-Cross0 means instance 0 of CrossPlatform Layer data for
> > screen 0.
> > 
> > Now suppose PreInit() for this Screen succeeds, and a Screen 1 adds
> > configuration options that have an impact on Screen 0 assumptions:
> > 
> > 5) Accumulate Screen1-Xorg
> > 6) Detect Conflicts/Limitations
> > 7) Build Screen1-Cross0 & Screen0-Cross1
> > 8.1) Validate Screen1
> > 8.2) Re-validate Screen0
> > 9) If validate succeeds, destroy Screen0-Cross0 (and replace by
> > Screen0-Cross1)
> > 10) Adjust Screen0-Xorg with new settings according to new ressources
> > allocation.  See note 2.
> > 11) Complete Screen1-Xorg
> > 
> > *Note 2: This have never happened for us.  But consider we have to
> > switch from one PLL to another, the refresh rate that we validated in
> > previous PreInit() could be revised to a lower value, and have an impact
> > on mode timings...  Or hardware memory allocation scheme that requires
> > two heads to be contiguous...
> > 
> > Suppose PreInit() succeeds again, and a Screen 2 adds configuration
> > options that have an impact on Screen 0 & Screen 1 assumptions:
> > 
> > 12) Accumulate Screen2-Xorg
> > 13) Detect Conflicts/Limitations
> > 14) Build Screen2-Cross0 & Screen1-Cross1 & Screen0-Cross2
> > 15.1) Validate Screen2
> > 15.2) Re-Validate Screen1
> > 15.3) Re-Validate Screen0
> > 16) If validate succeeds, destroy Screen0-Cross1 & Screen1-Cross0 ( and
> > replace by Screen0-Cross2 & Screen1-Cross1 )
> > 17) Adjust Screen0-Xorg & Screen1-Xorg
> > 18) Complete Screen2-Xorg
> > 
> > Adjusting & revalidating previously validated pScrn is not clean.
> > That's not to say that some modified values might have been previously
> > cached by other modules in between two PreInit() calls.
> > 
> > The following solutions are possible:
> > - expose our users to the complexity of our hardware by forcing them to
> > describe Screens in configuration files from the most constraint one to
> > the least constraint one. (quite impossible to explain & cover all
> > cases)
> > - limit some of the functionnality we offer.
> > - or simply give us access to xf86configptr and let us do the dirty work
> > for you !
> > 
> > Let's see what the latter solution means:
> > 
> > A) Gather config file requirements for the whole device (e.g: give
> > access to xf86configptr for Video Drivers)
> > B) Accumulate Screen0-Xorg
> > C) Make educated choices related to ressources and build Screen0-Cross0
> > D) Validate
> > E) Complete Screen0-Xorg
> > 
> > F) Accumulate Screen1-Xorg
> > G) Make educated choices related to ressources and build Screen1-Cross0
> > H) Validate
> > I) Complete Screen1-Xorg
> > 
> > J) Accumulate Screen2-Xorg
> > K) Make educated choices related to ressources and build Screen2-Cross0
> > L) Validate
> > M) Complete Screen2-Xorg
> > 
> > >From a graphic driver standpoint, the fact is that we are managing
> > resources for a DEVICE, not simply independant Screens.  Why not giving
> > us all the information when need to accomplish this task ?
> > No need to say that we are most probably not the only graphic chip
> > vendor in this situation...
> 
> I agree.  I'd much rather deal with one instance of the driver to
> handle all attached outputs than the current system.
> 
> Alex
> 
> 
> > 
> > Thank you for your time,
> > 
> > Michel
> > 
> > On Tue, 2005-03-15 at 08:36, Alan Cox wrote:
> > > On Llu, 2005-03-14 at 22:45, Michel Hubert wrote:
> > > > For example, let's take a triple head configuration in Xinerama.  3
> > > > ScrnInfoRec are created, and PreInit() is called for each one.  Using
> > > > Entity Private Data, each one can know how many outputs have been
> > > > requested up to now and to some degree, what are the requested
> > > > features.  But this implies that the complete config validation has to
> > > > be reperformed each time a new pScrn is added and that adds a lot of
> > > > unncessary complexity.
> > >
> > > It hurts performance at startup which ought to be minor. I'm not so sure
> > > about the complexity side although I've no idea how your driver looks.
> > >
> > > If you've got a single routine that takes a list of screens and
> > > validates them then you end up calling it three times, each with more
> > > data than the last but the routine itself is no more complex than it has
> > > to be anyway ?
> > >
> > --
> > Michel Hubert <mhubert at matrox.com>
> > Matrox Graphics Inc.
> > 
> > _______________________________________________
> > xorg mailing list
> > xorg at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/xorg
> >
-- 
Michel Hubert <mhubert at matrox.com>
Matrox Graphics Inc.
-------------- next part --------------
--- xc_orig/programs/Xserver/hw/xfree86/common/Imakefile	2005-03-23 11:25:50.000000000 +0000
+++ xc/programs/Xserver/hw/xfree86/common/Imakefile	2005-03-23 11:26:18.000000000 +0000
@@ -288,3 +288,5 @@
 InstallDriverSDKNonExecFile(xf86xv.h,$(DRIVERSDKINCLUDEDIR))
 InstallDriverSDKNonExecFile(xf86xvmc.h,$(DRIVERSDKINCLUDEDIR))
 InstallDriverSDKNonExecFile(xisb.h,$(DRIVERSDKINCLUDEDIR))
+InstallDriverSDKNonExecFile(xf86Config.h,$(DRIVERSDKINCLUDEDIR))
+
--- xc_orig/programs/Xserver/hw/xfree86/parser/Imakefile	2005-03-23 11:35:40.000000000 +0000
+++ xc/programs/Xserver/hw/xfree86/parser/Imakefile	2005-03-23 11:34:28.000000000 +0000
@@ -52,3 +52,6 @@
 AllTarget(ProgramTargetName(cpconfig))
 DependTarget()
 
+InstallDriverSDKNonExecFile(xf86Optrec.h,$(DRIVERSDKINCLUDEDIR))
+InstallDriverSDKNonExecFile(xf86Parser.h,$(DRIVERSDKINCLUDEDIR))
+
--- xc_orig/programs/Xserver/hw/xfree86/parser/Vendor.c	2005-03-23 12:02:23.000000000 +0000
+++ xc/programs/Xserver/hw/xfree86/parser/Vendor.c	2005-03-23 12:02:54.000000000 +0000
@@ -87,7 +87,7 @@
 			ptr->vs_comment = xf86addComment(ptr->vs_comment, val.str);
 			break;
 		case IDENTIFIER:
-			if (xf86getSubToken (&(ptr->vs_comment)))
+			if (xf86getSubToken (&(ptr->vs_comment)) != STRING)
 				Error (QUOTE_MSG, "Identifier");
 			if (has_ident == TRUE)
 				Error (MULTIPLE_MSG, "Identifier");
--- xc_orig/programs/Xserver/hw/xfree86/parser/xf86Optrec.h	2005-03-23 11:48:05.000000000 +0000
+++ xc/programs/Xserver/hw/xfree86/parser/xf86Optrec.h	2005-03-23 11:48:22.000000000 +0000
@@ -60,7 +60,10 @@
  */
 #ifndef _xf86Optrec_h_
 #define _xf86Optrec_h_
+
+#ifndef IN_MODULE
 #include <stdio.h>
+#endif
 
 /* 
  * all records that need to be linked lists should contain a GenericList as